Skip to content

Commit 0837956

Browse files
Enhance CMAB decision handling by returning reasons for decisions and updating type hints
1 parent 116d23a commit 0837956

File tree

5 files changed

+53
-17
lines changed

5 files changed

+53
-17
lines changed

optimizely/cmab/cmab_service.py

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
import hashlib
1616
import threading
1717

18-
from typing import Optional, List, TypedDict
18+
from typing import Optional, List, TypedDict, Tuple
1919
from optimizely.cmab.cmab_client import DefaultCmabClient
2020
from optimizely.odp.lru_cache import LRUCache
2121
from optimizely.optimizely_user_context import OptimizelyUserContext, UserAttributes
@@ -65,26 +65,40 @@ def _get_lock_index(self, user_id: str, rule_id: str) -> int:
6565
return hash_value % NUM_LOCK_STRIPES
6666

6767
def get_decision(self, project_config: ProjectConfig, user_context: OptimizelyUserContext,
68-
rule_id: str, options: List[str]) -> CmabDecision:
68+
rule_id: str, options: List[str]) -> Tuple[CmabDecision, List[str]]:
6969

7070
lock_index = self._get_lock_index(user_context.user_id, rule_id)
7171
with self.locks[lock_index]:
7272
return self._get_decision(project_config, user_context, rule_id, options)
7373

7474
def _get_decision(self, project_config: ProjectConfig, user_context: OptimizelyUserContext,
75-
rule_id: str, options: List[str]) -> CmabDecision:
75+
rule_id: str, options: List[str]) -> Tuple[CmabDecision, List[str]]:
7676

7777
filtered_attributes = self._filter_attributes(project_config, user_context, rule_id)
78+
reasons = []
7879

7980
if OptimizelyDecideOption.IGNORE_CMAB_CACHE in options:
80-
return self._fetch_decision(rule_id, user_context.user_id, filtered_attributes)
81+
reason = f"Ignoring CMAB cache for user '{user_context.user_id}' and rule '{rule_id}'"
82+
if self.logger:
83+
self.logger.debug(reason)
84+
reasons.append(reason)
85+
cmab_decision = self._fetch_decision(rule_id, user_context.user_id, filtered_attributes)
86+
return cmab_decision, reasons
8187

8288
if OptimizelyDecideOption.RESET_CMAB_CACHE in options:
89+
reason = f"Resetting CMAB cache for user '{user_context.user_id}' and rule '{rule_id}'"
90+
if self.logger:
91+
self.logger.debug(reason)
92+
reasons.append(reason)
8393
self.cmab_cache.reset()
8494

8595
cache_key = self._get_cache_key(user_context.user_id, rule_id)
8696

8797
if OptimizelyDecideOption.INVALIDATE_USER_CMAB_CACHE in options:
98+
reason = f"Invalidating CMAB cache for user '{user_context.user_id}' and rule '{rule_id}'"
99+
if self.logger:
100+
self.logger.debug(reason)
101+
reasons.append(reason)
88102
self.cmab_cache.remove(cache_key)
89103

