Skip to content

Commit c2a8349

Browse files
aliabbasrizvialiabbasrizvi
authored
Preventing events from being sent when user is part of a rollout (#85)
1 parent db4a9a6 commit c2a8349

File tree

4 files changed

+89
-30
lines changed

4 files changed

+89
-30
lines changed

‎optimizely/decision_service.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
from .user_profile import UserProfile
2323

2424

25-
Decision = namedtuple('Decision', 'experiment variation')
25+
Decision = namedtuple('Decision', 'experiment variation source')
26+
DECISION_SOURCE_EXPERIMENT = 'experiment'
27+
DECISION_SOURCE_ROLLOUT = 'rollout'
2628

2729

2830
class DecisionService(object):
@@ -190,7 +192,7 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None):
190192
if variation:
191193
self.logger.log(enums.LogLevels.DEBUG,
192194
'User "%s" is in variation %s of experiment %s.' % (user_id, variation.key, experiment.key))
193-
return Decision(experiment, variation)
195+
return Decision(experiment, variation, DECISION_SOURCE_ROLLOUT)
194196
else:
195197
# Evaluate no further rules
196198
self.logger.log(enums.LogLevels.DEBUG,
@@ -207,9 +209,9 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None):
207209
if variation:
208210
self.logger.log(enums.LogLevels.DEBUG,
209211
'User "%s" meets conditions for targeting rule "Everyone Else".' % user_id)
210-
return Decision(everyone_else_experiment, variation)
212+
return Decision(everyone_else_experiment, variation, DECISION_SOURCE_ROLLOUT)
211213

212-
return Decision(None, None)
214+
return Decision(None, None, DECISION_SOURCE_ROLLOUT)
213215

214216
def get_variation_for_feature(self, feature, user_id, attributes=None):
215217
""" Returns the experiment/variation the user is bucketed in for the given feature.
@@ -256,7 +258,7 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
256258
rollout = self.config.get_rollout_from_id(feature.rolloutId)
257259
return self.get_variation_for_rollout(rollout, user_id, attributes)
258260

259-
return Decision(experiment, variation)
261+
return Decision(experiment, variation, DECISION_SOURCE_EXPERIMENT)
260262

261263
def get_experiment_in_group(self, group, user_id):
262264
""" Determine which experiment in the group the user is bucketed into.

‎optimizely/optimizely.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -377,10 +377,12 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None):
377377
decision = self.decision_service.get_variation_for_feature(feature, user_id, attributes)
378378
if decision.variation:
379379
self.logger.log(enums.LogLevels.INFO, 'Feature "%s" is enabled for user "%s".' % (feature_key, user_id))
380-
self._send_impression_event(decision.experiment,
381-
decision.variation,
382-
user_id,
383-
attributes)
380+
# Send event if Decision came from an experiment.
381+
if decision.source == decision_service.DECISION_SOURCE_EXPERIMENT:
382+
self._send_impression_event(decision.experiment,
383+
decision.variation,
384+
user_id,
385+
attributes)
384386
return True
385387

386388
self.logger.log(enums.LogLevels.INFO, 'Feature "%s" is not enabled for user "%s".' % (feature_key, user_id))

‎tests/test_decision_service.py

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ def test_get_variation_for_rollout__returns_none_if_no_experiments(self, mock_lo
344344
""" Test that get_variation_for_rollout returns None if there are no experiments (targeting rules). """
345345

346346
no_experiment_rollout = self.project_config.get_rollout_from_id('201111')
347-
self.assertEqual(decision_service.Decision(None, None),
347+
self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_ROLLOUT),
348348
self.decision_service.get_variation_for_rollout(no_experiment_rollout, 'test_user'))
349349

350350
# Assert no log messages were generated
@@ -358,7 +358,7 @@ def test_get_variation_for_rollout__skips_to_everyone_else_rule(self, mock_loggi
358358

359359
with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
360360
mock.patch('optimizely.bucketer.Bucketer.bucket', return_value=None):
361-
self.assertEqual(decision_service.Decision(None, None),
361+
self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_ROLLOUT),
362362
self.decision_service.get_variation_for_rollout(rollout, 'test_user'))
363363

