From 28c18d8f497d64da3c011cd7f345dc531c337c17 Mon Sep 17 00:00:00 2001 From: Tyler Potter Date: Thu, 29 Oct 2026 06:41:34 -0600 Subject: [PATCH 1/4] feat(openfeature): evaluate dependent flags Add strict dependent-flag evaluation, ordered telemetry and transactional exposures, and the agentless capability header. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 --- .../com/datadog/openfeature/DDEvaluator.java | 202 +++++- .../openfeature/ExposureLoggingHook.java | 23 +- .../com/datadog/openfeature/Provider.java | 20 +- .../delivery/CdnConfigurationSource.java | 5 +- .../internal/evaluation/FlagEvaluator.java | 585 +++++++++++++++--- .../internal/ufc/ConditionConfiguration.java | 10 + .../internal/ufc/EvaluatorParams.java | 16 + .../internal/ufc/ServerConfiguration.java | 12 + .../ufc/UniversalFlagConfigParser.java | 53 +- .../EvaluationTelemetryOptOutTest.java | 103 ++- .../delivery/CdnConfigurationSourceTest.java | 3 + .../DependentFlagEvaluatorTest.java | 405 ++++++++++++ .../UniversalFlagConfigParserJsonApiTest.java | 5 +- .../ufc/UniversalFlagConfigParserTest.java | 44 ++ 14 files changed, 1369 insertions(+), 117 deletions(-) create mode 100644 dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/EvaluatorParams.java create mode 100644 dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java b/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java index 8c6ced76c65..8d1066862ac 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java @@ -1,5 +1,13 @@ package com.datadog.openfeature; +import static com.datadog.openfeature.internal.evaluation.MetadataKeys.ALLOCATION_KEY; +import static com.datadog.openfeature.internal.evaluation.MetadataKeys.DO_LOG; +import static com.datadog.openfeature.internal.evaluation.MetadataKeys.EVALUATION_FEATURE_PREFIX; +import static com.datadog.openfeature.internal.evaluation.MetadataKeys.EXPOSURE_FEATURE_PREFIX; +import static com.datadog.openfeature.internal.evaluation.MetadataKeys.HAS_EVALUATION_FEATURES; +import static com.datadog.openfeature.internal.evaluation.MetadataKeys.OBSERVE_FULL_EVALUATION_DATA; +import static com.datadog.openfeature.internal.evaluation.MetadataKeys.SPLIT_SERIAL_ID; + import com.datadog.openfeature.internal.FeatureFlagsRuntime; import com.datadog.openfeature.internal.bridge.v1.AgentBridge; import com.datadog.openfeature.internal.config.Settings; @@ -8,10 +16,13 @@ import com.datadog.openfeature.internal.exposure.ExposureDeduplicationCache; import com.datadog.openfeature.internal.exposure.ExposureEvent; import com.datadog.openfeature.internal.exposure.Subject; +import com.datadog.openfeature.internal.flagevaluation.FlagEvalEvent; +import com.datadog.openfeature.internal.flagevaluation.FlagEvaluationWriter; import com.datadog.openfeature.internal.ufc.Allocation; import com.datadog.openfeature.internal.ufc.ServerConfiguration; import com.datadog.openfeature.internal.ufc.ValueType; import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.ImmutableMetadata; import dev.openfeature.sdk.ImmutableStructure; import dev.openfeature.sdk.ProviderEvaluation; import dev.openfeature.sdk.Structure; @@ -27,11 +38,16 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; import java.util.function.Consumer; class DDEvaluator implements Evaluator, Consumer { + private static final String DEPENDENCY_EXPOSURES_TOKEN = "__dd_dependency_exposures_token"; + private static final String EVALUATION_TELEMETRY_OPT_OUT = "dd_evaluation_telemetry_opt_out"; + /** * Maximum evaluation-context nesting depth captured on the hot path. Recursion runs on the * caller's evaluation thread over a caller-owned Value tree, so an arbitrarily deep @@ -81,7 +97,14 @@ class DDEvaluator implements Evaluator, Consumer { private final Settings settings; private final FlagEvaluator core; private final ConfigurationStore configuration = new ConfigurationStore(); + private final ThreadLocal> stagedDependencyExposures = + new ThreadLocal<>(); + private final AtomicLong nextDependencyExposureToken = new AtomicLong(); + private final Map> pendingDependencyExposures = + new ConcurrentHashMap<>(); private volatile FeatureFlagsRuntime runtime; + private volatile boolean dependencyTelemetryOptOutEnabled; + private volatile FlagEvalMetrics dependencyMetrics; DDEvaluator(final Runnable configCallback, final AgentBridge bridge, final Settings settings) { this.configCallback = configCallback; @@ -89,7 +112,17 @@ class DDEvaluator implements Evaluator, Consumer { this.settings = settings; // When span enrichment is off, the __dd_* span-enrichment metadata is attached only to // evaluations of allocations logging exposures, for the exposure hook. - this.core = new FlagEvaluator(settings.isSpanEnrichmentEnabled(), this::isExposed); + this.core = + new FlagEvaluator( + settings.isSpanEnrichmentEnabled(), + this::isExposed, + this::recordDependencyEvaluation, + (items, context) -> stagedDependencyExposures.set(items)); + } + + void configureDependencyTelemetry(final boolean optOutEnabled, final FlagEvalMetrics metrics) { + this.dependencyTelemetryOptOutEnabled = optOutEnabled; + this.dependencyMetrics = metrics; } @Override @@ -147,7 +180,19 @@ public ProviderEvaluation evaluate( final String key, final T defaultValue, final EvaluationContext context) { - return core.evaluate(configuration.get(), target, key, defaultValue, context); + stagedDependencyExposures.remove(); + final ProviderEvaluation result = + core.evaluate(configuration.get(), target, key, defaultValue, context); + final List staged = stagedDependencyExposures.get(); + stagedDependencyExposures.remove(); + if (staged == null || staged.isEmpty() || result.getErrorCode() != null) { + return result; + } + final long token = nextDependencyExposureToken.incrementAndGet(); + pendingDependencyExposures.put(token, staged); + result.setFlagMetadata( + withLongMetadata(result.getFlagMetadata(), DEPENDENCY_EXPOSURES_TOKEN, token)); + return result; } static boolean isAllocationActive(final Allocation allocation, final Instant now) { @@ -232,6 +277,159 @@ void recordExposure(final ExposureEvent event) { } } + private void recordDependencyEvaluation( + final String flagKey, + final ProviderEvaluation details, + final EvaluationContext context, + final long evaluationTimestampMs) { + if (dependencyTelemetryOptOutEnabled && isEvaluationTelemetryOptedOut(context)) { + return; + } + final ImmutableMetadata metadata = details.getFlagMetadata(); + final Map values = + metadata == null ? Collections.emptyMap() : metadata.asUnmodifiableMap(); + final Object allocationValue = values.get(ALLOCATION_KEY); + final String allocationKey = + allocationValue instanceof String ? (String) allocationValue : null; + final FlagEvalMetrics metrics = dependencyMetrics; + if (metrics != null) { + metrics.record( + flagKey, + details.getVariant(), + details.getReason(), + details.getErrorCode(), + allocationKey); + } + final FeatureFlagsRuntime current = runtime(); + final FlagEvaluationWriter writer = current == null ? null : current.evaluations(); + if (writer == null || !writer.hasCapacityForEnqueue()) { + if (writer != null) { + writer.countPreQueueOverflow(); + } + return; + } + final boolean observeFullEvaluationData = + Boolean.TRUE.equals(values.get(OBSERVE_FULL_EVALUATION_DATA)); + String errorMessage = observeFullEvaluationData ? details.getErrorMessage() : null; + if ((errorMessage == null || errorMessage.isEmpty()) && details.getErrorCode() != null) { + errorMessage = details.getErrorCode().name(); + } + final Map attrs; + if (observeFullEvaluationData) { + final CopyResult copy = copyPrunedContext(context); + if (copy.truncatedReason != null) { + writer.countContextTruncated(copy.truncatedReason); + } + attrs = copy.attrs; + } else { + attrs = Collections.emptyMap(); + } + final Map features = + Boolean.TRUE.equals(values.get(HAS_EVALUATION_FEATURES)) + ? SplitFeatureMetadata.featuresWithPrefix(metadata, EVALUATION_FEATURE_PREFIX) + : Collections.emptyMap(); + writer.enqueue( + new FlagEvalEvent( + flagKey, + details.getVariant(), + allocationKey, + context == null ? null : context.getTargetingKey(), + errorMessage, + evaluationTimestampMs, + observeFullEvaluationData, + attrs, + features)); + } + + List takeDependencyExposures(final ImmutableMetadata metadata) { + if (metadata == null) { + return Collections.emptyList(); + } + final Object token = metadata.asUnmodifiableMap().get(DEPENDENCY_EXPOSURES_TOKEN); + if (!(token instanceof Long)) { + return Collections.emptyList(); + } + final List exposures = + pendingDependencyExposures.remove((Long) token); + return exposures == null + ? Collections.emptyList() + : exposures; + } + + void recordDependencyExposures( + final List exposures, final EvaluationContext context) { + final ExposureDeduplicationCache cache = exposureCache(); + if (cache == null || context == null) { + return; + } + for (final FlagEvaluator.DependencyExposure exposure : exposures) { + final ProviderEvaluation details = exposure.details; + final ImmutableMetadata metadata = details.getFlagMetadata(); + final Map values = + metadata == null ? Collections.emptyMap() : metadata.asUnmodifiableMap(); + if (!Boolean.TRUE.equals(values.get(DO_LOG))) { + continue; + } + final Object allocationValue = values.get(ALLOCATION_KEY); + final String allocationKey = + allocationValue instanceof String ? (String) allocationValue : null; + final String variantKey = details.getVariant(); + if (allocationKey == null || variantKey == null) { + continue; + } + final Object serialValue = values.get(SPLIT_SERIAL_ID); + final Integer serialId = serialValue instanceof Integer ? (Integer) serialValue : null; + if (cache.recordIfChanged( + exposure.flagKey, context.getTargetingKey(), allocationKey, variantKey, serialId) + && isExposureDeliveryEnabled()) { + recordExposure( + exposureEvent( + exposure.flagKey, + allocationKey, + variantKey, + context, + serialId, + SplitFeatureMetadata.featuresWithPrefix(metadata, EXPOSURE_FEATURE_PREFIX))); + } + } + } + + private static boolean isEvaluationTelemetryOptedOut(final EvaluationContext context) { + final Value value = context == null ? null : context.getValue(EVALUATION_TELEMETRY_OPT_OUT); + return value != null && value.isBoolean() && Boolean.TRUE.equals(value.asBoolean()); + } + + private static ImmutableMetadata withLongMetadata( + final ImmutableMetadata metadata, final String key, final long value) { + final ImmutableMetadata.ImmutableMetadataBuilder builder = ImmutableMetadata.builder(); + if (metadata != null) { + for (final Map.Entry entry : metadata.asUnmodifiableMap().entrySet()) { + addMetadata(builder, entry.getKey(), entry.getValue()); + } + } + builder.addLong(key, value); + return builder.build(); + } + + private static void addMetadata( + final ImmutableMetadata.ImmutableMetadataBuilder builder, + final String key, + final Object value) { + if (value instanceof String) { + builder.addString(key, (String) value); + } else if (value instanceof Boolean) { + builder.addBoolean(key, (Boolean) value); + } else if (value instanceof Integer) { + builder.addInteger(key, (Integer) value); + } else if (value instanceof Long) { + builder.addLong(key, (Long) value); + } else if (value instanceof Float) { + builder.addFloat(key, (Float) value); + } else if (value instanceof Double) { + builder.addDouble(key, (Double) value); + } + } + static AbstractMap flattenContext(final EvaluationContext context) { return flattenValues(snapshotValues(context)); } diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/ExposureLoggingHook.java b/dd-openfeature/src/main/java/com/datadog/openfeature/ExposureLoggingHook.java index db6eba8a70d..a9329e187a7 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/ExposureLoggingHook.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/ExposureLoggingHook.java @@ -12,6 +12,8 @@ import dev.openfeature.sdk.Hook; import dev.openfeature.sdk.HookContext; import dev.openfeature.sdk.ImmutableMetadata; +import dev.openfeature.sdk.Reason; +import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; @@ -46,10 +48,27 @@ public void finallyAfter( final FlagEvaluationDetails details, final Map hints) { try { - if (details == null || details.getErrorCode() != null) { + if (details == null) { log.trace("Feature Flagging exposure suppressed: evaluation failed or details unavailable"); return; } + final DDEvaluator evaluator = this.evaluatorSupplier.get(); + final List + dependencyExposures = + evaluator == null + ? java.util.Collections + . + emptyList() + : evaluator.takeDependencyExposures(details.getFlagMetadata()); + if (details.getErrorCode() != null || Reason.ERROR.name().equals(details.getReason())) { + log.trace("Feature Flagging exposure suppressed: evaluation failed or details unavailable"); + return; + } + final EvaluationContext context = ctx == null ? null : ctx.getCtx(); + if (evaluator != null) { + evaluator.recordDependencyExposures(dependencyExposures, context); + } final ImmutableMetadata metadata = details.getFlagMetadata(); if (metadata == null) { log.trace("Feature Flagging exposure suppressed: metadata unavailable"); @@ -69,13 +88,11 @@ public void finallyAfter( final String allocationKey = allocationValue instanceof String ? (String) allocationValue : null; final String variantKey = details.getVariant(); - final EvaluationContext context = ctx == null ? null : ctx.getCtx(); if (allocationKey == null || variantKey == null || context == null) { log.trace( "Feature Flagging exposure suppressed: allocation, variant, or context unavailable"); return; } - final DDEvaluator evaluator = this.evaluatorSupplier.get(); final ExposureDeduplicationCache cache = evaluator == null ? null : evaluator.exposureCache(); if (cache == null) { log.trace("Feature Flagging exposure suppressed: provider not initialized"); diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/Provider.java b/dd-openfeature/src/main/java/com/datadog/openfeature/Provider.java index 70ac3436b8c..980918d8706 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/Provider.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/Provider.java @@ -43,6 +43,7 @@ public class Provider extends EventProvider implements Metadata { new AtomicReference<>(InitializationState.NOT_STARTED); private final FlagEvalMetrics flagEvalMetrics; private final FlagEvalMetricsHook flagEvalMetricsHook; + private final boolean evaluationTelemetryOptOutEnabled; // Span enrichment: null unless the gate is on, so the feature has no idle overhead when off. private final SpanEnrichmentHook spanEnrichmentHook; // Precomputed hook list returned by getProviderHooks() on every evaluation. Immutable and built @@ -90,6 +91,9 @@ public Provider(final Options options) { } this.flagEvalMetrics = metrics; this.flagEvalMetricsHook = hook; + this.evaluationTelemetryOptOutEnabled = + options.isEvaluationTelemetryOptOutParameterEnabled(this.settings); + configureDependencyTelemetry(evaluator); // Span enrichment is wired ONLY when the gate is on — off means no capture hook and no idle // per-evaluation overhead. @@ -112,7 +116,7 @@ public Provider(final Options options) { if (spanEnrichmentHook != null) { hooks.add(spanEnrichmentHook); } - if (options.isEvaluationTelemetryOptOutParameterEnabled(this.settings)) { + if (this.evaluationTelemetryOptOutEnabled) { for (int i = 0; i < hooks.size(); i++) { hooks.set(i, new EvaluationTelemetryOptOutHook(hooks.get(i))); } @@ -279,9 +283,21 @@ private void markInitializationError() { private Evaluator buildEvaluator() { if (evaluator != null) { + configureDependencyTelemetry(evaluator); return evaluator; } - return new DDEvaluator(this::onConfigurationChange, this.bridge, this.settings); + final DDEvaluator built = + new DDEvaluator(this::onConfigurationChange, this.bridge, this.settings); + configureDependencyTelemetry(built); + return built; + } + + private void configureDependencyTelemetry(final Evaluator candidate) { + if (candidate instanceof DDEvaluator) { + ((DDEvaluator) candidate) + .configureDependencyTelemetry( + this.evaluationTelemetryOptOutEnabled, this.flagEvalMetrics); + } } private FlagEvaluationWriter flagEvaluationPipeline() { diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/CdnConfigurationSource.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/CdnConfigurationSource.java index 7eed50bb8d2..10b0504ca2c 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/CdnConfigurationSource.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/CdnConfigurationSource.java @@ -35,6 +35,8 @@ public final class CdnConfigurationSource implements ConfigurationService { static final String UFC_RULES_BASED_SERVER_PATH = "/api/v2/feature-flagging/config/rules-based/server"; + static final String FEATURE_FLAGS_CAPABILITIES_HEADER = "X-DD-FEATURE-FLAGS-CAPABILITIES"; + static final String DEPENDENT_FLAG_CAPABILITY = "dependent-flag-variant-key-v1"; static final int MAX_ATTEMPTS = 3; private static final long FIRST_RETRY_MIN_MILLIS = 2_000; private static final long FIRST_RETRY_MAX_MILLIS = 10_000; @@ -296,7 +298,8 @@ private HttpRequest buildRequest() { final HttpRequest.Builder request = HttpRequest.newBuilder(this.endpoint) .timeout(this.requestTimeout) - .header("Accept-Encoding", "gzip"); + .header("Accept-Encoding", "gzip") + .header(FEATURE_FLAGS_CAPABILITIES_HEADER, DEPENDENT_FLAG_CAPABILITY); Headers.addMetadata(request); if (this.apiKey != null) { request.header("DD-API-KEY", this.apiKey); diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java index dd4b4956492..b8a389b3c20 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java @@ -35,8 +35,11 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.time.Instant; +import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -53,12 +56,15 @@ public final class FlagEvaluator { static final String FEATURE_DESTINATION_HOOK = "HOOK"; static final String FEATURE_DESTINATION_EXPOSURE = "EXPOSURE"; static final String FEATURE_DESTINATION_EVALUATION = "EVALUATION"; + static final int DEFAULT_MAX_DEPENDENCY_DEPTH = 10; // A newer UFC can name destinations this SDK does not know; warn once per name, not per // evaluation. static final int MAX_WARNED_FEATURE_DESTINATIONS = 64; static final Set WARNED_FEATURE_DESTINATIONS = ConcurrentHashMap.newKeySet(); private final boolean spanEnrichmentEnabled; private final ExposureLookup exposureLookup; + private final DependencyEvaluationSink dependencyEvaluationSink; + private final DependencyExposureSink dependencyExposureSink; /** * Creates an evaluator. @@ -67,8 +73,22 @@ public final class FlagEvaluator { * @param exposureLookup the lookup telling whether an exposure was already recorded. */ public FlagEvaluator(final boolean spanEnrichmentEnabled, final ExposureLookup exposureLookup) { + this( + spanEnrichmentEnabled, + exposureLookup, + (flag, details, context, timestamp) -> {}, + (items, context) -> {}); + } + + public FlagEvaluator( + final boolean spanEnrichmentEnabled, + final ExposureLookup exposureLookup, + final DependencyEvaluationSink dependencyEvaluationSink, + final DependencyExposureSink dependencyExposureSink) { this.spanEnrichmentEnabled = spanEnrichmentEnabled; this.exposureLookup = exposureLookup; + this.dependencyEvaluationSink = dependencyEvaluationSink; + this.dependencyExposureSink = dependencyExposureSink; } public ProviderEvaluation evaluate( @@ -84,114 +104,131 @@ public ProviderEvaluation evaluate( // 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); + final EvaluationState state = + new EvaluationState(config, context, observeFullEvaluationData, System.currentTimeMillis()); try { - if (config == null) { - return error(defaultValue, ErrorCode.PROVIDER_NOT_READY, null, observeFullEvaluationData); - } - - if (context == null) { - return error(defaultValue, ErrorCode.INVALID_CONTEXT, null, observeFullEvaluationData); - } - - final Flag flag = config.flags.get(key); - if (flag == null) { - if (config.invalidFlags != null && config.invalidFlags.containsKey(key)) { - return error( - defaultValue, - ErrorCode.PARSE_ERROR, - "invalid configuration for flag " + key, - observeFullEvaluationData); - } - 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(); - } + final ProviderEvaluation result = evaluateFlag(state, target, key, defaultValue); + state.complete(result.getErrorCode() == null); + return result; + } catch (final DependencyGraphException e) { + state.complete(false); + return error(defaultValue, e.errorCode, e.getMessage(), observeFullEvaluationData); + } catch (final PatternSyntaxException e) { + state.complete(false); + return error(defaultValue, ErrorCode.PARSE_ERROR, e.getMessage(), observeFullEvaluationData); + } catch (final NumberFormatException e) { + state.complete(false); + return error( + defaultValue, ErrorCode.TYPE_MISMATCH, e.getMessage(), observeFullEvaluationData); + } catch (final Exception e) { + state.complete(false); + return error(defaultValue, ErrorCode.GENERAL, e.getMessage(), observeFullEvaluationData); + } + } - if (flag.allocations == null) { + private ProviderEvaluation evaluateFlag( + final EvaluationState state, final Class target, final String key, final T defaultValue) { + final ServerConfiguration config = state.config; + if (config == null) { + return error( + defaultValue, ErrorCode.PROVIDER_NOT_READY, null, state.observeFullEvaluationData); + } + if (state.context == null) { + return error(defaultValue, ErrorCode.INVALID_CONTEXT, null, state.observeFullEvaluationData); + } + final Flag flag = config.flags == null ? null : config.flags.get(key); + if (flag == null) { + if (config.invalidFlags != null && config.invalidFlags.containsKey(key)) { return error( defaultValue, - ErrorCode.GENERAL, - "Missing allocations for flag " + key, - observeFullEvaluationData); + ErrorCode.PARSE_ERROR, + "invalid configuration for flag " + key, + state.observeFullEvaluationData); } + return error(defaultValue, ErrorCode.FLAG_NOT_FOUND, null, state.observeFullEvaluationData); + } + if (!flag.enabled) { + return ProviderEvaluation.builder() + .value(defaultValue) + .reason(Reason.DISABLED.name()) + .flagMetadata(consentMetadata(state.observeFullEvaluationData)) + .build(); + } + if (flag.allocations == null) { + return error( + defaultValue, + ErrorCode.GENERAL, + "Missing allocations for flag " + key, + state.observeFullEvaluationData); + } - final Instant now = Instant.now(); - final long evalTimestampMs = now.toEpochMilli(); - final String targetingKey = context.getTargetingKey(); - - for (final Allocation allocation : flag.allocations) { - if (!isAllocationActive(allocation, now)) { - continue; - } - - if (!isEmpty(allocation.rules)) { - if (!evaluateRules(allocation.rules, context)) { - continue; - } - } - - if (!isEmpty(allocation.splits)) { - for (final Split split : allocation.splits) { - if (isEmpty(split.shards)) { - return resolveVariant( - target, + final Instant now = Instant.ofEpochMilli(state.evaluationTimestampMs); + final String targetingKey = state.context.getTargetingKey(); + for (final Allocation allocation : flag.allocations) { + if (!isAllocationActive(allocation, now)) { + continue; + } + final ExposureCheckpoint allocationCheckpoint = state.checkpoint(); + if (!isEmpty(allocation.rules) + && !evaluateRules(allocation.rules, state, key, state.context)) { + state.rollback(allocationCheckpoint); + continue; + } + if (!isEmpty(allocation.splits)) { + for (final Split split : allocation.splits) { + boolean allShardsMatch = true; + if (!isEmpty(split.shards)) { + if (targetingKey == null) { + state.rollback(allocationCheckpoint); + return error( defaultValue, - flag, - split.variationKey, - allocation, - split, - context, - evalTimestampMs, - observeFullEvaluationData); - } else { - if (targetingKey == null) { - return error( - defaultValue, ErrorCode.TARGETING_KEY_MISSING, null, observeFullEvaluationData); - } - // To match a split, subject must match ALL underlying shards - boolean allShardsMatch = true; - for (final Shard shard : split.shards) { - if (!matchesShard(shard, targetingKey)) { - allShardsMatch = false; - break; - } - } - if (allShardsMatch) { - return resolveVariant( - target, - defaultValue, - flag, - split.variationKey, - allocation, - split, - context, - evalTimestampMs, - observeFullEvaluationData); + ErrorCode.TARGETING_KEY_MISSING, + null, + state.observeFullEvaluationData); + } + for (final Shard shard : split.shards) { + if (!matchesShard(shard, targetingKey)) { + allShardsMatch = false; + break; } } } + if (allShardsMatch) { + return resolveVariant( + target, + defaultValue, + flag, + split.variationKey, + allocation, + split, + state.context, + state.evaluationTimestampMs, + state.observeFullEvaluationData); + } } } + state.rollback(allocationCheckpoint); + } + return ProviderEvaluation.builder() + .value(defaultValue) + .reason(Reason.DEFAULT.name()) + .flagMetadata(consentMetadata(state.observeFullEvaluationData)) + .build(); + } - return ProviderEvaluation.builder() - .value(defaultValue) - .reason(Reason.DEFAULT.name()) - .flagMetadata(consentMetadata(observeFullEvaluationData)) - .build(); - } catch (final PatternSyntaxException e) { - return error(defaultValue, ErrorCode.PARSE_ERROR, e.getMessage(), observeFullEvaluationData); - } catch (final NumberFormatException e) { - return error( - defaultValue, ErrorCode.TYPE_MISMATCH, e.getMessage(), observeFullEvaluationData); - } catch (final Exception e) { - return error(defaultValue, ErrorCode.GENERAL, e.getMessage(), observeFullEvaluationData); + private static int maxDependencyDepth(final ServerConfiguration config) { + if (config.evaluatorParams == null || !config.evaluatorParams.maxDependencyDepthPresent) { + return DEFAULT_MAX_DEPENDENCY_DEPTH; } + final Object configured = config.evaluatorParams.maxDependencyDepth; + if (configured instanceof Number) { + final double value = ((Number) configured).doubleValue(); + if (Double.isFinite(value) && value == Math.rint(value) && value >= 0 && value <= 255) { + return (int) value; + } + } + throw new DependencyGraphException( + ErrorCode.PARSE_ERROR, "maxDependencyDepth must be an integer from 0 through 255"); } private static ImmutableMetadata consentMetadata(final boolean observeFullEvaluationData) { @@ -237,15 +274,20 @@ public static boolean isAllocationActive(final Allocation allocation, final Inst return true; } - private static boolean evaluateRules(final List rules, final EvaluationContext context) { + private boolean evaluateRules( + final List rules, + final EvaluationState state, + final String rootFlagKey, + final EvaluationContext context) { for (final Rule rule : rules) { if (isEmpty(rule.conditions)) { continue; } + final ExposureCheckpoint ruleCheckpoint = state.checkpoint(); boolean allConditionsMatch = true; for (final ConditionConfiguration condition : rule.conditions) { - if (!evaluateCondition(condition, context)) { + if (!evaluateCondition(condition, state, rootFlagKey, context)) { allConditionsMatch = false; break; } @@ -254,21 +296,30 @@ private static boolean evaluateRules(final List rules, final EvaluationCon if (allConditionsMatch) { return true; } + state.rollback(ruleCheckpoint); } return false; } - private static boolean evaluateCondition( - final ConditionConfiguration condition, final EvaluationContext context) { + private boolean evaluateCondition( + final ConditionConfiguration condition, + final EvaluationState state, + final String rootFlagKey, + final EvaluationContext context) { + final Object attributeValue; + if (condition.flagEvaluationKey != null) { + attributeValue = + state.dependencySession(rootFlagKey).resolveCurrent(condition.flagEvaluationKey); + } else { + attributeValue = resolveAttribute(condition.attribute, context); + } if (condition.operator == ConditionOperator.IS_NULL) { - final Object value = resolveAttribute(condition.attribute, context); - boolean isNull = value == null; + boolean isNull = attributeValue == null; // condition.value determines if we're checking for null (true) or not null (false) boolean expectedNull = condition.value instanceof Boolean ? (Boolean) condition.value : true; return isNull == expectedNull; } - final Object attributeValue = resolveAttribute(condition.attribute, context); if (attributeValue == null) { return false; } @@ -623,6 +674,338 @@ private static Double parseDouble(final Object value) { return Double.parseDouble(String.valueOf(value)); } + private final class EvaluationState { + private final ServerConfiguration config; + private final EvaluationContext context; + private final boolean observeFullEvaluationData; + private final long evaluationTimestampMs; + private DependencyEvaluationSession session; + + private EvaluationState( + final ServerConfiguration config, + final EvaluationContext context, + final boolean observeFullEvaluationData, + final long evaluationTimestampMs) { + this.config = config; + this.context = context; + this.observeFullEvaluationData = observeFullEvaluationData; + this.evaluationTimestampMs = evaluationTimestampMs; + } + + private DependencyEvaluationSession dependencySession(final String rootFlagKey) { + if (session == null) { + session = new DependencyEvaluationSession(rootFlagKey, maxDependencyDepth(config), this); + } + return session; + } + + private ExposureCheckpoint checkpoint() { + return session == null ? ExposureCheckpoint.EMPTY : session.checkpoint(); + } + + private void rollback(final ExposureCheckpoint checkpoint) { + if (session != null) { + session.rollback(checkpoint); + } + } + + private void complete(final boolean commitExposures) { + if (session != null) { + session.complete(commitExposures); + } + } + } + + private final class DependencyEvaluationSession { + private final int maxDependencyDepth; + private final EvaluationState state; + private final Map memo = new HashMap<>(); + private final Set active = new HashSet<>(); + private final List frames = new ArrayList<>(); + private final List exposures = new ArrayList<>(); + private boolean completed; + + private DependencyEvaluationSession( + final String rootFlagKey, final int maxDependencyDepth, final EvaluationState state) { + this.maxDependencyDepth = maxDependencyDepth; + this.state = state; + active.add(rootFlagKey); + } + + private ExposureCheckpoint checkpoint() { + return new ExposureCheckpoint( + exposures.size(), + frames.isEmpty() ? 0 : frames.get(frames.size() - 1).exposureTrace.size()); + } + + private void rollback(final ExposureCheckpoint checkpoint) { + while (exposures.size() > checkpoint.exposureCount) { + exposures.remove(exposures.size() - 1); + } + if (!frames.isEmpty()) { + final DependencyFrame frame = frames.get(frames.size() - 1); + while (frame.exposureTrace.size() > checkpoint.frameExposureCount) { + frame.exposureTrace.remove(frame.exposureTrace.size() - 1); + } + } + } + + private void complete(final boolean commitExposures) { + if (completed) { + return; + } + completed = true; + if (commitExposures && !exposures.isEmpty()) { + try { + dependencyExposureSink.record(new ArrayList<>(exposures), state.context); + } catch (final RuntimeException | LinkageError ignored) { + // Telemetry is best effort and must not change evaluation semantics. + } + } + } + + private String resolveCurrent(final String flagKey) { + final int depth = frames.isEmpty() ? 1 : frames.get(frames.size() - 1).depth + 1; + return resolve(flagKey, depth); + } + + private String resolve(final String flagKey, final int depth) { + if (depth > maxDependencyDepth) { + if (!memo.containsKey(flagKey) && !active.contains(flagKey)) { + throw emitFailure(flagKey, ErrorCode.GENERAL, "maximum flag dependency depth exceeded"); + } + throw new DependencyGraphException( + ErrorCode.GENERAL, "maximum flag dependency depth exceeded"); + } + if (active.contains(flagKey)) { + throw new DependencyGraphException(ErrorCode.GENERAL, "flag dependency cycle detected"); + } + + final MemoizedDependency memoized = memo.get(flagKey); + if (memoized != null) { + if (depth + memoized.descendantDepth > maxDependencyDepth) { + throw new DependencyGraphException( + ErrorCode.GENERAL, "maximum flag dependency depth exceeded"); + } + recordReachedDepth(depth + memoized.descendantDepth); + recordExposureTrace(memoized.exposureTrace); + return memoized.details.getVariant(); + } + + final Flag flag = state.config.flags == null ? null : state.config.flags.get(flagKey); + if (flag == null) { + final ProviderEvaluation missing = + error( + null, + state.config.invalidFlags != null && state.config.invalidFlags.containsKey(flagKey) + ? ErrorCode.PARSE_ERROR + : ErrorCode.FLAG_NOT_FOUND, + "prerequisite flag is missing", + state.observeFullEvaluationData); + emitEvaluation(flagKey, missing); + throw new DependencyGraphException( + missing.getErrorCode(), + missing.getErrorCode() == ErrorCode.FLAG_NOT_FOUND + ? "prerequisite flag is missing" + : "invalid prerequisite flag configuration"); + } + + active.add(flagKey); + final DependencyFrame frame = new DependencyFrame(depth); + frames.add(frame); + try { + final ProviderEvaluation details = evaluateDeclaredFlag(state, flagKey, flag); + if (details.getErrorCode() != null || Reason.ERROR.name().equals(details.getReason())) { + throw new DependencyGraphException( + details.getErrorCode() == null ? ErrorCode.GENERAL : details.getErrorCode(), + details.getErrorMessage() == null + ? "prerequisite flag evaluation failed" + : details.getErrorMessage()); + } + if (details.getVariant() == null) { + throw new DependencyGraphException( + ErrorCode.GENERAL, "prerequisite flag evaluation returned no variant"); + } + final List exposureTrace = new ArrayList<>(frame.exposureTrace); + appendUnique(exposureTrace, new DependencyExposure(flagKey, details)); + final MemoizedDependency result = + new MemoizedDependency(details, frame.maxReachedDepth - depth, exposureTrace); + memo.put(flagKey, result); + emitEvaluation(flagKey, details); + recordReachedDepthOnParent(frame.maxReachedDepth); + recordExposureTraceOnParent(exposureTrace); + return details.getVariant(); + } catch (final DependencyGraphException e) { + final ProviderEvaluation failure = + error(null, e.errorCode, e.getMessage(), state.observeFullEvaluationData); + emitEvaluation(flagKey, failure); + throw e; + } catch (final PatternSyntaxException e) { + throw emitFailure(flagKey, ErrorCode.PARSE_ERROR, e.getMessage()); + } catch (final NumberFormatException e) { + throw emitFailure(flagKey, ErrorCode.TYPE_MISMATCH, e.getMessage()); + } catch (final Exception e) { + throw emitFailure(flagKey, ErrorCode.GENERAL, e.getMessage()); + } finally { + frames.remove(frames.size() - 1); + active.remove(flagKey); + } + } + + private DependencyGraphException emitFailure( + final String flagKey, final ErrorCode errorCode, final String message) { + final DependencyGraphException failure = + new DependencyGraphException(errorCode, message == null ? errorCode.name() : message); + emitEvaluation( + flagKey, error(null, errorCode, failure.getMessage(), state.observeFullEvaluationData)); + return failure; + } + + private void emitEvaluation(final String flagKey, final ProviderEvaluation details) { + try { + dependencyEvaluationSink.record( + flagKey, details, state.context, state.evaluationTimestampMs); + } catch (final RuntimeException | LinkageError ignored) { + // Telemetry is best effort and must not change evaluation semantics. + } + } + + private void recordReachedDepth(final int depth) { + if (!frames.isEmpty()) { + final DependencyFrame frame = frames.get(frames.size() - 1); + frame.maxReachedDepth = Math.max(frame.maxReachedDepth, depth); + } + } + + private void recordReachedDepthOnParent(final int depth) { + if (frames.size() > 1) { + final DependencyFrame parent = frames.get(frames.size() - 2); + parent.maxReachedDepth = Math.max(parent.maxReachedDepth, depth); + } + } + + private void recordExposureTrace(final List trace) { + for (final DependencyExposure candidate : trace) { + appendUnique(exposures, candidate); + if (!frames.isEmpty()) { + appendUnique(frames.get(frames.size() - 1).exposureTrace, candidate); + } + } + } + + private void recordExposureTraceOnParent(final List trace) { + for (final DependencyExposure candidate : trace) { + appendUnique(exposures, candidate); + if (frames.size() > 1) { + appendUnique(frames.get(frames.size() - 2).exposureTrace, candidate); + } + } + } + } + + private ProviderEvaluation evaluateDeclaredFlag( + final EvaluationState state, final String flagKey, final Flag flag) { + if (flag.variationType == null) { + throw new DependencyGraphException( + ErrorCode.PARSE_ERROR, "prerequisite flag has no variation type"); + } + switch (flag.variationType) { + case BOOLEAN: + return evaluateFlag(state, Boolean.class, flagKey, Boolean.FALSE); + case INTEGER: + return evaluateFlag(state, Integer.class, flagKey, 0); + case NUMERIC: + return evaluateFlag(state, Double.class, flagKey, 0D); + case STRING: + return evaluateFlag(state, String.class, flagKey, ""); + case JSON: + return evaluateFlag(state, Value.class, flagKey, new Value()); + default: + throw new DependencyGraphException( + ErrorCode.PARSE_ERROR, "unsupported prerequisite variation type"); + } + } + + private static void appendUnique( + final List target, final DependencyExposure candidate) { + for (final DependencyExposure existing : target) { + if (existing.flagKey.equals(candidate.flagKey)) { + return; + } + } + target.add(candidate); + } + + private static final class DependencyFrame { + private final int depth; + private int maxReachedDepth; + private final List exposureTrace = new ArrayList<>(); + + private DependencyFrame(final int depth) { + this.depth = depth; + this.maxReachedDepth = depth; + } + } + + private static final class ExposureCheckpoint { + private static final ExposureCheckpoint EMPTY = new ExposureCheckpoint(0, 0); + private final int exposureCount; + private final int frameExposureCount; + + private ExposureCheckpoint(final int exposureCount, final int frameExposureCount) { + this.exposureCount = exposureCount; + this.frameExposureCount = frameExposureCount; + } + } + + private static final class MemoizedDependency { + private final ProviderEvaluation details; + private final int descendantDepth; + private final List exposureTrace; + + private MemoizedDependency( + final ProviderEvaluation details, + final int descendantDepth, + final List exposureTrace) { + this.details = details; + this.descendantDepth = descendantDepth; + this.exposureTrace = exposureTrace; + } + } + + private static final class DependencyGraphException extends RuntimeException { + private final ErrorCode errorCode; + + private DependencyGraphException(final ErrorCode errorCode, final String message) { + super(message); + this.errorCode = errorCode; + } + } + + public static final class DependencyExposure { + public final String flagKey; + public final ProviderEvaluation details; + + private DependencyExposure(final String flagKey, final ProviderEvaluation details) { + this.flagKey = flagKey; + this.details = details; + } + } + + @FunctionalInterface + public interface DependencyEvaluationSink { + void record( + String flagKey, + ProviderEvaluation details, + EvaluationContext context, + long evaluationTimestampMs); + } + + @FunctionalInterface + public interface DependencyExposureSink { + void record(List exposures, EvaluationContext context); + } + /** Tells whether an exposure was already recorded, so it is not sent again. */ @FunctionalInterface public interface ExposureLookup { diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/ConditionConfiguration.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/ConditionConfiguration.java index c5d79cd67e1..74eb4d959da 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/ConditionConfiguration.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/ConditionConfiguration.java @@ -3,6 +3,7 @@ public final class ConditionConfiguration { public final ConditionOperator operator; public final String attribute; + public final String flagEvaluationKey; public final Object value; // The validated, parsed SemVer condition value. Set during configuration preprocessing @@ -11,8 +12,17 @@ public final class ConditionConfiguration { public ConditionConfiguration( final ConditionOperator operator, final String attribute, final Object value) { + this(operator, attribute, null, value); + } + + public ConditionConfiguration( + final ConditionOperator operator, + final String attribute, + final String flagEvaluationKey, + final Object value) { this.operator = operator; this.attribute = attribute; + this.flagEvaluationKey = flagEvaluationKey; this.value = value; } } diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/EvaluatorParams.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/EvaluatorParams.java new file mode 100644 index 00000000000..aef9e2b0440 --- /dev/null +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/EvaluatorParams.java @@ -0,0 +1,16 @@ +package com.datadog.openfeature.internal.ufc; + +/** Optional evaluator controls carried by UFC. */ +public final class EvaluatorParams { + public final Object maxDependencyDepth; + public final boolean maxDependencyDepthPresent; + + public EvaluatorParams(final Object maxDependencyDepth) { + this(maxDependencyDepth, maxDependencyDepth != null); + } + + public EvaluatorParams(final Object maxDependencyDepth, final boolean maxDependencyDepthPresent) { + this.maxDependencyDepth = maxDependencyDepth; + this.maxDependencyDepthPresent = maxDependencyDepthPresent; + } +} diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/ServerConfiguration.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/ServerConfiguration.java index 96aa6288154..2e645df3a82 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/ServerConfiguration.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/ServerConfiguration.java @@ -12,6 +12,7 @@ public final class ServerConfiguration { public final Environment environment; public final Map flags; + public final EvaluatorParams evaluatorParams; // Flags that could not be parsed or validated. The key is the flag key; the value is the error // type (e.g. "invalid_semver_comparand"). Set during configuration preprocessing, not from JSON. @@ -23,10 +24,21 @@ public ServerConfiguration( final Boolean observeFullEvaluationData, final Environment environment, final Map flags) { + this(createdAt, format, observeFullEvaluationData, environment, flags, null); + } + + public ServerConfiguration( + final String createdAt, + final String format, + final Boolean observeFullEvaluationData, + final Environment environment, + final Map flags, + final EvaluatorParams evaluatorParams) { this.createdAt = createdAt; this.format = format; this.observeFullEvaluationData = observeFullEvaluationData; this.environment = environment; this.flags = flags; + this.evaluatorParams = evaluatorParams; } } diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParser.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParser.java index 2721be5bfef..dadf8f63456 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParser.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParser.java @@ -149,7 +149,8 @@ private static ServerConfiguration toServerConfiguration(@Nullable final Object asString(json.get("format"), "format"), lenientBoolean(json.get("observeFullEvaluationData")), toEnvironment(json.get("environment")), - toFlags(json.get("flags"), invalidFlags)); + toFlags(json.get("flags"), invalidFlags), + toEvaluatorParams(json.get("evaluatorParams"))); if (!invalidFlags.isEmpty()) { configuration.invalidFlags = invalidFlags; } @@ -159,6 +160,16 @@ private static ServerConfiguration toServerConfiguration(@Nullable final Object } } + @Nullable + private static EvaluatorParams toEvaluatorParams(@Nullable final Object value) { + if (value == null) { + return null; + } + final Map json = asObject(value, "evaluatorParams"); + return new EvaluatorParams( + json.get("maxDependencyDepth"), json.containsKey("maxDependencyDepth")); + } + @Nullable private static Environment toEnvironment(@Nullable final Object value) { if (value == null) { @@ -260,10 +271,36 @@ private static ConditionConfiguration toCondition(@Nullable final Object value) return null; } final Map json = asObject(value, "condition"); - return new ConditionConfiguration( - asEnum(json.get("operator"), ConditionOperator.class, "operator"), - asString(json.get("attribute"), "attribute"), - json.get("value")); + final boolean hasAttribute = json.containsKey("attribute"); + final boolean hasFlagEvaluation = json.containsKey("flagEvaluation"); + if (hasAttribute == hasFlagEvaluation) { + throw new IllegalArgumentException( + "condition must contain exactly one of attribute or flagEvaluation"); + } + final String attribute = hasAttribute ? asString(json.get("attribute"), "attribute") : null; + final String flagEvaluationKey = + hasFlagEvaluation ? toFlagEvaluationKey(json.get("flagEvaluation")) : null; + final ConditionOperator operator = + asEnum(json.get("operator"), ConditionOperator.class, "operator"); + if (flagEvaluationKey != null + && operator != ConditionOperator.ONE_OF + && operator != ConditionOperator.NOT_ONE_OF) { + throw new IllegalArgumentException( + "flagEvaluation conditions support only ONE_OF and NOT_ONE_OF"); + } + return new ConditionConfiguration(operator, attribute, flagEvaluationKey, json.get("value")); + } + + private static String toFlagEvaluationKey(@Nullable final Object value) { + final Map json = asObject(value, "flagEvaluation"); + if (json.size() != 1 || !json.containsKey("key")) { + throw new IllegalArgumentException("flagEvaluation must contain only key"); + } + final String key = asString(json.get("key"), "flagEvaluation.key"); + if (key.isEmpty()) { + throw new IllegalArgumentException("flagEvaluation.key must not be empty"); + } + return key; } @Nullable @@ -551,6 +588,12 @@ private static void validateConditionOperands(final String flagKey, final Alloca throw new IllegalArgumentException( "flag \"" + flagKey + "\" has a non-list condition"); } + for (final Object member : (List) condition.value) { + if (!(member instanceof String)) { + throw new IllegalArgumentException( + "flag \"" + flagKey + "\" has a non-string membership operand"); + } + } break; case IS_NULL: if (!(condition.value instanceof Boolean)) { diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/EvaluationTelemetryOptOutTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/EvaluationTelemetryOptOutTest.java index b85b8dff282..788840113b7 100644 --- a/dd-openfeature/src/test/java/com/datadog/openfeature/EvaluationTelemetryOptOutTest.java +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/EvaluationTelemetryOptOutTest.java @@ -508,6 +508,60 @@ public void after( assertEquals(2, this.customerEvaluations.size()); } + @Test + void dependencyTelemetryHonorsEnabledProviderOptOut() throws Exception { + start( + new Provider.Options().enableEvaluationTelemetryOptOutParameter(true), + dependentConfiguration()); + + final FlagEvaluationDetails optedOut = + this.client.getIntegerDetails("root", 0, context("subject-1", new Value(true))); + + assertEquals(1, optedOut.getValue()); + assertTelemetryCounts(0); + assertEquals(2, this.exposures.size(), "dependency and root exposures remain independent"); + } + + @Test + void dependencyTelemetryIncludesMetricsWhenProviderOptOutIsDisabled() throws Exception { + start( + new Provider.Options().enableEvaluationTelemetryOptOutParameter(false), + dependentConfiguration()); + + final FlagEvaluationDetails included = + this.client.getIntegerDetails("root", 0, context("subject-2", new Value(true))); + + assertEquals(1, included.getValue()); + assertEquals(2, this.events.size(), "dependency and root evaluation events"); + assertEquals(2, this.metrics.get(), "dependency and root OTel metrics"); + assertEquals(1, this.spans.size(), "only the ordinary root span hook runs"); + assertEquals(2, this.exposures.size()); + } + + @Test + void failingAfterHookDiscardsBufferedDependencyExposures() throws Exception { + start( + new Provider.Options().enableEvaluationTelemetryOptOutParameter(false), + dependentConfiguration()); + this.api.addHooks( + new Hook() { + @Override + public void after( + final HookContext context, + final FlagEvaluationDetails details, + final Map hints) { + throw new IllegalStateException("application hook failed"); + } + }); + + final FlagEvaluationDetails details = + this.client.getIntegerDetails("root", 0, context("subject", new Value(false))); + + assertEquals(ErrorCode.GENERAL, details.getErrorCode()); + assertEquals(0, details.getValue()); + assertEquals(0, this.exposures.size()); + } + @Test void concurrentSubjectsCannotChangeEachOthersDecision() throws Exception { start(); @@ -604,7 +658,12 @@ private void start() throws Exception { } private void start(final Provider.Options options) throws Exception { - this.evaluator = new CapturingEvaluator(defaultConfiguration()); + start(options, defaultConfiguration()); + } + + private void start(final Provider.Options options, final ServerConfiguration configuration) + throws Exception { + this.evaluator = new CapturingEvaluator(configuration); this.api.setProviderAndWait(provider(options, this.evaluator)); this.client = this.api.getClient(); } @@ -651,6 +710,48 @@ private static ServerConfiguration defaultConfiguration() { return new ServerConfiguration("", "", false, null, singletonMap("flag", flag)); } + private static ServerConfiguration dependentConfiguration() { + final Allocation dependencyAllocation = + new Allocation( + "dependency-allocation", + null, + null, + null, + singletonList(new Split(emptyList(), "on", emptyMap(), 7)), + true); + final Flag dependency = + new Flag( + "dependency", + true, + ValueType.INTEGER, + singletonMap("on", new Variant("on", 1)), + singletonList(dependencyAllocation)); + final Rule rule = + new Rule( + singletonList( + new ConditionConfiguration( + ConditionOperator.ONE_OF, null, "dependency", singletonList("on")))); + final Allocation rootAllocation = + new Allocation( + "root-allocation", + singletonList(rule), + null, + null, + singletonList(new Split(emptyList(), "on", emptyMap(), 7)), + true); + final Flag root = + new Flag( + "root", + true, + ValueType.INTEGER, + singletonMap("on", new Variant("on", 1)), + singletonList(rootAllocation)); + final Map flags = new HashMap<>(); + flags.put("dependency", dependency); + flags.put("root", root); + return new ServerConfiguration("", "", false, null, flags); + } + private static ServerConfiguration targetingConfiguration( final boolean observeFullEvaluationData) { final Rule rule = diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/CdnConfigurationSourceTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/CdnConfigurationSourceTest.java index 9dc8168dd5c..f241fbc03ff 100644 --- a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/CdnConfigurationSourceTest.java +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/CdnConfigurationSourceTest.java @@ -162,6 +162,9 @@ void sendsApiKeyAndMetadataHeaders() throws Exception { assertEquals("GET", request.method); assertEquals("secret", request.header("DD-API-KEY")); assertEquals("gzip", request.header("Accept-Encoding")); + assertEquals( + CdnConfigurationSource.DEPENDENT_FLAG_CAPABILITY, + request.header(CdnConfigurationSource.FEATURE_FLAGS_CAPABILITIES_HEADER)); assertEquals("java", request.header("Datadog-Meta-Lang")); assertNull(request.header("If-None-Match")); } diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java new file mode 100644 index 00000000000..1f8d24dd544 --- /dev/null +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java @@ -0,0 +1,405 @@ +package com.datadog.openfeature.internal.evaluation; + +import static com.datadog.openfeature.internal.ufc.ConditionOperator.NOT_ONE_OF; +import static com.datadog.openfeature.internal.ufc.ConditionOperator.ONE_OF; +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.datadog.openfeature.internal.ufc.Allocation; +import com.datadog.openfeature.internal.ufc.ConditionConfiguration; +import com.datadog.openfeature.internal.ufc.EvaluatorParams; +import com.datadog.openfeature.internal.ufc.Flag; +import com.datadog.openfeature.internal.ufc.Rule; +import com.datadog.openfeature.internal.ufc.ServerConfiguration; +import com.datadog.openfeature.internal.ufc.Split; +import com.datadog.openfeature.internal.ufc.UniversalFlagConfigParser; +import com.datadog.openfeature.internal.ufc.ValueType; +import com.datadog.openfeature.internal.ufc.Variant; +import dev.openfeature.sdk.ErrorCode; +import dev.openfeature.sdk.MutableContext; +import dev.openfeature.sdk.ProviderEvaluation; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class DependentFlagEvaluatorTest { + + @Test + void memoizesDependenciesAndReplaysTheirExposureAfterRuleRollback() { + final List evaluations = new ArrayList<>(); + final List exposures = new ArrayList<>(); + final FlagEvaluator evaluator = + evaluator( + evaluations, + (items, context) -> { + for (final FlagEvaluator.DependencyExposure item : items) { + exposures.add(item.flagKey); + } + }); + final Map flags = new HashMap<>(); + flags.put("dependency", stringFlag("dependency", "on", null, true)); + flags.put( + "root", + integerFlag( + "root", + Arrays.asList( + allocation("rejected", dependencyRule("dependency", "off"), "selected", false), + allocation("accepted", dependencyRule("dependency", "on"), "selected", false)))); + + final ProviderEvaluation result = + evaluator.evaluate(config(flags, null), Integer.class, "root", 0, new MutableContext("u")); + + assertEquals(1, result.getValue()); + assertEquals(singletonList("dependency:on"), evaluations); + assertEquals(singletonList("dependency"), exposures); + } + + @Test + void rollsBackDependencyExposuresWhenTheRootEvaluationFails() { + final List evaluations = new ArrayList<>(); + final List> exposureCommits = new ArrayList<>(); + final FlagEvaluator evaluator = + evaluator(evaluations, (items, context) -> exposureCommits.add(items)); + final Map flags = new HashMap<>(); + flags.put("dependency", stringFlag("dependency", "on", null, true)); + flags.put( + "root", + integerFlag( + "root", + singletonList( + allocation("broken", dependencyRule("dependency", "on"), "missing", false)))); + + final ProviderEvaluation result = + evaluator.evaluate(config(flags, null), Integer.class, "root", 0, new MutableContext("u")); + + assertEquals(ErrorCode.GENERAL, result.getErrorCode()); + assertEquals(singletonList("dependency:on"), evaluations); + assertTrue(exposureCommits.isEmpty()); + } + + @Test + void propagatesCyclesAndEmitsAnErrorForEveryReachedDependency() { + final List evaluations = new ArrayList<>(); + final FlagEvaluator evaluator = evaluator(evaluations, (items, context) -> {}); + final Map flags = new HashMap<>(); + flags.put("a", stringFlag("a", "on", dependencyRule("b", "on"), false)); + flags.put("b", stringFlag("b", "on", dependencyRule("a", "on"), false)); + + final ProviderEvaluation result = + evaluator.evaluate( + config(flags, null), String.class, "a", "default", new MutableContext("u")); + + assertEquals(ErrorCode.GENERAL, result.getErrorCode()); + assertEquals(singletonList("b:GENERAL"), evaluations); + } + + @Test + void enforcesTheConfiguredMaximumDependencyDepth() { + final List evaluations = new ArrayList<>(); + final FlagEvaluator evaluator = evaluator(evaluations, (items, context) -> {}); + final Map flags = new HashMap<>(); + flags.put("root", stringFlag("root", "on", dependencyRule("a", "on"), false)); + flags.put("a", stringFlag("a", "on", dependencyRule("b", "on"), false)); + flags.put("b", stringFlag("b", "on", null, false)); + + final ProviderEvaluation result = + evaluator.evaluate( + config(flags, new EvaluatorParams(1D)), + String.class, + "root", + "default", + new MutableContext("u")); + + assertEquals(ErrorCode.GENERAL, result.getErrorCode()); + assertEquals(Arrays.asList("b:GENERAL", "a:GENERAL"), evaluations); + } + + @Test + void memoizedSubtreeDepthFailureDoesNotReemitMemoizedNodeInEitherOrder() { + for (final List rootDependencies : + Arrays.asList(Arrays.asList("a", "b"), Arrays.asList("b", "a"))) { + final List evaluations = new ArrayList<>(); + final List exposures = new ArrayList<>(); + final FlagEvaluator evaluator = + evaluator( + evaluations, + (items, context) -> { + for (final FlagEvaluator.DependencyExposure item : items) { + exposures.add(item.flagKey); + } + }); + final Map flags = new HashMap<>(); + flags.put("leaf", stringFlag("leaf", "on", null, true)); + flags.put("a", stringFlag("a", "on", dependencyRule("leaf", "on"), true)); + flags.put("b", stringFlag("b", "on", dependencyRule("a", "on"), true)); + final Rule rootRule = + new Rule( + Arrays.asList( + new ConditionConfiguration( + ONE_OF, null, rootDependencies.get(0), singletonList("on")), + new ConditionConfiguration( + ONE_OF, null, rootDependencies.get(1), singletonList("on")))); + flags.put("root", stringFlag("root", "on", rootRule, true)); + + final ProviderEvaluation result = + evaluator.evaluate( + config(flags, new EvaluatorParams(2D)), + String.class, + "root", + "default", + new MutableContext("u")); + + assertEquals(ErrorCode.GENERAL, result.getErrorCode(), rootDependencies.toString()); + final List evaluationKeys = new ArrayList<>(); + for (final String evaluation : evaluations) { + evaluationKeys.add(evaluation.substring(0, evaluation.indexOf(':'))); + } + assertEquals(Arrays.asList("leaf", "a", "b"), evaluationKeys); + assertTrue(exposures.isEmpty()); + } + } + + @Test + void validatesInvalidMaximumDependencyDepthOnlyWhenADependencyIsReached() { + final FlagEvaluator evaluator = evaluator(new ArrayList<>(), (items, context) -> {}); + final Map flags = new HashMap<>(); + flags.put("ordinary", stringFlag("ordinary", "on", null, false)); + flags.put("dependency", stringFlag("dependency", "on", null, false)); + flags.put( + "dependent", stringFlag("dependent", "on", dependencyRule("dependency", "on"), false)); + final ServerConfiguration config = config(flags, new EvaluatorParams(1.5D)); + + final ProviderEvaluation ordinary = + evaluator.evaluate(config, String.class, "ordinary", "default", new MutableContext("u")); + final ProviderEvaluation dependent = + evaluator.evaluate(config, String.class, "dependent", "default", new MutableContext("u")); + + assertEquals("on-value", ordinary.getValue()); + assertEquals(ErrorCode.PARSE_ERROR, dependent.getErrorCode()); + assertNotNull(dependent.getFlagMetadata()); + } + + @Test + void rejectsExplicitNullMaximumDependencyDepthButDefaultsAnEmptyObject() { + final Map flags = new HashMap<>(); + flags.put("ordinary", stringFlag("ordinary", "on", null, false)); + flags.put("dependency", stringFlag("dependency", "on", null, false)); + flags.put( + "dependent", stringFlag("dependent", "on", dependencyRule("dependency", "on"), false)); + final FlagEvaluator evaluator = evaluator(new ArrayList<>(), (items, context) -> {}); + + assertEquals( + "on-value", + evaluator + .evaluate( + config(flags, new EvaluatorParams(null, true)), + String.class, + "ordinary", + "default", + new MutableContext("u")) + .getValue()); + assertEquals( + ErrorCode.PARSE_ERROR, + evaluator + .evaluate( + config(flags, new EvaluatorParams(null, true)), + String.class, + "dependent", + "default", + new MutableContext("u")) + .getErrorCode()); + assertEquals( + "on-value", + evaluator + .evaluate( + config(flags, new EvaluatorParams(null, false)), + String.class, + "dependent", + "default", + new MutableContext("u")) + .getValue()); + } + + @Test + void rejectsNonStringDependencyMembershipOperandsWithParseError() throws Exception { + final ServerConfiguration config = + UniversalFlagConfigParser.parse( + ("{\"flags\":{\"root\":{\"key\":\"root\",\"enabled\":true," + + "\"variationType\":\"STRING\",\"variations\":{\"on\":{\"key\":\"on\"," + + "\"value\":\"on-value\"}},\"allocations\":[{\"key\":\"allocation\"," + + "\"rules\":[{\"conditions\":[{\"operator\":\"ONE_OF\"," + + "\"flagEvaluation\":{\"key\":\"dependency\"},\"value\":[\"on\",1]}]}]," + + "\"splits\":[{\"variationKey\":\"on\",\"shards\":[]}]}]}}}") + .getBytes(UTF_8)); + + final ProviderEvaluation result = + evaluator(new ArrayList<>(), (items, context) -> {}) + .evaluate(config, String.class, "root", "default", new MutableContext("u")); + + assertEquals(ErrorCode.PARSE_ERROR, result.getErrorCode()); + } + + @Test + void supportsNotOneOfAndShortCircuitsBeforeAFatalDependency() { + final List evaluations = new ArrayList<>(); + final FlagEvaluator evaluator = evaluator(evaluations, (items, context) -> {}); + final Map flags = new HashMap<>(); + flags.put("dependency", stringFlag("dependency", "on", null, false)); + final Rule rejectedBeforeMissing = + new Rule( + Arrays.asList( + new ConditionConfiguration(ONE_OF, "gate", null, singletonList("open")), + new ConditionConfiguration(ONE_OF, null, "missing", singletonList("on")))); + flags.put( + "root", + integerFlag( + "root", + Arrays.asList( + allocation("short-circuit", rejectedBeforeMissing, "selected", false), + allocation( + "not-one-of", + new Rule( + singletonList( + new ConditionConfiguration( + NOT_ONE_OF, null, "dependency", singletonList("off")))), + "selected", + false)))); + final MutableContext context = new MutableContext("u"); + context.add("gate", "closed"); + + final ProviderEvaluation result = + evaluator.evaluate(config(flags, null), Integer.class, "root", 0, context); + + assertEquals(1, result.getValue()); + assertEquals(singletonList("dependency:on"), evaluations); + } + + @Test + void emitsMissingAndInvalidPrerequisiteErrors() { + final List missing = new ArrayList<>(); + final Map missingFlags = new HashMap<>(); + missingFlags.put("root", stringFlag("root", "on", dependencyRule("missing", "on"), false)); + assertEquals( + ErrorCode.FLAG_NOT_FOUND, + evaluator(missing, (items, context) -> {}) + .evaluate( + config(missingFlags, null), + String.class, + "root", + "default", + new MutableContext("u")) + .getErrorCode()); + assertEquals(singletonList("missing:FLAG_NOT_FOUND"), missing); + + final List invalid = new ArrayList<>(); + final ServerConfiguration invalidConfig = config(missingFlags, null); + invalidConfig.invalidFlags = singletonMap("missing", "invalid_flag"); + assertEquals( + ErrorCode.PARSE_ERROR, + evaluator(invalid, (items, context) -> {}) + .evaluate(invalidConfig, String.class, "root", "default", new MutableContext("u")) + .getErrorCode()); + assertEquals(singletonList("missing:PARSE_ERROR"), invalid); + } + + @Test + void rollsBackATransitiveTraceWhenTheRootAllocationHasNoAssignment() { + final List evaluations = new ArrayList<>(); + final List exposures = new ArrayList<>(); + final FlagEvaluator evaluator = + evaluator( + evaluations, + (items, context) -> { + for (final FlagEvaluator.DependencyExposure item : items) { + exposures.add(item.flagKey); + } + }); + final Map flags = new HashMap<>(); + flags.put("leaf", stringFlag("leaf", "on", null, true)); + flags.put("parent", stringFlag("parent", "on", dependencyRule("leaf", "on"), true)); + final Allocation noAssignment = + new Allocation( + "no-assignment", + singletonList(dependencyRule("parent", "on")), + null, + null, + emptyList(), + false); + flags.put( + "root", + integerFlag( + "root", Arrays.asList(noAssignment, allocation("fallback", null, "selected", false)))); + + final ProviderEvaluation result = + evaluator.evaluate(config(flags, null), Integer.class, "root", 0, new MutableContext("u")); + + assertEquals(1, result.getValue()); + assertEquals(Arrays.asList("leaf:on", "parent:on"), evaluations); + assertTrue(exposures.isEmpty()); + } + + private static FlagEvaluator evaluator( + final List evaluations, final FlagEvaluator.DependencyExposureSink exposureSink) { + return new FlagEvaluator( + false, + (flag, subject, allocation, variant, serialId) -> false, + (flag, details, context, timestamp) -> + evaluations.add( + flag + + ":" + + (details.getErrorCode() == null + ? details.getVariant() + : details.getErrorCode().name())), + exposureSink); + } + + private static ServerConfiguration config( + final Map flags, final EvaluatorParams params) { + return new ServerConfiguration("", "", false, null, flags, params); + } + + private static Flag stringFlag( + final String key, final String variant, final Rule rule, final boolean doLog) { + final Map variations = new HashMap<>(); + variations.put(variant, new Variant(variant, variant + "-value")); + return new Flag( + key, + true, + ValueType.STRING, + variations, + singletonList(allocation(key + "-allocation", rule, variant, doLog))); + } + + private static Flag integerFlag(final String key, final List allocations) { + final Map variations = new HashMap<>(); + variations.put("selected", new Variant("selected", 1)); + return new Flag(key, true, ValueType.INTEGER, variations, allocations); + } + + private static Allocation allocation( + final String key, final Rule rule, final String variant, final boolean doLog) { + final List rules = rule == null ? null : singletonList(rule); + return new Allocation( + key, + rules, + null, + null, + singletonList(new Split(emptyList(), variant, emptyMap(), 1)), + doLog); + } + + private static Rule dependencyRule(final String flagKey, final String expectedVariant) { + return new Rule( + singletonList( + new ConditionConfiguration(ONE_OF, null, flagKey, singletonList(expectedVariant)))); + } +} diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParserJsonApiTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParserJsonApiTest.java index 8a7af915e14..77925f26dad 100644 --- a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParserJsonApiTest.java +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParserJsonApiTest.java @@ -119,13 +119,14 @@ void preprocessesSemverComparandsAndDropsMalformedFlags() throws Exception { assertTrue(configuration.flags.containsKey("null-split")); assertTrue(configuration.flags.containsKey("no-rules")); assertTrue(configuration.flags.containsKey("no-conditions")); - assertTrue(configuration.flags.containsKey("no-operator")); + assertFalse(configuration.flags.containsKey("no-operator")); assertTrue(configuration.flags.containsKey("non-semver")); assertTrue(configuration.flags.containsKey("valid-semver")); assertFalse(configuration.flags.containsKey("invalid-semver")); assertFalse(configuration.flags.containsKey("non-string-semver")); assertFalse(configuration.flags.containsKey("null-flag")); - assertEquals(2, configuration.invalidFlags.size()); + assertEquals(3, configuration.invalidFlags.size()); + assertEquals("invalid_flag", configuration.invalidFlags.get("no-operator")); assertEquals("invalid_semver_comparand", configuration.invalidFlags.get("invalid-semver")); assertEquals("invalid_semver_comparand", configuration.invalidFlags.get("non-string-semver")); diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParserTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParserTest.java index 5afe68fa544..7d56ff88616 100644 --- a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParserTest.java +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParserTest.java @@ -28,6 +28,50 @@ void parsesEmptyConfiguration() throws Exception { assertTrue(config.invalidFlags.isEmpty()); } + @Test + void parsesDependentFlagConditionsAndEvaluatorParams() throws Exception { + final ServerConfiguration config = + deserialize( + "{\"evaluatorParams\":{\"maxDependencyDepth\":4},\"flags\":{\"root\":{" + + "\"key\":\"root\",\"allocations\":[{\"key\":\"a\",\"rules\":[{" + + "\"conditions\":[{\"operator\":\"ONE_OF\"," + + "\"flagEvaluation\":{\"key\":\"dependency\"},\"value\":[\"on\"]}]}]," + + "\"splits\":[]}]}}}"); + + assertEquals(4.0, config.evaluatorParams.maxDependencyDepth); + final ConditionConfiguration condition = + config.flags.get("root").allocations.get(0).rules.get(0).conditions.get(0); + assertNull(condition.attribute); + assertEquals("dependency", condition.flagEvaluationKey); + assertEquals(ConditionOperator.ONE_OF, condition.operator); + } + + @Test + void distinguishesAbsentEmptyAndExplicitNullMaximumDependencyDepth() throws Exception { + final ServerConfiguration absent = deserialize(emptyConfig()); + final ServerConfiguration empty = deserialize("{\"evaluatorParams\":{},\"flags\":{}}"); + final ServerConfiguration explicitNull = + deserialize("{\"evaluatorParams\":{\"maxDependencyDepth\":null},\"flags\":{}}"); + + assertNull(absent.evaluatorParams); + assertFalse(empty.evaluatorParams.maxDependencyDepthPresent); + assertTrue(explicitNull.evaluatorParams.maxDependencyDepthPresent); + assertNull(explicitNull.evaluatorParams.maxDependencyDepth); + } + + @Test + void rejectsConditionsWithBothAttributeAndFlagEvaluation() throws Exception { + final ServerConfiguration config = + deserialize( + "{\"flags\":{\"bad\":{\"key\":\"bad\",\"allocations\":[{\"key\":\"a\"," + + "\"rules\":[{\"conditions\":[{\"operator\":\"ONE_OF\"," + + "\"attribute\":\"plan\",\"flagEvaluation\":{\"key\":\"dependency\"}," + + "\"value\":[\"on\"]}]}],\"splits\":[]}]}}}"); + + assertFalse(config.flags.containsKey("bad")); + assertEquals("invalid_flag", config.invalidFlags.get("bad")); + } + @Test void parsesNullDocumentAsNoConfiguration() throws Exception { assertNull(deserialize("null")); From 9a329ce5b5545765058800b9a642c785a78856ae Mon Sep 17 00:00:00 2001 From: Tyler Potter Date: Mon, 2 Nov 2026 10:27:26 -0700 Subject: [PATCH 2/4] refactor(openfeature): return dependency telemetry batches Publish detached prerequisite evaluations only after graph completion while keeping dependency exposures transactional. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 --- .../com/datadog/openfeature/DDEvaluator.java | 55 +++--- .../internal/evaluation/FlagEvaluator.java | 150 +++++++++------ .../EvaluationTelemetryOptOutTest.java | 74 ++++++++ .../DependentFlagEvaluatorTest.java | 177 ++++++++---------- 4 files changed, 267 insertions(+), 189 deletions(-) diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java b/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java index 8d1066862ac..3d851d58f0e 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java @@ -97,8 +97,6 @@ class DDEvaluator implements Evaluator, Consumer { private final Settings settings; private final FlagEvaluator core; private final ConfigurationStore configuration = new ConfigurationStore(); - private final ThreadLocal> stagedDependencyExposures = - new ThreadLocal<>(); private final AtomicLong nextDependencyExposureToken = new AtomicLong(); private final Map> pendingDependencyExposures = new ConcurrentHashMap<>(); @@ -112,12 +110,7 @@ class DDEvaluator implements Evaluator, Consumer { this.settings = settings; // When span enrichment is off, the __dd_* span-enrichment metadata is attached only to // evaluations of allocations logging exposures, for the exposure hook. - this.core = - new FlagEvaluator( - settings.isSpanEnrichmentEnabled(), - this::isExposed, - this::recordDependencyEvaluation, - (items, context) -> stagedDependencyExposures.set(items)); + this.core = new FlagEvaluator(settings.isSpanEnrichmentEnabled(), this::isExposed); } void configureDependencyTelemetry(final boolean optOutEnabled, final FlagEvalMetrics metrics) { @@ -180,11 +173,17 @@ public ProviderEvaluation evaluate( final String key, final T defaultValue, final EvaluationContext context) { - stagedDependencyExposures.remove(); - final ProviderEvaluation result = - core.evaluate(configuration.get(), target, key, defaultValue, context); - final List staged = stagedDependencyExposures.get(); - stagedDependencyExposures.remove(); + final FlagEvaluator.EvaluationResult evaluation = + core.evaluateWithDependencies(configuration.get(), target, key, defaultValue, context); + for (final FlagEvaluator.DependencyEvaluation dependency : evaluation.dependencyEvaluations) { + try { + publishDependencyEvaluation(dependency, context, evaluation.evaluationTimestampMs); + } catch (final RuntimeException | LinkageError ignored) { + // Telemetry is best effort and must not change the root evaluation. + } + } + final ProviderEvaluation result = evaluation.details; + final List staged = evaluation.dependencyExposures; if (staged == null || staged.isEmpty() || result.getErrorCode() != null) { return result; } @@ -277,15 +276,14 @@ void recordExposure(final ExposureEvent event) { } } - private void recordDependencyEvaluation( - final String flagKey, - final ProviderEvaluation details, + void publishDependencyEvaluation( + final FlagEvaluator.DependencyEvaluation dependency, final EvaluationContext context, final long evaluationTimestampMs) { if (dependencyTelemetryOptOutEnabled && isEvaluationTelemetryOptedOut(context)) { return; } - final ImmutableMetadata metadata = details.getFlagMetadata(); + final ImmutableMetadata metadata = dependency.metadata; final Map values = metadata == null ? Collections.emptyMap() : metadata.asUnmodifiableMap(); final Object allocationValue = values.get(ALLOCATION_KEY); @@ -294,10 +292,10 @@ private void recordDependencyEvaluation( final FlagEvalMetrics metrics = dependencyMetrics; if (metrics != null) { metrics.record( - flagKey, - details.getVariant(), - details.getReason(), - details.getErrorCode(), + dependency.flagKey, + dependency.variant, + dependency.reason, + dependency.errorCode, allocationKey); } final FeatureFlagsRuntime current = runtime(); @@ -310,9 +308,9 @@ private void recordDependencyEvaluation( } final boolean observeFullEvaluationData = Boolean.TRUE.equals(values.get(OBSERVE_FULL_EVALUATION_DATA)); - String errorMessage = observeFullEvaluationData ? details.getErrorMessage() : null; - if ((errorMessage == null || errorMessage.isEmpty()) && details.getErrorCode() != null) { - errorMessage = details.getErrorCode().name(); + String errorMessage = observeFullEvaluationData ? dependency.errorMessage : null; + if ((errorMessage == null || errorMessage.isEmpty()) && dependency.errorCode != null) { + errorMessage = dependency.errorCode.name(); } final Map attrs; if (observeFullEvaluationData) { @@ -330,8 +328,8 @@ private void recordDependencyEvaluation( : Collections.emptyMap(); writer.enqueue( new FlagEvalEvent( - flagKey, - details.getVariant(), + dependency.flagKey, + dependency.variant, allocationKey, context == null ? null : context.getTargetingKey(), errorMessage, @@ -363,8 +361,7 @@ void recordDependencyExposures( return; } for (final FlagEvaluator.DependencyExposure exposure : exposures) { - final ProviderEvaluation details = exposure.details; - final ImmutableMetadata metadata = details.getFlagMetadata(); + final ImmutableMetadata metadata = exposure.metadata; final Map values = metadata == null ? Collections.emptyMap() : metadata.asUnmodifiableMap(); if (!Boolean.TRUE.equals(values.get(DO_LOG))) { @@ -373,7 +370,7 @@ void recordDependencyExposures( final Object allocationValue = values.get(ALLOCATION_KEY); final String allocationKey = allocationValue instanceof String ? (String) allocationValue : null; - final String variantKey = details.getVariant(); + final String variantKey = exposure.variant; if (allocationKey == null || variantKey == null) { continue; } diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java index b8a389b3c20..cbbced47bc5 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java @@ -11,6 +11,8 @@ import static com.datadog.openfeature.internal.evaluation.MetadataKeys.OBSERVE_FULL_EVALUATION_DATA; import static com.datadog.openfeature.internal.evaluation.MetadataKeys.SPLIT_SERIAL_ID; import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; +import static java.util.Collections.unmodifiableList; import com.datadog.openfeature.internal.ufc.Allocation; import com.datadog.openfeature.internal.ufc.ConditionConfiguration; @@ -63,8 +65,6 @@ public final class FlagEvaluator { static final Set WARNED_FEATURE_DESTINATIONS = ConcurrentHashMap.newKeySet(); private final boolean spanEnrichmentEnabled; private final ExposureLookup exposureLookup; - private final DependencyEvaluationSink dependencyEvaluationSink; - private final DependencyExposureSink dependencyExposureSink; /** * Creates an evaluator. @@ -73,22 +73,8 @@ public final class FlagEvaluator { * @param exposureLookup the lookup telling whether an exposure was already recorded. */ public FlagEvaluator(final boolean spanEnrichmentEnabled, final ExposureLookup exposureLookup) { - this( - spanEnrichmentEnabled, - exposureLookup, - (flag, details, context, timestamp) -> {}, - (items, context) -> {}); - } - - public FlagEvaluator( - final boolean spanEnrichmentEnabled, - final ExposureLookup exposureLookup, - final DependencyEvaluationSink dependencyEvaluationSink, - final DependencyExposureSink dependencyExposureSink) { this.spanEnrichmentEnabled = spanEnrichmentEnabled; this.exposureLookup = exposureLookup; - this.dependencyEvaluationSink = dependencyEvaluationSink; - this.dependencyExposureSink = dependencyExposureSink; } public ProviderEvaluation evaluate( @@ -97,6 +83,16 @@ public ProviderEvaluation evaluate( final String key, final T defaultValue, final EvaluationContext context) { + return evaluateWithDependencies(config, target, key, defaultValue, context).details; + } + + /** Evaluates a root flag and returns all telemetry candidates produced by its graph. */ + public EvaluationResult evaluateWithDependencies( + final ServerConfiguration config, + final Class target, + 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. @@ -106,24 +102,27 @@ public ProviderEvaluation evaluate( config != null && Boolean.TRUE.equals(config.observeFullEvaluationData); final EvaluationState state = new EvaluationState(config, context, observeFullEvaluationData, System.currentTimeMillis()); + ProviderEvaluation result; + boolean commitExposures; try { - final ProviderEvaluation result = evaluateFlag(state, target, key, defaultValue); - state.complete(result.getErrorCode() == null); - return result; + result = evaluateFlag(state, target, key, defaultValue); + commitExposures = result.getErrorCode() == null; } catch (final DependencyGraphException e) { - state.complete(false); - return error(defaultValue, e.errorCode, e.getMessage(), observeFullEvaluationData); + result = error(defaultValue, e.errorCode, e.getMessage(), observeFullEvaluationData); + commitExposures = false; } catch (final PatternSyntaxException e) { - state.complete(false); - return error(defaultValue, ErrorCode.PARSE_ERROR, e.getMessage(), observeFullEvaluationData); + result = + error(defaultValue, ErrorCode.PARSE_ERROR, e.getMessage(), observeFullEvaluationData); + commitExposures = false; } catch (final NumberFormatException e) { - state.complete(false); - return error( - defaultValue, ErrorCode.TYPE_MISMATCH, e.getMessage(), observeFullEvaluationData); + result = + error(defaultValue, ErrorCode.TYPE_MISMATCH, e.getMessage(), observeFullEvaluationData); + commitExposures = false; } catch (final Exception e) { - state.complete(false); - return error(defaultValue, ErrorCode.GENERAL, e.getMessage(), observeFullEvaluationData); + result = error(defaultValue, ErrorCode.GENERAL, e.getMessage(), observeFullEvaluationData); + commitExposures = false; } + return state.complete(result, commitExposures); } private ProviderEvaluation evaluateFlag( @@ -709,10 +708,11 @@ private void rollback(final ExposureCheckpoint checkpoint) { } } - private void complete(final boolean commitExposures) { - if (session != null) { - session.complete(commitExposures); - } + private EvaluationResult complete( + final ProviderEvaluation details, final boolean commitExposures) { + return session == null + ? new EvaluationResult<>(details, emptyList(), emptyList(), evaluationTimestampMs) + : session.complete(details, commitExposures); } } @@ -722,8 +722,9 @@ private final class DependencyEvaluationSession { private final Map memo = new HashMap<>(); private final Set active = new HashSet<>(); private final List frames = new ArrayList<>(); + private final Set evaluatedFlags = new HashSet<>(); + private final List evaluations = new ArrayList<>(); private final List exposures = new ArrayList<>(); - private boolean completed; private DependencyEvaluationSession( final String rootFlagKey, final int maxDependencyDepth, final EvaluationState state) { @@ -750,18 +751,13 @@ private void rollback(final ExposureCheckpoint checkpoint) { } } - private void complete(final boolean commitExposures) { - if (completed) { - return; - } - completed = true; - if (commitExposures && !exposures.isEmpty()) { - try { - dependencyExposureSink.record(new ArrayList<>(exposures), state.context); - } catch (final RuntimeException | LinkageError ignored) { - // Telemetry is best effort and must not change evaluation semantics. - } - } + private EvaluationResult complete( + final ProviderEvaluation details, final boolean commitExposures) { + return new EvaluationResult<>( + details, + evaluations, + commitExposures ? exposures : emptyList(), + state.evaluationTimestampMs); } private String resolveCurrent(final String flagKey) { @@ -862,12 +858,10 @@ private DependencyGraphException emitFailure( } private void emitEvaluation(final String flagKey, final ProviderEvaluation details) { - try { - dependencyEvaluationSink.record( - flagKey, details, state.context, state.evaluationTimestampMs); - } catch (final RuntimeException | LinkageError ignored) { - // Telemetry is best effort and must not change evaluation semantics. + if (!evaluatedFlags.add(flagKey)) { + return; } + evaluations.add(new DependencyEvaluation(flagKey, details)); } private void recordReachedDepth(final int depth) { @@ -984,26 +978,58 @@ private DependencyGraphException(final ErrorCode errorCode, final String message public static final class DependencyExposure { public final String flagKey; - public final ProviderEvaluation details; + public final String variant; + public final ImmutableMetadata metadata; private DependencyExposure(final String flagKey, final ProviderEvaluation details) { this.flagKey = flagKey; - this.details = details; + this.variant = details.getVariant(); + this.metadata = details.getFlagMetadata(); } } - @FunctionalInterface - public interface DependencyEvaluationSink { - void record( - String flagKey, - ProviderEvaluation details, - EvaluationContext context, - long evaluationTimestampMs); + /** Detached evaluation telemetry for one reached prerequisite. */ + public static final class DependencyEvaluation { + public final String flagKey; + public final String variant; + public final String reason; + public final ErrorCode errorCode; + public final String errorMessage; + public final ImmutableMetadata metadata; + + private DependencyEvaluation(final String flagKey, final ProviderEvaluation details) { + this.flagKey = flagKey; + this.variant = details.getVariant(); + this.reason = details.getReason(); + this.errorCode = details.getErrorCode(); + this.errorMessage = details.getErrorMessage(); + this.metadata = details.getFlagMetadata(); + } } - @FunctionalInterface - public interface DependencyExposureSink { - void record(List exposures, EvaluationContext context); + /** Root evaluation plus the telemetry candidates accumulated while evaluating its graph. */ + public static final class EvaluationResult { + public final ProviderEvaluation details; + public final List dependencyEvaluations; + public final List dependencyExposures; + public final long evaluationTimestampMs; + + private EvaluationResult( + final ProviderEvaluation details, + final List dependencyEvaluations, + final List dependencyExposures, + final long evaluationTimestampMs) { + this.details = details; + this.dependencyEvaluations = + dependencyEvaluations.isEmpty() + ? emptyList() + : unmodifiableList(new ArrayList<>(dependencyEvaluations)); + this.dependencyExposures = + dependencyExposures.isEmpty() + ? emptyList() + : unmodifiableList(new ArrayList<>(dependencyExposures)); + this.evaluationTimestampMs = evaluationTimestampMs; + } } /** Tells whether an exposure was already recorded, so it is not sent again. */ diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/EvaluationTelemetryOptOutTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/EvaluationTelemetryOptOutTest.java index 788840113b7..bc5095d1071 100644 --- a/dd-openfeature/src/test/java/com/datadog/openfeature/EvaluationTelemetryOptOutTest.java +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/EvaluationTelemetryOptOutTest.java @@ -24,6 +24,7 @@ import com.datadog.openfeature.internal.bridge.v1.HealthMetrics; import com.datadog.openfeature.internal.bridge.v1.SpanEnricher; import com.datadog.openfeature.internal.config.TestSettings; +import com.datadog.openfeature.internal.evaluation.FlagEvaluator; import com.datadog.openfeature.internal.exposure.ExposureDeduplicationCache; import com.datadog.openfeature.internal.exposure.ExposureEvent; import com.datadog.openfeature.internal.exposure.ExposurePipeline; @@ -113,6 +114,8 @@ public void runtimeDefault(final String flagKey, final Object defaultValue) { private FeatureFlagsRuntime runtime; private CapturingEvaluator evaluator; private Client client; + private boolean failDependencyPublisher; + private boolean mutateDependencyRecord; @BeforeEach void setUp() throws Exception { @@ -533,6 +536,8 @@ void dependencyTelemetryIncludesMetricsWhenProviderOptOutIsDisabled() throws Exc assertEquals(1, included.getValue()); assertEquals(2, this.events.size(), "dependency and root evaluation events"); + assertEquals("dependency", this.events.get(0).flagKey, "dependency publishes before root"); + assertEquals("root", this.events.get(1).flagKey); assertEquals(2, this.metrics.get(), "dependency and root OTel metrics"); assertEquals(1, this.spans.size(), "only the ordinary root span hook runs"); assertEquals(2, this.exposures.size()); @@ -562,6 +567,61 @@ public void after( assertEquals(0, this.exposures.size()); } + @Test + void failingDependencyPublisherDoesNotChangeTheRootOrExposures() throws Exception { + this.failDependencyPublisher = true; + start( + new Provider.Options().enableEvaluationTelemetryOptOutParameter(false), + dependentConfiguration()); + + final FlagEvaluationDetails details = + this.client.getIntegerDetails("root", 0, context("subject", new Value(false))); + + assertEquals(1, details.getValue()); + assertEquals(1, this.events.size(), "only root telemetry survives publisher failure"); + assertEquals(1, this.metrics.get(), "only root metrics survive publisher failure"); + assertEquals(2, this.exposures.size(), "publisher failure does not affect exposures"); + } + + @Test + void dependencyRecordCannotBeMutatedByItsPublisher() throws Exception { + this.mutateDependencyRecord = true; + start( + new Provider.Options().enableEvaluationTelemetryOptOutParameter(false), + dependentConfiguration()); + + final FlagEvaluationDetails details = + this.client.getIntegerDetails("root", 0, context("subject", new Value(false))); + + assertEquals(1, details.getValue()); + assertEquals(1, this.events.size(), "immutable record rejects publisher mutation"); + assertEquals(2, this.exposures.size(), "mutation attempt does not affect exposures"); + } + + @Test + void parallelDependentRootsKeepIndependentReturnedBatches() throws Exception { + start( + new Provider.Options().enableEvaluationTelemetryOptOutParameter(false), + dependentConfiguration()); + final ExecutorService executor = Executors.newFixedThreadPool(2); + try { + final Future> first = + executor.submit( + () -> this.client.getIntegerDetails("root", 0, context("first", new Value(false)))); + final Future> second = + executor.submit( + () -> this.client.getIntegerDetails("root", 0, context("second", new Value(false)))); + + assertEquals(1, first.get(5, SECONDS).getValue()); + assertEquals(1, second.get(5, SECONDS).getValue()); + } finally { + executor.shutdownNow(); + } + assertEquals(4, this.events.size(), "dependency and root for each evaluation"); + assertEquals(4, this.metrics.get()); + assertEquals(4, this.exposures.size()); + } + @Test void concurrentSubjectsCannotChangeEachOthersDecision() throws Exception { start(); @@ -842,6 +902,20 @@ boolean isExposureDeliveryEnabled() { void recordExposure(final ExposureEvent event) { EvaluationTelemetryOptOutTest.this.exposures.add(event); } + + @Override + void publishDependencyEvaluation( + final FlagEvaluator.DependencyEvaluation dependency, + final EvaluationContext context, + final long evaluationTimestampMs) { + if (EvaluationTelemetryOptOutTest.this.failDependencyPublisher) { + throw new IllegalStateException("publisher failed"); + } + if (EvaluationTelemetryOptOutTest.this.mutateDependencyRecord) { + dependency.metadata.asUnmodifiableMap().clear(); + } + super.publishDependencyEvaluation(dependency, context, evaluationTimestampMs); + } } /** A flag evaluation pipeline capturing enqueued events, never started. */ diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java index 1f8d24dd544..042dd1725bf 100644 --- a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java @@ -35,16 +35,7 @@ class DependentFlagEvaluatorTest { @Test void memoizesDependenciesAndReplaysTheirExposureAfterRuleRollback() { - final List evaluations = new ArrayList<>(); - final List exposures = new ArrayList<>(); - final FlagEvaluator evaluator = - evaluator( - evaluations, - (items, context) -> { - for (final FlagEvaluator.DependencyExposure item : items) { - exposures.add(item.flagKey); - } - }); + final FlagEvaluator evaluator = evaluator(); final Map flags = new HashMap<>(); flags.put("dependency", stringFlag("dependency", "on", null, true)); flags.put( @@ -55,20 +46,19 @@ void memoizesDependenciesAndReplaysTheirExposureAfterRuleRollback() { allocation("rejected", dependencyRule("dependency", "off"), "selected", false), allocation("accepted", dependencyRule("dependency", "on"), "selected", false)))); - final ProviderEvaluation result = - evaluator.evaluate(config(flags, null), Integer.class, "root", 0, new MutableContext("u")); + final FlagEvaluator.EvaluationResult evaluation = + evaluator.evaluateWithDependencies( + config(flags, null), Integer.class, "root", 0, new MutableContext("u")); + final ProviderEvaluation result = evaluation.details; assertEquals(1, result.getValue()); - assertEquals(singletonList("dependency:on"), evaluations); - assertEquals(singletonList("dependency"), exposures); + assertEquals(singletonList("dependency:on"), evaluations(evaluation)); + assertEquals(singletonList("dependency"), exposures(evaluation)); } @Test void rollsBackDependencyExposuresWhenTheRootEvaluationFails() { - final List evaluations = new ArrayList<>(); - final List> exposureCommits = new ArrayList<>(); - final FlagEvaluator evaluator = - evaluator(evaluations, (items, context) -> exposureCommits.add(items)); + final FlagEvaluator evaluator = evaluator(); final Map flags = new HashMap<>(); flags.put("dependency", stringFlag("dependency", "on", null, true)); flags.put( @@ -78,65 +68,58 @@ void rollsBackDependencyExposuresWhenTheRootEvaluationFails() { singletonList( allocation("broken", dependencyRule("dependency", "on"), "missing", false)))); - final ProviderEvaluation result = - evaluator.evaluate(config(flags, null), Integer.class, "root", 0, new MutableContext("u")); + final FlagEvaluator.EvaluationResult evaluation = + evaluator.evaluateWithDependencies( + config(flags, null), Integer.class, "root", 0, new MutableContext("u")); + final ProviderEvaluation result = evaluation.details; assertEquals(ErrorCode.GENERAL, result.getErrorCode()); - assertEquals(singletonList("dependency:on"), evaluations); - assertTrue(exposureCommits.isEmpty()); + assertEquals(singletonList("dependency:on"), evaluations(evaluation)); + assertTrue(evaluation.dependencyExposures.isEmpty()); } @Test void propagatesCyclesAndEmitsAnErrorForEveryReachedDependency() { - final List evaluations = new ArrayList<>(); - final FlagEvaluator evaluator = evaluator(evaluations, (items, context) -> {}); + final FlagEvaluator evaluator = evaluator(); final Map flags = new HashMap<>(); flags.put("a", stringFlag("a", "on", dependencyRule("b", "on"), false)); flags.put("b", stringFlag("b", "on", dependencyRule("a", "on"), false)); - final ProviderEvaluation result = - evaluator.evaluate( + final FlagEvaluator.EvaluationResult evaluation = + evaluator.evaluateWithDependencies( config(flags, null), String.class, "a", "default", new MutableContext("u")); + final ProviderEvaluation result = evaluation.details; assertEquals(ErrorCode.GENERAL, result.getErrorCode()); - assertEquals(singletonList("b:GENERAL"), evaluations); + assertEquals(singletonList("b:GENERAL"), evaluations(evaluation)); } @Test void enforcesTheConfiguredMaximumDependencyDepth() { - final List evaluations = new ArrayList<>(); - final FlagEvaluator evaluator = evaluator(evaluations, (items, context) -> {}); + final FlagEvaluator evaluator = evaluator(); final Map flags = new HashMap<>(); flags.put("root", stringFlag("root", "on", dependencyRule("a", "on"), false)); flags.put("a", stringFlag("a", "on", dependencyRule("b", "on"), false)); flags.put("b", stringFlag("b", "on", null, false)); - final ProviderEvaluation result = - evaluator.evaluate( + final FlagEvaluator.EvaluationResult evaluation = + evaluator.evaluateWithDependencies( config(flags, new EvaluatorParams(1D)), String.class, "root", "default", new MutableContext("u")); + final ProviderEvaluation result = evaluation.details; assertEquals(ErrorCode.GENERAL, result.getErrorCode()); - assertEquals(Arrays.asList("b:GENERAL", "a:GENERAL"), evaluations); + assertEquals(Arrays.asList("b:GENERAL", "a:GENERAL"), evaluations(evaluation)); } @Test void memoizedSubtreeDepthFailureDoesNotReemitMemoizedNodeInEitherOrder() { for (final List rootDependencies : Arrays.asList(Arrays.asList("a", "b"), Arrays.asList("b", "a"))) { - final List evaluations = new ArrayList<>(); - final List exposures = new ArrayList<>(); - final FlagEvaluator evaluator = - evaluator( - evaluations, - (items, context) -> { - for (final FlagEvaluator.DependencyExposure item : items) { - exposures.add(item.flagKey); - } - }); + final FlagEvaluator evaluator = evaluator(); final Map flags = new HashMap<>(); flags.put("leaf", stringFlag("leaf", "on", null, true)); flags.put("a", stringFlag("a", "on", dependencyRule("leaf", "on"), true)); @@ -150,27 +133,28 @@ void memoizedSubtreeDepthFailureDoesNotReemitMemoizedNodeInEitherOrder() { ONE_OF, null, rootDependencies.get(1), singletonList("on")))); flags.put("root", stringFlag("root", "on", rootRule, true)); - final ProviderEvaluation result = - evaluator.evaluate( + final FlagEvaluator.EvaluationResult evaluation = + evaluator.evaluateWithDependencies( config(flags, new EvaluatorParams(2D)), String.class, "root", "default", new MutableContext("u")); + final ProviderEvaluation result = evaluation.details; assertEquals(ErrorCode.GENERAL, result.getErrorCode(), rootDependencies.toString()); final List evaluationKeys = new ArrayList<>(); - for (final String evaluation : evaluations) { - evaluationKeys.add(evaluation.substring(0, evaluation.indexOf(':'))); + for (final String record : evaluations(evaluation)) { + evaluationKeys.add(record.substring(0, record.indexOf(':'))); } assertEquals(Arrays.asList("leaf", "a", "b"), evaluationKeys); - assertTrue(exposures.isEmpty()); + assertTrue(evaluation.dependencyExposures.isEmpty()); } } @Test void validatesInvalidMaximumDependencyDepthOnlyWhenADependencyIsReached() { - final FlagEvaluator evaluator = evaluator(new ArrayList<>(), (items, context) -> {}); + final FlagEvaluator evaluator = evaluator(); final Map flags = new HashMap<>(); flags.put("ordinary", stringFlag("ordinary", "on", null, false)); flags.put("dependency", stringFlag("dependency", "on", null, false)); @@ -195,7 +179,7 @@ void rejectsExplicitNullMaximumDependencyDepthButDefaultsAnEmptyObject() { flags.put("dependency", stringFlag("dependency", "on", null, false)); flags.put( "dependent", stringFlag("dependent", "on", dependencyRule("dependency", "on"), false)); - final FlagEvaluator evaluator = evaluator(new ArrayList<>(), (items, context) -> {}); + final FlagEvaluator evaluator = evaluator(); assertEquals( "on-value", @@ -242,16 +226,14 @@ void rejectsNonStringDependencyMembershipOperandsWithParseError() throws Excepti .getBytes(UTF_8)); final ProviderEvaluation result = - evaluator(new ArrayList<>(), (items, context) -> {}) - .evaluate(config, String.class, "root", "default", new MutableContext("u")); + evaluator().evaluate(config, String.class, "root", "default", new MutableContext("u")); assertEquals(ErrorCode.PARSE_ERROR, result.getErrorCode()); } @Test void supportsNotOneOfAndShortCircuitsBeforeAFatalDependency() { - final List evaluations = new ArrayList<>(); - final FlagEvaluator evaluator = evaluator(evaluations, (items, context) -> {}); + final FlagEvaluator evaluator = evaluator(); final Map flags = new HashMap<>(); flags.put("dependency", stringFlag("dependency", "on", null, false)); final Rule rejectedBeforeMissing = @@ -276,53 +258,42 @@ void supportsNotOneOfAndShortCircuitsBeforeAFatalDependency() { final MutableContext context = new MutableContext("u"); context.add("gate", "closed"); - final ProviderEvaluation result = - evaluator.evaluate(config(flags, null), Integer.class, "root", 0, context); + final FlagEvaluator.EvaluationResult evaluation = + evaluator.evaluateWithDependencies(config(flags, null), Integer.class, "root", 0, context); + final ProviderEvaluation result = evaluation.details; assertEquals(1, result.getValue()); - assertEquals(singletonList("dependency:on"), evaluations); + assertEquals(singletonList("dependency:on"), evaluations(evaluation)); } @Test void emitsMissingAndInvalidPrerequisiteErrors() { - final List missing = new ArrayList<>(); final Map missingFlags = new HashMap<>(); missingFlags.put("root", stringFlag("root", "on", dependencyRule("missing", "on"), false)); - assertEquals( - ErrorCode.FLAG_NOT_FOUND, - evaluator(missing, (items, context) -> {}) - .evaluate( + final FlagEvaluator.EvaluationResult missingEvaluation = + evaluator() + .evaluateWithDependencies( config(missingFlags, null), String.class, "root", "default", - new MutableContext("u")) - .getErrorCode()); - assertEquals(singletonList("missing:FLAG_NOT_FOUND"), missing); + new MutableContext("u")); + assertEquals(ErrorCode.FLAG_NOT_FOUND, missingEvaluation.details.getErrorCode()); + assertEquals(singletonList("missing:FLAG_NOT_FOUND"), evaluations(missingEvaluation)); - final List invalid = new ArrayList<>(); final ServerConfiguration invalidConfig = config(missingFlags, null); invalidConfig.invalidFlags = singletonMap("missing", "invalid_flag"); - assertEquals( - ErrorCode.PARSE_ERROR, - evaluator(invalid, (items, context) -> {}) - .evaluate(invalidConfig, String.class, "root", "default", new MutableContext("u")) - .getErrorCode()); - assertEquals(singletonList("missing:PARSE_ERROR"), invalid); + final FlagEvaluator.EvaluationResult invalidEvaluation = + evaluator() + .evaluateWithDependencies( + invalidConfig, String.class, "root", "default", new MutableContext("u")); + assertEquals(ErrorCode.PARSE_ERROR, invalidEvaluation.details.getErrorCode()); + assertEquals(singletonList("missing:PARSE_ERROR"), evaluations(invalidEvaluation)); } @Test void rollsBackATransitiveTraceWhenTheRootAllocationHasNoAssignment() { - final List evaluations = new ArrayList<>(); - final List exposures = new ArrayList<>(); - final FlagEvaluator evaluator = - evaluator( - evaluations, - (items, context) -> { - for (final FlagEvaluator.DependencyExposure item : items) { - exposures.add(item.flagKey); - } - }); + final FlagEvaluator evaluator = evaluator(); final Map flags = new HashMap<>(); flags.put("leaf", stringFlag("leaf", "on", null, true)); flags.put("parent", stringFlag("parent", "on", dependencyRule("leaf", "on"), true)); @@ -339,27 +310,37 @@ void rollsBackATransitiveTraceWhenTheRootAllocationHasNoAssignment() { integerFlag( "root", Arrays.asList(noAssignment, allocation("fallback", null, "selected", false)))); - final ProviderEvaluation result = - evaluator.evaluate(config(flags, null), Integer.class, "root", 0, new MutableContext("u")); + final FlagEvaluator.EvaluationResult evaluation = + evaluator.evaluateWithDependencies( + config(flags, null), Integer.class, "root", 0, new MutableContext("u")); + final ProviderEvaluation result = evaluation.details; assertEquals(1, result.getValue()); - assertEquals(Arrays.asList("leaf:on", "parent:on"), evaluations); - assertTrue(exposures.isEmpty()); + assertEquals(Arrays.asList("leaf:on", "parent:on"), evaluations(evaluation)); + assertTrue(evaluation.dependencyExposures.isEmpty()); + } + + private static FlagEvaluator evaluator() { + return new FlagEvaluator(false, (flag, subject, allocation, variant, serialId) -> false); } - private static FlagEvaluator evaluator( - final List evaluations, final FlagEvaluator.DependencyExposureSink exposureSink) { - return new FlagEvaluator( - false, - (flag, subject, allocation, variant, serialId) -> false, - (flag, details, context, timestamp) -> - evaluations.add( - flag - + ":" - + (details.getErrorCode() == null - ? details.getVariant() - : details.getErrorCode().name())), - exposureSink); + private static List evaluations(final FlagEvaluator.EvaluationResult result) { + final List values = new ArrayList<>(); + for (final FlagEvaluator.DependencyEvaluation evaluation : result.dependencyEvaluations) { + values.add( + evaluation.flagKey + + ":" + + (evaluation.errorCode == null ? evaluation.variant : evaluation.errorCode.name())); + } + return values; + } + + private static List exposures(final FlagEvaluator.EvaluationResult result) { + final List values = new ArrayList<>(); + for (final FlagEvaluator.DependencyExposure exposure : result.dependencyExposures) { + values.add(exposure.flagKey); + } + return values; } private static ServerConfiguration config( From cb460967866181d4f5e946a9a79cf2d6bf0ed079 Mon Sep 17 00:00:00 2001 From: Codex GPT-5 Date: Tue, 3 Nov 2026 22:40:55 +0000 Subject: [PATCH 3/4] fix(openfeature): harden dependency exposure handoff Scope returned dependency exposures to their root hook lifecycle and isolate their delivery from ordinary root telemetry. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 --- .../com/datadog/openfeature/DDEvaluator.java | 136 +++++---- .../openfeature/ExposureLoggingHook.java | 87 +++++- .../internal/evaluation/FlagEvaluator.java | 4 +- .../openfeature/ExposureLoggingHookTest.java | 263 ++++++++++++++++++ .../DependentFlagEvaluatorTest.java | 18 +- 5 files changed, 435 insertions(+), 73 deletions(-) diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java b/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java index 3d851d58f0e..47cfefa883f 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/DDEvaluator.java @@ -38,14 +38,11 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicLong; import java.util.function.Consumer; class DDEvaluator implements Evaluator, Consumer { - private static final String DEPENDENCY_EXPOSURES_TOKEN = "__dd_dependency_exposures_token"; private static final String EVALUATION_TELEMETRY_OPT_OUT = "dd_evaluation_telemetry_opt_out"; /** @@ -97,9 +94,12 @@ class DDEvaluator implements Evaluator, Consumer { private final Settings settings; private final FlagEvaluator core; private final ConfigurationStore configuration = new ConfigurationStore(); - private final AtomicLong nextDependencyExposureToken = new AtomicLong(); - private final Map> pendingDependencyExposures = - new ConcurrentHashMap<>(); + // Dependency exposures belong to one OpenFeature hook lifecycle. The hook opens a scope before + // provider evaluation and closes it in finallyAfter, so direct provider calls neither retain + // candidates nor expose an internal hand-off token through public flag metadata. A stack keeps + // nested evaluations on the same thread isolated. + private final ThreadLocal> dependencyExposureScopes = + new ThreadLocal<>(); private volatile FeatureFlagsRuntime runtime; private volatile boolean dependencyTelemetryOptOutEnabled; private volatile FlagEvalMetrics dependencyMetrics; @@ -141,6 +141,7 @@ public boolean hasConfiguration() { @Override public synchronized void shutdown() { + dependencyExposureScopes.remove(); final FeatureFlagsRuntime released = this.runtime; this.runtime = null; if (released != null) { @@ -174,7 +175,7 @@ public ProviderEvaluation evaluate( final T defaultValue, final EvaluationContext context) { final FlagEvaluator.EvaluationResult evaluation = - core.evaluateWithDependencies(configuration.get(), target, key, defaultValue, context); + core.evaluateWithTrace(configuration.get(), target, key, defaultValue, context); for (final FlagEvaluator.DependencyEvaluation dependency : evaluation.dependencyEvaluations) { try { publishDependencyEvaluation(dependency, context, evaluation.evaluationTimestampMs); @@ -183,14 +184,12 @@ public ProviderEvaluation evaluate( } } final ProviderEvaluation result = evaluation.details; - final List staged = evaluation.dependencyExposures; - if (staged == null || staged.isEmpty() || result.getErrorCode() != null) { - return result; - } - final long token = nextDependencyExposureToken.incrementAndGet(); - pendingDependencyExposures.put(token, staged); - result.setFlagMetadata( - withLongMetadata(result.getFlagMetadata(), DEPENDENCY_EXPOSURES_TOKEN, token)); + stageDependencyExposures( + key, + context, + result.getErrorCode() == null + ? evaluation.dependencyExposures + : Collections.emptyList()); return result; } @@ -339,19 +338,65 @@ void publishDependencyEvaluation( features)); } - List takeDependencyExposures(final ImmutableMetadata metadata) { - if (metadata == null) { - return Collections.emptyList(); + void beginDependencyExposureScope( + final String rootFlagKey, final EvaluationContext evaluationContext) { + Deque scopes = dependencyExposureScopes.get(); + if (scopes == null) { + scopes = new LinkedList<>(); + dependencyExposureScopes.set(scopes); } - final Object token = metadata.asUnmodifiableMap().get(DEPENDENCY_EXPOSURES_TOKEN); - if (!(token instanceof Long)) { + scopes.push(new DependencyExposureScope(rootFlagKey, evaluationContext)); + } + + List endDependencyExposureScope() { + final Deque scopes = dependencyExposureScopes.get(); + if (scopes == null || scopes.isEmpty()) { return Collections.emptyList(); } - final List exposures = - pendingDependencyExposures.remove((Long) token); - return exposures == null - ? Collections.emptyList() - : exposures; + final List exposures = scopes.pop().exposures; + if (scopes.isEmpty()) { + dependencyExposureScopes.remove(); + } + return exposures; + } + + private void stageDependencyExposures( + final String rootFlagKey, + final EvaluationContext evaluationContext, + final List candidates) { + final Deque scopes = dependencyExposureScopes.get(); + if (scopes == null || scopes.isEmpty()) { + return; + } + final DependencyExposureScope scope = scopes.peek(); + if (scope.sealed || !scope.matches(rootFlagKey, evaluationContext)) { + return; + } + // The first matching provider evaluation owns this hook lifecycle, even when it has no + // eligible candidates. This prevents a later direct or reentrant evaluation from filling the + // scope before finallyAfter consumes it. + scope.sealed = true; + if (candidates == null || candidates.isEmpty()) { + return; + } + List eligible = null; + for (final FlagEvaluator.DependencyExposure candidate : candidates) { + final ImmutableMetadata metadata = candidate.metadata; + final Map values = + metadata == null ? Collections.emptyMap() : metadata.asUnmodifiableMap(); + if (!Boolean.TRUE.equals(values.get(DO_LOG)) + || !(values.get(ALLOCATION_KEY) instanceof String) + || candidate.variant == null) { + continue; + } + if (eligible == null) { + eligible = new ArrayList<>(); + } + eligible.add(candidate); + } + if (eligible != null) { + scope.exposures = Collections.unmodifiableList(eligible); + } } void recordDependencyExposures( @@ -396,34 +441,23 @@ private static boolean isEvaluationTelemetryOptedOut(final EvaluationContext con return value != null && value.isBoolean() && Boolean.TRUE.equals(value.asBoolean()); } - private static ImmutableMetadata withLongMetadata( - final ImmutableMetadata metadata, final String key, final long value) { - final ImmutableMetadata.ImmutableMetadataBuilder builder = ImmutableMetadata.builder(); - if (metadata != null) { - for (final Map.Entry entry : metadata.asUnmodifiableMap().entrySet()) { - addMetadata(builder, entry.getKey(), entry.getValue()); - } + private static final class DependencyExposureScope { + private final String rootFlagKey; + private final EvaluationContext evaluationContext; + private List exposures = Collections.emptyList(); + private boolean sealed; + + private DependencyExposureScope( + final String rootFlagKey, final EvaluationContext evaluationContext) { + this.rootFlagKey = rootFlagKey; + this.evaluationContext = evaluationContext; } - builder.addLong(key, value); - return builder.build(); - } - private static void addMetadata( - final ImmutableMetadata.ImmutableMetadataBuilder builder, - final String key, - final Object value) { - if (value instanceof String) { - builder.addString(key, (String) value); - } else if (value instanceof Boolean) { - builder.addBoolean(key, (Boolean) value); - } else if (value instanceof Integer) { - builder.addInteger(key, (Integer) value); - } else if (value instanceof Long) { - builder.addLong(key, (Long) value); - } else if (value instanceof Float) { - builder.addFloat(key, (Float) value); - } else if (value instanceof Double) { - builder.addDouble(key, (Double) value); + private boolean matches( + final String evaluatedFlagKey, final EvaluationContext evaluatedContext) { + return rootFlagKey != null + && rootFlagKey.equals(evaluatedFlagKey) + && evaluationContext == evaluatedContext; } } diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/ExposureLoggingHook.java b/dd-openfeature/src/main/java/com/datadog/openfeature/ExposureLoggingHook.java index a9329e187a7..3f2ba6f2b34 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/ExposureLoggingHook.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/ExposureLoggingHook.java @@ -13,8 +13,10 @@ import dev.openfeature.sdk.HookContext; import dev.openfeature.sdk.ImmutableMetadata; import dev.openfeature.sdk.Reason; +import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import org.slf4j.Logger; @@ -37,37 +39,68 @@ class ExposureLoggingHook implements Hook { private final Supplier evaluatorSupplier; private final AtomicBoolean warned = new AtomicBoolean(); + private final ThreadLocal> evaluatorScopes = new ThreadLocal<>(); ExposureLoggingHook(final Supplier evaluatorSupplier) { this.evaluatorSupplier = evaluatorSupplier; } + @Override + public Optional before( + final HookContext ctx, final Map hints) { + DDEvaluator evaluator = null; + try { + evaluator = this.evaluatorSupplier.get(); + if (evaluator != null) { + evaluator.beginDependencyExposureScope( + ctx == null ? null : ctx.getFlagKey(), ctx == null ? null : ctx.getCtx()); + } + } catch (final RuntimeException | LinkageError e) { + warnOnce(e); + evaluator = null; + } + List scopes = evaluatorScopes.get(); + if (scopes == null) { + scopes = new ArrayList<>(); + evaluatorScopes.set(scopes); + } + scopes.add(new ExposureScope(ctx, evaluator)); + return Optional.empty(); + } + @Override public void finallyAfter( final HookContext ctx, final FlagEvaluationDetails details, final Map hints) { try { - if (details == null) { - log.trace("Feature Flagging exposure suppressed: evaluation failed or details unavailable"); - return; - } - final DDEvaluator evaluator = this.evaluatorSupplier.get(); + final DDEvaluator scopedEvaluator = popEvaluatorScope(ctx); final List dependencyExposures = - evaluator == null + scopedEvaluator == null ? java.util.Collections . emptyList() - : evaluator.takeDependencyExposures(details.getFlagMetadata()); + : scopedEvaluator.endDependencyExposureScope(); + if (details == null) { + log.trace("Feature Flagging exposure suppressed: evaluation failed or details unavailable"); + return; + } + final DDEvaluator evaluator = + scopedEvaluator == null ? this.evaluatorSupplier.get() : scopedEvaluator; if (details.getErrorCode() != null || Reason.ERROR.name().equals(details.getReason())) { log.trace("Feature Flagging exposure suppressed: evaluation failed or details unavailable"); return; } final EvaluationContext context = ctx == null ? null : ctx.getCtx(); if (evaluator != null) { - evaluator.recordDependencyExposures(dependencyExposures, context); + try { + evaluator.recordDependencyExposures(dependencyExposures, context); + } catch (final RuntimeException | LinkageError e) { + // Dependency exposure delivery is best-effort and must not suppress the root exposure. + warnOnce(e); + } } final ImmutableMetadata metadata = details.getFlagMetadata(); if (metadata == null) { @@ -120,9 +153,41 @@ public void finallyAfter( } } catch (final RuntimeException | LinkageError e) { // Never let exposure reporting break flag evaluation. - if (this.warned.compareAndSet(false, true)) { - log.warn("Could not record a feature flag exposure; later failures are not logged", e); - } + warnOnce(e); + } + } + + private DDEvaluator popEvaluatorScope(final HookContext ctx) { + final List scopes = evaluatorScopes.get(); + if (scopes == null || scopes.isEmpty()) { + return null; + } + final ExposureScope scope = scopes.get(scopes.size() - 1); + // The SDK invokes finally hooks even for hooks whose before method was skipped because an + // earlier before hook threw. Do not let that unmatched lifecycle pop an enclosing evaluation. + if (scope.context != ctx) { + return null; + } + scopes.remove(scopes.size() - 1); + if (scopes.isEmpty()) { + evaluatorScopes.remove(); + } + return scope.evaluator; + } + + private void warnOnce(final Throwable failure) { + if (this.warned.compareAndSet(false, true)) { + log.warn("Could not record a feature flag exposure; later failures are not logged", failure); + } + } + + private static final class ExposureScope { + private final HookContext context; + private final DDEvaluator evaluator; + + private ExposureScope(final HookContext context, final DDEvaluator evaluator) { + this.context = context; + this.evaluator = evaluator; } } } diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java index cbbced47bc5..d0580d108de 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java @@ -83,11 +83,11 @@ public ProviderEvaluation evaluate( final String key, final T defaultValue, final EvaluationContext context) { - return evaluateWithDependencies(config, target, key, defaultValue, context).details; + return evaluateWithTrace(config, target, key, defaultValue, context).details; } /** Evaluates a root flag and returns all telemetry candidates produced by its graph. */ - public EvaluationResult evaluateWithDependencies( + public EvaluationResult evaluateWithTrace( final ServerConfiguration config, final Class target, final String key, diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/ExposureLoggingHookTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/ExposureLoggingHookTest.java index e705bb0ffef..2f381b5885b 100644 --- a/dd-openfeature/src/test/java/com/datadog/openfeature/ExposureLoggingHookTest.java +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/ExposureLoggingHookTest.java @@ -4,6 +4,7 @@ import static com.datadog.openfeature.internal.evaluation.MetadataKeys.DO_LOG; import static com.datadog.openfeature.internal.evaluation.MetadataKeys.EXPOSURE_CACHE_HIT; import static com.datadog.openfeature.internal.evaluation.MetadataKeys.SPLIT_SERIAL_ID; +import static com.datadog.openfeature.internal.ufc.ConditionOperator.ONE_OF; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; @@ -16,15 +17,19 @@ import com.datadog.openfeature.Provider.Options; import com.datadog.openfeature.internal.bridge.v1.AgentBridge; import com.datadog.openfeature.internal.config.TestSettings; +import com.datadog.openfeature.internal.evaluation.FlagEvaluator.DependencyExposure; import com.datadog.openfeature.internal.exposure.ExposureEvent; import com.datadog.openfeature.internal.ufc.Allocation; +import com.datadog.openfeature.internal.ufc.ConditionConfiguration; import com.datadog.openfeature.internal.ufc.Flag; +import com.datadog.openfeature.internal.ufc.Rule; import com.datadog.openfeature.internal.ufc.ServerConfiguration; import com.datadog.openfeature.internal.ufc.Split; import com.datadog.openfeature.internal.ufc.ValueType; import com.datadog.openfeature.internal.ufc.Variant; import dev.openfeature.sdk.Client; import dev.openfeature.sdk.ErrorCode; +import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.FlagEvaluationDetails; import dev.openfeature.sdk.FlagValueType; import dev.openfeature.sdk.Hook; @@ -32,6 +37,7 @@ import dev.openfeature.sdk.ImmutableMetadata; import dev.openfeature.sdk.MutableContext; import dev.openfeature.sdk.OpenFeatureAPI; +import dev.openfeature.sdk.ProviderEvaluation; import dev.openfeature.sdk.multiprovider.MultiProvider; import java.util.HashMap; import java.util.List; @@ -145,6 +151,184 @@ void sendsNothingForAnAllocationThatDoesNotLogExposures() { assertNull(cacheHit(details)); } + @Test + void publishesEligibleDependencyExposuresThroughTheRootHookLifecycle() throws Exception { + final ExposureTestEvaluator dependent = + new ExposureTestEvaluator(dependentConfiguration(true), true); + OpenFeatureAPI.getInstance() + .setProviderAndWait(new Provider(new Options().initTimeout(10, SECONDS), dependent)); + + OpenFeatureAPI.getInstance() + .getClient() + .getIntegerDetails("root", 0, new MutableContext("user-1")); + + assertEquals(1, dependent.sentToDatadog.size()); + assertEquals("dependency", dependent.sentToDatadog.get(0).flag.key); + } + + @Test + void doesNotStageIneligibleDependencyExposures() throws Exception { + final ExposureTestEvaluator dependent = + new ExposureTestEvaluator(dependentConfiguration(false), true); + OpenFeatureAPI.getInstance() + .setProviderAndWait(new Provider(new Options().initTimeout(10, SECONDS), dependent)); + + OpenFeatureAPI.getInstance() + .getClient() + .getIntegerDetails("root", 0, new MutableContext("user-1")); + + assertTrue(dependent.sentToDatadog.isEmpty()); + } + + @Test + void directProviderEvaluationDoesNotRetainOrExposeDependencyState() throws Exception { + final ExposureTestEvaluator dependent = + new ExposureTestEvaluator(dependentConfiguration(true), true); + final Provider provider = new Provider(new Options().initTimeout(10, SECONDS), dependent); + final MutableContext context = new MutableContext("user-1"); + provider.initialize(context); + + final ProviderEvaluation evaluation = + provider.getIntegerEvaluation("root", 0, context); + + assertTrue( + evaluation.getFlagMetadata() == null + || !evaluation + .getFlagMetadata() + .asUnmodifiableMap() + .containsKey("__dd_dependency_exposures_token")); + new ExposureLoggingHook(() -> dependent) + .finallyAfter( + HookContext.builder() + .flagKey("root") + .type(FlagValueType.INTEGER) + .defaultValue(0) + .ctx(context) + .build(), + FlagEvaluationDetails.from(evaluation, "root"), + emptyMap()); + assertTrue(dependent.sentToDatadog.isEmpty()); + + provider.shutdown(); + } + + @Test + void reentrantDirectEvaluationCannotReplaceTheOuterRootsDependencyExposures() { + final ExposureTestEvaluator dependent = + new ExposureTestEvaluator(reentrantDependentConfiguration(), true); + final ExposureLoggingHook hook = new ExposureLoggingHook<>(() -> dependent); + final MutableContext outerContext = new MutableContext("subject-a"); + final MutableContext reentrantContext = new MutableContext("subject-b"); + final HookContext outerHookContext = + HookContext.builder() + .flagKey("root-a") + .type(FlagValueType.INTEGER) + .defaultValue(0) + .ctx(outerContext) + .build(); + + hook.before(outerHookContext, emptyMap()); + final ProviderEvaluation outerEvaluation = + dependent.evaluate(Integer.class, "root-a", 0, outerContext); + dependent.evaluate(Integer.class, "root-b", 0, reentrantContext); + hook.finallyAfter( + outerHookContext, FlagEvaluationDetails.from(outerEvaluation, "root-a"), emptyMap()); + + assertEquals(1, dependent.sentToDatadog.size()); + assertEquals("dependency-a", dependent.sentToDatadog.get(0).flag.key); + assertEquals("subject-a", dependent.sentToDatadog.get(0).subject.id); + } + + @Test + void unmatchedNestedFinallyCannotPopTheOuterDependencyExposureScope() { + final ExposureTestEvaluator dependent = + new ExposureTestEvaluator(reentrantDependentConfiguration(), true); + final ExposureLoggingHook hook = new ExposureLoggingHook<>(() -> dependent); + final MutableContext outerContext = new MutableContext("subject-a"); + final HookContext outerHookContext = + HookContext.builder() + .flagKey("root-a") + .type(FlagValueType.INTEGER) + .defaultValue(0) + .ctx(outerContext) + .build(); + final HookContext innerHookContext = + HookContext.builder() + .flagKey("root-b") + .type(FlagValueType.INTEGER) + .defaultValue(0) + .ctx(new MutableContext("subject-b")) + .build(); + + hook.before(outerHookContext, emptyMap()); + final ProviderEvaluation outerEvaluation = + dependent.evaluate(Integer.class, "root-a", 0, outerContext); + + // OpenFeature invokes finally hooks for the inner evaluation even when an earlier before hook + // throws before this hook's before method is reached. + hook.finallyAfter( + innerHookContext, + FlagEvaluationDetails.builder() + .flagKey("root-b") + .value(0) + .errorCode(ErrorCode.GENERAL) + .build(), + emptyMap()); + hook.finallyAfter( + outerHookContext, FlagEvaluationDetails.from(outerEvaluation, "root-a"), emptyMap()); + + assertEquals(1, dependent.sentToDatadog.size()); + assertEquals("dependency-a", dependent.sentToDatadog.get(0).flag.key); + assertEquals("subject-a", dependent.sentToDatadog.get(0).subject.id); + assertTrue(dependent.endDependencyExposureScope().isEmpty()); + + // A subsequent lifecycle proves the hook's own ThreadLocal stack was removed as well. + hook.before(innerHookContext, emptyMap()); + final ProviderEvaluation innerEvaluation = + dependent.evaluate(Integer.class, "root-b", 0, innerHookContext.getCtx()); + hook.finallyAfter( + innerHookContext, FlagEvaluationDetails.from(innerEvaluation, "root-b"), emptyMap()); + assertEquals(2, dependent.sentToDatadog.size()); + assertEquals("dependency-b", dependent.sentToDatadog.get(1).flag.key); + } + + @Test + void emptyMatchingEvaluationSealsTheOuterDependencyExposureScope() { + final ExposureTestEvaluator dependent = + new ExposureTestEvaluator(dependentConfiguration(false), true); + final ExposureLoggingHook hook = new ExposureLoggingHook<>(() -> dependent); + final MutableContext context = new MutableContext("subject-a"); + final HookContext hookContext = + HookContext.builder() + .flagKey("root") + .type(FlagValueType.INTEGER) + .defaultValue(0) + .ctx(context) + .build(); + + hook.before(hookContext, emptyMap()); + final ProviderEvaluation outerEvaluation = + dependent.evaluate(Integer.class, "root", 0, context); + dependent.accept(dependentConfiguration(true)); + dependent.evaluate(Integer.class, "root", 0, context); + hook.finallyAfter(hookContext, FlagEvaluationDetails.from(outerEvaluation, "root"), emptyMap()); + + assertTrue(dependent.sentToDatadog.isEmpty()); + } + + @Test + void shutdownDiscardsTheCurrentDependencyExposureScope() { + final ExposureTestEvaluator dependent = + new ExposureTestEvaluator(dependentConfiguration(true), true); + final MutableContext context = new MutableContext("user-1"); + dependent.beginDependencyExposureScope("root", context); + dependent.evaluate(Integer.class, "root", 0, context); + + dependent.shutdown(); + + assertTrue(dependent.endDependencyExposureScope().isEmpty()); + } + @Test void recordsButDoesNotSendWhenDatadogLoggingIsOff() { final ExposureTestEvaluator notSending = @@ -254,6 +438,29 @@ void recordExposure(final ExposureEvent event) { context, details("off", exposureMetadata(true, false, "a")), emptyMap())); } + @Test + void dependencyExposureFailureDoesNotSuppressTheRootExposure() { + final ExposureTestEvaluator failingDependencies = + new ExposureTestEvaluator(configuration("on", Boolean.TRUE), true) { + @Override + void recordDependencyExposures( + final List exposures, final EvaluationContext context) { + throw new IllegalStateException("dependency queue failure"); + } + }; + final ExposureLoggingHook hook = new ExposureLoggingHook<>(() -> failingDependencies); + final HookContext context = hookContext(); + + hook.before(context, emptyMap()); + assertDoesNotThrow( + () -> + hook.finallyAfter( + context, details("on", exposureMetadata(true, false, "a")), emptyMap())); + + assertEquals(1, failingDependencies.sentToDatadog.size()); + assertEquals("flag", failingDependencies.sentToDatadog.get(0).flag.key); + } + private static Object cacheHit(final FlagEvaluationDetails details) { return details.getFlagMetadata().asUnmodifiableMap().get(EXPOSURE_CACHE_HIT); } @@ -305,4 +512,60 @@ private static ServerConfiguration configuration(final String variantKey, final "flag", new Flag("flag", true, ValueType.INTEGER, variations, singletonList(allocation))); return new ServerConfiguration("", "", false, null, flags); } + + private static ServerConfiguration dependentConfiguration(final boolean dependencyDoLog) { + final Map flags = new HashMap<>(); + addDependentFlagPair(flags, "root", "dependency", dependencyDoLog); + return new ServerConfiguration("", "", false, null, flags); + } + + private static ServerConfiguration reentrantDependentConfiguration() { + final Map flags = new HashMap<>(); + addDependentFlagPair(flags, "root-a", "dependency-a", true); + addDependentFlagPair(flags, "root-b", "dependency-b", true); + return new ServerConfiguration("", "", false, null, flags); + } + + private static void addDependentFlagPair( + final Map flags, + final String rootKey, + final String dependencyKey, + final boolean dependencyDoLog) { + final Map dependencyVariations = new HashMap<>(); + dependencyVariations.put("on", new Variant("on", "on-value")); + final Allocation dependencyAllocation = + new Allocation( + dependencyKey + "-allocation", + null, + null, + null, + singletonList(new Split(emptyList(), "on", emptyMap(), 8)), + dependencyDoLog); + final Flag dependency = + new Flag( + dependencyKey, + true, + ValueType.STRING, + dependencyVariations, + singletonList(dependencyAllocation)); + + final Map rootVariations = new HashMap<>(); + rootVariations.put("selected", new Variant("selected", 1)); + final Rule dependencyRule = + new Rule( + singletonList( + new ConditionConfiguration(ONE_OF, null, dependencyKey, singletonList("on")))); + final Allocation rootAllocation = + new Allocation( + rootKey + "-allocation", + singletonList(dependencyRule), + null, + null, + singletonList(new Split(emptyList(), "selected", emptyMap(), 9)), + false); + flags.put(dependencyKey, dependency); + flags.put( + rootKey, + new Flag(rootKey, true, ValueType.INTEGER, rootVariations, singletonList(rootAllocation))); + } } diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java index 042dd1725bf..dbda5caf195 100644 --- a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/DependentFlagEvaluatorTest.java @@ -47,7 +47,7 @@ void memoizesDependenciesAndReplaysTheirExposureAfterRuleRollback() { allocation("accepted", dependencyRule("dependency", "on"), "selected", false)))); final FlagEvaluator.EvaluationResult evaluation = - evaluator.evaluateWithDependencies( + evaluator.evaluateWithTrace( config(flags, null), Integer.class, "root", 0, new MutableContext("u")); final ProviderEvaluation result = evaluation.details; @@ -69,7 +69,7 @@ void rollsBackDependencyExposuresWhenTheRootEvaluationFails() { allocation("broken", dependencyRule("dependency", "on"), "missing", false)))); final FlagEvaluator.EvaluationResult evaluation = - evaluator.evaluateWithDependencies( + evaluator.evaluateWithTrace( config(flags, null), Integer.class, "root", 0, new MutableContext("u")); final ProviderEvaluation result = evaluation.details; @@ -86,7 +86,7 @@ void propagatesCyclesAndEmitsAnErrorForEveryReachedDependency() { flags.put("b", stringFlag("b", "on", dependencyRule("a", "on"), false)); final FlagEvaluator.EvaluationResult evaluation = - evaluator.evaluateWithDependencies( + evaluator.evaluateWithTrace( config(flags, null), String.class, "a", "default", new MutableContext("u")); final ProviderEvaluation result = evaluation.details; @@ -103,7 +103,7 @@ void enforcesTheConfiguredMaximumDependencyDepth() { flags.put("b", stringFlag("b", "on", null, false)); final FlagEvaluator.EvaluationResult evaluation = - evaluator.evaluateWithDependencies( + evaluator.evaluateWithTrace( config(flags, new EvaluatorParams(1D)), String.class, "root", @@ -134,7 +134,7 @@ void memoizedSubtreeDepthFailureDoesNotReemitMemoizedNodeInEitherOrder() { flags.put("root", stringFlag("root", "on", rootRule, true)); final FlagEvaluator.EvaluationResult evaluation = - evaluator.evaluateWithDependencies( + evaluator.evaluateWithTrace( config(flags, new EvaluatorParams(2D)), String.class, "root", @@ -259,7 +259,7 @@ void supportsNotOneOfAndShortCircuitsBeforeAFatalDependency() { context.add("gate", "closed"); final FlagEvaluator.EvaluationResult evaluation = - evaluator.evaluateWithDependencies(config(flags, null), Integer.class, "root", 0, context); + evaluator.evaluateWithTrace(config(flags, null), Integer.class, "root", 0, context); final ProviderEvaluation result = evaluation.details; assertEquals(1, result.getValue()); @@ -272,7 +272,7 @@ void emitsMissingAndInvalidPrerequisiteErrors() { missingFlags.put("root", stringFlag("root", "on", dependencyRule("missing", "on"), false)); final FlagEvaluator.EvaluationResult missingEvaluation = evaluator() - .evaluateWithDependencies( + .evaluateWithTrace( config(missingFlags, null), String.class, "root", @@ -285,7 +285,7 @@ void emitsMissingAndInvalidPrerequisiteErrors() { invalidConfig.invalidFlags = singletonMap("missing", "invalid_flag"); final FlagEvaluator.EvaluationResult invalidEvaluation = evaluator() - .evaluateWithDependencies( + .evaluateWithTrace( invalidConfig, String.class, "root", "default", new MutableContext("u")); assertEquals(ErrorCode.PARSE_ERROR, invalidEvaluation.details.getErrorCode()); assertEquals(singletonList("missing:PARSE_ERROR"), evaluations(invalidEvaluation)); @@ -311,7 +311,7 @@ void rollsBackATransitiveTraceWhenTheRootAllocationHasNoAssignment() { "root", Arrays.asList(noAssignment, allocation("fallback", null, "selected", false)))); final FlagEvaluator.EvaluationResult evaluation = - evaluator.evaluateWithDependencies( + evaluator.evaluateWithTrace( config(flags, null), Integer.class, "root", 0, new MutableContext("u")); final ProviderEvaluation result = evaluation.details; From 9122ac3d9a3b4c8cd4d14b3938ac01ebe9f10853 Mon Sep 17 00:00:00 2001 From: Helen Han Date: Wed, 4 Nov 2026 14:01:21 -0500 Subject: [PATCH 4/4] Advertise dependent flag support to Remote Configuration Advertise bit 50 with FFE_FLAGS on the agent-based configuration path. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 --- .../com/datadog/featureflag/RemoteConfigServiceImpl.java | 9 +++++++-- .../datadog/featureflag/RemoteConfigServiceImplTest.java | 8 +++++--- .../src/main/java/datadog/remoteconfig/Capabilities.java | 1 + 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java index 8922955d373..df907bffd91 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java @@ -1,6 +1,7 @@ package com.datadog.featureflag; import static datadog.remoteconfig.Capabilities.CAPABILITY_FFE_FLAG_CONFIGURATION_RULES; +import static datadog.remoteconfig.Capabilities.CAPABILITY_FFE_FLAG_DEPENDENT_FLAG_VARIANT_KEY_V1; import static datadog.remoteconfig.Product.FFE_FLAGS; import datadog.remoteconfig.ConfigurationChangesTypedListener; @@ -47,7 +48,9 @@ public synchronized void start() { } this.started = true; LOGGER.debug("Feature Flagging starting remote_config configuration source"); - this.configurationPoller.addCapabilities(CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); + this.configurationPoller.addCapabilities( + CAPABILITY_FFE_FLAG_CONFIGURATION_RULES + | CAPABILITY_FFE_FLAG_DEPENDENT_FLAG_VARIANT_KEY_V1); listen(); this.configurationPoller.start(); } @@ -111,7 +114,9 @@ public synchronized void close() { } this.started = false; LOGGER.debug("Feature Flagging remote_config configuration source stopped"); - this.configurationPoller.removeCapabilities(CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); + this.configurationPoller.removeCapabilities( + CAPABILITY_FFE_FLAG_CONFIGURATION_RULES + | CAPABILITY_FFE_FLAG_DEPENDENT_FLAG_VARIANT_KEY_V1); this.configurationPoller.removeListeners(FFE_FLAGS); this.configurationPoller.stop(); this.subscribers.clear(); 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 f4083331c6c..55698f56921 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 @@ -54,7 +54,8 @@ void subscriptionRegistersTheProductWithARawDeserializer() throws Exception { service.subscribe(content -> {}); service.subscribe(content -> {}); - verify(poller).addCapabilities(Capabilities.CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); + verify(poller) + .addCapabilities(Capabilities.CAPABILITY_FFE_FLAG_CONFIGURATION_RULES | (1L << 50)); final ArgumentCaptor> deserializer = ArgumentCaptor.forClass(ConfigurationDeserializer.class); verify(poller).addListener(eq(Product.FFE_FLAGS), deserializer.capture(), eq(service)); @@ -258,7 +259,8 @@ void startRegistersTheProductOnceBeforeAnySubscription() { service.start(); service.subscribe(content -> {}); - verify(poller, times(1)).addCapabilities(Capabilities.CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); + verify(poller, times(1)) + .addCapabilities(Capabilities.CAPABILITY_FFE_FLAG_CONFIGURATION_RULES | (1L << 50)); verify(poller, times(1)).start(); } @@ -288,7 +290,7 @@ void closeUnregistersTheProductAndDropsSubscribers() { service.accept("key", DOCUMENT, mock(PollingRateHinter.class)); verify(poller, times(1)) - .removeCapabilities(Capabilities.CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); + .removeCapabilities(Capabilities.CAPABILITY_FFE_FLAG_CONFIGURATION_RULES | (1L << 50)); verify(poller, times(1)).removeListeners(Product.FFE_FLAGS); verify(poller, times(1)).stop(); assertEquals(0, received.size()); diff --git a/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java b/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java index ce76e59000a..d0956d08364 100644 --- a/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java +++ b/remote-config/remote-config-api/src/main/java/datadog/remoteconfig/Capabilities.java @@ -47,4 +47,5 @@ public interface Capabilities { long CAPABILITY_ASM_EXTENDED_DATA_COLLECTION = 1L << 44; long CAPABILITY_APM_TRACING_MULTICONFIG = 1L << 45; long CAPABILITY_FFE_FLAG_CONFIGURATION_RULES = 1L << 46; + long CAPABILITY_FFE_FLAG_DEPENDENT_FLAG_VARIANT_KEY_V1 = 1L << 50; }