From 9ee17560fa90e66c6a15b9b667bcc37b4886ed3e Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Fri, 25 Sep 2020 15:57:26 -0700 Subject: [PATCH 01/11] feat: support for send_flag_decisions --- optimizely/event/event_factory.py | 5 +- optimizely/event/payload.py | 11 +- optimizely/event/user_event.py | 4 +- optimizely/event/user_event_factory.py | 4 +- optimizely/optimizely.py | 14 ++- optimizely/project_config.py | 10 ++ tests/test_event_factory.py | 57 ++++++++-- tests/test_event_payload.py | 9 +- tests/test_optimizely.py | 141 +++++++++++++++++++++++-- tests/test_user_event_factory.py | 5 +- 10 files changed, 234 insertions(+), 26 deletions(-) diff --git a/optimizely/event/event_factory.py b/optimizely/event/event_factory.py index e2851bfc3..8de72ac03 100644 --- a/optimizely/event/event_factory.py +++ b/optimizely/event/event_factory.py @@ -89,7 +89,10 @@ def _create_visitor(cls, event, logger): """ if isinstance(event, user_event.ImpressionEvent): - decision = payload.Decision(event.experiment.layerId, event.experiment.id, event.variation.id,) + + metadata = payload.Metadata(event.variation.key, event.flag_key, event.flag_type) + + decision = payload.Decision(event.experiment.layerId, event.experiment.id, event.variation.id, metadata) snapshot_event = payload.SnapshotEvent( event.experiment.layerId, event.uuid, cls.ACTIVATE_EVENT_KEY, event.timestamp, diff --git a/optimizely/event/payload.py b/optimizely/event/payload.py index 450acd557..35405b094 100644 --- a/optimizely/event/payload.py +++ b/optimizely/event/payload.py @@ -61,10 +61,19 @@ def get_event_params(self): class Decision(object): """ Class respresenting Decision. """ - def __init__(self, campaign_id, experiment_id, variation_id): + def __init__(self, campaign_id, experiment_id, variation_id, metadata): self.campaign_id = campaign_id self.experiment_id = experiment_id self.variation_id = variation_id + self.metadata = metadata + +class Metadata(object): + """ Class respresenting Metadata. """ + + def __init__(self, variation_key, flag_key, flag_type): + self.variation_key = variation_key + self.flag_key = flag_key + self.flag_type = flag_type class Snapshot(object): diff --git a/optimizely/event/user_event.py b/optimizely/event/user_event.py index 6eb014f9f..373dc1015 100644 --- a/optimizely/event/user_event.py +++ b/optimizely/event/user_event.py @@ -41,11 +41,13 @@ class ImpressionEvent(UserEvent): """ Class representing Impression Event. """ def __init__( - self, event_context, user_id, experiment, visitor_attributes, variation, bot_filtering=None, + self, event_context, user_id, experiment, visitor_attributes, variation, flag_key, flag_type, bot_filtering=None, ): super(ImpressionEvent, self).__init__(event_context, user_id, visitor_attributes, bot_filtering) self.experiment = experiment self.variation = variation + self.flag_key = flag_key + self.flag_type = flag_type class ConversionEvent(UserEvent): diff --git a/optimizely/event/user_event_factory.py b/optimizely/event/user_event_factory.py index 15908cc7f..ef01a41b3 100644 --- a/optimizely/event/user_event_factory.py +++ b/optimizely/event/user_event_factory.py @@ -20,7 +20,7 @@ class UserEventFactory(object): @classmethod def create_impression_event( - cls, project_config, activated_experiment, variation_id, user_id, user_attributes, + cls, project_config, activated_experiment, variation_id, user_id, user_attributes, flag_key, flag_type ): """ Create impression Event to be sent to the logging endpoint. @@ -52,6 +52,8 @@ def create_impression_event( activated_experiment, event_factory.EventFactory.build_attribute_list(user_attributes, project_config), variation, + flag_key, + flag_type, project_config.get_bot_filtering_value(), ) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index ff4f41a7f..82b3ff7ef 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -160,7 +160,7 @@ def _validate_user_inputs(self, attributes=None, event_tags=None): return True - def _send_impression_event(self, project_config, experiment, variation, user_id, attributes): + def _send_impression_event(self, project_config, experiment, variation, user_id, attributes, flag_key, flag_type): """ Helper method to send impression event. Args: @@ -172,7 +172,7 @@ def _send_impression_event(self, project_config, experiment, variation, user_id, """ user_event = user_event_factory.UserEventFactory.create_impression_event( - project_config, experiment, variation.id, user_id, attributes + project_config, experiment, variation.id, user_id, attributes, flag_key, flag_type ) self.event_processor.process(user_event) @@ -422,7 +422,7 @@ def activate(self, experiment_key, user_id, attributes=None): # Create and dispatch impression event self.logger.info('Activating user "%s" in experiment "%s".' % (user_id, experiment.key)) - self._send_impression_event(project_config, experiment, variation, user_id, attributes) + self._send_impression_event(project_config, experiment, variation, user_id, attributes, experiment.key, "experiment") return variation.key @@ -573,6 +573,12 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): source_info = {} decision = self.decision_service.get_variation_for_feature(project_config, feature, user_id, attributes) is_source_experiment = decision.source == enums.DecisionSources.FEATURE_TEST + is_source_rollout = decision.source == enums.DecisionSources.ROLLOUT + + if is_source_rollout and project_config.get_send_flag_decisions_value(): + self._send_impression_event( + project_config, decision.experiment, decision.variation, user_id, attributes, feature.key, decision.source + ) if decision.variation: if decision.variation.featureEnabled is True: @@ -584,7 +590,7 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): 'variation_key': decision.variation.key, } self._send_impression_event( - project_config, decision.experiment, decision.variation, user_id, attributes, + project_config, decision.experiment, decision.variation, user_id, attributes, feature.key, decision.source ) if feature_enabled: diff --git a/optimizely/project_config.py b/optimizely/project_config.py index 8d608890e..77b89e67f 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -61,6 +61,7 @@ def __init__(self, datafile, logger, error_handler): self.feature_flags = config.get('featureFlags', []) self.rollouts = config.get('rollouts', []) self.anonymize_ip = config.get('anonymizeIP', False) + self.send_flag_decisions = config.get('sendFlagDecisions', False) self.bot_filtering = config.get('botFiltering', None) # Utility maps for quick lookup @@ -514,6 +515,15 @@ def get_anonymize_ip_value(self): return self.anonymize_ip + def get_send_flag_decisions_value(self): + """ Gets the Send Flag Decisions value. + + Returns: + A boolean value that indicates if we should send flag decisions. + """ + + return self.send_flag_decisions + def get_bot_filtering_value(self): """ Gets the bot filtering value. diff --git a/tests/test_event_factory.py b/tests/test_event_factory.py index 73a8054be..e98ec96ca 100644 --- a/tests/test_event_factory.py +++ b/tests/test_event_factory.py @@ -74,7 +74,11 @@ def test_create_impression_event(self): 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'flag_key', + 'flag_type': 'experiment', + 'variation_key': 'variation'} + } ], 'events': [ { @@ -104,6 +108,8 @@ def test_create_impression_event(self): '111129', 'test_user', None, + 'flag_key', + 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -128,7 +134,11 @@ def test_create_impression_event__with_attributes(self): 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'flag_key', + 'flag_type': 'experiment', + 'variation_key': 'variation'}, + } ], 'events': [ { @@ -158,6 +168,8 @@ def test_create_impression_event__with_attributes(self): '111129', 'test_user', {'test_attribute': 'test_value'}, + 'flag_key', + 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -180,7 +192,11 @@ def test_create_impression_event_when_attribute_is_not_in_datafile(self): 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'flag_key', + 'flag_type': 'experiment', + 'variation_key': 'variation'} + } ], 'events': [ { @@ -210,6 +226,9 @@ def test_create_impression_event_when_attribute_is_not_in_datafile(self): '111129', 'test_user', {'do_you_know_me': 'test_value'}, + 'flag_key', + 'experiment' + ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -235,7 +254,11 @@ def test_create_impression_event_calls_is_attribute_valid(self): 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'flag_key', + 'flag_type': 'experiment', + 'variation_key': 'variation'}, + } ], 'events': [ { @@ -282,6 +305,8 @@ def side_effect(*args, **kwargs): '111129', 'test_user', attributes, + 'flag_key', + 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -317,7 +342,11 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_enabled( 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'flag_key', + 'flag_type': 'experiment', + 'variation_key': 'variation'}, + } ], 'events': [ { @@ -349,6 +378,8 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_enabled( '111129', 'test_user', {'$opt_user_agent': 'Edge'}, + 'flag_key', + 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -379,7 +410,11 @@ def test_create_impression_event__with_empty_attributes_when_bot_filtering_is_en 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'flag_key', + 'flag_type': 'experiment', + 'variation_key': 'variation'}, + } ], 'events': [ { @@ -411,6 +446,8 @@ def test_create_impression_event__with_empty_attributes_when_bot_filtering_is_en '111129', 'test_user', None, + 'flag_key', + 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -447,7 +484,11 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'flag_key', + 'flag_type': 'experiment', + 'variation_key': 'variation'}, + } ], 'events': [ { @@ -479,6 +520,8 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled '111129', 'test_user', {'$opt_user_agent': 'Chrome'}, + 'flag_key', + 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) diff --git a/tests/test_event_payload.py b/tests/test_event_payload.py index e8cd6fbc7..4847a4b6f 100644 --- a/tests/test_event_payload.py +++ b/tests/test_event_payload.py @@ -30,7 +30,11 @@ def test_impression_event_equals_serialized_payload(self): 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'flag_key', + 'flag_type': 'experiment', + 'variation_key': 'variation'}, + } ], 'events': [ { @@ -54,7 +58,8 @@ def test_impression_event_equals_serialized_payload(self): batch = payload.EventBatch('12001', '111001', '42', 'python-sdk', version.__version__, False, True) visitor_attr = payload.VisitorAttribute('111094', 'test_attribute', 'custom', 'test_value') event = payload.SnapshotEvent('111182', 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', 'campaign_activated', 42123,) - event_decision = payload.Decision('111182', '111127', '111129') + metadata = payload.Metadata('variation', 'flag_key', 'experiment') + event_decision = payload.Decision('111182', '111127', '111129', metadata) snapshots = payload.Snapshot([event], [event_decision]) user = payload.Visitor([snapshots], [visitor_attr], 'test_user') diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index f586c44c5..3cf3ec885 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -321,7 +321,11 @@ def test_activate(self): 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'test_experiment', + 'flag_type': 'experiment', + 'variation_key': 'variation'}, + } ], 'events': [ { @@ -694,7 +698,11 @@ def test_activate__with_attributes__audience_match(self): 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'test_experiment', + 'flag_type': 'experiment', + 'variation_key': 'variation'}, + } ], 'events': [ { @@ -771,7 +779,11 @@ def test_activate__with_attributes_of_different_types(self): 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'test_experiment', + 'flag_type': 'experiment', + 'variation_key': 'variation'}, + } ], 'events': [ { @@ -962,7 +974,11 @@ def test_activate__with_attributes__audience_match__forced_bucketing(self): 'snapshots': [ { 'decisions': [ - {'variation_id': '111128', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111128', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'test_experiment', + 'flag_type': 'experiment', + 'variation_key': 'control'}, + } ], 'events': [ { @@ -1032,7 +1048,11 @@ def test_activate__with_attributes__audience_match__bucketing_id_provided(self): 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'test_experiment', + 'flag_type': 'experiment', + 'variation_key': 'variation'}, + } ], 'events': [ { @@ -1975,7 +1995,11 @@ def test_is_feature_enabled__returns_true_for_feature_experiment_if_feature_enab 'snapshots': [ { 'decisions': [ - {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'test_feature_in_experiment', + 'flag_type': 'feature-test', + 'variation_key': 'variation'}, + } ], 'events': [ { @@ -2069,7 +2093,11 @@ def test_is_feature_enabled__returns_false_for_feature_experiment_if_feature_dis 'snapshots': [ { 'decisions': [ - {'variation_id': '111128', 'experiment_id': '111127', 'campaign_id': '111182'} + {'variation_id': '111128', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'test_feature_in_experiment', + 'flag_type': 'feature-test', + 'variation_key': 'control'}, + } ], 'events': [ { @@ -2148,6 +2176,105 @@ def test_is_feature_enabled__returns_true_for_feature_rollout_if_feature_enabled # Check that impression event is not sent self.assertEqual(0, mock_process.call_count) + def test_is_feature_enabled__returns_true_for_feature_rollout_if_feature_enabled_with_sending_decisions(self,): + """ Test that the feature is enabled for the user if bucketed into variation of a rollout and + the variation's featureEnabled property is True. Also confirm that an impression event is processed and + decision is broadcasted with proper parameters, as send_flag_decisions is set to true """ + + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) + project_config = opt_obj.config_manager.get_config() + project_config.send_flag_decisions = True + feature = project_config.get_feature_from_key('test_feature_in_experiment') + + mock_experiment = project_config.get_experiment_from_key('test_experiment') + mock_variation = project_config.get_variation_from_id('test_experiment', '111129') + + # Assert that featureEnabled property is True + self.assertTrue(mock_variation.featureEnabled) + + with mock.patch( + 'optimizely.decision_service.DecisionService.get_variation_for_feature', + return_value=decision_service.Decision(mock_experiment, mock_variation, enums.DecisionSources.ROLLOUT), + ) as mock_decision, mock.patch( + 'optimizely.event.event_processor.ForwardingEventProcessor.process' + ) as mock_process, mock.patch( + 'optimizely.notification_center.NotificationCenter.send_notifications' + ) as mock_broadcast_decision, mock.patch( + 'uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c' + ), mock.patch( + 'time.time', return_value=42 + ): + self.assertTrue(opt_obj.is_feature_enabled('test_feature_in_experiment', 'test_user')) + + mock_decision.assert_called_once_with(opt_obj.config_manager.get_config(), feature, 'test_user', None) + + mock_broadcast_decision.assert_called_with( + enums.NotificationTypes.DECISION, + 'feature', + 'test_user', + {}, + { + 'feature_key': 'test_feature_in_experiment', + 'feature_enabled': True, + 'source': 'rollout', + 'source_info': {}, + }, + ) + + # Check that impression event is sent + expected_params = { + 'account_id': '12001', + 'project_id': '111111', + 'visitors': [ + { + 'visitor_id': 'test_user', + 'attributes': [ + { + 'type': 'custom', + 'value': True, + 'entity_id': '$opt_bot_filtering', + 'key': '$opt_bot_filtering', + } + ], + 'snapshots': [ + { + 'decisions': [ + {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', + 'metadata': {'flag_key': 'test_feature_in_experiment', + 'flag_type': 'rollout', + 'variation_key': 'variation'}, + } + ], + 'events': [ + { + 'timestamp': 42000, + 'entity_id': '111182', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'key': 'campaign_activated', + } + ], + } + ], + } + ], + 'client_version': version.__version__, + 'client_name': 'python-sdk', + 'enrich_decisions': True, + 'anonymize_ip': False, + 'revision': '1', + } + log_event = EventFactory.create_log_event(mock_process.call_args[0][0], self.optimizely.logger) + + # Check that impression event is sent + self.assertEqual(1, mock_process.call_count) + self._validate_event_object( + log_event.__dict__, + 'https://logx.optimizely.com/v1/events', + expected_params, + 'POST', + {'Content-Type': 'application/json'}, + ) + def test_is_feature_enabled__returns_false_for_feature_rollout_if_feature_disabled(self,): """ Test that the feature is disabled for the user if bucketed into variation of a rollout and the variation's featureEnabled property is False. Also confirm that no impression event is processed and diff --git a/tests/test_user_event_factory.py b/tests/test_user_event_factory.py index b048bf5b7..c45e4d999 100644 --- a/tests/test_user_event_factory.py +++ b/tests/test_user_event_factory.py @@ -28,7 +28,8 @@ def test_impression_event(self): variation = self.project_config.get_variation_from_id(experiment.key, '111128') user_id = 'test_user' - impression_event = UserEventFactory.create_impression_event(project_config, experiment, '111128', user_id, None) + impression_event = UserEventFactory.create_impression_event(project_config, experiment, '111128', user_id, None, + 'flag_key', 'flag_type') self.assertEqual(self.project_config.project_id, impression_event.event_context.project_id) self.assertEqual(self.project_config.revision, impression_event.event_context.revision) @@ -50,7 +51,7 @@ def test_impression_event__with_attributes(self): user_attributes = {'test_attribute': 'test_value', 'boolean_key': True} impression_event = UserEventFactory.create_impression_event( - project_config, experiment, '111128', user_id, user_attributes + project_config, experiment, '111128', user_id, user_attributes, 'flag_key', 'flag_type' ) expected_attrs = EventFactory.build_attribute_list(user_attributes, project_config) From edcf313aa05e9551a82a8f18bbd143113f1639e9 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Fri, 25 Sep 2020 16:10:02 -0700 Subject: [PATCH 02/11] fix linters --- optimizely/event/user_event.py | 3 ++- optimizely/optimizely.py | 9 ++++++--- tests/test_event_factory.py | 3 +-- tests/test_optimizely.py | 6 ++---- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/optimizely/event/user_event.py b/optimizely/event/user_event.py index 373dc1015..56d34c875 100644 --- a/optimizely/event/user_event.py +++ b/optimizely/event/user_event.py @@ -41,7 +41,8 @@ class ImpressionEvent(UserEvent): """ Class representing Impression Event. """ def __init__( - self, event_context, user_id, experiment, visitor_attributes, variation, flag_key, flag_type, bot_filtering=None, + self, event_context, user_id, experiment, visitor_attributes, variation, flag_key, flag_type, + bot_filtering=None, ): super(ImpressionEvent, self).__init__(event_context, user_id, visitor_attributes, bot_filtering) self.experiment = experiment diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 82b3ff7ef..3a72fcf38 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -422,7 +422,8 @@ def activate(self, experiment_key, user_id, attributes=None): # Create and dispatch impression event self.logger.info('Activating user "%s" in experiment "%s".' % (user_id, experiment.key)) - self._send_impression_event(project_config, experiment, variation, user_id, attributes, experiment.key, "experiment") + self._send_impression_event(project_config, experiment, variation, user_id, attributes, experiment.key, + "experiment") return variation.key @@ -577,7 +578,8 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): if is_source_rollout and project_config.get_send_flag_decisions_value(): self._send_impression_event( - project_config, decision.experiment, decision.variation, user_id, attributes, feature.key, decision.source + project_config, decision.experiment, decision.variation, user_id, attributes, feature.key, + decision.source ) if decision.variation: @@ -590,7 +592,8 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): 'variation_key': decision.variation.key, } self._send_impression_event( - project_config, decision.experiment, decision.variation, user_id, attributes, feature.key, decision.source + project_config, decision.experiment, decision.variation, user_id, attributes, feature.key, + decision.source ) if feature_enabled: diff --git a/tests/test_event_factory.py b/tests/test_event_factory.py index e98ec96ca..b73b44e3e 100644 --- a/tests/test_event_factory.py +++ b/tests/test_event_factory.py @@ -77,8 +77,7 @@ def test_create_impression_event(self): {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'flag_key', 'flag_type': 'experiment', - 'variation_key': 'variation'} - } + 'variation_key': 'variation'}} ], 'events': [ { diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 3cf3ec885..f799a60a7 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -1998,8 +1998,7 @@ def test_is_feature_enabled__returns_true_for_feature_experiment_if_feature_enab {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'test_feature_in_experiment', 'flag_type': 'feature-test', - 'variation_key': 'variation'}, - } + 'variation_key': 'variation'}} ], 'events': [ { @@ -2096,8 +2095,7 @@ def test_is_feature_enabled__returns_false_for_feature_experiment_if_feature_dis {'variation_id': '111128', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'test_feature_in_experiment', 'flag_type': 'feature-test', - 'variation_key': 'control'}, - } + 'variation_key': 'control'}} ], 'events': [ { From d64fac40c64927f395e3c66b163307953e1b48f5 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Fri, 25 Sep 2020 16:12:32 -0700 Subject: [PATCH 03/11] fix linters --- optimizely/event/payload.py | 1 + 1 file changed, 1 insertion(+) diff --git a/optimizely/event/payload.py b/optimizely/event/payload.py index 35405b094..a83001494 100644 --- a/optimizely/event/payload.py +++ b/optimizely/event/payload.py @@ -67,6 +67,7 @@ def __init__(self, campaign_id, experiment_id, variation_id, metadata): self.variation_id = variation_id self.metadata = metadata + class Metadata(object): """ Class respresenting Metadata. """ From b1790ab2d4029b7906a374543b00a6099b0102ae Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Mon, 28 Sep 2020 12:35:52 -0700 Subject: [PATCH 04/11] fixing tests --- tests/test_event_processor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_event_processor.py b/tests/test_event_processor.py index 14dc40ac0..d1fffb08d 100644 --- a/tests/test_event_processor.py +++ b/tests/test_event_processor.py @@ -83,7 +83,7 @@ def dispatch_event(self, actual_log_event): for visitor in visitors: for snapshot in visitor.snapshots: - decisions = snapshot.get('decisions') or [Decision(None, None, None)] + decisions = snapshot.get('decisions') or [Decision(None, None, None, None)] for decision in decisions: for event in snapshot.get('events'): attributes = visitor.attributes From b9d45c9833ddc81e618a07e5248718f831a66b33 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Tue, 29 Sep 2020 15:30:14 -0700 Subject: [PATCH 05/11] addressing PR comments --- optimizely/event/user_event_factory.py | 12 ++++++++--- optimizely/optimizely.py | 20 ++++++++++-------- tests/base.py | 1 + tests/test_config.py | 14 +++++++++++++ tests/test_event_factory.py | 29 +++++++++++++------------- tests/test_optimizely.py | 22 +++++++++---------- tests/test_user_event_factory.py | 6 +++--- 7 files changed, 63 insertions(+), 41 deletions(-) diff --git a/optimizely/event/user_event_factory.py b/optimizely/event/user_event_factory.py index ef01a41b3..b471f1489 100644 --- a/optimizely/event/user_event_factory.py +++ b/optimizely/event/user_event_factory.py @@ -13,6 +13,7 @@ from . import event_factory from . import user_event +from optimizely.helpers import enums class UserEventFactory(object): @@ -20,7 +21,7 @@ class UserEventFactory(object): @classmethod def create_impression_event( - cls, project_config, activated_experiment, variation_id, user_id, user_attributes, flag_key, flag_type + cls, project_config, activated_experiment, variation_id, flag_key, flag_type, user_id, user_attributes ): """ Create impression Event to be sent to the logging endpoint. @@ -28,6 +29,8 @@ def create_impression_event( project_config: Instance of ProjectConfig. experiment: Experiment for which impression needs to be recorded. variation_id: ID for variation which would be presented to user. + flag_key: key for a feature flag or experiment + flag_type: type for the source user_id: ID for user. attributes: Dict representing user attributes and values which need to be recorded. @@ -36,10 +39,13 @@ def create_impression_event( - activated_experiment is None. """ - if not activated_experiment: + if not activated_experiment and flag_type is not enums.DecisionSources.ROLLOUT: return None - experiment_key = activated_experiment.key + if activated_experiment: + experiment_key = activated_experiment.key + else: + experiment_key = None variation = project_config.get_variation_from_id(experiment_key, variation_id) event_context = user_event.EventContext( diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 3a72fcf38..9f775ff93 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -160,19 +160,21 @@ def _validate_user_inputs(self, attributes=None, event_tags=None): return True - def _send_impression_event(self, project_config, experiment, variation, user_id, attributes, flag_key, flag_type): + def _send_impression_event(self, project_config, experiment, variation, flag_key, flag_type, user_id, attributes): """ Helper method to send impression event. Args: project_config: Instance of ProjectConfig. experiment: Experiment for which impression event is being sent. variation: Variation picked for user for the given experiment. + flag_key: key for a feature flag or experiment + flag_type: type for the source user_id: ID for user. attributes: Dict representing user attributes and values which need to be recorded. """ - + variation_id = variation.id if variation is not None else None user_event = user_event_factory.UserEventFactory.create_impression_event( - project_config, experiment, variation.id, user_id, attributes, flag_key, flag_type + project_config, experiment, variation_id, flag_key, flag_type, user_id, attributes ) self.event_processor.process(user_event) @@ -422,8 +424,8 @@ def activate(self, experiment_key, user_id, attributes=None): # Create and dispatch impression event self.logger.info('Activating user "%s" in experiment "%s".' % (user_id, experiment.key)) - self._send_impression_event(project_config, experiment, variation, user_id, attributes, experiment.key, - "experiment") + self._send_impression_event(project_config, experiment, variation, experiment.key, + "experiment", user_id, attributes) return variation.key @@ -578,8 +580,8 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): if is_source_rollout and project_config.get_send_flag_decisions_value(): self._send_impression_event( - project_config, decision.experiment, decision.variation, user_id, attributes, feature.key, - decision.source + project_config, decision.experiment, decision.variation, feature.key, + decision.source, user_id, attributes ) if decision.variation: @@ -592,8 +594,8 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): 'variation_key': decision.variation.key, } self._send_impression_event( - project_config, decision.experiment, decision.variation, user_id, attributes, feature.key, - decision.source + project_config, decision.experiment, decision.variation, feature.key, + decision.source, user_id, attributes ) if feature_enabled: diff --git a/tests/base.py b/tests/base.py index 9dceec2d0..88d5b73ff 100644 --- a/tests/base.py +++ b/tests/base.py @@ -129,6 +129,7 @@ def setUp(self, config_dict='config_dict'): 'projectId': '111111', 'version': '4', 'botFiltering': True, + 'sendFlagDecisions': True, 'events': [{'key': 'test_event', 'experimentIds': ['111127'], 'id': '111095'}], 'experiments': [ { diff --git a/tests/test_config.py b/tests/test_config.py index 6ef70133d..e88364714 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -613,6 +613,20 @@ def test_get_bot_filtering(self): self.config_dict_with_features['botFiltering'], project_config.get_bot_filtering_value(), ) + def test_get_send_flag_decisions(self): + """ Test that send_flag_decisions is retrieved correctly when using get_send_flag_decisions_value. """ + + # Assert send_flag_decisions is None when not provided in data file + self.assertTrue('sendFlagDecisions' not in self.config_dict) + self.assertFalse(self.project_config.get_send_flag_decisions_value()) + + # Assert send_flag_decisions is retrieved as provided in the data file + opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) + project_config = opt_obj.config_manager.get_config() + self.assertEqual( + self.config_dict_with_features['sendFlagDecisions'], project_config.get_send_flag_decisions_value(), + ) + def test_get_experiment_from_key__valid_key(self): """ Test that experiment is retrieved correctly for valid experiment key. """ diff --git a/tests/test_event_factory.py b/tests/test_event_factory.py index b73b44e3e..e03b6ab1a 100644 --- a/tests/test_event_factory.py +++ b/tests/test_event_factory.py @@ -105,10 +105,10 @@ def test_create_impression_event(self): self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', + 'flag_key', + 'experiment', 'test_user', None, - 'flag_key', - 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -165,10 +165,10 @@ def test_create_impression_event__with_attributes(self): self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', + 'flag_key', + 'experiment', 'test_user', {'test_attribute': 'test_value'}, - 'flag_key', - 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -223,11 +223,10 @@ def test_create_impression_event_when_attribute_is_not_in_datafile(self): self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', + 'flag_key', + 'experiment', 'test_user', {'do_you_know_me': 'test_value'}, - 'flag_key', - 'experiment' - ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -302,10 +301,10 @@ def side_effect(*args, **kwargs): self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', + 'flag_key', + 'experiment', 'test_user', attributes, - 'flag_key', - 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -375,10 +374,10 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_enabled( self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', + 'flag_key', + 'experiment', 'test_user', {'$opt_user_agent': 'Edge'}, - 'flag_key', - 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -443,10 +442,10 @@ def test_create_impression_event__with_empty_attributes_when_bot_filtering_is_en self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', + 'flag_key', + 'experiment', 'test_user', None, - 'flag_key', - 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) @@ -517,10 +516,10 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled self.project_config, self.project_config.get_experiment_from_key('test_experiment'), '111129', + 'flag_key', + 'experiment', 'test_user', {'$opt_user_agent': 'Chrome'}, - 'flag_key', - 'experiment' ) log_event = EventFactory.create_log_event(event_obj, self.logger) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index f799a60a7..88f2f09c6 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -667,9 +667,9 @@ def on_activate(experiment, user_id, attributes, variation, event): mock_decision.assert_called_once_with(project_config, feature, 'test_user', None) - # Check that impression event is not sent - self.assertEqual(0, mock_process.call_count) - self.assertEqual(False, access_callback[0]) + # Check that impression event is sent for rollout and send_flag_decisions = True + self.assertEqual(1, mock_process.call_count) + self.assertEqual(True, access_callback[0]) def test_activate__with_attributes__audience_match(self): """ Test that activate calls process with right params and returns expected @@ -2171,8 +2171,8 @@ def test_is_feature_enabled__returns_true_for_feature_rollout_if_feature_enabled }, ) - # Check that impression event is not sent - self.assertEqual(0, mock_process.call_count) + # Check that impression event is sent for rollout and send_flag_decisions = True + self.assertEqual(1, mock_process.call_count) def test_is_feature_enabled__returns_true_for_feature_rollout_if_feature_enabled_with_sending_decisions(self,): """ Test that the feature is enabled for the user if bucketed into variation of a rollout and @@ -2317,8 +2317,8 @@ def test_is_feature_enabled__returns_false_for_feature_rollout_if_feature_disabl }, ) - # Check that impression event is not sent - self.assertEqual(0, mock_process.call_count) + # Check that impression event is sent for rollout and send_flag_decisions = True + self.assertEqual(1, mock_process.call_count) def test_is_feature_enabled__returns_false_when_user_is_not_bucketed_into_any_variation(self,): """ Test that the feature is not enabled for the user if user is neither bucketed for @@ -2342,8 +2342,8 @@ def test_is_feature_enabled__returns_false_when_user_is_not_bucketed_into_any_va ): self.assertFalse(opt_obj.is_feature_enabled('test_feature_in_experiment', 'test_user')) - # Check that impression event is not sent - self.assertEqual(0, mock_process.call_count) + # Check that impression event is sent for rollout and send_flag_decisions = True + self.assertEqual(1, mock_process.call_count) mock_decision.assert_called_once_with(opt_obj.config_manager.get_config(), feature, 'test_user', None) @@ -2360,8 +2360,8 @@ def test_is_feature_enabled__returns_false_when_user_is_not_bucketed_into_any_va }, ) - # Check that impression event is not sent - self.assertEqual(0, mock_process.call_count) + # Check that impression event is sent for rollout and send_flag_decisions = True + self.assertEqual(1, mock_process.call_count) def test_is_feature_enabled__invalid_object(self): """ Test that is_feature_enabled returns False and logs error if Optimizely instance is invalid. """ diff --git a/tests/test_user_event_factory.py b/tests/test_user_event_factory.py index c45e4d999..ec31f2224 100644 --- a/tests/test_user_event_factory.py +++ b/tests/test_user_event_factory.py @@ -28,8 +28,8 @@ def test_impression_event(self): variation = self.project_config.get_variation_from_id(experiment.key, '111128') user_id = 'test_user' - impression_event = UserEventFactory.create_impression_event(project_config, experiment, '111128', user_id, None, - 'flag_key', 'flag_type') + impression_event = UserEventFactory.create_impression_event(project_config, experiment, '111128', 'flag_key', + 'flag_type', user_id, None) self.assertEqual(self.project_config.project_id, impression_event.event_context.project_id) self.assertEqual(self.project_config.revision, impression_event.event_context.revision) @@ -51,7 +51,7 @@ def test_impression_event__with_attributes(self): user_attributes = {'test_attribute': 'test_value', 'boolean_key': True} impression_event = UserEventFactory.create_impression_event( - project_config, experiment, '111128', user_id, user_attributes, 'flag_key', 'flag_type' + project_config, experiment, '111128', 'flag_key', 'flag_type', user_id, user_attributes ) expected_attrs = EventFactory.build_attribute_list(user_attributes, project_config) From 6c0354caf6d4cc96f16807c8482572cff4ad9c86 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Wed, 30 Sep 2020 14:37:20 -0700 Subject: [PATCH 06/11] addressing PR comments --- optimizely/event/event_factory.py | 2 +- optimizely/event/payload.py | 4 ++-- optimizely/event/user_event_factory.py | 5 ++++- tests/test_event_payload.py | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/optimizely/event/event_factory.py b/optimizely/event/event_factory.py index 8de72ac03..c337957c7 100644 --- a/optimizely/event/event_factory.py +++ b/optimizely/event/event_factory.py @@ -90,7 +90,7 @@ def _create_visitor(cls, event, logger): if isinstance(event, user_event.ImpressionEvent): - metadata = payload.Metadata(event.variation.key, event.flag_key, event.flag_type) + metadata = payload.Metadata(event.flag_key, event.flag_type, event.variation.key) decision = payload.Decision(event.experiment.layerId, event.experiment.id, event.variation.id, metadata) diff --git a/optimizely/event/payload.py b/optimizely/event/payload.py index a83001494..e4c95cc9e 100644 --- a/optimizely/event/payload.py +++ b/optimizely/event/payload.py @@ -71,10 +71,10 @@ def __init__(self, campaign_id, experiment_id, variation_id, metadata): class Metadata(object): """ Class respresenting Metadata. """ - def __init__(self, variation_key, flag_key, flag_type): - self.variation_key = variation_key + def __init__(self, flag_key, flag_type, variation_key): self.flag_key = flag_key self.flag_type = flag_type + self.variation_key = variation_key class Snapshot(object): diff --git a/optimizely/event/user_event_factory.py b/optimizely/event/user_event_factory.py index b471f1489..773c902df 100644 --- a/optimizely/event/user_event_factory.py +++ b/optimizely/event/user_event_factory.py @@ -46,8 +46,11 @@ def create_impression_event( experiment_key = activated_experiment.key else: experiment_key = None - variation = project_config.get_variation_from_id(experiment_key, variation_id) + if variation_id: + variation = project_config.get_variation_from_id(experiment_key, variation_id) + else: + variation = None event_context = user_event.EventContext( project_config.account_id, project_config.project_id, project_config.revision, project_config.anonymize_ip, ) diff --git a/tests/test_event_payload.py b/tests/test_event_payload.py index 4847a4b6f..e68f78ee9 100644 --- a/tests/test_event_payload.py +++ b/tests/test_event_payload.py @@ -58,7 +58,7 @@ def test_impression_event_equals_serialized_payload(self): batch = payload.EventBatch('12001', '111001', '42', 'python-sdk', version.__version__, False, True) visitor_attr = payload.VisitorAttribute('111094', 'test_attribute', 'custom', 'test_value') event = payload.SnapshotEvent('111182', 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', 'campaign_activated', 42123,) - metadata = payload.Metadata('variation', 'flag_key', 'experiment') + metadata = payload.Metadata('flag_key', 'experiment', 'variation') event_decision = payload.Decision('111182', '111127', '111129', metadata) snapshots = payload.Snapshot([event], [event_decision]) From fd986506c8b269589e599dba8d4653b09a1f429b Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Tue, 6 Oct 2020 17:22:46 -0700 Subject: [PATCH 07/11] addressing PR comments --- optimizely/event/event_factory.py | 3 --- optimizely/event/user_event_factory.py | 7 ++----- optimizely/helpers/enums.py | 1 + optimizely/optimizely.py | 2 +- 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/optimizely/event/event_factory.py b/optimizely/event/event_factory.py index c337957c7..5b4a63498 100644 --- a/optimizely/event/event_factory.py +++ b/optimizely/event/event_factory.py @@ -89,11 +89,8 @@ def _create_visitor(cls, event, logger): """ if isinstance(event, user_event.ImpressionEvent): - metadata = payload.Metadata(event.flag_key, event.flag_type, event.variation.key) - decision = payload.Decision(event.experiment.layerId, event.experiment.id, event.variation.id, metadata) - snapshot_event = payload.SnapshotEvent( event.experiment.layerId, event.uuid, cls.ACTIVATE_EVENT_KEY, event.timestamp, ) diff --git a/optimizely/event/user_event_factory.py b/optimizely/event/user_event_factory.py index 773c902df..231e9b3ce 100644 --- a/optimizely/event/user_event_factory.py +++ b/optimizely/event/user_event_factory.py @@ -42,15 +42,12 @@ def create_impression_event( if not activated_experiment and flag_type is not enums.DecisionSources.ROLLOUT: return None + variation, experiment_key = None, None if activated_experiment: experiment_key = activated_experiment.key - else: - experiment_key = None - if variation_id: + if variation_id and experiment_key: variation = project_config.get_variation_from_id(experiment_key, variation_id) - else: - variation = None event_context = user_event.EventContext( project_config.account_id, project_config.project_id, project_config.revision, project_config.anonymize_ip, ) diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 3eed4a304..5685f9c87 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -89,6 +89,7 @@ class DecisionNotificationTypes(object): class DecisionSources(object): + EXPERIMENT = 'experiment' FEATURE_TEST = 'feature-test' ROLLOUT = 'rollout' diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 9f775ff93..685ab376b 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -425,7 +425,7 @@ def activate(self, experiment_key, user_id, attributes=None): # Create and dispatch impression event self.logger.info('Activating user "%s" in experiment "%s".' % (user_id, experiment.key)) self._send_impression_event(project_config, experiment, variation, experiment.key, - "experiment", user_id, attributes) + enums.DecisionSources.EXPERIMENT, user_id, attributes) return variation.key From cc8147651d0ef834674d73ae39fb14968f08a8c9 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Tue, 6 Oct 2020 17:30:44 -0700 Subject: [PATCH 08/11] added changes --- CHANGELOG.md | 10 ++++++++++ optimizely/version.py | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb0effdc7..8a20fce28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Optimizely Python SDK Changelog +## 3.6.0 +October 1st, 2020 + +### New Features: +* Version targeting using semantic version syntax. [#293](https://github.com/optimizely/python-sdk/pull/293) +* Datafile accessor API added to access current config as a JSON string. [#283](https://github.com/optimizely/python-sdk/pull/283) + +### Bug Fixes: +* Fixed package installation for Python 3.4 and pypy. [#298](https://github.com/optimizely/python-sdk/pull/298) + ## 3.5.2 July 14th, 2020 diff --git a/optimizely/version.py b/optimizely/version.py index d7880e2cc..a8983656d 100644 --- a/optimizely/version.py +++ b/optimizely/version.py @@ -11,5 +11,5 @@ # See the License for the specific language governing permissions and # limitations under the License. -version_info = (3, 5, 2) +version_info = (3, 6, 0) __version__ = '.'.join(str(v) for v in version_info) From d46dac58903734fc89024aba511e3ef63e331d51 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Wed, 7 Oct 2020 16:47:55 -0700 Subject: [PATCH 09/11] added revised changes --- optimizely/event/event_factory.py | 2 +- optimizely/event/payload.py | 5 ++-- optimizely/event/user_event.py | 5 ++-- optimizely/event/user_event_factory.py | 12 +++++---- optimizely/optimizely.py | 17 +++++++------ tests/test_event_factory.py | 24 +++++++++++++----- tests/test_event_payload.py | 5 ++-- tests/test_optimizely.py | 34 ++++++++++++++++---------- tests/test_user_event_factory.py | 4 +-- 9 files changed, 67 insertions(+), 41 deletions(-) diff --git a/optimizely/event/event_factory.py b/optimizely/event/event_factory.py index 5b4a63498..f9e59b1b1 100644 --- a/optimizely/event/event_factory.py +++ b/optimizely/event/event_factory.py @@ -89,7 +89,7 @@ def _create_visitor(cls, event, logger): """ if isinstance(event, user_event.ImpressionEvent): - metadata = payload.Metadata(event.flag_key, event.flag_type, event.variation.key) + metadata = payload.Metadata(event.flag_key, event.rule_key, event.rule_type, event.variation.key) decision = payload.Decision(event.experiment.layerId, event.experiment.id, event.variation.id, metadata) snapshot_event = payload.SnapshotEvent( event.experiment.layerId, event.uuid, cls.ACTIVATE_EVENT_KEY, event.timestamp, diff --git a/optimizely/event/payload.py b/optimizely/event/payload.py index e4c95cc9e..53b24b9e1 100644 --- a/optimizely/event/payload.py +++ b/optimizely/event/payload.py @@ -71,9 +71,10 @@ def __init__(self, campaign_id, experiment_id, variation_id, metadata): class Metadata(object): """ Class respresenting Metadata. """ - def __init__(self, flag_key, flag_type, variation_key): + def __init__(self, flag_key, rule_key, rule_type, variation_key): self.flag_key = flag_key - self.flag_type = flag_type + self.rule_key = rule_key + self.rule_type = rule_type self.variation_key = variation_key diff --git a/optimizely/event/user_event.py b/optimizely/event/user_event.py index 56d34c875..57b2c2e5f 100644 --- a/optimizely/event/user_event.py +++ b/optimizely/event/user_event.py @@ -41,14 +41,15 @@ class ImpressionEvent(UserEvent): """ Class representing Impression Event. """ def __init__( - self, event_context, user_id, experiment, visitor_attributes, variation, flag_key, flag_type, + self, event_context, user_id, experiment, visitor_attributes, variation, flag_key, rule_key, rule_type, bot_filtering=None, ): super(ImpressionEvent, self).__init__(event_context, user_id, visitor_attributes, bot_filtering) self.experiment = experiment self.variation = variation self.flag_key = flag_key - self.flag_type = flag_type + self.rule_key = rule_key + self.rule_type = rule_type class ConversionEvent(UserEvent): diff --git a/optimizely/event/user_event_factory.py b/optimizely/event/user_event_factory.py index 231e9b3ce..8fcee7f8d 100644 --- a/optimizely/event/user_event_factory.py +++ b/optimizely/event/user_event_factory.py @@ -21,7 +21,7 @@ class UserEventFactory(object): @classmethod def create_impression_event( - cls, project_config, activated_experiment, variation_id, flag_key, flag_type, user_id, user_attributes + cls, project_config, activated_experiment, variation_id, flag_key, rule_key, rule_type, user_id, user_attributes ): """ Create impression Event to be sent to the logging endpoint. @@ -29,8 +29,9 @@ def create_impression_event( project_config: Instance of ProjectConfig. experiment: Experiment for which impression needs to be recorded. variation_id: ID for variation which would be presented to user. - flag_key: key for a feature flag or experiment - flag_type: type for the source + flag_key: key for a feature flag + rule_key: key for an experiment + rule_type: type for the source user_id: ID for user. attributes: Dict representing user attributes and values which need to be recorded. @@ -39,7 +40,7 @@ def create_impression_event( - activated_experiment is None. """ - if not activated_experiment and flag_type is not enums.DecisionSources.ROLLOUT: + if not activated_experiment and rule_type is not enums.DecisionSources.ROLLOUT: return None variation, experiment_key = None, None @@ -59,7 +60,8 @@ def create_impression_event( event_factory.EventFactory.build_attribute_list(user_attributes, project_config), variation, flag_key, - flag_type, + rule_key, + rule_type, project_config.get_bot_filtering_value(), ) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 685ab376b..f94713a1c 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -160,21 +160,22 @@ def _validate_user_inputs(self, attributes=None, event_tags=None): return True - def _send_impression_event(self, project_config, experiment, variation, flag_key, flag_type, user_id, attributes): + def _send_impression_event(self, project_config, experiment, variation, flag_key, rule_key, rule_type, user_id, attributes): """ Helper method to send impression event. Args: project_config: Instance of ProjectConfig. experiment: Experiment for which impression event is being sent. variation: Variation picked for user for the given experiment. - flag_key: key for a feature flag or experiment - flag_type: type for the source + flag_key: key for a feature flag + rule_key: key for an experiment + rule_type: type for the source user_id: ID for user. attributes: Dict representing user attributes and values which need to be recorded. """ variation_id = variation.id if variation is not None else None user_event = user_event_factory.UserEventFactory.create_impression_event( - project_config, experiment, variation_id, flag_key, flag_type, user_id, attributes + project_config, experiment, variation_id, flag_key, rule_key, rule_type, user_id, attributes ) self.event_processor.process(user_event) @@ -424,7 +425,7 @@ def activate(self, experiment_key, user_id, attributes=None): # Create and dispatch impression event self.logger.info('Activating user "%s" in experiment "%s".' % (user_id, experiment.key)) - self._send_impression_event(project_config, experiment, variation, experiment.key, + self._send_impression_event(project_config, experiment, variation, "", experiment.key, enums.DecisionSources.EXPERIMENT, user_id, attributes) return variation.key @@ -580,8 +581,8 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): if is_source_rollout and project_config.get_send_flag_decisions_value(): self._send_impression_event( - project_config, decision.experiment, decision.variation, feature.key, - decision.source, user_id, attributes + project_config, decision.experiment, decision.variation, feature.key, decision.experiment.key if + decision.experiment else '', decision.source, user_id, attributes ) if decision.variation: @@ -594,7 +595,7 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None): 'variation_key': decision.variation.key, } self._send_impression_event( - project_config, decision.experiment, decision.variation, feature.key, + project_config, decision.experiment, decision.variation, feature.key, decision.experiment.key, decision.source, user_id, attributes ) diff --git a/tests/test_event_factory.py b/tests/test_event_factory.py index e03b6ab1a..93e5db7c6 100644 --- a/tests/test_event_factory.py +++ b/tests/test_event_factory.py @@ -76,7 +76,8 @@ def test_create_impression_event(self): 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'flag_key', - 'flag_type': 'experiment', + 'rule_key': 'rule_key', + 'rule_type': 'experiment', 'variation_key': 'variation'}} ], 'events': [ @@ -106,6 +107,7 @@ def test_create_impression_event(self): self.project_config.get_experiment_from_key('test_experiment'), '111129', 'flag_key', + 'rule_key', 'experiment', 'test_user', None, @@ -135,7 +137,8 @@ def test_create_impression_event__with_attributes(self): 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'flag_key', - 'flag_type': 'experiment', + 'rule_key': 'rule_key', + 'rule_type': 'experiment', 'variation_key': 'variation'}, } ], @@ -166,6 +169,7 @@ def test_create_impression_event__with_attributes(self): self.project_config.get_experiment_from_key('test_experiment'), '111129', 'flag_key', + 'rule_key', 'experiment', 'test_user', {'test_attribute': 'test_value'}, @@ -193,7 +197,8 @@ def test_create_impression_event_when_attribute_is_not_in_datafile(self): 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'flag_key', - 'flag_type': 'experiment', + 'rule_key': 'rule_key', + 'rule_type': 'experiment', 'variation_key': 'variation'} } ], @@ -224,6 +229,7 @@ def test_create_impression_event_when_attribute_is_not_in_datafile(self): self.project_config.get_experiment_from_key('test_experiment'), '111129', 'flag_key', + 'rule_key', 'experiment', 'test_user', {'do_you_know_me': 'test_value'}, @@ -342,7 +348,8 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_enabled( 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'flag_key', - 'flag_type': 'experiment', + 'rule_key': 'rule_key', + 'rule_type': 'experiment', 'variation_key': 'variation'}, } ], @@ -375,6 +382,7 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_enabled( self.project_config.get_experiment_from_key('test_experiment'), '111129', 'flag_key', + 'rule_key', 'experiment', 'test_user', {'$opt_user_agent': 'Edge'}, @@ -410,7 +418,8 @@ def test_create_impression_event__with_empty_attributes_when_bot_filtering_is_en 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'flag_key', - 'flag_type': 'experiment', + 'rule_key': 'rule_key', + 'rule_type': 'experiment', 'variation_key': 'variation'}, } ], @@ -443,6 +452,7 @@ def test_create_impression_event__with_empty_attributes_when_bot_filtering_is_en self.project_config.get_experiment_from_key('test_experiment'), '111129', 'flag_key', + 'rule_key', 'experiment', 'test_user', None, @@ -484,7 +494,8 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'flag_key', - 'flag_type': 'experiment', + 'rule_key': 'rule_key', + 'rule_type': 'experiment', 'variation_key': 'variation'}, } ], @@ -517,6 +528,7 @@ def test_create_impression_event__with_user_agent_when_bot_filtering_is_disabled self.project_config.get_experiment_from_key('test_experiment'), '111129', 'flag_key', + 'rule_key', 'experiment', 'test_user', {'$opt_user_agent': 'Chrome'}, diff --git a/tests/test_event_payload.py b/tests/test_event_payload.py index e68f78ee9..ae168d8ee 100644 --- a/tests/test_event_payload.py +++ b/tests/test_event_payload.py @@ -32,7 +32,8 @@ def test_impression_event_equals_serialized_payload(self): 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'flag_key', - 'flag_type': 'experiment', + 'rule_key': 'rule_key', + 'rule_type': 'experiment', 'variation_key': 'variation'}, } ], @@ -58,7 +59,7 @@ def test_impression_event_equals_serialized_payload(self): batch = payload.EventBatch('12001', '111001', '42', 'python-sdk', version.__version__, False, True) visitor_attr = payload.VisitorAttribute('111094', 'test_attribute', 'custom', 'test_value') event = payload.SnapshotEvent('111182', 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', 'campaign_activated', 42123,) - metadata = payload.Metadata('flag_key', 'experiment', 'variation') + metadata = payload.Metadata('flag_key', 'rule_key', 'experiment', 'variation') event_decision = payload.Decision('111182', '111127', '111129', metadata) snapshots = payload.Snapshot([event], [event_decision]) diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 88f2f09c6..35c0004c9 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -322,8 +322,9 @@ def test_activate(self): { 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'test_experiment', - 'flag_type': 'experiment', + 'metadata': {'flag_key': '', + 'rule_key': 'test_experiment', + 'rule_type': 'experiment', 'variation_key': 'variation'}, } ], @@ -699,8 +700,9 @@ def test_activate__with_attributes__audience_match(self): { 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'test_experiment', - 'flag_type': 'experiment', + 'metadata': {'flag_key': '', + 'rule_key': 'test_experiment', + 'rule_type': 'experiment', 'variation_key': 'variation'}, } ], @@ -780,8 +782,9 @@ def test_activate__with_attributes_of_different_types(self): { 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'test_experiment', - 'flag_type': 'experiment', + 'metadata': {'flag_key': '', + 'rule_key': 'test_experiment', + 'rule_type': 'experiment', 'variation_key': 'variation'}, } ], @@ -975,8 +978,9 @@ def test_activate__with_attributes__audience_match__forced_bucketing(self): { 'decisions': [ {'variation_id': '111128', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'test_experiment', - 'flag_type': 'experiment', + 'metadata': {'flag_key': '', + 'rule_key': 'test_experiment', + 'rule_type': 'experiment', 'variation_key': 'control'}, } ], @@ -1049,8 +1053,9 @@ def test_activate__with_attributes__audience_match__bucketing_id_provided(self): { 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', - 'metadata': {'flag_key': 'test_experiment', - 'flag_type': 'experiment', + 'metadata': {'flag_key': '', + 'rule_key': 'test_experiment', + 'rule_type': 'experiment', 'variation_key': 'variation'}, } ], @@ -1997,7 +2002,8 @@ def test_is_feature_enabled__returns_true_for_feature_experiment_if_feature_enab 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'test_feature_in_experiment', - 'flag_type': 'feature-test', + 'rule_key': 'test_experiment', + 'rule_type': 'feature-test', 'variation_key': 'variation'}} ], 'events': [ @@ -2094,7 +2100,8 @@ def test_is_feature_enabled__returns_false_for_feature_experiment_if_feature_dis 'decisions': [ {'variation_id': '111128', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'test_feature_in_experiment', - 'flag_type': 'feature-test', + 'rule_key': 'test_experiment', + 'rule_type': 'feature-test', 'variation_key': 'control'}} ], 'events': [ @@ -2239,7 +2246,8 @@ def test_is_feature_enabled__returns_true_for_feature_rollout_if_feature_enabled 'decisions': [ {'variation_id': '111129', 'experiment_id': '111127', 'campaign_id': '111182', 'metadata': {'flag_key': 'test_feature_in_experiment', - 'flag_type': 'rollout', + 'rule_key': 'test_experiment', + 'rule_type': 'rollout', 'variation_key': 'variation'}, } ], diff --git a/tests/test_user_event_factory.py b/tests/test_user_event_factory.py index ec31f2224..4e8c2845d 100644 --- a/tests/test_user_event_factory.py +++ b/tests/test_user_event_factory.py @@ -29,7 +29,7 @@ def test_impression_event(self): user_id = 'test_user' impression_event = UserEventFactory.create_impression_event(project_config, experiment, '111128', 'flag_key', - 'flag_type', user_id, None) + 'rule_key', 'rule_type', user_id, None) self.assertEqual(self.project_config.project_id, impression_event.event_context.project_id) self.assertEqual(self.project_config.revision, impression_event.event_context.revision) @@ -51,7 +51,7 @@ def test_impression_event__with_attributes(self): user_attributes = {'test_attribute': 'test_value', 'boolean_key': True} impression_event = UserEventFactory.create_impression_event( - project_config, experiment, '111128', 'flag_key', 'flag_type', user_id, user_attributes + project_config, experiment, '111128', 'flag_key', 'rule_key', 'rule_type', user_id, user_attributes ) expected_attrs = EventFactory.build_attribute_list(user_attributes, project_config) From 0bdee33fed6b9f96561ecc945f7448a4d73d0e77 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Thu, 8 Oct 2020 09:11:03 -0700 Subject: [PATCH 10/11] fix linters --- optimizely/optimizely.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index f94713a1c..2c34f5223 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -160,7 +160,8 @@ def _validate_user_inputs(self, attributes=None, event_tags=None): return True - def _send_impression_event(self, project_config, experiment, variation, flag_key, rule_key, rule_type, user_id, attributes): + def _send_impression_event(self, project_config, experiment, variation, flag_key, rule_key, rule_type, user_id, + attributes): """ Helper method to send impression event. Args: From 2b9e45d4ba095a861e4352931afa3264520688da Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Thu, 8 Oct 2020 10:28:18 -0700 Subject: [PATCH 11/11] corrected nits --- optimizely/event/user_event_factory.py | 6 +++--- optimizely/optimizely.py | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/optimizely/event/user_event_factory.py b/optimizely/event/user_event_factory.py index 8fcee7f8d..002bee17b 100644 --- a/optimizely/event/user_event_factory.py +++ b/optimizely/event/user_event_factory.py @@ -29,9 +29,9 @@ def create_impression_event( project_config: Instance of ProjectConfig. experiment: Experiment for which impression needs to be recorded. variation_id: ID for variation which would be presented to user. - flag_key: key for a feature flag - rule_key: key for an experiment - rule_type: type for the source + flag_key: key for a feature flag. + rule_key: key for an experiment. + rule_type: type for the source. user_id: ID for user. attributes: Dict representing user attributes and values which need to be recorded. diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 2c34f5223..afd6f3829 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -168,9 +168,9 @@ def _send_impression_event(self, project_config, experiment, variation, flag_key project_config: Instance of ProjectConfig. experiment: Experiment for which impression event is being sent. variation: Variation picked for user for the given experiment. - flag_key: key for a feature flag - rule_key: key for an experiment - rule_type: type for the source + flag_key: key for a feature flag. + rule_key: key for an experiment. + rule_type: type for the source. user_id: ID for user. attributes: Dict representing user attributes and values which need to be recorded. """ @@ -426,7 +426,7 @@ def activate(self, experiment_key, user_id, attributes=None): # Create and dispatch impression event self.logger.info('Activating user "%s" in experiment "%s".' % (user_id, experiment.key)) - self._send_impression_event(project_config, experiment, variation, "", experiment.key, + self._send_impression_event(project_config, experiment, variation, '', experiment.key, enums.DecisionSources.EXPERIMENT, user_id, attributes) return variation.key