Moving log messages from bucketer to decision_service

Author: aliabbasrizvi

Diff for: optimizely/bucketer.py

@@ -38,41 +38,41 @@ def __init__(self):
     def _generate_unsigned_hash_code_32_bit(self, bucketing_id):
         """ Helper method to retrieve hash code.
 
-    Args:
-      bucketing_id: ID for bucketing.
+        Args:
+            bucketing_id: ID for bucketing.
 
-    Returns:
-      Hash code which is a 32 bit unsigned integer.
-    """
+        Returns:
+            Hash code which is a 32 bit unsigned integer.
+        """
 
         # Adjusting MurmurHash code to be unsigned
         return mmh3.hash(bucketing_id, self.bucket_seed) & UNSIGNED_MAX_32_BIT_VALUE
 
     def _generate_bucket_value(self, bucketing_id):
         """ Helper function to generate bucket value in half-closed interval [0, MAX_TRAFFIC_VALUE).
 
-    Args:
-      bucketing_id: ID for bucketing.
+        Args:
+            bucketing_id: ID for bucketing.
 
-    Returns:
-      Bucket value corresponding to the provided bucketing ID.
-    """
+        Returns:
+            Bucket value corresponding to the provided bucketing ID.
+        """
 
         ratio = float(self._generate_unsigned_hash_code_32_bit(bucketing_id)) / MAX_HASH_VALUE
         return math.floor(ratio * MAX_TRAFFIC_VALUE)
 
     def find_bucket(self, project_config, bucketing_id, parent_id, traffic_allocations):
         """ Determine entity based on bucket value and traffic allocations.
 
-    Args:
-      project_config: Instance of ProjectConfig.
-      bucketing_id: ID to be used for bucketing the user.
-      parent_id: ID representing group or experiment.
-      traffic_allocations: Traffic allocations representing traffic allotted to experiments or variations.
+        Args:
+            project_config: Instance of ProjectConfig.
+            bucketing_id: ID to be used for bucketing the user.
+            parent_id: ID representing group or experiment.
+            traffic_allocations: Traffic allocations representing traffic allotted to experiments or variations.
 
-    Returns:
-      Entity ID which may represent experiment or variation.
-    """
+        Returns:
+            Entity ID which may represent experiment or variation.
+        """
 
         bucketing_key = BUCKETING_ID_TEMPLATE.format(bucketing_id=bucketing_id, parent_id=parent_id)
         bucketing_number = self._generate_bucket_value(bucketing_key)
@@ -90,20 +90,21 @@ def find_bucket(self, project_config, bucketing_id, parent_id, traffic_allocatio
     def bucket(self, project_config, experiment, user_id, bucketing_id):
         """ For a given experiment and bucketing ID determines variation to be shown to user.
 
-    Args:
-      project_config: Instance of ProjectConfig.
-      experiment: Object representing the experiment for which user is to be bucketed.
-      user_id: ID for user.
-      bucketing_id: ID to be used for bucketing the user.
+        Args:
+            project_config: Instance of ProjectConfig.
+            experiment: Object representing the experiment or rollout rule in which user is to be bucketed.
+            user_id: ID for user.
+            bucketing_id: ID to be used for bucketing the user.
 
-    Returns:
-      Variation in which user with ID user_id will be put in. None if no variation.
-    """
+        Returns:
+            Variation in which user with ID user_id will be put in. None if no variation.
+        """
 
         if not experiment:
             return None
 
-        # Determine if experiment is in a mutually exclusive group
+        # Determine if experiment is in a mutually exclusive group.
+        # This will not affect evaluation of rollout rules.
         if experiment.groupPolicy in GROUP_POLICIES:
             group = project_config.get_group(experiment.groupId)
 
@@ -131,10 +132,6 @@ def bucket(self, project_config, experiment, user_id, bucketing_id):
         variation_id = self.find_bucket(project_config, bucketing_id, experiment.id, experiment.trafficAllocation)
         if variation_id:
             variation = project_config.get_variation_from_id(experiment.key, variation_id)
