Skip to content

Commit 106d8a6

Browse files
authored
feat: Record exposures under local evaluation (#251)
Part of Flagsmith/flagsmith-private#325. Java counterpart of Flagsmith/flagsmith-python-client#276, Flagsmith/flagsmith-go-client#246 and Flagsmith/flagsmith-nodejs-client#318. `getExperimentFlag` only recorded exposures under remote evaluation. The environment document carries `metadata.experiment` on feature states, but local evaluation dropped it, along with the variant. ## Changes - Map the multivariate option `key` into the evaluation context, and carry `variant` from the evaluation result onto `Flag`. Locally evaluated identity flags now report their variant. - Map the feature state's `metadata.experiment` into the context's feature metadata, and carry it onto identity flags. - Environment flags carry no experiment: it is removed once, when the environment evaluation context is built, so `getEnvironmentFlags` does no extra work per call. - `getExperimentFlag` records no exposure when the flag has no variant, as the other SDKs do. ## Tests - Local evaluation: an enrolled identity records one exposure with the variant and `experiment_id`; outside the rollout, an identity override, and a feature without an experiment record none. - Environment flags carry no experiment under local evaluation. - Remote: an enrolled flag without a variant records no exposure.
1 parent e278fd5 commit 106d8a6

7 files changed

Lines changed: 179 additions & 21 deletions

File tree

‎src/main/java/com/flagsmith/FlagsmithClient.java‎

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
import com.flagsmith.flagengine.Engine;
1010
import com.flagsmith.flagengine.EvaluationContext;
1111
import com.flagsmith.flagengine.EvaluationResult;
12+
import com.flagsmith.flagengine.FeatureContext;
13+
import com.flagsmith.flagengine.Features;
14+
import com.flagsmith.flagengine.SegmentContext;
1215
import com.flagsmith.flagengine.SegmentRule;
1316
import com.flagsmith.flagengine.Segments;
1417
import com.flagsmith.interfaces.FlagsmithCache;
@@ -79,6 +82,7 @@ public void setEvaluationContext(EvaluationContext evaluationContext) {
7982
this.environmentEvaluationContext = evaluationContext == null
8083
? null
8184
: new EvaluationContext(evaluationContext)
85+
.withFeatures(withoutExperiments(evaluationContext.getFeatures()))
8286
.withSegments(getIdentityFreeSegments(evaluationContext.getSegments()));
8387
this.evaluationContext = evaluationContext;
8488
}
@@ -250,8 +254,7 @@ public BaseFlag getExperimentFlag(String featureName, String identifier)
250254

251255
/**
252256
* Get an identity's flag, recording one {@code $flag_exposure} event if the identity is enrolled
253-
* in a running experiment on it. Only remote evaluation carries experiment metadata, so local
254-
* evaluation and offline mode record no exposure.
257+
* in a running experiment on it.
255258
*
256259
* @param featureName feature name
257260
* @param identifier identifier string
@@ -298,6 +301,12 @@ public BaseFlag getExperimentFlag(
298301
return flag;
299302
}
300303

304+
if (((Flag) flag).getVariant() == null) {
305+
logger.info("Not recording an exposure for feature {}: the flag has no variant.",
306+
featureName);
307+
return flag;
308+
}
309+
301310
Map<String, Object> metadata = new HashMap<>();
302311
metadata.put("experiment_id", experiment.getId());
303312
trackExposureEvent(featureName, identifier, ((Flag) flag).getVariant(), traits, metadata);
@@ -463,12 +472,40 @@ private static Segments getIdentityFreeSegments(Segments segments) {
463472
Segments identityFreeSegments = new Segments();
464473
segments.getAdditionalProperties().forEach((segmentKey, segmentContext) -> {
465474
if (isIdentityFree(segmentContext.getRules())) {
466-
identityFreeSegments.setAdditionalProperty(segmentKey, segmentContext);
475+
identityFreeSegments.setAdditionalProperty(segmentKey, segmentContext.getOverrides() == null
476+
? segmentContext
477+
: new SegmentContext(segmentContext).withOverrides(segmentContext.getOverrides()
478+
.stream()
479+
.map(FlagsmithClient::withoutExperiment)
480+
.collect(Collectors.toList())));
467481
}
468482
});
469483
return identityFreeSegments;
470484
}
471485

486+
/*
487+
* Only an identity is enrolled in an experiment, so environment flags carry none.
488+
*/
489+
private static Features withoutExperiments(Features features) {
490+
if (features == null) {
491+
return null;
492+
}
493+
Features environmentFeatures = new Features();
494+
features.getAdditionalProperties().forEach((featureName, featureContext) ->
495+
environmentFeatures.setAdditionalProperty(featureName, withoutExperiment(featureContext)));
496+
return environmentFeatures;
497+
}
498+
499+
private static FeatureContext withoutExperiment(FeatureContext featureContext) {
500+
Map<String, Object> metadata = featureContext.getMetadata();
501+
if (metadata == null || !metadata.containsKey("experiment")) {
502+
return featureContext;
503+
}
504+
Map<String, Object> environmentMetadata = new HashMap<>(metadata);
505+
environmentMetadata.remove("experiment");
506+
return new FeatureContext(featureContext).withMetadata(environmentMetadata);
507+
}
508+
472509
private static boolean isIdentityFree(List<SegmentRule> rules) {
473510
return rules == null || rules.stream().allMatch(rule ->
474511
(rule.getConditions() == null || rule.getConditions().stream().allMatch(condition ->

‎src/main/java/com/flagsmith/mappers/EngineMappers.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ public static Flag mapFlagResultToFlag(
6363
flag.setValue(flagResult.getValue());
6464
flag.setEnabled(flagResult.getEnabled());
6565
flag.setReason(flagResult.getReason());
66+
flag.setVariant(flagResult.getVariant());
67+
flag.setExperiment(metadata.getExperiment());
6668
return flag;
6769
}
6870

@@ -358,6 +360,7 @@ private static FeatureContext mapFeatureStateToFeatureContext(FeatureStateModel
358360
for (MultivariateFeatureStateValueModel mvValue :
359361
featureState.getMultivariateFeatureStateValues()) {
360362
FeatureValue variant = new FeatureValue()
363+
.withKey(mvValue.getMultivariateFeatureOption().getKey())
361364
.withValue(mvValue.getMultivariateFeatureOption().getValue())
362365
.withWeight(mvValue.getPercentageAllocation().doubleValue())
363366
.withPriority(getMultivariateFeatureValuePriority(mvValue));
@@ -381,6 +384,9 @@ private static Map<String, Object> mapFeatureStateToFeatureMetadata(
381384
FeatureStateModel featureState) {
382385
FeatureMetadata metadata = new FeatureMetadata();
383386
metadata.setFlagsmithId(featureState.getFeature().getId());
387+
if (featureState.getMetadata() != null) {
388+
metadata.setExperiment(featureState.getMetadata().getExperiment());
389+
}
384390
return MapperFactory.getMapper().convertValue(metadata,
385391
new com.fasterxml.jackson.core.type.TypeReference<Map<String, Object>>() {
386392
});

‎src/main/java/com/flagsmith/models/ExperimentMetadata.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import lombok.Data;
55

66
/**
7-
* Details of the experiment running on a feature, as returned by remote evaluation.
7+
* Details of the experiment running on a feature. Only identity evaluation carries it.
88
*/
99
@Data
1010
public class ExperimentMetadata {

‎src/main/java/com/flagsmith/models/FeatureMetadata.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.flagsmith.models;
22

3+
import com.fasterxml.jackson.annotation.JsonInclude;
4+
35
/**
46
* FeatureMetadata
57
*
@@ -8,6 +10,8 @@
810
*/
911
public class FeatureMetadata {
1012
private Integer flagsmithId;
13+
@JsonInclude(JsonInclude.Include.NON_NULL)
14+
private ExperimentMetadata experiment;
1115

1216
/*
1317
* FlagsmithId
@@ -28,4 +32,18 @@ public Integer getFlagsmithId() {
2832
public void setFlagsmithId(Integer flagsmithId) {
2933
this.flagsmithId = flagsmithId;
3034
}
35+
36+
/*
37+
* Experiment
38+
* <p>The running experiment the feature state belongs to, if any.
39+
*
40+
* @return The experiment
41+
*/
42+
public ExperimentMetadata getExperiment() {
43+
return experiment;
44+
}
45+
46+
public void setExperiment(ExperimentMetadata experiment) {
47+
this.experiment = experiment;
48+
}
3149
}

‎src/main/java/com/flagsmith/models/features/MultivariateFeatureOptionModel.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,5 @@
66
@Data
77
public class MultivariateFeatureOptionModel extends BaseModel {
88
private Object value;
9+
private String key;
910
}

‎src/test/java/com/flagsmith/FlagsmithClientTest.java‎

Lines changed: 72 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import org.junit.jupiter.api.Test;
5858
import org.junit.jupiter.params.ParameterizedTest;
5959
import org.junit.jupiter.params.provider.Arguments;
60+
import org.junit.jupiter.params.provider.CsvSource;
6061
import org.junit.jupiter.params.provider.MethodSource;
6162
import org.mockito.ArgumentCaptor;
6263
import org.mockito.Mockito;
@@ -1148,6 +1149,10 @@ public void testGetExperimentFlagRecordsNoExposureWhenNotEnrolledOrDisabled()
11481149
// enrolled, but the flag is off
11491150
assertEquals(Boolean.FALSE,
11501151
client.getExperimentFlag("disabled_feature", "user-1").getEnabled());
1152+
// enrolled, but the flag has no variant
1153+
Flag noVariant = (Flag) client.getExperimentFlag("no_variant", "user-1");
1154+
assertEquals(Boolean.TRUE, noVariant.getExperiment().getInExperiment());
1155+
assertNull(noVariant.getVariant());
11511156

11521157
verify(processor, never()).trackExposureEvent(any(), any(), any(), any(), any());
11531158
}
@@ -1175,39 +1180,90 @@ public void testGetExperimentFlagFallsBackToTheDefaultHandlerWithoutAnExposure(
11751180
verify(processor, never()).trackExposureEvent(any(), any(), any(), any(), any());
11761181
}
11771182

1178-
@Test
1179-
public void testGetExperimentFlagRecordsNoExposureWithLocalEvaluation()
1180-
throws JsonProcessingException, FlagsmithClientError {
1181-
String baseUrl = "http://bad-url";
1183+
/**
1184+
* A locally evaluating client serving
1185+
* {@link FlagsmithTestHelper#getEnvironmentDocumentWithExperiment()}.
1186+
*/
1187+
private static FlagsmithClient localExperimentClient(EventProcessor processor) {
11821188
MockInterceptor interceptor = new MockInterceptor();
1183-
EventProcessor processor = mock(EventProcessor.class);
11841189
interceptor.addRule()
1185-
.get(baseUrl + "/environment-document/")
1190+
.get("http://bad-url/environment-document/")
11861191
.anyTimes()
1187-
.respond(
1188-
MapperFactory.getMapper()
1189-
.writeValueAsString(FlagsmithTestHelper.environmentModel()),
1190-
MEDIATYPE_JSON);
1191-
1192+
.respond(FlagsmithTestHelper.getEnvironmentDocumentWithExperiment(), MEDIATYPE_JSON);
11921193
FlagsmithClient client = FlagsmithClient.newBuilder()
11931194
.withConfiguration(FlagsmithConfig.newBuilder()
1194-
.baseUri(baseUrl)
1195+
.baseUri("http://bad-url")
11951196
.addHttpInterceptor(interceptor)
11961197
.withEventProcessor(processor)
11971198
.withLocalEvaluation(true)
11981199
.build())
11991200
.setApiKey("ser.abcdefg")
12001201
.build();
12011202
client.updateEnvironment();
1203+
return client;
1204+
}
1205+
1206+
@ParameterizedTest
1207+
@CsvSource({
1208+
"user-1, new checkout, treatment",
1209+
"user-3, control, control"
1210+
})
1211+
public void testGetExperimentFlagRecordsAnExposureWithLocalEvaluationWhenEnrolled(
1212+
String identifier, String expectedValue, String expectedVariant)
1213+
throws FlagsmithClientError {
1214+
EventProcessor processor = mock(EventProcessor.class);
1215+
FlagsmithClient client = localExperimentClient(processor);
1216+
Map<String, Object> traits = Collections.singletonMap("plan", "premium");
12021217

1203-
BaseFlag flag = client.getExperimentFlag("some_feature", "user-1");
1218+
Flag flag = (Flag) client.getExperimentFlag("checkout", identifier, traits);
12041219

1205-
assertTrue(flag instanceof Flag);
1206-
assertNull(((Flag) flag).getVariant());
1207-
assertNull(((Flag) flag).getExperiment());
1220+
assertEquals(expectedValue, flag.getValue());
1221+
assertEquals(expectedVariant, flag.getVariant());
1222+
assertEquals(42, flag.getExperiment().getId());
1223+
assertEquals("Checkout experiment", flag.getExperiment().getName());
1224+
assertEquals(Boolean.TRUE, flag.getExperiment().getInExperiment());
1225+
verify(processor).trackExposureEvent("checkout", identifier, expectedVariant, traits,
1226+
Collections.singletonMap("experiment_id", 42));
1227+
}
1228+
1229+
private static Stream<Arguments> localExperimentFlagsWithoutAnExposure() {
1230+
return Stream.of(
1231+
// outside the rollout
1232+
Arguments.of("checkout", "user-1", "free", "control", Boolean.FALSE),
1233+
// an identity override carries no experiment
1234+
Arguments.of("checkout", "user_overridden", "premium", "overridden checkout", null),
1235+
// no experiment on the feature
1236+
Arguments.of("plain", "user-1", "premium", "hello", null));
1237+
}
1238+
1239+
@ParameterizedTest
1240+
@MethodSource("localExperimentFlagsWithoutAnExposure")
1241+
public void testGetExperimentFlagRecordsNoExposureWithLocalEvaluationWhenNotEnrolled(
1242+
String featureName, String identifier, String plan, String expectedValue,
1243+
Boolean expectedInExperiment) throws FlagsmithClientError {
1244+
EventProcessor processor = mock(EventProcessor.class);
1245+
FlagsmithClient client = localExperimentClient(processor);
1246+
1247+
Flag flag = (Flag) client.getExperimentFlag(
1248+
featureName, identifier, Collections.singletonMap("plan", plan));
1249+
1250+
assertEquals(expectedValue, flag.getValue());
1251+
assertEquals(expectedInExperiment,
1252+
flag.getExperiment() == null ? null : flag.getExperiment().getInExperiment());
12081253
verify(processor, never()).trackExposureEvent(any(), any(), any(), any(), any());
12091254
}
12101255

1256+
@Test
1257+
public void testEnvironmentFlagsCarryNoExperimentWithLocalEvaluation()
1258+
throws FlagsmithClientError {
1259+
FlagsmithClient client = localExperimentClient(mock(EventProcessor.class));
1260+
1261+
Flag flag = (Flag) client.getEnvironmentFlags().getFlag("checkout");
1262+
1263+
assertEquals("control", flag.getValue());
1264+
assertNull(flag.getExperiment());
1265+
}
1266+
12111267
/**
12121268
* An events-enabled config serving the experiment identity flags, which records each events
12131269
* batch as "environment key|body".

‎src/test/java/com/flagsmith/FlagsmithTestHelper.java‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,10 +447,50 @@ public static String getIdentitiesFlagsWithExperiment() {
447447
+ " {\"id\": 4, \"feature\": {\"id\": 4, \"name\": \"disabled_feature\", \"type\": \"MULTIVARIATE\"},\n"
448448
+ " \"feature_state_value\": \"off\", \"enabled\": false, \"variant\": \"treatment\",\n"
449449
+ " \"reason\": \"SPLIT; weight=50.0\", \"metadata\": {\"experiment\":\n"
450-
+ " {\"id\": 44, \"name\": \"disabled_experiment\", \"in_experiment\": true}}}\n"
450+
+ " {\"id\": 44, \"name\": \"disabled_experiment\", \"in_experiment\": true}}},\n"
451+
+ " {\"id\": 5, \"feature\": {\"id\": 5, \"name\": \"no_variant\", \"type\": \"STANDARD\"},\n"
452+
+ " \"feature_state_value\": \"on\", \"enabled\": true, \"metadata\": {\"experiment\":\n"
453+
+ " {\"id\": 45, \"name\": \"no_variant_experiment\", \"in_experiment\": true}}}\n"
451454
+ "]}";
452455
}
453456

457+
/**
458+
* An environment document running an experiment on {@code checkout}. Its rollout segment is
459+
* identities on the premium plan, split evenly between {@code control} and {@code treatment}.
460+
* {@code user_overridden} has an identity override for {@code checkout}, and {@code plain} has no
461+
* experiment.
462+
*/
463+
public static String getEnvironmentDocumentWithExperiment() {
464+
String experiment = "{\"id\": 42, \"name\": \"Checkout experiment\", \"in_experiment\": %s}";
465+
String checkout = "{\"name\": \"checkout\", \"type\": \"MULTIVARIATE\", \"id\": 10}";
466+
return "{\"api_key\": \"B62qaMZNwfiqT76p38ggrQ\", \"name\": \"Test Environment\", \"id\": 1,\n"
467+
+ " \"project\": {\"name\": \"Test project\", \"id\": 1, \"hide_disabled_flags\": false,\n"
468+
+ " \"organisation\": {\"id\": 1, \"name\": \"Test Org\", \"feature_analytics\": false,\n"
469+
+ " \"persist_trait_data\": true, \"stop_serving_flags\": false},\n"
470+
+ " \"segments\": [{\"id\": 7, \"name\": \"Checkout experiment rollout\",\n"
471+
+ " \"rules\": [{\"type\": \"ALL\", \"rules\": [], \"conditions\": [\n"
472+
+ " {\"operator\": \"EQUAL\", \"property_\": \"plan\", \"value\": \"premium\"}]}],\n"
473+
+ " \"feature_states\": [{\"id\": 2, \"featurestate_uuid\": \"c2c2c2c2-c2c2-c2c2-c2c2-c2c2c2c2c2c2\",\n"
474+
+ " \"feature\": " + checkout + ", \"feature_state_value\": \"control\", \"enabled\": true,\n"
475+
+ " \"feature_segment\": {\"priority\": 0},\n"
476+
+ " \"multivariate_feature_state_values\": [{\"id\": 3, \"percentage_allocation\": 50,\n"
477+
+ " \"mv_fs_value_uuid\": \"3e3e3e3e-3e3e-3e3e-3e3e-3e3e3e3e3e3e\",\n"
478+
+ " \"multivariate_feature_option\": {\"id\": 3, \"value\": \"new checkout\", \"key\": \"treatment\"}}],\n"
479+
+ " \"metadata\": {\"experiment\": " + String.format(experiment, "true") + "}}]}]},\n"
480+
+ " \"feature_states\": [\n"
481+
+ " {\"id\": 1, \"featurestate_uuid\": \"c1c1c1c1-c1c1-c1c1-c1c1-c1c1c1c1c1c1\",\n"
482+
+ " \"feature\": " + checkout + ", \"feature_state_value\": \"control\", \"enabled\": true,\n"
483+
+ " \"multivariate_feature_state_values\": [],\n"
484+
+ " \"metadata\": {\"experiment\": " + String.format(experiment, "false") + "}},\n"
485+
+ " {\"id\": 3, \"featurestate_uuid\": \"c3c3c3c3-c3c3-c3c3-c3c3-c3c3c3c3c3c3\",\n"
486+
+ " \"feature\": {\"name\": \"plain\", \"type\": \"STANDARD\", \"id\": 11},\n"
487+
+ " \"feature_state_value\": \"hello\", \"enabled\": true, \"multivariate_feature_state_values\": []}],\n"
488+
+ " \"identity_overrides\": [{\"identifier\": \"user_overridden\", \"identity_features\": [\n"
489+
+ " {\"featurestate_uuid\": \"c4c4c4c4-c4c4-c4c4-c4c4-c4c4c4c4c4c4\", \"feature\": " + checkout + ",\n"
490+
+ " \"feature_state_value\": \"overridden checkout\", \"enabled\": true,\n"
491+
+ " \"multivariate_feature_state_values\": []}]}]}";
492+
}
493+
454494
public static <T> Future<T> futurableReturn(T response) {
455495
CompletableFuture<T> promise = new CompletableFuture<>();
456496
promise.complete(response);

0 commit comments

Comments
 (0)