From 5546cc88fa4ec126eaf91e161a94b5e769dc2216 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Wed, 7 Oct 2026 11:05:10 -0400 Subject: [PATCH 01/13] Parse features on UFC splits A UFC split can carry a list of key-value features for exposure hooks. Split gains a nullable features field, and the parser reads it leniently: a malformed list or entry is dropped instead of rejecting the flag, so features never change which variation a subject gets. --- .../trace/api/featureflag/ufc/v1/Feature.java | 13 +++ .../trace/api/featureflag/ufc/v1/Split.java | 13 +++ .../UniversalFlagConfigParser.java | 93 +++++++++++++++++++ .../RemoteConfigServiceImplTest.java | 75 +++++++++++++++ .../remote-config/split-features.json | 35 +++++++ 5 files changed, 229 insertions(+) create mode 100644 products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java create mode 100644 products/feature-flagging/feature-flagging-lib/src/test/resources/remote-config/split-features.json diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java new file mode 100644 index 00000000000..a5430e97c37 --- /dev/null +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java @@ -0,0 +1,13 @@ +package datadog.trace.api.featureflag.ufc.v1; + +/** Key-value data that the UFC attaches to a split for exposure hooks. */ +public class Feature { + public final String key; + // A String, Double or Boolean; the parser drops features with any other value. + public final Object value; + + public Feature(final String key, final Object value) { + this.key = key; + this.value = value; + } +} diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Split.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Split.java index 37f6909eeea..5e98f90dabc 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Split.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Split.java @@ -11,15 +11,28 @@ public class Split { // Moshi reflective deserialization from the UFC "serialId" JSON field. Surfaced as // __dd_split_serial_id in eval metadata for APM span enrichment. public final Integer serialId; + // Null when the UFC split has no features. A provider must check that this field exists before + // reading it, because an older agent ships a Split without it. + public final List features; public Split( final List shards, final String variationKey, final Map extraLogging, final Integer serialId) { + this(shards, variationKey, extraLogging, serialId, null); + } + + public Split( + final List shards, + final String variationKey, + final Map extraLogging, + final Integer serialId, + final List features) { this.shards = shards; this.variationKey = variationKey; this.extraLogging = extraLogging; this.serialId = serialId; + this.features = features; } } diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java index 9778d980179..7ec108b4bd7 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java @@ -9,6 +9,7 @@ import datadog.remoteconfig.ConfigurationDeserializer; import datadog.trace.api.featureflag.ufc.v1.Allocation; import datadog.trace.api.featureflag.ufc.v1.ConditionConfiguration; +import datadog.trace.api.featureflag.ufc.v1.Feature; import datadog.trace.api.featureflag.ufc.v1.Flag; import datadog.trace.api.featureflag.ufc.v1.ParsedSemver; import datadog.trace.api.featureflag.ufc.v1.Rule; @@ -23,6 +24,7 @@ import java.time.Instant; import java.time.format.DateTimeFormatter; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -59,6 +61,7 @@ final class UniversalFlagConfigParser implements ConfigurationDeserializer V1_ADAPTER = MOSHI.adapter(ServerConfiguration.class); @@ -438,6 +441,96 @@ public void toJson(@Nonnull final JsonWriter writer, @Nullable final Boolean val } } + /** + * Reads a split's features. Features only feed exposure hooks, so a malformed list or entry is + * dropped instead of rejecting the flag: it must never change which variation a subject gets. + */ + static final class FeatureListAdapter extends JsonAdapter> { + + private static final Type FEATURES_TYPE = Types.newParameterizedType(List.class, Feature.class); + + static final Factory FACTORY = + new Factory() { + @Nullable + @Override + public JsonAdapter create( + @Nonnull final Type type, + @Nonnull final Set annotations, + @Nonnull final Moshi moshi) { + if (!annotations.isEmpty() || !Types.equals(type, FEATURES_TYPE)) { + return null; + } + return new FeatureListAdapter(); + } + }; + + @Nullable + @Override + public List fromJson(@Nonnull final JsonReader reader) throws IOException { + if (reader.peek() != JsonReader.Token.BEGIN_ARRAY) { + reader.skipValue(); + return null; + } + final List features = new ArrayList<>(); + reader.beginArray(); + while (reader.hasNext()) { + final Feature feature = readFeature(reader); + if (feature != null) { + features.add(feature); + } + } + reader.endArray(); + return Collections.unmodifiableList(features); + } + + @Nullable + private static Feature readFeature(final JsonReader reader) throws IOException { + if (reader.peek() != JsonReader.Token.BEGIN_OBJECT) { + reader.skipValue(); + return null; + } + String key = null; + Object value = null; + reader.beginObject(); + while (reader.hasNext()) { + final String name = reader.nextName(); + if ("key".equals(name) && reader.peek() == JsonReader.Token.STRING) { + key = reader.nextString(); + } else if ("value".equals(name)) { + value = readScalar(reader); + } else { + reader.skipValue(); + } + } + reader.endObject(); + if (key == null || key.isEmpty() || value == null) { + return null; + } + return new Feature(key, value); + } + + @Nullable + private static Object readScalar(final JsonReader reader) throws IOException { + switch (reader.peek()) { + case STRING: + return reader.nextString(); + case NUMBER: + return reader.nextDouble(); + case BOOLEAN: + return reader.nextBoolean(); + default: + reader.skipValue(); + return null; + } + } + + @Override + public void toJson(@Nonnull final JsonWriter writer, @Nullable final List value) + throws IOException { + throw new UnsupportedOperationException("Reading only adapter"); + } + } + static final class InstantAdapter extends JsonAdapter { @Nullable diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java index f37599feeb0..4bb1ed817bf 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java @@ -30,6 +30,7 @@ import datadog.trace.api.Config; import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.featureflag.ufc.v1.Allocation; +import datadog.trace.api.featureflag.ufc.v1.Feature; import datadog.trace.api.featureflag.ufc.v1.Flag; import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; import java.io.IOException; @@ -38,6 +39,7 @@ import java.lang.reflect.Type; import java.time.Instant; import java.util.Date; +import java.util.List; import java.util.Map; import okio.Buffer; import org.junit.jupiter.api.AfterEach; @@ -154,6 +156,79 @@ void coercesSerialIdWithoutRejectingTheFlag(final String wireValue, final int ex assertEquals(Integer.valueOf(expected), serialIdOf(config)); } + @Test + void parsesSplitFeatures() throws Exception { + final ServerConfiguration config = + deserialize( + configWithFeatures( + "[{\"key\": \"holdout.key\", \"value\": \"q4-global\"}," + + " {\"key\": \"holdout.weight\", \"value\": 0.5}," + + " {\"key\": \"holdout.should_include_in_holdout_analysis\", \"value\": true}]")); + + final List features = featuresOf(config); + assertEquals(3, features.size()); + assertFeature(features.get(0), "holdout.key", "q4-global"); + assertFeature(features.get(1), "holdout.weight", 0.5); + assertFeature(features.get(2), "holdout.should_include_in_holdout_analysis", true); + assertEquals(Integer.valueOf(7), serialIdOf(config)); + } + + @Test + void parsesAbsentSplitFeaturesAsNull() throws Exception { + final ServerConfiguration config = deserialize(configWithFeatures(null)); + + assertNull(featuresOf(config)); + assertEquals(Integer.valueOf(7), serialIdOf(config)); + } + + @Test + void dropsMalformedSplitFeaturesAndKeepsTheFlag() throws Exception { + final ServerConfiguration config = + deserialize( + configWithFeatures( + "[{\"key\": \"object-value\", \"value\": {\"nested\": true}}," + + " {\"key\": \"array-value\", \"value\": [1]}," + + " {\"key\": \"null-value\", \"value\": null}," + + " {\"key\": \"\", \"value\": \"empty key\"}," + + " {\"key\": 3, \"value\": \"numeric key\"}," + + " {\"value\": \"missing key\"}," + + " {\"key\": \"missing value\"}," + + " \"not-an-object\"," + + " {\"key\": \"holdout.key\", \"value\": \"q4-global\", \"destination\": \"HOOK\"}]")); + + assertTrue(config.flags.containsKey("valid-flag")); + final List features = featuresOf(config); + assertEquals(1, features.size()); + assertFeature(features.get(0), "holdout.key", "q4-global"); + } + + @Test + void ignoresSplitFeaturesThatAreNotAList() throws Exception { + final ServerConfiguration config = + deserialize(configWithFeatures("{\"holdout.key\": \"q4-global\"}")); + + assertTrue(config.flags.containsKey("valid-flag")); + assertNull(featuresOf(config)); + assertEquals(Integer.valueOf(7), serialIdOf(config)); + } + + private static void assertFeature(final Feature feature, final String key, final Object value) { + assertEquals(key, feature.key); + assertEquals(value, feature.value); + } + + private static List featuresOf(final ServerConfiguration config) { + return config.flags.get("valid-flag").allocations.get(0).splits.get(0).features; + } + + /** Inserts the raw features JSON; null omits the key. */ + private static String configWithFeatures(final String featuresJson) throws IOException { + final String json = resource("split-features.json"); + return featuresJson == null + ? json.replace("\"features\": \"${features}\",", "") + : json.replace("\"${features}\"", featuresJson); + } + private static Integer serialIdOf(final ServerConfiguration config) { return config.flags.get("valid-flag").allocations.get(0).splits.get(0).serialId; } diff --git a/products/feature-flagging/feature-flagging-lib/src/test/resources/remote-config/split-features.json b/products/feature-flagging/feature-flagging-lib/src/test/resources/remote-config/split-features.json new file mode 100644 index 00000000000..d8ff27d02a0 --- /dev/null +++ b/products/feature-flagging/feature-flagging-lib/src/test/resources/remote-config/split-features.json @@ -0,0 +1,35 @@ +{ + "createdAt": "2024-04-17T19:40:53.716Z", + "format": "SERVER", + "environment": { + "name": "Test" + }, + "flags": { + "valid-flag": { + "key": "valid-flag", + "enabled": true, + "variationType": "STRING", + "variations": { + "expected": { + "key": "expected", + "value": "expected" + } + }, + "allocations": [ + { + "key": "default-allocation", + "rules": [], + "splits": [ + { + "serialId": 7, + "features": "${features}", + "variationKey": "expected", + "shards": [] + } + ], + "doLog": true + } + ] + } + } +} From 96b182b76b5574a8aeada68c1af7b043c935d8b4 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Wed, 7 Oct 2026 13:11:48 -0400 Subject: [PATCH 02/13] Cover the features adapter factory and exclude the Feature POJO from coverage Test the factory's type and annotation checks and the read-only adapter, as the other UFC adapters do, and list the Feature model class with the other UFC POJOs that are excluded from coverage verification. --- .../build.gradle.kts | 1 + .../RemoteConfigServiceImplTest.java | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/products/feature-flagging/feature-flagging-bootstrap/build.gradle.kts b/products/feature-flagging/feature-flagging-bootstrap/build.gradle.kts index 848b16a2841..94f08ae55fe 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/build.gradle.kts +++ b/products/feature-flagging/feature-flagging-bootstrap/build.gradle.kts @@ -18,6 +18,7 @@ extra["excludedClassesCoverage"] = listOf( "datadog.trace.api.featureflag.ufc.v1.ConditionConfiguration", "datadog.trace.api.featureflag.ufc.v1.ConditionOperator", "datadog.trace.api.featureflag.ufc.v1.Environment", + "datadog.trace.api.featureflag.ufc.v1.Feature", "datadog.trace.api.featureflag.ufc.v1.Flag", "datadog.trace.api.featureflag.ufc.v1.Rule", "datadog.trace.api.featureflag.ufc.v1.ServerConfiguration", diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java index 4bb1ed817bf..951fc22feb3 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java @@ -1,6 +1,7 @@ package com.datadog.featureflag; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static java.util.Collections.singleton; @@ -336,6 +337,35 @@ void lenientBooleanAdapterIsReadOnly() { UnsupportedOperationException.class, () -> adapter.toJson(mock(JsonWriter.class), true)); } + @Test + void featureListAdapterFactoryOnlyCreatesAdapterForUnannotatedFeatureList() { + final Moshi moshi = moshi(); + final Type featuresType = Types.newParameterizedType(List.class, Feature.class); + + final JsonAdapter adapter = + UniversalFlagConfigParser.FeatureListAdapter.FACTORY.create( + featuresType, emptySet(), moshi); + + assertNotNull(adapter); + assertTrue(adapter instanceof UniversalFlagConfigParser.FeatureListAdapter); + assertNull( + UniversalFlagConfigParser.FeatureListAdapter.FACTORY.create( + Types.newParameterizedType(List.class, String.class), emptySet(), moshi)); + assertNull( + UniversalFlagConfigParser.FeatureListAdapter.FACTORY.create( + featuresType, singleton(mock(Annotation.class)), moshi)); + } + + @Test + void featureListAdapterIsReadOnly() { + final UniversalFlagConfigParser.FeatureListAdapter adapter = + new UniversalFlagConfigParser.FeatureListAdapter(); + + assertThrows( + UnsupportedOperationException.class, + () -> adapter.toJson(mock(JsonWriter.class), emptyList())); + } + @Test void allowsNullFlagMap() throws Exception { final ServerConfiguration config = deserialize(resource("null-flags.json")); From de6f1aa50275732e4e17b34674e9d5edf8c10f81 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Wed, 7 Oct 2026 13:31:55 -0400 Subject: [PATCH 03/13] Let applications record feature flag exposures with ExposureHook Add a public ExposureHook that hands each completed evaluation to a callback with the provider's exposure decision. shouldSend() is true for exactly the exposures the provider sends to Datadog. getFeatures() returns the selected split's features, which the provider now copies into the flag metadata as __dd_feature.. DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED=false stops sending exposures to Datadog. The exposure cache keeps running, so ExposureHook callbacks get the same decisions either way. --- metadata/supported-configurations.json | 8 + .../feature-flagging-api/README.md | 69 ++++ .../trace/api/openfeature/DDEvaluator.java | 43 +++ .../DatadogExposureLoggingGate.java | 32 ++ .../trace/api/openfeature/ExposureHook.java | 126 +++++++ .../api/openfeature/ExposureLoggingHook.java | 15 +- .../trace/api/openfeature/Provider.java | 6 +- .../api/openfeature/ExposureHookTest.java | 325 ++++++++++++++++++ .../openfeature/ExposureLoggingHookTest.java | 2 +- .../config/FeatureFlaggingConfig.java | 7 + 10 files changed, 627 insertions(+), 6 deletions(-) create mode 100644 products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DatadogExposureLoggingGate.java create mode 100644 products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java create mode 100644 products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index 84ee1174006..45e12e44c85 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -1604,6 +1604,14 @@ "aliases": [] } ], + "DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED": [ + { + "version": "A", + "type": "boolean", + "default": "true", + "aliases": [] + } + ], "DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED": [ { "version": "A", diff --git a/products/feature-flagging/feature-flagging-api/README.md b/products/feature-flagging/feature-flagging-api/README.md index 7af28582732..0631aa744ef 100644 --- a/products/feature-flagging/feature-flagging-api/README.md +++ b/products/feature-flagging/feature-flagging-api/README.md @@ -86,6 +86,75 @@ OTEL_EXPORTER_OTLP_PROTOCOL=grpc | `error.type` | Error code (lowercased, only on error) | | `feature_flag.result.allocation_key` | Allocation key (when present) | +## Exposures in your own data store + +The provider sends exposures to Datadog. To also record them in your own data store, register an +`ExposureHook`. It receives every completed evaluation, with the same exposure decision that the +provider uses for Datadog. + +```java +import datadog.trace.api.openfeature.ExposureHook; + +BlockingQueue queue = new LinkedBlockingQueue<>(10_000); + +OpenFeatureAPI.getInstance().addHooks(new ExposureHook(evaluation -> { + if (!evaluation.shouldSend()) { + return; + } + Map features = evaluation.getFeatures(); + queue.offer(new ExposureRecord( + evaluation.getDetails().getFlagKey(), + evaluation.getDetails().getVariant(), + evaluation.getContext().getTargetingKey(), + (String) features.get("holdout.key"), + (String) features.get("holdout.assignment_group"))); +})); +// A background thread drains the queue and writes to the data store. +``` + +- The callback runs on the evaluation thread. Do not write to the data store from the callback. + Copy what you need and hand it to a background queue. +- `shouldSend()` is true for exactly the exposures that the provider sends to Datadog: a successful + evaluation of an allocation that logs exposures, for a subject not yet exposed to the same + allocation, variant and serial id. +- `isExposure()` is true for every successful evaluation of an allocation that logs exposures, + including repeats. `getCacheHit()` tells repeats apart. +- An evaluation that fails, including one that a later `after` hook fails, is not an exposure. +- An exception from the callback is logged and does not affect the evaluation. + +### Features + +`getFeatures()` returns the features of the selected split. Values are a `String`, `Double` or +`Boolean`. Holdout splits carry: + +| Feature | Value | +|---|---| +| `holdout.key` | The holdout key | +| `holdout.assignment_group` | `status_quo` or `all_shipped` | + +The map is empty when the split has no features. + +### Flag metadata + +The same data is in the evaluation's flag metadata, for hooks that use only the OpenFeature API: + +| Key | Type | Meaning | +|---|---|---| +| `allocationKey` | String | The selected allocation | +| `__dd_do_log` | Boolean | The allocation logs exposures | +| `__dd_exposure_cache_hit` | Boolean | The subject was already exposed. Present only when `__dd_do_log` is true | +| `__dd_split_serial_id` | Integer | The split's serial id. Present when the split has one and `__dd_do_log` is true | +| `__dd_feature.` | String, Double or Boolean | One entry for each feature of the selected split | + +### Stop sending exposures to Datadog + +```shell +DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED=false +``` + +The provider then sends no exposures to Datadog. `ExposureHook` callbacks still receive every +evaluation, and `shouldSend()` gives the same result as before. The default is `true`. + ## Requirements - Java 11+ diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java index 9e65ed9f938..8f6914f1c3d 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java @@ -8,6 +8,7 @@ import datadog.trace.api.featureflag.ufc.v1.Allocation; import datadog.trace.api.featureflag.ufc.v1.ConditionConfiguration; import datadog.trace.api.featureflag.ufc.v1.ConditionOperator; +import datadog.trace.api.featureflag.ufc.v1.Feature; import datadog.trace.api.featureflag.ufc.v1.Flag; import datadog.trace.api.featureflag.ufc.v1.ParsedSemver; import datadog.trace.api.featureflag.ufc.v1.Rule; @@ -63,6 +64,19 @@ class DDEvaluator implements Evaluator, FeatureFlaggingGateway.ConfigListener { new AtomicBoolean( SPLIT_SERIAL_ID_SUPPORTED.get() && exposureSerialIdSupported(ExposureEvent.class)); + static final AtomicBoolean SPLIT_FEATURES_SUPPORTED = + new AtomicBoolean(splitFeaturesSupported(Split.class)); + + /** An older agent ships a Split without features; its evaluations then carry no features. */ + static boolean splitFeaturesSupported(final Class splitClass) { + try { + return splitClass.getField("features").getType() == List.class; + } catch (final NoSuchFieldException | LinkageError | RuntimeException e) { + log.debug("The installed Datadog Java agent does not carry split features", e); + return false; + } + } + static boolean splitSerialIdSupported(final Class splitClass) { try { return splitClass.getField("serialId").getType() == Integer.class; @@ -149,6 +163,8 @@ static boolean exposureSerialIdSupported(final Class eventClass) { // Emitted only when the allocation logs exposures: true when this subject's exposure was already // sent, so the exposure hook does not send it again. static final String METADATA_EXPOSURE_CACHE_HIT = "__dd_exposure_cache_hit"; + // Each feature of the selected split is copied to the metadata under this prefix. + static final String METADATA_FEATURE_PREFIX = "__dd_feature."; // Stamped on every DD-produced evaluation (including PROVIDER_NOT_READY, with false). Missing // key = non-DD provider; the hook falls back to false (fail-closed). @@ -611,6 +627,9 @@ private static ProviderEvaluation resolveVariant( } metadataBuilder.addBoolean(METADATA_DO_LOG, doLog); } + if (SPLIT_FEATURES_SUPPORTED.get()) { + addSplitFeatures(metadataBuilder, split); + } if (doLog) { metadataBuilder.addBoolean( METADATA_EXPOSURE_CACHE_HIT, @@ -636,6 +655,30 @@ private static ProviderEvaluation resolveVariant( return result; } + private static void addSplitFeatures( + final ImmutableMetadata.ImmutableMetadataBuilder metadataBuilder, final Split split) { + if (split.features == null) { + return; + } + for (final Object item : split.features) { + if (!(item instanceof Feature)) { + continue; + } + final Feature feature = (Feature) item; + if (feature.key == null) { + continue; + } + final String key = METADATA_FEATURE_PREFIX + feature.key; + if (feature.value instanceof String) { + metadataBuilder.addString(key, (String) feature.value); + } else if (feature.value instanceof Boolean) { + metadataBuilder.addBoolean(key, (Boolean) feature.value); + } else if (feature.value instanceof Number) { + metadataBuilder.addDouble(key, ((Number) feature.value).doubleValue()); + } + } + } + /** The serial id an exposure carries: none when the agent's exposure event predates it. */ static Integer exposureSerialId(final Integer splitSerialId) { return EXPOSURE_SERIAL_ID_SUPPORTED.get() && SPLIT_SERIAL_ID_SUPPORTED.get() diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DatadogExposureLoggingGate.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DatadogExposureLoggingGate.java new file mode 100644 index 00000000000..41585f4c929 --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DatadogExposureLoggingGate.java @@ -0,0 +1,32 @@ +package datadog.trace.api.openfeature; + +import datadog.trace.api.featureflag.config.FeatureFlaggingConfig; +import datadog.trace.bootstrap.config.provider.ConfigProvider; +import java.util.function.BooleanSupplier; + +/** + * Reads {@code DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED} with full {@link ConfigProvider} + * precedence. When false, exposures are not sent to Datadog, but the exposure cache and customer + * {@link ExposureHook}s keep working. On by default. + */ +final class DatadogExposureLoggingGate { + + private DatadogExposureLoggingGate() {} + + static boolean isEnabled() { + return isEnabled( + () -> + ConfigProvider.getInstance() + .getBoolean( + FeatureFlaggingConfig.FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED, true)); + } + + static boolean isEnabled(final BooleanSupplier setting) { + try { + return setting.getAsBoolean(); + } catch (final Throwable t) { + // Keep the default rather than silently stop sending exposures. + return true; + } + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java new file mode 100644 index 00000000000..37f965b99b3 --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java @@ -0,0 +1,126 @@ +package datadog.trace.api.openfeature; + +import static java.util.Objects.requireNonNull; + +import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.Hook; +import dev.openfeature.sdk.HookContext; +import dev.openfeature.sdk.ImmutableMetadata; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Consumer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * An OpenFeature hook that hands each completed flag evaluation to a callback, with the Datadog + * exposure decision, so an application can record exposures in its own data store. + * + *

Register it with {@code OpenFeatureAPI.getInstance().addHooks(new ExposureHook(callback))} or + * on a client. The callback runs for every evaluation, including errors and repeats. Use {@link + * Evaluation#shouldSend()} to keep exactly the exposures that Datadog sends. That decision is the + * same whether or not {@code DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED} is set. + * + *

The callback runs on the evaluation thread: hand the data to a background queue instead of + * writing it from the callback. An exception from the callback is logged and does not affect the + * evaluation. + */ +public final class ExposureHook implements Hook { + + private static final Logger log = LoggerFactory.getLogger(ExposureHook.class); + + private final Consumer callback; + + public ExposureHook(final Consumer callback) { + this.callback = requireNonNull(callback, "callback"); + } + + @Override + public void finallyAfter( + final HookContext context, + final FlagEvaluationDetails details, + final Map hints) { + try { + callback.accept(new Evaluation(context.getCtx(), details)); + } catch (final LinkageError | RuntimeException e) { + log.debug("Exposure callback failed", e); + } + } + + /** A completed evaluation and its Datadog exposure decision. */ + public static final class Evaluation { + private final EvaluationContext context; + private final FlagEvaluationDetails details; + + Evaluation(final EvaluationContext context, final FlagEvaluationDetails details) { + this.context = context; + this.details = details; + } + + /** The evaluation context, including the targeting key. */ + public EvaluationContext getContext() { + return context; + } + + /** The flag key, value, variant, error and flag metadata of the evaluation. */ + public FlagEvaluationDetails getDetails() { + return details; + } + + /** + * True when the evaluation succeeded and selected an allocation that logs exposures, whether or + * not this subject was already exposed. + */ + public boolean isExposure() { + final ImmutableMetadata metadata = details.getFlagMetadata(); + return details.getErrorCode() == null + && details.getVariant() != null + && metadata != null + && metadata.getString("allocationKey") != null + && Boolean.TRUE.equals(metadata.getBoolean(DDEvaluator.METADATA_DO_LOG)); + } + + /** + * True when this subject was already exposed to the same allocation, variant and serial id, + * false when the exposure is new or changed, null when the evaluation is not an exposure. + */ + public Boolean getCacheHit() { + return isExposure() + ? details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT) + : null; + } + + /** True for exactly the exposures that Datadog sends: new or changed exposures. */ + public boolean shouldSend() { + return isExposure() && Boolean.FALSE.equals(getCacheHit()); + } + + /** + * The features of the selected split, such as {@code holdout.key} and {@code + * holdout.assignment_group}. Values are a String, Double or Boolean. Empty when the split has + * none. + */ + public Map getFeatures() { + final ImmutableMetadata metadata = details.getFlagMetadata(); + if (metadata == null) { + return Collections.emptyMap(); + } + Map features = null; + for (final Map.Entry entry : metadata.asUnmodifiableMap().entrySet()) { + if (entry.getKey().startsWith(DDEvaluator.METADATA_FEATURE_PREFIX)) { + if (features == null) { + features = new HashMap<>(); + } + features.put( + entry.getKey().substring(DDEvaluator.METADATA_FEATURE_PREFIX.length()), + entry.getValue()); + } + } + return features == null + ? Collections.emptyMap() + : Collections.unmodifiableMap(features); + } + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java index 7da34ca7c0c..c7dd44f2cfd 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java @@ -19,12 +19,17 @@ class ExposureLoggingHook implements Hook { static final ExposureLoggingHook INSTANCE = - new ExposureLoggingHook<>(ExposureDeduplicationCache.INSTANCE); + new ExposureLoggingHook<>( + ExposureDeduplicationCache.INSTANCE, DatadogExposureLoggingGate.isEnabled()); private final ExposureDeduplicationCache cache; + // When false the hook still records exposures, so customer ExposureHooks see the same cache + // verdicts whether or not exposures go to Datadog. + private final boolean sendToDatadog; - ExposureLoggingHook(final ExposureDeduplicationCache cache) { + ExposureLoggingHook(final ExposureDeduplicationCache cache, final boolean sendToDatadog) { this.cache = cache; + this.sendToDatadog = sendToDatadog; } @Override @@ -60,8 +65,10 @@ public void finallyAfter( DDEvaluator.exposureSerialId( serialValue instanceof Integer ? (Integer) serialValue : null); cache.record(flag, context.getTargetingKey(), allocationKey, variantKey, serialId); - FeatureFlaggingGateway.dispatch( - DDEvaluator.exposureEvent(flag, allocationKey, variantKey, context, serialId)); + if (sendToDatadog) { + FeatureFlaggingGateway.dispatch( + DDEvaluator.exposureEvent(flag, allocationKey, variantKey, context, serialId)); + } } catch (LinkageError e) { // Never let exposure reporting break flag evaluation. } diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/Provider.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/Provider.java index 3a5f07fdf53..436f4facb4a 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/Provider.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/Provider.java @@ -105,7 +105,7 @@ public Provider(final Options options) { hooks.add(spanEnrichmentHook); } try { - hooks.add(ExposureLoggingHook.INSTANCE); + hooks.add(buildExposureLoggingHook()); } catch (LinkageError e) { log.warn("Feature flag exposure reporting is unavailable with the installed agent", e); } @@ -235,6 +235,10 @@ Hook buildFlagEvalLoggingHook() { return FlagEvalLoggingHook.INSTANCE; } + Hook buildExposureLoggingHook() { + return ExposureLoggingHook.INSTANCE; + } + @Override public void shutdown() { if (flagEvalMetrics != null) { diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java new file mode 100644 index 00000000000..d4d6a02f475 --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java @@ -0,0 +1,325 @@ +package datadog.trace.api.openfeature; + +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import datadog.trace.api.featureflag.FeatureFlaggingGateway; +import datadog.trace.api.featureflag.exposure.ExposureEvent; +import datadog.trace.api.featureflag.ufc.v1.Allocation; +import datadog.trace.api.featureflag.ufc.v1.Feature; +import datadog.trace.api.featureflag.ufc.v1.Flag; +import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; +import datadog.trace.api.featureflag.ufc.v1.Split; +import datadog.trace.api.featureflag.ufc.v1.ValueType; +import datadog.trace.api.featureflag.ufc.v1.Variant; +import datadog.trace.api.openfeature.Provider.Options; +import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.ErrorCode; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.FlagValueType; +import dev.openfeature.sdk.Hook; +import dev.openfeature.sdk.HookContext; +import dev.openfeature.sdk.ImmutableMetadata; +import dev.openfeature.sdk.MutableContext; +import dev.openfeature.sdk.OpenFeatureAPI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class ExposureHookTest { + + private final List sentToDatadog = new ArrayList<>(); + private final FeatureFlaggingGateway.ExposureListener listener = sentToDatadog::add; + private final List evaluations = new ArrayList<>(); + private Client client; + + @BeforeEach + void setUp() { + ExposureDeduplicationCache.INSTANCE.clear(); + FeatureFlaggingGateway.addExposureListener(listener); + } + + @AfterEach + void tearDown() { + FeatureFlaggingGateway.removeExposureListener(listener); + OpenFeatureAPI.getInstance().clearHooks(); + OpenFeatureAPI.getInstance().shutdown(); + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + ExposureDeduplicationCache.INSTANCE.clear(); + } + + @Test + void shouldSendMatchesTheExposuresSentToDatadog() throws Exception { + start(true, configuration(Boolean.TRUE, null)); + + evaluate("user-1"); + evaluate("user-1"); + evaluate("user-2"); + + assertEquals(Arrays.asList(true, false, true), shouldSend()); + assertEquals(2, sentToDatadog.size()); + assertEquals("user-1", sentToDatadog.get(0).subject.id); + assertEquals("user-2", sentToDatadog.get(1).subject.id); + } + + @Test + void customerHookStillReceivesExposuresWhenDatadogLoggingIsOff() throws Exception { + start(false, configuration(Boolean.TRUE, null)); + + evaluate("user-1"); + evaluate("user-1"); + evaluate("user-2"); + + assertTrue(sentToDatadog.isEmpty()); + assertEquals(Arrays.asList(true, false, true), shouldSend()); + } + + @Test + void aFailedEvaluationIsNotAnExposure() throws Exception { + start(true, configuration(Boolean.TRUE, null)); + OpenFeatureAPI.getInstance() + .addHooks( + new Hook() { + @Override + public void after( + final HookContext ctx, + final FlagEvaluationDetails details, + final Map hints) { + throw new IllegalStateException("application validation failed"); + } + }); + + final FlagEvaluationDetails details = evaluate("user-1"); + + assertEquals(ErrorCode.GENERAL, details.getErrorCode()); + final ExposureHook.Evaluation evaluation = evaluations.get(0); + assertFalse(evaluation.isExposure()); + assertNull(evaluation.getCacheHit()); + assertFalse(evaluation.shouldSend()); + } + + @Test + void anAllocationThatDoesNotLogExposuresIsNotAnExposure() throws Exception { + start(true, configuration(Boolean.FALSE, null)); + + evaluate("user-1"); + + final ExposureHook.Evaluation evaluation = evaluations.get(0); + assertFalse(evaluation.isExposure()); + assertNull(evaluation.getCacheHit()); + assertFalse(evaluation.shouldSend()); + assertEquals("flag", evaluation.getDetails().getFlagKey()); + assertEquals("user-1", evaluation.getContext().getTargetingKey()); + } + + @Test + void exposesTheFeaturesOfTheSelectedSplit() throws Exception { + start( + true, + configuration( + Boolean.TRUE, + Arrays.asList( + new Feature("holdout.key", "q4-global"), + new Feature("holdout.assignment_group", "status_quo"), + new Feature("holdout.weight", 0.5), + new Feature("holdout.should_include_in_holdout_analysis", true)))); + + final FlagEvaluationDetails details = evaluate("user-1"); + + final Map features = evaluations.get(0).getFeatures(); + assertEquals(4, features.size()); + assertEquals("q4-global", features.get("holdout.key")); + assertEquals("status_quo", features.get("holdout.assignment_group")); + assertEquals(0.5, features.get("holdout.weight")); + assertEquals(true, features.get("holdout.should_include_in_holdout_analysis")); + assertEquals( + "q4-global", + details.getFlagMetadata().getString(DDEvaluator.METADATA_FEATURE_PREFIX + "holdout.key")); + assertThrows(UnsupportedOperationException.class, () -> features.put("other", "value")); + } + + @Test + void hasNoFeaturesWhenTheSplitHasNone() throws Exception { + start(true, configuration(Boolean.TRUE, null)); + + evaluate("user-1"); + + assertTrue(evaluations.get(0).getFeatures().isEmpty()); + } + + @Test + void aFailingCallbackDoesNotAffectTheEvaluation() throws Exception { + start(true, configuration(Boolean.TRUE, null)); + OpenFeatureAPI.getInstance() + .addHooks( + new ExposureHook( + evaluation -> { + throw new IllegalStateException("warehouse unavailable"); + })); + + final FlagEvaluationDetails details = evaluate("user-1"); + + assertNull(details.getErrorCode()); + assertEquals(Integer.valueOf(1), details.getValue()); + assertEquals(1, sentToDatadog.size()); + } + + @Test + void anEvaluationIsAnExposureOnlyWithEverythingAnExposureNeeds() { + assertFalse(evaluation("on", null).isExposure()); + assertFalse(evaluation(null, exposureMetadata("allocation", true, false)).isExposure()); + assertFalse(evaluation("on", exposureMetadata(null, true, false)).isExposure()); + assertFalse(evaluation("on", exposureMetadata("allocation", false, false)).isExposure()); + assertTrue(evaluation("on", exposureMetadata("allocation", true, false)).shouldSend()); + assertEquals( + Boolean.TRUE, evaluation("on", exposureMetadata("allocation", true, true)).getCacheHit()); + assertFalse(evaluation("on", exposureMetadata("allocation", true, true)).shouldSend()); + } + + @Test + void featuresIgnoreOtherMetadataAndAreEmptyWithoutMetadata() { + assertTrue(evaluation("on", null).getFeatures().isEmpty()); + final ExposureHook.Evaluation evaluation = + evaluation( + "on", + ImmutableMetadata.builder() + .addString("allocationKey", "allocation") + .addString(DDEvaluator.METADATA_FEATURE_PREFIX + "holdout.key", "q4-global") + .build()); + + assertEquals(1, evaluation.getFeatures().size()); + assertEquals("q4-global", evaluation.getFeatures().get("holdout.key")); + } + + @Test + void datadogLoggingStaysOnWhenItsSettingCannotBeRead() { + assertTrue(DatadogExposureLoggingGate.isEnabled()); + assertFalse(DatadogExposureLoggingGate.isEnabled(() -> false)); + assertTrue( + DatadogExposureLoggingGate.isEnabled( + () -> { + throw new NoClassDefFoundError("old agent"); + })); + } + + @Test + void theHookContainsCallbackFailures() { + final HookContext context = + HookContext.builder() + .flagKey("flag") + .type(FlagValueType.INTEGER) + .defaultValue(0) + .ctx(new MutableContext("user-1")) + .build(); + final FlagEvaluationDetails details = + FlagEvaluationDetails.builder().flagKey("flag").value(1).build(); + + assertDoesNotThrow( + () -> + new ExposureHook( + evaluation -> { + throw new IllegalStateException("warehouse unavailable"); + }) + .finallyAfter(context, details, emptyMap())); + assertDoesNotThrow( + () -> + new ExposureHook( + evaluation -> { + throw new NoClassDefFoundError("missing class"); + }) + .finallyAfter(context, details, emptyMap())); + } + + @Test + void requiresACallback() { + assertThrows(NullPointerException.class, () -> new ExposureHook(null)); + } + + @Test + void featuresAreSkippedWithAnAgentWhoseSplitHasNoFeatures() { + assertTrue(DDEvaluator.splitFeaturesSupported(Split.class)); + assertFalse(DDEvaluator.splitFeaturesSupported(LegacySplit.class)); + } + + /** A Split from an agent that predates features. */ + static final class LegacySplit { + public Integer serialId; + } + + private void start(final boolean sendToDatadog, final ServerConfiguration configuration) + throws Exception { + FeatureFlaggingGateway.dispatch(configuration); + final OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProviderAndWait( + new Provider( + new Options().initTimeout(10, SECONDS), new DDEvaluator(mock(Runnable.class))) { + @Override + Hook buildExposureLoggingHook() { + return new ExposureLoggingHook<>(ExposureDeduplicationCache.INSTANCE, sendToDatadog); + } + }); + api.addHooks(new ExposureHook(evaluations::add)); + client = api.getClient(); + } + + private static ExposureHook.Evaluation evaluation( + final String variant, final ImmutableMetadata metadata) { + final FlagEvaluationDetails.FlagEvaluationDetailsBuilder builder = + FlagEvaluationDetails.builder().flagKey("flag").value(1).variant(variant); + if (metadata != null) { + builder.flagMetadata(metadata); + } + return new ExposureHook.Evaluation(new MutableContext("user-1"), builder.build()); + } + + private static ImmutableMetadata exposureMetadata( + final String allocationKey, final boolean doLog, final boolean cacheHit) { + final ImmutableMetadata.ImmutableMetadataBuilder builder = + ImmutableMetadata.builder() + .addBoolean(DDEvaluator.METADATA_DO_LOG, doLog) + .addBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT, cacheHit); + if (allocationKey != null) { + builder.addString("allocationKey", allocationKey); + } + return builder.build(); + } + + private FlagEvaluationDetails evaluate(final String targetingKey) { + return client.getIntegerDetails("flag", 0, new MutableContext(targetingKey)); + } + + private List shouldSend() { + final List result = new ArrayList<>(); + for (final ExposureHook.Evaluation evaluation : evaluations) { + result.add(evaluation.shouldSend()); + } + return result; + } + + private static ServerConfiguration configuration( + final Boolean doLog, final List features) { + final Map variations = new HashMap<>(); + variations.put("on", new Variant("on", 1)); + final Split split = new Split(emptyList(), "on", emptyMap(), 7, features); + final Allocation allocation = + new Allocation("allocation", null, null, null, singletonList(split), doLog); + final Map flags = new HashMap<>(); + flags.put( + "flag", new Flag("flag", true, ValueType.INTEGER, variations, singletonList(allocation))); + return new ServerConfiguration("", "", false, null, flags); + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureLoggingHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureLoggingHookTest.java index 6d3e4171af3..95e9ebc7e5d 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureLoggingHookTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureLoggingHookTest.java @@ -155,7 +155,7 @@ void sendsNothingForAnAllocationThatDoesNotLogExposures() { @Test void sendsNothingWithoutTheDetailsAnExposureNeeds() { final ExposureDeduplicationCache cache = new ExposureDeduplicationCache(10); - final ExposureLoggingHook hook = new ExposureLoggingHook<>(cache); + final ExposureLoggingHook hook = new ExposureLoggingHook<>(cache, true); final HookContext withContext = hookContext(new MutableContext("user-1")); hook.finallyAfter(withContext, null, emptyMap()); diff --git a/products/feature-flagging/feature-flagging-config/src/main/java/datadog/trace/api/featureflag/config/FeatureFlaggingConfig.java b/products/feature-flagging/feature-flagging-config/src/main/java/datadog/trace/api/featureflag/config/FeatureFlaggingConfig.java index 1edfd5bcb26..ac6210c1175 100644 --- a/products/feature-flagging/feature-flagging-config/src/main/java/datadog/trace/api/featureflag/config/FeatureFlaggingConfig.java +++ b/products/feature-flagging/feature-flagging-config/src/main/java/datadog/trace/api/featureflag/config/FeatureFlaggingConfig.java @@ -31,6 +31,13 @@ public class FeatureFlaggingConfig { public static final String FLAGGING_EVALUATION_COUNTS_ENABLED = "flagging.evaluation.counts.enabled"; + /** + * Turns off sending exposures to Datadog while customer exposure hooks keep receiving them. + * Default: enabled. Maps to {@code DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED}. + */ + public static final String FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED = + "feature.flags.exposures.datadog.logging.enabled"; + public static final String FEATURE_FLAGS_CONFIGURATION_SOURCE = "feature.flags.configuration.source"; public static final String FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL = From b601b2605f236adb561fe3118572112919e0e86f Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Wed, 7 Oct 2026 16:10:59 -0400 Subject: [PATCH 04/13] Parse the destinations of UFC split features Each feature now lists where SDKs deliver it, such as HOOK, EXPOSURE or EVALUATION. Destinations are open strings: the parser keeps every non-empty string, including ones this SDK does not know, and drops a feature that has no destinations. --- .../trace/api/featureflag/ufc/v1/Feature.java | 10 ++- .../UniversalFlagConfigParser.java | 29 ++++++++- .../RemoteConfigServiceImplTest.java | 64 ++++++++++++++----- 3 files changed, 83 insertions(+), 20 deletions(-) diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java index a5430e97c37..495b547462f 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java @@ -1,13 +1,19 @@ package datadog.trace.api.featureflag.ufc.v1; -/** Key-value data that the UFC attaches to a split for exposure hooks. */ +import java.util.List; + +/** Key-value data that the UFC attaches to a split, with where SDKs deliver it. */ public class Feature { public final String key; // A String, Double or Boolean; the parser drops features with any other value. public final Object value; + // Open strings such as HOOK, EXPOSURE or EVALUATION. Never empty: the parser drops a feature + // without destinations. + public final List destinations; - public Feature(final String key, final Object value) { + public Feature(final String key, final Object value, final List destinations) { this.key = key; this.value = value; + this.destinations = destinations; } } diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java index 7ec108b4bd7..27fd3f70c70 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java @@ -491,6 +491,7 @@ private static Feature readFeature(final JsonReader reader) throws IOException { } String key = null; Object value = null; + List destinations = Collections.emptyList(); reader.beginObject(); while (reader.hasNext()) { final String name = reader.nextName(); @@ -498,15 +499,39 @@ private static Feature readFeature(final JsonReader reader) throws IOException { key = reader.nextString(); } else if ("value".equals(name)) { value = readScalar(reader); + } else if ("destinations".equals(name)) { + destinations = readDestinations(reader); } else { reader.skipValue(); } } reader.endObject(); - if (key == null || key.isEmpty() || value == null) { + if (key == null || key.isEmpty() || value == null || destinations.isEmpty()) { return null; } - return new Feature(key, value); + return new Feature(key, value, destinations); + } + + /** Keeps every non-empty string, including destinations this SDK does not know yet. */ + private static List readDestinations(final JsonReader reader) throws IOException { + if (reader.peek() != JsonReader.Token.BEGIN_ARRAY) { + reader.skipValue(); + return Collections.emptyList(); + } + final List destinations = new ArrayList<>(); + reader.beginArray(); + while (reader.hasNext()) { + if (reader.peek() == JsonReader.Token.STRING) { + final String destination = reader.nextString(); + if (!destination.isEmpty()) { + destinations.add(destination); + } + } else { + reader.skipValue(); + } + } + reader.endArray(); + return Collections.unmodifiableList(destinations); } @Nullable diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java index 951fc22feb3..f11816a9bc0 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java @@ -39,6 +39,7 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Type; import java.time.Instant; +import java.util.Arrays; import java.util.Date; import java.util.List; import java.util.Map; @@ -162,15 +163,22 @@ void parsesSplitFeatures() throws Exception { final ServerConfiguration config = deserialize( configWithFeatures( - "[{\"key\": \"holdout.key\", \"value\": \"q4-global\"}," - + " {\"key\": \"holdout.weight\", \"value\": 0.5}," - + " {\"key\": \"holdout.should_include_in_holdout_analysis\", \"value\": true}]")); + "[{\"key\": \"holdout.key\", \"value\": \"q4-global\", \"destinations\": [\"HOOK\"]}," + + " {\"key\": \"holdout.weight\", \"value\": 0.5," + + " \"destinations\": [\"EXPOSURE\", \"EVALUATION\"]}," + + " {\"key\": \"holdout.should_include_in_holdout_analysis\", \"value\": true," + + " \"destinations\": [\"HOOK\", \"SOME_FUTURE_DESTINATION\"]}]")); final List features = featuresOf(config); assertEquals(3, features.size()); - assertFeature(features.get(0), "holdout.key", "q4-global"); - assertFeature(features.get(1), "holdout.weight", 0.5); - assertFeature(features.get(2), "holdout.should_include_in_holdout_analysis", true); + assertFeature(features.get(0), "holdout.key", "q4-global", "HOOK"); + assertFeature(features.get(1), "holdout.weight", 0.5, "EXPOSURE", "EVALUATION"); + assertFeature( + features.get(2), + "holdout.should_include_in_holdout_analysis", + true, + "HOOK", + "SOME_FUTURE_DESTINATION"); assertEquals(Integer.valueOf(7), serialIdOf(config)); } @@ -184,23 +192,45 @@ void parsesAbsentSplitFeaturesAsNull() throws Exception { @Test void dropsMalformedSplitFeaturesAndKeepsTheFlag() throws Exception { + final String hook = ", \"destinations\": [\"HOOK\"]"; final ServerConfiguration config = deserialize( configWithFeatures( - "[{\"key\": \"object-value\", \"value\": {\"nested\": true}}," - + " {\"key\": \"array-value\", \"value\": [1]}," - + " {\"key\": \"null-value\", \"value\": null}," - + " {\"key\": \"\", \"value\": \"empty key\"}," - + " {\"key\": 3, \"value\": \"numeric key\"}," - + " {\"value\": \"missing key\"}," - + " {\"key\": \"missing value\"}," + "[{\"key\": \"object-value\", \"value\": {\"nested\": true}" + + hook + + "}," + + " {\"key\": \"array-value\", \"value\": [1]" + + hook + + "}," + + " {\"key\": \"null-value\", \"value\": null" + + hook + + "}," + + " {\"key\": \"\", \"value\": \"empty key\"" + + hook + + "}," + + " {\"key\": 3, \"value\": \"numeric key\"" + + hook + + "}," + + " {\"value\": \"missing key\"" + + hook + + "}," + + " {\"key\": \"missing value\"" + + hook + + "}," + + " {\"key\": \"no-destinations\", \"value\": \"dropped\"}," + + " {\"key\": \"empty-destinations\", \"value\": \"dropped\", \"destinations\": []}," + + " {\"key\": \"destinations-not-a-list\", \"value\": \"dropped\"," + + " \"destinations\": \"HOOK\"}," + + " {\"key\": \"only-invalid-destinations\", \"value\": \"dropped\"," + + " \"destinations\": [1, \"\", null, {\"name\": \"HOOK\"}]}," + " \"not-an-object\"," - + " {\"key\": \"holdout.key\", \"value\": \"q4-global\", \"destination\": \"HOOK\"}]")); + + " {\"key\": \"holdout.key\", \"value\": \"q4-global\"," + + " \"destinations\": [1, \"HOOK\", \"\"], \"destination\": \"HOOK\"}]")); assertTrue(config.flags.containsKey("valid-flag")); final List features = featuresOf(config); assertEquals(1, features.size()); - assertFeature(features.get(0), "holdout.key", "q4-global"); + assertFeature(features.get(0), "holdout.key", "q4-global", "HOOK"); } @Test @@ -213,9 +243,11 @@ void ignoresSplitFeaturesThatAreNotAList() throws Exception { assertEquals(Integer.valueOf(7), serialIdOf(config)); } - private static void assertFeature(final Feature feature, final String key, final Object value) { + private static void assertFeature( + final Feature feature, final String key, final Object value, final String... destinations) { assertEquals(key, feature.key); assertEquals(value, feature.value); + assertEquals(Arrays.asList(destinations), feature.destinations); } private static List featuresOf(final ServerConfiguration config) { From 45bf1fb3022521cd0caf46b8779ec4a4d29824d6 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Wed, 7 Oct 2026 16:15:04 -0400 Subject: [PATCH 05/13] Deliver only HOOK features to exposure hooks Copy a split feature into the __dd_feature. metadata only when its destinations include HOOK. EXPOSURE and EVALUATION are known destinations that are not delivered to hooks. A destination the SDK does not know is ignored, with one warning for each new name. --- .../feature-flagging-api/README.md | 9 +++-- .../trace/api/openfeature/DDEvaluator.java | 26 +++++++++++- .../api/openfeature/ExposureHookTest.java | 40 +++++++++++++++++-- 3 files changed, 66 insertions(+), 9 deletions(-) diff --git a/products/feature-flagging/feature-flagging-api/README.md b/products/feature-flagging/feature-flagging-api/README.md index 0631aa744ef..fb716265934 100644 --- a/products/feature-flagging/feature-flagging-api/README.md +++ b/products/feature-flagging/feature-flagging-api/README.md @@ -124,15 +124,16 @@ OpenFeatureAPI.getInstance().addHooks(new ExposureHook(evaluation -> { ### Features -`getFeatures()` returns the features of the selected split. Values are a `String`, `Double` or -`Boolean`. Holdout splits carry: +`getFeatures()` returns the features of the selected split whose destinations include `HOOK`. +Values are a `String`, `Double` or `Boolean`. Holdout splits carry: | Feature | Value | |---|---| | `holdout.key` | The holdout key | | `holdout.assignment_group` | `status_quo` or `all_shipped` | -The map is empty when the split has no features. +The map is empty when the split has no such features. Datadog can add destinations later; the +provider ignores a destination it does not know and logs a warning once for each one. ### Flag metadata @@ -144,7 +145,7 @@ The same data is in the evaluation's flag metadata, for hooks that use only the | `__dd_do_log` | Boolean | The allocation logs exposures | | `__dd_exposure_cache_hit` | Boolean | The subject was already exposed. Present only when `__dd_do_log` is true | | `__dd_split_serial_id` | Integer | The split's serial id. Present when the split has one and `__dd_do_log` is true | -| `__dd_feature.` | String, Double or Boolean | One entry for each feature of the selected split | +| `__dd_feature.` | String, Double or Boolean | One entry for each feature of the selected split that lists the `HOOK` destination | ### Stop sending exposures to Datadog diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java index 8f6914f1c3d..c4747046c36 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java @@ -42,6 +42,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -166,6 +167,13 @@ static boolean exposureSerialIdSupported(final Class eventClass) { // Each feature of the selected split is copied to the metadata under this prefix. static final String METADATA_FEATURE_PREFIX = "__dd_feature."; + static final String FEATURE_DESTINATION_HOOK = "HOOK"; + private static final Set KNOWN_FEATURE_DESTINATIONS = + new HashSet<>(asList(FEATURE_DESTINATION_HOOK, "EXPOSURE", "EVALUATION")); + // A newer UFC can name destinations this SDK does not know; warn once per name, not per + // evaluation. + static final Set WARNED_FEATURE_DESTINATIONS = ConcurrentHashMap.newKeySet(); + // Stamped on every DD-produced evaluation (including PROVIDER_NOT_READY, with false). Missing // key = non-DD provider; the hook falls back to false (fail-closed). static final String METADATA_OBSERVE_FULL_EVALUATION_DATA = "observe_full_evaluation_data"; @@ -665,7 +673,7 @@ private static void addSplitFeatures( continue; } final Feature feature = (Feature) item; - if (feature.key == null) { + if (feature.key == null || !deliversToHook(feature)) { continue; } final String key = METADATA_FEATURE_PREFIX + feature.key; @@ -679,6 +687,22 @@ private static void addSplitFeatures( } } + private static boolean deliversToHook(final Feature feature) { + if (feature.destinations == null) { + return false; + } + boolean hook = false; + for (final String destination : feature.destinations) { + if (FEATURE_DESTINATION_HOOK.equals(destination)) { + hook = true; + } else if (!KNOWN_FEATURE_DESTINATIONS.contains(destination) + && WARNED_FEATURE_DESTINATIONS.add(destination)) { + log.warn("Ignoring unknown feature destination {}", destination); + } + } + return hook; + } + /** The serial id an exposure carries: none when the agent's exposure event predates it. */ static Integer exposureSerialId(final Integer splitSerialId) { return EXPOSURE_SERIAL_ID_SUPPORTED.get() && SPLIT_SERIAL_ID_SUPPORTED.get() diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java index d4d6a02f475..cc934d7902d 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java @@ -34,6 +34,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import org.junit.jupiter.api.AfterEach; @@ -133,15 +134,26 @@ void exposesTheFeaturesOfTheSelectedSplit() throws Exception { configuration( Boolean.TRUE, Arrays.asList( - new Feature("holdout.key", "q4-global"), - new Feature("holdout.assignment_group", "status_quo"), - new Feature("holdout.weight", 0.5), - new Feature("holdout.should_include_in_holdout_analysis", true)))); + new Feature("holdout.key", "q4-global", singletonList("HOOK")), + new Feature("holdout.assignment_group", "status_quo", singletonList("HOOK")), + new Feature("holdout.weight", 0.5, Arrays.asList("EVALUATION", "HOOK")), + new Feature( + "holdout.should_include_in_holdout_analysis", + true, + Arrays.asList("HOOK", "SOME_FUTURE_DESTINATION")), + new Feature("bandit.policy_id", "p2", Arrays.asList("EXPOSURE", "EVALUATION")), + new Feature("unknown.only", "x", singletonList("SOME_FUTURE_DESTINATION"))))); final FlagEvaluationDetails details = evaluate("user-1"); final Map features = evaluations.get(0).getFeatures(); assertEquals(4, features.size()); + assertFalse( + features.containsKey("bandit.policy_id"), + "a feature without HOOK is not delivered to hooks"); + assertFalse( + features.containsKey("unknown.only"), + "a feature with only unknown destinations goes nowhere"); assertEquals("q4-global", features.get("holdout.key")); assertEquals("status_quo", features.get("holdout.assignment_group")); assertEquals(0.5, features.get("holdout.weight")); @@ -152,6 +164,26 @@ void exposesTheFeaturesOfTheSelectedSplit() throws Exception { assertThrows(UnsupportedOperationException.class, () -> features.put("other", "value")); } + @Test + void warnsOnceForEachUnknownDestination() throws Exception { + DDEvaluator.WARNED_FEATURE_DESTINATIONS.clear(); + start( + true, + configuration( + Boolean.TRUE, + Arrays.asList( + new Feature("holdout.key", "q4-global", Arrays.asList("HOOK", "NEW_A")), + new Feature("other", "x", Arrays.asList("NEW_A", "NEW_B", "EXPOSURE"))))); + + evaluate("user-1"); + evaluate("user-2"); + + assertEquals( + new HashSet<>(Arrays.asList("NEW_A", "NEW_B")), DDEvaluator.WARNED_FEATURE_DESTINATIONS); + assertEquals(1, evaluations.get(1).getFeatures().size()); + DDEvaluator.WARNED_FEATURE_DESTINATIONS.clear(); + } + @Test void hasNoFeaturesWhenTheSplitHasNone() throws Exception { start(true, configuration(Boolean.TRUE, null)); From 7744320bb3ab1ba80bde9a6a3f21ef5646823c56 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Thu, 8 Oct 2026 15:00:52 -0400 Subject: [PATCH 06/13] Replace getCacheHit() with a primitive isCacheHit() SpotBugs reports NP_BOOLEAN_RETURN_NULL because getCacheHit() returned a Boolean that is null when the evaluation is not an exposure. A caller that unboxes it gets a NullPointerException. isCacheHit() returns false in that case; isExposure() still tells callers whether it is one. shouldSend() still needs an explicit false stamp, so sending is unchanged. --- .../feature-flagging-api/README.md | 2 +- .../trace/api/openfeature/ExposureHook.java | 17 ++++++++++------- .../trace/api/openfeature/ExposureHookTest.java | 11 +++++++---- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/products/feature-flagging/feature-flagging-api/README.md b/products/feature-flagging/feature-flagging-api/README.md index fb716265934..b3ed5f0073a 100644 --- a/products/feature-flagging/feature-flagging-api/README.md +++ b/products/feature-flagging/feature-flagging-api/README.md @@ -118,7 +118,7 @@ OpenFeatureAPI.getInstance().addHooks(new ExposureHook(evaluation -> { evaluation of an allocation that logs exposures, for a subject not yet exposed to the same allocation, variant and serial id. - `isExposure()` is true for every successful evaluation of an allocation that logs exposures, - including repeats. `getCacheHit()` tells repeats apart. + including repeats. `isCacheHit()` tells repeats apart. - An evaluation that fails, including one that a later `after` hook fails, is not an exposure. - An exception from the callback is logged and does not affect the evaluation. diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java index 37f965b99b3..bb315f77cb1 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java @@ -83,18 +83,21 @@ public boolean isExposure() { } /** - * True when this subject was already exposed to the same allocation, variant and serial id, - * false when the exposure is new or changed, null when the evaluation is not an exposure. + * True when this exposure repeats one already recorded for this subject: same allocation, + * variant and serial id. False for a new or changed exposure, and for an evaluation that is not + * an exposure. */ - public Boolean getCacheHit() { - return isExposure() - ? details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT) - : null; + public boolean isCacheHit() { + return isExposure() && Boolean.TRUE.equals(cacheHitStamp()); } /** True for exactly the exposures that Datadog sends: new or changed exposures. */ public boolean shouldSend() { - return isExposure() && Boolean.FALSE.equals(getCacheHit()); + return isExposure() && Boolean.FALSE.equals(cacheHitStamp()); + } + + private Boolean cacheHitStamp() { + return details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT); } /** diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java index cc934d7902d..fd4a809be3a 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java @@ -109,7 +109,7 @@ public void after( assertEquals(ErrorCode.GENERAL, details.getErrorCode()); final ExposureHook.Evaluation evaluation = evaluations.get(0); assertFalse(evaluation.isExposure()); - assertNull(evaluation.getCacheHit()); + assertFalse(evaluation.isCacheHit()); assertFalse(evaluation.shouldSend()); } @@ -121,7 +121,7 @@ void anAllocationThatDoesNotLogExposuresIsNotAnExposure() throws Exception { final ExposureHook.Evaluation evaluation = evaluations.get(0); assertFalse(evaluation.isExposure()); - assertNull(evaluation.getCacheHit()); + assertFalse(evaluation.isCacheHit()); assertFalse(evaluation.shouldSend()); assertEquals("flag", evaluation.getDetails().getFlagKey()); assertEquals("user-1", evaluation.getContext().getTargetingKey()); @@ -217,8 +217,11 @@ void anEvaluationIsAnExposureOnlyWithEverythingAnExposureNeeds() { assertFalse(evaluation("on", exposureMetadata(null, true, false)).isExposure()); assertFalse(evaluation("on", exposureMetadata("allocation", false, false)).isExposure()); assertTrue(evaluation("on", exposureMetadata("allocation", true, false)).shouldSend()); - assertEquals( - Boolean.TRUE, evaluation("on", exposureMetadata("allocation", true, true)).getCacheHit()); + assertFalse(evaluation("on", exposureMetadata("allocation", true, false)).isCacheHit()); + assertTrue(evaluation("on", exposureMetadata("allocation", true, true)).isCacheHit()); + assertFalse( + evaluation("on", exposureMetadata("allocation", false, true)).isCacheHit(), + "an evaluation that is not an exposure is never a cache hit"); assertFalse(evaluation("on", exposureMetadata("allocation", true, true)).shouldSend()); } From 4a665f26641c4803cc341058b02005dae2342d01 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Fri, 9 Oct 2026 15:41:47 -0400 Subject: [PATCH 07/13] Cap the unknown feature destinations the evaluator remembers The warn-once set kept every unknown destination name for the life of the application, so a stream of configurations with new names grew it without bound. It now stops at 64 names; later unknown destinations are still ignored, only without a warning. --- .../trace/api/openfeature/DDEvaluator.java | 2 ++ .../api/openfeature/ExposureHookTest.java | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java index c4747046c36..8509671ba1b 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java @@ -172,6 +172,7 @@ static boolean exposureSerialIdSupported(final Class eventClass) { new HashSet<>(asList(FEATURE_DESTINATION_HOOK, "EXPOSURE", "EVALUATION")); // A newer UFC can name destinations this SDK does not know; warn once per name, not per // evaluation. + static final int MAX_WARNED_FEATURE_DESTINATIONS = 64; static final Set WARNED_FEATURE_DESTINATIONS = ConcurrentHashMap.newKeySet(); // Stamped on every DD-produced evaluation (including PROVIDER_NOT_READY, with false). Missing @@ -696,6 +697,7 @@ private static boolean deliversToHook(final Feature feature) { if (FEATURE_DESTINATION_HOOK.equals(destination)) { hook = true; } else if (!KNOWN_FEATURE_DESTINATIONS.contains(destination) + && WARNED_FEATURE_DESTINATIONS.size() < MAX_WARNED_FEATURE_DESTINATIONS && WARNED_FEATURE_DESTINATIONS.add(destination)) { log.warn("Ignoring unknown feature destination {}", destination); } diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java index fd4a809be3a..87599519362 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java @@ -33,6 +33,7 @@ import dev.openfeature.sdk.OpenFeatureAPI; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -184,6 +185,33 @@ void warnsOnceForEachUnknownDestination() throws Exception { DDEvaluator.WARNED_FEATURE_DESTINATIONS.clear(); } + @Test + void stopsRememberingUnknownDestinationsAtTheCap() throws Exception { + DDEvaluator.WARNED_FEATURE_DESTINATIONS.clear(); + for (int i = 0; i < DDEvaluator.MAX_WARNED_FEATURE_DESTINATIONS; i++) { + DDEvaluator.WARNED_FEATURE_DESTINATIONS.add("OLD_" + i); + } + try { + start( + true, + configuration( + Boolean.TRUE, + singletonList( + new Feature("holdout.key", "q4-global", Arrays.asList("HOOK", "NEW_A"))))); + + evaluate("user-1"); + + assertEquals( + DDEvaluator.MAX_WARNED_FEATURE_DESTINATIONS, + DDEvaluator.WARNED_FEATURE_DESTINATIONS.size()); + assertFalse(DDEvaluator.WARNED_FEATURE_DESTINATIONS.contains("NEW_A")); + assertEquals( + Collections.singletonMap("holdout.key", "q4-global"), evaluations.get(0).getFeatures()); + } finally { + DDEvaluator.WARNED_FEATURE_DESTINATIONS.clear(); + } + } + @Test void hasNoFeaturesWhenTheSplitHasNone() throws Exception { start(true, configuration(Boolean.TRUE, null)); From 5d6c7de19eb80c505b73ebec88008528bdea2eee Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Tue, 13 Oct 2026 10:49:39 -0400 Subject: [PATCH 08/13] State when shouldSend() matches Datadog, and surface hook failures shouldSend() matches the exposures Datadog receives only when the provider's own hooks run. The Javadoc now says so, names the MultiProvider case before OpenFeature SDK 1.23.0, and says that ExposureHook is optional because the flag metadata carries the same data. ExposureHook now skips the callback when OpenFeature passes no details, and logs the first callback failure at warn level instead of debug. ExposureLoggingHook logs its first LinkageError at warn level instead of swallowing it. Both read flag metadata through its map, because the ImmutableMetadata getters build a debug message for every absent key. --- .../trace/api/openfeature/ExposureHook.java | 41 +++++++++++++++---- .../api/openfeature/ExposureLoggingHook.java | 9 ++++ .../api/openfeature/ExposureHookTest.java | 16 ++++++++ 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java index bb315f77cb1..839fbbb7207 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java @@ -10,6 +10,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -18,10 +19,17 @@ * An OpenFeature hook that hands each completed flag evaluation to a callback, with the Datadog * exposure decision, so an application can record exposures in its own data store. * + *

This hook is optional. The provider puts the same data in the flag metadata, so any + * OpenFeature hook can read it (see "Flag metadata" in the README). This class reads it for you. + * *

Register it with {@code OpenFeatureAPI.getInstance().addHooks(new ExposureHook(callback))} or * on a client. The callback runs for every evaluation, including errors and repeats. Use {@link - * Evaluation#shouldSend()} to keep exactly the exposures that Datadog sends. That decision is the - * same whether or not {@code DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED} is set. + * Evaluation#shouldSend()} to keep the new or changed exposures. When the provider's own hooks run, + * these are exactly the exposures that Datadog sends, whether or not {@code + * DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED} is set. The provider's hooks run when the + * application evaluates through an OpenFeature client. Inside a {@code MultiProvider} they run only + * with OpenFeature SDK 1.23.0 or later. With an older SDK, Datadog receives no exposures from a + * provider inside a {@code MultiProvider}, and {@code shouldSend()} is true for every exposure. * *

The callback runs on the evaluation thread: hand the data to a background queue instead of * writing it from the callback. An exception from the callback is logged and does not affect the @@ -32,6 +40,7 @@ public final class ExposureHook implements Hook { private static final Logger log = LoggerFactory.getLogger(ExposureHook.class); private final Consumer callback; + private final AtomicBoolean warned = new AtomicBoolean(); public ExposureHook(final Consumer callback) { this.callback = requireNonNull(callback, "callback"); @@ -42,10 +51,17 @@ public void finallyAfter( final HookContext context, final FlagEvaluationDetails details, final Map hints) { + if (details == null) { + return; + } try { callback.accept(new Evaluation(context.getCtx(), details)); } catch (final LinkageError | RuntimeException e) { - log.debug("Exposure callback failed", e); + if (warned.compareAndSet(false, true)) { + log.warn("Exposure callback failed; later failures are logged at debug level", e); + } else { + log.debug("Exposure callback failed", e); + } } } @@ -74,12 +90,11 @@ public FlagEvaluationDetails getDetails() { * not this subject was already exposed. */ public boolean isExposure() { - final ImmutableMetadata metadata = details.getFlagMetadata(); + final Map values = metadata(); return details.getErrorCode() == null && details.getVariant() != null - && metadata != null - && metadata.getString("allocationKey") != null - && Boolean.TRUE.equals(metadata.getBoolean(DDEvaluator.METADATA_DO_LOG)); + && values.get("allocationKey") instanceof String + && Boolean.TRUE.equals(values.get(DDEvaluator.METADATA_DO_LOG)); } /** @@ -96,8 +111,16 @@ public boolean shouldSend() { return isExposure() && Boolean.FALSE.equals(cacheHitStamp()); } - private Boolean cacheHitStamp() { - return details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT); + private Object cacheHitStamp() { + return metadata().get(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT); + } + + // The ImmutableMetadata getters build a debug message for every absent key. + private Map metadata() { + final ImmutableMetadata metadata = details.getFlagMetadata(); + return metadata == null + ? Collections.emptyMap() + : metadata.asUnmodifiableMap(); } /** diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java index c7dd44f2cfd..73c588ed5b3 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java @@ -7,6 +7,9 @@ import dev.openfeature.sdk.HookContext; import dev.openfeature.sdk.ImmutableMetadata; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Sends the exposure for a logged allocation to the agent once the evaluation has succeeded. @@ -18,6 +21,9 @@ */ class ExposureLoggingHook implements Hook { + private static final Logger log = LoggerFactory.getLogger(ExposureLoggingHook.class); + private static final AtomicBoolean WARNED = new AtomicBoolean(); + static final ExposureLoggingHook INSTANCE = new ExposureLoggingHook<>( ExposureDeduplicationCache.INSTANCE, DatadogExposureLoggingGate.isEnabled()); @@ -71,6 +77,9 @@ public void finallyAfter( } } catch (LinkageError e) { // Never let exposure reporting break flag evaluation. + if (WARNED.compareAndSet(false, true)) { + log.warn("Could not send a feature flag exposure; later failures are not logged", e); + } } } } diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java index 87599519362..5ed1f9e818c 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java @@ -312,6 +312,22 @@ void requiresACallback() { assertThrows(NullPointerException.class, () -> new ExposureHook(null)); } + @Test + void skipsTheCallbackWhenThereAreNoDetails() { + final List seen = new ArrayList<>(); + final HookContext context = + HookContext.builder() + .flagKey("flag") + .type(FlagValueType.INTEGER) + .defaultValue(0) + .ctx(new MutableContext("user-1")) + .build(); + + new ExposureHook(seen::add).finallyAfter(context, null, Collections.emptyMap()); + + assertTrue(seen.isEmpty()); + } + @Test void featuresAreSkippedWithAnAgentWhoseSplitHasNoFeatures() { assertTrue(DDEvaluator.splitFeaturesSupported(Split.class)); From 0a4631e6ccd0645aa80ab831bb48802d14203238 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Tue, 13 Oct 2026 10:49:41 -0400 Subject: [PATCH 09/13] Test that the Datadog exposure logging setting turns sending off The earlier tests passed false straight to the gate or the hook, so a wrong key would still pass. This forked test sets the real system property and checks that the provider sends nothing to Datadog while ExposureHook keeps its decisions. --- ...adogExposureLoggingDisabledForkedTest.java | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DatadogExposureLoggingDisabledForkedTest.java diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DatadogExposureLoggingDisabledForkedTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DatadogExposureLoggingDisabledForkedTest.java new file mode 100644 index 00000000000..6494203d798 --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DatadogExposureLoggingDisabledForkedTest.java @@ -0,0 +1,86 @@ +package datadog.trace.api.openfeature; + +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import datadog.trace.api.featureflag.FeatureFlaggingGateway; +import datadog.trace.api.featureflag.exposure.ExposureEvent; +import datadog.trace.api.featureflag.ufc.v1.Allocation; +import datadog.trace.api.featureflag.ufc.v1.Flag; +import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; +import datadog.trace.api.featureflag.ufc.v1.Split; +import datadog.trace.api.featureflag.ufc.v1.ValueType; +import datadog.trace.api.featureflag.ufc.v1.Variant; +import datadog.trace.api.openfeature.Provider.Options; +import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.MutableContext; +import dev.openfeature.sdk.OpenFeatureAPI; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +/** + * Turns off Datadog exposure logging through the real setting. The gate is read once, when + * ExposureLoggingHook loads, so the property is set before that and the forked task gives this + * class its own JVM. + */ +class DatadogExposureLoggingDisabledForkedTest { + + static { + System.setProperty("dd.feature.flags.exposures.datadog.logging.enabled", "false"); + } + + private final List dispatched = new ArrayList<>(); + private final FeatureFlaggingGateway.ExposureListener listener = dispatched::add; + + @AfterEach + void tearDown() { + FeatureFlaggingGateway.removeExposureListener(listener); + OpenFeatureAPI.getInstance().clearHooks(); + OpenFeatureAPI.getInstance().shutdown(); + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + ExposureDeduplicationCache.INSTANCE.clear(); + } + + @Test + void sendsNoExposuresToDatadogWhileExposureHooksKeepTheirDecisions() throws Exception { + final List evaluations = new ArrayList<>(); + FeatureFlaggingGateway.dispatch(configuration()); + final OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProviderAndWait( + new Provider( + new Options().initTimeout(10, SECONDS), new DDEvaluator(mock(Runnable.class)))); + api.addHooks(new ExposureHook(evaluations::add)); + FeatureFlaggingGateway.addExposureListener(listener); + final Client client = api.getClient(); + + client.getIntegerDetails("flag", 0, new MutableContext("user-1")); + client.getIntegerDetails("flag", 0, new MutableContext("user-1")); + + assertTrue(dispatched.isEmpty(), "nothing is sent to Datadog"); + assertEquals(2, evaluations.size()); + assertTrue(evaluations.get(0).shouldSend()); + assertFalse(evaluations.get(1).shouldSend(), "the cache still records the first exposure"); + } + + private static ServerConfiguration configuration() { + final Map variations = new HashMap<>(); + variations.put("on", new Variant("on", 1)); + final Split split = new Split(emptyList(), "on", emptyMap(), 7); + final Allocation allocation = + new Allocation("allocation", null, null, null, singletonList(split), Boolean.TRUE); + final Map flags = new HashMap<>(); + flags.put( + "flag", new Flag("flag", true, ValueType.INTEGER, variations, singletonList(allocation))); + return new ServerConfiguration("", "", false, null, flags); + } +} From 9135fa20e93fed786bc5e51f7a54a4bcaf0279f9 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Tue, 13 Oct 2026 10:51:05 -0400 Subject: [PATCH 10/13] Lead the exposure README with the flag metadata any hook can read The README now says first that the provider puts its exposure decision and the split's features in the flag metadata, and that ExposureHook is a convenience on top of it. It says how to read the metadata, narrows the shouldSend() claim to when the provider's hooks run, corrects the __dd_do_log and __dd_split_serial_id rows, and adds a table of the dd-java-agent version each capability needs. --- .../feature-flagging-api/README.md | 42 ++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/products/feature-flagging/feature-flagging-api/README.md b/products/feature-flagging/feature-flagging-api/README.md index b3ed5f0073a..318e77ab160 100644 --- a/products/feature-flagging/feature-flagging-api/README.md +++ b/products/feature-flagging/feature-flagging-api/README.md @@ -88,9 +88,13 @@ OTEL_EXPORTER_OTLP_PROTOCOL=grpc ## Exposures in your own data store -The provider sends exposures to Datadog. To also record them in your own data store, register an -`ExposureHook`. It receives every completed evaluation, with the same exposure decision that the -provider uses for Datadog. +The provider sends exposures to Datadog. It also puts its exposure decision and the selected split's +features into the flag metadata of every evaluation, so any OpenFeature hook can record exposures in +your own data store (see [Flag metadata](#flag-metadata)). + +For convenience, `dd-openfeature` ships `ExposureHook`, which reads that metadata for you. It +receives every completed evaluation, with the same exposure decision that the provider uses for +Datadog. ```java import datadog.trace.api.openfeature.ExposureHook; @@ -114,13 +118,17 @@ OpenFeatureAPI.getInstance().addHooks(new ExposureHook(evaluation -> { - The callback runs on the evaluation thread. Do not write to the data store from the callback. Copy what you need and hand it to a background queue. -- `shouldSend()` is true for exactly the exposures that the provider sends to Datadog: a successful - evaluation of an allocation that logs exposures, for a subject not yet exposed to the same - allocation, variant and serial id. +- `shouldSend()` is true for a successful evaluation of an allocation that logs exposures, for a + subject not yet exposed to the same allocation, variant and serial id. These are exactly the + exposures that the provider sends to Datadog, as long as the provider's own hooks run: evaluate + through an OpenFeature client, and inside a `MultiProvider` use OpenFeature SDK 1.23.0 or later. + With an older SDK, a provider inside a `MultiProvider` sends no exposures, and `shouldSend()` is + true for every exposure. - `isExposure()` is true for every successful evaluation of an allocation that logs exposures, including repeats. `isCacheHit()` tells repeats apart. - An evaluation that fails, including one that a later `after` hook fails, is not an exposure. -- An exception from the callback is logged and does not affect the evaluation. +- An exception from the callback does not affect the evaluation. The first one is logged at warn + level, later ones at debug level. ### Features @@ -137,16 +145,30 @@ provider ignores a destination it does not know and logs a warning once for each ### Flag metadata -The same data is in the evaluation's flag metadata, for hooks that use only the OpenFeature API: +The same data is in the evaluation's flag metadata, for hooks that use only the OpenFeature API. +Read it in the hook's `finallyAfter` stage, where the details match what the application receives, +and skip details that have an error code. `ExposureHook` does both. | Key | Type | Meaning | |---|---|---| | `allocationKey` | String | The selected allocation | -| `__dd_do_log` | Boolean | The allocation logs exposures | +| `__dd_do_log` | Boolean | Whether the allocation logs exposures. Present when the allocation logs exposures or span enrichment is on | | `__dd_exposure_cache_hit` | Boolean | The subject was already exposed. Present only when `__dd_do_log` is true | -| `__dd_split_serial_id` | Integer | The split's serial id. Present when the split has one and `__dd_do_log` is true | +| `__dd_split_serial_id` | Integer | The split's serial id. Present when `__dd_do_log` is present, the split has a serial id, and the installed `dd-java-agent` supports serial ids | | `__dd_feature.` | String, Double or Boolean | One entry for each feature of the selected split that lists the `HOOK` destination | +### Agent versions + +`dd-openfeature` and `dd-java-agent` ship separately. The provider turns a capability off when the +installed agent lacks it: + +| Capability | Needs `dd-java-agent` | Without it | +|---|---|---| +| Serial ids on splits (`__dd_split_serial_id`) | 1.65.0 | The key is absent | +| Serial ids on exposures sent to Datadog | 1.67.0 | Exposures are sent without a serial id, so holdout attribution is lost | +| Flag evaluation events sent to Datadog | 1.66.0 | No flag evaluation events are sent | +| Split features (`getFeatures()`, `__dd_feature.`) | The first release that includes this change | `getFeatures()` is empty | + ### Stop sending exposures to Datadog ```shell From 821c32aed5ff4baefb6afc0d44077b5a7914e705 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Wed, 14 Oct 2026 09:47:27 -0400 Subject: [PATCH 11/13] Test evaluation with an agent whose Split has no features No released dd-java-agent has Split.features yet. The provider must skip reading it there, or every evaluation throws NoSuchFieldError. --- .../api/openfeature/ExposureHookTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java index 5ed1f9e818c..c5ee0b70320 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java @@ -212,6 +212,26 @@ void stopsRememberingUnknownDestinationsAtTheCap() throws Exception { } } + @Test + void evaluatesWithoutFeaturesWhenTheAgentSplitHasNone() throws Exception { + start( + true, + configuration( + Boolean.TRUE, + singletonList(new Feature("holdout.key", "q4-global", singletonList("HOOK"))))); + DDEvaluator.SPLIT_FEATURES_SUPPORTED.set(false); + final FlagEvaluationDetails details; + try { + details = evaluate("user-1"); + } finally { + DDEvaluator.SPLIT_FEATURES_SUPPORTED.set(true); + } + + assertNull(details.getErrorCode()); + assertEquals(Integer.valueOf(1), details.getValue()); + assertTrue(evaluations.get(0).getFeatures().isEmpty()); + } + @Test void hasNoFeaturesWhenTheSplitHasNone() throws Exception { start(true, configuration(Boolean.TRUE, null)); From 5dff911a0b0c57230bad93c86d9e075749896295 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Wed, 14 Oct 2026 09:47:28 -0400 Subject: [PATCH 12/13] Correct when shouldSend() matches what Datadog receives It matches only while Datadog exposure logging is on, and a changed serial id counts only with dd-java-agent 1.67.0 or later. The README also gives 1.68.0 for split features, says that the logging setting does not stop flag evaluation events, and that the provider does not read dd-trace-py's DD_FFE_INTAKE_ENABLED. --- .../feature-flagging-api/README.md | 17 +++++++----- .../trace/api/openfeature/ExposureHook.java | 26 ++++++++++++------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/products/feature-flagging/feature-flagging-api/README.md b/products/feature-flagging/feature-flagging-api/README.md index 318e77ab160..7afc0cd0359 100644 --- a/products/feature-flagging/feature-flagging-api/README.md +++ b/products/feature-flagging/feature-flagging-api/README.md @@ -119,11 +119,12 @@ OpenFeatureAPI.getInstance().addHooks(new ExposureHook(evaluation -> { - The callback runs on the evaluation thread. Do not write to the data store from the callback. Copy what you need and hand it to a background queue. - `shouldSend()` is true for a successful evaluation of an allocation that logs exposures, for a - subject not yet exposed to the same allocation, variant and serial id. These are exactly the - exposures that the provider sends to Datadog, as long as the provider's own hooks run: evaluate - through an OpenFeature client, and inside a `MultiProvider` use OpenFeature SDK 1.23.0 or later. - With an older SDK, a provider inside a `MultiProvider` sends no exposures, and `shouldSend()` is - true for every exposure. + subject not yet exposed to the same allocation, variant and serial id. The serial id counts only + with `dd-java-agent` 1.67.0 or later. These are exactly the exposures that the provider sends to + Datadog, as long as Datadog logging is on and the provider's own hooks run: evaluate through an + OpenFeature client, and inside a `MultiProvider` use OpenFeature SDK 1.23.0 or later. With an + older SDK, a provider inside a `MultiProvider` sends no exposures, and `shouldSend()` is true for + every exposure. - `isExposure()` is true for every successful evaluation of an allocation that logs exposures, including repeats. `isCacheHit()` tells repeats apart. - An evaluation that fails, including one that a later `after` hook fails, is not an exposure. @@ -167,7 +168,7 @@ installed agent lacks it: | Serial ids on splits (`__dd_split_serial_id`) | 1.65.0 | The key is absent | | Serial ids on exposures sent to Datadog | 1.67.0 | Exposures are sent without a serial id, so holdout attribution is lost | | Flag evaluation events sent to Datadog | 1.66.0 | No flag evaluation events are sent | -| Split features (`getFeatures()`, `__dd_feature.`) | The first release that includes this change | `getFeatures()` is empty | +| Split features (`getFeatures()`, `__dd_feature.`) | 1.68.0 | `getFeatures()` is empty | ### Stop sending exposures to Datadog @@ -178,6 +179,10 @@ DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED=false The provider then sends no exposures to Datadog. `ExposureHook` callbacks still receive every evaluation, and `shouldSend()` gives the same result as before. The default is `true`. +This setting stops exposures only. Flag evaluation events have their own setting, +`DD_FLAGGING_EVALUATION_COUNTS_ENABLED`. The Java provider does not read dd-trace-py's +`DD_FFE_INTAKE_ENABLED`. + ## Requirements - Java 11+ diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java index 839fbbb7207..72a45cdc1bb 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java @@ -24,16 +24,21 @@ * *

Register it with {@code OpenFeatureAPI.getInstance().addHooks(new ExposureHook(callback))} or * on a client. The callback runs for every evaluation, including errors and repeats. Use {@link - * Evaluation#shouldSend()} to keep the new or changed exposures. When the provider's own hooks run, - * these are exactly the exposures that Datadog sends, whether or not {@code - * DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED} is set. The provider's hooks run when the - * application evaluates through an OpenFeature client. Inside a {@code MultiProvider} they run only - * with OpenFeature SDK 1.23.0 or later. With an older SDK, Datadog receives no exposures from a - * provider inside a {@code MultiProvider}, and {@code shouldSend()} is true for every exposure. + * Evaluation#shouldSend()} to keep the new or changed exposures. A changed serial id counts as a + * change only with {@code dd-java-agent} 1.67.0 or later; with an older agent, only a changed + * allocation or variant does. + * + *

When the provider's own hooks run and {@code + * DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED} is on, these are exactly the exposures that + * Datadog sends. With that setting off, Datadog receives no exposures, and {@code shouldSend()} + * gives the same results. The provider's hooks run when the application evaluates through an + * OpenFeature client. Inside a {@code MultiProvider} they run only with OpenFeature SDK 1.23.0 or + * later. With an older SDK, Datadog receives no exposures from a provider inside a {@code + * MultiProvider}, and {@code shouldSend()} is true for every exposure. * *

The callback runs on the evaluation thread: hand the data to a background queue instead of - * writing it from the callback. An exception from the callback is logged and does not affect the - * evaluation. + * writing it from the callback. An exception from the callback does not affect the evaluation. The + * first one is logged at warn level, later ones at debug level. */ public final class ExposureHook implements Hook { @@ -106,7 +111,10 @@ public boolean isCacheHit() { return isExposure() && Boolean.TRUE.equals(cacheHitStamp()); } - /** True for exactly the exposures that Datadog sends: new or changed exposures. */ + /** + * True for a new or changed exposure: the decision the provider uses for Datadog. The class + * Javadoc says when that decision reaches Datadog. + */ public boolean shouldSend() { return isExposure() && Boolean.FALSE.equals(cacheHitStamp()); } From c2e18385e489eac30f0d3c9c397e1da2361fe23a Mon Sep 17 00:00:00 2001 From: Danyal Khan <50880807+danyal002@users.noreply.github.com> Date: Wed, 14 Oct 2026 10:00:54 -0400 Subject: [PATCH 13/13] Send EXPOSURE and EVALUATION feature flag features to Datadog (#12797) * Send EXPOSURE and EVALUATION features to Datadog A UFC split feature can list the EXPOSURE and EVALUATION destinations. The evaluator now writes each feature once for each destination it lists, under that destination's metadata prefix. The exposure hook puts EXPOSURE features on the exposure event, and the flag-evaluation hook puts EVALUATION features on the flag-evaluation event, whatever the consent, because they come from the flag configuration. Flag-evaluation buckets now include the features in both tiers, so rows with different features never merge, and each row serializes them. Both events gain a features field with new constructors; with an older agent the provider sends them without features. * Keep the first feature when a split repeats a feature key The FFE_FLAGS schema documents that keys are unique within a split and that SDKs keep the first feature with a repeated key. Flag metadata is a map, so the provider used to keep the last one. It now ignores a later feature with a key it has already seen, for every destination. * Check flag-evaluation feature support inside FlagEvalLoggingHook DDEvaluator resolved FlagEvalEvent.class in its static initializer. Agents released before FlagEvalEvent existed then failed DDEvaluator's initialization with NoClassDefFoundError, which broke the provider. The check now lives in FlagEvalLoggingHook, the only class that uses it, and Provider already guards that hook's loading against LinkageError. * Read split features without DDEvaluator, and scan only when needed - Moves featuresWithPrefix into SplitFeatureMetadata, which depends only on the OpenFeature API. ExposureHook.getFeatures() called it on DDEvaluator, which needs the Datadog Java agent, so it threw NoClassDefFoundError where the agent is absent, such as unit tests. - DDEvaluator marks the metadata with __dd_evaluation_features only when the split routes a feature to EVALUATION. FlagEvalLoggingHook scans the metadata only then, instead of on every evaluation. - Adds an end-to-end test of EVALUATION features for an allocation that does not log exposures. Every earlier provider-backed test used doLog=true, so adding features only for logged allocations passed. - Reflows a broken comment. * Share one copy of each feature set across a flush window Each bucket kept the features map that the provider built for its own evaluation, and each key kept its own canonical string. Full-tier keys include the targeting key, so up to PER_FLAG_CAP subjects of one split held that many equal maps and strings until the flush. The aggregator now keeps one key and map for each distinct feature set, and clears them with the rest of the window. * Test that ExposureHook works without the Datadog Java agent The test classpath always has the agent classes, so no other test sees ExposureHook reach for them. This test loads ExposureHook in a class loader that hides DDEvaluator and the agent classes, as in an application without the agent. * Read flag evaluation metadata from its map, and rename the marker - FlagEvalLoggingHook reads allocationKey, the evaluation timestamp, the consent key and the evaluation-features marker from one map. The ImmutableMetadata getters build a debug message for every absent key, and error and default results carry only the consent key. - Renames the marker to __dd_has_evaluation_features, so that it does not differ from the __dd_evaluation_feature. prefix by one character. - Rewraps a comment that google-java-format had split. --- .../trace/api/openfeature/DDEvaluator.java | 112 +++++++++---- .../trace/api/openfeature/ExposureHook.java | 21 +-- .../api/openfeature/ExposureLoggingHook.java | 9 +- .../api/openfeature/FlagEvalLoggingHook.java | 81 +++++++-- .../api/openfeature/SplitFeatureMetadata.java | 36 ++++ .../api/openfeature/ExposureHookTest.java | 158 ++++++++++++++++++ .../ExposureHookWithoutAgentTest.java | 103 ++++++++++++ .../openfeature/FlagEvalLoggingHookTest.java | 56 +++++++ .../featureflag/exposure/ExposureEvent.java | 18 ++ .../flagevaluation/FlagEvalEvent.java | 29 ++++ .../flagevaluation/FlagEvalEventTest.java | 15 ++ .../featureflag/FlagEvaluationAggregator.java | 96 ++++++++--- .../featureflag/FlagEvaluationPayloads.java | 13 +- .../FeatureFlagEvpPublisherTest.java | 20 +++ .../FlagEvaluationAggregatorTest.java | 90 +++++++++- .../FlagEvaluationPayloadsTest.java | 55 +++++- 16 files changed, 817 insertions(+), 95 deletions(-) create mode 100644 products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/SplitFeatureMetadata.java create mode 100644 products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookWithoutAgentTest.java diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java index 8509671ba1b..2afec791851 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java @@ -68,6 +68,27 @@ class DDEvaluator implements Evaluator, FeatureFlaggingGateway.ConfigListener { static final AtomicBoolean SPLIT_FEATURES_SUPPORTED = new AtomicBoolean(splitFeaturesSupported(Split.class)); + static final AtomicBoolean EXPOSURE_FEATURES_SUPPORTED = + new AtomicBoolean(exposureFeaturesSupported(ExposureEvent.class)); + + /** An older agent's exposure event has no features; its exposures are then sent without them. */ + static boolean exposureFeaturesSupported(final Class eventClass) { + try { + eventClass.getConstructor( + long.class, + datadog.trace.api.featureflag.exposure.Allocation.class, + datadog.trace.api.featureflag.exposure.Flag.class, + datadog.trace.api.featureflag.exposure.Variant.class, + Subject.class, + Integer.class, + Map.class); + return true; + } catch (final NoSuchMethodException | LinkageError | RuntimeException e) { + log.debug("The installed Datadog Java agent does not send features on exposures", e); + return false; + } + } + /** An older agent ships a Split without features; its evaluations then carry no features. */ static boolean splitFeaturesSupported(final Class splitClass) { try { @@ -164,12 +185,19 @@ static boolean exposureSerialIdSupported(final Class eventClass) { // Emitted only when the allocation logs exposures: true when this subject's exposure was already // sent, so the exposure hook does not send it again. static final String METADATA_EXPOSURE_CACHE_HIT = "__dd_exposure_cache_hit"; - // Each feature of the selected split is copied to the metadata under this prefix. + // Each feature of the selected split is copied to the metadata once for each destination it + // lists, under that destination's prefix. Only the HOOK prefix is a public contract; the + // other two carry features to Datadog's own exposure and flag-evaluation hooks. static final String METADATA_FEATURE_PREFIX = "__dd_feature."; + static final String METADATA_EXPOSURE_FEATURE_PREFIX = "__dd_exposure_feature."; + static final String METADATA_EVALUATION_FEATURE_PREFIX = "__dd_evaluation_feature."; + // Present only when the split routes a feature to EVALUATION, so FlagEvalLoggingHook can skip + // scanning the metadata on every other evaluation. + static final String METADATA_HAS_EVALUATION_FEATURES = "__dd_has_evaluation_features"; static final String FEATURE_DESTINATION_HOOK = "HOOK"; - private static final Set KNOWN_FEATURE_DESTINATIONS = - new HashSet<>(asList(FEATURE_DESTINATION_HOOK, "EXPOSURE", "EVALUATION")); + static final String FEATURE_DESTINATION_EXPOSURE = "EXPOSURE"; + static final String FEATURE_DESTINATION_EVALUATION = "EVALUATION"; // A newer UFC can name destinations this SDK does not know; warn once per name, not per // evaluation. static final int MAX_WARNED_FEATURE_DESTINATIONS = 64; @@ -669,40 +697,62 @@ private static void addSplitFeatures( if (split.features == null) { return; } + Set seenKeys = null; + boolean evaluationFeatures = false; for (final Object item : split.features) { if (!(item instanceof Feature)) { continue; } final Feature feature = (Feature) item; - if (feature.key == null || !deliversToHook(feature)) { + if (feature.key == null || feature.destinations == null) { continue; } - final String key = METADATA_FEATURE_PREFIX + feature.key; - if (feature.value instanceof String) { - metadataBuilder.addString(key, (String) feature.value); - } else if (feature.value instanceof Boolean) { - metadataBuilder.addBoolean(key, (Boolean) feature.value); - } else if (feature.value instanceof Number) { - metadataBuilder.addDouble(key, ((Number) feature.value).doubleValue()); + if (seenKeys == null) { + seenKeys = new HashSet<>(); + } + if (!seenKeys.add(feature.key)) { + continue; + } + for (final String destination : feature.destinations) { + final String prefix = metadataPrefix(destination); + if (prefix != null) { + addFeature(metadataBuilder, prefix + feature.key, feature.value); + evaluationFeatures |= prefix.equals(METADATA_EVALUATION_FEATURE_PREFIX); + } else if (WARNED_FEATURE_DESTINATIONS.size() < MAX_WARNED_FEATURE_DESTINATIONS + && WARNED_FEATURE_DESTINATIONS.add(destination)) { + log.warn("Ignoring unknown feature destination {}", destination); + } } } + if (evaluationFeatures) { + metadataBuilder.addBoolean(METADATA_HAS_EVALUATION_FEATURES, true); + } } - private static boolean deliversToHook(final Feature feature) { - if (feature.destinations == null) { - return false; + private static String metadataPrefix(final String destination) { + if (FEATURE_DESTINATION_HOOK.equals(destination)) { + return METADATA_FEATURE_PREFIX; } - boolean hook = false; - for (final String destination : feature.destinations) { - if (FEATURE_DESTINATION_HOOK.equals(destination)) { - hook = true; - } else if (!KNOWN_FEATURE_DESTINATIONS.contains(destination) - && WARNED_FEATURE_DESTINATIONS.size() < MAX_WARNED_FEATURE_DESTINATIONS - && WARNED_FEATURE_DESTINATIONS.add(destination)) { - log.warn("Ignoring unknown feature destination {}", destination); - } + if (FEATURE_DESTINATION_EXPOSURE.equals(destination)) { + return METADATA_EXPOSURE_FEATURE_PREFIX; + } + if (FEATURE_DESTINATION_EVALUATION.equals(destination)) { + return METADATA_EVALUATION_FEATURE_PREFIX; + } + return null; + } + + private static void addFeature( + final ImmutableMetadata.ImmutableMetadataBuilder metadataBuilder, + final String key, + final Object value) { + if (value instanceof String) { + metadataBuilder.addString(key, (String) value); + } else if (value instanceof Boolean) { + metadataBuilder.addBoolean(key, (Boolean) value); + } else if (value instanceof Number) { + metadataBuilder.addDouble(key, ((Number) value).doubleValue()); } - return hook; } /** The serial id an exposure carries: none when the agent's exposure event predates it. */ @@ -784,7 +834,8 @@ static ExposureEvent exposureEvent( final String allocationKey, final String variantKey, final EvaluationContext context, - final Integer serialId) { + final Integer serialId, + final Map features) { final long timestamp = System.currentTimeMillis(); // Exposure types share names with the imported UFC Allocation, Flag, and Variant types. final datadog.trace.api.featureflag.exposure.Allocation allocation = @@ -795,9 +846,14 @@ static ExposureEvent exposureEvent( new datadog.trace.api.featureflag.exposure.Variant(variantKey); final Subject subject = new Subject(context.getTargetingKey(), flattenContext(context)); - return EXPOSURE_SERIAL_ID_SUPPORTED.get() - ? new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject, serialId) - : new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject); + if (!EXPOSURE_SERIAL_ID_SUPPORTED.get()) { + return new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject); + } + if (features.isEmpty() || !EXPOSURE_FEATURES_SUPPORTED.get()) { + return new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject, serialId); + } + return new ExposureEvent( + timestamp, allocation, exposureFlag, variant, subject, serialId, features); } static AbstractMap flattenContext(final EvaluationContext context) { diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java index 72a45cdc1bb..01f8a479c53 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java @@ -8,7 +8,6 @@ import dev.openfeature.sdk.HookContext; import dev.openfeature.sdk.ImmutableMetadata; import java.util.Collections; -import java.util.HashMap; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; @@ -137,24 +136,8 @@ private Map metadata() { * none. */ public Map getFeatures() { - final ImmutableMetadata metadata = details.getFlagMetadata(); - if (metadata == null) { - return Collections.emptyMap(); - } - Map features = null; - for (final Map.Entry entry : metadata.asUnmodifiableMap().entrySet()) { - if (entry.getKey().startsWith(DDEvaluator.METADATA_FEATURE_PREFIX)) { - if (features == null) { - features = new HashMap<>(); - } - features.put( - entry.getKey().substring(DDEvaluator.METADATA_FEATURE_PREFIX.length()), - entry.getValue()); - } - } - return features == null - ? Collections.emptyMap() - : Collections.unmodifiableMap(features); + return SplitFeatureMetadata.featuresWithPrefix( + details.getFlagMetadata(), DDEvaluator.METADATA_FEATURE_PREFIX); } } } diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java index 73c588ed5b3..67ab6b27cfd 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java @@ -73,7 +73,14 @@ public void finallyAfter( cache.record(flag, context.getTargetingKey(), allocationKey, variantKey, serialId); if (sendToDatadog) { FeatureFlaggingGateway.dispatch( - DDEvaluator.exposureEvent(flag, allocationKey, variantKey, context, serialId)); + DDEvaluator.exposureEvent( + flag, + allocationKey, + variantKey, + context, + serialId, + SplitFeatureMetadata.featuresWithPrefix( + metadata, DDEvaluator.METADATA_EXPOSURE_FEATURE_PREFIX))); } } catch (LinkageError e) { // Never let exposure reporting break flag evaluation. diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/FlagEvalLoggingHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/FlagEvalLoggingHook.java index 322f11ac9e1..41426fd3683 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/FlagEvalLoggingHook.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/FlagEvalLoggingHook.java @@ -9,7 +9,10 @@ import dev.openfeature.sdk.ImmutableMetadata; import java.util.Collections; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * OpenFeature Hook that captures flag evaluation events for EVP flagevaluation emission. @@ -33,6 +36,33 @@ */ class FlagEvalLoggingHook implements Hook { + private static final Logger log = LoggerFactory.getLogger(FlagEvalLoggingHook.class); + + static final AtomicBoolean FEATURES_SUPPORTED = + new AtomicBoolean(featuresSupported(FlagEvalEvent.class)); + + /** + * An older agent's flag-evaluation event has no features; its rows are then sent without them. + */ + static boolean featuresSupported(final Class eventClass) { + try { + eventClass.getConstructor( + String.class, + String.class, + String.class, + String.class, + String.class, + long.class, + boolean.class, + Map.class, + Map.class); + return true; + } catch (final NoSuchMethodException | LinkageError | RuntimeException e) { + log.debug("The installed Datadog Java agent does not send features on flag evaluations", e); + return false; + } + } + /** * Singleton instance: always registered when the provider is created; harmless when writer=null * (killswitch off or not yet started). @@ -98,11 +128,17 @@ public void finallyAfter( final ImmutableMetadata metadata = details.getFlagMetadata(); // allocationKey: "allocationKey" (camelCase) - consistent with FlagEvalMetricsHook.java - final String allocationKey = metadata != null ? metadata.getString("allocationKey") : null; + // The ImmutableMetadata getters build a debug message for every absent key. + final Map values = + metadata == null ? Collections.emptyMap() : metadata.asUnmodifiableMap(); + final Object allocationValue = values.get("allocationKey"); + final String allocationKey = + allocationValue instanceof String ? (String) allocationValue : null; // eval-time: from flag metadata "__dd_eval_timestamp_ms" (Long), fallback to hook-fire time. // ImmutableMetadata.getLong available since sdk 1.4+. - final Long evalTimeObj = metadata != null ? metadata.getLong("__dd_eval_timestamp_ms") : null; + final Object evalTimeValue = values.get("__dd_eval_timestamp_ms"); + final Long evalTimeObj = evalTimeValue instanceof Long ? (Long) evalTimeValue : null; final long evalTimeMs = evalTimeObj != null ? evalTimeObj : System.currentTimeMillis(); // variant: the OpenFeature variant key (same source as the OTel FlagEvalMetricsHook), NOT the @@ -115,10 +151,9 @@ public void finallyAfter( // Consent is read from metadata stamped by DDEvaluator (pinned to its ServerConfiguration). // Missing key = non-DD provider → false, the privacy-preserving default. + final Object consentValue = values.get(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA); final Boolean consentFromMetadata = - metadata != null - ? metadata.getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA) - : null; + consentValue instanceof Boolean ? (Boolean) consentValue : null; final boolean observeFullEvaluationData = consentFromMetadata != null && consentFromMetadata; // Error message: prefer the human-readable message under consent-on; under consent-off the @@ -151,16 +186,34 @@ public void finallyAfter( attrs = Collections.emptyMap(); } + // Features come from the flag configuration, not the evaluation context, so they are sent + // whatever the consent. + final Map features = + Boolean.TRUE.equals(values.get(DDEvaluator.METADATA_HAS_EVALUATION_FEATURES)) + ? SplitFeatureMetadata.featuresWithPrefix( + metadata, DDEvaluator.METADATA_EVALUATION_FEATURE_PREFIX) + : Collections.emptyMap(); w.enqueue( - new FlagEvalEvent( - flagKey, - variant, - allocationKey, - targetingKey, - errorMessage, - evalTimeMs, - observeFullEvaluationData, - attrs)); + features.isEmpty() || !FEATURES_SUPPORTED.get() + ? new FlagEvalEvent( + flagKey, + variant, + allocationKey, + targetingKey, + errorMessage, + evalTimeMs, + observeFullEvaluationData, + attrs) + : new FlagEvalEvent( + flagKey, + variant, + allocationKey, + targetingKey, + errorMessage, + evalTimeMs, + observeFullEvaluationData, + attrs, + features)); } catch (LinkageError e) { // Never let EVP recording break flag evaluation } diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/SplitFeatureMetadata.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/SplitFeatureMetadata.java new file mode 100644 index 00000000000..ac24054871f --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/SplitFeatureMetadata.java @@ -0,0 +1,36 @@ +package datadog.trace.api.openfeature; + +import dev.openfeature.sdk.ImmutableMetadata; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +/** + * Reads the split features that {@link DDEvaluator} copies into flag metadata. It depends only on + * the OpenFeature API, so {@link ExposureHook} keeps working where the Datadog Java agent is + * absent. + */ +final class SplitFeatureMetadata { + + private SplitFeatureMetadata() {} + + /** The features stored in the metadata under {@code prefix}, keyed without the prefix. */ + static Map featuresWithPrefix( + final ImmutableMetadata metadata, final String prefix) { + if (metadata == null) { + return Collections.emptyMap(); + } + Map features = null; + for (final Map.Entry entry : metadata.asUnmodifiableMap().entrySet()) { + if (entry.getKey().startsWith(prefix)) { + if (features == null) { + features = new HashMap<>(); + } + features.put(entry.getKey().substring(prefix.length()), entry.getValue()); + } + } + return features == null + ? Collections.emptyMap() + : Collections.unmodifiableMap(features); + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java index c5ee0b70320..d76b8d68b8f 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java @@ -14,6 +14,8 @@ import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.featureflag.exposure.ExposureEvent; +import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; +import datadog.trace.api.featureflag.flagevaluation.FlagEvaluationWriter; import datadog.trace.api.featureflag.ufc.v1.Allocation; import datadog.trace.api.featureflag.ufc.v1.Feature; import datadog.trace.api.featureflag.ufc.v1.Flag; @@ -38,6 +40,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -212,6 +215,135 @@ void stopsRememberingUnknownDestinationsAtTheCap() throws Exception { } } + @Test + void routesEachFeatureToItsDestinations() throws Exception { + start( + true, + configuration( + Boolean.TRUE, + Arrays.asList( + new Feature("holdout.key", "q4-global", singletonList("HOOK")), + new Feature("bandit.arm", "a", singletonList("EXPOSURE")), + new Feature("experiment.tag", "pricing", singletonList("EVALUATION")), + new Feature("bandit.policy_id", "p2", Arrays.asList("EXPOSURE", "EVALUATION"))))); + + final FlagEvaluationDetails details = evaluate("user-1"); + + final ImmutableMetadata metadata = details.getFlagMetadata(); + assertEquals( + Collections.singletonMap("holdout.key", "q4-global"), + SplitFeatureMetadata.featuresWithPrefix(metadata, DDEvaluator.METADATA_FEATURE_PREFIX)); + final Map exposureFeatures = new HashMap<>(); + exposureFeatures.put("bandit.arm", "a"); + exposureFeatures.put("bandit.policy_id", "p2"); + assertEquals( + exposureFeatures, + SplitFeatureMetadata.featuresWithPrefix( + metadata, DDEvaluator.METADATA_EXPOSURE_FEATURE_PREFIX)); + final Map evaluationFeatures = new HashMap<>(); + evaluationFeatures.put("experiment.tag", "pricing"); + evaluationFeatures.put("bandit.policy_id", "p2"); + assertEquals( + evaluationFeatures, + SplitFeatureMetadata.featuresWithPrefix( + metadata, DDEvaluator.METADATA_EVALUATION_FEATURE_PREFIX)); + assertEquals( + Collections.singletonMap("holdout.key", "q4-global"), evaluations.get(0).getFeatures()); + assertEquals(exposureFeatures, sentToDatadog.get(0).features); + } + + @Test + void aRepeatedKeyKeepsTheFirstFeatureForEveryDestination() throws Exception { + start( + true, + configuration( + Boolean.TRUE, + Arrays.asList( + new Feature("holdout.key", "q4-global", singletonList("HOOK")), + new Feature("holdout.key", "q1-global", Arrays.asList("HOOK", "EXPOSURE"))))); + + final FlagEvaluationDetails details = evaluate("user-1"); + + assertEquals( + Collections.singletonMap("holdout.key", "q4-global"), evaluations.get(0).getFeatures()); + assertTrue( + SplitFeatureMetadata.featuresWithPrefix( + details.getFlagMetadata(), DDEvaluator.METADATA_EXPOSURE_FEATURE_PREFIX) + .isEmpty(), + "the repeated feature's EXPOSURE destination is ignored with it"); + assertNull(sentToDatadog.get(0).features); + } + + @Test + void sendsEvaluationFeaturesForAnAllocationThatDoesNotLogExposures() throws Exception { + final AtomicReference captured = new AtomicReference<>(); + FeatureFlaggingGateway.setFlagEvalWriter(capturingWriter(captured)); + try { + start( + true, + configuration( + Boolean.FALSE, + singletonList(new Feature("bandit.policy_id", "p2", singletonList("EVALUATION"))))); + + evaluate("user-1"); + + assertEquals(Collections.singletonMap("bandit.policy_id", "p2"), captured.get().features); + assertTrue(sentToDatadog.isEmpty(), "the allocation does not log exposures"); + } finally { + FeatureFlaggingGateway.setFlagEvalWriter(null); + } + } + + @Test + void sendsExposuresWithoutFeaturesWhenTheAgentLacksThem() throws Exception { + start( + true, + configuration( + Boolean.TRUE, + singletonList(new Feature("bandit.policy_id", "p2", singletonList("EXPOSURE"))))); + DDEvaluator.EXPOSURE_FEATURES_SUPPORTED.set(false); + try { + evaluate("user-1"); + } finally { + DDEvaluator.EXPOSURE_FEATURES_SUPPORTED.set(true); + } + + assertEquals(1, sentToDatadog.size()); + assertNull(sentToDatadog.get(0).features); + } + + @Test + void featureSupportChecksRejectEventsFromAnOlderAgent() { + assertTrue(DDEvaluator.exposureFeaturesSupported(ExposureEvent.class)); + assertFalse(DDEvaluator.exposureFeaturesSupported(SerialIdExposureEvent.class)); + assertTrue(FlagEvalLoggingHook.featuresSupported(FlagEvalEvent.class)); + assertFalse(FlagEvalLoggingHook.featuresSupported(LegacyFlagEvalEvent.class)); + } + + /** An exposure event from an agent that has serial ids but predates features. */ + public static final class SerialIdExposureEvent { + public SerialIdExposureEvent( + final long timestamp, + final datadog.trace.api.featureflag.exposure.Allocation allocation, + final datadog.trace.api.featureflag.exposure.Flag flag, + final datadog.trace.api.featureflag.exposure.Variant variant, + final datadog.trace.api.featureflag.exposure.Subject subject, + final Integer serialId) {} + } + + /** A flag-evaluation event from an agent that predates features. */ + public static final class LegacyFlagEvalEvent { + public LegacyFlagEvalEvent( + final String flagKey, + final String variant, + final String allocationKey, + final String targetingKey, + final String errorMessage, + final long evalTimeMs, + final boolean observeFullEvaluationData, + final Map attrs) {} + } + @Test void evaluatesWithoutFeaturesWhenTheAgentSplitHasNone() throws Exception { start( @@ -421,4 +553,30 @@ private static ServerConfiguration configuration( "flag", new Flag("flag", true, ValueType.INTEGER, variations, singletonList(allocation))); return new ServerConfiguration("", "", false, null, flags); } + + private static FlagEvaluationWriter capturingWriter(final AtomicReference ref) { + return new FlagEvaluationWriter() { + @Override + public void enqueue(final FlagEvalEvent event) { + ref.set(event); + } + + @Override + public boolean hasCapacityForEnqueue() { + return true; + } + + @Override + public void countPreQueueOverflow() {} + + @Override + public void countContextTruncated(final String reason) {} + + @Override + public void start() {} + + @Override + public void close() {} + }; + } } diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookWithoutAgentTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookWithoutAgentTest.java new file mode 100644 index 00000000000..efd42915b6d --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookWithoutAgentTest.java @@ -0,0 +1,103 @@ +package datadog.trace.api.openfeature; + +import static java.util.Collections.singletonMap; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.ImmutableMetadata; +import dev.openfeature.sdk.MutableContext; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Constructor; +import org.junit.jupiter.api.Test; + +/** + * Loads ExposureHook in a class loader that hides DDEvaluator and the Datadog Java agent classes, + * as in an application without the agent. The normal test classpath always has them, so only a + * loader like this sees ExposureHook reach for them. + */ +class ExposureHookWithoutAgentTest { + + @Test + void exposureHookWorksWithoutTheAgentClasses() throws Exception { + final ClassLoader loader = new AgentlessClassLoader(getClass().getClassLoader()); + Class.forName("datadog.trace.api.openfeature.ExposureHook", true, loader); + final Class evaluationClass = + Class.forName("datadog.trace.api.openfeature.ExposureHook$Evaluation", true, loader); + final Constructor constructor = + evaluationClass.getDeclaredConstructor( + EvaluationContext.class, FlagEvaluationDetails.class); + constructor.setAccessible(true); + final FlagEvaluationDetails details = + FlagEvaluationDetails.builder() + .flagKey("flag") + .value(1) + .variant("on") + .flagMetadata( + ImmutableMetadata.builder() + .addString("allocationKey", "allocation") + .addBoolean("__dd_do_log", true) + .addBoolean("__dd_exposure_cache_hit", false) + .addString("__dd_feature.holdout.key", "q4-global") + .build()) + .build(); + + final Object evaluation = constructor.newInstance(new MutableContext("user-1"), details); + + assertTrue((Boolean) evaluationClass.getMethod("isExposure").invoke(evaluation)); + assertTrue((Boolean) evaluationClass.getMethod("shouldSend").invoke(evaluation)); + assertEquals( + singletonMap("holdout.key", "q4-global"), + evaluationClass.getMethod("getFeatures").invoke(evaluation)); + } + + private static final class AgentlessClassLoader extends ClassLoader { + AgentlessClassLoader(final ClassLoader parent) { + super(parent); + } + + @Override + protected Class loadClass(final String name, final boolean resolve) + throws ClassNotFoundException { + synchronized (getClassLoadingLock(name)) { + if (name.startsWith("datadog.trace.api.featureflag.") + || name.startsWith("datadog.trace.api.openfeature.DDEvaluator")) { + throw new ClassNotFoundException(name); + } + if (name.startsWith("datadog.trace.api.openfeature.ExposureHook") + || name.equals("datadog.trace.api.openfeature.SplitFeatureMetadata")) { + Class loaded = findLoadedClass(name); + if (loaded == null) { + final byte[] bytes = classBytes(name); + loaded = defineClass(name, bytes, 0, bytes.length); + } + if (resolve) { + resolveClass(loaded); + } + return loaded; + } + return super.loadClass(name, resolve); + } + } + + private byte[] classBytes(final String name) throws ClassNotFoundException { + try (InputStream in = getParent().getResourceAsStream(name.replace('.', '/') + ".class")) { + if (in == null) { + throw new ClassNotFoundException(name); + } + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + final byte[] buffer = new byte[8192]; + int read; + while ((read = in.read(buffer)) != -1) { + out.write(buffer, 0, read); + } + return out.toByteArray(); + } catch (final IOException e) { + throw new ClassNotFoundException(name, e); + } + } + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/FlagEvalLoggingHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/FlagEvalLoggingHookTest.java index 3b7910d4712..9fd6c79e350 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/FlagEvalLoggingHookTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/FlagEvalLoggingHookTest.java @@ -144,6 +144,62 @@ void finallyAfterEnqueuesEventWithAllBasicFields() { assertEquals("alloc-1", e.allocationKey); } + @Test + void finallyAfterAddsEvaluationFeaturesWhateverTheConsent() { + final AtomicReference captured = new AtomicReference<>(); + final FlagEvalLoggingHook hook = hookWithWriter(capturingWriter(captured)); + + hook.finallyAfter( + null, + details( + "my-flag", + "on-value", + "on", + Reason.TARGETING_MATCH.name(), + ImmutableMetadata.builder() + .addString("allocationKey", "alloc-1") + .addBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA, false) + .addString( + DDEvaluator.METADATA_EVALUATION_FEATURE_PREFIX + "bandit.policy_id", "p2") + .addBoolean(DDEvaluator.METADATA_HAS_EVALUATION_FEATURES, true) + .addString(DDEvaluator.METADATA_FEATURE_PREFIX + "holdout.key", "q4-global") + .addString(DDEvaluator.METADATA_EXPOSURE_FEATURE_PREFIX + "bandit.arm", "a") + .build()), + Collections.emptyMap()); + + assertEquals( + Collections.singletonMap("bandit.policy_id", "p2"), + captured.get().features, + "only EVALUATION features reach the event, also without consent"); + } + + @Test + void finallyAfterSendsNoFeaturesWhenTheAgentLacksThem() { + final AtomicReference captured = new AtomicReference<>(); + final FlagEvalLoggingHook hook = hookWithWriter(capturingWriter(captured)); + FlagEvalLoggingHook.FEATURES_SUPPORTED.set(false); + try { + hook.finallyAfter( + null, + details( + "my-flag", + "on-value", + "on", + Reason.TARGETING_MATCH.name(), + ImmutableMetadata.builder() + .addString("allocationKey", "alloc-1") + .addString( + DDEvaluator.METADATA_EVALUATION_FEATURE_PREFIX + "bandit.policy_id", "p2") + .addBoolean(DDEvaluator.METADATA_HAS_EVALUATION_FEATURES, true) + .build()), + Collections.emptyMap()); + } finally { + FlagEvalLoggingHook.FEATURES_SUPPORTED.set(true); + } + + assertTrue(captured.get().features.isEmpty()); + } + // ---- variant comes from details.getVariant(), NOT details.getValue() ---- @Test diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/exposure/ExposureEvent.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/exposure/ExposureEvent.java index 4101b767760..a2cc090adf2 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/exposure/ExposureEvent.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/exposure/ExposureEvent.java @@ -1,5 +1,7 @@ package datadog.trace.api.featureflag.exposure; +import java.util.Map; + public class ExposureEvent { // milliseconds since epoch as given by System.currentTimeMillis() public final long timestamp; @@ -10,6 +12,10 @@ public class ExposureEvent { public final Integer serial_id; + // Split features routed to the EXPOSURE destination. Null when there are none, so the field is + // omitted from the exposure payload. + public final Map features; + public ExposureEvent( final long timestamp, final Allocation allocation, @@ -26,11 +32,23 @@ public ExposureEvent( final Variant variant, final Subject subject, final Integer serialId) { + this(timestamp, allocation, flag, variant, subject, serialId, null); + } + + public ExposureEvent( + final long timestamp, + final Allocation allocation, + final Flag flag, + final Variant variant, + final Subject subject, + final Integer serialId, + final Map features) { this.timestamp = timestamp; this.allocation = allocation; this.flag = flag; this.variant = variant; this.subject = subject; this.serial_id = serialId; + this.features = features; } } diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEvent.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEvent.java index f5f44b94d7b..d73285a9aa9 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEvent.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEvent.java @@ -54,6 +54,12 @@ public final class FlagEvalEvent { */ public final boolean observeFullEvaluationData; + /** + * Split features routed to the EVALUATION destination. They come from the flag configuration, not + * the evaluation context, so they are kept whatever the consent. May be empty but never null. + */ + public final Map features; + /** Convenience constructor; consent defaults to the privacy-preserving false. */ public FlagEvalEvent( final String flagKey, @@ -86,6 +92,28 @@ public FlagEvalEvent( final long evalTimeMs, final boolean observeFullEvaluationData, final Map attrs) { + this( + flagKey, + variant, + allocationKey, + targetingKey, + errorMessage, + evalTimeMs, + observeFullEvaluationData, + attrs, + null); + } + + public FlagEvalEvent( + final String flagKey, + final String variant, + final String allocationKey, + final String targetingKey, + final String errorMessage, + final long evalTimeMs, + final boolean observeFullEvaluationData, + final Map attrs, + final Map features) { this.flagKey = flagKey; this.variant = variant; this.allocationKey = allocationKey; @@ -94,5 +122,6 @@ public FlagEvalEvent( this.evalTimeMs = evalTimeMs; this.observeFullEvaluationData = observeFullEvaluationData; this.attrs = attrs != null ? attrs : Collections.emptyMap(); + this.features = features != null ? features : Collections.emptyMap(); } } diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEventTest.java b/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEventTest.java index cc614150178..6470364c660 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEventTest.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEventTest.java @@ -51,4 +51,19 @@ void storesExplicitObserveFullEvaluationData() { assertTrue( new FlagEvalEvent("f", "on", "a", "t", null, 1L, true, attrs).observeFullEvaluationData); } + + @Test + void storesFeaturesAndDefaultsThemToEmpty() { + final Map features = Collections.singletonMap("bandit.policy_id", "p2"); + + assertSame( + features, + new FlagEvalEvent("f", "on", "a", "t", null, 1L, false, Collections.emptyMap(), features) + .features); + assertTrue( + new FlagEvalEvent("f", "on", "a", "t", null, 1L, false, Collections.emptyMap(), null) + .features.isEmpty()); + assertTrue( + new FlagEvalEvent("f", "on", "a", "t", 1L, Collections.emptyMap()).features.isEmpty()); + } } diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationAggregator.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationAggregator.java index 5d0eeaa0e82..531dbe82a2f 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationAggregator.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationAggregator.java @@ -4,6 +4,7 @@ import static datadog.trace.util.HashingUtils.hash; import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Objects; @@ -46,6 +47,9 @@ final class FlagEvaluationAggregator { final Map perFlagCount = new HashMap<>(); final AtomicLong droppedDegradedOverflow = new AtomicLong(0); final AtomicInteger globalFullCount = new AtomicInteger(0); + // One shared key and map for each distinct feature set in the flush window, so buckets of many + // subjects do not each keep their own copy until the flush. + final Map featureSets = new HashMap<>(); void aggregate(final FlagEvalEvent event) { final boolean isDefault = event.variant == null; @@ -55,7 +59,11 @@ void aggregate(final FlagEvalEvent event) { // and spills every subsequent evaluation into the degraded tier. final Map prunedAttrs = observeFullEvaluationData ? event.attrs : null; final String ctxKey = observeFullEvaluationData ? canonicalContextKey(prunedAttrs) : ""; - final FullKey fullKey = buildFullKey(event, ctxKey); + // Features come from the flag configuration, not the evaluation context, so they split buckets + // in both tiers and whatever the consent: rows with different features must not merge. + final FeatureSet featureSet = featureSet(event.features); + final String featuresKey = featureSet.key; + final FullKey fullKey = buildFullKey(event, ctxKey, featuresKey); EvalBucket bucket = fullTier.get(fullKey); if (bucket != null) { @@ -77,13 +85,14 @@ void aggregate(final FlagEvalEvent event) { event.evalTimeMs, isDefault, prunedAttrs, - observeFullEvaluationData)); + observeFullEvaluationData, + featureSet.features)); globalFullCount.incrementAndGet(); perFlagCount.put(event.flagKey, flagCount + 1); return; } - final DegradedKey degradedKey = buildDegradedKey(event); + final DegradedKey degradedKey = buildDegradedKey(event, featuresKey); bucket = degradedTier.get(degradedKey); if (bucket != null) { bucket.merge(event.evalTimeMs, isDefault); @@ -103,7 +112,8 @@ void aggregate(final FlagEvalEvent event) { event.evalTimeMs, isDefault, null, - observeFullEvaluationData)); + observeFullEvaluationData, + featureSet.features)); return; } @@ -141,10 +151,36 @@ Iterable degradedBuckets() { void clear() { fullTier.clear(); degradedTier.clear(); + featureSets.clear(); perFlagCount.clear(); globalFullCount.set(0); } + private FeatureSet featureSet(final Map features) { + if (features == null || features.isEmpty()) { + return FeatureSet.NONE; + } + final String key = canonicalContextKey(features); + FeatureSet featureSet = featureSets.get(key); + if (featureSet == null) { + featureSet = new FeatureSet(key, features); + featureSets.put(key, featureSet); + } + return featureSet; + } + + static final class FeatureSet { + static final FeatureSet NONE = new FeatureSet("", Collections.emptyMap()); + + final String key; + final Map features; + + FeatureSet(final String key, final Map features) { + this.key = key; + this.features = features; + } + } + AggregatedState snapshot() { return new AggregatedState( new HashMap<>(fullTier), new HashMap<>(degradedTier), droppedDegradedOverflow.get()); @@ -154,8 +190,8 @@ void simulateFullTierAtCap() { for (int i = globalFullCount.get(); i < GLOBAL_CAP; i++) { final String key = "synthetic-full-" + i; fullTier.put( - new FullKey(key, "on", "alloc", false, null, null, "", false), - new EvalBucket(key, "on", "alloc", null, null, 1L, false, null, false)); + new FullKey(key, "on", "alloc", false, null, null, "", false, ""), + new EvalBucket(key, "on", "alloc", null, null, 1L, false, null, false, null)); globalFullCount.incrementAndGet(); perFlagCount.merge(key, 1, Integer::sum); } @@ -165,8 +201,8 @@ void simulateDegradedTierAtCap() { for (int i = degradedTier.size(); i < DEGRADED_CAP; i++) { final String key = "synthetic-dg-" + i; degradedTier.put( - new DegradedKey(key, "on", "alloc", false, null), - new EvalBucket(key, "on", "alloc", null, null, 1L, false, null, false)); + new DegradedKey(key, "on", "alloc", false, null, ""), + new EvalBucket(key, "on", "alloc", null, null, 1L, false, null, false, null)); } } @@ -177,7 +213,7 @@ void addDegradedBucketForTest( final String errorMessage, final long evalTimeMs) { degradedTier.put( - new DegradedKey(flagKey, variant, allocationKey, variant == null, errorMessage), + new DegradedKey(flagKey, variant, allocationKey, variant == null, errorMessage, ""), new EvalBucket( flagKey, variant, @@ -187,10 +223,12 @@ void addDegradedBucketForTest( evalTimeMs, variant == null, null, - false)); + false, + null)); } - private static FullKey buildFullKey(final FlagEvalEvent event, final String ctxKey) { + private static FullKey buildFullKey( + final FlagEvalEvent event, final String ctxKey, final String featuresKey) { return new FullKey( event.flagKey, event.variant, @@ -199,16 +237,18 @@ private static FullKey buildFullKey(final FlagEvalEvent event, final String ctxK event.errorMessage, event.targetingKey, ctxKey, - event.observeFullEvaluationData); + event.observeFullEvaluationData, + featuresKey); } - private static DegradedKey buildDegradedKey(final FlagEvalEvent event) { + private static DegradedKey buildDegradedKey(final FlagEvalEvent event, final String featuresKey) { return new DegradedKey( event.flagKey, event.variant, event.allocationKey, event.variant == null, - event.errorMessage); + event.errorMessage, + featuresKey); } static String canonicalContextKey(final Map prunedAttrs) { @@ -271,6 +311,8 @@ static class EvalBucket { String targetingKey; String errorMessage; Map prunedAttrs; + // Identical for every evaluation in the bucket, because the features key is a bucket dimension. + Map features; // Consent to emit raw PII. For full-tier buckets this is uniform (consent is a FullKey // dimension) and the AND-fold on merge is defensive. For degraded-tier buckets consent is NOT // a key dimension — mixed-consent events merge here — so the AND-fold produces false whenever @@ -288,7 +330,8 @@ static class EvalBucket { final long evalTimeMs, final boolean runtimeDefaultUsed, final Map prunedAttrs, - final boolean observeFullEvaluationData) { + final boolean observeFullEvaluationData, + final Map features) { this.flagKey = flagKey; this.variant = variant; this.allocationKey = allocationKey; @@ -300,6 +343,7 @@ static class EvalBucket { this.runtimeDefaultUsed = runtimeDefaultUsed; this.prunedAttrs = prunedAttrs; this.observeFullEvaluationData = observeFullEvaluationData; + this.features = features; } int prunedContextFieldCount() { @@ -332,6 +376,7 @@ static final class FullKey { // serializer branches on this to hash the targeting key and drop the context, so events with // different consent produce different wire rows and belong in different buckets. private final boolean observeFullEvaluationData; + private final String featuresKey; FullKey( final String flagKey, @@ -341,7 +386,8 @@ static final class FullKey { final String errorMessage, final String targetingKey, final String contextKey, - final boolean observeFullEvaluationData) { + final boolean observeFullEvaluationData, + final String featuresKey) { this.flagKey = flagKey; this.variant = variant; this.allocationKey = allocationKey; @@ -350,6 +396,7 @@ static final class FullKey { this.targetingKey = targetingKey; this.contextKey = contextKey; this.observeFullEvaluationData = observeFullEvaluationData; + this.featuresKey = featuresKey; } @Override @@ -368,7 +415,8 @@ public boolean equals(final Object o) { && Objects.equals(allocationKey, fullKey.allocationKey) && Objects.equals(errorMessage, fullKey.errorMessage) && Objects.equals(targetingKey, fullKey.targetingKey) - && Objects.equals(contextKey, fullKey.contextKey); + && Objects.equals(contextKey, fullKey.contextKey) + && Objects.equals(featuresKey, fullKey.featuresKey); } @Override @@ -380,7 +428,8 @@ public int hashCode() { result = addToHash(result, errorMessage); result = addToHash(result, targetingKey); result = addToHash(result, contextKey); - return addToHash(result, observeFullEvaluationData); + result = addToHash(result, observeFullEvaluationData); + return addToHash(result, featuresKey); } } @@ -397,18 +446,21 @@ static final class DegradedKey { private final String allocationKey; private final boolean runtimeDefaultUsed; private final String errorMessage; + private final String featuresKey; DegradedKey( final String flagKey, final String variant, final String allocationKey, final boolean runtimeDefaultUsed, - final String errorMessage) { + final String errorMessage, + final String featuresKey) { this.flagKey = flagKey; this.variant = variant; this.allocationKey = allocationKey; this.runtimeDefaultUsed = runtimeDefaultUsed; this.errorMessage = errorMessage; + this.featuresKey = featuresKey; } @Override @@ -424,7 +476,8 @@ public boolean equals(final Object o) { && Objects.equals(flagKey, that.flagKey) && Objects.equals(variant, that.variant) && Objects.equals(allocationKey, that.allocationKey) - && Objects.equals(errorMessage, that.errorMessage); + && Objects.equals(errorMessage, that.errorMessage) + && Objects.equals(featuresKey, that.featuresKey); } @Override @@ -433,7 +486,8 @@ public int hashCode() { // on this hot bucket-lookup path. int result = hash(flagKey, variant, allocationKey); result = addToHash(result, runtimeDefaultUsed); - return addToHash(result, errorMessage); + result = addToHash(result, errorMessage); + return addToHash(result, featuresKey); } } diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationPayloads.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationPayloads.java index 3a8734fd516..e7e027cc2d2 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationPayloads.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationPayloads.java @@ -169,6 +169,7 @@ static class FlagEvaluationEvent { public final Boolean runtime_default_used; public final EventContext context; public final ErrorObject error; + public final Map features; FlagEvaluationEvent( final long timestamp, @@ -181,7 +182,8 @@ static class FlagEvaluationEvent { final String targetingKey, final boolean runtimeDefaultUsed, final String errorMessage, - final Map evaluationAttrs) { + final Map evaluationAttrs, + final Map features) { this.timestamp = timestamp; this.flag = new FlagKeyObject(flagKey); this.first_evaluation = firstEvalMs; @@ -198,6 +200,7 @@ static class FlagEvaluationEvent { : null; this.error = (errorMessage != null && !errorMessage.isEmpty()) ? new ErrorObject(errorMessage) : null; + this.features = (features != null && !features.isEmpty()) ? features : null; } static FlagEvaluationEvent fromBucket( @@ -217,7 +220,10 @@ static FlagEvaluationEvent fromBucket( resolveTargetingKey(bucket.targetingKey, isFullTier, observeFullEvaluationData), bucket.runtimeDefaultUsed, bucket.errorMessage, - includeRawContext ? bucket.prunedAttrs : null); + includeRawContext ? bucket.prunedAttrs : null, + // Features come from the flag configuration, so both tiers keep them whatever the + // consent. + bucket.features); } private static String resolveTargetingKey( @@ -248,7 +254,8 @@ FlagEvaluationEvent withoutTargetingKeyAndContext() { null, Boolean.TRUE.equals(runtime_default_used), messageOf(error), - null); + null, + features); } } diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagEvpPublisherTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagEvpPublisherTest.java index 711e1b90e29..6f034119cdf 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagEvpPublisherTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagEvpPublisherTest.java @@ -125,4 +125,24 @@ static class TestRequest { this.value = value; } } + + @Test + void serializesExposureFeatures() { + final ExposureEvent event = + new ExposureEvent( + 1234L, + new Allocation("allocation"), + new Flag("flag"), + new Variant("variant"), + new Subject("subject", emptyMap()), + 7, + java.util.Collections.singletonMap("bandit.policy_id", "p2")); + + assertTrue(exposureJsonOf(event).contains("\"features\":{\"bandit.policy_id\":\"p2\"}")); + } + + @Test + void omitsFeaturesKeyWhenAbsent() { + assertFalse(exposureJson(7).contains("features")); + } } diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationAggregatorTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationAggregatorTest.java index 06faeadefea..6fb7692f8bb 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationAggregatorTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationAggregatorTest.java @@ -1,14 +1,18 @@ package com.datadog.featureflag; import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.junit.jupiter.api.Test; @@ -218,7 +222,7 @@ void flagEvalEventDoesNotCarryReason() { void evalBucketTracksBoundsDefaultStateAndNullContextFieldCount() { final FlagEvaluationAggregator.EvalBucket bucket = new FlagEvaluationAggregator.EvalBucket( - "bucket-flag", "on", "alloc1", "user-1", null, 1000L, false, null, false); + "bucket-flag", "on", "alloc1", "user-1", null, 1000L, false, null, false, null); assertEquals(0, bucket.prunedContextFieldCount()); @@ -410,7 +414,8 @@ private static FlagEvaluationAggregator.FullKey fullKey( errorMessage, targetingKey, contextKey, - false); + false, + ""); } private static FlagEvaluationAggregator.DegradedKey degradedKey( @@ -420,6 +425,85 @@ private static FlagEvaluationAggregator.DegradedKey degradedKey( final boolean runtimeDefaultUsed, final String errorMessage) { return new FlagEvaluationAggregator.DegradedKey( - flagKey, variant, allocationKey, runtimeDefaultUsed, errorMessage); + flagKey, variant, allocationKey, runtimeDefaultUsed, errorMessage, ""); + } + + @Test + void featuresSplitBucketsInBothTiersWhateverTheConsent() { + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + final Map p1 = singletonMap("bandit.policy_id", "p1"); + final Map p2 = singletonMap("bandit.policy_id", "p2"); + + aggregator.aggregate(featureEvent("user-1", 1000L, false, p1)); + aggregator.aggregate(featureEvent("user-1", 2000L, false, p2)); + aggregator.aggregate(featureEvent("user-1", 3000L, false, p1)); + + final FlagEvaluationAggregator.AggregatedState full = aggregator.snapshot(); + assertEquals(2, full.fullTier.size()); + for (final FlagEvaluationAggregator.EvalBucket bucket : full.fullTier.values()) { + assertEquals(p1.equals(bucket.features) ? 2 : 1, bucket.count); + } + + aggregator.simulateFullTierAtCap(); + aggregator.aggregate(featureEvent("user-2", 4000L, false, p1)); + aggregator.aggregate(featureEvent("user-3", 5000L, false, p2)); + aggregator.aggregate(featureEvent("user-4", 6000L, false, p1)); + + final FlagEvaluationAggregator.AggregatedState degraded = aggregator.snapshot(); + assertEquals(2, degraded.degradedTier.size()); + for (final FlagEvaluationAggregator.EvalBucket bucket : degraded.degradedTier.values()) { + assertEquals(p1.equals(bucket.features) ? 2 : 1, bucket.count); + } + } + + @Test + void bucketsShareOneCopyOfEachFeatureSetUntilTheWindowIsCleared() { + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + + aggregator.aggregate(featureEvent("user-1", 1000L, false, policy("p1"))); + aggregator.aggregate(featureEvent("user-2", 2000L, false, policy("p1"))); + aggregator.aggregate(featureEvent("user-3", 3000L, false, policy("p2"))); + + final List p1Buckets = new ArrayList<>(); + for (final FlagEvaluationAggregator.EvalBucket bucket : aggregator.fullTier.values()) { + if (policy("p1").equals(bucket.features)) { + p1Buckets.add(bucket); + } + } + assertEquals(2, p1Buckets.size()); + assertSame(p1Buckets.get(0).features, p1Buckets.get(1).features); + assertEquals(2, aggregator.featureSets.size()); + + aggregator.simulateFullTierAtCap(); + aggregator.aggregate(featureEvent("user-4", 4000L, false, policy("p1"))); + for (final FlagEvaluationAggregator.EvalBucket bucket : aggregator.degradedTier.values()) { + assertSame(p1Buckets.get(0).features, bucket.features); + } + + aggregator.clear(); + assertTrue(aggregator.featureSets.isEmpty()); + } + + private static Map policy(final String id) { + final Map features = new HashMap<>(); + features.put("bandit.policy_id", id); + return features; + } + + private static FlagEvalEvent featureEvent( + final String targetingKey, + final long evalTimeMs, + final boolean observeFullEvaluationData, + final Map features) { + return new FlagEvalEvent( + "feature-flag", + "on", + "alloc1", + targetingKey, + null, + evalTimeMs, + observeFullEvaluationData, + emptyMap(), + features); } } diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationPayloadsTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationPayloadsTest.java index d4ca517fffd..75df3ac5269 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationPayloadsTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationPayloadsTest.java @@ -58,7 +58,7 @@ void fullTierPayloadUsesWorkerWireShape() throws Exception { void eventFromFullBucketUsesFlushTimeAndEvaluationBounds() throws Exception { final FlagEvaluationAggregator.EvalBucket bucket = new FlagEvaluationAggregator.EvalBucket( - "ts-flag", "on", "alloc1", "user-1", null, EVAL_MS, false, emptyMap(), true); + "ts-flag", "on", "alloc1", "user-1", null, EVAL_MS, false, emptyMap(), true, null); bucket.merge(EVAL_MS + 10, false); final long flushTimeMs = EVAL_MS + 5_000; @@ -82,7 +82,7 @@ void eventFromFullBucketUsesFlushTimeAndEvaluationBounds() throws Exception { void degradedTierEventOmitsTargetingKeyAndContext() throws Exception { final FlagEvaluationAggregator.EvalBucket bucket = new FlagEvaluationAggregator.EvalBucket( - "dg-flag", "on", "alloc1", null, null, EVAL_MS, false, null, false); + "dg-flag", "on", "alloc1", null, null, EVAL_MS, false, null, false, null); final Map json = firstPayload( @@ -112,7 +112,8 @@ void fullTierWithObserveFullEvaluationDataTrueEmitsRawTargetingKeyAndContext() t EVAL_MS, false, attrs, - true); + true, + null); final Map json = firstPayload( @@ -147,7 +148,8 @@ void fullTierWithObserveFullEvaluationDataFalseHashesTargetingKeyAndOmitsContext EVAL_MS, false, attrs, - false); + false, + null); final FlagEvaluationPayloads.EncodedPayloads payloads = FlagEvaluationPayloads.buildPayloads( @@ -308,6 +310,7 @@ void errorPayloadSerializesErrorObject() throws Exception { null, true, "type mismatch", + null, null)), CONTEXT, 1_000_000)); @@ -356,6 +359,7 @@ void degradingEventWithMissingOptionalFieldsKeepsOptionalObjectsAbsent() { "user-1", true, null, + null, null) .withoutTargetingKeyAndContext(); @@ -374,7 +378,7 @@ void emptyOptionalStringsAreTreatedAsAbsentWhenContextIsPresent() { attrs.put("tier", "gold"); final FlagEvaluationPayloads.FlagEvaluationEvent event = new FlagEvaluationPayloads.FlagEvaluationEvent( - EVAL_MS, "empty-optionals", EVAL_MS, EVAL_MS, 1, "", "", null, false, "", attrs); + EVAL_MS, "empty-optionals", EVAL_MS, EVAL_MS, 1, "", "", null, false, "", attrs, null); assertNull(event.variant); assertNull(event.allocation); @@ -406,7 +410,8 @@ private static FlagEvaluationPayloads.FlagEvaluationEvent event( targetingKey, false, null, - attrs); + attrs, + null); } private static Map firstPayload( @@ -450,4 +455,42 @@ private static Map context() { context.put("service", "test-service"); return context; } + + @Test + void rowsCarryFeaturesInEveryTierAndWithoutConsent() throws Exception { + final Map features = new HashMap<>(); + features.put("bandit.policy_id", "p2"); + features.put("bandit.weight", 0.5); + final FlagEvaluationAggregator.EvalBucket bucket = + new FlagEvaluationAggregator.EvalBucket( + "feature-flag", "on", "alloc1", "user-1", null, EVAL_MS, false, null, false, features); + + final FlagEvaluationPayloads.FlagEvaluationEvent full = + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket(bucket, true, false, EVAL_MS); + final FlagEvaluationPayloads.FlagEvaluationEvent degraded = + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket(bucket, false, false, EVAL_MS); + + assertEquals(features, firstEvent(onlyPayload(full)).get("features")); + assertEquals(features, firstEvent(onlyPayload(degraded)).get("features")); + assertEquals(features, full.withoutTargetingKeyAndContext().features); + } + + @Test + void rowsWithoutFeaturesOmitTheField() throws Exception { + final FlagEvaluationAggregator.EvalBucket bucket = + new FlagEvaluationAggregator.EvalBucket( + "plain-flag", "on", "alloc1", null, null, EVAL_MS, false, null, false, emptyMap()); + + final FlagEvaluationPayloads.FlagEvaluationEvent row = + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket(bucket, false, false, EVAL_MS); + + assertFalse(firstEvent(onlyPayload(row)).containsKey("features")); + } + + private static Map onlyPayload( + final FlagEvaluationPayloads.FlagEvaluationEvent row) throws Exception { + return firstPayload( + FlagEvaluationPayloads.buildPayloads( + java.util.Collections.singletonList(row), CONTEXT, 1_000_000)); + } }