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 8509671ba1b..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 @@ -68,6 +68,27 @@ class DDEvaluator implements Evaluator, FeatureFlaggingGateway.ConfigListener { 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 { @@ -164,12 +185,19 @@ 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 under this prefix. + // 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"; - private static final Set KNOWN_FEATURE_DESTINATIONS = - new HashSet<>(asList(FEATURE_DESTINATION_HOOK, "EXPOSURE", "EVALUATION")); + 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; @@ -669,40 +697,62 @@ private static void addSplitFeatures( 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 || !deliversToHook(feature)) { + if (feature.key == null || feature.destinations == null) { continue; } - final String key = METADATA_FEATURE_PREFIX + feature.key; - if (feature.value instanceof String) { - metadataBuilder.addString(key, (String) feature.value); - } else if (feature.value instanceof Boolean) { - metadataBuilder.addBoolean(key, (Boolean) feature.value); - } else if (feature.value instanceof Number) { - metadataBuilder.addDouble(key, ((Number) feature.value).doubleValue()); + 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 boolean deliversToHook(final Feature feature) { - if (feature.destinations == null) { - return false; + private static String metadataPrefix(final String destination) { + if (FEATURE_DESTINATION_HOOK.equals(destination)) { + return METADATA_FEATURE_PREFIX; } - boolean hook = false; - for (final String destination : feature.destinations) { - if (FEATURE_DESTINATION_HOOK.equals(destination)) { - hook = true; - } else if (!KNOWN_FEATURE_DESTINATIONS.contains(destination) - && WARNED_FEATURE_DESTINATIONS.size() < MAX_WARNED_FEATURE_DESTINATIONS - && WARNED_FEATURE_DESTINATIONS.add(destination)) { - log.warn("Ignoring unknown feature destination {}", destination); - } + 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()); } - return hook; } /** The serial id an exposure carries: none when the agent's exposure event predates it. */ @@ -784,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 = @@ -795,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/ExposureHook.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/ExposureHook.java index 72a45cdc1bb..01f8a479c53 100644 --- 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 @@ -8,7 +8,6 @@ import dev.openfeature.sdk.HookContext; import dev.openfeature.sdk.ImmutableMetadata; import java.util.Collections; -import java.util.HashMap; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; @@ -137,24 +136,8 @@ private Map metadata() { * none. */ public Map getFeatures() { - final ImmutableMetadata metadata = details.getFlagMetadata(); - if (metadata == null) { - return Collections.emptyMap(); - } - Map features = null; - for (final Map.Entry entry : metadata.asUnmodifiableMap().entrySet()) { - if (entry.getKey().startsWith(DDEvaluator.METADATA_FEATURE_PREFIX)) { - if (features == null) { - features = new HashMap<>(); - } - features.put( - entry.getKey().substring(DDEvaluator.METADATA_FEATURE_PREFIX.length()), - entry.getValue()); - } - } - return features == null - ? Collections.emptyMap() - : Collections.unmodifiableMap(features); + 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 73c588ed5b3..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 @@ -73,7 +73,14 @@ public void finallyAfter( cache.record(flag, context.getTargetingKey(), allocationKey, variantKey, serialId); if (sendToDatadog) { FeatureFlaggingGateway.dispatch( - DDEvaluator.exposureEvent(flag, allocationKey, variantKey, context, serialId)); + 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. 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/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/ExposureHookTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/ExposureHookTest.java index c5ee0b70320..d76b8d68b8f 100644 --- 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 @@ -14,6 +14,8 @@ 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; @@ -38,6 +40,7 @@ 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; @@ -212,6 +215,135 @@ void stopsRememberingUnknownDestinationsAtTheCap() throws Exception { } } + @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( @@ -421,4 +553,30 @@ private static ServerConfiguration configuration( "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/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/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/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-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/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)); + } }