Skip to content

Commit 28d6d5e

Browse files
[FSSDK-12039] update: Fix reasons and logging in CMAB decision process (#590)
1 parent 54bafad commit 28d6d5e

File tree

5 files changed

+22
-7
lines changed

5 files changed

+22
-7
lines changed

core-api/src/main/java/com/optimizely/ab/Optimizely.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1494,14 +1494,15 @@ private Map<String, OptimizelyDecision> decideForKeysInternal(@Nonnull Optimizel
14941494
for (int i = 0; i < flagsWithoutForcedDecision.size(); i++) {
14951495
DecisionResponse<FeatureDecision> decision = decisionList.get(i);
14961496
boolean error = decision.isError();
1497+
List<String> reasons = decision.getReasons().toReport();
14971498
String experimentKey = null;
14981499
if (decision.getResult() != null && decision.getResult().experiment != null) {
14991500
experimentKey = decision.getResult().experiment.getKey();
15001501
}
15011502
String flagKey = flagsWithoutForcedDecision.get(i).getKey();
15021503

15031504
if (error) {
1504-
OptimizelyDecision optimizelyDecision = OptimizelyDecision.newErrorDecision(flagKey, user, DecisionMessage.CMAB_ERROR.reason(experimentKey));
1505+
OptimizelyDecision optimizelyDecision = OptimizelyDecision.newErrorDecision(flagKey, user, reasons);
15051506
decisionMap.put(flagKey, optimizelyDecision);
15061507
if (validKeys.contains(flagKey)) {
15071508
validKeys.remove(flagKey);

core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -957,11 +957,12 @@ private DecisionResponse<CmabDecision> getDecisionForCmabExperiment(@Nonnull Pro
957957
// User is in CMAB allocation, proceed to CMAB decision
958958
try {
959959
CmabDecision cmabDecision = cmabService.getDecision(projectConfig, userContext, experiment.getId(), options);
960-
960+
String message = String.format("Successfully fetched CMAB decision %s for experiment %s.", cmabDecision.toString(), experiment.getKey());
961+
reasons.addInfo(message);
961962
return new DecisionResponse<>(cmabDecision, reasons);
962963
} catch (Exception e) {
963-
String errorMessage = String.format("CMAB fetch failed for experiment \"%s\"", experiment.getKey());
964-
reasons.addInfo(errorMessage);
964+
String errorMessage = String.format("Failed to fetch CMAB data for experiment %s.", experiment.getKey());
965+
reasons.addError(errorMessage);
965966
logger.error("{} {}", errorMessage, e.getMessage());
966967

967968
return new DecisionResponse<>(null, reasons, true, null);

core-api/src/main/java/com/optimizely/ab/optimizelydecision/OptimizelyDecision.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,20 @@ public static OptimizelyDecision newErrorDecision(@Nonnull String key,
130130
user,
131131
Arrays.asList(error));
132132
}
133-
133+
134+
public static OptimizelyDecision newErrorDecision(@Nonnull String key,
135+
@Nonnull OptimizelyUserContext user,
136+
@Nonnull List<String> reasons) {
137+
return new OptimizelyDecision(
138+
null,
139+
false,
140+
new OptimizelyJSON(Collections.emptyMap()),
141+
null,
142+
key,
143+
user,
144+
reasons);
145+
}
146+
134147
@Override
135148
public boolean equals(Object obj) {
136149
if (obj == null || getClass() != obj.getClass()) return false;

core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5090,7 +5090,6 @@ public void identifyUser() {
50905090
@Test
50915091
public void testDecideReturnsErrorDecisionWhenDecisionServiceFails() throws Exception {
50925092
assumeTrue(datafileVersion >= Integer.parseInt(ProjectConfig.Version.V4.toString()));
5093-
50945093
// Use the CMAB datafile
50955094
Optimizely optimizely = Optimizely.builder()
50965095
.withDatafile(validConfigJsonCMAB())
@@ -5099,6 +5098,7 @@ public void testDecideReturnsErrorDecisionWhenDecisionServiceFails() throws Exce
50995098

51005099
// Mock decision service to return an error from CMAB
51015100
DecisionReasons reasons = new DefaultDecisionReasons();
5101+
reasons.addError("Failed to fetch CMAB data for experiment exp-cmab.");
51025102
FeatureDecision errorFeatureDecision = new FeatureDecision(new Experiment("123", "exp-cmab", "123"), null, FeatureDecision.DecisionSource.ROLLOUT);
51035103
DecisionResponse<FeatureDecision> errorDecisionResponse = new DecisionResponse<>(
51045104
errorFeatureDecision,
@@ -5129,7 +5129,6 @@ public void testDecideReturnsErrorDecisionWhenDecisionServiceFails() throws Exce
51295129
OptimizelyUserContext userContext = optimizely.createUserContext("test_user");
51305130
OptimizelyDecision decision = userContext.decide("feature_1"); // This is the feature flag key from cmab-config.json
51315131

5132-
System.out.println("reasons: " + decision.getReasons());
51335132
// Verify the decision contains the error information
51345133
assertFalse(decision.getEnabled());
51355134
assertNull(decision.getVariationKey());

core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ private String doFetch(String url, String requestBody) {
101101
request.setHeader("content-type", "application/json");
102102
CloseableHttpResponse response = null;
103103
try {
104+
logger.info("Fetching CMAB decision: {} with body: {}", url, requestBody);
104105
response = httpClient.execute(request);
105106

106107
if (!CmabClientHelper.isSuccessStatusCode(response.getStatusLine().getStatusCode())) {

0 commit comments

Comments
 (0)