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 7a0a588cc7b..fa394417c8a 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 @@ -100,6 +100,10 @@ class DDEvaluator implements Evaluator, FeatureFlaggingGateway.ConfigListener { static final String METADATA_SPLIT_SERIAL_ID = "__dd_split_serial_id"; static final String METADATA_DO_LOG = "__dd_do_log"; + // Stamped on every DD-produced evaluation (including PROVIDER_NOT_READY, with false). Missing + // key = non-DD provider; the hook falls back to false (fail-closed). + static final String METADATA_OBSERVE_FULL_EVALUATION_DATA = "observe_full_evaluation_data"; + // Read once: when off, the __dd_* span-enrichment metadata is not attached to evaluations, so an // enabled provider pays nothing extra unless span enrichment is also enabled. The gate does not // change at runtime, and this class is loaded lazily (well after startup) so config is ready. @@ -148,30 +152,42 @@ public ProviderEvaluation evaluate( final String key, final T defaultValue, final EvaluationContext context) { + // Snapshot the config once and thread observeFullEvaluationData through every + // ProviderEvaluation returned, so the hook's consent decision is pinned to this evaluation's + // config and cannot drift on a concurrent Remote Config swap. + final ServerConfiguration config = configuration.get(); + // Boolean.TRUE.equals covers both null (privacy-preserving default) and Boolean.FALSE without + // an NPE — the field is boxed so a malformed UFC message doesn't abort the whole parse. + final boolean observeFullEvaluationData = + config != null && Boolean.TRUE.equals(config.observeFullEvaluationData); try { - final ServerConfiguration config = configuration.get(); if (config == null) { - return error(defaultValue, ErrorCode.PROVIDER_NOT_READY); + return error(defaultValue, ErrorCode.PROVIDER_NOT_READY, null, observeFullEvaluationData); } if (context == null) { - return error(defaultValue, ErrorCode.INVALID_CONTEXT); + return error(defaultValue, ErrorCode.INVALID_CONTEXT, null, observeFullEvaluationData); } final Flag flag = config.flags.get(key); if (flag == null) { - return error(defaultValue, ErrorCode.FLAG_NOT_FOUND); + return error(defaultValue, ErrorCode.FLAG_NOT_FOUND, null, observeFullEvaluationData); } if (!flag.enabled) { return ProviderEvaluation.builder() .value(defaultValue) .reason(Reason.DISABLED.name()) + .flagMetadata(consentMetadata(observeFullEvaluationData)) .build(); } if (flag.allocations == null) { - return error(defaultValue, ErrorCode.GENERAL, "Missing allocations for flag " + key); + return error( + defaultValue, + ErrorCode.GENERAL, + "Missing allocations for flag " + key, + observeFullEvaluationData); } final Instant now = Instant.now(); @@ -201,10 +217,12 @@ public ProviderEvaluation evaluate( allocation, split, context, - evalTimestampMs); + evalTimestampMs, + observeFullEvaluationData); } else { if (targetingKey == null) { - return error(defaultValue, ErrorCode.TARGETING_KEY_MISSING); + return error( + defaultValue, ErrorCode.TARGETING_KEY_MISSING, null, observeFullEvaluationData); } // To match a split, subject must match ALL underlying shards boolean allShardsMatch = true; @@ -224,7 +242,8 @@ public ProviderEvaluation evaluate( allocation, split, context, - evalTimestampMs); + evalTimestampMs, + observeFullEvaluationData); } } } @@ -234,32 +253,40 @@ public ProviderEvaluation evaluate( return ProviderEvaluation.builder() .value(defaultValue) .reason(Reason.DEFAULT.name()) + .flagMetadata(consentMetadata(observeFullEvaluationData)) .build(); } catch (final PatternSyntaxException e) { - return error(defaultValue, ErrorCode.PARSE_ERROR, e); + return error(defaultValue, ErrorCode.PARSE_ERROR, e.getMessage(), observeFullEvaluationData); } catch (final NumberFormatException e) { - return error(defaultValue, ErrorCode.TYPE_MISMATCH, e); + return error( + defaultValue, ErrorCode.TYPE_MISMATCH, e.getMessage(), observeFullEvaluationData); } catch (final Exception e) { - return error(defaultValue, ErrorCode.GENERAL, e); + return error(defaultValue, ErrorCode.GENERAL, e.getMessage(), observeFullEvaluationData); } } - private static ProviderEvaluation error(final T defaultValue, final ErrorCode code) { - return error(defaultValue, code, (String) null); - } - - private static ProviderEvaluation error( - final T defaultValue, final ErrorCode code, final Throwable cause) { - return error(defaultValue, code, cause == null ? null : cause.getMessage()); + private static ImmutableMetadata consentMetadata(final boolean observeFullEvaluationData) { + return ImmutableMetadata.builder() + .addBoolean(METADATA_OBSERVE_FULL_EVALUATION_DATA, observeFullEvaluationData) + .build(); } private static ProviderEvaluation error( - final T defaultValue, final ErrorCode code, final String errorMessage) { + final T defaultValue, + final ErrorCode code, + final String errorMessage, + final boolean observeFullEvaluationData) { + // Under consent-off the errorMessage is dropped: exception messages from the outer catch blocks + // (NumberFormatException, generic Exception) can echo raw evaluation-context values, so they + // must never reach any consumer of ProviderEvaluation.getErrorMessage() — not just our own + // wire hook. Downstream (FlagEvalLoggingHook) falls back to ErrorCode.name(), so operators + // still get a stable signal like "TYPE_MISMATCH". return ProviderEvaluation.builder() .value(defaultValue) .reason(Reason.ERROR.name()) .errorCode(code) - .errorMessage(errorMessage) + .errorMessage(observeFullEvaluationData ? errorMessage : null) + .flagMetadata(consentMetadata(observeFullEvaluationData)) .build(); } @@ -422,15 +449,15 @@ private static ProviderEvaluation resolveVariant( final Allocation allocation, final Split split, final EvaluationContext context, - final long evalTimestampMs) { + final long evalTimestampMs, + final boolean observeFullEvaluationData) { final Variant variant = flag.variations.get(variationKey); if (variant == null) { - return ProviderEvaluation.builder() - .value(defaultValue) - .reason(Reason.ERROR.name()) - .errorCode(ErrorCode.GENERAL) - .errorMessage("Variant not found for: " + variationKey) - .build(); + return error( + defaultValue, + ErrorCode.GENERAL, + "Variant not found for: " + variationKey, + observeFullEvaluationData); } if (!isTypeCompatible(target, flag.variationType)) { @@ -440,7 +467,8 @@ private static ProviderEvaluation resolveVariant( "Requested type " + target.getSimpleName() + " does not match flag variationType " - + flag.variationType.name()); + + flag.variationType.name(), + observeFullEvaluationData); } final T mappedValue; @@ -455,7 +483,8 @@ private static ProviderEvaluation resolveVariant( + "' value does not match declared type " + flag.variationType.name() + ": " - + e.getMessage()); + + e.getMessage(), + observeFullEvaluationData); } // Stamp eval-time at the resolution point so first/last_evaluation reflect evaluation time, @@ -465,7 +494,8 @@ private static ProviderEvaluation resolveVariant( .addString("flagKey", flag.key) .addString("variationType", flag.variationType.name()) .addString("allocationKey", allocation.key) - .addLong("__dd_eval_timestamp_ms", evalTimestampMs); + .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 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 0bec8f8b3d0..322f11ac9e1 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 @@ -18,10 +18,12 @@ * evaluation context, and a non-blocking offer to the writer's bounded queue. Aggregation and * posting are deferred to the writer's worker thread. * - *

Hot-path cost: DDEvaluator.copyPrunedContext performs one bounded walk of the caller-owned - * EvaluationContext, applying every retained-size cap inline so work is proportional to what is - * kept, never to what the caller supplied. The returned map is capped by field count, key length, - * value length, list width, structure width, and depth. + *

Hot-path cost: under consent-on (observeFullEvaluationData=true) DDEvaluator.copyPrunedContext + * performs one bounded walk of the caller-owned EvaluationContext, applying every retained-size cap + * inline so work is proportional to what is kept, never to what the caller supplied. The returned + * map is capped by field count, key length, value length, list width, structure width, and depth. + * Under consent-off the context is dropped on emit, so the copy is skipped entirely and the hot + * path is scalar-only. * *

This hook is registered alongside the existing OTel FlagEvalMetricsHook - it does NOT replace * it. @@ -107,9 +109,24 @@ public void finallyAfter( // evaluated value. A null variant means no variant was selected (runtime default). final String variant = details.getVariant(); - // error message: prefer the human-readable message; fall back to the error code name when - // the message is empty (some providers populate only the code). null on success. - String errorMessage = details.getErrorMessage(); + // targetingKey from evaluation context + final String targetingKey = + ctx != null && ctx.getCtx() != null ? ctx.getCtx().getTargetingKey() : null; + + // Consent is read from metadata stamped by DDEvaluator (pinned to its ServerConfiguration). + // Missing key = non-DD provider → false, the privacy-preserving default. + final Boolean consentFromMetadata = + metadata != null + ? metadata.getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA) + : null; + final boolean observeFullEvaluationData = consentFromMetadata != null && consentFromMetadata; + + // Error message: prefer the human-readable message under consent-on; under consent-off the + // provider's raw message can echo evaluation-context values (e.g. NumberFormatException: + // "For input string: \"jane.doe@...\""), so replace it with the ErrorCode name — a stable, + // PII-free signal. Same substitution path is used when the message is absent regardless of + // consent (some providers populate only the code). Null on success. + String errorMessage = observeFullEvaluationData ? details.getErrorMessage() : null; if ((errorMessage == null || errorMessage.isEmpty()) && details.getErrorCode() != null) { errorMessage = details.getErrorCode().name(); } @@ -117,25 +134,33 @@ public void finallyAfter( errorMessage = null; } - // targetingKey from evaluation context - final String targetingKey = - ctx != null && ctx.getCtx() != null ? ctx.getCtx().getTargetingKey() : null; - - // Bounded copy of the caller's mutable context (see DDEvaluator#copyPrunedContext for - // every retained-size cap). Runs inline because the event is consumed asynchronously and - // the source context is caller-owned; work is proportional to what is retained. - final DDEvaluator.CopyResult copy = - ctx != null && ctx.getCtx() != null - ? DDEvaluator.copyPrunedContext(ctx.getCtx()) - : new DDEvaluator.CopyResult(Collections.emptyMap(), null); - - if (copy.truncatedReason != null) { - w.countContextTruncated(copy.truncatedReason); + // On the protected path (consent-off) the evaluation context is dropped on emit and never + // consulted by the aggregator, so skip the bounded copy entirely — the copy cost only + // applies under consent-on. + final Map attrs; + if (observeFullEvaluationData && ctx != null && ctx.getCtx() != null) { + // Bounded copy of the caller's mutable context (see DDEvaluator.copyPrunedContext for + // every retained-size cap). Runs inline because the event is consumed asynchronously + // and the source context is caller-owned; work is proportional to what is retained. + final DDEvaluator.CopyResult copy = DDEvaluator.copyPrunedContext(ctx.getCtx()); + if (copy.truncatedReason != null) { + w.countContextTruncated(copy.truncatedReason); + } + attrs = copy.attrs; + } else { + attrs = Collections.emptyMap(); } w.enqueue( new FlagEvalEvent( - flagKey, variant, allocationKey, targetingKey, errorMessage, evalTimeMs, copy.attrs)); + flagKey, + variant, + allocationKey, + targetingKey, + errorMessage, + evalTimeMs, + observeFullEvaluationData, + attrs)); } catch (LinkageError e) { // Never let EVP recording break flag evaluation } 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 7bd3a3790ad..06d3d42af24 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 @@ -12,6 +12,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasEntry; +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.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -25,8 +27,14 @@ import com.squareup.moshi.Types; import datadog.trace.api.featureflag.FeatureFlaggingGateway; 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.Flag; +import datadog.trace.api.featureflag.ufc.v1.Rule; 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 dev.openfeature.sdk.ErrorCode; import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.MutableContext; @@ -199,7 +207,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"); @@ -214,6 +222,241 @@ public void testNoAllocations() { assertThat(details.getErrorCode(), nullValue()); } + // ---- observeFullEvaluationData metadata is stamped from the evaluator's ServerConfiguration + // ---- + // + // Every code path that returns a ProviderEvaluation must stamp the consent boolean so downstream + // hooks can honour it. These tests exercise each stamp site with both consent values (on/off) so + // a mutation to any stamp — deleting the line, hardcoding the value — flips at least one + // assertion. + + // -- success path: resolveVariant (variant metadata builder) -- + + @Test + public void observeFullEvaluationDataStampedTrueOnResolvedVariant() { + final ProviderEvaluation details = evaluateMatchingFlag(true); + + assertThat(details.getReason(), equalTo("STATIC")); + assertThat(details.getVariant(), equalTo("on")); + assertThat( + details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA), + equalTo(true)); + } + + @Test + public void observeFullEvaluationDataStampedFalseOnResolvedVariant() { + // Symmetric consent-off assertion. Paired with the consent-on test above this pins the + // resolveVariant metadata line (DDEvaluator.java: METADATA_OBSERVE_FULL_EVALUATION_DATA) so + // deleting it or hardcoding either value would fail at least one assertion. + final ProviderEvaluation details = evaluateMatchingFlag(false); + + assertThat(details.getReason(), equalTo("STATIC")); + assertThat(details.getVariant(), equalTo("on")); + assertThat( + details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA), + equalTo(false)); + } + + // -- DISABLED path: flag.enabled=false -- + + @Test + public void observeFullEvaluationDataStampedTrueOnDisabledFlag() { + final ProviderEvaluation details = evaluateDisabledFlag(true); + + assertThat(details.getReason(), equalTo("DISABLED")); + assertThat( + details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA), + equalTo(true)); + } + + @Test + public void observeFullEvaluationDataStampedFalseOnDisabledFlag() { + final ProviderEvaluation details = evaluateDisabledFlag(false); + + assertThat(details.getReason(), equalTo("DISABLED")); + assertThat( + details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA), + equalTo(false)); + } + + // -- DEFAULT path: no allocation matches -- + + @Test + public void observeFullEvaluationDataStampedTrueOnDefault() { + // Allocation exists but has empty splits, so the loop finishes without returning and we fall + // through to the DEFAULT branch. + final ProviderEvaluation details = evaluateWithEmptySplits(true); + + assertThat(details.getReason(), equalTo("DEFAULT")); + assertThat( + details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA), + equalTo(true)); + } + + @Test + public void observeFullEvaluationDataStampedFalseOnDefault() { + final ProviderEvaluation details = evaluateWithEmptySplits(false); + + assertThat(details.getReason(), equalTo("DEFAULT")); + assertThat( + details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA), + equalTo(false)); + } + + // -- error paths: FLAG_NOT_FOUND / PROVIDER_NOT_READY (via consentMetadata in error()) -- + + @Test + public void observeFullEvaluationDataStampedOnFlagNotFoundError() { + // Was previously named "…OnSuccess" but actually exercises the error() helper's stamp via + // FLAG_NOT_FOUND — kept for that stamp site, correctly named. + final DDEvaluator evaluator = new DDEvaluator(mock(Runnable.class)); + evaluator.accept(new ServerConfiguration("", "", true, null, new HashMap<>())); + + final EvaluationContext ctx = new MutableContext("target").setTargetingKey("k"); + final ProviderEvaluation details = + evaluator.evaluate(Integer.class, "unknown-flag", 23, ctx); + + assertThat(details.getErrorCode(), equalTo(ErrorCode.FLAG_NOT_FOUND)); + assertThat( + details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA), + equalTo(true)); + } + + @Test + public void observeFullEvaluationDataDefaultsToFalseWhenEvaluatorHasNoConfig() { + final DDEvaluator evaluator = new DDEvaluator(mock(Runnable.class)); + final ProviderEvaluation details = + evaluator.evaluate(Integer.class, "test", 23, mock(EvaluationContext.class)); + assertThat(details.getErrorCode(), equalTo(ErrorCode.PROVIDER_NOT_READY)); + assertThat( + details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA), + equalTo(false)); + } + + @Test + public void observeFullEvaluationDataNullConfigFieldTreatedAsFalse() { + // The field is boxed so Moshi tolerates a malformed consent value in the UFC JSON without + // aborting the whole parse. The evaluator must then interpret null as the privacy-preserving + // default. An auto-unbox at the read site (config.observeFullEvaluationData) would NPE here. + final Map flags = new HashMap<>(); + flags.put("target", new Flag("target", true, ValueType.INTEGER, emptyMap(), emptyList())); + final DDEvaluator evaluator = new DDEvaluator(mock(Runnable.class)); + evaluator.accept(new ServerConfiguration("", "", null, null, flags)); + + final EvaluationContext ctx = new MutableContext("target").setTargetingKey("k"); + final ProviderEvaluation details = evaluator.evaluate(Integer.class, "target", 23, ctx); + + // Flags still evaluate — availability preserved despite the malformed consent field. + assertThat(details.getReason(), equalTo("DEFAULT")); + assertThat( + details.getFlagMetadata().getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA), + equalTo(false)); + } + + // Builds a flag that reaches resolveVariant: enabled, one allocation with no rules, one split + // with empty shards (so the shard-match branch is skipped and the split is picked immediately), + // and a single "on" variant whose value maps to the requested Integer type. + private static ProviderEvaluation evaluateMatchingFlag( + final boolean observeFullEvaluationData) { + final Map variations = new HashMap<>(); + variations.put("on", new Variant("on", 1)); + final Split split = new Split(emptyList(), "on", emptyMap(), null); + final Allocation allocation = + new Allocation("alloc-1", null, null, null, singletonList(split), Boolean.FALSE); + return evaluateFlag( + new Flag("target", true, ValueType.INTEGER, variations, singletonList(allocation)), + observeFullEvaluationData); + } + + private static ProviderEvaluation evaluateDisabledFlag( + final boolean observeFullEvaluationData) { + return evaluateFlag( + new Flag("target", false, ValueType.INTEGER, emptyMap(), null), observeFullEvaluationData); + } + + private static ProviderEvaluation evaluateWithEmptySplits( + final boolean observeFullEvaluationData) { + // Enabled, allocations present, allocation active, no rules, empty splits → falls through the + // for-loop to the DEFAULT return. + final Allocation allocation = + new Allocation("alloc-1", null, null, null, emptyList(), Boolean.FALSE); + return evaluateFlag( + new Flag("target", true, ValueType.INTEGER, emptyMap(), singletonList(allocation)), + observeFullEvaluationData); + } + + private static ProviderEvaluation evaluateFlag( + final Flag flag, final boolean observeFullEvaluationData) { + final Map flags = new HashMap<>(); + flags.put("target", flag); + final DDEvaluator evaluator = new DDEvaluator(mock(Runnable.class)); + evaluator.accept(new ServerConfiguration("", "", observeFullEvaluationData, null, flags)); + + final EvaluationContext ctx = new MutableContext("target").setTargetingKey("user-1"); + return evaluator.evaluate(Integer.class, "target", 23, ctx); + } + + // ---- error message redaction respects observeFullEvaluationData ---- + + @Test + public void numericConditionOnTargetingKeyDropsExceptionMessageUnderConsentOff() { + // Rule {attribute:"id", operator:GT, value:0} + "id" not in context → + // DDEvaluator.resolveAttribute + // falls back to the targeting key, so Double.parseDouble("jane.doe@datadoghq.com") throws + // NumberFormatException. The exception message echoes the raw context value verbatim, so it + // must be dropped when observeFullEvaluationData=false. + final ProviderEvaluation details = + evaluateWithNumericRuleOnId("jane.doe@datadoghq.com", false); + + assertThat(details.getErrorCode(), equalTo(ErrorCode.TYPE_MISMATCH)); + assertNull(details.getErrorMessage(), "consent-off must not surface the raw exception message"); + } + + @Test + public void numericConditionOnTargetingKeyPreservesExceptionMessageUnderConsentOn() { + // Symmetric case: with consent on, the raw exception message flows through unchanged so + // operators keep the diagnostic detail they opted in to. + final ProviderEvaluation details = + evaluateWithNumericRuleOnId("jane.doe@datadoghq.com", true); + + assertThat(details.getErrorCode(), equalTo(ErrorCode.TYPE_MISMATCH)); + assertThat(details.getErrorMessage(), equalTo("For input string: \"jane.doe@datadoghq.com\"")); + } + + @Test + public void numericConditionOnTargetingKeyErrorMessageNeverContainsPiiUnderConsentOff() { + // Belt-and-suspenders: independent of the exact null/empty form, the raw PII value must never + // appear in the message under consent-off. Guards against future changes that might replace + // null with a redacted string or a code-name suffix. + final ProviderEvaluation details = + evaluateWithNumericRuleOnId("jane.doe@datadoghq.com", false); + + final String message = details.getErrorMessage(); + assertFalse( + message != null && message.contains("jane.doe@datadoghq.com"), + "consent-off errorMessage must not contain raw context values"); + } + + private static ProviderEvaluation evaluateWithNumericRuleOnId( + final String targetingKey, final boolean observeFullEvaluationData) { + final Map flags = new HashMap<>(); + final List rules = + singletonList( + new Rule(singletonList(new ConditionConfiguration(ConditionOperator.GT, "id", 0)))); + // Split must be non-empty so the allocation is considered a match target; its contents don't + // matter because the rule throws before a split is picked. + final Allocation allocation = + new Allocation("alloc", rules, null, null, emptyList(), Boolean.FALSE); + flags.put( + "num-rule", + new Flag("num-rule", true, ValueType.INTEGER, emptyMap(), singletonList(allocation))); + final DDEvaluator evaluator = new DDEvaluator(mock(Runnable.class)); + evaluator.accept(new ServerConfiguration("", "", observeFullEvaluationData, null, flags)); + + final EvaluationContext ctx = new MutableContext(targetingKey); + return evaluator.evaluate(Integer.class, "num-rule", 23, ctx); + } + @Test public void testAllocationDateAbiAndInstantAccessors() throws Exception { final Date startAt = Date.from(Instant.parse("2024-01-01T00:00:00Z")); 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 360521e0c86..3b7910d4712 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 @@ -17,6 +17,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; @@ -50,6 +51,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 ---- @@ -234,7 +238,9 @@ void absentVariantProducesNullVariant() { // ---- test: error message captured from details (error object support) ---- @Test - void errorMessageCapturedFromDetails() { + void errorMessageCapturedFromDetailsUnderConsentOn() { + // With observeFullEvaluationData=true the provider's raw message is preserved verbatim so + // operators keep the diagnostic detail they opted in to. final AtomicReference captured = new AtomicReference<>(); final FlagEvalLoggingHook hook = hookWithWriter(capturingWriter(captured)); @@ -245,6 +251,7 @@ void errorMessageCapturedFromDetails() { .reason(Reason.ERROR.name()) .errorCode(ErrorCode.TYPE_MISMATCH) .errorMessage("value does not match declared type") + .flagMetadata(consentOnMetadata()) .build(); hook.finallyAfter(null, det, Collections.emptyMap()); @@ -253,7 +260,61 @@ void errorMessageCapturedFromDetails() { assertEquals( "value does not match declared type", captured.get().errorMessage, - "errorMessage must be captured from the evaluation details"); + "errorMessage must be captured from the evaluation details under consent-on"); + } + + @Test + void errorMessageReplacedByErrorCodeUnderConsentOff() { + // Defense-in-depth: even if a provider hands us a raw message under consent-off (a bug in the + // provider, or a third-party provider that doesn't distinguish consent tiers), the hook must + // substitute the ErrorCode name so raw PII from exception messages never reaches the wire. + // Uses a PII-looking marker exactly like the wire-level guards in FlagEvaluationWriterImplTest. + final AtomicReference captured = new AtomicReference<>(); + final FlagEvalLoggingHook hook = hookWithWriter(capturingWriter(captured)); + + final FlagEvaluationDetails det = + FlagEvaluationDetails.builder() + .flagKey("err-flag") + .value("default") + .reason(Reason.ERROR.name()) + .errorCode(ErrorCode.TYPE_MISMATCH) + .errorMessage("For input string: \"jane.doe@datadoghq.com\"") + .flagMetadata(consentOffMetadata()) + .build(); + + hook.finallyAfter(null, det, Collections.emptyMap()); + + assertNotNull(captured.get()); + assertEquals( + "TYPE_MISMATCH", + captured.get().errorMessage, + "consent-off must replace the raw message with the ErrorCode name"); + assertFalse( + captured.get().errorMessage.contains("jane.doe@datadoghq.com"), + "raw PII must never survive into the enqueued event under consent-off"); + } + + @Test + void errorMessageDroppedWhenConsentOffAndNoErrorCode() { + // Edge case: no ErrorCode available (unusual — providers should set one for ERROR reason). + // Consent-off drops the message and there's nothing to substitute, so the enqueued event has + // no error message at all. This is the strictest privacy-preserving outcome. + final AtomicReference captured = new AtomicReference<>(); + final FlagEvalLoggingHook hook = hookWithWriter(capturingWriter(captured)); + + final FlagEvaluationDetails det = + FlagEvaluationDetails.builder() + .flagKey("err-flag") + .value("default") + .reason(Reason.ERROR.name()) + .errorMessage("For input string: \"jane.doe@datadoghq.com\"") + .flagMetadata(consentOffMetadata()) + .build(); + + hook.finallyAfter(null, det, Collections.emptyMap()); + + assertNotNull(captured.get()); + assertNull(captured.get().errorMessage); } // ---- test: error code used as fallback message when error message is empty ---- @@ -354,7 +415,12 @@ void countContextTruncatedCalledWhenContextIsTruncated() { .ctx(ctx) .build(); final FlagEvaluationDetails det = - details("flag", "v", "v", dev.openfeature.sdk.Reason.TARGETING_MATCH.name(), null); + details( + "flag", + "v", + "v", + dev.openfeature.sdk.Reason.TARGETING_MATCH.name(), + consentOnMetadata()); hook.finallyAfter(hookCtx, det, Collections.emptyMap()); @@ -378,7 +444,12 @@ void countContextTruncatedNotCalledWhenNoTruncation() { .ctx(ctx) .build(); final FlagEvaluationDetails det = - details("flag", "v", "v", dev.openfeature.sdk.Reason.TARGETING_MATCH.name(), null); + details( + "flag", + "v", + "v", + dev.openfeature.sdk.Reason.TARGETING_MATCH.name(), + consentOnMetadata()); hook.finallyAfter(hookCtx, det, Collections.emptyMap()); @@ -485,7 +556,7 @@ void contextAttributesAreFlattenedAndConvertedInline() { .ctx(context) .build(); final FlagEvaluationDetails det = - details("ctx-flag", "v", "v", Reason.TARGETING_MATCH.name(), null); + details("ctx-flag", "v", "v", Reason.TARGETING_MATCH.name(), consentOnMetadata()); hook.finallyAfter(hookCtx, det, Collections.emptyMap()); @@ -524,7 +595,7 @@ void contextAttributesUseEnqueueTimeSnapshot() { .ctx(context) .build(); final FlagEvaluationDetails det = - details("ctx-flag", "v", "v", Reason.TARGETING_MATCH.name(), null); + details("ctx-flag", "v", "v", Reason.TARGETING_MATCH.name(), consentOnMetadata()); hook.finallyAfter(hookCtx, det, Collections.emptyMap()); context.add("region", "eu-west-1"); @@ -543,4 +614,107 @@ void contextAttributesUseEnqueueTimeSnapshot() { assertFalse(attrs.containsKey("profile.late")); assertFalse(attrs.containsKey("cohorts[1]")); } + + // ---- observeFullEvaluationData is read from evaluation metadata, never the gateway ---- + + @Test + void readsObserveFullEvaluationDataTrueFromEvaluationMetadata() { + assertTrue(enqueuedEventWithConsentMetadata(true).observeFullEvaluationData); + } + + @Test + void readsObserveFullEvaluationDataFalseFromEvaluationMetadata() { + assertFalse(enqueuedEventWithConsentMetadata(false).observeFullEvaluationData); + } + + @Test + void observeFullEvaluationDataDefaultsToFalseWhenMetadataAbsent() { + // No metadata at all: fail-closed toward privacy. + assertFalse(enqueuedEventWithConsentMetadata(null).observeFullEvaluationData); + } + + @Test + void protectedPathSkipsEvaluationContextCapture() { + // Consent off → the hook must not snapshot the evaluation context at all. Verified by mutating + // the context after finallyAfter returns and asserting the enqueued event still sees nothing. + final AtomicReference captured = new AtomicReference<>(); + final FlagEvalLoggingHook hook = hookWithWriter(capturingWriter(captured)); + + final MutableContext context = new MutableContext("user-1"); + context.add("region", "us-east-1"); + + final HookContext hookCtx = + HookContext.builder() + .flagKey("ctx-flag") + .type(FlagValueType.STRING) + .defaultValue("default") + .ctx(context) + .build(); + final ImmutableMetadata consentOff = + ImmutableMetadata.builder() + .addBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA, false) + .build(); + + hook.finallyAfter( + hookCtx, + details("ctx-flag", "v", "v", Reason.TARGETING_MATCH.name(), consentOff), + Collections.emptyMap()); + context.add("region", "eu-west-1"); + + assertNotNull(captured.get()); + assertTrue(captured.get().attrs.isEmpty()); + } + + @Test + void ignoresGatewayConsentEvenWhenItDisagreesWithMetadata() { + // Gateway says on, metadata says off; hook must trust metadata. + FeatureFlaggingGateway.dispatch(observeConfig(true)); + try { + assertFalse(enqueuedEventWithConsentMetadata(false).observeFullEvaluationData); + } finally { + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + } + } + + /** + * Fires the hook once for a simple targeted evaluation whose metadata carries the given consent + * value ({@code null} = key absent) and returns the enqueued event. + */ + private FlagEvalEvent enqueuedEventWithConsentMetadata(final Boolean consent) { + final AtomicReference captured = new AtomicReference<>(); + final FlagEvalLoggingHook hook = hookWithWriter(capturingWriter(captured)); + final ImmutableMetadata metadata = + consent == null + ? null + : ImmutableMetadata.builder() + .addBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA, consent) + .build(); + hook.finallyAfter( + hookCtxWithTargetingKey("obs-flag", "user-1"), + details("obs-flag", "on", "on", Reason.TARGETING_MATCH.name(), metadata), + Collections.emptyMap()); + assertNotNull(captured.get(), "writer.enqueue must be called once"); + return captured.get(); + } + + private static ImmutableMetadata consentOnMetadata() { + return ImmutableMetadata.builder() + .addBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA, true) + .build(); + } + + private static ImmutableMetadata consentOffMetadata() { + return ImmutableMetadata.builder() + .addBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA, false) + .build(); + } + + 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-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 ed5143a8aca..8f4ed7de81e 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 @@ -376,6 +376,7 @@ public void testClientEvaluationRoutesThroughFlagEvalLoggingHook() throws Except ImmutableMetadata.builder() .addString("allocationKey", "allocation-1") .addLong("__dd_eval_timestamp_ms", 1_700_000_000_000L) + .addBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA, true) .build()) .build()); final OpenFeatureAPI api = OpenFeatureAPI.getInstance(); 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 470df9b9bb4..f5f44b94d7b 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 @@ -6,8 +6,8 @@ /** * Lightweight data record capturing a single flag evaluation for EVP flagevaluation emission. * - *

This is the currency passed from the {@code FlagEvalLoggingHook} (feature-flagging-api) to the - * {@code FlagEvaluationWriter} (feature-flagging-lib) via a non-blocking bounded queue. + *

This is the currency passed from the FlagEvalLoggingHook (feature-flagging-api) to the + * FlagEvaluationWriter (feature-flagging-lib) via a non-blocking bounded queue. * *

Scalar fields and context attributes are captured at hook-fire time on the evaluation thread. * No aggregation happens here. @@ -18,8 +18,8 @@ public final class FlagEvalEvent { public final String flagKey; /** - * The OpenFeature variant key selected for the evaluation. {@code null} means the default value - * was returned (runtime default). + * The OpenFeature variant key selected for the evaluation. Null means the default value was + * returned (runtime default). */ public final String variant; @@ -30,14 +30,14 @@ public final class FlagEvalEvent { public final String targetingKey; /** - * The evaluation error message when the evaluation failed, else {@code null}. Sourced from the + * The evaluation error message when the evaluation failed, else null. Sourced from the * OpenFeature evaluation details (error message, falling back to the error code). */ public final String errorMessage; /** * Evaluation timestamp in milliseconds since epoch. Stamped at eval-entry time from flag metadata - * key {@code "dd.eval.timestamp_ms"}, or falls back to hook-fire time when absent. This ensures + * key __dd_eval_timestamp_ms, or falls back to hook-fire time when absent. This ensures * first/last_evaluation reflect evaluation time, not hook-fire time. */ public final long evalTimeMs; @@ -48,6 +48,13 @@ public final class FlagEvalEvent { */ public final Map attrs; + /** + * PII consent from the ServerConfiguration used by the evaluation. When false (privacy-preserving + * default), the targeting key is hashed and the per-evaluation context is omitted on emission. + */ + public final boolean observeFullEvaluationData; + + /** Convenience constructor; consent defaults to the privacy-preserving false. */ public FlagEvalEvent( final String flagKey, final String variant, @@ -55,7 +62,19 @@ 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 false. */ + public FlagEvalEvent( + final String flagKey, + final String variant, + final String allocationKey, + final String targetingKey, + final String errorMessage, + final long evalTimeMs, + final Map attrs) { + this(flagKey, variant, allocationKey, targetingKey, errorMessage, evalTimeMs, false, attrs); } public FlagEvalEvent( @@ -65,6 +84,7 @@ public FlagEvalEvent( final String targetingKey, final String errorMessage, final long evalTimeMs, + final boolean observeFullEvaluationData, final Map attrs) { this.flagKey = flagKey; this.variant = variant; @@ -72,6 +92,7 @@ public FlagEvalEvent( this.targetingKey = targetingKey; this.errorMessage = errorMessage; this.evalTimeMs = evalTimeMs; + this.observeFullEvaluationData = observeFullEvaluationData; this.attrs = attrs != null ? attrs : Collections.emptyMap(); } } 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..caaa85a611f 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,24 @@ public class ServerConfiguration { public final String createdAt; public final String format; + // Boxed on purpose. Moshi's reflective adapter for a primitive boolean field aborts the whole + // UFC parse when the JSON value is null or not a boolean; with a Boolean field it tolerates + // null (and other malformed values are still caught locally) so a malformed consent field + // doesn't strand a fresh pod on PROVIDER_NOT_READY. Read sites must use + // Boolean.TRUE.equals(...) so null falls to the privacy-preserving default. + 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/flagevaluation/FlagEvalEventTest.java b/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEventTest.java index 42b5805816d..cc614150178 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; @@ -36,4 +37,18 @@ void storesErrorMessageAndDefaultsNullContextAttributes() { assertEquals("type mismatch", event.errorMessage); assertTrue(event.attrs.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); + } + + @Test + void storesExplicitObserveFullEvaluationData() { + final Map attrs = Collections.emptyMap(); + 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 7997ff7d89b..7f4ae23a0d5 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 @@ -46,13 +46,18 @@ final class FlagEvaluationAggregator { void aggregate(final FlagEvalEvent event) { final boolean isDefault = event.variant == null; - final Map prunedAttrs = event.attrs; - final String ctxKey = canonicalContextKey(prunedAttrs); + final boolean observeFullEvaluationData = event.observeFullEvaluationData; + // On the protected path the context is dropped on emit, so it must not fragment buckets or be + // stored — otherwise a high-cardinality field (request_id, timestamp) blows out PER_FLAG_CAP + // 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); EvalBucket bucket = fullTier.get(fullKey); if (bucket != null) { bucket.merge(event.evalTimeMs, isDefault); + bucket.observeFullEvaluationData &= observeFullEvaluationData; return; } @@ -68,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; @@ -78,6 +84,7 @@ void aggregate(final FlagEvalEvent event) { bucket = degradedTier.get(degradedKey); if (bucket != null) { bucket.merge(event.evalTimeMs, isDefault); + bucket.observeFullEvaluationData &= observeFullEvaluationData; return; } @@ -92,7 +99,8 @@ void aggregate(final FlagEvalEvent event) { event.errorMessage, event.evalTimeMs, isDefault, - null)); + null, + observeFullEvaluationData)); return; } @@ -143,8 +151,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, ""), - new EvalBucket(key, "on", "alloc", null, null, 1L, false, null)); + new FullKey(key, "on", "alloc", false, null, null, "", false), + new EvalBucket(key, "on", "alloc", null, null, 1L, false, null, false)); globalFullCount.incrementAndGet(); perFlagCount.merge(key, 1, Integer::sum); } @@ -155,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)); } } @@ -175,7 +183,8 @@ void addDegradedBucketForTest( errorMessage, evalTimeMs, variant == null, - null)); + null, + false)); } private static FullKey buildFullKey(final FlagEvalEvent event, final String ctxKey) { @@ -186,7 +195,8 @@ private static FullKey buildFullKey(final FlagEvalEvent event, final String ctxK event.variant == null, event.errorMessage, event.targetingKey, - ctxKey); + ctxKey, + event.observeFullEvaluationData); } private static DegradedKey buildDegradedKey(final FlagEvalEvent event) { @@ -253,6 +263,13 @@ static class EvalBucket { String targetingKey; String errorMessage; Map prunedAttrs; + // 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 + // any consent-off event lands in the bucket. That's benign because the degraded wire path + // drops the targeting key and context regardless of consent, so this field has no downstream + // effect for degraded rows. + boolean observeFullEvaluationData; EvalBucket( final String flagKey, @@ -262,7 +279,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; @@ -273,6 +291,7 @@ static class EvalBucket { this.count = 1; this.runtimeDefaultUsed = runtimeDefaultUsed; this.prunedAttrs = prunedAttrs; + this.observeFullEvaluationData = observeFullEvaluationData; } int prunedContextFieldCount() { @@ -301,6 +320,10 @@ static final class FullKey { private final String errorMessage; private final String targetingKey; private final String contextKey; + // Part of the key so consent-on and consent-off evaluations never share a bucket. The + // 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; FullKey( final String flagKey, @@ -309,7 +332,8 @@ static final class FullKey { final boolean runtimeDefaultUsed, final String errorMessage, final String targetingKey, - final String contextKey) { + final String contextKey, + final boolean observeFullEvaluationData) { this.flagKey = flagKey; this.variant = variant; this.allocationKey = allocationKey; @@ -317,6 +341,7 @@ static final class FullKey { this.errorMessage = errorMessage; this.targetingKey = targetingKey; this.contextKey = contextKey; + this.observeFullEvaluationData = observeFullEvaluationData; } @Override @@ -329,6 +354,7 @@ public boolean equals(final Object o) { } final FullKey fullKey = (FullKey) o; return runtimeDefaultUsed == fullKey.runtimeDefaultUsed + && observeFullEvaluationData == fullKey.observeFullEvaluationData && Objects.equals(flagKey, fullKey.flagKey) && Objects.equals(variant, fullKey.variant) && Objects.equals(allocationKey, fullKey.allocationKey) @@ -346,11 +372,19 @@ public int hashCode() { runtimeDefaultUsed, errorMessage, targetingKey, - contextKey); + contextKey, + observeFullEvaluationData); } } static final class DegradedKey { + // Unlike FullKey, consent is NOT a bucket dimension here: the wire serializer for degraded rows + // (FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket with isFullTier=false) drops the + // targeting key and context unconditionally, so two degraded buckets differing only in consent + // would emit byte-identical JSON with evaluation_count split — halving effective DEGRADED_CAP + // for zero wire fidelity. Mixed-consent events merge into one bucket; the AND-fold on + // EvalBucket.observeFullEvaluationData still runs but has no downstream effect for degraded + // rows. private final String flagKey; private final String variant; private final String allocationKey; 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..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; @@ -194,7 +203,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 +214,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 HASHED_TARGETING_KEY_PREFIX + 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 6aa3cb74654..3518ef1afd0 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 @@ -491,15 +491,20 @@ void flush() { private List buildEventList() { final long flushTimeMs = System.currentTimeMillis(); + // 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()) { events.add( - FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket(bucket, true, flushTimeMs)); + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket( + bucket, true, bucket.observeFullEvaluationData, flushTimeMs)); } for (final FlagEvaluationAggregator.EvalBucket bucket : aggregator.degradedBuckets()) { events.add( - FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket(bucket, false, flushTimeMs)); + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket( + bucket, false, bucket.observeFullEvaluationData, flushTimeMs)); } return events; } 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 8951f20f866..f89ac1ddc38 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 @@ -40,6 +40,7 @@ final class UniversalFlagConfigParser implements ConfigurationDeserializer V1_ADAPTER = MOSHI.adapter(ServerConfiguration.class); @@ -126,6 +127,50 @@ public void toJson(@Nonnull final JsonWriter writer, @Nullable final MapOnly applies to Boolean.class (not primitive boolean), so mandatory primitive-boolean fields + * (e.g. Flag.enabled) keep their strict parse. + */ + static final class LenientBooleanAdapter extends JsonAdapter { + + 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() || type != Boolean.class) { + return null; + } + return new LenientBooleanAdapter(); + } + }; + + @Nullable + @Override + public Boolean fromJson(@Nonnull final JsonReader reader) throws IOException { + if (reader.peek() == JsonReader.Token.BOOLEAN) { + return reader.nextBoolean(); + } + // null and every wrong-typed value collapse to null so the caller falls back to its default + // rather than the enclosing config being rejected wholesale. + reader.skipValue(); + return null; + } + + @Override + public void toJson(@Nonnull final JsonWriter writer, @Nullable final Boolean 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/FlagEvaluationAggregatorTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationAggregatorTest.java index a661698ab7a..06faeadefea 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 @@ -38,8 +38,8 @@ void differentValueTypesProduceDifferentBuckets() { final Map attrsStr = new HashMap<>(); attrsStr.put("score", "1"); - aggregator.aggregate(event("flag-b", "on", "alloc1", "user-1", 1000L, attrsInt)); - aggregator.aggregate(event("flag-b", "on", "alloc1", "user-1", 1000L, attrsStr)); + aggregator.aggregate(event("flag-b", "on", "alloc1", "user-1", 1000L, true, attrsInt)); + aggregator.aggregate(event("flag-b", "on", "alloc1", "user-1", 1000L, true, attrsStr)); final FlagEvaluationAggregator.AggregatedState state = aggregator.snapshot(); assertEquals(2, state.fullTier.size()); @@ -98,6 +98,30 @@ void perFlagCapOverflowRoutesToDegradedTierAndMergesSameDegradedKey() { assertEquals(2000L, bucket.lastEvalMs); } + @Test + void mixedConsentDegradedEvaluationsMergeIntoOneBucket() { + // Mirror of mixedConsentEvaluationsForSameSubjectLandInDistinctBuckets, but for the degraded + // tier: the wire serializer for degraded rows drops the targeting key and context regardless + // of consent, so two events differing only in consent emit byte-identical JSON. They must + // share a bucket, otherwise DEGRADED_CAP is effectively halved for zero wire fidelity gain. + // The AND-fold on EvalBucket.observeFullEvaluationData still runs but the value has no + // downstream effect for degraded rows. + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + aggregator.perFlagCount.put("hot-flag", FlagEvaluationAggregator.PER_FLAG_CAP); + + aggregator.aggregate(event("hot-flag", "on", "alloc1", "user-1", 1000L, true, emptyMap())); + aggregator.aggregate(event("hot-flag", "on", "alloc1", "user-2", 2000L, false, emptyMap())); + + final FlagEvaluationAggregator.AggregatedState state = aggregator.snapshot(); + assertEquals(0, state.fullTier.size()); + assertEquals(1, state.degradedTier.size()); + final FlagEvaluationAggregator.EvalBucket bucket = + state.degradedTier.values().iterator().next(); + assertEquals(2, bucket.count); + // AND-fold collapses to consent-off; benign for degraded rows but a documented invariant. + assertFalse(bucket.observeFullEvaluationData); + } + @Test void absentVariantSetsRuntimeDefaultUsed() { final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); @@ -120,7 +144,7 @@ void aggregatorStoresPrunedAttrsVerbatim() { preprunedAttrs.put("key" + i, "v" + i); } - aggregator.aggregate(event("flag-d", "on", "alloc1", "user-1", 1000L, preprunedAttrs)); + aggregator.aggregate(event("flag-d", "on", "alloc1", "user-1", 1000L, true, preprunedAttrs)); final FlagEvaluationAggregator.AggregatedState state = aggregator.snapshot(); final FlagEvaluationAggregator.EvalBucket bucket = state.fullTier.values().iterator().next(); @@ -162,7 +186,7 @@ void aggregatorStoresPrePrunedAttrsWithoutRePruning() { final Map preprunedAttrs = new HashMap<>(); preprunedAttrs.put("short-val", "ok"); - aggregator.aggregate(event("flag-e", "on", "alloc1", "user-1", 1000L, preprunedAttrs)); + aggregator.aggregate(event("flag-e", "on", "alloc1", "user-1", 1000L, true, preprunedAttrs)); final FlagEvaluationAggregator.AggregatedState state = aggregator.snapshot(); final FlagEvaluationAggregator.EvalBucket bucket = state.fullTier.values().iterator().next(); @@ -194,7 +218,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()); @@ -248,6 +272,114 @@ void degradedKeyEqualityUsesEveryDimension() { assertNotEquals(base, degradedKey("flag", "on", "alloc", false, "other")); } + @Test + void mixedConsentEvaluationsForSameSubjectLandInDistinctBuckets() { + // Consent is part of FullKey: two evaluations that differ only in consent produce different + // wire rows (raw vs hashed targeting key, context vs no context) and belong in different + // buckets. Merging them would silently downgrade the consent-on row to the protected shape. + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 1000L, true, emptyMap())); + aggregator.aggregate(event("fold-flag", "on", "alloc1", "user-1", 2000L, false, emptyMap())); + + assertEquals(2, aggregator.fullTierSize()); + int onCount = 0; + int offCount = 0; + for (final FlagEvaluationAggregator.EvalBucket bucket : + aggregator.snapshot().fullTier.values()) { + if (bucket.observeFullEvaluationData) { + onCount++; + } else { + offCount++; + } + } + assertEquals(1, onCount); + assertEquals(1, offCount); + } + + @Test + void sameConsentEvaluationsForSameSubjectMergeIntoOneBucket() { + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + 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); + } + + @Test + void protectedPathCollapsesDifferingContextIntoOneBucket() { + // Same subject, different request-id contexts, consent off: the context is dropped on emit so + // it must not fragment full-tier buckets or the per-flag cap blows out under real traffic. + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + final Map ctx1 = new HashMap<>(); + ctx1.put("request_id", "req-1"); + final Map ctx2 = new HashMap<>(); + ctx2.put("request_id", "req-2"); + final Map ctx3 = new HashMap<>(); + ctx3.put("request_id", "req-3"); + + aggregator.aggregate(event("checkout", "on", "alloc1", "alice", 1000L, false, ctx1)); + aggregator.aggregate(event("checkout", "on", "alloc1", "alice", 2000L, false, ctx2)); + aggregator.aggregate(event("checkout", "on", "alloc1", "alice", 3000L, false, ctx3)); + + assertEquals(1, aggregator.fullTierSize()); + final FlagEvaluationAggregator.EvalBucket bucket = + aggregator.snapshot().fullTier.values().iterator().next(); + assertEquals(3, bucket.count); + assertFalse(bucket.observeFullEvaluationData); + assertEquals(0, bucket.prunedContextFieldCount()); + } + + @Test + void protectedPathSeparatesDifferentSubjects() { + // Different targeting keys must still fall into distinct buckets on the protected path — the + // (hashed) targeting key stays part of the aggregation identity. + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + aggregator.aggregate(event("checkout", "on", "alloc1", "alice", 1000L, false, emptyMap())); + aggregator.aggregate(event("checkout", "on", "alloc1", "bob", 2000L, false, emptyMap())); + + assertEquals(2, aggregator.fullTierSize()); + } + + @Test + void fullPathStillSplitsBucketsOnDifferingContext() { + // Consent-on preserves the previous behaviour: distinct contexts remain distinct buckets so + // each raw context is emitted verbatim. + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + final Map ctx1 = new HashMap<>(); + ctx1.put("plan", "pro"); + ctx1.put("request_id", "req-1"); + final Map ctx2 = new HashMap<>(); + ctx2.put("plan", "pro"); + ctx2.put("request_id", "req-2"); + + aggregator.aggregate(event("checkout", "on", "alloc1", "alice", 1000L, true, ctx1)); + aggregator.aggregate(event("checkout", "on", "alloc1", "alice", 2000L, true, ctx2)); + + assertEquals(2, aggregator.fullTierSize()); + } + + 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, + evalTimeMs, + observeFullEvaluationData, + attrs); + } + private static FlagEvalEvent event( final String flagKey, final String variant, @@ -277,7 +409,8 @@ private static FlagEvaluationAggregator.FullKey fullKey( runtimeDefaultUsed, errorMessage, targetingKey, - contextKey); + contextKey, + false); } private static FlagEvaluationAggregator.DegradedKey degradedKey( 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..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; @@ -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)); @@ -82,13 +82,14 @@ 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( 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,84 @@ 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, + true); + + 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, + false); + + 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/FlagEvaluationTestSupport.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationTestSupport.java index a04f12d7de7..4a65a81a8bf 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 @@ -55,6 +55,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 e5f8c019db7..6fef520b8ca 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,6 +6,7 @@ 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; @@ -33,10 +34,12 @@ 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; import java.io.IOException; +import java.lang.reflect.Field; import java.util.Collection; import java.util.HashMap; import java.util.Map; @@ -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 @@ -445,6 +450,143 @@ void splitPostFailureDoesNotRetryAlreadySentPayloads() throws Exception { assertEquals(2, posts.get()); } + private static final String HASHED_JANE_DOE = + "sha256_b4698f9b6d186781fa8dc59e533578fa2d8379a46b1cf6db85cda6aa9c99e51b"; + + @Test + void observeFullEvaluationDataTrueEmitsRawTargetingKeyAndContext() throws Exception { + // 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(true)); + + 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 { + assertHashedTargetingKeyAndOmittedContext(piiEvent(false)); + } + + @Test + 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 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(false)); + + // 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))) + .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")); + } + + @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")); + } + + @Test + void consentOffPreservesErrorCodeSignalAndNeverLeaksPiiInErrorMessage() throws Exception { + // Upstream contract: the hook substitutes the ErrorCode name for the raw exception message + // under consent-off (see + // FlagEvalLoggingHookTest#errorMessageReplacedByErrorCodeUnderConsentOff). + // This wire-level guard pins that a properly-formed consent-off event (a) still surfaces the + // stable ErrorCode signal for operators and (b) never lets a PII-shaped string escape onto the + // wire. Mirrors the existing PII guards on the targeting_key axis. + final BackendApi mockEvp = mock(BackendApi.class); + final FlagEvaluationTestSupport.TestWriterSetup setup = buildTestWriter(mockEvp); + setup.handler.add( + new FlagEvalEvent( + "err-flag", + null, + "alloc1", + "jane.doe@datadoghq.com", + "TYPE_MISMATCH", + 1000L, + false, + emptyMap())); + + final FlagEvaluationTestSupport.CapturedJson captured = flushAndCapture(setup); + + final Map ev = eventForFlag(captured.parsed, "err-flag"); + assertNotNull(ev); + final Map error = (Map) ev.get("error"); + assertNotNull(error, "error object must be present so operators keep the ErrorCode signal"); + assertEquals("TYPE_MISMATCH", error.get("message")); + assertEquals(HASHED_JANE_DOE, ev.get("targeting_key")); + assertFalse(captured.raw.contains("jane.doe@datadoghq.com")); + assertFalse( + captured.raw.contains("For input string"), + "no exception-message-shaped text may reach the wire under consent-off"); + } + @Test void encodeFailureClearsAggregatorSoLaterFlushesRecover() throws Exception { final BackendApi mockEvp = mock(BackendApi.class); @@ -454,7 +596,7 @@ void encodeFailureClearsAggregatorSoLaterFlushesRecover() throws Exception { // bucket. Before the fix, the aggregator kept the bucket and every later flush re-threw. final Map poison = new HashMap<>(); poison.put("bad-number", Double.NaN); - setup.handler.add(event("poison-flag", "on", "alloc1", "user-1", 1000L, poison)); + setup.handler.add(event("poison-flag", "on", "alloc1", "user-1", 1000L, true, poison)); setup.handler.drainAndAggregate(); setup.handler.flush(); verify(mockEvp, org.mockito.Mockito.never()) @@ -539,6 +681,68 @@ void hasCapacityForEnqueueReflectsQueueSaturationAndCountsPreQueueOverflow() { writer.close(); } + private void assertHashedTargetingKeyAndOmittedContext(final FlagEvalEvent piiEvent) + 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 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 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); + return field.get(writer); + } + + private static void awaitThreadState(final Thread thread, final Thread.State state) + throws InterruptedException { + final long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (thread.getState() != state && System.nanoTime() < deadline) { + Thread.sleep(10); + } + assertEquals(state, thread.getState()); + } + private static Map context() { final Map context = new HashMap<>(); context.put("service", "test-service"); 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..104adef4cbe 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; @@ -10,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 { @@ -73,10 +76,101 @@ void rejectsTrailingJson() { + "}}{}")); } + @Test + void observeFullEvaluationDataDefaultsToFalseWhenAbsent() throws Exception { + // Absent → Moshi leaves the boxed field null; the read site's Boolean.TRUE.equals(...) then + // resolves to the privacy-preserving default (consent-off). Either null-or-false is the + // documented invariant; assert the field never reads as true. + final ServerConfiguration configuration = parse(wrap(emptyConfig())); + assertNotNull(configuration); + assertFalse(Boolean.TRUE.equals(configuration.observeFullEvaluationData)); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void observeFullEvaluationDataParsesExplicitValue(final boolean value) throws Exception { + final ServerConfiguration configuration = + parse(wrap(configWithObserveFullEvaluationData(value))); + assertNotNull(configuration); + assertEquals(value, configuration.observeFullEvaluationData); + } + + @Test + void observeFullEvaluationDataExplicitNullDefaultsToFalseWithoutRejectingConfig() + throws Exception { + // An explicit null (or a wrong-typed value) for this field must not abort the whole UFC parse. + // A pod that starts after a malformed UFC has no last-known-good, so aborting would strand + // every flag on PROVIDER_NOT_READY (its default value). We fail closed on privacy (consent + // stays false) but preserve availability: flags parse and evaluate. + final ServerConfiguration configuration = + parse(wrap(configWithRawObserveFullEvaluationData("null"))); + + assertNotNull(configuration); + assertFalse( + configuration.observeFullEvaluationData != null && configuration.observeFullEvaluationData); + assertNotNull(configuration.flags); + } + + @Test + void observeFullEvaluationDataWrongTypedStringDefaultsToFalse() throws Exception { + // Moshi tolerates a stringified boolean like "true" via nullSafe/boxed handling: it either + // parses as null or throws locally and leaves the field null. Either way, downstream reads + // via Boolean.TRUE.equals(...) treat it as consent-off. The rest of the config must parse. + final ServerConfiguration configuration = + parse(wrap(configWithRawObserveFullEvaluationData("\"true\""))); + + assertNotNull(configuration); + assertFalse( + configuration.observeFullEvaluationData != null && configuration.observeFullEvaluationData); + assertNotNull(configuration.flags); + } + + @Test + void observeFullEvaluationDataWrongTypedNumberDefaultsToFalse() throws Exception { + final ServerConfiguration configuration = + parse(wrap(configWithRawObserveFullEvaluationData("1"))); + + assertNotNull(configuration); + assertFalse( + configuration.observeFullEvaluationData != null && configuration.observeFullEvaluationData); + assertNotNull(configuration.flags); + } + 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 configWithRawObserveFullEvaluationData(final String rawJsonValue) { + // Emit the field with a caller-controlled raw JSON value (null / "true" / 1 / ...) so we can + // assert the parser's tolerance of malformed shapes without going through configWith's + // boolean-typed helper. + return "{" + + "\"createdAt\":\"2024-04-17T19:40:53.716Z\"," + + "\"observeFullEvaluationData\":" + + rawJsonValue + + "," + + "\"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/RemoteConfigServiceImplTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java index 6d14a28f796..c1da0a11dcc 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 @@ -242,6 +242,35 @@ void flagMapAdapterFactoryOnlyCreatesFlagMapAdapterForFlagMapType() { flagsType, singleton(mock(Annotation.class)), moshi)); } + @Test + void lenientBooleanAdapterFactoryOnlyCreatesAdapterForUnannotatedBoxedBoolean() { + final Moshi moshi = moshi(); + + final JsonAdapter adapter = + UniversalFlagConfigParser.LenientBooleanAdapter.FACTORY.create( + Boolean.class, emptySet(), moshi); + + assertNotNull(adapter); + assertTrue(adapter instanceof UniversalFlagConfigParser.LenientBooleanAdapter); + // Primitive boolean keeps Moshi's strict adapter so mandatory fields still reject bad values. + assertNull( + UniversalFlagConfigParser.LenientBooleanAdapter.FACTORY.create( + boolean.class, emptySet(), moshi)); + // A qualified Boolean belongs to whichever adapter declared the qualifier, not to this one. + assertNull( + UniversalFlagConfigParser.LenientBooleanAdapter.FACTORY.create( + Boolean.class, singleton(mock(Annotation.class)), moshi)); + } + + @Test + void lenientBooleanAdapterIsReadOnly() { + final UniversalFlagConfigParser.LenientBooleanAdapter adapter = + new UniversalFlagConfigParser.LenientBooleanAdapter(); + + assertThrows( + UnsupportedOperationException.class, () -> adapter.toJson(mock(JsonWriter.class), true)); + } + @Test void allowsNullFlagMap() throws Exception { final ServerConfiguration config = 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}")); + } +}