diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index 84ee1174006..45e12e44c85 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -1604,6 +1604,14 @@ "aliases": [] } ], + "DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED": [ + { + "version": "A", + "type": "boolean", + "default": "true", + "aliases": [] + } + ], "DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED": [ { "version": "A", diff --git a/products/feature-flagging/feature-flagging-api/README.md b/products/feature-flagging/feature-flagging-api/README.md index 7af28582732..7afc0cd0359 100644 --- a/products/feature-flagging/feature-flagging-api/README.md +++ b/products/feature-flagging/feature-flagging-api/README.md @@ -86,6 +86,103 @@ OTEL_EXPORTER_OTLP_PROTOCOL=grpc | `error.type` | Error code (lowercased, only on error) | | `feature_flag.result.allocation_key` | Allocation key (when present) | +## Exposures in your own data store + +The provider sends exposures to Datadog. It also puts its exposure decision and the selected split's +features into the flag metadata of every evaluation, so any OpenFeature hook can record exposures in +your own data store (see [Flag metadata](#flag-metadata)). + +For convenience, `dd-openfeature` ships `ExposureHook`, which reads that metadata for you. It +receives every completed evaluation, with the same exposure decision that the provider uses for +Datadog. + +```java +import datadog.trace.api.openfeature.ExposureHook; + +BlockingQueue queue = new LinkedBlockingQueue<>(10_000); + +OpenFeatureAPI.getInstance().addHooks(new ExposureHook(evaluation -> { + if (!evaluation.shouldSend()) { + return; + } + Map features = evaluation.getFeatures(); + queue.offer(new ExposureRecord( + evaluation.getDetails().getFlagKey(), + evaluation.getDetails().getVariant(), + evaluation.getContext().getTargetingKey(), + (String) features.get("holdout.key"), + (String) features.get("holdout.assignment_group"))); +})); +// A background thread drains the queue and writes to the data store. +``` + +- The callback runs on the evaluation thread. Do not write to the data store from the callback. + Copy what you need and hand it to a background queue. +- `shouldSend()` is true for a successful evaluation of an allocation that logs exposures, for a + subject not yet exposed to the same allocation, variant and serial id. The serial id counts only + with `dd-java-agent` 1.67.0 or later. These are exactly the exposures that the provider sends to + Datadog, as long as Datadog logging is on and the provider's own hooks run: evaluate through an + OpenFeature client, and inside a `MultiProvider` use OpenFeature SDK 1.23.0 or later. With an + older SDK, a provider inside a `MultiProvider` sends no exposures, and `shouldSend()` is true for + every exposure. +- `isExposure()` is true for every successful evaluation of an allocation that logs exposures, + including repeats. `isCacheHit()` tells repeats apart. +- An evaluation that fails, including one that a later `after` hook fails, is not an exposure. +- An exception from the callback does not affect the evaluation. The first one is logged at warn + level, later ones at debug level. + +### Features + +`getFeatures()` returns the features of the selected split whose destinations include `HOOK`. +Values are a `String`, `Double` or `Boolean`. Holdout splits carry: + +| Feature | Value | +|---|---| +| `holdout.key` | The holdout key | +| `holdout.assignment_group` | `status_quo` or `all_shipped` | + +The map is empty when the split has no such features. Datadog can add destinations later; the +provider ignores a destination it does not know and logs a warning once for each one. + +### Flag metadata + +The same data is in the evaluation's flag metadata, for hooks that use only the OpenFeature API. +Read it in the hook's `finallyAfter` stage, where the details match what the application receives, +and skip details that have an error code. `ExposureHook` does both. + +| Key | Type | Meaning | +|---|---|---| +| `allocationKey` | String | The selected allocation | +| `__dd_do_log` | Boolean | Whether the allocation logs exposures. Present when the allocation logs exposures or span enrichment is on | +| `__dd_exposure_cache_hit` | Boolean | The subject was already exposed. Present only when `__dd_do_log` is true | +| `__dd_split_serial_id` | Integer | The split's serial id. Present when `__dd_do_log` is present, the split has a serial id, and the installed `dd-java-agent` supports serial ids | +| `__dd_feature.` | String, Double or Boolean | One entry for each feature of the selected split that lists the `HOOK` destination | + +### Agent versions + +`dd-openfeature` and `dd-java-agent` ship separately. The provider turns a capability off when the +installed agent lacks it: + +| Capability | Needs `dd-java-agent` | Without it | +|---|---|---| +| Serial ids on splits (`__dd_split_serial_id`) | 1.65.0 | The key is absent | +| Serial ids on exposures sent to Datadog | 1.67.0 | Exposures are sent without a serial id, so holdout attribution is lost | +| Flag evaluation events sent to Datadog | 1.66.0 | No flag evaluation events are sent | +| Split features (`getFeatures()`, `__dd_feature.`) | 1.68.0 | `getFeatures()` is empty | + +### Stop sending exposures to Datadog + +```shell +DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED=false +``` + +The provider then sends no exposures to Datadog. `ExposureHook` callbacks still receive every +evaluation, and `shouldSend()` gives the same result as before. The default is `true`. + +This setting stops exposures only. Flag evaluation events have their own setting, +`DD_FLAGGING_EVALUATION_COUNTS_ENABLED`. The Java provider does not read dd-trace-py's +`DD_FFE_INTAKE_ENABLED`. + ## Requirements - Java 11+ diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java index 9e65ed9f938..2afec791851 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java @@ -8,6 +8,7 @@ import datadog.trace.api.featureflag.ufc.v1.Allocation; import datadog.trace.api.featureflag.ufc.v1.ConditionConfiguration; import datadog.trace.api.featureflag.ufc.v1.ConditionOperator; +import datadog.trace.api.featureflag.ufc.v1.Feature; import datadog.trace.api.featureflag.ufc.v1.Flag; import datadog.trace.api.featureflag.ufc.v1.ParsedSemver; import datadog.trace.api.featureflag.ufc.v1.Rule; @@ -41,6 +42,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -63,6 +65,40 @@ class DDEvaluator implements Evaluator, FeatureFlaggingGateway.ConfigListener { new AtomicBoolean( SPLIT_SERIAL_ID_SUPPORTED.get() && exposureSerialIdSupported(ExposureEvent.class)); + static final AtomicBoolean SPLIT_FEATURES_SUPPORTED = + new AtomicBoolean(splitFeaturesSupported(Split.class)); + + static final AtomicBoolean EXPOSURE_FEATURES_SUPPORTED = + new AtomicBoolean(exposureFeaturesSupported(ExposureEvent.class)); + + /** An older agent's exposure event has no features; its exposures are then sent without them. */ + static boolean exposureFeaturesSupported(final Class eventClass) { + try { + eventClass.getConstructor( + long.class, + datadog.trace.api.featureflag.exposure.Allocation.class, + datadog.trace.api.featureflag.exposure.Flag.class, + datadog.trace.api.featureflag.exposure.Variant.class, + Subject.class, + Integer.class, + Map.class); + return true; + } catch (final NoSuchMethodException | LinkageError | RuntimeException e) { + log.debug("The installed Datadog Java agent does not send features on exposures", e); + return false; + } + } + + /** An older agent ships a Split without features; its evaluations then carry no features. */ + static boolean splitFeaturesSupported(final Class splitClass) { + try { + return splitClass.getField("features").getType() == List.class; + } catch (final NoSuchFieldException | LinkageError | RuntimeException e) { + log.debug("The installed Datadog Java agent does not carry split features", e); + return false; + } + } + static boolean splitSerialIdSupported(final Class splitClass) { try { return splitClass.getField("serialId").getType() == Integer.class; @@ -149,6 +185,23 @@ static boolean exposureSerialIdSupported(final Class eventClass) { // Emitted only when the allocation logs exposures: true when this subject's exposure was already // sent, so the exposure hook does not send it again. static final String METADATA_EXPOSURE_CACHE_HIT = "__dd_exposure_cache_hit"; + // Each feature of the selected split is copied to the metadata once for each destination it + // lists, under that destination's prefix. Only the HOOK prefix is a public contract; the + // other two carry features to Datadog's own exposure and flag-evaluation hooks. + static final String METADATA_FEATURE_PREFIX = "__dd_feature."; + static final String METADATA_EXPOSURE_FEATURE_PREFIX = "__dd_exposure_feature."; + static final String METADATA_EVALUATION_FEATURE_PREFIX = "__dd_evaluation_feature."; + // Present only when the split routes a feature to EVALUATION, so FlagEvalLoggingHook can skip + // scanning the metadata on every other evaluation. + static final String METADATA_HAS_EVALUATION_FEATURES = "__dd_has_evaluation_features"; + + static final String FEATURE_DESTINATION_HOOK = "HOOK"; + static final String FEATURE_DESTINATION_EXPOSURE = "EXPOSURE"; + static final String FEATURE_DESTINATION_EVALUATION = "EVALUATION"; + // A newer UFC can name destinations this SDK does not know; warn once per name, not per + // evaluation. + static final int MAX_WARNED_FEATURE_DESTINATIONS = 64; + static final Set WARNED_FEATURE_DESTINATIONS = ConcurrentHashMap.newKeySet(); // Stamped on every DD-produced evaluation (including PROVIDER_NOT_READY, with false). Missing // key = non-DD provider; the hook falls back to false (fail-closed). @@ -611,6 +664,9 @@ private static ProviderEvaluation resolveVariant( } metadataBuilder.addBoolean(METADATA_DO_LOG, doLog); } + if (SPLIT_FEATURES_SUPPORTED.get()) { + addSplitFeatures(metadataBuilder, split); + } if (doLog) { metadataBuilder.addBoolean( METADATA_EXPOSURE_CACHE_HIT, @@ -636,6 +692,69 @@ private static ProviderEvaluation resolveVariant( return result; } + private static void addSplitFeatures( + final ImmutableMetadata.ImmutableMetadataBuilder metadataBuilder, final Split split) { + if (split.features == null) { + return; + } + Set seenKeys = null; + boolean evaluationFeatures = false; + for (final Object item : split.features) { + if (!(item instanceof Feature)) { + continue; + } + final Feature feature = (Feature) item; + if (feature.key == null || feature.destinations == null) { + continue; + } + if (seenKeys == null) { + seenKeys = new HashSet<>(); + } + if (!seenKeys.add(feature.key)) { + continue; + } + for (final String destination : feature.destinations) { + final String prefix = metadataPrefix(destination); + if (prefix != null) { + addFeature(metadataBuilder, prefix + feature.key, feature.value); + evaluationFeatures |= prefix.equals(METADATA_EVALUATION_FEATURE_PREFIX); + } else if (WARNED_FEATURE_DESTINATIONS.size() < MAX_WARNED_FEATURE_DESTINATIONS + && WARNED_FEATURE_DESTINATIONS.add(destination)) { + log.warn("Ignoring unknown feature destination {}", destination); + } + } + } + if (evaluationFeatures) { + metadataBuilder.addBoolean(METADATA_HAS_EVALUATION_FEATURES, true); + } + } + + private static String metadataPrefix(final String destination) { + if (FEATURE_DESTINATION_HOOK.equals(destination)) { + return METADATA_FEATURE_PREFIX; + } + if (FEATURE_DESTINATION_EXPOSURE.equals(destination)) { + return METADATA_EXPOSURE_FEATURE_PREFIX; + } + if (FEATURE_DESTINATION_EVALUATION.equals(destination)) { + return METADATA_EVALUATION_FEATURE_PREFIX; + } + return null; + } + + private static void addFeature( + final ImmutableMetadata.ImmutableMetadataBuilder metadataBuilder, + final String key, + final Object value) { + if (value instanceof String) { + metadataBuilder.addString(key, (String) value); + } else if (value instanceof Boolean) { + metadataBuilder.addBoolean(key, (Boolean) value); + } else if (value instanceof Number) { + metadataBuilder.addDouble(key, ((Number) value).doubleValue()); + } + } + /** The serial id an exposure carries: none when the agent's exposure event predates it. */ static Integer exposureSerialId(final Integer splitSerialId) { return EXPOSURE_SERIAL_ID_SUPPORTED.get() && SPLIT_SERIAL_ID_SUPPORTED.get() @@ -715,7 +834,8 @@ static ExposureEvent exposureEvent( final String allocationKey, final String variantKey, final EvaluationContext context, - final Integer serialId) { + final Integer serialId, + final Map features) { final long timestamp = System.currentTimeMillis(); // Exposure types share names with the imported UFC Allocation, Flag, and Variant types. final datadog.trace.api.featureflag.exposure.Allocation allocation = @@ -726,9 +846,14 @@ static ExposureEvent exposureEvent( new datadog.trace.api.featureflag.exposure.Variant(variantKey); final Subject subject = new Subject(context.getTargetingKey(), flattenContext(context)); - return EXPOSURE_SERIAL_ID_SUPPORTED.get() - ? new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject, serialId) - : new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject); + if (!EXPOSURE_SERIAL_ID_SUPPORTED.get()) { + return new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject); + } + if (features.isEmpty() || !EXPOSURE_FEATURES_SUPPORTED.get()) { + return new ExposureEvent(timestamp, allocation, exposureFlag, variant, subject, serialId); + } + return new ExposureEvent( + timestamp, allocation, exposureFlag, variant, subject, serialId, features); } static AbstractMap flattenContext(final EvaluationContext context) { diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DatadogExposureLoggingGate.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DatadogExposureLoggingGate.java new file mode 100644 index 00000000000..41585f4c929 --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DatadogExposureLoggingGate.java @@ -0,0 +1,32 @@ +package datadog.trace.api.openfeature; + +import datadog.trace.api.featureflag.config.FeatureFlaggingConfig; +import datadog.trace.bootstrap.config.provider.ConfigProvider; +import java.util.function.BooleanSupplier; + +/** + * Reads {@code DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED} with full {@link ConfigProvider} + * precedence. When false, exposures are not sent to Datadog, but the exposure cache and customer + * {@link ExposureHook}s keep working. On by default. + */ +final class DatadogExposureLoggingGate { + + private DatadogExposureLoggingGate() {} + + static boolean isEnabled() { + return isEnabled( + () -> + ConfigProvider.getInstance() + .getBoolean( + FeatureFlaggingConfig.FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED, true)); + } + + static boolean isEnabled(final BooleanSupplier setting) { + try { + return setting.getAsBoolean(); + } catch (final Throwable t) { + // Keep the default rather than silently stop sending exposures. + return true; + } + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java new file mode 100644 index 00000000000..01f8a479c53 --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java @@ -0,0 +1,143 @@ +package datadog.trace.api.openfeature; + +import static java.util.Objects.requireNonNull; + +import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.Hook; +import dev.openfeature.sdk.HookContext; +import dev.openfeature.sdk.ImmutableMetadata; +import java.util.Collections; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * An OpenFeature hook that hands each completed flag evaluation to a callback, with the Datadog + * exposure decision, so an application can record exposures in its own data store. + * + *

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

Register it with {@code OpenFeatureAPI.getInstance().addHooks(new ExposureHook(callback))} or + * on a client. The callback runs for every evaluation, including errors and repeats. Use {@link + * Evaluation#shouldSend()} to keep the new or changed exposures. A changed serial id counts as a + * change only with {@code dd-java-agent} 1.67.0 or later; with an older agent, only a changed + * allocation or variant does. + * + *

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

The callback runs on the evaluation thread: hand the data to a background queue instead of + * writing it from the callback. An exception from the callback does not affect the evaluation. The + * first one is logged at warn level, later ones at debug level. + */ +public final class ExposureHook implements Hook { + + private static final Logger log = LoggerFactory.getLogger(ExposureHook.class); + + private final Consumer callback; + private final AtomicBoolean warned = new AtomicBoolean(); + + public ExposureHook(final Consumer callback) { + this.callback = requireNonNull(callback, "callback"); + } + + @Override + public void finallyAfter( + final HookContext context, + final FlagEvaluationDetails details, + final Map hints) { + if (details == null) { + return; + } + try { + callback.accept(new Evaluation(context.getCtx(), details)); + } catch (final LinkageError | RuntimeException e) { + if (warned.compareAndSet(false, true)) { + log.warn("Exposure callback failed; later failures are logged at debug level", e); + } else { + log.debug("Exposure callback failed", e); + } + } + } + + /** A completed evaluation and its Datadog exposure decision. */ + public static final class Evaluation { + private final EvaluationContext context; + private final FlagEvaluationDetails details; + + Evaluation(final EvaluationContext context, final FlagEvaluationDetails details) { + this.context = context; + this.details = details; + } + + /** The evaluation context, including the targeting key. */ + public EvaluationContext getContext() { + return context; + } + + /** The flag key, value, variant, error and flag metadata of the evaluation. */ + public FlagEvaluationDetails getDetails() { + return details; + } + + /** + * True when the evaluation succeeded and selected an allocation that logs exposures, whether or + * not this subject was already exposed. + */ + public boolean isExposure() { + final Map values = metadata(); + return details.getErrorCode() == null + && details.getVariant() != null + && values.get("allocationKey") instanceof String + && Boolean.TRUE.equals(values.get(DDEvaluator.METADATA_DO_LOG)); + } + + /** + * True when this exposure repeats one already recorded for this subject: same allocation, + * variant and serial id. False for a new or changed exposure, and for an evaluation that is not + * an exposure. + */ + public boolean isCacheHit() { + return isExposure() && Boolean.TRUE.equals(cacheHitStamp()); + } + + /** + * True for a new or changed exposure: the decision the provider uses for Datadog. The class + * Javadoc says when that decision reaches Datadog. + */ + public boolean shouldSend() { + return isExposure() && Boolean.FALSE.equals(cacheHitStamp()); + } + + private Object cacheHitStamp() { + return metadata().get(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT); + } + + // The ImmutableMetadata getters build a debug message for every absent key. + private Map metadata() { + final ImmutableMetadata metadata = details.getFlagMetadata(); + return metadata == null + ? Collections.emptyMap() + : metadata.asUnmodifiableMap(); + } + + /** + * The features of the selected split, such as {@code holdout.key} and {@code + * holdout.assignment_group}. Values are a String, Double or Boolean. Empty when the split has + * none. + */ + public Map getFeatures() { + return SplitFeatureMetadata.featuresWithPrefix( + details.getFlagMetadata(), DDEvaluator.METADATA_FEATURE_PREFIX); + } + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java index 7da34ca7c0c..67ab6b27cfd 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureLoggingHook.java @@ -7,6 +7,9 @@ import dev.openfeature.sdk.HookContext; import dev.openfeature.sdk.ImmutableMetadata; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Sends the exposure for a logged allocation to the agent once the evaluation has succeeded. @@ -18,13 +21,21 @@ */ class ExposureLoggingHook implements Hook { + private static final Logger log = LoggerFactory.getLogger(ExposureLoggingHook.class); + private static final AtomicBoolean WARNED = new AtomicBoolean(); + static final ExposureLoggingHook INSTANCE = - new ExposureLoggingHook<>(ExposureDeduplicationCache.INSTANCE); + new ExposureLoggingHook<>( + ExposureDeduplicationCache.INSTANCE, DatadogExposureLoggingGate.isEnabled()); private final ExposureDeduplicationCache cache; + // When false the hook still records exposures, so customer ExposureHooks see the same cache + // verdicts whether or not exposures go to Datadog. + private final boolean sendToDatadog; - ExposureLoggingHook(final ExposureDeduplicationCache cache) { + ExposureLoggingHook(final ExposureDeduplicationCache cache, final boolean sendToDatadog) { this.cache = cache; + this.sendToDatadog = sendToDatadog; } @Override @@ -60,10 +71,22 @@ public void finallyAfter( DDEvaluator.exposureSerialId( serialValue instanceof Integer ? (Integer) serialValue : null); cache.record(flag, context.getTargetingKey(), allocationKey, variantKey, serialId); - FeatureFlaggingGateway.dispatch( - DDEvaluator.exposureEvent(flag, allocationKey, variantKey, context, serialId)); + if (sendToDatadog) { + FeatureFlaggingGateway.dispatch( + DDEvaluator.exposureEvent( + flag, + allocationKey, + variantKey, + context, + serialId, + SplitFeatureMetadata.featuresWithPrefix( + metadata, DDEvaluator.METADATA_EXPOSURE_FEATURE_PREFIX))); + } } catch (LinkageError e) { // Never let exposure reporting break flag evaluation. + if (WARNED.compareAndSet(false, true)) { + log.warn("Could not send a feature flag exposure; later failures are not logged", e); + } } } } diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/FlagEvalLoggingHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/FlagEvalLoggingHook.java index 322f11ac9e1..41426fd3683 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/FlagEvalLoggingHook.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/FlagEvalLoggingHook.java @@ -9,7 +9,10 @@ import dev.openfeature.sdk.ImmutableMetadata; import java.util.Collections; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * OpenFeature Hook that captures flag evaluation events for EVP flagevaluation emission. @@ -33,6 +36,33 @@ */ class FlagEvalLoggingHook implements Hook { + private static final Logger log = LoggerFactory.getLogger(FlagEvalLoggingHook.class); + + static final AtomicBoolean FEATURES_SUPPORTED = + new AtomicBoolean(featuresSupported(FlagEvalEvent.class)); + + /** + * An older agent's flag-evaluation event has no features; its rows are then sent without them. + */ + static boolean featuresSupported(final Class eventClass) { + try { + eventClass.getConstructor( + String.class, + String.class, + String.class, + String.class, + String.class, + long.class, + boolean.class, + Map.class, + Map.class); + return true; + } catch (final NoSuchMethodException | LinkageError | RuntimeException e) { + log.debug("The installed Datadog Java agent does not send features on flag evaluations", e); + return false; + } + } + /** * Singleton instance: always registered when the provider is created; harmless when writer=null * (killswitch off or not yet started). @@ -98,11 +128,17 @@ public void finallyAfter( final ImmutableMetadata metadata = details.getFlagMetadata(); // allocationKey: "allocationKey" (camelCase) - consistent with FlagEvalMetricsHook.java - final String allocationKey = metadata != null ? metadata.getString("allocationKey") : null; + // The ImmutableMetadata getters build a debug message for every absent key. + final Map values = + metadata == null ? Collections.emptyMap() : metadata.asUnmodifiableMap(); + final Object allocationValue = values.get("allocationKey"); + final String allocationKey = + allocationValue instanceof String ? (String) allocationValue : null; // eval-time: from flag metadata "__dd_eval_timestamp_ms" (Long), fallback to hook-fire time. // ImmutableMetadata.getLong available since sdk 1.4+. - final Long evalTimeObj = metadata != null ? metadata.getLong("__dd_eval_timestamp_ms") : null; + final Object evalTimeValue = values.get("__dd_eval_timestamp_ms"); + final Long evalTimeObj = evalTimeValue instanceof Long ? (Long) evalTimeValue : null; final long evalTimeMs = evalTimeObj != null ? evalTimeObj : System.currentTimeMillis(); // variant: the OpenFeature variant key (same source as the OTel FlagEvalMetricsHook), NOT the @@ -115,10 +151,9 @@ public void finallyAfter( // Consent is read from metadata stamped by DDEvaluator (pinned to its ServerConfiguration). // Missing key = non-DD provider → false, the privacy-preserving default. + final Object consentValue = values.get(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA); final Boolean consentFromMetadata = - metadata != null - ? metadata.getBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA) - : null; + consentValue instanceof Boolean ? (Boolean) consentValue : null; final boolean observeFullEvaluationData = consentFromMetadata != null && consentFromMetadata; // Error message: prefer the human-readable message under consent-on; under consent-off the @@ -151,16 +186,34 @@ public void finallyAfter( attrs = Collections.emptyMap(); } + // Features come from the flag configuration, not the evaluation context, so they are sent + // whatever the consent. + final Map features = + Boolean.TRUE.equals(values.get(DDEvaluator.METADATA_HAS_EVALUATION_FEATURES)) + ? SplitFeatureMetadata.featuresWithPrefix( + metadata, DDEvaluator.METADATA_EVALUATION_FEATURE_PREFIX) + : Collections.emptyMap(); w.enqueue( - new FlagEvalEvent( - flagKey, - variant, - allocationKey, - targetingKey, - errorMessage, - evalTimeMs, - observeFullEvaluationData, - attrs)); + features.isEmpty() || !FEATURES_SUPPORTED.get() + ? new FlagEvalEvent( + flagKey, + variant, + allocationKey, + targetingKey, + errorMessage, + evalTimeMs, + observeFullEvaluationData, + attrs) + : new FlagEvalEvent( + flagKey, + variant, + allocationKey, + targetingKey, + errorMessage, + evalTimeMs, + observeFullEvaluationData, + attrs, + features)); } catch (LinkageError e) { // Never let EVP recording break flag evaluation } diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/Provider.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/Provider.java index 3a5f07fdf53..436f4facb4a 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/Provider.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/Provider.java @@ -105,7 +105,7 @@ public Provider(final Options options) { hooks.add(spanEnrichmentHook); } try { - hooks.add(ExposureLoggingHook.INSTANCE); + hooks.add(buildExposureLoggingHook()); } catch (LinkageError e) { log.warn("Feature flag exposure reporting is unavailable with the installed agent", e); } @@ -235,6 +235,10 @@ Hook buildFlagEvalLoggingHook() { return FlagEvalLoggingHook.INSTANCE; } + Hook buildExposureLoggingHook() { + return ExposureLoggingHook.INSTANCE; + } + @Override public void shutdown() { if (flagEvalMetrics != null) { diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/SplitFeatureMetadata.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/SplitFeatureMetadata.java new file mode 100644 index 00000000000..ac24054871f --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/SplitFeatureMetadata.java @@ -0,0 +1,36 @@ +package datadog.trace.api.openfeature; + +import dev.openfeature.sdk.ImmutableMetadata; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +/** + * Reads the split features that {@link DDEvaluator} copies into flag metadata. It depends only on + * the OpenFeature API, so {@link ExposureHook} keeps working where the Datadog Java agent is + * absent. + */ +final class SplitFeatureMetadata { + + private SplitFeatureMetadata() {} + + /** The features stored in the metadata under {@code prefix}, keyed without the prefix. */ + static Map featuresWithPrefix( + final ImmutableMetadata metadata, final String prefix) { + if (metadata == null) { + return Collections.emptyMap(); + } + Map features = null; + for (final Map.Entry entry : metadata.asUnmodifiableMap().entrySet()) { + if (entry.getKey().startsWith(prefix)) { + if (features == null) { + features = new HashMap<>(); + } + features.put(entry.getKey().substring(prefix.length()), entry.getValue()); + } + } + return features == null + ? Collections.emptyMap() + : Collections.unmodifiableMap(features); + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DatadogExposureLoggingDisabledForkedTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DatadogExposureLoggingDisabledForkedTest.java new file mode 100644 index 00000000000..6494203d798 --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DatadogExposureLoggingDisabledForkedTest.java @@ -0,0 +1,86 @@ +package datadog.trace.api.openfeature; + +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import datadog.trace.api.featureflag.FeatureFlaggingGateway; +import datadog.trace.api.featureflag.exposure.ExposureEvent; +import datadog.trace.api.featureflag.ufc.v1.Allocation; +import datadog.trace.api.featureflag.ufc.v1.Flag; +import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; +import datadog.trace.api.featureflag.ufc.v1.Split; +import datadog.trace.api.featureflag.ufc.v1.ValueType; +import datadog.trace.api.featureflag.ufc.v1.Variant; +import datadog.trace.api.openfeature.Provider.Options; +import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.MutableContext; +import dev.openfeature.sdk.OpenFeatureAPI; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +/** + * Turns off Datadog exposure logging through the real setting. The gate is read once, when + * ExposureLoggingHook loads, so the property is set before that and the forked task gives this + * class its own JVM. + */ +class DatadogExposureLoggingDisabledForkedTest { + + static { + System.setProperty("dd.feature.flags.exposures.datadog.logging.enabled", "false"); + } + + private final List dispatched = new ArrayList<>(); + private final FeatureFlaggingGateway.ExposureListener listener = dispatched::add; + + @AfterEach + void tearDown() { + FeatureFlaggingGateway.removeExposureListener(listener); + OpenFeatureAPI.getInstance().clearHooks(); + OpenFeatureAPI.getInstance().shutdown(); + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + ExposureDeduplicationCache.INSTANCE.clear(); + } + + @Test + void sendsNoExposuresToDatadogWhileExposureHooksKeepTheirDecisions() throws Exception { + final List evaluations = new ArrayList<>(); + FeatureFlaggingGateway.dispatch(configuration()); + final OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProviderAndWait( + new Provider( + new Options().initTimeout(10, SECONDS), new DDEvaluator(mock(Runnable.class)))); + api.addHooks(new ExposureHook(evaluations::add)); + FeatureFlaggingGateway.addExposureListener(listener); + final Client client = api.getClient(); + + client.getIntegerDetails("flag", 0, new MutableContext("user-1")); + client.getIntegerDetails("flag", 0, new MutableContext("user-1")); + + assertTrue(dispatched.isEmpty(), "nothing is sent to Datadog"); + assertEquals(2, evaluations.size()); + assertTrue(evaluations.get(0).shouldSend()); + assertFalse(evaluations.get(1).shouldSend(), "the cache still records the first exposure"); + } + + private static ServerConfiguration configuration() { + final Map variations = new HashMap<>(); + variations.put("on", new Variant("on", 1)); + final Split split = new Split(emptyList(), "on", emptyMap(), 7); + final Allocation allocation = + new Allocation("allocation", null, null, null, singletonList(split), Boolean.TRUE); + final Map flags = new HashMap<>(); + flags.put( + "flag", new Flag("flag", true, ValueType.INTEGER, variations, singletonList(allocation))); + return new ServerConfiguration("", "", false, null, flags); + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java new file mode 100644 index 00000000000..d76b8d68b8f --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java @@ -0,0 +1,582 @@ +package datadog.trace.api.openfeature; + +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import datadog.trace.api.featureflag.FeatureFlaggingGateway; +import datadog.trace.api.featureflag.exposure.ExposureEvent; +import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; +import datadog.trace.api.featureflag.flagevaluation.FlagEvaluationWriter; +import datadog.trace.api.featureflag.ufc.v1.Allocation; +import datadog.trace.api.featureflag.ufc.v1.Feature; +import datadog.trace.api.featureflag.ufc.v1.Flag; +import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; +import datadog.trace.api.featureflag.ufc.v1.Split; +import datadog.trace.api.featureflag.ufc.v1.ValueType; +import datadog.trace.api.featureflag.ufc.v1.Variant; +import datadog.trace.api.openfeature.Provider.Options; +import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.ErrorCode; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.FlagValueType; +import dev.openfeature.sdk.Hook; +import dev.openfeature.sdk.HookContext; +import dev.openfeature.sdk.ImmutableMetadata; +import dev.openfeature.sdk.MutableContext; +import dev.openfeature.sdk.OpenFeatureAPI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class ExposureHookTest { + + private final List sentToDatadog = new ArrayList<>(); + private final FeatureFlaggingGateway.ExposureListener listener = sentToDatadog::add; + private final List evaluations = new ArrayList<>(); + private Client client; + + @BeforeEach + void setUp() { + ExposureDeduplicationCache.INSTANCE.clear(); + FeatureFlaggingGateway.addExposureListener(listener); + } + + @AfterEach + void tearDown() { + FeatureFlaggingGateway.removeExposureListener(listener); + OpenFeatureAPI.getInstance().clearHooks(); + OpenFeatureAPI.getInstance().shutdown(); + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + ExposureDeduplicationCache.INSTANCE.clear(); + } + + @Test + void shouldSendMatchesTheExposuresSentToDatadog() throws Exception { + start(true, configuration(Boolean.TRUE, null)); + + evaluate("user-1"); + evaluate("user-1"); + evaluate("user-2"); + + assertEquals(Arrays.asList(true, false, true), shouldSend()); + assertEquals(2, sentToDatadog.size()); + assertEquals("user-1", sentToDatadog.get(0).subject.id); + assertEquals("user-2", sentToDatadog.get(1).subject.id); + } + + @Test + void customerHookStillReceivesExposuresWhenDatadogLoggingIsOff() throws Exception { + start(false, configuration(Boolean.TRUE, null)); + + evaluate("user-1"); + evaluate("user-1"); + evaluate("user-2"); + + assertTrue(sentToDatadog.isEmpty()); + assertEquals(Arrays.asList(true, false, true), shouldSend()); + } + + @Test + void aFailedEvaluationIsNotAnExposure() throws Exception { + start(true, configuration(Boolean.TRUE, null)); + OpenFeatureAPI.getInstance() + .addHooks( + new Hook() { + @Override + public void after( + final HookContext ctx, + final FlagEvaluationDetails details, + final Map hints) { + throw new IllegalStateException("application validation failed"); + } + }); + + final FlagEvaluationDetails details = evaluate("user-1"); + + assertEquals(ErrorCode.GENERAL, details.getErrorCode()); + final ExposureHook.Evaluation evaluation = evaluations.get(0); + assertFalse(evaluation.isExposure()); + assertFalse(evaluation.isCacheHit()); + assertFalse(evaluation.shouldSend()); + } + + @Test + void anAllocationThatDoesNotLogExposuresIsNotAnExposure() throws Exception { + start(true, configuration(Boolean.FALSE, null)); + + evaluate("user-1"); + + final ExposureHook.Evaluation evaluation = evaluations.get(0); + assertFalse(evaluation.isExposure()); + assertFalse(evaluation.isCacheHit()); + assertFalse(evaluation.shouldSend()); + assertEquals("flag", evaluation.getDetails().getFlagKey()); + assertEquals("user-1", evaluation.getContext().getTargetingKey()); + } + + @Test + void exposesTheFeaturesOfTheSelectedSplit() throws Exception { + start( + true, + configuration( + Boolean.TRUE, + Arrays.asList( + new Feature("holdout.key", "q4-global", singletonList("HOOK")), + new Feature("holdout.assignment_group", "status_quo", singletonList("HOOK")), + new Feature("holdout.weight", 0.5, Arrays.asList("EVALUATION", "HOOK")), + new Feature( + "holdout.should_include_in_holdout_analysis", + true, + Arrays.asList("HOOK", "SOME_FUTURE_DESTINATION")), + new Feature("bandit.policy_id", "p2", Arrays.asList("EXPOSURE", "EVALUATION")), + new Feature("unknown.only", "x", singletonList("SOME_FUTURE_DESTINATION"))))); + + final FlagEvaluationDetails details = evaluate("user-1"); + + final Map features = evaluations.get(0).getFeatures(); + assertEquals(4, features.size()); + assertFalse( + features.containsKey("bandit.policy_id"), + "a feature without HOOK is not delivered to hooks"); + assertFalse( + features.containsKey("unknown.only"), + "a feature with only unknown destinations goes nowhere"); + assertEquals("q4-global", features.get("holdout.key")); + assertEquals("status_quo", features.get("holdout.assignment_group")); + assertEquals(0.5, features.get("holdout.weight")); + assertEquals(true, features.get("holdout.should_include_in_holdout_analysis")); + assertEquals( + "q4-global", + details.getFlagMetadata().getString(DDEvaluator.METADATA_FEATURE_PREFIX + "holdout.key")); + assertThrows(UnsupportedOperationException.class, () -> features.put("other", "value")); + } + + @Test + void warnsOnceForEachUnknownDestination() throws Exception { + DDEvaluator.WARNED_FEATURE_DESTINATIONS.clear(); + start( + true, + configuration( + Boolean.TRUE, + Arrays.asList( + new Feature("holdout.key", "q4-global", Arrays.asList("HOOK", "NEW_A")), + new Feature("other", "x", Arrays.asList("NEW_A", "NEW_B", "EXPOSURE"))))); + + evaluate("user-1"); + evaluate("user-2"); + + assertEquals( + new HashSet<>(Arrays.asList("NEW_A", "NEW_B")), DDEvaluator.WARNED_FEATURE_DESTINATIONS); + assertEquals(1, evaluations.get(1).getFeatures().size()); + DDEvaluator.WARNED_FEATURE_DESTINATIONS.clear(); + } + + @Test + void stopsRememberingUnknownDestinationsAtTheCap() throws Exception { + DDEvaluator.WARNED_FEATURE_DESTINATIONS.clear(); + for (int i = 0; i < DDEvaluator.MAX_WARNED_FEATURE_DESTINATIONS; i++) { + DDEvaluator.WARNED_FEATURE_DESTINATIONS.add("OLD_" + i); + } + try { + start( + true, + configuration( + Boolean.TRUE, + singletonList( + new Feature("holdout.key", "q4-global", Arrays.asList("HOOK", "NEW_A"))))); + + evaluate("user-1"); + + assertEquals( + DDEvaluator.MAX_WARNED_FEATURE_DESTINATIONS, + DDEvaluator.WARNED_FEATURE_DESTINATIONS.size()); + assertFalse(DDEvaluator.WARNED_FEATURE_DESTINATIONS.contains("NEW_A")); + assertEquals( + Collections.singletonMap("holdout.key", "q4-global"), evaluations.get(0).getFeatures()); + } finally { + DDEvaluator.WARNED_FEATURE_DESTINATIONS.clear(); + } + } + + @Test + void routesEachFeatureToItsDestinations() throws Exception { + start( + true, + configuration( + Boolean.TRUE, + Arrays.asList( + new Feature("holdout.key", "q4-global", singletonList("HOOK")), + new Feature("bandit.arm", "a", singletonList("EXPOSURE")), + new Feature("experiment.tag", "pricing", singletonList("EVALUATION")), + new Feature("bandit.policy_id", "p2", Arrays.asList("EXPOSURE", "EVALUATION"))))); + + final FlagEvaluationDetails details = evaluate("user-1"); + + final ImmutableMetadata metadata = details.getFlagMetadata(); + assertEquals( + Collections.singletonMap("holdout.key", "q4-global"), + SplitFeatureMetadata.featuresWithPrefix(metadata, DDEvaluator.METADATA_FEATURE_PREFIX)); + final Map exposureFeatures = new HashMap<>(); + exposureFeatures.put("bandit.arm", "a"); + exposureFeatures.put("bandit.policy_id", "p2"); + assertEquals( + exposureFeatures, + SplitFeatureMetadata.featuresWithPrefix( + metadata, DDEvaluator.METADATA_EXPOSURE_FEATURE_PREFIX)); + final Map evaluationFeatures = new HashMap<>(); + evaluationFeatures.put("experiment.tag", "pricing"); + evaluationFeatures.put("bandit.policy_id", "p2"); + assertEquals( + evaluationFeatures, + SplitFeatureMetadata.featuresWithPrefix( + metadata, DDEvaluator.METADATA_EVALUATION_FEATURE_PREFIX)); + assertEquals( + Collections.singletonMap("holdout.key", "q4-global"), evaluations.get(0).getFeatures()); + assertEquals(exposureFeatures, sentToDatadog.get(0).features); + } + + @Test + void aRepeatedKeyKeepsTheFirstFeatureForEveryDestination() throws Exception { + start( + true, + configuration( + Boolean.TRUE, + Arrays.asList( + new Feature("holdout.key", "q4-global", singletonList("HOOK")), + new Feature("holdout.key", "q1-global", Arrays.asList("HOOK", "EXPOSURE"))))); + + final FlagEvaluationDetails details = evaluate("user-1"); + + assertEquals( + Collections.singletonMap("holdout.key", "q4-global"), evaluations.get(0).getFeatures()); + assertTrue( + SplitFeatureMetadata.featuresWithPrefix( + details.getFlagMetadata(), DDEvaluator.METADATA_EXPOSURE_FEATURE_PREFIX) + .isEmpty(), + "the repeated feature's EXPOSURE destination is ignored with it"); + assertNull(sentToDatadog.get(0).features); + } + + @Test + void sendsEvaluationFeaturesForAnAllocationThatDoesNotLogExposures() throws Exception { + final AtomicReference captured = new AtomicReference<>(); + FeatureFlaggingGateway.setFlagEvalWriter(capturingWriter(captured)); + try { + start( + true, + configuration( + Boolean.FALSE, + singletonList(new Feature("bandit.policy_id", "p2", singletonList("EVALUATION"))))); + + evaluate("user-1"); + + assertEquals(Collections.singletonMap("bandit.policy_id", "p2"), captured.get().features); + assertTrue(sentToDatadog.isEmpty(), "the allocation does not log exposures"); + } finally { + FeatureFlaggingGateway.setFlagEvalWriter(null); + } + } + + @Test + void sendsExposuresWithoutFeaturesWhenTheAgentLacksThem() throws Exception { + start( + true, + configuration( + Boolean.TRUE, + singletonList(new Feature("bandit.policy_id", "p2", singletonList("EXPOSURE"))))); + DDEvaluator.EXPOSURE_FEATURES_SUPPORTED.set(false); + try { + evaluate("user-1"); + } finally { + DDEvaluator.EXPOSURE_FEATURES_SUPPORTED.set(true); + } + + assertEquals(1, sentToDatadog.size()); + assertNull(sentToDatadog.get(0).features); + } + + @Test + void featureSupportChecksRejectEventsFromAnOlderAgent() { + assertTrue(DDEvaluator.exposureFeaturesSupported(ExposureEvent.class)); + assertFalse(DDEvaluator.exposureFeaturesSupported(SerialIdExposureEvent.class)); + assertTrue(FlagEvalLoggingHook.featuresSupported(FlagEvalEvent.class)); + assertFalse(FlagEvalLoggingHook.featuresSupported(LegacyFlagEvalEvent.class)); + } + + /** An exposure event from an agent that has serial ids but predates features. */ + public static final class SerialIdExposureEvent { + public SerialIdExposureEvent( + final long timestamp, + final datadog.trace.api.featureflag.exposure.Allocation allocation, + final datadog.trace.api.featureflag.exposure.Flag flag, + final datadog.trace.api.featureflag.exposure.Variant variant, + final datadog.trace.api.featureflag.exposure.Subject subject, + final Integer serialId) {} + } + + /** A flag-evaluation event from an agent that predates features. */ + public static final class LegacyFlagEvalEvent { + public LegacyFlagEvalEvent( + final String flagKey, + final String variant, + final String allocationKey, + final String targetingKey, + final String errorMessage, + final long evalTimeMs, + final boolean observeFullEvaluationData, + final Map attrs) {} + } + + @Test + void evaluatesWithoutFeaturesWhenTheAgentSplitHasNone() throws Exception { + start( + true, + configuration( + Boolean.TRUE, + singletonList(new Feature("holdout.key", "q4-global", singletonList("HOOK"))))); + DDEvaluator.SPLIT_FEATURES_SUPPORTED.set(false); + final FlagEvaluationDetails details; + try { + details = evaluate("user-1"); + } finally { + DDEvaluator.SPLIT_FEATURES_SUPPORTED.set(true); + } + + assertNull(details.getErrorCode()); + assertEquals(Integer.valueOf(1), details.getValue()); + assertTrue(evaluations.get(0).getFeatures().isEmpty()); + } + + @Test + void hasNoFeaturesWhenTheSplitHasNone() throws Exception { + start(true, configuration(Boolean.TRUE, null)); + + evaluate("user-1"); + + assertTrue(evaluations.get(0).getFeatures().isEmpty()); + } + + @Test + void aFailingCallbackDoesNotAffectTheEvaluation() throws Exception { + start(true, configuration(Boolean.TRUE, null)); + OpenFeatureAPI.getInstance() + .addHooks( + new ExposureHook( + evaluation -> { + throw new IllegalStateException("warehouse unavailable"); + })); + + final FlagEvaluationDetails details = evaluate("user-1"); + + assertNull(details.getErrorCode()); + assertEquals(Integer.valueOf(1), details.getValue()); + assertEquals(1, sentToDatadog.size()); + } + + @Test + void anEvaluationIsAnExposureOnlyWithEverythingAnExposureNeeds() { + assertFalse(evaluation("on", null).isExposure()); + assertFalse(evaluation(null, exposureMetadata("allocation", true, false)).isExposure()); + assertFalse(evaluation("on", exposureMetadata(null, true, false)).isExposure()); + assertFalse(evaluation("on", exposureMetadata("allocation", false, false)).isExposure()); + assertTrue(evaluation("on", exposureMetadata("allocation", true, false)).shouldSend()); + assertFalse(evaluation("on", exposureMetadata("allocation", true, false)).isCacheHit()); + assertTrue(evaluation("on", exposureMetadata("allocation", true, true)).isCacheHit()); + assertFalse( + evaluation("on", exposureMetadata("allocation", false, true)).isCacheHit(), + "an evaluation that is not an exposure is never a cache hit"); + assertFalse(evaluation("on", exposureMetadata("allocation", true, true)).shouldSend()); + } + + @Test + void featuresIgnoreOtherMetadataAndAreEmptyWithoutMetadata() { + assertTrue(evaluation("on", null).getFeatures().isEmpty()); + final ExposureHook.Evaluation evaluation = + evaluation( + "on", + ImmutableMetadata.builder() + .addString("allocationKey", "allocation") + .addString(DDEvaluator.METADATA_FEATURE_PREFIX + "holdout.key", "q4-global") + .build()); + + assertEquals(1, evaluation.getFeatures().size()); + assertEquals("q4-global", evaluation.getFeatures().get("holdout.key")); + } + + @Test + void datadogLoggingStaysOnWhenItsSettingCannotBeRead() { + assertTrue(DatadogExposureLoggingGate.isEnabled()); + assertFalse(DatadogExposureLoggingGate.isEnabled(() -> false)); + assertTrue( + DatadogExposureLoggingGate.isEnabled( + () -> { + throw new NoClassDefFoundError("old agent"); + })); + } + + @Test + void theHookContainsCallbackFailures() { + final HookContext context = + HookContext.builder() + .flagKey("flag") + .type(FlagValueType.INTEGER) + .defaultValue(0) + .ctx(new MutableContext("user-1")) + .build(); + final FlagEvaluationDetails details = + FlagEvaluationDetails.builder().flagKey("flag").value(1).build(); + + assertDoesNotThrow( + () -> + new ExposureHook( + evaluation -> { + throw new IllegalStateException("warehouse unavailable"); + }) + .finallyAfter(context, details, emptyMap())); + assertDoesNotThrow( + () -> + new ExposureHook( + evaluation -> { + throw new NoClassDefFoundError("missing class"); + }) + .finallyAfter(context, details, emptyMap())); + } + + @Test + void requiresACallback() { + assertThrows(NullPointerException.class, () -> new ExposureHook(null)); + } + + @Test + void skipsTheCallbackWhenThereAreNoDetails() { + final List seen = new ArrayList<>(); + final HookContext context = + HookContext.builder() + .flagKey("flag") + .type(FlagValueType.INTEGER) + .defaultValue(0) + .ctx(new MutableContext("user-1")) + .build(); + + new ExposureHook(seen::add).finallyAfter(context, null, Collections.emptyMap()); + + assertTrue(seen.isEmpty()); + } + + @Test + void featuresAreSkippedWithAnAgentWhoseSplitHasNoFeatures() { + assertTrue(DDEvaluator.splitFeaturesSupported(Split.class)); + assertFalse(DDEvaluator.splitFeaturesSupported(LegacySplit.class)); + } + + /** A Split from an agent that predates features. */ + static final class LegacySplit { + public Integer serialId; + } + + private void start(final boolean sendToDatadog, final ServerConfiguration configuration) + throws Exception { + FeatureFlaggingGateway.dispatch(configuration); + final OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProviderAndWait( + new Provider( + new Options().initTimeout(10, SECONDS), new DDEvaluator(mock(Runnable.class))) { + @Override + Hook buildExposureLoggingHook() { + return new ExposureLoggingHook<>(ExposureDeduplicationCache.INSTANCE, sendToDatadog); + } + }); + api.addHooks(new ExposureHook(evaluations::add)); + client = api.getClient(); + } + + private static ExposureHook.Evaluation evaluation( + final String variant, final ImmutableMetadata metadata) { + final FlagEvaluationDetails.FlagEvaluationDetailsBuilder builder = + FlagEvaluationDetails.builder().flagKey("flag").value(1).variant(variant); + if (metadata != null) { + builder.flagMetadata(metadata); + } + return new ExposureHook.Evaluation(new MutableContext("user-1"), builder.build()); + } + + private static ImmutableMetadata exposureMetadata( + final String allocationKey, final boolean doLog, final boolean cacheHit) { + final ImmutableMetadata.ImmutableMetadataBuilder builder = + ImmutableMetadata.builder() + .addBoolean(DDEvaluator.METADATA_DO_LOG, doLog) + .addBoolean(DDEvaluator.METADATA_EXPOSURE_CACHE_HIT, cacheHit); + if (allocationKey != null) { + builder.addString("allocationKey", allocationKey); + } + return builder.build(); + } + + private FlagEvaluationDetails evaluate(final String targetingKey) { + return client.getIntegerDetails("flag", 0, new MutableContext(targetingKey)); + } + + private List shouldSend() { + final List result = new ArrayList<>(); + for (final ExposureHook.Evaluation evaluation : evaluations) { + result.add(evaluation.shouldSend()); + } + return result; + } + + private static ServerConfiguration configuration( + final Boolean doLog, final List features) { + final Map variations = new HashMap<>(); + variations.put("on", new Variant("on", 1)); + final Split split = new Split(emptyList(), "on", emptyMap(), 7, features); + final Allocation allocation = + new Allocation("allocation", null, null, null, singletonList(split), doLog); + final Map flags = new HashMap<>(); + flags.put( + "flag", new Flag("flag", true, ValueType.INTEGER, variations, singletonList(allocation))); + return new ServerConfiguration("", "", false, null, flags); + } + + private static FlagEvaluationWriter capturingWriter(final AtomicReference ref) { + return new FlagEvaluationWriter() { + @Override + public void enqueue(final FlagEvalEvent event) { + ref.set(event); + } + + @Override + public boolean hasCapacityForEnqueue() { + return true; + } + + @Override + public void countPreQueueOverflow() {} + + @Override + public void countContextTruncated(final String reason) {} + + @Override + public void start() {} + + @Override + public void close() {} + }; + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookWithoutAgentTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookWithoutAgentTest.java new file mode 100644 index 00000000000..efd42915b6d --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookWithoutAgentTest.java @@ -0,0 +1,103 @@ +package datadog.trace.api.openfeature; + +import static java.util.Collections.singletonMap; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.ImmutableMetadata; +import dev.openfeature.sdk.MutableContext; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Constructor; +import org.junit.jupiter.api.Test; + +/** + * Loads ExposureHook in a class loader that hides DDEvaluator and the Datadog Java agent classes, + * as in an application without the agent. The normal test classpath always has them, so only a + * loader like this sees ExposureHook reach for them. + */ +class ExposureHookWithoutAgentTest { + + @Test + void exposureHookWorksWithoutTheAgentClasses() throws Exception { + final ClassLoader loader = new AgentlessClassLoader(getClass().getClassLoader()); + Class.forName("datadog.trace.api.openfeature.ExposureHook", true, loader); + final Class evaluationClass = + Class.forName("datadog.trace.api.openfeature.ExposureHook$Evaluation", true, loader); + final Constructor constructor = + evaluationClass.getDeclaredConstructor( + EvaluationContext.class, FlagEvaluationDetails.class); + constructor.setAccessible(true); + final FlagEvaluationDetails details = + FlagEvaluationDetails.builder() + .flagKey("flag") + .value(1) + .variant("on") + .flagMetadata( + ImmutableMetadata.builder() + .addString("allocationKey", "allocation") + .addBoolean("__dd_do_log", true) + .addBoolean("__dd_exposure_cache_hit", false) + .addString("__dd_feature.holdout.key", "q4-global") + .build()) + .build(); + + final Object evaluation = constructor.newInstance(new MutableContext("user-1"), details); + + assertTrue((Boolean) evaluationClass.getMethod("isExposure").invoke(evaluation)); + assertTrue((Boolean) evaluationClass.getMethod("shouldSend").invoke(evaluation)); + assertEquals( + singletonMap("holdout.key", "q4-global"), + evaluationClass.getMethod("getFeatures").invoke(evaluation)); + } + + private static final class AgentlessClassLoader extends ClassLoader { + AgentlessClassLoader(final ClassLoader parent) { + super(parent); + } + + @Override + protected Class loadClass(final String name, final boolean resolve) + throws ClassNotFoundException { + synchronized (getClassLoadingLock(name)) { + if (name.startsWith("datadog.trace.api.featureflag.") + || name.startsWith("datadog.trace.api.openfeature.DDEvaluator")) { + throw new ClassNotFoundException(name); + } + if (name.startsWith("datadog.trace.api.openfeature.ExposureHook") + || name.equals("datadog.trace.api.openfeature.SplitFeatureMetadata")) { + Class loaded = findLoadedClass(name); + if (loaded == null) { + final byte[] bytes = classBytes(name); + loaded = defineClass(name, bytes, 0, bytes.length); + } + if (resolve) { + resolveClass(loaded); + } + return loaded; + } + return super.loadClass(name, resolve); + } + } + + private byte[] classBytes(final String name) throws ClassNotFoundException { + try (InputStream in = getParent().getResourceAsStream(name.replace('.', '/') + ".class")) { + if (in == null) { + throw new ClassNotFoundException(name); + } + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + final byte[] buffer = new byte[8192]; + int read; + while ((read = in.read(buffer)) != -1) { + out.write(buffer, 0, read); + } + return out.toByteArray(); + } catch (final IOException e) { + throw new ClassNotFoundException(name, e); + } + } + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureLoggingHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureLoggingHookTest.java index 6d3e4171af3..95e9ebc7e5d 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureLoggingHookTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureLoggingHookTest.java @@ -155,7 +155,7 @@ void sendsNothingForAnAllocationThatDoesNotLogExposures() { @Test void sendsNothingWithoutTheDetailsAnExposureNeeds() { final ExposureDeduplicationCache cache = new ExposureDeduplicationCache(10); - final ExposureLoggingHook hook = new ExposureLoggingHook<>(cache); + final ExposureLoggingHook hook = new ExposureLoggingHook<>(cache, true); final HookContext withContext = hookContext(new MutableContext("user-1")); hook.finallyAfter(withContext, null, emptyMap()); diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/FlagEvalLoggingHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/FlagEvalLoggingHookTest.java index 3b7910d4712..9fd6c79e350 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/FlagEvalLoggingHookTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/FlagEvalLoggingHookTest.java @@ -144,6 +144,62 @@ void finallyAfterEnqueuesEventWithAllBasicFields() { assertEquals("alloc-1", e.allocationKey); } + @Test + void finallyAfterAddsEvaluationFeaturesWhateverTheConsent() { + final AtomicReference captured = new AtomicReference<>(); + final FlagEvalLoggingHook hook = hookWithWriter(capturingWriter(captured)); + + hook.finallyAfter( + null, + details( + "my-flag", + "on-value", + "on", + Reason.TARGETING_MATCH.name(), + ImmutableMetadata.builder() + .addString("allocationKey", "alloc-1") + .addBoolean(DDEvaluator.METADATA_OBSERVE_FULL_EVALUATION_DATA, false) + .addString( + DDEvaluator.METADATA_EVALUATION_FEATURE_PREFIX + "bandit.policy_id", "p2") + .addBoolean(DDEvaluator.METADATA_HAS_EVALUATION_FEATURES, true) + .addString(DDEvaluator.METADATA_FEATURE_PREFIX + "holdout.key", "q4-global") + .addString(DDEvaluator.METADATA_EXPOSURE_FEATURE_PREFIX + "bandit.arm", "a") + .build()), + Collections.emptyMap()); + + assertEquals( + Collections.singletonMap("bandit.policy_id", "p2"), + captured.get().features, + "only EVALUATION features reach the event, also without consent"); + } + + @Test + void finallyAfterSendsNoFeaturesWhenTheAgentLacksThem() { + final AtomicReference captured = new AtomicReference<>(); + final FlagEvalLoggingHook hook = hookWithWriter(capturingWriter(captured)); + FlagEvalLoggingHook.FEATURES_SUPPORTED.set(false); + try { + hook.finallyAfter( + null, + details( + "my-flag", + "on-value", + "on", + Reason.TARGETING_MATCH.name(), + ImmutableMetadata.builder() + .addString("allocationKey", "alloc-1") + .addString( + DDEvaluator.METADATA_EVALUATION_FEATURE_PREFIX + "bandit.policy_id", "p2") + .addBoolean(DDEvaluator.METADATA_HAS_EVALUATION_FEATURES, true) + .build()), + Collections.emptyMap()); + } finally { + FlagEvalLoggingHook.FEATURES_SUPPORTED.set(true); + } + + assertTrue(captured.get().features.isEmpty()); + } + // ---- variant comes from details.getVariant(), NOT details.getValue() ---- @Test diff --git a/products/feature-flagging/feature-flagging-bootstrap/build.gradle.kts b/products/feature-flagging/feature-flagging-bootstrap/build.gradle.kts index 848b16a2841..94f08ae55fe 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/build.gradle.kts +++ b/products/feature-flagging/feature-flagging-bootstrap/build.gradle.kts @@ -18,6 +18,7 @@ extra["excludedClassesCoverage"] = listOf( "datadog.trace.api.featureflag.ufc.v1.ConditionConfiguration", "datadog.trace.api.featureflag.ufc.v1.ConditionOperator", "datadog.trace.api.featureflag.ufc.v1.Environment", + "datadog.trace.api.featureflag.ufc.v1.Feature", "datadog.trace.api.featureflag.ufc.v1.Flag", "datadog.trace.api.featureflag.ufc.v1.Rule", "datadog.trace.api.featureflag.ufc.v1.ServerConfiguration", diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/exposure/ExposureEvent.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/exposure/ExposureEvent.java index 4101b767760..a2cc090adf2 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/exposure/ExposureEvent.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/exposure/ExposureEvent.java @@ -1,5 +1,7 @@ package datadog.trace.api.featureflag.exposure; +import java.util.Map; + public class ExposureEvent { // milliseconds since epoch as given by System.currentTimeMillis() public final long timestamp; @@ -10,6 +12,10 @@ public class ExposureEvent { public final Integer serial_id; + // Split features routed to the EXPOSURE destination. Null when there are none, so the field is + // omitted from the exposure payload. + public final Map features; + public ExposureEvent( final long timestamp, final Allocation allocation, @@ -26,11 +32,23 @@ public ExposureEvent( final Variant variant, final Subject subject, final Integer serialId) { + this(timestamp, allocation, flag, variant, subject, serialId, null); + } + + public ExposureEvent( + final long timestamp, + final Allocation allocation, + final Flag flag, + final Variant variant, + final Subject subject, + final Integer serialId, + final Map features) { this.timestamp = timestamp; this.allocation = allocation; this.flag = flag; this.variant = variant; this.subject = subject; this.serial_id = serialId; + this.features = features; } } diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEvent.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEvent.java index f5f44b94d7b..d73285a9aa9 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEvent.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEvent.java @@ -54,6 +54,12 @@ public final class FlagEvalEvent { */ public final boolean observeFullEvaluationData; + /** + * Split features routed to the EVALUATION destination. They come from the flag configuration, not + * the evaluation context, so they are kept whatever the consent. May be empty but never null. + */ + public final Map features; + /** Convenience constructor; consent defaults to the privacy-preserving false. */ public FlagEvalEvent( final String flagKey, @@ -86,6 +92,28 @@ public FlagEvalEvent( final long evalTimeMs, final boolean observeFullEvaluationData, final Map attrs) { + this( + flagKey, + variant, + allocationKey, + targetingKey, + errorMessage, + evalTimeMs, + observeFullEvaluationData, + attrs, + null); + } + + public FlagEvalEvent( + final String flagKey, + final String variant, + final String allocationKey, + final String targetingKey, + final String errorMessage, + final long evalTimeMs, + final boolean observeFullEvaluationData, + final Map attrs, + final Map features) { this.flagKey = flagKey; this.variant = variant; this.allocationKey = allocationKey; @@ -94,5 +122,6 @@ public FlagEvalEvent( this.evalTimeMs = evalTimeMs; this.observeFullEvaluationData = observeFullEvaluationData; this.attrs = attrs != null ? attrs : Collections.emptyMap(); + this.features = features != null ? features : Collections.emptyMap(); } } diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java new file mode 100644 index 00000000000..495b547462f --- /dev/null +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Feature.java @@ -0,0 +1,19 @@ +package datadog.trace.api.featureflag.ufc.v1; + +import java.util.List; + +/** Key-value data that the UFC attaches to a split, with where SDKs deliver it. */ +public class Feature { + public final String key; + // A String, Double or Boolean; the parser drops features with any other value. + public final Object value; + // Open strings such as HOOK, EXPOSURE or EVALUATION. Never empty: the parser drops a feature + // without destinations. + public final List destinations; + + public Feature(final String key, final Object value, final List destinations) { + this.key = key; + this.value = value; + this.destinations = destinations; + } +} diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Split.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Split.java index 37f6909eeea..5e98f90dabc 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Split.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/Split.java @@ -11,15 +11,28 @@ public class Split { // Moshi reflective deserialization from the UFC "serialId" JSON field. Surfaced as // __dd_split_serial_id in eval metadata for APM span enrichment. public final Integer serialId; + // Null when the UFC split has no features. A provider must check that this field exists before + // reading it, because an older agent ships a Split without it. + public final List features; public Split( final List shards, final String variationKey, final Map extraLogging, final Integer serialId) { + this(shards, variationKey, extraLogging, serialId, null); + } + + public Split( + final List shards, + final String variationKey, + final Map extraLogging, + final Integer serialId, + final List features) { this.shards = shards; this.variationKey = variationKey; this.extraLogging = extraLogging; this.serialId = serialId; + this.features = features; } } diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEventTest.java b/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEventTest.java index cc614150178..6470364c660 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEventTest.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/flagevaluation/FlagEvalEventTest.java @@ -51,4 +51,19 @@ void storesExplicitObserveFullEvaluationData() { assertTrue( new FlagEvalEvent("f", "on", "a", "t", null, 1L, true, attrs).observeFullEvaluationData); } + + @Test + void storesFeaturesAndDefaultsThemToEmpty() { + final Map features = Collections.singletonMap("bandit.policy_id", "p2"); + + assertSame( + features, + new FlagEvalEvent("f", "on", "a", "t", null, 1L, false, Collections.emptyMap(), features) + .features); + assertTrue( + new FlagEvalEvent("f", "on", "a", "t", null, 1L, false, Collections.emptyMap(), null) + .features.isEmpty()); + assertTrue( + new FlagEvalEvent("f", "on", "a", "t", 1L, Collections.emptyMap()).features.isEmpty()); + } } diff --git a/products/feature-flagging/feature-flagging-config/src/main/java/datadog/trace/api/featureflag/config/FeatureFlaggingConfig.java b/products/feature-flagging/feature-flagging-config/src/main/java/datadog/trace/api/featureflag/config/FeatureFlaggingConfig.java index 1edfd5bcb26..ac6210c1175 100644 --- a/products/feature-flagging/feature-flagging-config/src/main/java/datadog/trace/api/featureflag/config/FeatureFlaggingConfig.java +++ b/products/feature-flagging/feature-flagging-config/src/main/java/datadog/trace/api/featureflag/config/FeatureFlaggingConfig.java @@ -31,6 +31,13 @@ public class FeatureFlaggingConfig { public static final String FLAGGING_EVALUATION_COUNTS_ENABLED = "flagging.evaluation.counts.enabled"; + /** + * Turns off sending exposures to Datadog while customer exposure hooks keep receiving them. + * Default: enabled. Maps to {@code DD_FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED}. + */ + public static final String FEATURE_FLAGS_EXPOSURES_DATADOG_LOGGING_ENABLED = + "feature.flags.exposures.datadog.logging.enabled"; + public static final String FEATURE_FLAGS_CONFIGURATION_SOURCE = "feature.flags.configuration.source"; public static final String FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL = diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationAggregator.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationAggregator.java index 5d0eeaa0e82..531dbe82a2f 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationAggregator.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationAggregator.java @@ -4,6 +4,7 @@ import static datadog.trace.util.HashingUtils.hash; import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Objects; @@ -46,6 +47,9 @@ final class FlagEvaluationAggregator { final Map perFlagCount = new HashMap<>(); final AtomicLong droppedDegradedOverflow = new AtomicLong(0); final AtomicInteger globalFullCount = new AtomicInteger(0); + // One shared key and map for each distinct feature set in the flush window, so buckets of many + // subjects do not each keep their own copy until the flush. + final Map featureSets = new HashMap<>(); void aggregate(final FlagEvalEvent event) { final boolean isDefault = event.variant == null; @@ -55,7 +59,11 @@ void aggregate(final FlagEvalEvent event) { // and spills every subsequent evaluation into the degraded tier. final Map prunedAttrs = observeFullEvaluationData ? event.attrs : null; final String ctxKey = observeFullEvaluationData ? canonicalContextKey(prunedAttrs) : ""; - final FullKey fullKey = buildFullKey(event, ctxKey); + // Features come from the flag configuration, not the evaluation context, so they split buckets + // in both tiers and whatever the consent: rows with different features must not merge. + final FeatureSet featureSet = featureSet(event.features); + final String featuresKey = featureSet.key; + final FullKey fullKey = buildFullKey(event, ctxKey, featuresKey); EvalBucket bucket = fullTier.get(fullKey); if (bucket != null) { @@ -77,13 +85,14 @@ void aggregate(final FlagEvalEvent event) { event.evalTimeMs, isDefault, prunedAttrs, - observeFullEvaluationData)); + observeFullEvaluationData, + featureSet.features)); globalFullCount.incrementAndGet(); perFlagCount.put(event.flagKey, flagCount + 1); return; } - final DegradedKey degradedKey = buildDegradedKey(event); + final DegradedKey degradedKey = buildDegradedKey(event, featuresKey); bucket = degradedTier.get(degradedKey); if (bucket != null) { bucket.merge(event.evalTimeMs, isDefault); @@ -103,7 +112,8 @@ void aggregate(final FlagEvalEvent event) { event.evalTimeMs, isDefault, null, - observeFullEvaluationData)); + observeFullEvaluationData, + featureSet.features)); return; } @@ -141,10 +151,36 @@ Iterable degradedBuckets() { void clear() { fullTier.clear(); degradedTier.clear(); + featureSets.clear(); perFlagCount.clear(); globalFullCount.set(0); } + private FeatureSet featureSet(final Map features) { + if (features == null || features.isEmpty()) { + return FeatureSet.NONE; + } + final String key = canonicalContextKey(features); + FeatureSet featureSet = featureSets.get(key); + if (featureSet == null) { + featureSet = new FeatureSet(key, features); + featureSets.put(key, featureSet); + } + return featureSet; + } + + static final class FeatureSet { + static final FeatureSet NONE = new FeatureSet("", Collections.emptyMap()); + + final String key; + final Map features; + + FeatureSet(final String key, final Map features) { + this.key = key; + this.features = features; + } + } + AggregatedState snapshot() { return new AggregatedState( new HashMap<>(fullTier), new HashMap<>(degradedTier), droppedDegradedOverflow.get()); @@ -154,8 +190,8 @@ void simulateFullTierAtCap() { for (int i = globalFullCount.get(); i < GLOBAL_CAP; i++) { final String key = "synthetic-full-" + i; fullTier.put( - new FullKey(key, "on", "alloc", false, null, null, "", false), - new EvalBucket(key, "on", "alloc", null, null, 1L, false, null, false)); + new FullKey(key, "on", "alloc", false, null, null, "", false, ""), + new EvalBucket(key, "on", "alloc", null, null, 1L, false, null, false, null)); globalFullCount.incrementAndGet(); perFlagCount.merge(key, 1, Integer::sum); } @@ -165,8 +201,8 @@ void simulateDegradedTierAtCap() { for (int i = degradedTier.size(); i < DEGRADED_CAP; i++) { final String key = "synthetic-dg-" + i; degradedTier.put( - new DegradedKey(key, "on", "alloc", false, null), - new EvalBucket(key, "on", "alloc", null, null, 1L, false, null, false)); + new DegradedKey(key, "on", "alloc", false, null, ""), + new EvalBucket(key, "on", "alloc", null, null, 1L, false, null, false, null)); } } @@ -177,7 +213,7 @@ void addDegradedBucketForTest( final String errorMessage, final long evalTimeMs) { degradedTier.put( - new DegradedKey(flagKey, variant, allocationKey, variant == null, errorMessage), + new DegradedKey(flagKey, variant, allocationKey, variant == null, errorMessage, ""), new EvalBucket( flagKey, variant, @@ -187,10 +223,12 @@ void addDegradedBucketForTest( evalTimeMs, variant == null, null, - false)); + false, + null)); } - private static FullKey buildFullKey(final FlagEvalEvent event, final String ctxKey) { + private static FullKey buildFullKey( + final FlagEvalEvent event, final String ctxKey, final String featuresKey) { return new FullKey( event.flagKey, event.variant, @@ -199,16 +237,18 @@ private static FullKey buildFullKey(final FlagEvalEvent event, final String ctxK event.errorMessage, event.targetingKey, ctxKey, - event.observeFullEvaluationData); + event.observeFullEvaluationData, + featuresKey); } - private static DegradedKey buildDegradedKey(final FlagEvalEvent event) { + private static DegradedKey buildDegradedKey(final FlagEvalEvent event, final String featuresKey) { return new DegradedKey( event.flagKey, event.variant, event.allocationKey, event.variant == null, - event.errorMessage); + event.errorMessage, + featuresKey); } static String canonicalContextKey(final Map prunedAttrs) { @@ -271,6 +311,8 @@ static class EvalBucket { String targetingKey; String errorMessage; Map prunedAttrs; + // Identical for every evaluation in the bucket, because the features key is a bucket dimension. + Map features; // Consent to emit raw PII. For full-tier buckets this is uniform (consent is a FullKey // dimension) and the AND-fold on merge is defensive. For degraded-tier buckets consent is NOT // a key dimension — mixed-consent events merge here — so the AND-fold produces false whenever @@ -288,7 +330,8 @@ static class EvalBucket { final long evalTimeMs, final boolean runtimeDefaultUsed, final Map prunedAttrs, - final boolean observeFullEvaluationData) { + final boolean observeFullEvaluationData, + final Map features) { this.flagKey = flagKey; this.variant = variant; this.allocationKey = allocationKey; @@ -300,6 +343,7 @@ static class EvalBucket { this.runtimeDefaultUsed = runtimeDefaultUsed; this.prunedAttrs = prunedAttrs; this.observeFullEvaluationData = observeFullEvaluationData; + this.features = features; } int prunedContextFieldCount() { @@ -332,6 +376,7 @@ static final class FullKey { // serializer branches on this to hash the targeting key and drop the context, so events with // different consent produce different wire rows and belong in different buckets. private final boolean observeFullEvaluationData; + private final String featuresKey; FullKey( final String flagKey, @@ -341,7 +386,8 @@ static final class FullKey { final String errorMessage, final String targetingKey, final String contextKey, - final boolean observeFullEvaluationData) { + final boolean observeFullEvaluationData, + final String featuresKey) { this.flagKey = flagKey; this.variant = variant; this.allocationKey = allocationKey; @@ -350,6 +396,7 @@ static final class FullKey { this.targetingKey = targetingKey; this.contextKey = contextKey; this.observeFullEvaluationData = observeFullEvaluationData; + this.featuresKey = featuresKey; } @Override @@ -368,7 +415,8 @@ public boolean equals(final Object o) { && Objects.equals(allocationKey, fullKey.allocationKey) && Objects.equals(errorMessage, fullKey.errorMessage) && Objects.equals(targetingKey, fullKey.targetingKey) - && Objects.equals(contextKey, fullKey.contextKey); + && Objects.equals(contextKey, fullKey.contextKey) + && Objects.equals(featuresKey, fullKey.featuresKey); } @Override @@ -380,7 +428,8 @@ public int hashCode() { result = addToHash(result, errorMessage); result = addToHash(result, targetingKey); result = addToHash(result, contextKey); - return addToHash(result, observeFullEvaluationData); + result = addToHash(result, observeFullEvaluationData); + return addToHash(result, featuresKey); } } @@ -397,18 +446,21 @@ static final class DegradedKey { private final String allocationKey; private final boolean runtimeDefaultUsed; private final String errorMessage; + private final String featuresKey; DegradedKey( final String flagKey, final String variant, final String allocationKey, final boolean runtimeDefaultUsed, - final String errorMessage) { + final String errorMessage, + final String featuresKey) { this.flagKey = flagKey; this.variant = variant; this.allocationKey = allocationKey; this.runtimeDefaultUsed = runtimeDefaultUsed; this.errorMessage = errorMessage; + this.featuresKey = featuresKey; } @Override @@ -424,7 +476,8 @@ public boolean equals(final Object o) { && Objects.equals(flagKey, that.flagKey) && Objects.equals(variant, that.variant) && Objects.equals(allocationKey, that.allocationKey) - && Objects.equals(errorMessage, that.errorMessage); + && Objects.equals(errorMessage, that.errorMessage) + && Objects.equals(featuresKey, that.featuresKey); } @Override @@ -433,7 +486,8 @@ public int hashCode() { // on this hot bucket-lookup path. int result = hash(flagKey, variant, allocationKey); result = addToHash(result, runtimeDefaultUsed); - return addToHash(result, errorMessage); + result = addToHash(result, errorMessage); + return addToHash(result, featuresKey); } } diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationPayloads.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationPayloads.java index 3a8734fd516..e7e027cc2d2 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationPayloads.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FlagEvaluationPayloads.java @@ -169,6 +169,7 @@ static class FlagEvaluationEvent { public final Boolean runtime_default_used; public final EventContext context; public final ErrorObject error; + public final Map features; FlagEvaluationEvent( final long timestamp, @@ -181,7 +182,8 @@ static class FlagEvaluationEvent { final String targetingKey, final boolean runtimeDefaultUsed, final String errorMessage, - final Map evaluationAttrs) { + final Map evaluationAttrs, + final Map features) { this.timestamp = timestamp; this.flag = new FlagKeyObject(flagKey); this.first_evaluation = firstEvalMs; @@ -198,6 +200,7 @@ static class FlagEvaluationEvent { : null; this.error = (errorMessage != null && !errorMessage.isEmpty()) ? new ErrorObject(errorMessage) : null; + this.features = (features != null && !features.isEmpty()) ? features : null; } static FlagEvaluationEvent fromBucket( @@ -217,7 +220,10 @@ static FlagEvaluationEvent fromBucket( resolveTargetingKey(bucket.targetingKey, isFullTier, observeFullEvaluationData), bucket.runtimeDefaultUsed, bucket.errorMessage, - includeRawContext ? bucket.prunedAttrs : null); + includeRawContext ? bucket.prunedAttrs : null, + // Features come from the flag configuration, so both tiers keep them whatever the + // consent. + bucket.features); } private static String resolveTargetingKey( @@ -248,7 +254,8 @@ FlagEvaluationEvent withoutTargetingKeyAndContext() { null, Boolean.TRUE.equals(runtime_default_used), messageOf(error), - null); + null, + features); } } diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java index 9778d980179..27fd3f70c70 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java @@ -9,6 +9,7 @@ import datadog.remoteconfig.ConfigurationDeserializer; import datadog.trace.api.featureflag.ufc.v1.Allocation; import datadog.trace.api.featureflag.ufc.v1.ConditionConfiguration; +import datadog.trace.api.featureflag.ufc.v1.Feature; import datadog.trace.api.featureflag.ufc.v1.Flag; import datadog.trace.api.featureflag.ufc.v1.ParsedSemver; import datadog.trace.api.featureflag.ufc.v1.Rule; @@ -23,6 +24,7 @@ import java.time.Instant; import java.time.format.DateTimeFormatter; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -59,6 +61,7 @@ final class UniversalFlagConfigParser implements ConfigurationDeserializer V1_ADAPTER = MOSHI.adapter(ServerConfiguration.class); @@ -438,6 +441,121 @@ public void toJson(@Nonnull final JsonWriter writer, @Nullable final Boolean val } } + /** + * Reads a split's features. Features only feed exposure hooks, so a malformed list or entry is + * dropped instead of rejecting the flag: it must never change which variation a subject gets. + */ + static final class FeatureListAdapter extends JsonAdapter> { + + private static final Type FEATURES_TYPE = Types.newParameterizedType(List.class, Feature.class); + + static final Factory FACTORY = + new Factory() { + @Nullable + @Override + public JsonAdapter create( + @Nonnull final Type type, + @Nonnull final Set annotations, + @Nonnull final Moshi moshi) { + if (!annotations.isEmpty() || !Types.equals(type, FEATURES_TYPE)) { + return null; + } + return new FeatureListAdapter(); + } + }; + + @Nullable + @Override + public List fromJson(@Nonnull final JsonReader reader) throws IOException { + if (reader.peek() != JsonReader.Token.BEGIN_ARRAY) { + reader.skipValue(); + return null; + } + final List features = new ArrayList<>(); + reader.beginArray(); + while (reader.hasNext()) { + final Feature feature = readFeature(reader); + if (feature != null) { + features.add(feature); + } + } + reader.endArray(); + return Collections.unmodifiableList(features); + } + + @Nullable + private static Feature readFeature(final JsonReader reader) throws IOException { + if (reader.peek() != JsonReader.Token.BEGIN_OBJECT) { + reader.skipValue(); + return null; + } + String key = null; + Object value = null; + List destinations = Collections.emptyList(); + reader.beginObject(); + while (reader.hasNext()) { + final String name = reader.nextName(); + if ("key".equals(name) && reader.peek() == JsonReader.Token.STRING) { + key = reader.nextString(); + } else if ("value".equals(name)) { + value = readScalar(reader); + } else if ("destinations".equals(name)) { + destinations = readDestinations(reader); + } else { + reader.skipValue(); + } + } + reader.endObject(); + if (key == null || key.isEmpty() || value == null || destinations.isEmpty()) { + return null; + } + return new Feature(key, value, destinations); + } + + /** Keeps every non-empty string, including destinations this SDK does not know yet. */ + private static List readDestinations(final JsonReader reader) throws IOException { + if (reader.peek() != JsonReader.Token.BEGIN_ARRAY) { + reader.skipValue(); + return Collections.emptyList(); + } + final List destinations = new ArrayList<>(); + reader.beginArray(); + while (reader.hasNext()) { + if (reader.peek() == JsonReader.Token.STRING) { + final String destination = reader.nextString(); + if (!destination.isEmpty()) { + destinations.add(destination); + } + } else { + reader.skipValue(); + } + } + reader.endArray(); + return Collections.unmodifiableList(destinations); + } + + @Nullable + private static Object readScalar(final JsonReader reader) throws IOException { + switch (reader.peek()) { + case STRING: + return reader.nextString(); + case NUMBER: + return reader.nextDouble(); + case BOOLEAN: + return reader.nextBoolean(); + default: + reader.skipValue(); + return null; + } + } + + @Override + public void toJson(@Nonnull final JsonWriter writer, @Nullable final List value) + throws IOException { + throw new UnsupportedOperationException("Reading only adapter"); + } + } + static final class InstantAdapter extends JsonAdapter { @Nullable diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagEvpPublisherTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagEvpPublisherTest.java index 711e1b90e29..6f034119cdf 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagEvpPublisherTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagEvpPublisherTest.java @@ -125,4 +125,24 @@ static class TestRequest { this.value = value; } } + + @Test + void serializesExposureFeatures() { + final ExposureEvent event = + new ExposureEvent( + 1234L, + new Allocation("allocation"), + new Flag("flag"), + new Variant("variant"), + new Subject("subject", emptyMap()), + 7, + java.util.Collections.singletonMap("bandit.policy_id", "p2")); + + assertTrue(exposureJsonOf(event).contains("\"features\":{\"bandit.policy_id\":\"p2\"}")); + } + + @Test + void omitsFeaturesKeyWhenAbsent() { + assertFalse(exposureJson(7).contains("features")); + } } diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationAggregatorTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationAggregatorTest.java index 06faeadefea..6fb7692f8bb 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationAggregatorTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationAggregatorTest.java @@ -1,14 +1,18 @@ package com.datadog.featureflag; import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import datadog.trace.api.featureflag.flagevaluation.FlagEvalEvent; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.junit.jupiter.api.Test; @@ -218,7 +222,7 @@ void flagEvalEventDoesNotCarryReason() { void evalBucketTracksBoundsDefaultStateAndNullContextFieldCount() { final FlagEvaluationAggregator.EvalBucket bucket = new FlagEvaluationAggregator.EvalBucket( - "bucket-flag", "on", "alloc1", "user-1", null, 1000L, false, null, false); + "bucket-flag", "on", "alloc1", "user-1", null, 1000L, false, null, false, null); assertEquals(0, bucket.prunedContextFieldCount()); @@ -410,7 +414,8 @@ private static FlagEvaluationAggregator.FullKey fullKey( errorMessage, targetingKey, contextKey, - false); + false, + ""); } private static FlagEvaluationAggregator.DegradedKey degradedKey( @@ -420,6 +425,85 @@ private static FlagEvaluationAggregator.DegradedKey degradedKey( final boolean runtimeDefaultUsed, final String errorMessage) { return new FlagEvaluationAggregator.DegradedKey( - flagKey, variant, allocationKey, runtimeDefaultUsed, errorMessage); + flagKey, variant, allocationKey, runtimeDefaultUsed, errorMessage, ""); + } + + @Test + void featuresSplitBucketsInBothTiersWhateverTheConsent() { + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + final Map p1 = singletonMap("bandit.policy_id", "p1"); + final Map p2 = singletonMap("bandit.policy_id", "p2"); + + aggregator.aggregate(featureEvent("user-1", 1000L, false, p1)); + aggregator.aggregate(featureEvent("user-1", 2000L, false, p2)); + aggregator.aggregate(featureEvent("user-1", 3000L, false, p1)); + + final FlagEvaluationAggregator.AggregatedState full = aggregator.snapshot(); + assertEquals(2, full.fullTier.size()); + for (final FlagEvaluationAggregator.EvalBucket bucket : full.fullTier.values()) { + assertEquals(p1.equals(bucket.features) ? 2 : 1, bucket.count); + } + + aggregator.simulateFullTierAtCap(); + aggregator.aggregate(featureEvent("user-2", 4000L, false, p1)); + aggregator.aggregate(featureEvent("user-3", 5000L, false, p2)); + aggregator.aggregate(featureEvent("user-4", 6000L, false, p1)); + + final FlagEvaluationAggregator.AggregatedState degraded = aggregator.snapshot(); + assertEquals(2, degraded.degradedTier.size()); + for (final FlagEvaluationAggregator.EvalBucket bucket : degraded.degradedTier.values()) { + assertEquals(p1.equals(bucket.features) ? 2 : 1, bucket.count); + } + } + + @Test + void bucketsShareOneCopyOfEachFeatureSetUntilTheWindowIsCleared() { + final FlagEvaluationAggregator aggregator = new FlagEvaluationAggregator(); + + aggregator.aggregate(featureEvent("user-1", 1000L, false, policy("p1"))); + aggregator.aggregate(featureEvent("user-2", 2000L, false, policy("p1"))); + aggregator.aggregate(featureEvent("user-3", 3000L, false, policy("p2"))); + + final List p1Buckets = new ArrayList<>(); + for (final FlagEvaluationAggregator.EvalBucket bucket : aggregator.fullTier.values()) { + if (policy("p1").equals(bucket.features)) { + p1Buckets.add(bucket); + } + } + assertEquals(2, p1Buckets.size()); + assertSame(p1Buckets.get(0).features, p1Buckets.get(1).features); + assertEquals(2, aggregator.featureSets.size()); + + aggregator.simulateFullTierAtCap(); + aggregator.aggregate(featureEvent("user-4", 4000L, false, policy("p1"))); + for (final FlagEvaluationAggregator.EvalBucket bucket : aggregator.degradedTier.values()) { + assertSame(p1Buckets.get(0).features, bucket.features); + } + + aggregator.clear(); + assertTrue(aggregator.featureSets.isEmpty()); + } + + private static Map policy(final String id) { + final Map features = new HashMap<>(); + features.put("bandit.policy_id", id); + return features; + } + + private static FlagEvalEvent featureEvent( + final String targetingKey, + final long evalTimeMs, + final boolean observeFullEvaluationData, + final Map features) { + return new FlagEvalEvent( + "feature-flag", + "on", + "alloc1", + targetingKey, + null, + evalTimeMs, + observeFullEvaluationData, + emptyMap(), + features); } } diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationPayloadsTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationPayloadsTest.java index d4ca517fffd..75df3ac5269 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationPayloadsTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FlagEvaluationPayloadsTest.java @@ -58,7 +58,7 @@ void fullTierPayloadUsesWorkerWireShape() throws Exception { void eventFromFullBucketUsesFlushTimeAndEvaluationBounds() throws Exception { final FlagEvaluationAggregator.EvalBucket bucket = new FlagEvaluationAggregator.EvalBucket( - "ts-flag", "on", "alloc1", "user-1", null, EVAL_MS, false, emptyMap(), true); + "ts-flag", "on", "alloc1", "user-1", null, EVAL_MS, false, emptyMap(), true, null); bucket.merge(EVAL_MS + 10, false); final long flushTimeMs = EVAL_MS + 5_000; @@ -82,7 +82,7 @@ void eventFromFullBucketUsesFlushTimeAndEvaluationBounds() throws Exception { void degradedTierEventOmitsTargetingKeyAndContext() throws Exception { final FlagEvaluationAggregator.EvalBucket bucket = new FlagEvaluationAggregator.EvalBucket( - "dg-flag", "on", "alloc1", null, null, EVAL_MS, false, null, false); + "dg-flag", "on", "alloc1", null, null, EVAL_MS, false, null, false, null); final Map json = firstPayload( @@ -112,7 +112,8 @@ void fullTierWithObserveFullEvaluationDataTrueEmitsRawTargetingKeyAndContext() t EVAL_MS, false, attrs, - true); + true, + null); final Map json = firstPayload( @@ -147,7 +148,8 @@ void fullTierWithObserveFullEvaluationDataFalseHashesTargetingKeyAndOmitsContext EVAL_MS, false, attrs, - false); + false, + null); final FlagEvaluationPayloads.EncodedPayloads payloads = FlagEvaluationPayloads.buildPayloads( @@ -308,6 +310,7 @@ void errorPayloadSerializesErrorObject() throws Exception { null, true, "type mismatch", + null, null)), CONTEXT, 1_000_000)); @@ -356,6 +359,7 @@ void degradingEventWithMissingOptionalFieldsKeepsOptionalObjectsAbsent() { "user-1", true, null, + null, null) .withoutTargetingKeyAndContext(); @@ -374,7 +378,7 @@ void emptyOptionalStringsAreTreatedAsAbsentWhenContextIsPresent() { attrs.put("tier", "gold"); final FlagEvaluationPayloads.FlagEvaluationEvent event = new FlagEvaluationPayloads.FlagEvaluationEvent( - EVAL_MS, "empty-optionals", EVAL_MS, EVAL_MS, 1, "", "", null, false, "", attrs); + EVAL_MS, "empty-optionals", EVAL_MS, EVAL_MS, 1, "", "", null, false, "", attrs, null); assertNull(event.variant); assertNull(event.allocation); @@ -406,7 +410,8 @@ private static FlagEvaluationPayloads.FlagEvaluationEvent event( targetingKey, false, null, - attrs); + attrs, + null); } private static Map firstPayload( @@ -450,4 +455,42 @@ private static Map context() { context.put("service", "test-service"); return context; } + + @Test + void rowsCarryFeaturesInEveryTierAndWithoutConsent() throws Exception { + final Map features = new HashMap<>(); + features.put("bandit.policy_id", "p2"); + features.put("bandit.weight", 0.5); + final FlagEvaluationAggregator.EvalBucket bucket = + new FlagEvaluationAggregator.EvalBucket( + "feature-flag", "on", "alloc1", "user-1", null, EVAL_MS, false, null, false, features); + + final FlagEvaluationPayloads.FlagEvaluationEvent full = + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket(bucket, true, false, EVAL_MS); + final FlagEvaluationPayloads.FlagEvaluationEvent degraded = + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket(bucket, false, false, EVAL_MS); + + assertEquals(features, firstEvent(onlyPayload(full)).get("features")); + assertEquals(features, firstEvent(onlyPayload(degraded)).get("features")); + assertEquals(features, full.withoutTargetingKeyAndContext().features); + } + + @Test + void rowsWithoutFeaturesOmitTheField() throws Exception { + final FlagEvaluationAggregator.EvalBucket bucket = + new FlagEvaluationAggregator.EvalBucket( + "plain-flag", "on", "alloc1", null, null, EVAL_MS, false, null, false, emptyMap()); + + final FlagEvaluationPayloads.FlagEvaluationEvent row = + FlagEvaluationPayloads.FlagEvaluationEvent.fromBucket(bucket, false, false, EVAL_MS); + + assertFalse(firstEvent(onlyPayload(row)).containsKey("features")); + } + + private static Map onlyPayload( + final FlagEvaluationPayloads.FlagEvaluationEvent row) throws Exception { + return firstPayload( + FlagEvaluationPayloads.buildPayloads( + java.util.Collections.singletonList(row), CONTEXT, 1_000_000)); + } } diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java index f37599feeb0..f11816a9bc0 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java @@ -1,6 +1,7 @@ package com.datadog.featureflag; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static java.util.Collections.singleton; @@ -30,6 +31,7 @@ import datadog.trace.api.Config; import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.featureflag.ufc.v1.Allocation; +import datadog.trace.api.featureflag.ufc.v1.Feature; import datadog.trace.api.featureflag.ufc.v1.Flag; import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; import java.io.IOException; @@ -37,7 +39,9 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Type; import java.time.Instant; +import java.util.Arrays; import java.util.Date; +import java.util.List; import java.util.Map; import okio.Buffer; import org.junit.jupiter.api.AfterEach; @@ -154,6 +158,110 @@ void coercesSerialIdWithoutRejectingTheFlag(final String wireValue, final int ex assertEquals(Integer.valueOf(expected), serialIdOf(config)); } + @Test + void parsesSplitFeatures() throws Exception { + final ServerConfiguration config = + deserialize( + configWithFeatures( + "[{\"key\": \"holdout.key\", \"value\": \"q4-global\", \"destinations\": [\"HOOK\"]}," + + " {\"key\": \"holdout.weight\", \"value\": 0.5," + + " \"destinations\": [\"EXPOSURE\", \"EVALUATION\"]}," + + " {\"key\": \"holdout.should_include_in_holdout_analysis\", \"value\": true," + + " \"destinations\": [\"HOOK\", \"SOME_FUTURE_DESTINATION\"]}]")); + + final List features = featuresOf(config); + assertEquals(3, features.size()); + assertFeature(features.get(0), "holdout.key", "q4-global", "HOOK"); + assertFeature(features.get(1), "holdout.weight", 0.5, "EXPOSURE", "EVALUATION"); + assertFeature( + features.get(2), + "holdout.should_include_in_holdout_analysis", + true, + "HOOK", + "SOME_FUTURE_DESTINATION"); + assertEquals(Integer.valueOf(7), serialIdOf(config)); + } + + @Test + void parsesAbsentSplitFeaturesAsNull() throws Exception { + final ServerConfiguration config = deserialize(configWithFeatures(null)); + + assertNull(featuresOf(config)); + assertEquals(Integer.valueOf(7), serialIdOf(config)); + } + + @Test + void dropsMalformedSplitFeaturesAndKeepsTheFlag() throws Exception { + final String hook = ", \"destinations\": [\"HOOK\"]"; + final ServerConfiguration config = + deserialize( + configWithFeatures( + "[{\"key\": \"object-value\", \"value\": {\"nested\": true}" + + hook + + "}," + + " {\"key\": \"array-value\", \"value\": [1]" + + hook + + "}," + + " {\"key\": \"null-value\", \"value\": null" + + hook + + "}," + + " {\"key\": \"\", \"value\": \"empty key\"" + + hook + + "}," + + " {\"key\": 3, \"value\": \"numeric key\"" + + hook + + "}," + + " {\"value\": \"missing key\"" + + hook + + "}," + + " {\"key\": \"missing value\"" + + hook + + "}," + + " {\"key\": \"no-destinations\", \"value\": \"dropped\"}," + + " {\"key\": \"empty-destinations\", \"value\": \"dropped\", \"destinations\": []}," + + " {\"key\": \"destinations-not-a-list\", \"value\": \"dropped\"," + + " \"destinations\": \"HOOK\"}," + + " {\"key\": \"only-invalid-destinations\", \"value\": \"dropped\"," + + " \"destinations\": [1, \"\", null, {\"name\": \"HOOK\"}]}," + + " \"not-an-object\"," + + " {\"key\": \"holdout.key\", \"value\": \"q4-global\"," + + " \"destinations\": [1, \"HOOK\", \"\"], \"destination\": \"HOOK\"}]")); + + assertTrue(config.flags.containsKey("valid-flag")); + final List features = featuresOf(config); + assertEquals(1, features.size()); + assertFeature(features.get(0), "holdout.key", "q4-global", "HOOK"); + } + + @Test + void ignoresSplitFeaturesThatAreNotAList() throws Exception { + final ServerConfiguration config = + deserialize(configWithFeatures("{\"holdout.key\": \"q4-global\"}")); + + assertTrue(config.flags.containsKey("valid-flag")); + assertNull(featuresOf(config)); + assertEquals(Integer.valueOf(7), serialIdOf(config)); + } + + private static void assertFeature( + final Feature feature, final String key, final Object value, final String... destinations) { + assertEquals(key, feature.key); + assertEquals(value, feature.value); + assertEquals(Arrays.asList(destinations), feature.destinations); + } + + private static List featuresOf(final ServerConfiguration config) { + return config.flags.get("valid-flag").allocations.get(0).splits.get(0).features; + } + + /** Inserts the raw features JSON; null omits the key. */ + private static String configWithFeatures(final String featuresJson) throws IOException { + final String json = resource("split-features.json"); + return featuresJson == null + ? json.replace("\"features\": \"${features}\",", "") + : json.replace("\"${features}\"", featuresJson); + } + private static Integer serialIdOf(final ServerConfiguration config) { return config.flags.get("valid-flag").allocations.get(0).splits.get(0).serialId; } @@ -261,6 +369,35 @@ void lenientBooleanAdapterIsReadOnly() { UnsupportedOperationException.class, () -> adapter.toJson(mock(JsonWriter.class), true)); } + @Test + void featureListAdapterFactoryOnlyCreatesAdapterForUnannotatedFeatureList() { + final Moshi moshi = moshi(); + final Type featuresType = Types.newParameterizedType(List.class, Feature.class); + + final JsonAdapter adapter = + UniversalFlagConfigParser.FeatureListAdapter.FACTORY.create( + featuresType, emptySet(), moshi); + + assertNotNull(adapter); + assertTrue(adapter instanceof UniversalFlagConfigParser.FeatureListAdapter); + assertNull( + UniversalFlagConfigParser.FeatureListAdapter.FACTORY.create( + Types.newParameterizedType(List.class, String.class), emptySet(), moshi)); + assertNull( + UniversalFlagConfigParser.FeatureListAdapter.FACTORY.create( + featuresType, singleton(mock(Annotation.class)), moshi)); + } + + @Test + void featureListAdapterIsReadOnly() { + final UniversalFlagConfigParser.FeatureListAdapter adapter = + new UniversalFlagConfigParser.FeatureListAdapter(); + + assertThrows( + UnsupportedOperationException.class, + () -> adapter.toJson(mock(JsonWriter.class), emptyList())); + } + @Test void allowsNullFlagMap() throws Exception { final ServerConfiguration config = deserialize(resource("null-flags.json")); diff --git a/products/feature-flagging/feature-flagging-lib/src/test/resources/remote-config/split-features.json b/products/feature-flagging/feature-flagging-lib/src/test/resources/remote-config/split-features.json new file mode 100644 index 00000000000..d8ff27d02a0 --- /dev/null +++ b/products/feature-flagging/feature-flagging-lib/src/test/resources/remote-config/split-features.json @@ -0,0 +1,35 @@ +{ + "createdAt": "2024-04-17T19:40:53.716Z", + "format": "SERVER", + "environment": { + "name": "Test" + }, + "flags": { + "valid-flag": { + "key": "valid-flag", + "enabled": true, + "variationType": "STRING", + "variations": { + "expected": { + "key": "expected", + "value": "expected" + } + }, + "allocations": [ + { + "key": "default-allocation", + "rules": [], + "splits": [ + { + "serialId": 7, + "features": "${features}", + "variationKey": "expected", + "shards": [] + } + ], + "doLog": true + } + ] + } + } +}