Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

experiment_id and variation_id added to payloads #447

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
# Line break before operand needs to be ignored for line lengths
# greater than max-line-length. Best practice shows W504
ignore = E722, W504
exclude = optimizely/lib/pymmh3.py,*virtualenv*
exclude = optimizely/lib/pymmh3.py,*virtualenv*,tests/testapp/application.py
max-line-length = 120
22 changes: 20 additions & 2 deletions optimizely/optimizely.py
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,10 @@ def get_variation(
decision_notification_type,
user_id,
attributes or {},
{'experiment_key': experiment_key, 'variation_key': variation_key},
{
'experiment_key': experiment_key,
'variation_key': variation_key,
},
)

return variation_key
Expand Down Expand Up @@ -1202,6 +1205,19 @@ def _create_optimizely_decision(
if flag_decision is not None and flag_decision.variation is not None
else None
)

rollout_id = None
if decision_source == DecisionSources.ROLLOUT and feature_flag is not None:
rollout_id = feature_flag.rolloutId
experiment_id = None
if rule_key is not None:
experiment_id = project_config.get_experiment_id_by_key_or_rollout_id(rule_key, rollout_id)
variation_id = None
if experiment_id and variation_key:
variation = project_config.get_variation_from_key_by_experiment_id(experiment_id, variation_key)
if variation:
variation_id = variation.id

# Send notification
self.notification_center.send_notifications(
enums.NotificationTypes.DECISION,
Expand All @@ -1215,7 +1231,9 @@ def _create_optimizely_decision(
'variation_key': variation_key,
'rule_key': rule_key,
'reasons': decision_reasons if should_include_reasons else [],
'decision_event_dispatched': decision_event_dispatched
'decision_event_dispatched': decision_event_dispatched,
'experiment_id': experiment_id,
'variation_id': variation_id

},
)
Expand Down
28 changes: 28 additions & 0 deletions optimizely/project_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -716,3 +716,31 @@ def get_flag_variation(
return variation

return None

def get_experiment_id_by_key_or_rollout_id(self, key: str, rollout_id: Optional[str] = None) -> Optional[str]:
"""
Retrieves the experiment ID associated with a given rule key or a specific rollout.

Args:
key: The key associated with the experiment rule. It can be experiment key or rule key.
rollout_id: The ID of the rollout to be searched if the key is not found in the experiment key map.

Returns:
Optional[str]: The experiment ID if found, otherwise None.
"""
# Try getting the experiment from experiment_key_map first
if key:
experiment = self.get_experiment_from_key(key)
if experiment:
return experiment.id

# If key is not found in experiment_key_map, check a specific rollout (if provided)
if rollout_id:
rollout = self.get_rollout_from_id(rollout_id)
if rollout:
for experiment_data in rollout.experiments:
experiment = entities.Experiment(**experiment_data)
if experiment.key == key:
return experiment.id

return None
70 changes: 47 additions & 23 deletions tests/test_user_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,8 @@ def test_decide__feature_test(self):
'reasons': expected.reasons,
'decision_event_dispatched': True,
'variables': expected.variables,
'experiment_id': mock_experiment.id,
'variation_id': mock_variation.id
},
)

Expand Down Expand Up @@ -391,6 +393,24 @@ def test_decide_feature_rollout(self):

self.compare_opt_decisions(expected, actual)

# assert event count
self.assertEqual(1, mock_send_event.call_count)

# assert event payload
expected_experiment = project_config.get_experiment_from_key(expected.rule_key)
expected_var = project_config.get_variation_from_key(expected.rule_key, expected.variation_key)
mock_send_event.assert_called_with(
project_config,
expected_experiment,
expected_var,
expected.flag_key,
expected.rule_key,
'rollout',
expected.enabled,
'test_user',
user_attributes
)

# assert notification count
self.assertEqual(1, mock_broadcast_decision.call_count)

Expand All @@ -408,27 +428,11 @@ def test_decide_feature_rollout(self):
'reasons': expected.reasons,
'decision_event_dispatched': True,
'variables': expected.variables,
'experiment_id': expected_experiment.id,
'variation_id': expected_var.id
},
)

# assert event count
self.assertEqual(1, mock_send_event.call_count)

# assert event payload
expected_experiment = project_config.get_experiment_from_key(expected.rule_key)
expected_var = project_config.get_variation_from_key(expected.rule_key, expected.variation_key)
mock_send_event.assert_called_with(
project_config,
expected_experiment,
expected_var,
expected.flag_key,
expected.rule_key,
'rollout',
expected.enabled,
'test_user',
user_attributes
)