364364
# Check that after first experiment, it skips to the last experiment to check
@@ -381,7 +381,7 @@ def test_get_variation_for_rollout__returns_none_for_user_not_in_rollout(self, m
381381
rollout = self.project_config.get_rollout_from_id('211111')
382382

383383
with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=False) as mock_audience_check:
384-
self.assertEqual(decision_service.Decision(None, None),
384+
self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_ROLLOUT),
385385
self.decision_service.get_variation_for_rollout(rollout, 'test_user'))
386386

387387
# Check that all experiments in rollout layer were checked
@@ -407,7 +407,9 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_experiment(
407407
expected_variation = self.project_config.get_variation_from_id('test_experiment', '111129')
408408
with mock.patch('optimizely.decision_service.DecisionService.get_variation',
409409
return_value=expected_variation) as mock_decision:
410-
self.assertEqual(decision_service.Decision(expected_experiment, expected_variation),
410+
self.assertEqual(decision_service.Decision(expected_experiment,
411+
expected_variation,
412+
decision_service.DECISION_SOURCE_EXPERIMENT),
411413
self.decision_service.get_variation_for_feature(feature, 'user1'))
412414

413415
mock_decision.assert_called_once_with(
@@ -447,7 +449,9 @@ def test_get_variation_for_feature__returns_variation_if_user_not_in_experiment_
447449
'optimizely.helpers.audience.is_user_in_experiment',
448450
side_effect=[False, True]) as mock_audience_check, \
449451
mock.patch('optimizely.bucketer.Bucketer.bucket', return_value=expected_variation):
450-
self.assertEqual(decision_service.Decision(expected_experiment, expected_variation),
452+
self.assertEqual(decision_service.Decision(expected_experiment,
453+
expected_variation,
454+
decision_service.DECISION_SOURCE_ROLLOUT),
451455
self.decision_service.get_variation_for_feature(feature, 'user1'))
452456

453457
self.assertEqual(2, mock_audience_check.call_count)
@@ -469,7 +473,9 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_group(self,
469473
return_value=self.project_config.get_experiment_from_key('group_exp_1')) as mock_get_experiment_in_group, \
470474
mock.patch('optimizely.decision_service.DecisionService.get_variation',
471475
return_value=expected_variation) as mock_decision:
472-
self.assertEqual(decision_service.Decision(expected_experiment, expected_variation),
476+
self.assertEqual(decision_service.Decision(expected_experiment,
477+
expected_variation,
478+
decision_service.DECISION_SOURCE_EXPERIMENT),
473479
self.decision_service.get_variation_for_feature(feature, 'user1'))
474480

475481
mock_get_experiment_in_group.assert_called_once_with(self.project_config.get_group('19228'), 'user1')
@@ -484,7 +490,7 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_group(self, _):
484490
with mock.patch('optimizely.decision_service.DecisionService.get_experiment_in_group',
485491
return_value=None) as mock_get_experiment_in_group, \
486492
mock.patch('optimizely.decision_service.DecisionService.get_variation') as mock_decision:
487-
self.assertEqual(decision_service.Decision(None, None),
493+
self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_EXPERIMENT),
488494
self.decision_service.get_variation_for_feature(feature, 'user1'))
489495

490496
mock_get_experiment_in_group.assert_called_once_with(self.project_config.get_group('19228'), 'user1')
@@ -497,7 +503,9 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self
497503
expected_experiment = self.project_config.get_experiment_from_key('test_experiment')
498504

499505
with mock.patch('optimizely.decision_service.DecisionService.get_variation', return_value=None) as mock_decision:
500-
self.assertEqual(decision_service.Decision(expected_experiment, None),
506+
self.assertEqual(decision_service.Decision(expected_experiment,
507+
None,
508+
decision_service.DECISION_SOURCE_EXPERIMENT),
501509
self.decision_service.get_variation_for_feature(feature, 'user1'))
502510

503511
mock_decision.assert_called_once_with(
@@ -513,7 +521,9 @@ def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_no
513521

514522
with mock.patch('optimizely.decision_service.DecisionService.get_experiment_in_group',
515523
return_value=self.project_config.get_experiment_from_key('group_exp_2')) as mock_decision:
516-
self.assertEqual(decision_service.Decision(expected_experiment, None),
524+
self.assertEqual(decision_service.Decision(expected_experiment,
525+
None,
526+
decision_service.DECISION_SOURCE_EXPERIMENT),
517527
self.decision_service.get_variation_for_feature(feature, 'user_1'))
518528

519529
mock_decision.assert_called_once_with(self.project_config.get_group('19228'), 'user_1')

0 commit comments

Comments
 (0)