diff --git a/.gitmodules b/.gitmodules index 27fc232a993..d5cf1158fd7 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,7 @@ [submodule "dd-java-agent/agent-jmxfetch/integrations-core"] path = dd-java-agent/agent-jmxfetch/integrations-core url = https://github.com/DataDog/integrations-core.git +[submodule "products/feature-flagging/feature-flagging-api/src/test/resources/ffe-system-test-data"] + path = products/feature-flagging/feature-flagging-api/src/test/resources/ffe-system-test-data + url = git@github.com:DataDog/ffe-system-test-data.git + branch = main 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 8b04534b7d1..a0d96b2c3de 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 @@ -125,7 +125,7 @@ public ProviderEvaluation evaluate( for (final Split split : allocation.splits) { if (isEmpty(split.shards)) { return resolveVariant( - target, key, defaultValue, flag, split.variationKey, allocation, context); + target, key, defaultValue, flag, split.variationKey, allocation, split, context); } else { // To match a split, subject must match ALL underlying shards boolean allShardsMatch = true; @@ -137,7 +137,14 @@ public ProviderEvaluation evaluate( } if (allShardsMatch) { return resolveVariant( - target, key, defaultValue, flag, split.variationKey, allocation, context); + target, + key, + defaultValue, + flag, + split.variationKey, + allocation, + split, + context); } } } @@ -333,6 +340,7 @@ private static ProviderEvaluation resolveVariant( final Flag flag, final String variationKey, final Allocation allocation, + final Split split, final EvaluationContext context) { final Variant variant = flag.variations.get(variationKey); if (variant == null) { @@ -352,7 +360,10 @@ private static ProviderEvaluation resolveVariant( final ProviderEvaluation result = ProviderEvaluation.builder() .value(mapValue(target, variant.value)) - .reason(Reason.TARGETING_MATCH.name()) + .reason( + !isEmpty(allocation.rules) + ? Reason.TARGETING_MATCH.name() + : !isEmpty(split.shards) ? Reason.SPLIT.name() : Reason.STATIC.name()) .variant(variant.key) .flagMetadata(metadataBuilder.build()) .build(); diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorFixtureTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorFixtureTest.java new file mode 100644 index 00000000000..e2e79f41169 --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorFixtureTest.java @@ -0,0 +1,275 @@ +package datadog.trace.api.openfeature; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import com.squareup.moshi.JsonAdapter; +import com.squareup.moshi.JsonReader; +import com.squareup.moshi.JsonWriter; +import com.squareup.moshi.Moshi; +import com.squareup.moshi.Types; +import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; +import dev.openfeature.sdk.MutableContext; +import dev.openfeature.sdk.ProviderEvaluation; +import dev.openfeature.sdk.Value; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Type; +import java.time.OffsetDateTime; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Date; +import java.util.List; +import java.util.Map; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import okio.BufferedSource; +import okio.Okio; +import org.junit.jupiter.api.DynamicContainer; +import org.junit.jupiter.api.DynamicNode; +import org.junit.jupiter.api.DynamicTest; +import org.junit.jupiter.api.TestFactory; + +class DDEvaluatorFixtureTest { + + private static final Moshi MOSHI = new Moshi.Builder().add(Date.class, new DateAdapter()).build(); + + @TestFactory + Collection fixtureTests() throws Exception { + ServerConfiguration config = loadServerConfig(); + assertNotNull(config, "Failed to parse ufc-config.json"); + + DDEvaluator evaluator = new DDEvaluator(() -> {}); + evaluator.accept(config); + + File[] caseFiles = getCaseFiles(); + assertNotNull(caseFiles, "No evaluation-case files found"); + + List containers = new ArrayList<>(); + for (File caseFile : caseFiles) { + containers.add(buildContainerForFile(caseFile, evaluator)); + } + return containers; + } + + private static ServerConfiguration loadServerConfig() throws IOException { + JsonAdapter adapter = MOSHI.adapter(ServerConfiguration.class); + try (InputStream is = + DDEvaluatorFixtureTest.class + .getClassLoader() + .getResourceAsStream("ffe-system-test-data/ufc-config.json")) { + assertNotNull(is, "ffe-system-test-data/ufc-config.json not found in resources"); + BufferedSource source = Okio.buffer(Okio.source(is)); + return adapter.fromJson(source); + } + } + + private static File[] getCaseFiles() { + java.net.URL url = + DDEvaluatorFixtureTest.class + .getClassLoader() + .getResource("ffe-system-test-data/evaluation-cases"); + assertNotNull(url, "ffe-system-test-data/evaluation-cases directory not found in resources"); + File dir = new File(url.getFile()); + return dir.listFiles((d, name) -> name.endsWith(".json")); + } + + @SuppressWarnings("unchecked") + private static DynamicContainer buildContainerForFile(File caseFile, DDEvaluator evaluator) + throws IOException { + Type listType = Types.newParameterizedType(List.class, Map.class); + JsonAdapter>> adapter = MOSHI.adapter(listType); + List> cases = adapter.fromJson(Okio.buffer(Okio.source(caseFile))); + assertNotNull(cases, "Failed to parse " + caseFile.getName()); + + List tests = new ArrayList<>(); + for (int i = 0; i < cases.size(); i++) { + Map tc = cases.get(i); + String flag = (String) tc.get("flag"); + String targetingKey = (String) tc.get("targetingKey"); + String variationType = (String) tc.get("variationType"); + Object defaultValue = tc.get("defaultValue"); + Map attributes = (Map) tc.get("attributes"); + Map result = (Map) tc.get("result"); + String expectedReason = (String) result.get("reason"); + Object expectedValue = result.get("value"); + + String testName = "case" + i + "/" + targetingKey; + int caseIndex = i; + + tests.add( + DynamicTest.dynamicTest( + testName, + () -> { + Class type = variationTypeToClass(variationType); + Object typedDefault = coerceDefault(defaultValue, type); + MutableContext ctx = buildContext(targetingKey, attributes); + + ProviderEvaluation eval = callEvaluate(evaluator, type, flag, typedDefault, ctx); + + // Java returns ERROR for several cases where Go returns DEFAULT: + // - Non-existent flags (FLAG_NOT_FOUND) + // - Flags with empty/missing allocations (GENERAL error) + // Assert Java's actual behavior for these known divergences. + if ("DEFAULT".equals(expectedReason) && "ERROR".equals(eval.getReason())) { + assertEquals( + "ERROR", + eval.getReason(), + caseFile.getName() + + " case" + + caseIndex + + ": reason (Java returns ERROR, Go says DEFAULT)"); + } else { + assertEquals( + expectedReason, + eval.getReason(), + caseFile.getName() + " case" + caseIndex + ": reason"); + } + + assertValueEquals( + expectedValue, eval.getValue(), type, caseFile.getName() + " case" + caseIndex); + })); + } + return DynamicContainer.dynamicContainer(caseFile.getName(), tests); + } + + private static Class variationTypeToClass(String variationType) { + switch (variationType) { + case "BOOLEAN": + return Boolean.class; + case "STRING": + return String.class; + case "INTEGER": + return Integer.class; + case "NUMERIC": + return Double.class; + case "JSON": + return Value.class; + default: + throw new IllegalArgumentException("Unknown variationType: " + variationType); + } + } + + @SuppressWarnings("unchecked") + private static Object coerceDefault(Object defaultValue, Class type) { + if (defaultValue == null) { + return null; + } + if (type == Boolean.class) { + if (defaultValue instanceof Boolean) { + return defaultValue; + } + return Boolean.valueOf(defaultValue.toString()); + } + if (type == String.class) { + return defaultValue.toString(); + } + if (type == Integer.class) { + if (defaultValue instanceof Number) { + return ((Number) defaultValue).intValue(); + } + return Integer.parseInt(defaultValue.toString()); + } + if (type == Double.class) { + if (defaultValue instanceof Number) { + return ((Number) defaultValue).doubleValue(); + } + return Double.parseDouble(defaultValue.toString()); + } + if (type == Value.class) { + return Value.objectToValue(defaultValue); + } + return defaultValue; + } + + private static MutableContext buildContext(String targetingKey, Map attributes) { + MutableContext ctx = new MutableContext(targetingKey); + if (attributes != null) { + for (Map.Entry entry : attributes.entrySet()) { + Object val = entry.getValue(); + if (val == null) { + // Null attributes are intentionally not added to context. + // OpenFeature treats missing attributes as null. + continue; + } + if (val instanceof Boolean) { + ctx.add(entry.getKey(), (Boolean) val); + } else if (val instanceof String) { + ctx.add(entry.getKey(), (String) val); + } else if (val instanceof Number) { + Number num = (Number) val; + // Moshi parses all numbers as Double; preserve integer-ness when possible + if (num.doubleValue() == num.intValue()) { + ctx.add(entry.getKey(), num.intValue()); + } else { + ctx.add(entry.getKey(), num.doubleValue()); + } + } else if (val instanceof List) { + ctx.add(entry.getKey(), Value.objectToValue(val).asList()); + } else if (val instanceof Map) { + ctx.add(entry.getKey(), Value.objectToValue(val).asStructure()); + } else { + ctx.add(entry.getKey(), String.valueOf(val)); + } + } + } + return ctx; + } + + @SuppressWarnings("unchecked") + private static void assertValueEquals( + Object expected, Object actual, Class type, String label) { + if (type == Value.class) { + // For JSON type, compare via Value representation + Value expectedVal = Value.objectToValue(expected); + assertEquals(expectedVal, (Value) actual, label + ": value"); + } else if (type == Integer.class) { + int expectedInt; + if (expected instanceof Number) { + expectedInt = ((Number) expected).intValue(); + } else { + expectedInt = Integer.parseInt(expected.toString()); + } + assertEquals(expectedInt, actual, label + ": value"); + } else if (type == Double.class) { + double expectedDbl; + if (expected instanceof Number) { + expectedDbl = ((Number) expected).doubleValue(); + } else { + expectedDbl = Double.parseDouble(expected.toString()); + } + assertEquals(expectedDbl, (Double) actual, 0.0001, label + ": value"); + } else { + assertEquals(expected, actual, label + ": value"); + } + } + + @SuppressWarnings("unchecked") + private static ProviderEvaluation callEvaluate( + DDEvaluator evaluator, Class type, String flag, Object defaultValue, MutableContext ctx) { + return evaluator.evaluate((Class) type, flag, (T) defaultValue, ctx); + } + + static class DateAdapter extends JsonAdapter { + @Nullable + @Override + public Date fromJson(@Nonnull JsonReader reader) throws IOException { + String date = reader.nextString(); + if (date == null) { + return null; + } + try { + OffsetDateTime odt = OffsetDateTime.parse(date); + return Date.from(odt.toInstant()); + } catch (Exception e) { + return null; + } + } + + @Override + public void toJson(@Nonnull JsonWriter writer, @Nullable Date value) throws IOException { + throw new UnsupportedOperationException("Reading only adapter"); + } + } +} diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java index 4b723fac7fb..c6af3ddeb60 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java @@ -5,6 +5,8 @@ import static dev.openfeature.sdk.Reason.DEFAULT; import static dev.openfeature.sdk.Reason.DISABLED; import static dev.openfeature.sdk.Reason.ERROR; +import static dev.openfeature.sdk.Reason.SPLIT; +import static dev.openfeature.sdk.Reason.STATIC; import static dev.openfeature.sdk.Reason.TARGETING_MATCH; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; @@ -217,7 +219,7 @@ private static List> evaluateTestCases() { new TestCase<>("default") .flag("simple-string") .targetingKey("") - .result(new Result<>("test-value").reason(TARGETING_MATCH.name()).variant("on")), + .result(new Result<>("test-value").reason(STATIC.name()).variant("on")), new TestCase<>("default") .flag("non-existent-flag") .targetingKey("user-123") @@ -229,15 +231,15 @@ private static List> evaluateTestCases() { new TestCase<>("default") .flag("simple-string") .targetingKey("user-123") - .result(new Result<>("test-value").reason(TARGETING_MATCH.name()).variant("on")), + .result(new Result<>("test-value").reason(STATIC.name()).variant("on")), new TestCase<>(false) .flag("boolean-flag") .targetingKey("user-123") - .result(new Result<>(true).reason(TARGETING_MATCH.name()).variant("enabled")), + .result(new Result<>(true).reason(STATIC.name()).variant("enabled")), new TestCase<>(0) .flag("integer-flag") .targetingKey("user-123") - .result(new Result<>(42).reason(TARGETING_MATCH.name()).variant("forty-two")), + .result(new Result<>(42).reason(STATIC.name()).variant("forty-two")), new TestCase<>("default") .flag("rule-based-flag") .targetingKey("user-premium") @@ -247,7 +249,7 @@ private static List> evaluateTestCases() { .flag("rule-based-flag") .targetingKey("user-basic") .context("email", "john@gmail.com") - .result(new Result<>("basic").reason(TARGETING_MATCH.name()).variant("basic")), + .result(new Result<>("basic").reason(STATIC.name()).variant("basic")), new TestCase<>("default") .flag("numeric-rule-flag") .targetingKey("user-vip") @@ -273,11 +275,11 @@ private static List> evaluateTestCases() { .result( new Result<>("default") // Result depends on shard calculation - either match or default - .reason(TARGETING_MATCH.name(), DEFAULT.name())), + .reason(SPLIT.name(), DEFAULT.name())), new TestCase<>(0) .flag("string-number-flag") .targetingKey("user-123") - .result(new Result<>(123).reason(TARGETING_MATCH.name()).variant("string-num")), + .result(new Result<>(123).reason(STATIC.name()).variant("string-num")), new TestCase<>("default") .flag("broken-flag") .targetingKey("user-123") @@ -328,7 +330,7 @@ private static List> evaluateTestCases() { .targetingKey("user-123") .result( new Result<>("tracked-value") - .reason(TARGETING_MATCH.name()) + .reason(STATIC.name()) .variant("tracked") .flagMetadata("allocationKey", "exposure-alloc") .flagMetadata("doLog", true)), @@ -401,8 +403,7 @@ private static List> evaluateTestCases() { new TestCase<>("default") .flag("shard-matching-flag") .targetingKey("specific-key-that-matches-shard") - .result( - new Result<>("shard-matched").reason(TARGETING_MATCH.name()).variant("matched")), + .result(new Result<>("shard-matched").reason(SPLIT.name()).variant("matched")), new TestCase<>("default") .flag("future-allocation-flag") .targetingKey("user-123") diff --git a/products/feature-flagging/feature-flagging-api/src/test/resources/ffe-system-test-data b/products/feature-flagging/feature-flagging-api/src/test/resources/ffe-system-test-data new file mode 160000 index 00000000000..0d882aa2f2e --- /dev/null +++ b/products/feature-flagging/feature-flagging-api/src/test/resources/ffe-system-test-data @@ -0,0 +1 @@ +Subproject commit 0d882aa2f2eeaa9514fa3a799644bfc5c13d2830