From 81141869cad0ca82c6055b88e1a6f7476a084cee Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Wed, 7 Oct 2026 13:02:11 -0400 Subject: [PATCH 01/12] Send exposures only after a successful evaluation The provider now deduplicates exposures itself and sends them from a finally-stage hook instead of at resolution. Resolution only checks the new provider cache and stamps __dd_exposure_cache_hit in the flag metadata. The hook records the exposure and sends it when the allocation logs exposures, the subject is not already exposed, and the evaluation has no error, so an evaluation that an application hook fails no longer sends an exposure or marks the subject as exposed. The agent's writer keeps its own cache for now, because an older provider still sends every evaluation. --- .../trace/api/openfeature/DDEvaluator.java | 67 +++---- .../ExposureDeduplicationCache.java | 108 ++++++++++++ .../api/openfeature/ExposureLoggingHook.java | 61 +++++++ .../trace/api/openfeature/Provider.java | 7 +- .../api/openfeature/DDEvaluatorTest.java | 47 +++-- .../openfeature/ExposureLoggingHookTest.java | 165 ++++++++++++++++++ .../trace/api/openfeature/ProviderTest.java | 12 +- 7 files changed, 419 insertions(+), 48 deletions(-) create mode 100644 products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java create mode 100644 products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java create mode 100644 products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureLoggingHookTest.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 9bc55fe63e9..8b7f4afe286 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 @@ -141,10 +141,14 @@ static boolean exposureSerialIdSupported(final Class eventClass) { */ static final int MAX_STRUCTURE_PROPERTIES = 256; - // Evaluation-metadata keys consumed by the span-enrichment capture hook (see - // SpanEnrichmentHook). Emitted only when the span-enrichment gate is on. + // Evaluation-metadata keys consumed by the span-enrichment capture hook (see SpanEnrichmentHook) + // and the exposure hook (see ExposureLoggingHook). Emitted when span enrichment is on or the + // allocation logs exposures. static final String METADATA_SPLIT_SERIAL_ID = "__dd_split_serial_id"; static final String METADATA_DO_LOG = "__dd_do_log"; + // 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"; // 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). @@ -596,16 +600,26 @@ private static ProviderEvaluation resolveVariant( .addString("allocationKey", allocation.key) .addLong("__dd_eval_timestamp_ms", evalTimestampMs) .addBoolean(METADATA_OBSERVE_FULL_EVALUATION_DATA, observeFullEvaluationData); - // Surface the UFC split's serial id and the allocation's doLog flag for APM span enrichment — - // only when span enrichment is on, so a provider without enrichment pays nothing extra. - // __dd_split_serial_id is omitted when the split carries no serial id; __dd_do_log is always - // present (when enrichment is on) so the span-enrichment hook can decide whether to record the - // subject. - if (SPAN_ENRICHMENT_ENABLED) { + // Surface the UFC split's serial id and the allocation's doLog flag for APM span enrichment and + // for the exposure hook, which sends the exposure after the evaluation succeeds. An evaluation + // that neither logs exposures nor enriches spans carries neither key. + // __dd_split_serial_id is omitted when the split carries no serial id. + final boolean doLog = allocation.doLog != null && allocation.doLog; + if (SPAN_ENRICHMENT_ENABLED || doLog) { if (SPLIT_SERIAL_ID_SUPPORTED.get() && split.serialId != null) { metadataBuilder.addInteger(METADATA_SPLIT_SERIAL_ID, split.serialId); } - metadataBuilder.addBoolean(METADATA_DO_LOG, allocation.doLog != null && allocation.doLog); + metadataBuilder.addBoolean(METADATA_DO_LOG, doLog); + } + if (doLog) { + metadataBuilder.addBoolean( + METADATA_EXPOSURE_CACHE_HIT, + ExposureDeduplicationCache.INSTANCE.contains( + flag.key, + context.getTargetingKey(), + allocation.key, + variant.key, + exposureSerialId(split.serialId))); } final ProviderEvaluation result = ProviderEvaluation.builder() @@ -619,13 +633,14 @@ private static ProviderEvaluation resolveVariant( .variant(variant.key) .flagMetadata(metadataBuilder.build()) .build(); - final boolean doLog = allocation.doLog != null && allocation.doLog; - if (doLog) { - dispatchExposure(key, result, context, split); - } return result; } + /** The serial id an exposure carries: none when the agent's exposure event predates it. */ + static Integer exposureSerialId(final Integer splitSerialId) { + return USE_LEGACY_EXPOSURE_API.get() || !SPLIT_SERIAL_ID_SUPPORTED.get() ? null : splitSerialId; + } + private static Object resolveAttribute(final String name, final EvaluationContext context) { // Special handling for "id" attribute: if not explicitly provided, use targeting key if ("id".equals(name) && !context.keySet().contains(name)) { @@ -693,16 +708,12 @@ private static Double parseDouble(final Object value) { return Double.parseDouble(String.valueOf(value)); } - private static void dispatchExposure( + static ExposureEvent exposureEvent( final String flag, - final ProviderEvaluation evaluation, + final String allocationKey, + final String variantKey, final EvaluationContext context, - final Split split) { - final String allocationKey = allocationKey(evaluation); - final String variantKey = evaluation.getVariant(); - if (allocationKey == null || variantKey == null) { - return; - } + final Integer serialId) { 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 = @@ -713,17 +724,9 @@ private static void dispatchExposure( new datadog.trace.api.featureflag.exposure.Variant(variantKey); final Subject subject = new Subject(context.getTargetingKey(), flattenContext(context)); - final ExposureEvent event = - USE_LEGACY_EXPOSURE_API.get() - ? new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject) - : new ExposureEvent( - timestamp, allocation, exposureFlag, variant, subject, split.serialId); - FeatureFlaggingGateway.dispatch(event); - } - - private static String allocationKey(final ProviderEvaluation resolution) { - final ImmutableMetadata meta = resolution.getFlagMetadata(); - return meta == null ? null : meta.getString("allocationKey"); + return USE_LEGACY_EXPOSURE_API.get() + ? new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject) + : new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject, serialId); } static AbstractMap flattenContext(final EvaluationContext context) { diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java new file mode 100644 index 00000000000..8f76bef6e54 --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java @@ -0,0 +1,108 @@ +package datadog.trace.api.openfeature; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Objects; + +/** + * Remembers the last exposure sent for each (flag, subject). An exposure is sent again only when + * its allocation, variant or serial id changes. Thread-safe. + */ +final class ExposureDeduplicationCache { + + // Matches the agent's exposure writer, which still deduplicates on its own, so both caches evict + // at the same size. + static final int CAPACITY = 1 << 16; + + static final ExposureDeduplicationCache INSTANCE = new ExposureDeduplicationCache(CAPACITY); + + private final Map entries; + + ExposureDeduplicationCache(final int capacity) { + this.entries = + new LinkedHashMap(16, 0.75f, true) { + @Override + protected boolean removeEldestEntry(final Map.Entry eldest) { + return size() > capacity; + } + }; + } + + synchronized boolean contains( + final String flag, + final String subject, + final String allocation, + final String variant, + final Integer serialId) { + return new Sent(allocation, variant, serialId).equals(entries.get(new Key(flag, subject))); + } + + synchronized void record( + final String flag, + final String subject, + final String allocation, + final String variant, + final Integer serialId) { + entries.put(new Key(flag, subject), new Sent(allocation, variant, serialId)); + } + + synchronized int size() { + return entries.size(); + } + + synchronized void clear() { + entries.clear(); + } + + private static final class Key { + private final String flag; + private final String subject; + + Key(final String flag, final String subject) { + this.flag = flag; + this.subject = subject; + } + + @Override + public boolean equals(final Object o) { + if (!(o instanceof Key)) { + return false; + } + final Key key = (Key) o; + return Objects.equals(flag, key.flag) && Objects.equals(subject, key.subject); + } + + @Override + public int hashCode() { + return Objects.hash(flag, subject); + } + } + + private static final class Sent { + private final String allocation; + private final String variant; + private final Integer serialId; + + Sent(final String allocation, final String variant, final Integer serialId) { + this.allocation = allocation; + this.variant = variant; + this.serialId = serialId; + } + + @Override + public boolean equals(final Object o) { + if (!(o instanceof Sent)) { + return false; + } + final Sent entry = (Sent) o; + return Objects.equals(allocation, entry.allocation) + && Objects.equals(variant, entry.variant) + && Objects.equals(serialId, entry.serialId); + } + + @Override + public int hashCode() { + return Objects.hash(allocation, variant, serialId); + } + } +} 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 new file mode 100644 index 00000000000..40ae5fab325 --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java @@ -0,0 +1,61 @@ +package datadog.trace.api.openfeature; + +import datadog.trace.api.featureflag.FeatureFlaggingGateway; +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.Map; + +/** + * Sends the exposure for a logged allocation to the agent once the evaluation has succeeded. + * + *

It runs at the finally stage because OpenFeature runs provider hooks first: an after hook + * added by the application could still throw and turn the result into the default value. At the + * finally stage the details carry that error, so a failed evaluation sends no exposure and does not + * mark the subject as exposed. + */ +class ExposureLoggingHook implements Hook { + + static final ExposureLoggingHook INSTANCE = + new ExposureLoggingHook<>(ExposureDeduplicationCache.INSTANCE); + + private final ExposureDeduplicationCache cache; + + ExposureLoggingHook(final ExposureDeduplicationCache cache) { + this.cache = cache; + } + + @Override + public void finallyAfter( + final HookContext ctx, + final FlagEvaluationDetails details, + final Map hints) { + try { + if (details == null || details.getErrorCode() != null) { + return; + } + final ImmutableMetadata metadata = details.getFlagMetadata(); + if (metadata == null + || !Boolean.TRUE.equals(metadata.getBoolean(DDEvaluator.METADATA_DO_LOG)) + || !Boolean.FALSE.equals(metadata.getBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT))) { + return; + } + final String flag = details.getFlagKey(); + final String allocationKey = metadata.getString("allocationKey"); + final String variantKey = details.getVariant(); + final EvaluationContext context = ctx == null ? null : ctx.getCtx(); + if (allocationKey == null || variantKey == null || context == null) { + return; + } + final Integer serialId = + DDEvaluator.exposureSerialId(metadata.getInteger(DDEvaluator.METADATA_SPLIT_SERIAL_ID)); + cache.record(flag, context.getTargetingKey(), allocationKey, variantKey, serialId); + 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 17009d9933a..3a5f07fdf53 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 @@ -87,7 +87,7 @@ public Provider(final Options options) { // Precompute the immutable hook list once so getProviderHooks() (called on every evaluation) // allocates nothing, including when the gate is off. - final List hooks = new ArrayList<>(3); + final List hooks = new ArrayList<>(4); if (flagEvalMetricsHook != null) { hooks.add(flagEvalMetricsHook); } @@ -104,6 +104,11 @@ public Provider(final Options options) { if (spanEnrichmentHook != null) { hooks.add(spanEnrichmentHook); } + try { + hooks.add(ExposureLoggingHook.INSTANCE); + } catch (LinkageError e) { + log.warn("Feature flag exposure reporting is unavailable with the installed agent", e); + } this.providerHooks = hooks.isEmpty() ? Collections.emptyList() : Collections.unmodifiableList(hooks); diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java index 940c84998d4..42815e69444 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java @@ -44,6 +44,9 @@ import datadog.trace.api.featureflag.ufc.v1.Variant; import dev.openfeature.sdk.ErrorCode; import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.FlagValueType; +import dev.openfeature.sdk.HookContext; import dev.openfeature.sdk.MutableContext; import dev.openfeature.sdk.ProviderEvaluation; import dev.openfeature.sdk.Value; @@ -485,24 +488,48 @@ public void probeKeepsSplitSupportWhenOnlyTheEventConstructorIsMissing() { } /** - * Evaluates a logging allocation whose split carries the given serial id and returns the single - * dispatched exposure. Span enrichment is off here, as it is by default, so this also pins that - * the serial id does not travel via the enrichment-gated evaluation metadata. + * Evaluates a logging allocation whose split carries the given serial id, then runs the exposure + * hook as OpenFeature would after a successful evaluation, and returns the single dispatched + * exposure. Span enrichment is off here, as it is by default, so this also pins that the serial + * id reaches the hook without span enrichment. */ + @SuppressWarnings({"unchecked", "rawtypes"}) private static ExposureEvent exposureFor(final Integer serialId) { + ExposureDeduplicationCache.INSTANCE.clear(); + final Map variations = new HashMap<>(); + variations.put("on", new Variant("on", 1)); + final Split split = new Split(emptyList(), "on", emptyMap(), serialId); + final Allocation allocation = + new Allocation("alloc-1", null, null, null, singletonList(split), Boolean.TRUE); + final Map flags = new HashMap<>(); + flags.put( + "target", + new Flag("target", true, ValueType.INTEGER, variations, singletonList(allocation))); + final DDEvaluator evaluator = new DDEvaluator(mock(Runnable.class)); + evaluator.accept(new ServerConfiguration("", "", true, null, flags)); + final EvaluationContext ctx = new MutableContext("target").setTargetingKey("user-1"); + final List dispatched = new ArrayList<>(); final FeatureFlaggingGateway.ExposureListener listener = dispatched::add; FeatureFlaggingGateway.addExposureListener(listener); try { - final Map variations = new HashMap<>(); - variations.put("on", new Variant("on", 1)); - final Split split = new Split(emptyList(), "on", emptyMap(), serialId); - final Allocation allocation = - new Allocation("alloc-1", null, null, null, singletonList(split), Boolean.TRUE); - evaluateFlag( - new Flag("target", true, ValueType.INTEGER, variations, singletonList(allocation)), true); + final ProviderEvaluation evaluation = + evaluator.evaluate(Integer.class, "target", 23, ctx); + assertTrue(dispatched.isEmpty(), "resolving a flag must not send an exposure by itself"); + final HookContext hookContext = + HookContext.builder() + .flagKey("target") + .type(FlagValueType.INTEGER) + .defaultValue(23) + .ctx(ctx) + .build(); + ExposureLoggingHook.INSTANCE.finallyAfter( + hookContext, + (FlagEvaluationDetails) FlagEvaluationDetails.from(evaluation, "target"), + emptyMap()); } finally { FeatureFlaggingGateway.removeExposureListener(listener); + ExposureDeduplicationCache.INSTANCE.clear(); } assertEquals(1, dispatched.size()); return dispatched.get(0); 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 new file mode 100644 index 00000000000..ae1a29b0d8d --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureLoggingHookTest.java @@ -0,0 +1,165 @@ +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.assertNull; +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.ErrorCode; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.Hook; +import dev.openfeature.sdk.HookContext; +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.BeforeEach; +import org.junit.jupiter.api.Test; + +class ExposureLoggingHookTest { + + private final List dispatched = new ArrayList<>(); + private final FeatureFlaggingGateway.ExposureListener listener = dispatched::add; + private Client client; + + @BeforeEach + void setUp() throws Exception { + ExposureDeduplicationCache.INSTANCE.clear(); + FeatureFlaggingGateway.dispatch(configuration("on", Boolean.TRUE)); + final OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProviderAndWait( + new Provider( + new Options().initTimeout(10, SECONDS), new DDEvaluator(mock(Runnable.class)))); + client = api.getClient(); + FeatureFlaggingGateway.addExposureListener(listener); + } + + @AfterEach + void tearDown() { + FeatureFlaggingGateway.removeExposureListener(listener); + OpenFeatureAPI.getInstance().clearHooks(); + OpenFeatureAPI.getInstance().shutdown(); + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + ExposureDeduplicationCache.INSTANCE.clear(); + } + + @Test + void sendsOneExposurePerSubject() { + evaluate("user-1"); + evaluate("user-1"); + evaluate("user-2"); + + assertEquals(2, dispatched.size()); + assertEquals("user-1", dispatched.get(0).subject.id); + assertEquals("user-2", dispatched.get(1).subject.id); + assertEquals("flag", dispatched.get(0).flag.key); + assertEquals("allocation", dispatched.get(0).allocation.key); + assertEquals("on", dispatched.get(0).variant.key); + assertEquals(Integer.valueOf(7), dispatched.get(0).serial_id); + } + + @Test + void stampsWhetherTheSubjectWasAlreadyExposed() { + assertEquals( + Boolean.FALSE, + evaluate("user-1").getFlagMetadata().getBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT)); + assertEquals( + Boolean.TRUE, + evaluate("user-1").getFlagMetadata().getBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT)); + } + + @Test + void sendsAgainWhenTheVariantChanges() { + evaluate("user-1"); + FeatureFlaggingGateway.dispatch(configuration("off", Boolean.TRUE)); + evaluate("user-1"); + + assertEquals(2, dispatched.size()); + assertEquals("off", dispatched.get(1).variant.key); + } + + @Test + void aFailedEvaluationSendsNothingAndLeavesTheSubjectUnexposed() { + final Hook failingAfterHook = + new Hook() { + @Override + public void after( + final HookContext ctx, + final FlagEvaluationDetails details, + final Map hints) { + throw new IllegalStateException("application validation failed"); + } + }; + OpenFeatureAPI.getInstance().addHooks(failingAfterHook); + + final FlagEvaluationDetails failed = evaluate("user-1"); + + assertEquals(ErrorCode.GENERAL, failed.getErrorCode()); + assertTrue(dispatched.isEmpty()); + assertEquals(0, ExposureDeduplicationCache.INSTANCE.size()); + + OpenFeatureAPI.getInstance().clearHooks(); + evaluate("user-1"); + + assertEquals(1, dispatched.size()); + } + + @Test + void sendsNothingForAnAllocationThatDoesNotLogExposures() { + FeatureFlaggingGateway.dispatch(configuration("on", Boolean.FALSE)); + + final FlagEvaluationDetails details = evaluate("user-1"); + + assertTrue(dispatched.isEmpty()); + assertNull(details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT)); + } + + @Test + void cacheEvictsTheLeastRecentlyUsedSubject() { + final ExposureDeduplicationCache cache = new ExposureDeduplicationCache(2); + cache.record("flag", "user-1", "allocation", "on", 7); + cache.record("flag", "user-2", "allocation", "on", 7); + assertTrue(cache.contains("flag", "user-1", "allocation", "on", 7)); + cache.record("flag", "user-3", "allocation", "on", 7); + + assertTrue(cache.contains("flag", "user-1", "allocation", "on", 7)); + assertFalse(cache.contains("flag", "user-2", "allocation", "on", 7)); + assertTrue(cache.contains("flag", "user-3", "allocation", "on", 7)); + assertFalse(cache.contains("flag", "user-1", "allocation", "on", 8)); + } + + private FlagEvaluationDetails evaluate(final String targetingKey) { + return client.getIntegerDetails("flag", 0, new MutableContext(targetingKey)); + } + + private static ServerConfiguration configuration(final String variantKey, final Boolean doLog) { + final Map variations = new HashMap<>(); + variations.put("on", new Variant("on", 1)); + variations.put("off", new Variant("off", 0)); + final Split split = new Split(emptyList(), variantKey, emptyMap(), 7); + 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/ProviderTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ProviderTest.java index d7adf645e74..2156eae7fdf 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ProviderTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ProviderTest.java @@ -324,10 +324,11 @@ public void testGetProviderHooksReturnsFlagEvalMetricsHook() { Provider provider = new Provider(new Options().initTimeout(10, MILLISECONDS), mock(Evaluator.class)); List hooks = provider.getProviderHooks(); - // Two hooks: OTel FlagEvalMetricsHook (index 0) + FlagEvalLoggingHook (index 1) - assertThat(hooks.size(), equalTo(2)); + // OTel FlagEvalMetricsHook (index 0), FlagEvalLoggingHook (index 1), ExposureLoggingHook (last) + assertThat(hooks.size(), equalTo(3)); assertThat(hooks.get(0) instanceof FlagEvalMetricsHook, equalTo(true)); assertThat(hooks.get(1) instanceof FlagEvalLoggingHook, equalTo(true)); + assertThat(hooks.get(2) instanceof ExposureLoggingHook, equalTo(true)); } @Test @@ -342,8 +343,9 @@ Hook buildFlagEvalLoggingHook() { List hooks = provider.getProviderHooks(); - assertThat(hooks.size(), equalTo(1)); + assertThat(hooks.size(), equalTo(2)); assertThat(hooks.get(0) instanceof FlagEvalMetricsHook, equalTo(true)); + assertThat(hooks.get(1) instanceof ExposureLoggingHook, equalTo(true)); } @Test @@ -409,8 +411,8 @@ public void testShutdownCleansUpEvaluator() throws Exception { provider.shutdown(); verify(evaluator).shutdown(); - // After shutdown, getProviderHooks still returns a list with both OTel + logging hooks - assertThat(provider.getProviderHooks().size(), equalTo(2)); + // After shutdown, getProviderHooks still returns the OTel, logging and exposure hooks + assertThat(provider.getProviderHooks().size(), equalTo(3)); } private static void assertHasFlagEvalMetricsHook(final Provider provider) { From e43b49a974eda302cb999928ed7db026c5dff7cc Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Wed, 7 Oct 2026 13:16:38 -0400 Subject: [PATCH 02/12] Cover the exposure hook guards and exclude the cache POJOs from coverage Test each condition under which the exposure hook sends nothing, and list the exposure cache's key and value classes as POJOs excluded from coverage verification, as the writer's cache classes are. --- .../feature-flagging-api/build.gradle.kts | 6 ++ .../openfeature/ExposureLoggingHookTest.java | 60 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/products/feature-flagging/feature-flagging-api/build.gradle.kts b/products/feature-flagging/feature-flagging-api/build.gradle.kts index a787e7b3fd7..c7cab0c1efd 100644 --- a/products/feature-flagging/feature-flagging-api/build.gradle.kts +++ b/products/feature-flagging/feature-flagging-api/build.gradle.kts @@ -14,6 +14,12 @@ configure { description = "Implementation of the OpenFeature Provider interface." +extra["excludedClassesCoverage"] = listOf( + // POJOs + "datadog.trace.api.openfeature.ExposureDeduplicationCache.Key", + "datadog.trace.api.openfeature.ExposureDeduplicationCache.Sent" +) + // Set both JAR and Maven artifact name val openFeatureArtifactId = "dd-openfeature" base { 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 ae1a29b0d8d..c9386162fbc 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 @@ -22,8 +22,10 @@ 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; @@ -132,6 +134,33 @@ void sendsNothingForAnAllocationThatDoesNotLogExposures() { assertNull(details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT)); } + @Test + void sendsNothingWithoutTheDetailsAnExposureNeeds() { + final ExposureDeduplicationCache cache = new ExposureDeduplicationCache(10); + final ExposureLoggingHook hook = new ExposureLoggingHook<>(cache); + final HookContext withContext = hookContext(new MutableContext("user-1")); + + hook.finallyAfter(withContext, null, emptyMap()); + hook.finallyAfter(withContext, details("variant", null), emptyMap()); + hook.finallyAfter( + withContext, details("variant", exposureMetadata(false, false, "a")), emptyMap()); + hook.finallyAfter( + withContext, details("variant", exposureMetadata(true, true, "a")), emptyMap()); + hook.finallyAfter( + withContext, details("variant", exposureMetadata(true, false, null)), emptyMap()); + hook.finallyAfter(withContext, details(null, exposureMetadata(true, false, "a")), emptyMap()); + hook.finallyAfter(null, details("variant", exposureMetadata(true, false, "a")), emptyMap()); + + assertTrue(dispatched.isEmpty()); + assertEquals(0, cache.size()); + + hook.finallyAfter( + withContext, details("variant", exposureMetadata(true, false, "a")), emptyMap()); + + assertEquals(1, dispatched.size()); + assertEquals(1, cache.size()); + } + @Test void cacheEvictsTheLeastRecentlyUsedSubject() { final ExposureDeduplicationCache cache = new ExposureDeduplicationCache(2); @@ -146,6 +175,37 @@ void cacheEvictsTheLeastRecentlyUsedSubject() { assertFalse(cache.contains("flag", "user-1", "allocation", "on", 8)); } + private static HookContext hookContext(final MutableContext context) { + return HookContext.builder() + .flagKey("flag") + .type(FlagValueType.INTEGER) + .defaultValue(0) + .ctx(context) + .build(); + } + + private static FlagEvaluationDetails details( + 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 builder.build(); + } + + private static ImmutableMetadata exposureMetadata( + final boolean doLog, final boolean cacheHit, final String allocationKey) { + 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)); } From 268677e1677122b7c8b400c0dd11c3b5fb90ea00 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Thu, 8 Oct 2026 14:44:19 -0400 Subject: [PATCH 03/12] Bound the exposure cache with a static map class SpotBugs reports IA_AMBIGUOUS_INVOCATION_OF_INHERITED_OR_OUTER_METHOD because the anonymous LinkedHashMap's size() call could resolve to the cache's own size(). A static nested map class has no outer instance, so the call is unambiguous. --- .../ExposureDeduplicationCache.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java index 8f76bef6e54..73483045364 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java @@ -19,13 +19,7 @@ final class ExposureDeduplicationCache { private final Map entries; ExposureDeduplicationCache(final int capacity) { - this.entries = - new LinkedHashMap(16, 0.75f, true) { - @Override - protected boolean removeEldestEntry(final Map.Entry eldest) { - return size() > capacity; - } - }; + this.entries = new BoundedLru(capacity); } synchronized boolean contains( @@ -105,4 +99,18 @@ public int hashCode() { return Objects.hash(allocation, variant, serialId); } } + + private static final class BoundedLru extends LinkedHashMap { + private final int capacity; + + BoundedLru(final int capacity) { + super(16, 0.75f, true); + this.capacity = capacity; + } + + @Override + protected boolean removeEldestEntry(final Map.Entry eldest) { + return size() > capacity; + } + } } From 0cf56fae677fee4466caed6ced2f70a3eeaf238b Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Fri, 9 Oct 2026 14:05:16 -0400 Subject: [PATCH 04/12] Explain why the exposure cache value classes skip coverage --- products/feature-flagging/feature-flagging-api/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/products/feature-flagging/feature-flagging-api/build.gradle.kts b/products/feature-flagging/feature-flagging-api/build.gradle.kts index c7cab0c1efd..33bff940c4b 100644 --- a/products/feature-flagging/feature-flagging-api/build.gradle.kts +++ b/products/feature-flagging/feature-flagging-api/build.gradle.kts @@ -15,7 +15,7 @@ configure { description = "Implementation of the OpenFeature Provider interface." extra["excludedClassesCoverage"] = listOf( - // POJOs + // Value classes with only equals/hashCode; the cache tests cover their use. "datadog.trace.api.openfeature.ExposureDeduplicationCache.Key", "datadog.trace.api.openfeature.ExposureDeduplicationCache.Sent" ) From 689a6c40aabb6d5945755ad3d9b61cba2f83c226 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Fri, 9 Oct 2026 15:40:44 -0400 Subject: [PATCH 05/12] Read the split serial id only when the agent's Split has it The exposure cache check read split.serialId before exposureSerialId could apply SPLIT_SERIAL_ID_SUPPORTED. With an older dd-java-agent whose Split has no serialId field, every evaluation of an allocation that logs exposures then threw NoSuchFieldError. The read now sits behind the same guard as the other serial id reads. --- .../main/java/datadog/trace/api/openfeature/DDEvaluator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8b7f4afe286..51db16e3e75 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 @@ -619,7 +619,7 @@ private static ProviderEvaluation resolveVariant( context.getTargetingKey(), allocation.key, variant.key, - exposureSerialId(split.serialId))); + SPLIT_SERIAL_ID_SUPPORTED.get() ? exposureSerialId(split.serialId) : null)); } final ProviderEvaluation result = ProviderEvaluation.builder() From 069c8ef564d6ad520f30e04f9f54e9fb85b87d13 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Tue, 13 Oct 2026 10:25:25 -0400 Subject: [PATCH 06/12] Require OpenFeature SDK 1.23.0 so exposures work inside a MultiProvider Exposures are now sent from a provider hook. OpenFeature SDK 1.20.1 to 1.22.1 does not run the hooks of providers wrapped in a MultiProvider, so a Datadog provider inside one sent no exposures. SDK 1.23.0 runs them. The README states 1.23.0 as the minimum for MultiProvider, and a test covers the MultiProvider path; it fails on 1.20.1. --- .../openfeature/application/build.gradle | 2 +- .../feature-flagging-api/README.md | 4 ++++ .../feature-flagging-api/build.gradle.kts | 2 +- .../feature-flagging-api/gradle.lockfile | 5 +++-- .../openfeature/ExposureLoggingHookTest.java | 18 ++++++++++++++++++ 5 files changed, 27 insertions(+), 4 deletions(-) diff --git a/dd-smoke-tests/openfeature/application/build.gradle b/dd-smoke-tests/openfeature/application/build.gradle index 9f7f9558e9b..4e2537caa7e 100644 --- a/dd-smoke-tests/openfeature/application/build.gradle +++ b/dd-smoke-tests/openfeature/application/build.gradle @@ -33,6 +33,6 @@ if (hasProperty('featureFlaggingApiJar')) { dependencies { // OpenFeature SDK is an API dependency of feature-flagging-api but is not // transitively resolved when the jar is passed as a files() dependency. - implementation 'dev.openfeature:sdk:1.20.1' + implementation 'dev.openfeature:sdk:1.23.0' implementation 'org.springframework.boot:spring-boot-starter-web' } diff --git a/products/feature-flagging/feature-flagging-api/README.md b/products/feature-flagging/feature-flagging-api/README.md index 74bbccca2e2..7af28582732 100644 --- a/products/feature-flagging/feature-flagging-api/README.md +++ b/products/feature-flagging/feature-flagging-api/README.md @@ -16,6 +16,10 @@ Published as `com.datadoghq:dd-openfeature` on Maven Central. The OpenFeature SDK (`dev.openfeature:sdk`) is included as a transitive dependency. +The provider sends exposures from an OpenFeature provider hook. If you wrap it in OpenFeature's +`MultiProvider`, use `dev.openfeature:sdk` 1.23.0 or later: earlier versions do not run the hooks of +the providers inside a `MultiProvider`, so the provider sends no exposures there. + ### Evaluation metrics (optional) To enable evaluation metrics (`feature_flag.evaluations` counter), enable the Datadog Java agent's diff --git a/products/feature-flagging/feature-flagging-api/build.gradle.kts b/products/feature-flagging/feature-flagging-api/build.gradle.kts index 33bff940c4b..621b355b2fc 100644 --- a/products/feature-flagging/feature-flagging-api/build.gradle.kts +++ b/products/feature-flagging/feature-flagging-api/build.gradle.kts @@ -45,7 +45,7 @@ java { } dependencies { - api("dev.openfeature:sdk:1.20.1") + api("dev.openfeature:sdk:1.23.0") compileOnly(project(":products:feature-flagging:feature-flagging-bootstrap")) compileOnly(project(":products:feature-flagging:feature-flagging-config")) diff --git a/products/feature-flagging/feature-flagging-api/gradle.lockfile b/products/feature-flagging/feature-flagging-api/gradle.lockfile index a2fc735b6a4..fa0bfccc495 100644 --- a/products/feature-flagging/feature-flagging-api/gradle.lockfile +++ b/products/feature-flagging/feature-flagging-api/gradle.lockfile @@ -16,7 +16,7 @@ com.squareup.okio:okio:1.17.5=jmhRuntimeClasspath,testCompileClasspath,testRunti com.thoughtworks.qdox:qdox:1.12.1=codenarc commons-io:commons-io:2.21.0=spotbugs de.thetaphi:forbiddenapis:3.11=compileClasspath,jmhCompileClasspath -dev.openfeature:sdk:1.20.1=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath +dev.openfeature:sdk:1.23.0=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath io.leangen.geantyref:geantyref:1.3.16=jmhRuntimeClasspath,testRuntimeClasspath io.opentelemetry:opentelemetry-api:1.47.0=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,testCompileClasspath,testRuntimeClasspath io.opentelemetry:opentelemetry-context:1.47.0=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,testCompileClasspath,testRuntimeClasspath @@ -76,8 +76,9 @@ org.ow2.asm:asm:9.10.1=jacocoAnt,spotbugs org.slf4j:jcl-over-slf4j:1.7.30=jmhRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.slf4j:jul-to-slf4j:1.7.30=jmhRuntimeClasspath,testCompileClasspath,testRuntimeClasspath org.slf4j:log4j-over-slf4j:1.7.30=jmhRuntimeClasspath,testCompileClasspath,testRuntimeClasspath -org.slf4j:slf4j-api:2.0.17=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,spotbugsSlf4j,testCompileClasspath,testRuntimeClasspath +org.slf4j:slf4j-api:2.0.17=spotbugsSlf4j org.slf4j:slf4j-api:2.0.18=spotbugs +org.slf4j:slf4j-api:2.0.19=compileClasspath,jmhCompileClasspath,jmhRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath org.slf4j:slf4j-simple:2.0.17=spotbugsSlf4j org.snakeyaml:snakeyaml-engine:2.9=jmhRuntimeClasspath,testRuntimeClasspath org.spockframework:spock-bom:2.4-groovy-3.0=jmhRuntimeClasspath,testCompileClasspath,testRuntimeClasspath 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 c9386162fbc..6d3e4171af3 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 @@ -28,6 +28,7 @@ import dev.openfeature.sdk.ImmutableMetadata; import dev.openfeature.sdk.MutableContext; import dev.openfeature.sdk.OpenFeatureAPI; +import dev.openfeature.sdk.multiprovider.MultiProvider; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -63,6 +64,23 @@ void tearDown() { ExposureDeduplicationCache.INSTANCE.clear(); } + @Test + void sendsExposuresWhenWrappedInAMultiProvider() throws Exception { + OpenFeatureAPI.getInstance() + .setProviderAndWait( + new MultiProvider( + singletonList( + new Provider( + new Options().initTimeout(10, SECONDS), + new DDEvaluator(mock(Runnable.class)))))); + + evaluate("user-1"); + evaluate("user-1"); + + assertEquals(1, dispatched.size()); + assertEquals("user-1", dispatched.get(0).subject.id); + } + @Test void sendsOneExposurePerSubject() { evaluate("user-1"); From 5d56f18d6c31a1b5b2e1d0951ef1d44d117b5156 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Tue, 13 Oct 2026 10:28:18 -0400 Subject: [PATCH 07/12] Read exposure metadata from its map in ExposureLoggingHook The ImmutableMetadata getters build a debug message for every absent key, whether or not debug logging is on. __dd_do_log is absent from most evaluations by default, so the hook built that string on most of them. It now reads the keys from asUnmodifiableMap(). --- .../api/openfeature/ExposureLoggingHook.java | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) 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 40ae5fab325..7da34ca7c0c 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 @@ -37,20 +37,28 @@ public void finallyAfter( return; } final ImmutableMetadata metadata = details.getFlagMetadata(); - if (metadata == null - || !Boolean.TRUE.equals(metadata.getBoolean(DDEvaluator.METADATA_DO_LOG)) - || !Boolean.FALSE.equals(metadata.getBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT))) { + if (metadata == null) { + return; + } + // The ImmutableMetadata getters build a debug message for every absent key. + final Map values = metadata.asUnmodifiableMap(); + if (!Boolean.TRUE.equals(values.get(DDEvaluator.METADATA_DO_LOG)) + || !Boolean.FALSE.equals(values.get(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT))) { return; } final String flag = details.getFlagKey(); - final String allocationKey = metadata.getString("allocationKey"); + final Object allocationValue = values.get("allocationKey"); + final String allocationKey = + allocationValue instanceof String ? (String) allocationValue : null; final String variantKey = details.getVariant(); final EvaluationContext context = ctx == null ? null : ctx.getCtx(); if (allocationKey == null || variantKey == null || context == null) { return; } + final Object serialValue = values.get(DDEvaluator.METADATA_SPLIT_SERIAL_ID); final Integer serialId = - DDEvaluator.exposureSerialId(metadata.getInteger(DDEvaluator.METADATA_SPLIT_SERIAL_ID)); + 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)); From 92e732d830b12160db5160b15d03c387bc75e50b Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Tue, 13 Oct 2026 10:28:20 -0400 Subject: [PATCH 08/12] Rename USE_LEGACY_EXPOSURE_API to EXPOSURE_SERIAL_ID_SUPPORTED The flag now names the capability it detects, like SPLIT_SERIAL_ID_SUPPORTED next to it, instead of the case to avoid. Its value is inverted, and every reader is inverted with it. --- .../datadog/trace/api/openfeature/DDEvaluator.java | 14 ++++++++------ .../DDEvaluatorSpanEnrichmentForkedTest.java | 4 ++-- .../trace/api/openfeature/DDEvaluatorTest.java | 6 +++--- 3 files changed, 13 insertions(+), 11 deletions(-) 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 51db16e3e75..9e65ed9f938 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 @@ -59,9 +59,9 @@ class DDEvaluator implements Evaluator, FeatureFlaggingGateway.ConfigListener { static final AtomicBoolean SPLIT_SERIAL_ID_SUPPORTED = new AtomicBoolean(splitSerialIdSupported(Split.class)); - static final AtomicBoolean USE_LEGACY_EXPOSURE_API = + static final AtomicBoolean EXPOSURE_SERIAL_ID_SUPPORTED = new AtomicBoolean( - !(SPLIT_SERIAL_ID_SUPPORTED.get() && exposureSerialIdSupported(ExposureEvent.class))); + SPLIT_SERIAL_ID_SUPPORTED.get() && exposureSerialIdSupported(ExposureEvent.class)); static boolean splitSerialIdSupported(final Class splitClass) { try { @@ -638,7 +638,9 @@ private static ProviderEvaluation resolveVariant( /** The serial id an exposure carries: none when the agent's exposure event predates it. */ static Integer exposureSerialId(final Integer splitSerialId) { - return USE_LEGACY_EXPOSURE_API.get() || !SPLIT_SERIAL_ID_SUPPORTED.get() ? null : splitSerialId; + return EXPOSURE_SERIAL_ID_SUPPORTED.get() && SPLIT_SERIAL_ID_SUPPORTED.get() + ? splitSerialId + : null; } private static Object resolveAttribute(final String name, final EvaluationContext context) { @@ -724,9 +726,9 @@ static ExposureEvent exposureEvent( new datadog.trace.api.featureflag.exposure.Variant(variantKey); final Subject subject = new Subject(context.getTargetingKey(), flattenContext(context)); - return USE_LEGACY_EXPOSURE_API.get() - ? new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject) - : new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject, serialId); + return EXPOSURE_SERIAL_ID_SUPPORTED.get() + ? new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject, serialId) + : new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject); } static AbstractMap flattenContext(final EvaluationContext context) { diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorSpanEnrichmentForkedTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorSpanEnrichmentForkedTest.java index 2e5a4c483bf..0fba4c9b5c0 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorSpanEnrichmentForkedTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorSpanEnrichmentForkedTest.java @@ -52,13 +52,13 @@ void enrichmentMetadataCarriesTheSplitSerialId() { */ @Test void enrichmentMetadataSurvivesAnAgentWithoutTheExposureConstructor() { - final boolean previous = DDEvaluator.USE_LEGACY_EXPOSURE_API.getAndSet(true); + final boolean previous = DDEvaluator.EXPOSURE_SERIAL_ID_SUPPORTED.getAndSet(false); try { assertEquals( Integer.valueOf(340132), evaluate(340132).getFlagMetadata().getInteger(DDEvaluator.METADATA_SPLIT_SERIAL_ID)); } finally { - DDEvaluator.USE_LEGACY_EXPOSURE_API.set(previous); + DDEvaluator.EXPOSURE_SERIAL_ID_SUPPORTED.set(previous); } } diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java index 42815e69444..953450807f7 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java @@ -420,11 +420,11 @@ public void exposureOmitsSerialIdWhenTheSplitHasNone() { @Test public void legacyExposureApiDispatchesAnExposureWithoutSerialId() { - final boolean previous = DDEvaluator.USE_LEGACY_EXPOSURE_API.getAndSet(true); + final boolean previous = DDEvaluator.EXPOSURE_SERIAL_ID_SUPPORTED.getAndSet(false); try { assertNull(exposureFor(7).serial_id); } finally { - DDEvaluator.USE_LEGACY_EXPOSURE_API.set(previous); + DDEvaluator.EXPOSURE_SERIAL_ID_SUPPORTED.set(previous); } } @@ -458,7 +458,7 @@ public void probeAcceptsTheBootstrapOnTheClasspath() { assertTrue(DDEvaluator.splitSerialIdSupported(Split.class)); assertTrue(DDEvaluator.exposureSerialIdSupported(ExposureEvent.class)); assertTrue(DDEvaluator.SPLIT_SERIAL_ID_SUPPORTED.get()); - assertFalse(DDEvaluator.USE_LEGACY_EXPOSURE_API.get()); + assertTrue(DDEvaluator.EXPOSURE_SERIAL_ID_SUPPORTED.get()); } @Test From 43ea0cdad036c1a4bc0755bb1938dcc0d98391d8 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Tue, 13 Oct 2026 10:28:21 -0400 Subject: [PATCH 09/12] Correct how the provider and writer exposure caches relate They have the same size but refresh entries at different times, so once full they can evict different subjects. The comment said they evict the same ones. --- .../trace/api/openfeature/ExposureDeduplicationCache.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java index 73483045364..88772bbc46f 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java @@ -10,8 +10,9 @@ */ final class ExposureDeduplicationCache { - // Matches the agent's exposure writer, which still deduplicates on its own, so both caches evict - // at the same size. + // The same size as the agent's exposure writer cache, which still deduplicates on its own. The + // two + // caches refresh entries at different times, so once full they can evict different subjects. static final int CAPACITY = 1 << 16; static final ExposureDeduplicationCache INSTANCE = new ExposureDeduplicationCache(CAPACITY); From 5b3fbd7bff17ee81e0a622138cb36576664b212f Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Wed, 14 Oct 2026 09:44:28 -0400 Subject: [PATCH 10/12] Test deduplication with an agent that predates serial-id exposures On dd-java-agent 1.65 and 1.66 the lookup at resolution uses no serial id, so the hook must record none either. Otherwise every repeat of a split with a serial id is sent again. --- .../api/openfeature/ExposureLoggingHookTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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..392417cc670 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 @@ -81,6 +81,20 @@ void sendsExposuresWhenWrappedInAMultiProvider() throws Exception { assertEquals("user-1", dispatched.get(0).subject.id); } + @Test + void deduplicatesWithAnAgentThatPredatesSerialIdExposures() { + final boolean previous = DDEvaluator.EXPOSURE_SERIAL_ID_SUPPORTED.getAndSet(false); + try { + evaluate("user-1"); + evaluate("user-1"); + } finally { + DDEvaluator.EXPOSURE_SERIAL_ID_SUPPORTED.set(previous); + } + + assertEquals(1, dispatched.size(), "a repeat is deduplicated without serial ids"); + assertNull(dispatched.get(0).serial_id); + } + @Test void sendsOneExposurePerSubject() { evaluate("user-1"); From 77e0b25d25930b9a35131ea6db9ce3e007dc8a14 Mon Sep 17 00:00:00 2001 From: Danyal Khan Date: Wed, 14 Oct 2026 09:44:30 -0400 Subject: [PATCH 11/12] Rewrap the exposure cache size comment google-java-format had split it, leaving a line that held only one word. --- .../trace/api/openfeature/ExposureDeduplicationCache.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java index 88772bbc46f..c7d7a9623c8 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureDeduplicationCache.java @@ -11,8 +11,8 @@ final class ExposureDeduplicationCache { // The same size as the agent's exposure writer cache, which still deduplicates on its own. The - // two - // caches refresh entries at different times, so once full they can evict different subjects. + // two caches refresh entries at different times, so once full they can evict different + // subjects. static final int CAPACITY = 1 << 16; static final ExposureDeduplicationCache INSTANCE = new ExposureDeduplicationCache(CAPACITY); From 761bda45e872fb8dcd83ff4a21ccef28419bc980 Mon Sep 17 00:00:00 2001 From: Danyal Khan <50880807+danyal002@users.noreply.github.com> Date: Wed, 14 Oct 2026 10:01:18 -0400 Subject: [PATCH 12/12] Let applications record feature flag exposures with ExposureHook (#12775) * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. --- metadata/supported-configurations.json | 8 + .../feature-flagging-api/README.md | 97 +++ .../trace/api/openfeature/DDEvaluator.java | 133 +++- .../DatadogExposureLoggingGate.java | 32 + .../trace/api/openfeature/ExposureHook.java | 143 +++++ .../api/openfeature/ExposureLoggingHook.java | 31 +- .../api/openfeature/FlagEvalLoggingHook.java | 81 ++- .../trace/api/openfeature/Provider.java | 6 +- .../api/openfeature/SplitFeatureMetadata.java | 36 ++ ...adogExposureLoggingDisabledForkedTest.java | 86 +++ .../api/openfeature/ExposureHookTest.java | 582 ++++++++++++++++++ .../ExposureHookWithoutAgentTest.java | 103 ++++ .../openfeature/ExposureLoggingHookTest.java | 2 +- .../openfeature/FlagEvalLoggingHookTest.java | 56 ++ .../build.gradle.kts | 1 + .../featureflag/exposure/ExposureEvent.java | 18 + .../flagevaluation/FlagEvalEvent.java | 29 + .../trace/api/featureflag/ufc/v1/Feature.java | 19 + .../trace/api/featureflag/ufc/v1/Split.java | 13 + .../flagevaluation/FlagEvalEventTest.java | 15 + .../config/FeatureFlaggingConfig.java | 7 + .../featureflag/FlagEvaluationAggregator.java | 96 ++- .../featureflag/FlagEvaluationPayloads.java | 13 +- .../UniversalFlagConfigParser.java | 118 ++++ .../FeatureFlagEvpPublisherTest.java | 20 + .../FlagEvaluationAggregatorTest.java | 90 ++- .../FlagEvaluationPayloadsTest.java | 55 +- .../RemoteConfigServiceImplTest.java | 137 +++++ .../remote-config/split-features.json | 35 ++ 29 files changed, 2005 insertions(+), 57 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/main/java/datadog/trace/api/openfeature/SplitFeatureMetadata.java create mode 100644 products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DatadogExposureLoggingDisabledForkedTest.java create mode 100644 products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java create mode 100644 products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookWithoutAgentTest.java 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/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..7afc0cd0359 100644 --- a/products/feature-flagging/feature-flagging-api/README.md +++ b/products/feature-flagging/feature-flagging-api/README.md @@ -86,6 +86,103 @@ 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. 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; + +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 a successful evaluation of an allocation that logs exposures, for a + 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. +- An exception from the callback does not affect the evaluation. The first one is logged at warn + level, later ones at debug level. + +### Features + +`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 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 + +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 | 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 `__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.`) | 1.68.0 | `getFeatures()` is empty | + +### 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`. + +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/DDEvaluator.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java index 9e65ed9f938..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 @@ -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; @@ -41,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; @@ -63,6 +65,40 @@ 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)); + + 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 { + 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 +185,23 @@ 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 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"; + 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; + 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). @@ -611,6 +664,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 +692,69 @@ private static ProviderEvaluation resolveVariant( return result; } + private static void addSplitFeatures( + final ImmutableMetadata.ImmutableMetadataBuilder metadataBuilder, final Split split) { + 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 || feature.destinations == null) { + continue; + } + 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 String metadataPrefix(final String destination) { + if (FEATURE_DESTINATION_HOOK.equals(destination)) { + return METADATA_FEATURE_PREFIX; + } + 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()); + } + } + /** 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() @@ -715,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 = @@ -726,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/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..01f8a479c53 --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java @@ -0,0 +1,143 @@ +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.Map; +import java.util.concurrent.atomic.AtomicBoolean; +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. + * + *

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 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 does not affect the evaluation. The + * first one is logged at warn level, later ones at debug level. + */ +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"); + } + + @Override + 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) { + 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); + } + } + } + + /** 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 Map values = metadata(); + return details.getErrorCode() == null + && details.getVariant() != null + && values.get("allocationKey") instanceof String + && Boolean.TRUE.equals(values.get(DDEvaluator.METADATA_DO_LOG)); + } + + /** + * 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 isCacheHit() { + return isExposure() && Boolean.TRUE.equals(cacheHitStamp()); + } + + /** + * 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()); + } + + 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(); + } + + /** + * 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() { + 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 7da34ca7c0c..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 @@ -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,13 +21,21 @@ */ 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); + 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,10 +71,22 @@ 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, + SplitFeatureMetadata.featuresWithPrefix( + metadata, DDEvaluator.METADATA_EXPOSURE_FEATURE_PREFIX))); + } } 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/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/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/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/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); + } +} 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..d76b8d68b8f --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java @@ -0,0 +1,582 @@ +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.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; +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.Collections; +import java.util.HashMap; +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; + +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()); + assertFalse(evaluation.isCacheHit()); + 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()); + assertFalse(evaluation.isCacheHit()); + 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", 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")); + 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 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 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 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( + 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)); + + 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()); + 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()); + } + + @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 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)); + 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); + } + + 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/ExposureLoggingHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureLoggingHookTest.java index 392417cc670..5879bfa8689 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 @@ -169,7 +169,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-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/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-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/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..495b547462f --- /dev/null +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java @@ -0,0 +1,19 @@ +package datadog.trace.api.featureflag.ufc.v1; + +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, final List destinations) { + this.key = key; + this.value = value; + this.destinations = destinations; + } +} 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-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-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 = 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/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java index 9778d980179..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 @@ -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,121 @@ 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; + List destinations = Collections.emptyList(); + 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 if ("destinations".equals(name)) { + destinations = readDestinations(reader); + } else { + reader.skipValue(); + } + } + reader.endObject(); + if (key == null || key.isEmpty() || value == null || destinations.isEmpty()) { + return null; + } + 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 + 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/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)); + } } 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..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 @@ -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; @@ -30,6 +31,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; @@ -37,7 +39,9 @@ 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; import okio.Buffer; import org.junit.jupiter.api.AfterEach; @@ -154,6 +158,110 @@ 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\", \"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", "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)); + } + + @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 String hook = ", \"destinations\": [\"HOOK\"]"; + final ServerConfiguration config = + deserialize( + configWithFeatures( + "[{\"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\"," + + " \"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", "HOOK"); + } + + @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, final String... destinations) { + assertEquals(key, feature.key); + assertEquals(value, feature.value); + assertEquals(Arrays.asList(destinations), feature.destinations); + } + + 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; } @@ -261,6 +369,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")); 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 + } + ] + } + } +}