90104
cached_value = self.cmab_cache.lookup(cache_key)
@@ -93,17 +107,35 @@ def _get_decision(self, project_config: ProjectConfig, user_context: OptimizelyU
93107

94108
if cached_value:
95109
if cached_value['attributes_hash'] == attributes_hash:
96-
return CmabDecision(variation_id=cached_value['variation_id'], cmab_uuid=cached_value['cmab_uuid'])
110+
reason = f"CMAB cache hit for user '{user_context.user_id}' and rule '{rule_id}'"
111+
if self.logger:
112+
self.logger.debug(reason)
113+
reasons.append(reason)
114+
return CmabDecision(variation_id=cached_value['variation_id'], cmab_uuid=cached_value['cmab_uuid']), reasons
97115
else:
116+
reason = f"CMAB cache attributes mismatch for user '{user_context.user_id}' and rule '{rule_id}', fetching new decision."
117+
if self.logger:
118+
self.logger.debug(reason)
119+
reasons.append(reason)
98120
self.cmab_cache.remove(cache_key)
121+
else:
122+
reason = f"CMAB cache miss for user '{user_context.user_id}' and rule '{rule_id}'"
123+
if self.logger:
124+
self.logger.debug(reason)
125+
reasons.append(reason)
99126

100127
cmab_decision = self._fetch_decision(rule_id, user_context.user_id, filtered_attributes)
128+
reason = f"CMAB decision is {cmab_decision}"
129+
if self.logger:
130+
self.logger.debug(reason)
131+
reasons.append(reason)
132+
101133
self.cmab_cache.save(cache_key, {
102134
'attributes_hash': attributes_hash,
103135
'variation_id': cmab_decision['variation_id'],
104136
'cmab_uuid': cmab_decision['cmab_uuid'],
105137
})
106-
return cmab_decision
138+
return cmab_decision, reasons
107139

108140
def _fetch_decision(self, rule_id: str, user_id: str, attributes: UserAttributes) -> CmabDecision:
109141
cmab_uuid = str(uuid.uuid4())

optimizely/decision_service.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,10 @@ def _get_decision_for_cmab_experiment(
175175
# User is in CMAB allocation, proceed to CMAB decision
176176
try:
177177
options_list = list(options) if options is not None else []
178-
cmab_decision = self.cmab_service.get_decision(
178+
cmab_decision, cmab_reasons = self.cmab_service.get_decision(
179179
project_config, user_context, experiment.id, options_list
180180
)
181+
decide_reasons.extend(cmab_reasons)
181182
return {
182183
"error": False,
183184
"result": cmab_decision,

optimizely/event/event_processor.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ class Signal:
7272

7373
def __init__(
7474
self,
75-
event_dispatcher: Optional[type[EventDispatcher] | CustomEventDispatcher] = None,
75+
event_dispatcher: Optional[EventDispatcher | CustomEventDispatcher] = None,
7676
logger: Optional[_logging.Logger] = None,
7777
start_on_init: bool = False,
7878
event_queue: Optional[queue.Queue[UserEvent | Signal]] = None,

tests/test_cmab_service.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def test_returns_decision_from_cache_when_valid(self):
6060
"cmab_uuid": "uuid-123"
6161
}
6262

63-
decision = self.cmab_service.get_decision(
63+
decision, _ = self.cmab_service.get_decision(
6464
self.mock_project_config, self.mock_user_context, "exp1", []
6565
)
6666

@@ -72,7 +72,7 @@ def test_ignores_cache_when_option_given(self):
7272
self.mock_cmab_client.fetch_decision.return_value = "varB"
7373
expected_attributes = {"age": 25, "location": "USA"}
7474

75-
decision = self.cmab_service.get_decision(
75+
decision, _ = self.cmab_service.get_decision(
7676
self.mock_project_config,
7777
self.mock_user_context,
7878
"exp1",
@@ -105,7 +105,7 @@ def test_invalidates_user_cache_when_option_given(self):
105105
def test_resets_cache_when_option_given(self):
106106
self.mock_cmab_client.fetch_decision.return_value = "varD"
107107

108-
decision = self.cmab_service.get_decision(
108+
decision, _ = self.cmab_service.get_decision(
109109
self.mock_project_config,
110110
self.mock_user_context,
111111
"exp1",
@@ -128,7 +128,7 @@ def test_new_decision_when_hash_changes(self):
128128
expected_hash = self.cmab_service._hash_attributes(expected_attribute)
129129
expected_key = self.cmab_service._get_cache_key("user123", "exp1")
130130

131-
decision = self.cmab_service.get_decision(self.mock_project_config, self.mock_user_context, "exp1", [])
131+
decision, _ = self.cmab_service.get_decision(self.mock_project_config, self.mock_user_context, "exp1", [])
132132
self.mock_cmab_cache.remove.assert_called_once_with(expected_key)
133133
self.mock_cmab_cache.save.assert_called_once_with(
134134
expected_key,
@@ -171,7 +171,7 @@ def test_only_cmab_attributes_passed_to_client(self):
171171
}
172172
self.mock_cmab_client.fetch_decision.return_value = "varF"
173173

174-
decision = self.cmab_service.get_decision(
174+
decision, _ = self.cmab_service.get_decision(
175175
self.mock_project_config,
176176
self.mock_user_context,
177177
"exp1",

tests/test_decision_service.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -792,10 +792,13 @@ def test_get_variation_cmab_experiment_user_in_traffic_allocation(self):
792792
'logger') as mock_logger:
793793

794794
# Configure CMAB service to return a decision
795-
mock_cmab_service.get_decision.return_value = {
796-
'variation_id': '111151',
797-
'cmab_uuid': 'test-cmab-uuid-123'
798-
}
795+
mock_cmab_service.get_decision.return_value = (
796+
{
797+
'variation_id': '111151',
798+
'cmab_uuid': 'test-cmab-uuid-123'
799+
},
800+
[] # reasons list
801+
)
799802

800803
# Call get_variation with the CMAB experiment
801804
variation_result = self.decision_service.get_variation(

0 commit comments

Comments
 (0)