From 6722aad9fd9ce9825b24204206e9349cc588e882 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Fri, 28 Dec 2018 13:17:14 -0800 Subject: [PATCH 1/3] Introducing easier event tracking --- optimizely/event_builder.py | 19 +---- optimizely/optimizely.py | 34 +++----- tests/test_event_builder.py | 65 ++++------------ tests/test_optimizely.py | 149 +++++++++--------------------------- 4 files changed, 65 insertions(+), 202 deletions(-) diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index c726295f..6b690351 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -233,13 +233,12 @@ def _get_required_params_for_impression(self, experiment, variation_id): return snapshot - def _get_required_params_for_conversion(self, event_key, event_tags, decisions): + def _get_required_params_for_conversion(self, event_key, event_tags): """ Get parameters that are required for the conversion event to register. Args: event_key: Key representing the event which needs to be recorded. event_tags: Dict representing metadata associated with the event. - decisions: List of tuples representing valid experiments IDs and variation IDs. Returns: Dict consisting of the decisions and events info for conversion event. @@ -247,17 +246,6 @@ def _get_required_params_for_conversion(self, event_key, event_tags, decisions): snapshot = {} snapshot[self.EventParams.DECISIONS] = [] - for experiment_id, variation_id in decisions: - - experiment = self.config.get_experiment_from_id(experiment_id) - - if variation_id: - snapshot[self.EventParams.DECISIONS].append({ - self.EventParams.EXPERIMENT_ID: experiment_id, - self.EventParams.VARIATION_ID: variation_id, - self.EventParams.CAMPAIGN_ID: experiment.layerId - }) - event_dict = { self.EventParams.EVENT_ID: self.config.get_event(event_key).id, self.EventParams.TIME: self._get_time(), @@ -303,7 +291,7 @@ def create_impression_event(self, experiment, variation_id, user_id, attributes) http_verb=self.HTTP_VERB, headers=self.HTTP_HEADERS) - def create_conversion_event(self, event_key, user_id, attributes, event_tags, decisions): + def create_conversion_event(self, event_key, user_id, attributes, event_tags): """ Create conversion Event to be sent to the logging endpoint. Args: @@ -311,14 +299,13 @@ def create_conversion_event(self, event_key, user_id, attributes, event_tags, de user_id: ID for user. attributes: Dict representing user attributes and values. event_tags: Dict representing metadata associated with the event. - decisions: List of tuples representing experiments IDs and variation IDs. Returns: Event object encapsulating the conversion event. """ params = self._get_common_params(user_id, attributes) - conversion_params = self._get_required_params_for_conversion(event_key, event_tags, decisions) + conversion_params = self._get_required_params_for_conversion(event_key, event_tags) params[self.EventParams.USERS][0][self.EventParams.SNAPSHOTS].append(conversion_params) return Event(self.EVENTS_URL, diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 56aecfe4..c0dad632 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -322,28 +322,18 @@ def track(self, event_key, user_id, attributes=None, event_tags=None): self.logger.info('Not tracking user "%s" for event "%s".' % (user_id, event_key)) return - # Filter out experiments that are not running or that do not include the user in audience - # conditions and then determine the decision i.e. the corresponding variation - decisions = self._get_decisions(event, user_id, attributes) - - # Create and dispatch conversion event if there are any decisions - if decisions: - conversion_event = self.event_builder.create_conversion_event( - event_key, user_id, attributes, event_tags, decisions - ) - self.logger.info('Tracking event "%s" for user "%s".' % (event_key, user_id)) - self.logger.debug('Dispatching conversion event to URL %s with params %s.' % ( - conversion_event.url, - conversion_event.params - )) - try: - self.event_dispatcher.dispatch_event(conversion_event) - except: - self.logger.exception('Unable to dispatch conversion event!') - self.notification_center.send_notifications(enums.NotificationTypes.TRACK, event_key, user_id, - attributes, event_tags, conversion_event) - else: - self.logger.info('There are no valid experiments for event "%s" to track.' % event_key) + conversion_event = self.event_builder.create_conversion_event(event_key, user_id, attributes, event_tags) + self.logger.info('Tracking event "%s" for user "%s".' % (event_key, user_id)) + self.logger.debug('Dispatching conversion event to URL %s with params %s.' % ( + conversion_event.url, + conversion_event.params + )) + try: + self.event_dispatcher.dispatch_event(conversion_event) + except: + self.logger.exception('Unable to dispatch conversion event!') + self.notification_center.send_notifications(enums.NotificationTypes.TRACK, event_key, user_id, + attributes, event_tags, conversion_event) def get_variation(self, experiment_key, user_id, attributes=None): """ Gets variation where user will be bucketed. diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index ae611d04..b0831b23 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -429,11 +429,7 @@ def test_create_conversion_event(self): 'visitor_id': 'test_user', 'attributes': [], 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111129', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'timestamp': 42123, 'entity_id': '111095', @@ -451,7 +447,7 @@ def test_create_conversion_event(self): with mock.patch('time.time', return_value=42.123), \ mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'): event_obj = self.event_builder.create_conversion_event( - 'test_event', 'test_user', None, None, [('111127', '111129')] + 'test_event', 'test_user', None, None ) self._validate_event_object(event_obj, event_builder.EventBuilder.EVENTS_URL, @@ -475,11 +471,7 @@ def test_create_conversion_event__with_attributes(self): 'key': 'test_attribute' }], 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111129', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'timestamp': 42123, 'entity_id': '111095', @@ -497,7 +489,7 @@ def test_create_conversion_event__with_attributes(self): with mock.patch('time.time', return_value=42.123), \ mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'): event_obj = self.event_builder.create_conversion_event( - 'test_event', 'test_user', {'test_attribute': 'test_value'}, None, [('111127', '111129')] + 'test_event', 'test_user', {'test_attribute': 'test_value'}, None ) self._validate_event_object(event_obj, event_builder.EventBuilder.EVENTS_URL, @@ -527,11 +519,7 @@ def test_create_conversion_event__with_user_agent_when_bot_filtering_is_enabled( 'key': '$opt_bot_filtering' }], 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111129', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'timestamp': 42123, 'entity_id': '111095', @@ -550,7 +538,7 @@ def test_create_conversion_event__with_user_agent_when_bot_filtering_is_enabled( mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \ mock.patch('optimizely.event_builder.EventBuilder._get_bot_filtering', return_value=True): event_obj = self.event_builder.create_conversion_event( - 'test_event', 'test_user', {'$opt_user_agent': 'Edge'}, None, [('111127', '111129')] + 'test_event', 'test_user', {'$opt_user_agent': 'Edge'}, None ) self._validate_event_object(event_obj, @@ -581,11 +569,7 @@ def test_create_conversion_event__with_user_agent_when_bot_filtering_is_disabled 'key': '$opt_bot_filtering' }], 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111129', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'timestamp': 42123, 'entity_id': '111095', @@ -604,8 +588,8 @@ def test_create_conversion_event__with_user_agent_when_bot_filtering_is_disabled mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \ mock.patch('optimizely.event_builder.EventBuilder._get_bot_filtering', return_value=False): event_obj = self.event_builder.create_conversion_event( - 'test_event', 'test_user', {'$opt_user_agent': 'Chrome'}, None, [('111127', '111129')] - ) + 'test_event', 'test_user', {'$opt_user_agent': 'Chrome'}, None + ) self._validate_event_object(event_obj, event_builder.EventBuilder.EVENTS_URL, @@ -629,11 +613,7 @@ def test_create_conversion_event__with_event_tags(self): }], 'visitor_id': 'test_user', 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111129', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', 'tags': { @@ -661,8 +641,7 @@ def test_create_conversion_event__with_event_tags(self): 'test_event', 'test_user', {'test_attribute': 'test_value'}, - {'revenue': 4200, 'value': 1.234, 'non-revenue': 'abc'}, - [('111127', '111129')] + {'revenue': 4200, 'value': 1.234, 'non-revenue': 'abc'} ) self._validate_event_object(event_obj, event_builder.EventBuilder.EVENTS_URL, @@ -686,11 +665,7 @@ def test_create_conversion_event__with_invalid_event_tags(self): }], 'visitor_id': 'test_user', 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111129', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'timestamp': 42123, 'entity_id': '111095', @@ -716,8 +691,7 @@ def test_create_conversion_event__with_invalid_event_tags(self): 'test_event', 'test_user', {'test_attribute': 'test_value'}, - {'revenue': '4200', 'value': True, 'non-revenue': 'abc'}, - [('111127', '111129')] + {'revenue': '4200', 'value': True, 'non-revenue': 'abc'} ) self._validate_event_object(event_obj, event_builder.EventBuilder.EVENTS_URL, @@ -741,15 +715,7 @@ def test_create_conversion_event__when_event_is_used_in_multiple_experiments(sel }], 'visitor_id': 'test_user', 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111129', - 'experiment_id': '111127', - 'campaign_id': '111182' - }, { - 'experiment_id': '111130', - 'variation_id': '111131', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', 'tags': { @@ -777,8 +743,7 @@ def test_create_conversion_event__when_event_is_used_in_multiple_experiments(sel 'test_event', 'test_user', {'test_attribute': 'test_value'}, - {'revenue': 4200, 'value': 1.234, 'non-revenue': 'abc'}, - [('111127', '111129'), ('111130', '111131')] + {'revenue': 4200, 'value': 1.234, 'non-revenue': 'abc'} ) self._validate_event_object(event_obj, event_builder.EventBuilder.EVENTS_URL, diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index ddcd3b74..ec7fc332 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -937,11 +937,7 @@ def test_activate__invalid_object(self): def test_track__with_attributes(self): """ Test that track calls dispatch_event with right params when attributes are provided. """ - with mock.patch('optimizely.decision_service.DecisionService.get_variation', - return_value=self.project_config.get_variation_from_id( - 'test_experiment', '111128' - )) as mock_get_variation, \ - mock.patch('time.time', return_value=42), \ + with mock.patch('time.time', return_value=42), \ mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \ mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: self.optimizely.track('test_event', 'test_user', attributes={'test_attribute': 'test_value'}) @@ -958,11 +954,7 @@ def test_track__with_attributes(self): 'key': 'test_attribute' }], 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111128', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'timestamp': 42000, 'entity_id': '111095', @@ -976,8 +968,6 @@ def test_track__with_attributes(self): 'anonymize_ip': False, 'revision': '42' } - mock_get_variation.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'), - 'test_user', {'test_attribute': 'test_value'}) self.assertEqual(1, mock_dispatch_event.call_count) self._validate_event_object(mock_dispatch_event.call_args[0][0], 'https://logx.optimizely.com/v1/events', expected_params, 'POST', {'Content-Type': 'application/json'}) @@ -1006,14 +996,14 @@ def test_track__with_attributes__typed_audience_match(self): ) def test_track__with_attributes__typed_audience_mismatch(self): - """ Test that track does not call dispatch_event when typed audience conditions do not match. """ + """ Test that track calls dispatch_event even if audience conditions do not match. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences)) with mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: opt_obj.track('item_bought', 'test_user', {'house': 'Welcome to Hufflepuff!'}) - self.assertEqual(0, mock_dispatch_event.call_count) + self.assertEqual(1, mock_dispatch_event.call_count) def test_track__with_attributes__complex_audience_match(self): """ Test that track calls dispatch_event with right params when attributes are provided @@ -1052,7 +1042,7 @@ def test_track__with_attributes__complex_audience_match(self): ) def test_track__with_attributes__complex_audience_mismatch(self): - """ Test that track does not call dispatch_event when complex audience conditions do not match. """ + """ Test that track calls dispatch_event even when complex audience conditions do not match. """ opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences)) @@ -1062,19 +1052,15 @@ def test_track__with_attributes__complex_audience_mismatch(self): user_attr = {'house': 'Gryffindor', 'should_do_it': False} opt_obj.track('user_signed_up', 'test_user', user_attr) - self.assertEqual(0, mock_dispatch_event.call_count) + self.assertEqual(1, mock_dispatch_event.call_count) def test_track__with_attributes__bucketing_id_provided(self): """ Test that track calls dispatch_event with right params when attributes (including bucketing ID) are provided. """ - with mock.patch('optimizely.decision_service.DecisionService.get_variation', - return_value=self.project_config.get_variation_from_id( - 'test_experiment', '111128' - )) as mock_get_variation, \ - mock.patch('time.time', return_value=42), \ - mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \ - mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: + with mock.patch('time.time', return_value=42), \ + mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \ + mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: self.optimizely.track('test_event', 'test_user', attributes={'test_attribute': 'test_value', '$opt_bucketing_id': 'user_bucket_value'}) @@ -1095,11 +1081,7 @@ def test_track__with_attributes__bucketing_id_provided(self): 'key': 'test_attribute' }], 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111128', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'timestamp': 42000, 'entity_id': '111095', @@ -1113,26 +1095,18 @@ def test_track__with_attributes__bucketing_id_provided(self): 'anonymize_ip': False, 'revision': '42' } - mock_get_variation.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'), - 'test_user', {'test_attribute': 'test_value', - '$opt_bucketing_id': 'user_bucket_value'}) self.assertEqual(1, mock_dispatch_event.call_count) self._validate_event_object(mock_dispatch_event.call_args[0][0], 'https://logx.optimizely.com/v1/events', expected_params, 'POST', {'Content-Type': 'application/json'}) def test_track__with_attributes__no_audience_match(self): - """ Test that track does not call dispatch_event when audience conditions do not match. """ + """ Test that track calls dispatch_event even if audience conditions do not match. """ - with mock.patch('optimizely.bucketer.Bucketer.bucket', - return_value=self.project_config.get_variation_from_id( - 'test_experiment', '111128' - )) as mock_bucket, \ - mock.patch('time.time', return_value=42), \ + with mock.patch('time.time', return_value=42), \ mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: self.optimizely.track('test_event', 'test_user', attributes={'test_attribute': 'wrong_test_value'}) - self.assertEqual(0, mock_bucket.call_count) - self.assertEqual(0, mock_dispatch_event.call_count) + self.assertEqual(1, mock_dispatch_event.call_count) def test_track__with_attributes__invalid_attributes(self): """ Test that track does not bucket or dispatch event if attributes are invalid. """ @@ -1147,11 +1121,7 @@ def test_track__with_attributes__invalid_attributes(self): def test_track__with_event_tags(self): """ Test that track calls dispatch_event with right params when event tags are provided. """ - with mock.patch('optimizely.decision_service.DecisionService.get_variation', - return_value=self.project_config.get_variation_from_id( - 'test_experiment', '111128' - )) as mock_get_variation, \ - mock.patch('time.time', return_value=42), \ + with mock.patch('time.time', return_value=42), \ mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \ mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: self.optimizely.track('test_event', 'test_user', attributes={'test_attribute': 'test_value'}, @@ -1169,11 +1139,7 @@ def test_track__with_event_tags(self): 'key': 'test_attribute' }], 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111128', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'entity_id': '111095', 'key': 'test_event', @@ -1194,8 +1160,6 @@ def test_track__with_event_tags(self): 'anonymize_ip': False, 'revision': '42' } - mock_get_variation.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'), - 'test_user', {'test_attribute': 'test_value'}) self.assertEqual(1, mock_dispatch_event.call_count) self._validate_event_object(mock_dispatch_event.call_args[0][0], 'https://logx.optimizely.com/v1/events', expected_params, 'POST', {'Content-Type': 'application/json'}) @@ -1204,11 +1168,7 @@ def test_track__with_event_tags_revenue(self): """ Test that track calls dispatch_event with right params when only revenue event tags are provided only. """ - with mock.patch('optimizely.decision_service.DecisionService.get_variation', - return_value=self.project_config.get_variation_from_id( - 'test_experiment', '111128' - )) as mock_get_variation, \ - mock.patch('time.time', return_value=42), \ + with mock.patch('time.time', return_value=42), \ mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \ mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: self.optimizely.track('test_event', 'test_user', attributes={'test_attribute': 'test_value'}, @@ -1224,11 +1184,7 @@ def test_track__with_event_tags_revenue(self): }], 'visitor_id': 'test_user', 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111128', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'entity_id': '111095', 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', @@ -1249,8 +1205,6 @@ def test_track__with_event_tags_revenue(self): 'anonymize_ip': False, 'revision': '42' } - mock_get_variation.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'), - 'test_user', {'test_attribute': 'test_value'}) self.assertEqual(1, mock_dispatch_event.call_count) self._validate_event_object(mock_dispatch_event.call_args[0][0], 'https://logx.optimizely.com/v1/events', expected_params, 'POST', {'Content-Type': 'application/json'}) @@ -1259,12 +1213,7 @@ def test_track__with_event_tags_numeric_metric(self): """ Test that track calls dispatch_event with right params when only numeric metric event tags are provided. """ - with mock.patch('optimizely.decision_service.DecisionService.get_variation', - return_value=self.project_config.get_variation_from_id( - 'test_experiment', '111128' - )) as mock_get_variation, \ - mock.patch('time.time', return_value=42), \ - mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: + with mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: self.optimizely.track('test_event', 'test_user', attributes={'test_attribute': 'test_value'}, event_tags={'value': 1.234, 'non-revenue': 'abc'}) @@ -1279,8 +1228,6 @@ def test_track__with_event_tags_numeric_metric(self): 'value': 'test_value', 'key': 'test_attribute' } - mock_get_variation.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'), - 'test_user', {'test_attribute': 'test_value'}) self.assertEqual(1, mock_dispatch_event.call_count) self._validate_event_object_event_tags(mock_dispatch_event.call_args[0][0], expected_event_metrics_params, @@ -1309,11 +1256,7 @@ def test_track__with_event_tags__forced_bucketing(self): 'key': 'test_attribute' }], 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111129', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'entity_id': '111095', 'key': 'test_event', @@ -1343,11 +1286,7 @@ def test_track__with_event_tags__forced_bucketing(self): def test_track__with_invalid_event_tags(self): """ Test that track calls dispatch_event with right params when invalid event tags are provided. """ - with mock.patch('optimizely.decision_service.DecisionService.get_variation', - return_value=self.project_config.get_variation_from_id( - 'test_experiment', '111128' - )) as mock_get_variation, \ - mock.patch('time.time', return_value=42), \ + with mock.patch('time.time', return_value=42), \ mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \ mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: self.optimizely.track('test_event', 'test_user', attributes={'test_attribute': 'test_value'}, @@ -1363,11 +1302,7 @@ def test_track__with_invalid_event_tags(self): }], 'visitor_id': 'test_user', 'snapshots': [{ - 'decisions': [{ - 'variation_id': '111128', - 'experiment_id': '111127', - 'campaign_id': '111182' - }], + 'decisions': [], 'events': [{ 'timestamp': 42000, 'entity_id': '111095', @@ -1387,14 +1322,12 @@ def test_track__with_invalid_event_tags(self): 'anonymize_ip': False, 'revision': '42' } - mock_get_variation.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'), - 'test_user', {'test_attribute': 'test_value'}) self.assertEqual(1, mock_dispatch_event.call_count) self._validate_event_object(mock_dispatch_event.call_args[0][0], 'https://logx.optimizely.com/v1/events', expected_params, 'POST', {'Content-Type': 'application/json'}) def test_track__experiment_not_running(self): - """ Test that track does not call dispatch_event when experiment is not running. """ + """ Test that track calls dispatch_event even if experiment is not running. """ with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=False) as mock_is_experiment_running, \ @@ -1402,8 +1335,9 @@ def test_track__experiment_not_running(self): mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: self.optimizely.track('test_event', 'test_user') - mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment')) - self.assertEqual(0, mock_dispatch_event.call_count) + # Assert that experiment is running is not performed + self.assertEqual(0, mock_is_experiment_running.call_count) + self.assertEqual(1, mock_dispatch_event.call_count) def test_track_invalid_event_key(self): """ Test that track does not call dispatch_event when event does not exist. """ @@ -1418,20 +1352,14 @@ def test_track_invalid_event_key(self): ) def test_track__whitelisted_user_overrides_audience_check(self): - """ Test that track does not check for user in audience when user is in whitelist. """ + """ Test that event is tracked when user is whitelisted. """ - with mock.patch('optimizely.helpers.experiment.is_experiment_running', - return_value=True) as mock_is_experiment_running, \ - mock.patch('optimizely.helpers.audience.is_user_in_experiment', - return_value=False) as mock_audience_check, \ - mock.patch('time.time', return_value=42), \ + with mock.patch('time.time', return_value=42), \ mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \ mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event: self.optimizely.track('test_event', 'user_1') - mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment')) self.assertEqual(1, mock_dispatch_event.call_count) - self.assertEqual(0, mock_audience_check.call_count) def test_track__invalid_object(self): """ Test that track logs error if Optimizely object is not created correctly. """ @@ -2374,26 +2302,19 @@ def test_track(self): user_id = 'test_user' event_key = 'test_event' mock_client_logger = mock.patch.object(self.optimizely, 'logger') - mock_config_logger = mock.patch.object(self.optimizely.config, 'logger') - mock_decision_logger = mock.patch.object(self.optimizely.decision_service, 'logger') - with mock.patch('optimizely.helpers.audience.is_user_in_experiment', - return_value=False), \ - mock.patch('time.time', return_value=42), \ + + mock_conversion_event = event_builder.Event('logx.optimizely.com', {'event_key': event_key}) + with mock.patch('optimizely.event_builder.EventBuilder.create_conversion_event', return_value=mock_conversion_event), \ mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event'), \ - mock_decision_logger as mock_decision_logging, \ - mock_config_logger as mock_config_logging, \ mock_client_logger as mock_client_logging: self.optimizely.track(event_key, user_id) - mock_config_logging.debug.assert_called_once_with( - 'User "test_user" is not in the forced variation map.' - ) - mock_decision_logging.info.assert_called_once_with( - 'User "test_user" does not meet conditions to be in experiment "test_experiment".' - ) mock_client_logging.info.assert_has_calls([ - mock.call('Not tracking user "test_user" for experiment "test_experiment".'), - mock.call('There are no valid experiments for event "test_event" to track.') + mock.call('Tracking event "%s" for user "%s".' % (event_key, user_id)), + ]) + mock_client_logging.debug.assert_has_calls([ + mock.call('Dispatching conversion event to URL %s with params %s.' % ( + mock_conversion_event.url, mock_conversion_event.params)), ]) def test_activate__experiment_not_running(self): From a144f2830ddfa91f0d4a9e04b00925cb44635da1 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Fri, 28 Dec 2018 13:25:03 -0800 Subject: [PATCH 2/3] Lint fix --- tests/test_optimizely.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index ec7fc332..59bd73c0 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -2304,7 +2304,8 @@ def test_track(self): mock_client_logger = mock.patch.object(self.optimizely, 'logger') mock_conversion_event = event_builder.Event('logx.optimizely.com', {'event_key': event_key}) - with mock.patch('optimizely.event_builder.EventBuilder.create_conversion_event', return_value=mock_conversion_event), \ + with mock.patch('optimizely.event_builder.EventBuilder.create_conversion_event', + return_value=mock_conversion_event), \ mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event'), \ mock_client_logger as mock_client_logging: self.optimizely.track(event_key, user_id) From 18a36440853bc38b9223b6dbac5fccc3623f00f0 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 8 Jan 2019 14:24:12 -0800 Subject: [PATCH 3/3] Updating to use newer option. Updated unit tests as well. --- optimizely/event_builder.py | 3 ++- tests/test_event_builder.py | 21 ++++++++++++++------- tests/test_optimizely.py | 19 +++++++++++++------ 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/optimizely/event_builder.py b/optimizely/event_builder.py index 6b690351..4a6113a8 100644 --- a/optimizely/event_builder.py +++ b/optimizely/event_builder.py @@ -130,6 +130,7 @@ def _get_common_params(self, user_id, attributes): commonParams[self.EventParams.USERS][0][self.EventParams.ATTRIBUTES] = self._get_attributes(attributes) commonParams[self.EventParams.SOURCE_SDK_TYPE] = 'python-sdk' + commonParams[self.EventParams.ENRICH_DECISIONS] = True commonParams[self.EventParams.SOURCE_SDK_VERSION] = version.__version__ commonParams[self.EventParams.ANONYMIZE_IP] = self._get_anonymize_ip() commonParams[self.EventParams.REVISION] = self._get_revision() @@ -152,6 +153,7 @@ class EventParams(object): CAMPAIGN_ID = 'campaign_id' VARIATION_ID = 'variation_id' END_USER_ID = 'visitor_id' + ENRICH_DECISIONS = 'enrich_decisions' EVENTS = 'events' EVENT_ID = 'entity_id' ATTRIBUTES = 'attributes' @@ -244,7 +246,6 @@ def _get_required_params_for_conversion(self, event_key, event_tags): Dict consisting of the decisions and events info for conversion event. """ snapshot = {} - snapshot[self.EventParams.DECISIONS] = [] event_dict = { self.EventParams.EVENT_ID: self.config.get_event(event_key).id, diff --git a/tests/test_event_builder.py b/tests/test_event_builder.py index b0831b23..1bd05cae 100644 --- a/tests/test_event_builder.py +++ b/tests/test_event_builder.py @@ -84,6 +84,7 @@ def test_create_impression_event(self): }], 'client_name': 'python-sdk', 'client_version': version.__version__, + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -131,6 +132,7 @@ def test_create_impression_event__with_attributes(self): }], 'client_name': 'python-sdk', 'client_version': version.__version__, + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -173,6 +175,7 @@ def test_create_impression_event_when_attribute_is_not_in_datafile(self): }], 'client_name': 'python-sdk', 'client_version': version.__version__, + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -225,6 +228,7 @@ def test_create_impression_event_calls_is_attribute_valid(self): }], 'client_name': 'python-sdk', 'client_version': version.__version__, + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -295,6 +299,7 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_enabled( }], 'client_name': 'python-sdk', 'client_version': version.__version__, + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -345,6 +350,7 @@ def test_create_impression_event__with_empty_attributes_when_bot_filtering_is_en }], 'client_name': 'python-sdk', 'client_version': version.__version__, + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -400,6 +406,7 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled }], 'client_name': 'python-sdk', 'client_version': version.__version__, + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -429,7 +436,6 @@ def test_create_conversion_event(self): 'visitor_id': 'test_user', 'attributes': [], 'snapshots': [{ - 'decisions': [], 'events': [{ 'timestamp': 42123, 'entity_id': '111095', @@ -440,6 +446,7 @@ def test_create_conversion_event(self): }], 'client_name': 'python-sdk', 'client_version': version.__version__, + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -471,7 +478,6 @@ def test_create_conversion_event__with_attributes(self): 'key': 'test_attribute' }], 'snapshots': [{ - 'decisions': [], 'events': [{ 'timestamp': 42123, 'entity_id': '111095', @@ -482,6 +488,7 @@ def test_create_conversion_event__with_attributes(self): }], 'client_name': 'python-sdk', 'client_version': version.__version__, + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -519,7 +526,6 @@ def test_create_conversion_event__with_user_agent_when_bot_filtering_is_enabled( 'key': '$opt_bot_filtering' }], 'snapshots': [{ - 'decisions': [], 'events': [{ 'timestamp': 42123, 'entity_id': '111095', @@ -530,6 +536,7 @@ def test_create_conversion_event__with_user_agent_when_bot_filtering_is_enabled( }], 'client_name': 'python-sdk', 'client_version': version.__version__, + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -569,7 +576,6 @@ def test_create_conversion_event__with_user_agent_when_bot_filtering_is_disabled 'key': '$opt_bot_filtering' }], 'snapshots': [{ - 'decisions': [], 'events': [{ 'timestamp': 42123, 'entity_id': '111095', @@ -580,6 +586,7 @@ def test_create_conversion_event__with_user_agent_when_bot_filtering_is_disabled }], 'client_name': 'python-sdk', 'client_version': version.__version__, + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -613,7 +620,6 @@ def test_create_conversion_event__with_event_tags(self): }], 'visitor_id': 'test_user', 'snapshots': [{ - 'decisions': [], 'events': [{ 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', 'tags': { @@ -631,6 +637,7 @@ def test_create_conversion_event__with_event_tags(self): }], 'account_id': '12001', 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -665,7 +672,6 @@ def test_create_conversion_event__with_invalid_event_tags(self): }], 'visitor_id': 'test_user', 'snapshots': [{ - 'decisions': [], 'events': [{ 'timestamp': 42123, 'entity_id': '111095', @@ -681,6 +687,7 @@ def test_create_conversion_event__with_invalid_event_tags(self): }], 'account_id': '12001', 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -715,7 +722,6 @@ def test_create_conversion_event__when_event_is_used_in_multiple_experiments(sel }], 'visitor_id': 'test_user', 'snapshots': [{ - 'decisions': [], 'events': [{ 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', 'tags': { @@ -733,6 +739,7 @@ def test_create_conversion_event__when_event_is_used_in_multiple_experiments(sel }], 'account_id': '12001', 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 59bd73c0..ef07cb14 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -246,6 +246,7 @@ def test_activate(self): }], 'client_version': version.__version__, 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -587,6 +588,7 @@ def test_activate__with_attributes__audience_match(self): }], 'client_version': version.__version__, 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -659,6 +661,7 @@ def test_activate__with_attributes_of_different_types(self): }], 'client_version': version.__version__, 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -801,6 +804,7 @@ def test_activate__with_attributes__audience_match__forced_bucketing(self): }], 'client_version': version.__version__, 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -855,6 +859,7 @@ def test_activate__with_attributes__audience_match__bucketing_id_provided(self): }], 'client_version': version.__version__, 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -954,7 +959,6 @@ def test_track__with_attributes(self): 'key': 'test_attribute' }], 'snapshots': [{ - 'decisions': [], 'events': [{ 'timestamp': 42000, 'entity_id': '111095', @@ -965,6 +969,7 @@ def test_track__with_attributes(self): }], 'client_version': version.__version__, 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -1081,7 +1086,6 @@ def test_track__with_attributes__bucketing_id_provided(self): 'key': 'test_attribute' }], 'snapshots': [{ - 'decisions': [], 'events': [{ 'timestamp': 42000, 'entity_id': '111095', @@ -1092,6 +1096,7 @@ def test_track__with_attributes__bucketing_id_provided(self): }], 'client_version': version.__version__, 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -1139,7 +1144,6 @@ def test_track__with_event_tags(self): 'key': 'test_attribute' }], 'snapshots': [{ - 'decisions': [], 'events': [{ 'entity_id': '111095', 'key': 'test_event', @@ -1157,6 +1161,7 @@ def test_track__with_event_tags(self): }], 'client_version': version.__version__, 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -1184,7 +1189,6 @@ def test_track__with_event_tags_revenue(self): }], 'visitor_id': 'test_user', 'snapshots': [{ - 'decisions': [], 'events': [{ 'entity_id': '111095', 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', @@ -1201,6 +1205,7 @@ def test_track__with_event_tags_revenue(self): 'client_name': 'python-sdk', 'project_id': '111001', 'client_version': version.__version__, + 'enrich_decisions': True, 'account_id': '12001', 'anonymize_ip': False, 'revision': '42' @@ -1256,7 +1261,6 @@ def test_track__with_event_tags__forced_bucketing(self): 'key': 'test_attribute' }], 'snapshots': [{ - 'decisions': [], 'events': [{ 'entity_id': '111095', 'key': 'test_event', @@ -1274,6 +1278,7 @@ def test_track__with_event_tags__forced_bucketing(self): }], 'client_version': version.__version__, 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '42' } @@ -1302,7 +1307,6 @@ def test_track__with_invalid_event_tags(self): }], 'visitor_id': 'test_user', 'snapshots': [{ - 'decisions': [], 'events': [{ 'timestamp': 42000, 'entity_id': '111095', @@ -1318,6 +1322,7 @@ def test_track__with_invalid_event_tags(self): 'client_name': 'python-sdk', 'project_id': '111001', 'client_version': version.__version__, + 'enrich_decisions': True, 'account_id': '12001', 'anonymize_ip': False, 'revision': '42' @@ -1549,6 +1554,7 @@ def test_is_feature_enabled__returns_true_for_feature_experiment_if_feature_enab }], 'client_version': version.__version__, 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '1' } @@ -1613,6 +1619,7 @@ def test_is_feature_enabled__returns_false_for_feature_experiment_if_feature_dis }], 'client_version': version.__version__, 'client_name': 'python-sdk', + 'enrich_decisions': True, 'anonymize_ip': False, 'revision': '1' }