Skip to content

Commit 37f2cb4

Browse files
committed
[AI-FSSDK] [FSSDK-12262] Exclude CMAB from UserProfileService
1 parent 88b0644 commit 37f2cb4

File tree

3 files changed

+173
-2
lines changed

3 files changed

+173
-2
lines changed

optimizely/decision_service.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,13 @@ def get_variation(
457457
}
458458

459459
# Check to see if user has a decision available for the given experiment
460-
if user_profile_tracker is not None and not ignore_user_profile:
460+
# CMAB experiments are excluded from UPS because CMAB decisions are dynamic
461+
# and should not use sticky bucketing.
462+
if experiment.cmab:
463+
message = Errors.CMAB_UPS_EXCLUDED.format(experiment.key)
464+
self.logger.info(message)
465+
decide_reasons.append(message)
466+
elif user_profile_tracker is not None and not ignore_user_profile:
461467
variation = self.get_stored_variation(project_config, experiment, user_profile_tracker.get_user_profile())
462468
if variation:
463469
message = f'Returning previously activated variation ID "{variation}" of experiment ' \
@@ -529,7 +535,8 @@ def get_variation(
529535
self.logger.info(message)
530536
decide_reasons.append(message)
531537
# Store this new decision and return the variation for the user
532-
if user_profile_tracker is not None and not ignore_user_profile:
538+
# CMAB experiments are excluded from UPS - do not save their decisions
539+
if user_profile_tracker is not None and not ignore_user_profile and not experiment.cmab:
533540
try:
534541
user_profile_tracker.update_user_profile(experiment, variation)
535542
except:

optimizely/helpers/enums.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ class Errors:
131131
CMAB_FETCH_FAILED: Final = 'CMAB decision fetch failed with status: {}.'
132132
INVALID_CMAB_FETCH_RESPONSE: Final = 'Invalid CMAB fetch response.'
133133
CMAB_FETCH_FAILED_DETAILED: Final = 'Failed to fetch CMAB data for experiment {}.'
134+
CMAB_UPS_EXCLUDED: Final = (
135+
'Skipping user profile service for CMAB experiment "{}". '
136+
'CMAB decisions should not use UPS for sticky bucketing.'
137+
)
134138

135139

136140
class ForcedDecisionLogs:

tests/test_decision_service.py

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1890,3 +1890,163 @@ def test_get_variation_for_feature_returns_rollout_in_experiment_bucket_range_25
18901890
mock_config_logging.debug.assert_called_with(
18911891
'Assigned bucket 4000 to user with bucketing ID "test_user".')
18921892
mock_generate_bucket_value.assert_called_with("test_user211147")
1893+
1894+
def test_get_variation__cmab_experiment_skips_ups_lookup(self):
1895+
"""Test that CMAB experiments skip user profile service lookup."""
1896+
user = optimizely_user_context.OptimizelyUserContext(
1897+
optimizely_client=None,
1898+
logger=None,
1899+
user_id="test_user",
1900+
user_attributes={}
1901+
)
1902+
1903+
cmab_experiment = entities.Experiment(
1904+
'111150',
1905+
'cmab_experiment',
1906+
'Running',
1907+
'111150',
1908+
[],
1909+
{},
1910+
[
1911+
entities.Variation('111151', 'variation_1'),
1912+
entities.Variation('111152', 'variation_2')
1913+
],
1914+
[
1915+
{'entityId': '111151', 'endOfRange': 5000},
1916+
{'entityId': '111152', 'endOfRange': 10000}
1917+
],
1918+
cmab={'trafficAllocation': 5000}
1919+
)
1920+
1921+
user_profile_service = user_profile.UserProfileService()
1922+
user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service)
1923+
1924+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1925+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions',
1926+
return_value=[True, []]), \
1927+
mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id',
1928+
return_value=['$', []]), \
1929+
mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \
1930+
mock.patch.object(self.project_config, 'get_variation_from_id',
1931+
return_value=entities.Variation('111151', 'variation_1')), \
1932+
mock.patch.object(self.decision_service, 'get_stored_variation') as mock_get_stored, \
1933+
mock.patch.object(self.decision_service, 'logger') as mock_logger:
1934+
1935+
mock_cmab_service.get_decision.return_value = (
1936+
{'variation_id': '111151', 'cmab_uuid': 'test-cmab-uuid'},
1937+
[]
1938+
)
1939+
1940+
variation_result = self.decision_service.get_variation(
1941+
self.project_config,
1942+
cmab_experiment,
1943+
user,
1944+
user_profile_tracker
1945+
)
1946+
1947+
# UPS lookup should NOT be called for CMAB experiments
1948+
mock_get_stored.assert_not_called()
1949+
1950+
# Verify the decision reason is present
1951+
reasons = variation_result['reasons']
1952+
self.assertIn(
1953+
'Skipping user profile service for CMAB experiment "cmab_experiment". '
1954+
'CMAB decisions should not use UPS for sticky bucketing.',
1955+
reasons
1956+
)
1957+
1958+
# Verify variation is still returned correctly
1959+
self.assertEqual(entities.Variation('111151', 'variation_1'), variation_result['variation'])
1960+
self.assertEqual('test-cmab-uuid', variation_result['cmab_uuid'])
1961+
1962+
def test_get_variation__cmab_experiment_skips_ups_save(self):
1963+
"""Test that CMAB experiments do not save decisions to user profile service."""
1964+
user = optimizely_user_context.OptimizelyUserContext(
1965+
optimizely_client=None,
1966+
logger=None,
1967+
user_id="test_user",
1968+
user_attributes={}
1969+
)
1970+
1971+
cmab_experiment = entities.Experiment(
1972+
'111150',
1973+
'cmab_experiment',
1974+
'Running',
1975+
'111150',
1976+
[],
1977+
{},
1978+
[
1979+
entities.Variation('111151', 'variation_1'),
1980+
entities.Variation('111152', 'variation_2')
1981+
],
1982+
[
1983+
{'entityId': '111151', 'endOfRange': 5000},
1984+
{'entityId': '111152', 'endOfRange': 10000}
1985+
],
1986+
cmab={'trafficAllocation': 5000}
1987+
)
1988+
1989+
user_profile_service = user_profile.UserProfileService()
1990+
user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service)
1991+
1992+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1993+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions',
1994+
return_value=[True, []]), \
1995+
mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id',
1996+
return_value=['$', []]), \
1997+
mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \
1998+
mock.patch.object(self.project_config, 'get_variation_from_id',
1999+
return_value=entities.Variation('111151', 'variation_1')), \
2000+
mock.patch.object(self.decision_service, 'logger'):
2001+
2002+
mock_cmab_service.get_decision.return_value = (
2003+
{'variation_id': '111151', 'cmab_uuid': 'test-cmab-uuid'},
2004+
[]
2005+
)
2006+
2007+
variation_result = self.decision_service.get_variation(
2008+
self.project_config,
2009+
cmab_experiment,
2010+
user,
2011+
user_profile_tracker
2012+
)
2013+
2014+
# User profile tracker should NOT have been updated
2015+
self.assertFalse(user_profile_tracker.profile_updated)
2016+
2017+
# Verify variation is returned correctly
2018+
self.assertEqual(entities.Variation('111151', 'variation_1'), variation_result['variation'])
2019+
2020+
def test_get_variation__non_cmab_experiment_still_uses_ups(self):
2021+
"""Test that non-CMAB experiments still use user profile service normally."""
2022+
user = optimizely_user_context.OptimizelyUserContext(
2023+
optimizely_client=None,
2024+
logger=None,
2025+
user_id="test_user",
2026+
user_attributes={}
2027+
)
2028+
2029+
experiment = self.project_config.get_experiment_from_key('test_experiment')
2030+
2031+
user_profile_service = user_profile.UserProfileService()
2032+
user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service)
2033+
2034+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
2035+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions',
2036+
return_value=[True, []]), \
2037+
mock.patch.object(self.decision_service, 'get_stored_variation',
2038+
return_value=entities.Variation('111128', 'control')) as mock_get_stored, \
2039+
mock.patch.object(self.decision_service, 'logger'):
2040+
2041+
variation_result = self.decision_service.get_variation(
2042+
self.project_config,
2043+
experiment,
2044+
user,
2045+
user_profile_tracker
2046+
)
2047+
2048+
# UPS lookup SHOULD be called for non-CMAB experiments
2049+
mock_get_stored.assert_called_once()
2050+
2051+
# Verify the stored variation is returned
2052+
self.assertEqual(entities.Variation('111128', 'control'), variation_result['variation'])

0 commit comments

Comments
 (0)