def test_decide_feature_rollout__send_flag_decision_false(self):
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
project_config = opt_obj.config_manager.get_config()
Expand Down Expand Up @@ -467,6 +471,8 @@ def test_decide_feature_rollout__send_flag_decision_false(self):
self.assertEqual(1, mock_broadcast_decision.call_count)

# assert notification
expected_experiment = project_config.get_experiment_from_key(expected.rule_key)
expected_var = project_config.get_variation_from_key(expected.rule_key, expected.variation_key)
mock_broadcast_decision.assert_called_with(
enums.NotificationTypes.DECISION,
'flag',
Expand All @@ -480,6 +486,8 @@ def test_decide_feature_rollout__send_flag_decision_false(self):
'reasons': expected.reasons,
'decision_event_dispatched': False,
'variables': expected.variables,
'experiment_id': expected_experiment.id,
'variation_id': expected_var.id
},
)

Expand Down Expand Up @@ -549,7 +557,9 @@ def test_decide_feature_null_variation(self):
'reasons': expected.reasons,
'decision_event_dispatched': True,
'variables': expected.variables,
},
'experiment_id': None,
'variation_id': None
}
)

# assert event count
Expand Down Expand Up @@ -632,6 +642,8 @@ def test_decide_feature_null_variation__send_flag_decision_false(self):
'reasons': expected.reasons,
'decision_event_dispatched': False,
'variables': expected.variables,
'experiment_id': None,
'variation_id': None
},
)

Expand Down Expand Up @@ -701,6 +713,8 @@ def test_decide__option__disable_decision_event(self):
'reasons': expected.reasons,
'decision_event_dispatched': False,
'variables': expected.variables,
'experiment_id': mock_experiment.id,
'variation_id': mock_variation.id,
},
)

Expand Down Expand Up @@ -773,6 +787,8 @@ def test_decide__default_option__disable_decision_event(self):
'reasons': expected.reasons,
'decision_event_dispatched': False,
'variables': expected.variables,
'experiment_id': mock_experiment.id,
'variation_id': mock_variation.id
},
)

Expand Down Expand Up @@ -834,6 +850,8 @@ def test_decide__option__exclude_variables(self):
'reasons': expected.reasons,
'decision_event_dispatched': True,
'variables': expected.variables,
'experiment_id': mock_experiment.id,
'variation_id': mock_variation.id,
},
)

Expand Down Expand Up @@ -948,6 +966,8 @@ def test_decide__option__enabled_flags_only(self):
'reasons': expected.reasons,
'decision_event_dispatched': True,
'variables': expected.variables,
'experiment_id': expected_experiment.id,
'variation_id': expected_var.id,
},
)

Expand Down Expand Up @@ -1006,7 +1026,7 @@ def test_decide__default_options__with__options(self):
enabled=True,
variables=expected_variables,
flag_key='test_feature_in_experiment',
user_context=user_context
user_context=user_context,
)

self.compare_opt_decisions(expected, actual)
Expand All @@ -1025,6 +1045,8 @@ def test_decide__default_options__with__options(self):
'reasons': expected.reasons,
'decision_event_dispatched': False,
'variables': expected.variables,
'experiment_id': mock_experiment.id,
'variation_id': mock_variation.id
},
)

Expand Down Expand Up @@ -1490,6 +1512,9 @@ def test_should_return_valid_decision_after_setting_and_removing_forced_decision
'User "test_user" is in variation "control" of experiment test_experiment.']
)

expected_experiment = project_config.get_experiment_from_key(expected.rule_key)
expected_var = project_config.get_variation_from_key('test_experiment', expected.variation_key)

# assert notification count
self.assertEqual(1, mock_broadcast_decision.call_count)

Expand All @@ -1507,12 +1532,11 @@ def test_should_return_valid_decision_after_setting_and_removing_forced_decision
'reasons': expected.reasons,
'decision_event_dispatched': True,
'variables': expected.variables,
'experiment_id': expected_experiment.id,
'variation_id': expected_var.id
},
)

expected_experiment = project_config.get_experiment_from_key(expected.rule_key)
expected_var = project_config.get_variation_from_key('test_experiment', expected.variation_key)

mock_send_event.assert_called_with(
project_config,
expected_experiment,
Expand Down