From 34aa3937707ba0d8e94132ebbb62e9660fabc678 Mon Sep 17 00:00:00 2001 From: Vickie Boettcher Date: Wed, 22 Jul 2026 17:36:57 -0400 Subject: [PATCH 1/5] Parse observeFullEvaluationData and hash targeting_key in flagevaluations events Adds the top-level observeFullEvaluationData boolean to the UFC model, plumbs it through to the EVP flagevaluation event serializer, and gates PII handling on it: when the flag is absent/false the targeting key is SHA-256 hashed (sha256_) and the raw evaluation context is omitted from the wire; when true the raw targeting key and context are emitted. Environment: Datadog workspace Co-Authored-By: Claude Opus 4.8 --- .../api/openfeature/DDEvaluatorTest.java | 2 +- .../featureflag/FeatureFlaggingGateway.java | 10 +++ .../ufc/v1/ServerConfiguration.java | 3 + .../FeatureFlaggingGatewayTest.java | 22 ++++++ .../featureflag/FlagEvaluationPayloads.java | 19 ++++- .../featureflag/FlagEvaluationWriterImpl.java | 8 ++- .../FlagEvaluationPayloadsTest.java | 67 +++++++++++++++++- .../FlagEvaluationWriterImplTest.java | 70 +++++++++++++++++++ .../JsonApiUfcResponseParserTest.java | 41 +++++++++++ .../featureflag/ULeb128EncoderTest.java | 39 +++++++++++ 10 files changed, 274 insertions(+), 7 deletions(-) create mode 100644 products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/ULeb128EncoderTest.java 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 da02bf72706..d663191a456 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 @@ -181,7 +181,7 @@ public void testNoAllocations() { flags.put("null-allocation", new Flag("target", true, null, null, null)); flags.put("empty-allocation", new Flag("target", true, null, null, emptyList())); final DDEvaluator evaluator = new DDEvaluator(mock(Runnable.class)); - evaluator.accept(new ServerConfiguration("", "", null, flags)); + evaluator.accept(new ServerConfiguration("", "", false, null, flags)); final EvaluationContext ctx = new MutableContext("target").setTargetingKey("allocation"); diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java index 0cca3d03de0..5cbe151c164 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java @@ -99,6 +99,16 @@ public static boolean isFlagEvaluationEnqueueEnabled() { return flagEvalEnqueueEnabled; } + /** + * Returns whether the currently active UFC environment has {@code observeFullEvaluationData} + * enabled. {@code false} (privacy-preserving default) when no UFC has been dispatched yet or when + * the field was absent/false on the last dispatched configuration. + */ + public static boolean isObserveFullEvaluationDataEnabled() { + final ServerConfiguration current = CURRENT_CONFIG.get(); + return current != null && current.observeFullEvaluationData; + } + public static void addSpanEnrichmentListener(final SpanEnrichmentListener listener) { SPAN_ENRICHMENT_LISTENERS.add(listener); } diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/ServerConfiguration.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/ServerConfiguration.java index 221fc74079a..8128eca7b32 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/ServerConfiguration.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/ServerConfiguration.java @@ -5,16 +5,19 @@ public class ServerConfiguration { public final String createdAt; public final String format; + public final boolean observeFullEvaluationData; public final Environment environment; public final Map flags; public ServerConfiguration( final String createdAt, final String format, + final boolean observeFullEvaluationData, final Environment environment, final Map flags) { this.createdAt = createdAt; this.format = format; + this.observeFullEvaluationData = observeFullEvaluationData; this.environment = environment; this.flags = flags; } diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/FeatureFlaggingGatewayTest.java b/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/FeatureFlaggingGatewayTest.java index e1a3291f5da..998dd40b3a0 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/FeatureFlaggingGatewayTest.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/FeatureFlaggingGatewayTest.java @@ -1,11 +1,14 @@ package datadog.trace.api.featureflag; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import datadog.trace.api.featureflag.exposure.ExposureEvent; import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; +import java.util.Collections; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -96,6 +99,25 @@ void testAttachingASpanEnrichmentListener() { verifyNoMoreInteractions(spanEnrichmentListener); } + @Test + void isObserveFullEvaluationDataEnabledDefaultsToFalseWithNoConfig() { + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + assertFalse(FeatureFlaggingGateway.isObserveFullEvaluationDataEnabled()); + } + + @Test + void isObserveFullEvaluationDataEnabledReflectsLastDispatchedConfig() { + final ServerConfiguration enabled = + new ServerConfiguration("", "", true, null, Collections.emptyMap()); + FeatureFlaggingGateway.dispatch(enabled); + assertTrue(FeatureFlaggingGateway.isObserveFullEvaluationDataEnabled()); + + final ServerConfiguration disabled = + new ServerConfiguration("", "", false, null, Collections.emptyMap()); + FeatureFlaggingGateway.dispatch(disabled); + assertFalse(FeatureFlaggingGateway.isObserveFullEvaluationDataEnabled()); + } + private static void clearCurrentServerConfiguration() { FeatureFlaggingGateway.dispatch((ServerConfiguration) null); } 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 c2b3cbfb517..142cef2a6a7 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 @@ -194,7 +194,9 @@ static class FlagEvaluationEvent { static FlagEvaluationEvent fromBucket( final FlagEvaluationAggregator.EvalBucket bucket, final boolean isFullTier, + final boolean observeFullEvaluationData, final long flushTimeMs) { + final boolean includeRawContext = isFullTier && observeFullEvaluationData; return new FlagEvaluationEvent( flushTimeMs, bucket.flagKey, @@ -203,10 +205,23 @@ static FlagEvaluationEvent fromBucket( bucket.count, bucket.variant, bucket.allocationKey, - isFullTier ? bucket.targetingKey : null, + resolveTargetingKey(bucket.targetingKey, isFullTier, observeFullEvaluationData), bucket.runtimeDefaultUsed, bucket.errorMessage, - isFullTier ? bucket.prunedAttrs : null); + includeRawContext ? bucket.prunedAttrs : null); + } + + private static String resolveTargetingKey( + final String rawTargetingKey, + final boolean isFullTier, + final boolean observeFullEvaluationData) { + if (!isFullTier || rawTargetingKey == null) { + return null; + } + if (observeFullEvaluationData) { + return rawTargetingKey; + } + return "sha256_" + ULeb128Encoder.hashTargetingKey(rawTargetingKey); } FlagEvaluationEvent withoutTargetingKeyAndContext() { diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java index 685d85fec79..5777ed3e08e 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java @@ -471,15 +471,19 @@ void flush() { private List buildEventList() { final long flushTimeMs = System.currentTimeMillis(); + final boolean observeFullEvaluationData = + FeatureFlaggingGateway.isObserveFullEvaluationDataEnabled(); final List events = new ArrayList<>(aggregator.bucketCount()); for (final FlagEvaluationAggregator.EvalBucket bucket : aggregator.fullBuckets()) { events.add( - FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket(bucket, true, flushTimeMs)); + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket( + bucket, true, observeFullEvaluationData, flushTimeMs)); } for (final FlagEvaluationAggregator.EvalBucket bucket : aggregator.degradedBuckets()) { events.add( - FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket(bucket, false, flushTimeMs)); + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket( + bucket, false, observeFullEvaluationData, flushTimeMs)); } return events; } 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 e3c2d120b00..65a8fa6e791 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 @@ -67,7 +67,7 @@ void eventFromFullBucketUsesFlushTimeAndEvaluationBounds() throws Exception { FlagEvaluationPayloads.buildPayloads( java.util.Collections.singletonList( FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket( - bucket, true, flushTimeMs)), + bucket, true, true, flushTimeMs)), CONTEXT, 1_000_000)); @@ -88,7 +88,8 @@ void degradedTierEventOmitsTargetingKeyAndContext() throws Exception { firstPayload( FlagEvaluationPayloads.buildPayloads( java.util.Collections.singletonList( - FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket(bucket, false, EVAL_MS)), + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket( + bucket, false, true, EVAL_MS)), CONTEXT, 1_000_000)); @@ -97,6 +98,68 @@ void degradedTierEventOmitsTargetingKeyAndContext() throws Exception { assertNull(ev.get("context")); } + @Test + void fullTierWithObserveFullEvaluationDataTrueEmitsRawTargetingKeyAndContext() throws Exception { + final Map attrs = new HashMap<>(); + attrs.put("region", "us-east-1"); + final FlagEvaluationAggregator.EvalBucket bucket = + new FlagEvaluationAggregator.EvalBucket( + "pii-flag", "on", "alloc1", "jane.doe@datadoghq.com", null, EVAL_MS, false, attrs); + + final Map json = + firstPayload( + FlagEvaluationPayloads.buildPayloads( + java.util.Collections.singletonList( + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket( + bucket, true, true, EVAL_MS)), + CONTEXT, + 1_000_000)); + + final Map ev = firstEvent(json); + assertEquals("jane.doe@datadoghq.com", ev.get("targeting_key")); + final Map ctx = (Map) ev.get("context"); + assertNotNull(ctx); + final Map evalAttrs = (Map) ctx.get("evaluation"); + assertNotNull(evalAttrs); + assertEquals("us-east-1", evalAttrs.get("region")); + } + + @Test + void fullTierWithObserveFullEvaluationDataFalseHashesTargetingKeyAndOmitsContext() + throws Exception { + final Map attrs = new HashMap<>(); + attrs.put("region", "us-east-1"); + final FlagEvaluationAggregator.EvalBucket bucket = + new FlagEvaluationAggregator.EvalBucket( + "pii-flag", "on", "alloc1", "jane.doe@datadoghq.com", null, EVAL_MS, false, attrs); + + final FlagEvaluationPayloads.EncodedPayloads payloads = + FlagEvaluationPayloads.buildPayloads( + java.util.Collections.singletonList( + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket( + bucket, true, false, EVAL_MS)), + CONTEXT, + 1_000_000); + final String rawJson = + new String(payloads.bodies.get(0), java.nio.charset.StandardCharsets.UTF_8); + + // The raw wire bytes must carry the hashed key and must not leak the raw PII value or the + // per-event evaluation context — these are the exact properties system-tests asserts over the + // wire. (The batch envelope has its own top-level "context" field, so we guard on the nested + // "evaluation" key instead, which only appears inside a per-event context object.) + assertTrue( + rawJson.contains( + "sha256_b4698f9b6d186781fa8dc59e533578fa2d8379a46b1cf6db85cda6aa9c99e51b")); + assertFalse(rawJson.contains("jane.doe@datadoghq.com")); + assertFalse(rawJson.contains("\"evaluation\":")); + + final Map ev = firstEvent(parse(payloads.bodies.get(0))); + assertEquals( + "sha256_b4698f9b6d186781fa8dc59e533578fa2d8379a46b1cf6db85cda6aa9c99e51b", + ev.get("targeting_key")); + assertFalse(ev.containsKey("context")); + } + @Test void splitPayloadsByEncodedSize() throws Exception { final Map attrs = new HashMap<>(); diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java index 280b3028803..350edf2e83b 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java @@ -6,12 +6,14 @@ import static com.datadog.featureflag.FlagEvaluationTestSupport.clearCoreMetrics; import static com.datadog.featureflag.FlagEvaluationTestSupport.event; import static com.datadog.featureflag.FlagEvaluationTestSupport.eventForFlag; +import static com.datadog.featureflag.FlagEvaluationTestSupport.flushAndCapture; import static com.datadog.featureflag.FlagEvaluationTestSupport.flushAndCaptureJson; import static com.datadog.featureflag.FlagEvaluationTestSupport.metricSum; import static com.datadog.featureflag.FlagEvaluationTestSupport.repeat; import static com.datadog.featureflag.FlagEvaluationTestSupport.simpleEvent; import static java.util.Collections.emptyMap; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -32,6 +34,7 @@ import datadog.communication.ddagent.SharedCommunicationObjects; import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; +import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; import datadog.trace.api.intake.Intake; import datadog.trace.api.telemetry.CoreMetricCollector; import datadog.trace.api.telemetry.MetricCollector; @@ -62,6 +65,8 @@ void clearCoreMetricsAfter() { clearCoreMetrics(); FeatureFlaggingGateway.setFlagEvalWriter(null); FeatureFlaggingGateway.setFlagEvaluationEnqueueEnabled(true); + // Reset the dispatched UFC state so observeFullEvaluationData can't leak into other tests. + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); } @Test @@ -533,6 +538,71 @@ void splitPostFailureDoesNotRetryAlreadySentPayloads() throws Exception { assertEquals(2, posts.get()); } + private static final String HASHED_JANE_DOE = + "sha256_b4698f9b6d186781fa8dc59e533578fa2d8379a46b1cf6db85cda6aa9c99e51b"; + + @Test + void observeFullEvaluationDataTrueEmitsRawTargetingKeyAndContext() throws Exception { + dispatchObserveFullEvaluationData(true); + final BackendApi mockEvp = mock(BackendApi.class); + final FlagEvaluationTestSupport.TestWriterSetup setup = buildTestWriter(mockEvp); + setup.handler.add(piiEvent()); + + final Map json = flushAndCapture(setup).parsed; + + final Map ev = eventForFlag(json, "pii-flag"); + assertNotNull(ev); + assertEquals("jane.doe@datadoghq.com", ev.get("targeting_key")); + final Map ctx = (Map) ev.get("context"); + assertNotNull(ctx); + final Map evalAttrs = (Map) ctx.get("evaluation"); + assertNotNull(evalAttrs); + assertEquals("us-east-1", evalAttrs.get("region")); + } + + @Test + void observeFullEvaluationDataFalseHashesTargetingKeyAndOmitsContext() throws Exception { + dispatchObserveFullEvaluationData(false); + assertHashedTargetingKeyAndOmittedContext(); + } + + @Test + void observeFullEvaluationDataAbsentDefaultsToHashedBehavior() throws Exception { + // No UFC dispatched (default state) — must behave exactly like the explicit "false" case. + assertHashedTargetingKeyAndOmittedContext(); + } + + private void assertHashedTargetingKeyAndOmittedContext() throws Exception { + final BackendApi mockEvp = mock(BackendApi.class); + final FlagEvaluationTestSupport.TestWriterSetup setup = buildTestWriter(mockEvp); + setup.handler.add(piiEvent()); + + final FlagEvaluationTestSupport.CapturedJson captured = flushAndCapture(setup); + + final Map ev = eventForFlag(captured.parsed, "pii-flag"); + assertNotNull(ev); + assertEquals(HASHED_JANE_DOE, ev.get("targeting_key")); + assertFalse(ev.containsKey("context")); + // The raw wire bytes must carry the hashed key and never leak the raw PII value or a per-event + // evaluation context (the batch envelope owns the top-level "context" key, so guard on the + // nested "evaluation" field instead). + assertTrue(captured.raw.contains(HASHED_JANE_DOE)); + assertFalse(captured.raw.contains("jane.doe@datadoghq.com")); + assertFalse(captured.raw.contains("\"evaluation\":")); + } + + private static FlagEvalEvent piiEvent() { + final Map attrs = new HashMap<>(); + attrs.put("region", "us-east-1"); + return event("pii-flag", "on", "alloc1", "jane.doe@datadoghq.com", 1000L, attrs); + } + + private static void dispatchObserveFullEvaluationData(final boolean value) { + FeatureFlaggingGateway.dispatch( + new ServerConfiguration( + "2024-04-17T19:40:53.716Z", "SERVER", value, null, java.util.Collections.emptyMap())); + } + private static Object lifecycleLock(final FlagEvaluationWriterImpl writer) throws Exception { final Field field = FlagEvaluationWriterImpl.class.getDeclaredField("lifecycleLock"); field.setAccessible(true); diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/JsonApiUfcResponseParserTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/JsonApiUfcResponseParserTest.java index a31d47889ba..239c9c434eb 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/JsonApiUfcResponseParserTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/JsonApiUfcResponseParserTest.java @@ -2,6 +2,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -73,10 +74,50 @@ void rejectsTrailingJson() { + "}}{}")); } + @Test + void observeFullEvaluationDataDefaultsToFalseWhenAbsent() throws Exception { + final ServerConfiguration configuration = parse(wrap(emptyConfig())); + assertNotNull(configuration); + assertFalse(configuration.observeFullEvaluationData); + } + + @Test + void observeFullEvaluationDataParsesTrue() throws Exception { + final ServerConfiguration configuration = + parse(wrap(configWithObserveFullEvaluationData(true))); + assertNotNull(configuration); + assertTrue(configuration.observeFullEvaluationData); + } + + @Test + void observeFullEvaluationDataParsesFalse() throws Exception { + final ServerConfiguration configuration = + parse(wrap(configWithObserveFullEvaluationData(false))); + assertNotNull(configuration); + assertFalse(configuration.observeFullEvaluationData); + } + private static ServerConfiguration parse(final String json) throws Exception { return JsonApiUfcResponseParser.INSTANCE.parse(json.getBytes(UTF_8)); } + private static String wrap(final String attributes) { + return "{\"data\":{\"type\":\"universal-flag-configuration\",\"attributes\":" + + attributes + + "}}"; + } + + private static String configWithObserveFullEvaluationData(final boolean value) { + return "{" + + "\"createdAt\":\"2024-04-17T19:40:53.716Z\"," + + "\"observeFullEvaluationData\":" + + value + + "," + + "\"environment\":{\"name\":\"Test\"}," + + "\"flags\":{}" + + "}"; + } + private static String emptyConfig() { return "{" + "\"createdAt\":\"2024-04-17T19:40:53.716Z\"," diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/ULeb128EncoderTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/ULeb128EncoderTest.java new file mode 100644 index 00000000000..f47fdeaef1c --- /dev/null +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/ULeb128EncoderTest.java @@ -0,0 +1,39 @@ +package com.datadog.featureflag; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class ULeb128EncoderTest { + + @Test + void hashTargetingKeyMatchesCanonicalPiiVector() { + // Canonical vector shared across all SDK implementations — see system-tests PR #7316. + assertEquals( + "b4698f9b6d186781fa8dc59e533578fa2d8379a46b1cf6db85cda6aa9c99e51b", + ULeb128Encoder.hashTargetingKey("jane.doe@datadoghq.com")); + } + + @Test + void hashTargetingKeyPreservesWhitespaceExactly() { + assertNotEquals( + ULeb128Encoder.hashTargetingKey("jane.doe@datadoghq.com"), + ULeb128Encoder.hashTargetingKey(" jane.doe@datadoghq.com ")); + } + + @Test + void hashTargetingKeyPreservesCaseExactly() { + assertNotEquals( + ULeb128Encoder.hashTargetingKey("jane.doe@datadoghq.com"), + ULeb128Encoder.hashTargetingKey("JANE.DOE@DATADOGHQ.COM")); + } + + @Test + void hashTargetingKeyIsLowercase64CharHex() { + final String hash = ULeb128Encoder.hashTargetingKey("some-arbitrary-key"); + assertEquals(64, hash.length()); + assertTrue(hash.matches("[0-9a-f]{64}")); + } +} From d65c79f266332b4ad0611acd20c415ca9d77ebdd Mon Sep 17 00:00:00 2001 From: "vickie.fridge" Date: Thu, 23 Jul 2026 15:58:41 +0000 Subject: [PATCH 2/5] Extract hashed targeting key prefix into a named constant Replace the inline "sha256_" literal with a documented HASHED_TARGETING_KEY_PREFIX constant describing the cross-SDK wire contract for privacy-preserving hashed targeting keys. Environment: Datadog workspace Co-Authored-By: Claude Opus 4.8 --- .../datadog/featureflag/FlagEvaluationPayloads.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 142cef2a6a7..3a8734fd516 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 @@ -13,6 +13,15 @@ final class FlagEvaluationPayloads { private static final byte[] PAYLOAD_SUFFIX = FeatureFlagEvpPublisher.utf8Bytes("]}"); private static final byte[] JSON_COMMA = FeatureFlagEvpPublisher.utf8Bytes(","); + + /** + * Wire prefix identifying a privacy-preserving, hashed targeting key. Emitted for full-tier rows + * when {@code observeFullEvaluationData} is off. The suffix is the lower-case hex SHA-256 of the + * UTF-8 targeting key (see {@link ULeb128Encoder#hashTargetingKey}). This is a cross-SDK wire + * contract - keep it in sync with the other server SDKs and the UFC/EVP spec. + */ + private static final String HASHED_TARGETING_KEY_PREFIX = "sha256_"; + private static final JsonAdapter EVENT_JSON_ADAPTER; private static final JsonAdapter> CONTEXT_JSON_ADAPTER; @@ -221,7 +230,7 @@ private static String resolveTargetingKey( if (observeFullEvaluationData) { return rawTargetingKey; } - return "sha256_" + ULeb128Encoder.hashTargetingKey(rawTargetingKey); + return HASHED_TARGETING_KEY_PREFIX + ULeb128Encoder.hashTargetingKey(rawTargetingKey); } FlagEvaluationEvent withoutTargetingKeyAndContext() { From 4e9a0eadef62303dabdde2835a96a758004468c6 Mon Sep 17 00:00:00 2001 From: "vickie.fridge" Date: Thu, 23 Jul 2026 16:23:30 +0000 Subject: [PATCH 3/5] Test observeFullEvaluationData parsing edge cases Parameterize the true/false config-parsing assertions with @ValueSource and add a test locking in the fail-closed behaviour for an explicit JSON null: malformed config is rejected so full evaluation data is never observed off the back of it. Environment: Datadog workspace Co-Authored-By: Claude Opus 4.8 --- .../JsonApiUfcResponseParserTest.java | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/JsonApiUfcResponseParserTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/JsonApiUfcResponseParserTest.java index 239c9c434eb..3db21f3c8dc 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/JsonApiUfcResponseParserTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/JsonApiUfcResponseParserTest.java @@ -11,6 +11,8 @@ import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; import java.io.IOException; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; class JsonApiUfcResponseParserTest { @@ -81,20 +83,24 @@ void observeFullEvaluationDataDefaultsToFalseWhenAbsent() throws Exception { assertFalse(configuration.observeFullEvaluationData); } - @Test - void observeFullEvaluationDataParsesTrue() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void observeFullEvaluationDataParsesExplicitValue(final boolean value) throws Exception { final ServerConfiguration configuration = - parse(wrap(configWithObserveFullEvaluationData(true))); + parse(wrap(configWithObserveFullEvaluationData(value))); assertNotNull(configuration); - assertTrue(configuration.observeFullEvaluationData); + assertEquals(value, configuration.observeFullEvaluationData); } @Test - void observeFullEvaluationDataParsesFalse() throws Exception { - final ServerConfiguration configuration = - parse(wrap(configWithObserveFullEvaluationData(false))); - assertNotNull(configuration); - assertFalse(configuration.observeFullEvaluationData); + void observeFullEvaluationDataRejectsExplicitNull() { + // An explicit null for this boolean is malformed input. Parsing rejects the whole + // configuration, which is the fail-closed outcome we want: full evaluation data is never + // observed off the back of a malformed config. Callers (AgentlessConfigurationSource and the + // remote-config poller) swallow the failure and keep the last-known-good config, and the + // gateway defaults to the privacy-preserving behaviour when no valid config was dispatched. + // Servers send true/false or omit the field; null is not a value they emit. + assertThrows(Exception.class, () -> parse(wrap(configWithNullObserveFullEvaluationData()))); } private static ServerConfiguration parse(final String json) throws Exception { @@ -118,6 +124,15 @@ private static String configWithObserveFullEvaluationData(final boolean value) { + "}"; } + private static String configWithNullObserveFullEvaluationData() { + return "{" + + "\"createdAt\":\"2024-04-17T19:40:53.716Z\"," + + "\"observeFullEvaluationData\":null," + + "\"environment\":{\"name\":\"Test\"}," + + "\"flags\":{}" + + "}"; + } + private static String emptyConfig() { return "{" + "\"createdAt\":\"2024-04-17T19:40:53.716Z\"," From 81af153ca75559668c7b34f53fd8ec80bee1ed1a Mon Sep 17 00:00:00 2001 From: "vickie.fridge" Date: Thu, 23 Jul 2026 17:59:53 +0000 Subject: [PATCH 4/5] Capture observeFullEvaluationData per bucket at aggregation time The flush-time read of FeatureFlaggingGateway.isObserveFullEvaluationDataEnabled() was a TOCTOU bug: CURRENT_CONFIG could be overwritten by a later RC update between when an evaluation happened and when the batch flushed, so events could be emitted under the wrong environment's consent (the system test observed a targeting key hashed even though the active UFC said observeFullEvaluationData=true). Capture consent when the evaluation is folded into its EvalBucket instead. On merge the value is folded with AND, so any no-consent evaluation in a bucket's lifetime sinks the whole bucket to hashed/omitted (fail-closed). buildEventList now reads bucket.observeFullEvaluationData rather than the gateway. The gateway accessor is retained; it is read at aggregation time. Adds a writer-level regression guard (a bucket aggregated under consent-off stays hashed even if the gateway later reports consent-on) plus aggregator fold tests, and an end-to-end parse->dispatch->flush test. Environment: Datadog workspace Co-Authored-By: Claude Opus 4.8 --- .../featureflag/FlagEvaluationAggregator.java | 31 +++++++-- .../featureflag/FlagEvaluationWriterImpl.java | 9 +-- .../FlagEvaluationAggregatorTest.java | 50 +++++++++++++- .../FlagEvaluationPayloadsTest.java | 24 +++++-- .../FlagEvaluationWriterImplTest.java | 67 +++++++++++++++++++ 5 files changed, 166 insertions(+), 15 deletions(-) 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 a146501f825..86b33a4e10c 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 @@ -1,5 +1,6 @@ package com.datadog.featureflag; +import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; import java.util.HashMap; import java.util.Map; @@ -44,6 +45,11 @@ final class FlagEvaluationAggregator { void aggregate(final FlagEvalEvent event) { final boolean isDefault = event.variant == null; + // Capture consent now, when the evaluation is folded into a bucket, so it reflects the + // configuration active at evaluation time rather than whatever CURRENT_CONFIG happens to be at + // flush. Existing buckets fold with AND: one no-consent evaluation sinks the bucket to hashed. + final boolean observeFullEvaluationData = + FeatureFlaggingGateway.isObserveFullEvaluationDataEnabled(); final Map prunedAttrs = pruneContext(event.contextAttributes()); final String ctxKey = canonicalContextKey(prunedAttrs); final FullKey fullKey = buildFullKey(event, ctxKey); @@ -51,6 +57,7 @@ void aggregate(final FlagEvalEvent event) { EvalBucket bucket = fullTier.get(fullKey); if (bucket != null) { bucket.merge(event.evalTimeMs, isDefault); + bucket.observeFullEvaluationData &= observeFullEvaluationData; return; } @@ -66,7 +73,8 @@ void aggregate(final FlagEvalEvent event) { event.errorMessage, event.evalTimeMs, isDefault, - prunedAttrs)); + prunedAttrs, + observeFullEvaluationData)); globalFullCount.incrementAndGet(); perFlagCount.put(event.flagKey, flagCount + 1); return; @@ -76,6 +84,7 @@ void aggregate(final FlagEvalEvent event) { bucket = degradedTier.get(degradedKey); if (bucket != null) { bucket.merge(event.evalTimeMs, isDefault); + bucket.observeFullEvaluationData &= observeFullEvaluationData; return; } @@ -90,7 +99,8 @@ void aggregate(final FlagEvalEvent event) { event.errorMessage, event.evalTimeMs, isDefault, - null)); + null, + observeFullEvaluationData)); return; } @@ -142,7 +152,7 @@ void simulateFullTierAtCap() { final String key = "synthetic-full-" + i; fullTier.put( new FullKey(key, "on", "alloc", false, null, null, ""), - new EvalBucket(key, "on", "alloc", null, null, 1L, false, null)); + new EvalBucket(key, "on", "alloc", null, null, 1L, false, null, false)); globalFullCount.incrementAndGet(); perFlagCount.merge(key, 1, Integer::sum); } @@ -153,7 +163,7 @@ void simulateDegradedTierAtCap() { 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)); + new EvalBucket(key, "on", "alloc", null, null, 1L, false, null, false)); } } @@ -173,7 +183,8 @@ void addDegradedBucketForTest( errorMessage, evalTimeMs, variant == null, - null)); + null, + false)); } private static FullKey buildFullKey(final FlagEvalEvent event, final String ctxKey) { @@ -272,6 +283,12 @@ static class EvalBucket { String targetingKey; String errorMessage; Map prunedAttrs; + // Consent to emit raw PII (targeting key + context) captured when this bucket was created, not + // read at flush time. CURRENT_CONFIG can be overwritten by a later RC update between capture + // and flush, so reading the gateway at flush would apply the wrong environment's consent. On + // merge the value is folded with AND (see aggregate()): if any evaluation in the bucket's + // lifetime saw consent off, the whole bucket falls back to hashed/omitted — fail-closed. + boolean observeFullEvaluationData; EvalBucket( final String flagKey, @@ -281,7 +298,8 @@ static class EvalBucket { final String errorMessage, final long evalTimeMs, final boolean runtimeDefaultUsed, - final Map prunedAttrs) { + final Map prunedAttrs, + final boolean observeFullEvaluationData) { this.flagKey = flagKey; this.variant = variant; this.allocationKey = allocationKey; @@ -292,6 +310,7 @@ static class EvalBucket { this.count = 1; this.runtimeDefaultUsed = runtimeDefaultUsed; this.prunedAttrs = prunedAttrs; + this.observeFullEvaluationData = observeFullEvaluationData; } int prunedContextFieldCount() { diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java index 5777ed3e08e..b62c3728cf9 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java @@ -471,19 +471,20 @@ void flush() { private List buildEventList() { final long flushTimeMs = System.currentTimeMillis(); - final boolean observeFullEvaluationData = - FeatureFlaggingGateway.isObserveFullEvaluationDataEnabled(); + // Consent is read per bucket from the value captured at aggregation time, not from the + // gateway here: CURRENT_CONFIG may have been overwritten by a later RC update since these + // evaluations happened, and reading it at flush would apply the wrong environment's consent. final List events = new ArrayList<>(aggregator.bucketCount()); for (final FlagEvaluationAggregator.EvalBucket bucket : aggregator.fullBuckets()) { events.add( FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket( - bucket, true, observeFullEvaluationData, flushTimeMs)); + bucket, true, bucket.observeFullEvaluationData, flushTimeMs)); } for (final FlagEvaluationAggregator.EvalBucket bucket : aggregator.degradedBuckets()) { events.add( FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket( - bucket, false, observeFullEvaluationData, flushTimeMs)); + bucket, false, bucket.observeFullEvaluationData, flushTimeMs)); } return events; } 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 57bc7251c1e..d324ca3012f 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 @@ -6,7 +6,9 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; +import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -212,7 +214,7 @@ void flagEvalEventDoesNotCarryReason() { void evalBucketTracksBoundsDefaultStateAndNullContextFieldCount() { final FlagEvaluationAggregator.EvalBucket bucket = new FlagEvaluationAggregator.EvalBucket( - "bucket-flag", "on", "alloc1", "user-1", null, 1000L, false, null); + "bucket-flag", "on", "alloc1", "user-1", null, 1000L, false, null, false); assertEquals(0, bucket.prunedContextFieldCount()); @@ -266,6 +268,52 @@ void degradedKeyEqualityUsesEveryDimension() { assertNotEquals(base, degradedKey("flag", "on", "alloc", false, "other")); } + @Test + void observeFullEvaluationDataFoldsToFalseWhenAnyMergedEvaluationLacksConsent() { + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + try { + FeatureFlaggingGateway.dispatch(observeConfig(true)); + aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 1000L, emptyMap())); + // A later RC update turns consent off; the second evaluation folds into the same bucket. + FeatureFlaggingGateway.dispatch(observeConfig(false)); + aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 2000L, emptyMap())); + + final FlagEvaluationAggregator.EvalBucket bucket = + aggregator.snapshot().fullTier.values().iterator().next(); + assertEquals(2, bucket.count); + // Conservative fold: one no-consent evaluation sinks the whole bucket to hashed/omitted. + assertFalse(bucket.observeFullEvaluationData); + } finally { + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + } + } + + @Test + void observeFullEvaluationDataStaysTrueWhenEveryMergedEvaluationConsents() { + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + try { + FeatureFlaggingGateway.dispatch(observeConfig(true)); + aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 1000L, emptyMap())); + aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 2000L, emptyMap())); + + final FlagEvaluationAggregator.EvalBucket bucket = + aggregator.snapshot().fullTier.values().iterator().next(); + assertEquals(2, bucket.count); + assertTrue(bucket.observeFullEvaluationData); + } finally { + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + } + } + + private static ServerConfiguration observeConfig(final boolean observeFullEvaluationData) { + return new ServerConfiguration( + "2024-04-17T19:40:53.716Z", + "SERVER", + observeFullEvaluationData, + null, + java.util.Collections.emptyMap()); + } + private static FlagEvalEvent event( final String flagKey, final String variant, 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 65a8fa6e791..d4ca517fffd 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()); + "ts-flag", "on", "alloc1", "user-1", null, EVAL_MS, false, emptyMap(), true); 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); + "dg-flag", "on", "alloc1", null, null, EVAL_MS, false, null, false); final Map json = firstPayload( @@ -104,7 +104,15 @@ void fullTierWithObserveFullEvaluationDataTrueEmitsRawTargetingKeyAndContext() t attrs.put("region", "us-east-1"); final FlagEvaluationAggregator.EvalBucket bucket = new FlagEvaluationAggregator.EvalBucket( - "pii-flag", "on", "alloc1", "jane.doe@datadoghq.com", null, EVAL_MS, false, attrs); + "pii-flag", + "on", + "alloc1", + "jane.doe@datadoghq.com", + null, + EVAL_MS, + false, + attrs, + true); final Map json = firstPayload( @@ -131,7 +139,15 @@ void fullTierWithObserveFullEvaluationDataFalseHashesTargetingKeyAndOmitsContext attrs.put("region", "us-east-1"); final FlagEvaluationAggregator.EvalBucket bucket = new FlagEvaluationAggregator.EvalBucket( - "pii-flag", "on", "alloc1", "jane.doe@datadoghq.com", null, EVAL_MS, false, attrs); + "pii-flag", + "on", + "alloc1", + "jane.doe@datadoghq.com", + null, + EVAL_MS, + false, + attrs, + false); final FlagEvaluationPayloads.EncodedPayloads payloads = FlagEvaluationPayloads.buildPayloads( diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java index 350edf2e83b..7d00ec2b3d1 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java @@ -560,6 +560,38 @@ void observeFullEvaluationDataTrueEmitsRawTargetingKeyAndContext() throws Except assertEquals("us-east-1", evalAttrs.get("region")); } + @Test + void observeFullEvaluationDataTrueFromParsedUfcEmitsRawTargetingKey() throws Exception { + // Exercises the config-source -> gateway -> writer seam that the hand-built dispatch skips: + // parse a UFC exactly as the (default) agentless source does, dispatch it, then flush. Guards + // against observeFullEvaluationData being lost between config parsing and the flush-time gate + // read. Field placement mirrors the system-test fixture (after "flags", with format SERVER). + final String attributes = + "{" + + "\"createdAt\":\"2024-04-17T19:40:53.716Z\"," + + "\"format\":\"SERVER\"," + + "\"environment\":{\"name\":\"Test\"}," + + "\"flags\":{}," + + "\"observeFullEvaluationData\":true" + + "}"; + final String wrapped = + "{\"data\":{\"type\":\"universal-flag-configuration\",\"attributes\":" + attributes + "}}"; + final ServerConfiguration parsed = + JsonApiUfcResponseParser.INSTANCE.parse( + wrapped.getBytes(java.nio.charset.StandardCharsets.UTF_8)); + assertNotNull(parsed); + assertTrue(parsed.observeFullEvaluationData); + FeatureFlaggingGateway.dispatch(parsed); + + final BackendApi mockEvp = mock(BackendApi.class); + final FlagEvaluationTestSupport.TestWriterSetup setup = buildTestWriter(mockEvp); + setup.handler.add(piiEvent()); + + final Map ev = eventForFlag(flushAndCapture(setup).parsed, "pii-flag"); + assertNotNull(ev); + assertEquals("jane.doe@datadoghq.com", ev.get("targeting_key")); + } + @Test void observeFullEvaluationDataFalseHashesTargetingKeyAndOmitsContext() throws Exception { dispatchObserveFullEvaluationData(false); @@ -572,6 +604,41 @@ void observeFullEvaluationDataAbsentDefaultsToHashedBehavior() throws Exception assertHashedTargetingKeyAndOmittedContext(); } + @Test + void bucketCapturedUnderFalseStaysHashedEvenIfGatewayLaterReportsTrue() throws Exception { + // Regression guard for the flush-time TOCTOU bug: consent is captured when the evaluation is + // aggregated, not read from the gateway at flush. A bucket aggregated while consent was off + // must stay hashed even if a later RC update turns consent on before the flush drains. + dispatchObserveFullEvaluationData(false); + final BackendApi mockEvp = mock(BackendApi.class); + final FlagEvaluationTestSupport.TestWriterSetup setup = buildTestWriter(mockEvp); + setup.handler.add(piiEvent()); + setup.handler.drainAndAggregate(); // captures consent=false into the bucket + + // Simulate a later RC update flipping consent on; the already-aggregated bucket must not + // follow. + dispatchObserveFullEvaluationData(true); + + final java.util.List captured = new java.util.ArrayList<>(); + when(mockEvp.post(eq("flagevaluation"), any(RequestBody.class), any(), any(), eq(false))) + .thenAnswer( + inv -> { + captured.add(inv.getArgument(1)); + return null; + }); + setup.handler.flush(); + + assertEquals(1, captured.size()); + final FlagEvaluationTestSupport.CapturedJson json = + FlagEvaluationTestSupport.readJson(captured.get(0)); + final Map ev = eventForFlag(json.parsed, "pii-flag"); + assertNotNull(ev); + assertEquals(HASHED_JANE_DOE, ev.get("targeting_key")); + assertFalse(ev.containsKey("context")); + assertTrue(json.raw.contains(HASHED_JANE_DOE)); + assertFalse(json.raw.contains("jane.doe@datadoghq.com")); + } + private void assertHashedTargetingKeyAndOmittedContext() throws Exception { final BackendApi mockEvp = mock(BackendApi.class); final FlagEvaluationTestSupport.TestWriterSetup setup = buildTestWriter(mockEvp); From 0d204bd22370f5cacf675f6fae6b0e031a7aa986 Mon Sep 17 00:00:00 2001 From: "vickie.fridge" Date: Fri, 24 Jul 2026 20:06:52 +0000 Subject: [PATCH 5/5] Capture observeFullEvaluationData consent at evaluation time Snapshot the PII consent flag on the evaluation thread (in the OpenFeature hook) and carry it on FlagEvalEvent, instead of reading the gateway when the event is aggregated/flushed. This pins the hashed-vs-raw decision to the configuration active at evaluation time, closing a one-directional leak window where a later Remote Config update could retroactively apply a different environment's consent to already-collected evaluations. Aggregation and flush now read event.observeFullEvaluationData and never consult the gateway; the AND-fold across a bucket's evaluations is unchanged (any no-consent evaluation sinks the bucket to hashed/omitted). Environment: Datadog workspace Co-Authored-By: Claude Opus 4.8 --- .../api/openfeature/FlagEvalLoggingHook.java | 8 ++ .../openfeature/FlagEvalLoggingHookTest.java | 54 ++++++++ .../flagevaluation/FlagEvalEvent.java | 49 ++++++- .../flagevaluation/FlagEvalEventTest.java | 20 +++ .../featureflag/FlagEvaluationAggregator.java | 17 ++- .../featureflag/FlagEvaluationWriterImpl.java | 6 +- .../FlagEvaluationAggregatorTest.java | 70 +++++----- .../FlagEvaluationTestSupport.java | 19 +++ .../FlagEvaluationWriterImplTest.java | 121 ++++++++++-------- 9 files changed, 264 insertions(+), 100 deletions(-) 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 5a3c852a207..e2e5d79e650 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 @@ -109,6 +109,13 @@ public void finallyAfter( ctx != null && ctx.getCtx() != null ? ctx.getCtx().getTargetingKey() : null; final Map attrs = snapshotAttrs(ctx); + // Snapshot the PII consent flag now, on the evaluation thread, so it is pinned to the + // configuration active at evaluation time. The event is drained and flushed later, by which + // point a subsequent RC update may have changed CURRENT_CONFIG; reading consent here (not at + // drain/flush) is what makes the hashed-vs-raw decision faithful to the evaluation. + final boolean observeFullEvaluationData = + FeatureFlaggingGateway.isObserveFullEvaluationDataEnabled(); + w.enqueue( new FlagEvalEvent( flagKey, @@ -117,6 +124,7 @@ public void finallyAfter( targetingKey, errorMessage, evalTimeMs, + observeFullEvaluationData, () -> extractAttrs(attrs))); } catch (LinkageError | Exception e) { // Never let EVP recording break flag evaluation 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 b498faf42bc..fe31ceb4d60 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 @@ -16,6 +16,7 @@ import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; import datadog.trace.api.featureflag.flagevaluation.FlagEvaluationWriter; +import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; import dev.openfeature.sdk.ErrorCode; import dev.openfeature.sdk.FlagEvaluationDetails; import dev.openfeature.sdk.FlagValueType; @@ -49,6 +50,9 @@ void enableFlagEvaluationEnqueue() { @AfterEach void resetFlagEvaluationEnqueue() { FeatureFlaggingGateway.setFlagEvaluationEnqueueEnabled(true); + // Clear any dispatched UFC so an observeFullEvaluationData value can't leak into other tests + // that share the static gateway. + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); } // ---- helpers ---- @@ -457,4 +461,54 @@ void contextAttributesUseEnqueueTimeSnapshot() { assertFalse(attrs.containsKey("profile.late")); assertFalse(attrs.containsKey("cohorts[1]")); } + + // ---- observeFullEvaluationData is snapshotted from the gateway at evaluation time ---- + + @Test + void snapshotsObserveFullEvaluationDataTrueFromGatewayAtEvaluationTime() { + FeatureFlaggingGateway.dispatch(observeConfig(true)); + try { + assertTrue(enqueuedEvent().observeFullEvaluationData); + } finally { + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + } + } + + @Test + void snapshotsObserveFullEvaluationDataFalseFromGatewayAtEvaluationTime() { + FeatureFlaggingGateway.dispatch(observeConfig(false)); + try { + assertFalse(enqueuedEvent().observeFullEvaluationData); + } finally { + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + } + } + + @Test + void observeFullEvaluationDataDefaultsToFalseWhenNoConfigDispatched() { + // No UFC dispatched: the gateway reports the privacy-preserving default and the hook stamps it. + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + assertFalse(enqueuedEvent().observeFullEvaluationData); + } + + /** Fires the hook once for a simple targeted evaluation and returns the enqueued event. */ + private FlagEvalEvent enqueuedEvent() { + final AtomicReference captured = new AtomicReference<>(); + final FlagEvalLoggingHook hook = hookWithWriter(capturingWriter(captured)); + hook.finallyAfter( + hookCtxWithTargetingKey("obs-flag", "user-1"), + details("obs-flag", "on", "on", Reason.TARGETING_MATCH.name(), null), + Collections.emptyMap()); + assertNotNull(captured.get(), "writer.enqueue must be called once"); + return captured.get(); + } + + private static ServerConfiguration observeConfig(final boolean observeFullEvaluationData) { + return new ServerConfiguration( + "2024-04-17T19:40:53.716Z", + "SERVER", + observeFullEvaluationData, + null, + Collections.emptyMap()); + } } 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 83397e5dccd..716b5bd6e3b 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 @@ -50,8 +50,19 @@ public final class FlagEvalEvent { */ public final Map attrs; + /** + * Whether the UFC environment active at evaluation time had {@code + * observeFullEvaluationData} enabled. Snapshotted on the evaluation thread (from {@code + * FeatureFlaggingGateway.isObserveFullEvaluationDataEnabled()}) so the consent decision is pinned + * to the instant the flag was evaluated, not to whatever configuration happens to be active when + * the event is later drained and flushed. {@code false} is the privacy-preserving default: when + * off, the targeting key is hashed and the per-evaluation context is omitted on emission. + */ + public final boolean observeFullEvaluationData; + private final Supplier> attrsSupplier; + /** Convenience constructor; consent defaults to the privacy-preserving {@code false}. */ public FlagEvalEvent( final String flagKey, final String variant, @@ -59,9 +70,10 @@ public FlagEvalEvent( final String targetingKey, final long evalTimeMs, final Map attrs) { - this(flagKey, variant, allocationKey, targetingKey, null, evalTimeMs, attrs); + this(flagKey, variant, allocationKey, targetingKey, null, evalTimeMs, false, attrs); } + /** Convenience constructor; consent defaults to the privacy-preserving {@code false}. */ public FlagEvalEvent( final String flagKey, final String variant, @@ -70,16 +82,49 @@ public FlagEvalEvent( final String errorMessage, final long evalTimeMs, final Map attrs) { + this(flagKey, variant, allocationKey, targetingKey, errorMessage, evalTimeMs, false, attrs); + } + + 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) { this.flagKey = flagKey; this.variant = variant; this.allocationKey = allocationKey; this.targetingKey = targetingKey; this.errorMessage = errorMessage; this.evalTimeMs = evalTimeMs; + this.observeFullEvaluationData = observeFullEvaluationData; this.attrs = attrs != null ? attrs : Collections.emptyMap(); this.attrsSupplier = null; } + /** Convenience constructor; consent defaults to the privacy-preserving {@code false}. */ + public FlagEvalEvent( + final String flagKey, + final String variant, + final String allocationKey, + final String targetingKey, + final String errorMessage, + final long evalTimeMs, + final Supplier> attrsSupplier) { + this( + flagKey, + variant, + allocationKey, + targetingKey, + errorMessage, + evalTimeMs, + false, + attrsSupplier); + } + public FlagEvalEvent( final String flagKey, final String variant, @@ -87,6 +132,7 @@ public FlagEvalEvent( final String targetingKey, final String errorMessage, final long evalTimeMs, + final boolean observeFullEvaluationData, final Supplier> attrsSupplier) { this.flagKey = flagKey; this.variant = variant; @@ -94,6 +140,7 @@ public FlagEvalEvent( this.targetingKey = targetingKey; this.errorMessage = errorMessage; this.evalTimeMs = evalTimeMs; + this.observeFullEvaluationData = observeFullEvaluationData; this.attrs = Collections.emptyMap(); this.attrsSupplier = attrsSupplier; } 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 7faa19c72b3..78b8344502a 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 @@ -1,6 +1,7 @@ package datadog.trace.api.featureflag.flagevaluation; 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.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -70,4 +71,23 @@ void defaultsNullLazyContextAttributes() { assertTrue(event.contextAttributes().isEmpty()); } + + @Test + void observeFullEvaluationDataDefaultsToFalseOnConvenienceConstructors() { + final Map attrs = Collections.emptyMap(); + assertFalse(new FlagEvalEvent("f", "on", "a", "t", 1L, attrs).observeFullEvaluationData); + assertFalse(new FlagEvalEvent("f", "on", "a", "t", null, 1L, attrs).observeFullEvaluationData); + assertFalse( + new FlagEvalEvent("f", "on", "a", "t", null, 1L, () -> attrs).observeFullEvaluationData); + } + + @Test + void storesExplicitObserveFullEvaluationData() { + final Map attrs = Collections.emptyMap(); + assertTrue( + new FlagEvalEvent("f", "on", "a", "t", null, 1L, true, attrs).observeFullEvaluationData); + assertTrue( + new FlagEvalEvent("f", "on", "a", "t", null, 1L, true, () -> attrs) + .observeFullEvaluationData); + } } 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 86b33a4e10c..41e7d10b58f 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 @@ -1,6 +1,5 @@ package com.datadog.featureflag; -import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; import java.util.HashMap; import java.util.Map; @@ -45,11 +44,12 @@ final class FlagEvaluationAggregator { void aggregate(final FlagEvalEvent event) { final boolean isDefault = event.variant == null; - // Capture consent now, when the evaluation is folded into a bucket, so it reflects the - // configuration active at evaluation time rather than whatever CURRENT_CONFIG happens to be at - // flush. Existing buckets fold with AND: one no-consent evaluation sinks the bucket to hashed. - final boolean observeFullEvaluationData = - FeatureFlaggingGateway.isObserveFullEvaluationDataEnabled(); + // Consent is read from the event, where it was snapshotted on the evaluation thread at + // evaluation time. We deliberately do NOT read the gateway here: aggregation runs later, on the + // serializer thread, by which point a subsequent RC update may have changed CURRENT_CONFIG, and + // that must not retroactively alter the hashed-vs-raw decision for an already-evaluated flag. + // Existing buckets fold with AND: one no-consent evaluation sinks the bucket to hashed. + final boolean observeFullEvaluationData = event.observeFullEvaluationData; final Map prunedAttrs = pruneContext(event.contextAttributes()); final String ctxKey = canonicalContextKey(prunedAttrs); final FullKey fullKey = buildFullKey(event, ctxKey); @@ -283,9 +283,8 @@ static class EvalBucket { String targetingKey; String errorMessage; Map prunedAttrs; - // Consent to emit raw PII (targeting key + context) captured when this bucket was created, not - // read at flush time. CURRENT_CONFIG can be overwritten by a later RC update between capture - // and flush, so reading the gateway at flush would apply the wrong environment's consent. On + // Consent to emit raw PII (targeting key + context), sourced from each event's evaluation-time + // snapshot (FlagEvalEvent.observeFullEvaluationData), never read from the gateway at flush. On // merge the value is folded with AND (see aggregate()): if any evaluation in the bucket's // lifetime saw consent off, the whole bucket falls back to hashed/omitted — fail-closed. boolean observeFullEvaluationData; diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java index b62c3728cf9..eba631ee955 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationWriterImpl.java @@ -471,9 +471,9 @@ void flush() { private List buildEventList() { final long flushTimeMs = System.currentTimeMillis(); - // Consent is read per bucket from the value captured at aggregation time, not from the - // gateway here: CURRENT_CONFIG may have been overwritten by a later RC update since these - // evaluations happened, and reading it at flush would apply the wrong environment's consent. + // Consent is read per bucket from the value each event snapshotted at evaluation time, not + // from the gateway here: CURRENT_CONFIG may have been overwritten by a later RC update since + // these evaluations happened, and reading it at flush would apply the wrong config's consent. final List events = new ArrayList<>(aggregator.bucketCount()); for (final FlagEvaluationAggregator.EvalBucket bucket : aggregator.fullBuckets()) { 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 d324ca3012f..2f218eb1e15 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 @@ -6,9 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; -import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; -import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -271,47 +269,49 @@ void degradedKeyEqualityUsesEveryDimension() { @Test void observeFullEvaluationDataFoldsToFalseWhenAnyMergedEvaluationLacksConsent() { final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); - try { - FeatureFlaggingGateway.dispatch(observeConfig(true)); - aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 1000L, emptyMap())); - // A later RC update turns consent off; the second evaluation folds into the same bucket. - FeatureFlaggingGateway.dispatch(observeConfig(false)); - aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 2000L, emptyMap())); - - final FlagEvaluationAggregator.EvalBucket bucket = - aggregator.snapshot().fullTier.values().iterator().next(); - assertEquals(2, bucket.count); - // Conservative fold: one no-consent evaluation sinks the whole bucket to hashed/omitted. - assertFalse(bucket.observeFullEvaluationData); - } finally { - FeatureFlaggingGateway.dispatch((ServerConfiguration) null); - } + // Consent travels on the event (snapshotted at evaluation time). The first evaluation + // consented; + // a later one - e.g. after an RC update flipped consent off - folds into the same bucket + // carrying consent=false. + aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 1000L, true, emptyMap())); + aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 2000L, false, emptyMap())); + + final FlagEvaluationAggregator.EvalBucket bucket = + aggregator.snapshot().fullTier.values().iterator().next(); + assertEquals(2, bucket.count); + // Conservative fold: one no-consent evaluation sinks the whole bucket to hashed/omitted. + assertFalse(bucket.observeFullEvaluationData); } @Test void observeFullEvaluationDataStaysTrueWhenEveryMergedEvaluationConsents() { final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); - try { - FeatureFlaggingGateway.dispatch(observeConfig(true)); - aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 1000L, emptyMap())); - aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 2000L, emptyMap())); - - final FlagEvaluationAggregator.EvalBucket bucket = - aggregator.snapshot().fullTier.values().iterator().next(); - assertEquals(2, bucket.count); - assertTrue(bucket.observeFullEvaluationData); - } finally { - FeatureFlaggingGateway.dispatch((ServerConfiguration) null); - } + aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 1000L, true, emptyMap())); + aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 2000L, true, emptyMap())); + + final FlagEvaluationAggregator.EvalBucket bucket = + aggregator.snapshot().fullTier.values().iterator().next(); + assertEquals(2, bucket.count); + assertTrue(bucket.observeFullEvaluationData); } - private static ServerConfiguration observeConfig(final boolean observeFullEvaluationData) { - return new ServerConfiguration( - "2024-04-17T19:40:53.716Z", - "SERVER", - observeFullEvaluationData, + private static FlagEvalEvent event( + final String flagKey, + final String variant, + final String allocationKey, + final String targetingKey, + final long evalTimeMs, + final boolean observeFullEvaluationData, + final Map attrs) { + return new FlagEvalEvent( + flagKey, + variant, + allocationKey, + targetingKey, null, - java.util.Collections.emptyMap()); + evalTimeMs, + observeFullEvaluationData, + attrs); } private static FlagEvalEvent event( diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationTestSupport.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationTestSupport.java index 2ec8629dbfc..abb26485c6c 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationTestSupport.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationTestSupport.java @@ -54,6 +54,25 @@ static FlagEvalEvent event( return new FlagEvalEvent(flagKey, variant, allocationKey, targetingKey, evalTimeMs, attrs); } + static FlagEvalEvent event( + final String flagKey, + final String variant, + final String allocationKey, + final String targetingKey, + final long evalTimeMs, + final boolean observeFullEvaluationData, + final Map attrs) { + return new FlagEvalEvent( + flagKey, + variant, + allocationKey, + targetingKey, + null, + evalTimeMs, + observeFullEvaluationData, + attrs); + } + static FlagEvalEvent errorEvent( final String flagKey, final String errorMessage, final long evalTimeMs) { return new FlagEvalEvent( diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java index 7d00ec2b3d1..b0fd6a2ce0c 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationWriterImplTest.java @@ -543,10 +543,11 @@ void splitPostFailureDoesNotRetryAlreadySentPayloads() throws Exception { @Test void observeFullEvaluationDataTrueEmitsRawTargetingKeyAndContext() throws Exception { - dispatchObserveFullEvaluationData(true); + // Consent travels on the event (snapshotted by the hook at evaluation time); the writer honours + // it verbatim and never consults the gateway. final BackendApi mockEvp = mock(BackendApi.class); final FlagEvaluationTestSupport.TestWriterSetup setup = buildTestWriter(mockEvp); - setup.handler.add(piiEvent()); + setup.handler.add(piiEvent(true)); final Map json = flushAndCapture(setup).parsed; @@ -560,64 +561,32 @@ void observeFullEvaluationDataTrueEmitsRawTargetingKeyAndContext() throws Except assertEquals("us-east-1", evalAttrs.get("region")); } - @Test - void observeFullEvaluationDataTrueFromParsedUfcEmitsRawTargetingKey() throws Exception { - // Exercises the config-source -> gateway -> writer seam that the hand-built dispatch skips: - // parse a UFC exactly as the (default) agentless source does, dispatch it, then flush. Guards - // against observeFullEvaluationData being lost between config parsing and the flush-time gate - // read. Field placement mirrors the system-test fixture (after "flags", with format SERVER). - final String attributes = - "{" - + "\"createdAt\":\"2024-04-17T19:40:53.716Z\"," - + "\"format\":\"SERVER\"," - + "\"environment\":{\"name\":\"Test\"}," - + "\"flags\":{}," - + "\"observeFullEvaluationData\":true" - + "}"; - final String wrapped = - "{\"data\":{\"type\":\"universal-flag-configuration\",\"attributes\":" + attributes + "}}"; - final ServerConfiguration parsed = - JsonApiUfcResponseParser.INSTANCE.parse( - wrapped.getBytes(java.nio.charset.StandardCharsets.UTF_8)); - assertNotNull(parsed); - assertTrue(parsed.observeFullEvaluationData); - FeatureFlaggingGateway.dispatch(parsed); - - final BackendApi mockEvp = mock(BackendApi.class); - final FlagEvaluationTestSupport.TestWriterSetup setup = buildTestWriter(mockEvp); - setup.handler.add(piiEvent()); - - final Map ev = eventForFlag(flushAndCapture(setup).parsed, "pii-flag"); - assertNotNull(ev); - assertEquals("jane.doe@datadoghq.com", ev.get("targeting_key")); - } - @Test void observeFullEvaluationDataFalseHashesTargetingKeyAndOmitsContext() throws Exception { - dispatchObserveFullEvaluationData(false); - assertHashedTargetingKeyAndOmittedContext(); + assertHashedTargetingKeyAndOmittedContext(piiEvent(false)); } @Test - void observeFullEvaluationDataAbsentDefaultsToHashedBehavior() throws Exception { - // No UFC dispatched (default state) — must behave exactly like the explicit "false" case. - assertHashedTargetingKeyAndOmittedContext(); + void flagEvalEventDefaultConsentHashesTargetingKeyAndOmitsContext() throws Exception { + // An event built without an explicit consent value defaults to the privacy-preserving false, so + // it must behave exactly like the explicit "false" case. This is the state the hook produces + // when no UFC has been dispatched (the gateway reports false). + assertHashedTargetingKeyAndOmittedContext(piiEventDefaultConsent()); } @Test - void bucketCapturedUnderFalseStaysHashedEvenIfGatewayLaterReportsTrue() throws Exception { - // Regression guard for the flush-time TOCTOU bug: consent is captured when the evaluation is - // aggregated, not read from the gateway at flush. A bucket aggregated while consent was off - // must stay hashed even if a later RC update turns consent on before the flush drains. - dispatchObserveFullEvaluationData(false); + void eventConsentFalseStaysHashedEvenWhenGatewayLaterReportsTrue() throws Exception { + // Regression guard: consent is decided by the value the event carried at evaluation time, never + // re-read from the gateway at flush. An event evaluated under consent=false must stay hashed + // even if a later RC update turns the gateway's consent on before the flush drains. final BackendApi mockEvp = mock(BackendApi.class); final FlagEvaluationTestSupport.TestWriterSetup setup = buildTestWriter(mockEvp); - setup.handler.add(piiEvent()); - setup.handler.drainAndAggregate(); // captures consent=false into the bucket + setup.handler.add(piiEvent(false)); - // Simulate a later RC update flipping consent on; the already-aggregated bucket must not - // follow. + // Flip the gateway's consent on before both aggregation and flush; the event's evaluation-time + // snapshot (false) must win at every downstream step, so neither may consult the gateway. dispatchObserveFullEvaluationData(true); + setup.handler.drainAndAggregate(); final java.util.List captured = new java.util.ArrayList<>(); when(mockEvp.post(eq("flagevaluation"), any(RequestBody.class), any(), any(), eq(false))) @@ -639,10 +608,43 @@ void bucketCapturedUnderFalseStaysHashedEvenIfGatewayLaterReportsTrue() throws E assertFalse(json.raw.contains("jane.doe@datadoghq.com")); } - private void assertHashedTargetingKeyAndOmittedContext() throws Exception { + @Test + void eventConsentTrueStaysRawEvenWhenGatewayLaterReportsFalse() throws Exception { + // Symmetric guard: an event evaluated under consent=true must stay raw even if a later RC + // update + // turns the gateway's consent off before aggregation and flush. Together with the false-stays- + // hashed test this pins that neither aggregation nor flush ever consults the gateway. + final BackendApi mockEvp = mock(BackendApi.class); + final FlagEvaluationTestSupport.TestWriterSetup setup = buildTestWriter(mockEvp); + setup.handler.add(piiEvent(true)); + + dispatchObserveFullEvaluationData(false); + setup.handler.drainAndAggregate(); + + final java.util.List captured = new java.util.ArrayList<>(); + when(mockEvp.post(eq("flagevaluation"), any(RequestBody.class), any(), any(), eq(false))) + .thenAnswer( + inv -> { + captured.add(inv.getArgument(1)); + return null; + }); + setup.handler.flush(); + + assertEquals(1, captured.size()); + final Map ev = + eventForFlag(FlagEvaluationTestSupport.readJson(captured.get(0)).parsed, "pii-flag"); + assertNotNull(ev); + assertEquals("jane.doe@datadoghq.com", ev.get("targeting_key")); + final Map ctx = (Map) ev.get("context"); + assertNotNull(ctx); + assertNotNull(ctx.get("evaluation")); + } + + private void assertHashedTargetingKeyAndOmittedContext(final FlagEvalEvent piiEvent) + throws Exception { final BackendApi mockEvp = mock(BackendApi.class); final FlagEvaluationTestSupport.TestWriterSetup setup = buildTestWriter(mockEvp); - setup.handler.add(piiEvent()); + setup.handler.add(piiEvent); final FlagEvaluationTestSupport.CapturedJson captured = flushAndCapture(setup); @@ -658,10 +660,25 @@ private void assertHashedTargetingKeyAndOmittedContext() throws Exception { assertFalse(captured.raw.contains("\"evaluation\":")); } - private static FlagEvalEvent piiEvent() { + private static FlagEvalEvent piiEvent(final boolean observeFullEvaluationData) { + return event( + "pii-flag", + "on", + "alloc1", + "jane.doe@datadoghq.com", + 1000L, + observeFullEvaluationData, + piiAttrs()); + } + + private static FlagEvalEvent piiEventDefaultConsent() { + return event("pii-flag", "on", "alloc1", "jane.doe@datadoghq.com", 1000L, piiAttrs()); + } + + private static Map piiAttrs() { final Map attrs = new HashMap<>(); attrs.put("region", "us-east-1"); - return event("pii-flag", "on", "alloc1", "jane.doe@datadoghq.com", 1000L, attrs); + return attrs; } private static void dispatchObserveFullEvaluationData(final boolean value) {