-            project_config.logger.info(
-                'User "%s" is in variation "%s" of experiment %s.' % (user_id, variation.key, experiment.key)
-            )
             return variation
 
-        project_config.logger.info('User "%s" is in no variation.' % user_id)
         return None

Diff for: optimizely/decision_service.py

@@ -274,6 +274,9 @@ def get_variation(self, project_config, experiment, user_id, attributes, ignore_
         variation = self.bucketer.bucket(project_config, experiment, user_id, bucketing_id)
 
         if variation:
+            self.logger.info(
+                'User "%s" is in variation "%s" of experiment %s.' % (user_id, variation.key, experiment.key)
+            )
             # Store this new decision and return the variation for the user
             if not ignore_user_profile and self.user_profile_service:
                 try:
@@ -283,6 +286,7 @@ def get_variation(self, project_config, experiment, user_id, attributes, ignore_
                     self.logger.exception('Unable to save user profile for user "{}".'.format(user_id))
             return variation
 
+        self.logger.info('User "%s" is in no variation.' % user_id)
         return None
 
     def get_variation_for_rollout(self, project_config, rollout, user_id, attributes=None):

Diff for: tests/test_bucketing.py

@@ -234,9 +234,6 @@ def test_bucket(self):
             )
 
         mock_config_logging.debug.assert_called_once_with('Assigned bucket 42 to user with bucketing ID "test_user".')
-        mock_config_logging.info.assert_called_once_with(
-            'User "test_user" is in variation "control" of experiment test_experiment.'
-        )
 
         # Empty entity ID
         with mock.patch('optimizely.bucketer.Bucketer._generate_bucket_value', return_value=4242), mock.patch.object(
@@ -252,7 +249,6 @@ def test_bucket(self):
             )
 
         mock_config_logging.debug.assert_called_once_with('Assigned bucket 4242 to user with bucketing ID "test_user".')
-        mock_config_logging.info.assert_called_once_with('User "test_user" is in no variation.')
 
         # Variation 2
         with mock.patch('optimizely.bucketer.Bucketer._generate_bucket_value', return_value=5042), mock.patch.object(
@@ -269,9 +265,6 @@ def test_bucket(self):
             )
 
         mock_config_logging.debug.assert_called_once_with('Assigned bucket 5042 to user with bucketing ID "test_user".')
-        mock_config_logging.info.assert_called_once_with(
-            'User "test_user" is in variation "variation" of experiment test_experiment.'
-        )
 
         # No matching variation
         with mock.patch('optimizely.bucketer.Bucketer._generate_bucket_value', return_value=424242), mock.patch.object(
@@ -289,7 +282,6 @@ def test_bucket(self):
         mock_config_logging.debug.assert_called_once_with(
             'Assigned bucket 424242 to user with bucketing ID "test_user".'
         )
-        mock_config_logging.info.assert_called_once_with('User "test_user" is in no variation.')
 
     def test_bucket__experiment_in_group(self):
         """ Test that for provided bucket values correct variation ID is returned. """
@@ -316,7 +308,6 @@ def test_bucket__experiment_in_group(self):
         mock_config_logging.info.assert_has_calls(
             [
                 mock.call('User "test_user" is in experiment group_exp_1 of group 19228.'),
-                mock.call('User "test_user" is in variation "group_exp_1_variation" of experiment group_exp_1.'),
             ]
         )
 
@@ -356,7 +347,6 @@ def test_bucket__experiment_in_group(self):
         mock_config_logging.info.assert_has_calls(
             [
                 mock.call('User "test_user" is in experiment group_exp_1 of group 19228.'),
-                mock.call('User "test_user" is in no variation.'),
             ]
         )
 
@@ -399,6 +389,5 @@ def test_bucket__experiment_in_group(self):
         mock_config_logging.info.assert_has_calls(
             [
                 mock.call('User "test_user" is in experiment group_exp_1 of group 19228.'),
-                mock.call('User "test_user" is in no variation.'),
             ]
         )