From d404f0d6c52a7cf2960db04f7d65a69427a629bd Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Wed, 21 Oct 2026 13:47:21 +0200 Subject: [PATCH 01/16] feat(ffe): Restore the agent Feature Flags behaviors through the bridge Port the agent-side behaviors of the removed writers: send each event batch once with proxy discovery and recovery, and the Java SDK identity headers (#12477, #12575); flush the SDK pending events at agent shutdown, with or without tracing; declare the SDK settings the agent exposes (#12884, #12774). Move the Feature Flags shutdown tests from the agent tests to the OpenFeature smoke tests. Co-Authored-By: Claude Opus 5.5 --- .../DDOpenFeatureInstrumentation.java | 1 + .../openfeature/AgentShutdownSource.java | 18 ++ .../openfeature/JavaAgentBridge.java | 3 +- .../DDOpenFeatureInstrumentationTest.java | 11 + .../lang/ShutdownInstrumentationTest.java | 58 ----- .../delivery/DirectIntakeTransport.java | 8 +- .../delivery/DirectIntakeTransportTest.java | 7 +- ...bstractOpenFeatureShutdownSmokeTest.groovy | 135 ++++++++++ .../OpenFeatureShutdownSmokeTest.groovy | 10 + ...atureTraceDisabledShutdownSmokeTest.groovy | 13 + .../smoketest/AbstractSmokeTest.groovy | 4 +- .../api/config/FeatureFlaggingConfig.java | 15 ++ .../featureflag/FeatureFlaggingSystem.java | 38 ++- .../FeatureFlaggingSystemTest.java | 59 +++++ .../featureflag/FeatureFlaggingGateway.java | 8 + .../com/datadog/featureflag/EventProxy.java | 233 +++++++++++++++--- .../featureflag/FeatureFlagsBackend.java | 62 ++++- .../datadog/featureflag/EventProxyTest.java | 218 +++++++++++++--- .../featureflag/FeatureFlagsBackendTest.java | 45 +++- 19 files changed, 809 insertions(+), 137 deletions(-) create mode 100644 dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/AgentShutdownSource.java delete mode 100644 dd-java-agent/instrumentation/java/java-lang/java-lang-1.8/src/test/java/datadog/trace/instrumentation/java/lang/ShutdownInstrumentationTest.java create mode 100644 dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/AbstractOpenFeatureShutdownSmokeTest.groovy create mode 100644 dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureShutdownSmokeTest.groovy create mode 100644 dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureTraceDisabledShutdownSmokeTest.groovy diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentation.java b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentation.java index 20b6256dce2..972858e5a01 100644 --- a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentation.java +++ b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentation.java @@ -41,6 +41,7 @@ public String[] helperClassNames() { packageName + ".AgentConfigurationSource", packageName + ".AgentEventProxy", packageName + ".AgentHealthMetrics", + packageName + ".AgentShutdownSource", packageName + ".AgentSpanEnricher", packageName + ".DisabledAgentBridge", packageName + ".JavaAgentBridge", diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/AgentShutdownSource.java b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/AgentShutdownSource.java new file mode 100644 index 00000000000..9e73869ded4 --- /dev/null +++ b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/AgentShutdownSource.java @@ -0,0 +1,18 @@ +package datadog.trace.instrumentation.openfeature; + +import com.datadog.openfeature.internal.bridge.v1.ShutdownSource; +import datadog.trace.api.featureflag.FeatureFlaggingGateway; + +/** Notifies the agent shutdown, before the JVM shutdown hooks run. */ +public final class AgentShutdownSource implements ShutdownSource { + private final FeatureFlaggingGateway.Backend backend; + + AgentShutdownSource(final FeatureFlaggingGateway.Backend backend) { + this.backend = backend; + } + + @Override + public AutoCloseable subscribe(final Runnable listener) { + return this.backend.subscribeShutdown(listener); + } +} diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java index a2608c075cd..4bb9b0f351b 100644 --- a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java +++ b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java @@ -69,7 +69,6 @@ public SpanEnricher spanEnricher() { @Override public ShutdownSource shutdown() { - // TODO Flush the SDK through the agent shutdown. Until then, the SDK uses a JVM shutdown hook. - return null; + return new AgentShutdownSource(this.backend); } } diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentationTest.java b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentationTest.java index 61470453cd9..18fe2081cf7 100644 --- a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentationTest.java +++ b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentationTest.java @@ -88,6 +88,17 @@ void postsThroughTheEventProxyOnlyWhenAvailable() throws Exception { assertThrows(EventProxyUnavailableException.class, () -> proxy.post("exposures", payload)); } + @Test + void subscribesToTheAgentShutdown() throws Exception { + final FeatureFlaggingGateway.Backend backend = mock(FeatureFlaggingGateway.Backend.class); + final AutoCloseable subscription = mock(AutoCloseable.class); + final Runnable listener = mock(Runnable.class); + when(backend.subscribeShutdown(listener)).thenReturn(subscription); + FeatureFlaggingGateway.register(backend); + + assertSame(subscription, AgentBridges.detect().shutdown().subscribe(listener)); + } + @Test void delegatesSpanEnrichment() { final FeatureFlaggingGateway.Backend backend = mock(FeatureFlaggingGateway.Backend.class); diff --git a/dd-java-agent/instrumentation/java/java-lang/java-lang-1.8/src/test/java/datadog/trace/instrumentation/java/lang/ShutdownInstrumentationTest.java b/dd-java-agent/instrumentation/java/java-lang/java-lang-1.8/src/test/java/datadog/trace/instrumentation/java/lang/ShutdownInstrumentationTest.java deleted file mode 100644 index 13736934cda..00000000000 --- a/dd-java-agent/instrumentation/java/java-lang/java-lang-1.8/src/test/java/datadog/trace/instrumentation/java/lang/ShutdownInstrumentationTest.java +++ /dev/null @@ -1,58 +0,0 @@ -package datadog.trace.instrumentation.java.lang; - -import static datadog.trace.agent.tooling.InstrumenterModule.TargetSystem.TRACING; -import static java.util.Collections.emptySet; -import static java.util.Collections.singleton; -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 datadog.trace.agent.tooling.InstrumenterModule.TargetSystem; -import datadog.trace.test.junit.utils.config.WithConfig; -import datadog.trace.test.util.DDJavaSpecification; -import java.util.EnumSet; -import org.junit.jupiter.api.Test; - -class ShutdownInstrumentationTest extends DDJavaSpecification { - @Test - void defaultConfigurationDoesNotEnableWholeAgentShutdownWithoutTracing() { - assertFalse(new ShutdownInstrumentation().isApplicable(emptySet())); - } - - @Test - @WithConfig(key = "feature.flags.enabled", value = "true") - void generatedTargetMaskDoesNotDependOnFeatureFlagBuildConfiguration() { - final EnumSet systems = EnumSet.noneOf(TargetSystem.class); - for (TargetSystem system : TargetSystem.values()) { - if (new ShutdownInstrumentation().isApplicable(singleton(system))) { - systems.add(system); - } - } - assertEquals(EnumSet.of(TRACING), systems); - } - - @Test - @WithConfig(key = "feature.flags.enabled", value = "false") - void doesNotInstrumentShutdownWhenTracingAndFeatureFlagsAreDisabled() { - assertFalse(new ShutdownInstrumentation().isApplicable(emptySet())); - } - - @Test - @WithConfig(key = "feature.flags.enabled", value = "false") - void preservesTracingShutdown() { - assertTrue(new ShutdownInstrumentation().isApplicable(singleton(TRACING))); - } - - @Test - @WithConfig(key = "feature.flags.configuration.source", value = "agentless") - void featureFlagsDoNotEnableWholeAgentShutdownWithoutTracing() { - assertFalse(new ShutdownInstrumentation().isApplicable(emptySet())); - } - - @Test - @WithConfig(key = "feature.flags.enabled", value = "false") - @WithConfig(key = "feature.flags.configuration.source", value = "agentless") - void honorsExplicitDisableEvenWithAConfigurationSource() { - assertFalse(new ShutdownInstrumentation().isApplicable(emptySet())); - } -} diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/DirectIntakeTransport.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/DirectIntakeTransport.java index 509f24fb699..fa87a169b42 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/DirectIntakeTransport.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/DirectIntakeTransport.java @@ -4,6 +4,7 @@ import static java.net.http.HttpRequest.BodyPublishers.ofByteArray; import com.datadog.openfeature.internal.bridge.v1.EventTransport; +import com.datadog.openfeature.internal.config.SdkVersion; import com.datadog.openfeature.internal.config.Settings; import java.io.IOException; import java.net.URI; @@ -22,6 +23,9 @@ public final class DirectIntakeTransport implements EventTransport { private static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(10); + /** Identifies the events like the agent event proxy does. */ + static final String ORIGIN = "dd-trace-java"; + private final URI intake; private final String apiKey; private final String traceId; @@ -62,7 +66,9 @@ public void post(final String route, final byte[] json) throws IOException { .header("Content-Type", "application/json") .header("dd-api-key", this.apiKey) .header("x-datadog-trace-id", this.traceId) - .header("x-datadog-parent-id", this.traceId); + .header("x-datadog-parent-id", this.traceId) + .header("DD-EVP-ORIGIN", ORIGIN) + .header("DD-EVP-ORIGIN-VERSION", SdkVersion.VERSION); Headers.addMetadata(builder); final HttpRequest request = builder.POST(ofByteArray(json)).build(); final int status; diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/DirectIntakeTransportTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/DirectIntakeTransportTest.java index ed3f92e35cc..d2b87e44304 100644 --- a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/DirectIntakeTransportTest.java +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/DirectIntakeTransportTest.java @@ -2,6 +2,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -12,6 +13,7 @@ import static org.mockito.Mockito.verify; import com.datadog.openfeature.internal.LocalHttpServer; +import com.datadog.openfeature.internal.config.SdkVersion; import com.datadog.openfeature.internal.config.TestSettings; import java.io.IOException; import java.io.InterruptedIOException; @@ -56,7 +58,7 @@ void rejectsInvalidSite() { } @Test - void postsJsonWithApiKeyAndTraceHeaders() throws Exception { + void postsJsonWithApiKeyTraceAndIdentityHeaders() throws Exception { this.server.enqueue(202, null); transport().post("exposures", "{\"a\":1}".getBytes(UTF_8)); @@ -71,6 +73,9 @@ void postsJsonWithApiKeyAndTraceHeaders() throws Exception { assertEquals(request.header("x-datadog-trace-id"), request.header("x-datadog-parent-id")); assertEquals("java", request.header("Datadog-Meta-Lang")); assertNull(request.header("X-Datadog-EVP-Subdomain")); + assertEquals("dd-trace-java", request.header("DD-EVP-ORIGIN")); + assertEquals(SdkVersion.VERSION, request.header("DD-EVP-ORIGIN-VERSION")); + assertNotEquals("unknown", request.header("DD-EVP-ORIGIN-VERSION")); } @TableTest({ diff --git a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/AbstractOpenFeatureShutdownSmokeTest.groovy b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/AbstractOpenFeatureShutdownSmokeTest.groovy new file mode 100644 index 00000000000..240131577d1 --- /dev/null +++ b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/AbstractOpenFeatureShutdownSmokeTest.groovy @@ -0,0 +1,135 @@ +package datadog.smoketest.springboot + +import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer +import static java.util.concurrent.TimeUnit.SECONDS + +import datadog.smoketest.AbstractSmokeTest +import datadog.trace.agent.test.server.http.TestHttpServer +import datadog.trace.agent.test.server.http.TestHttpServer.HandlerApi.RequestApi +import datadog.trace.agent.test.utils.OkHttpUtils +import datadog.trace.agent.test.utils.PortUtils +import groovy.json.JsonOutput +import groovy.json.JsonSlurper +import okhttp3.MediaType +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.RequestBody +import spock.lang.AutoCleanup +import spock.lang.Shared +import spock.util.concurrent.PollingConditions + +/** The SDK with the Datadog Java agent flushes its pending events when the application exits. */ +abstract class AbstractOpenFeatureShutdownSmokeTest extends AbstractSmokeTest { + + @Shared + protected int httpPort = PortUtils.randomOpenPort() + + protected OkHttpClient client = OkHttpUtils.client() + + @Shared + protected final ufcConfig = new JsonSlurper().parse(fetchResource("ffe-system-test-data/ufc-config.json")) as Map + + /** Serves the flag configuration as the Datadog managed CDN does. */ + @Shared + @AutoCleanup + protected TestHttpServer cdn = httpServer { + handlers { + prefix("/api/v2/feature-flagging/config/rules-based/server") { + response.status(200).send(JsonOutput.toJson([ + data: [type: 'universal-flag-configuration', attributes: ufcConfig] + ])) + } + } + } + + /** @return whether the application runs with tracing enabled. */ + abstract boolean tracing() + + def setupSpec() { + PortUtils.waitForPortToOpen(httpPort, 240, SECONDS, testedProcess) + } + + @Override + ProcessBuilder createProcessBuilder() { + final springBootShadowJar = System.getProperty("datadog.smoketest.springboot.shadowJar.path") + final command = [javaPath()] + command.addAll(defaultJavaProperties) + command.add("-Ddd.trace.enabled=${tracing()}".toString()) + command.addAll(['-jar', springBootShadowJar, "--server.port=${httpPort}".toString()]) + final builder = new ProcessBuilder(command).directory(new File(buildDirectory)) + builder.environment().put('DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL', "http://localhost:${cdn.address.port}".toString()) + return builder + } + + @Override + boolean testTelemetry() { + return false + } + + @Override + Closure decodedTracesCallback() { + return {} + } + + @Override + Closure decodedEvpProxyMessageCallback() { + return { String path, RequestApi request -> + if (!path.contains('api/v2/flagevaluation')) { + return null + } + return [ + origin: request.getHeader('DD-EVP-ORIGIN'), + originVersion: request.getHeader('DD-EVP-ORIGIN-VERSION'), + body: new JsonSlurper().parse(request.body) + ] + } + } + + void 'test pending flag evaluations are flushed at application exit'() { + when: + // The first evaluation is flushed within the 10 seconds flush interval, which then restarts: + // the next evaluation stays pending until the application exits. + evaluate('test-case-boolean-false-assignment.json') + new PollingConditions(timeout: 20).eventually { + assert evaluatedFlags().contains('boolean-false-assignment') + } + evaluate('test-case-boolean-one-of-matches.json') + + then: + !evaluatedFlags().contains('boolean-one-of-matches') + + when: + testedProcess.destroy() + + then: + testedProcess.waitFor(30, SECONDS) + evaluatedFlags().contains('boolean-one-of-matches') + evaluationRequests().every { it.origin == 'dd-trace-java' && it.originVersion } + } + + protected void evaluate(final String testCaseFile) { + final testCase = (new JsonSlurper().parse(fetchResource("ffe-system-test-data/evaluation-cases/${testCaseFile}")) as List>).first() + final request = new Request.Builder() + .url("http://localhost:${httpPort}/openfeature/evaluate") + .post(RequestBody.create(MediaType.parse('application/json'), JsonOutput.toJson(testCase))) + .build() + final response = client.newCall(request).execute() + try { + assert response.code() == 200 + } finally { + response.close() + } + } + + protected List evaluatedFlags() { + return evaluationRequests()*.body*.flagEvaluations.flatten()*.flag*.key as List + } + + protected List> evaluationRequests() { + return evpProxyMessages*.getV2() as List> + } + + protected static URL fetchResource(final String name) { + return Thread.currentThread().getContextClassLoader().getResource(name) + } +} diff --git a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureShutdownSmokeTest.groovy b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureShutdownSmokeTest.groovy new file mode 100644 index 00000000000..70192aa9331 --- /dev/null +++ b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureShutdownSmokeTest.groovy @@ -0,0 +1,10 @@ +package datadog.smoketest.springboot + +/** The tracing shutdown hook flushes the SDK pending events. */ +class OpenFeatureShutdownSmokeTest extends AbstractOpenFeatureShutdownSmokeTest { + + @Override + boolean tracing() { + return true + } +} diff --git a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureTraceDisabledShutdownSmokeTest.groovy b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureTraceDisabledShutdownSmokeTest.groovy new file mode 100644 index 00000000000..c300962d260 --- /dev/null +++ b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureTraceDisabledShutdownSmokeTest.groovy @@ -0,0 +1,13 @@ +package datadog.smoketest.springboot + +/** + * Without tracing, the Feature Flagging system registers its own shutdown hook to flush the SDK + * pending events. + */ +class OpenFeatureTraceDisabledShutdownSmokeTest extends AbstractOpenFeatureShutdownSmokeTest { + + @Override + boolean tracing() { + return false + } +} diff --git a/dd-smoke-tests/src/main/groovy/datadog/smoketest/AbstractSmokeTest.groovy b/dd-smoke-tests/src/main/groovy/datadog/smoketest/AbstractSmokeTest.groovy index fbc65898777..5187f1f8af1 100644 --- a/dd-smoke-tests/src/main/groovy/datadog/smoketest/AbstractSmokeTest.groovy +++ b/dd-smoke-tests/src/main/groovy/datadog/smoketest/AbstractSmokeTest.groovy @@ -96,7 +96,9 @@ abstract class AbstractSmokeTest extends ProcessManager { "Accept-Encoding", "Content-Encoding", "User-Agent", - "DD-CI-PROVIDER-NAME" + "DD-CI-PROVIDER-NAME", + "DD-EVP-ORIGIN", + "DD-EVP-ORIGIN-VERSION" ], "config": {} }""") diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/FeatureFlaggingConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/FeatureFlaggingConfig.java index 9a232111344..a2e6a5378f4 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/FeatureFlaggingConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/FeatureFlaggingConfig.java @@ -13,5 +13,20 @@ public final class FeatureFlaggingConfig { public static final String FEATURE_FLAGS_CONFIGURATION_SOURCE = "feature.flags.configuration.source"; + /** + * 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"; + + /** + * Lets the {@code dd_evaluation_telemetry_opt_out} evaluation context attribute opt subjects out + * of evaluation telemetry. Default: disabled. Maps to {@code + * DD_FEATURE_FLAGS_EVALUATION_TELEMETRY_PER_SUBJECT_OPT_OUT_ENABLED}. + */ + public static final String FEATURE_FLAGS_EVALUATION_TELEMETRY_PER_SUBJECT_OPT_OUT_ENABLED = + "feature.flags.evaluation.telemetry.per.subject.opt.out.enabled"; + private FeatureFlaggingConfig() {} } diff --git a/products/feature-flagging/feature-flagging-agent/src/main/java/com/datadog/featureflag/FeatureFlaggingSystem.java b/products/feature-flagging/feature-flagging-agent/src/main/java/com/datadog/featureflag/FeatureFlaggingSystem.java index 8bdd58f2be8..63590036b16 100644 --- a/products/feature-flagging/feature-flagging-agent/src/main/java/com/datadog/featureflag/FeatureFlaggingSystem.java +++ b/products/feature-flagging/feature-flagging-agent/src/main/java/com/datadog/featureflag/FeatureFlaggingSystem.java @@ -1,5 +1,8 @@ package com.datadog.featureflag; +import static datadog.trace.util.AgentThreadFactory.AgentThread.FEATURE_FLAG_SHUTDOWN; +import static datadog.trace.util.AgentThreadFactory.newAgentThread; + import datadog.communication.ddagent.SharedCommunicationObjects; import datadog.trace.api.Config; import datadog.trace.api.featureflag.FeatureFlaggingGateway; @@ -16,6 +19,7 @@ public class FeatureFlaggingSystem { private static final Logger LOGGER = LoggerFactory.getLogger(FeatureFlaggingSystem.class); private static volatile FeatureFlagsBackend BACKEND; + private static volatile Thread SHUTDOWN_HOOK; private FeatureFlaggingSystem() {} @@ -34,7 +38,12 @@ public static synchronized void start(final SharedCommunicationObjects sco) { return; } LOGGER.debug("Feature Flagging enabled; env={}", config.getEnv()); - final FeatureFlagsBackend backend = new FeatureFlagsBackend(sco, config); + // Tracing owns the shutdown path running before the JVM shutdown hooks. Without it, stop from a + // shutdown hook once the SDK subscribes to the shutdown, so it still flushes its pending + // events. + final FeatureFlagsBackend backend = + new FeatureFlagsBackend( + sco, config, config.isTraceEnabled() ? null : FeatureFlaggingSystem::addShutdownHook); try { backend.start(); } catch (final RuntimeException | Error e) { @@ -55,10 +64,37 @@ public static synchronized void start(final SharedCommunicationObjects sco) { public static synchronized void stop() { final FeatureFlagsBackend backend = BACKEND; BACKEND = null; + final Thread shutdownHook = SHUTDOWN_HOOK; + SHUTDOWN_HOOK = null; + if (shutdownHook != null && shutdownHook != Thread.currentThread()) { + try { + Runtime.getRuntime().removeShutdownHook(shutdownHook); + } catch (final IllegalStateException ignored) { + // The JVM is already shutting down; the hook finds the system stopped. + } + } FeatureFlaggingGateway.register(null); if (backend != null) { backend.close(); } LOGGER.debug("Feature Flagging system stopped"); } + + private static synchronized void addShutdownHook() { + if (BACKEND == null || SHUTDOWN_HOOK != null) { + return; + } + final Thread hook = newAgentThread(FEATURE_FLAG_SHUTDOWN, FeatureFlaggingSystem::stop); + try { + Runtime.getRuntime().addShutdownHook(hook); + } catch (final IllegalStateException | SecurityException e) { + LOGGER.debug("Could not register the Feature Flagging shutdown hook", e); + return; + } + SHUTDOWN_HOOK = hook; + } + + static boolean isShutdownHookRegistered() { + return SHUTDOWN_HOOK != null; + } } diff --git a/products/feature-flagging/feature-flagging-agent/src/test/java/com/datadog/featureflag/FeatureFlaggingSystemTest.java b/products/feature-flagging/feature-flagging-agent/src/test/java/com/datadog/featureflag/FeatureFlaggingSystemTest.java index 0935d1914cb..c2a265544d1 100644 --- a/products/feature-flagging/feature-flagging-agent/src/test/java/com/datadog/featureflag/FeatureFlaggingSystemTest.java +++ b/products/feature-flagging/feature-flagging-agent/src/test/java/com/datadog/featureflag/FeatureFlaggingSystemTest.java @@ -3,10 +3,13 @@ import static datadog.trace.api.config.FeatureFlaggingConfig.FEATURE_FLAGS_CONFIGURATION_SOURCE; import static datadog.trace.api.config.FeatureFlaggingConfig.FEATURE_FLAGS_ENABLED; import static datadog.trace.api.config.RemoteConfigConfig.REMOTE_CONFIGURATION_ENABLED; +import static datadog.trace.api.config.TraceInstrumentationConfig.TRACE_ENABLED; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -19,6 +22,7 @@ import datadog.remoteconfig.Product; import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.test.junit.utils.config.WithConfig; +import java.lang.reflect.Field; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -65,6 +69,61 @@ void failedStartRollsBackTheRemoteConfigRegistration() { verify(poller).removeListeners(Product.FFE_FLAGS); } + @Test + @WithConfig(key = TRACE_ENABLED, value = "false") + void stopsFromAShutdownHookOnceTheSdkSubscribesWhenTracingIsDisabled() { + FeatureFlaggingSystem.start(mock(SharedCommunicationObjects.class)); + assertFalse( + FeatureFlaggingSystem.isShutdownHookRegistered(), "no hook while the SDK is unused"); + + FeatureFlaggingGateway.backend().subscribeShutdown(() -> {}); + assertTrue(FeatureFlaggingSystem.isShutdownHookRegistered()); + + FeatureFlaggingSystem.stop(); + assertFalse(FeatureFlaggingSystem.isShutdownHookRegistered()); + } + + @Test + @WithConfig(key = TRACE_ENABLED, value = "false") + void shutdownHookFlushesListenersAndStopsTheBackend() throws Exception { + FeatureFlaggingSystem.start(mock(SharedCommunicationObjects.class)); + final Runnable listener = mock(Runnable.class); + FeatureFlaggingGateway.backend().subscribeShutdown(listener); + + final Field field = FeatureFlaggingSystem.class.getDeclaredField("SHUTDOWN_HOOK"); + field.setAccessible(true); + final Thread hook = (Thread) field.get(null); + // Run the real hook without shutting down the test JVM. + assertTrue(Runtime.getRuntime().removeShutdownHook(hook)); + hook.start(); + hook.join(5000); + + assertFalse(hook.isAlive(), "the shutdown hook must finish"); + assertNull(FeatureFlaggingGateway.backend()); + assertFalse(FeatureFlaggingSystem.isShutdownHookRegistered()); + verify(listener).run(); + } + + @Test + @WithConfig(key = TRACE_ENABLED, value = "false") + void lateShutdownSubscriptionDoesNotRegisterAHookAfterStop() throws Exception { + FeatureFlaggingSystem.start(mock(SharedCommunicationObjects.class)); + final FeatureFlaggingGateway.Backend backend = FeatureFlaggingGateway.backend(); + FeatureFlaggingSystem.stop(); + + try (final AutoCloseable ignored = backend.subscribeShutdown(() -> {})) { + assertNull(FeatureFlaggingGateway.backend()); + assertFalse(FeatureFlaggingSystem.isShutdownHookRegistered()); + } + } + + @Test + void leavesTheShutdownToTracingWhenEnabled() { + FeatureFlaggingSystem.start(mock(SharedCommunicationObjects.class)); + FeatureFlaggingGateway.backend().subscribeShutdown(() -> {}); + assertFalse(FeatureFlaggingSystem.isShutdownHookRegistered()); + } + @Test @WithConfig(key = FEATURE_FLAGS_ENABLED, value = "false") void startDoesNothingWhenFeatureFlagsIsDisabled() { diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java index 1f1cfcaca76..a20b965a96c 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java @@ -98,5 +98,13 @@ public interface Backend { * @param defaultValue the native default value. */ void enrichRuntimeDefault(String flagKey, @Nullable Object defaultValue); + + /** + * Subscribes to the agent shutdown, notified before the JVM shutdown hooks run. + * + * @param listener the listener run at shutdown, typically flushing the SDK pending events. + * @return the handle to close to unsubscribe. + */ + AutoCloseable subscribeShutdown(Runnable listener); } } diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/EventProxy.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/EventProxy.java index ccc28f1e858..6b00fca54e4 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/EventProxy.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/EventProxy.java @@ -2,45 +2,108 @@ import static com.datadog.featureflag.FeatureFlagEventType.EXPOSURE; import static com.datadog.featureflag.FeatureFlagEventType.FLAG_EVALUATION; +import static datadog.communication.EvpProxy.JAVA_TRACING_LIBRARY; +import static datadog.communication.EvpProxy.ORIGIN_HEADER; +import static datadog.communication.EvpProxy.ORIGIN_VERSION_HEADER; +import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V2_EVP_PROXY_ENDPOINT; import static datadog.communication.http.HttpRetryPolicy.Factory.NEVER_RETRY; import static datadog.trace.api.intake.Intake.EVENT_PLATFORM; +import static java.util.Arrays.asList; +import static java.util.Collections.unmodifiableMap; +import static java.util.concurrent.TimeUnit.MINUTES; import datadog.communication.BackendApi; import datadog.communication.BackendApiFactory; import datadog.communication.HttpResponseException; +import datadog.communication.ddagent.TracerVersion; +import datadog.communication.http.HttpRetryPolicy; import java.io.IOException; import java.net.ConnectException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.LongSupplier; import javax.annotation.Nullable; import okhttp3.MediaType; import okhttp3.RequestBody; +import okhttp3.Response; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; -/** Posts Feature Flags events through the Datadog Agent event platform proxy. */ +/** + * Posts Feature Flags events through the Datadog Agent event platform proxy. + * + *

With the Remote Configuration source, events always go to the Agent through the fixed v2 proxy + * endpoint. With the agentless source, the proxy endpoint is discovered once and shared by both + * routes, provided it forwards the identity headers sent with every event. When it is not available + * and the SDK cannot fall back to direct intake, the proxy route stays in place and the endpoint is + * discovered again at most once per recovery interval. + * + *

Payloads are sent once: only a connection refusal, proven to happen before sending, is + * retried, and only with the Remote Configuration source. + */ class EventProxy { + private static final Logger LOGGER = LoggerFactory.getLogger(EventProxy.class); private static final MediaType JSON = MediaType.parse("application/json"); static final String EXPOSURES_ROUTE = "exposures"; + static final long RECOVERY_INTERVAL_NANOS = MINUTES.toNanos(1); + + /** Identifies the library sending the events, on every proxy request. */ + static final Map IDENTITY_HEADERS = identityHeaders(); + + /** The headers a discovered proxy endpoint must forward to keep the event identity. */ + static final List IDENTITY_HEADER_NAMES = asList(ORIGIN_HEADER, ORIGIN_VERSION_HEADER); + + /** Retries connection refusal only; automatic retries and redirects remain disabled. */ + static final HttpRetryPolicy.Factory PRE_SEND_RETRY = + new HttpRetryPolicy.Factory(3, 100, 2.0) { + @Override + public HttpRetryPolicy create() { + return new PreSendRetryPolicy(); + } + }; private final BackendApiFactory backendApiFactory; + private final boolean agentless; private final boolean directFallbackAvailable; - private volatile BackendApi exposures; - private volatile BackendApi flagEvaluations; - private volatile boolean resolved; + private final LongSupplier nanoTime; + private volatile Clients clients; + private boolean discovered; + private long nextDiscoveryNanos; /** - * @param backendApiFactory the factory of event platform proxy clients. + * @param backendApiFactory the send-once factory of event platform proxy clients. + * @param agentless whether the configuration source is agentless, so events use the discovered + * proxy endpoint, or direct intake when the SDK can fall back to it. * @param directFallbackAvailable whether the SDK falls back to direct intake when the proxy is - * unavailable, in which case the proxy must not retry what the intake would accept. + * unavailable. */ - EventProxy(final BackendApiFactory backendApiFactory, final boolean directFallbackAvailable) { + EventProxy( + final BackendApiFactory backendApiFactory, + final boolean agentless, + final boolean directFallbackAvailable) { + this(backendApiFactory, agentless, directFallbackAvailable, System::nanoTime); + } + + EventProxy( + final BackendApiFactory backendApiFactory, + final boolean agentless, + final boolean directFallbackAvailable, + final LongSupplier nanoTime) { this.backendApiFactory = backendApiFactory; + this.agentless = agentless; this.directFallbackAvailable = directFallbackAvailable; + this.nanoTime = nanoTime; } /** - * @return whether the Datadog Agent supports the event platform proxy. + * @return whether events go through the event platform proxy: always with the Remote + * Configuration source, and with the agentless source when the proxy endpoint is discovered + * or when direct intake is not available. */ boolean isAvailable() { - resolve(); - return this.exposures != null; + // Only the agentless source can lack proxy clients. + return current() != null || !this.directFallbackAvailable; } /** @@ -48,57 +111,167 @@ boolean isAvailable() { * * @param route the event platform route. * @param json the UTF-8 JSON payload. - * @return {@code true} if delivered, {@code false} if the proxy is definitively unavailable. + * @return {@code true} if delivered, {@code false} if the proxy definitively did not receive the + * payload. * @throws IOException if the payload could not be delivered. */ boolean post(final String route, final byte[] json) throws IOException { - resolve(); - final BackendApi api = EXPOSURES_ROUTE.equals(route) ? this.exposures : this.flagEvaluations; - if (api == null) { - return false; + Clients current = current(); + if (current == null) { + current = recover(); + if (current == null) { + return false; + } } + final BackendApi api = + EXPOSURES_ROUTE.equals(route) ? current.exposures : current.flagEvaluations; try { api.post(route, RequestBody.create(JSON, json), stream -> null, null, false); return true; } catch (final IOException e) { if (isDefinitiveRejection(e)) { + onDefinitiveRejection(current); return false; } throw e; } } - private void resolve() { - if (this.resolved) { - return; + @Nullable + private Clients current() { + final Clients current = this.clients; + if (current != null) { + return current; } synchronized (this) { - if (this.resolved) { - return; + if (!this.discovered) { + this.discovered = true; + this.clients = + this.agentless ? discover(false) : clients(PRE_SEND_RETRY, V2_EVP_PROXY_ENDPOINT); + if (this.clients == null) { + onUnavailable(); + } } - this.exposures = create(EXPOSURE); - this.flagEvaluations = create(FLAG_EVALUATION); - this.resolved = true; + return this.clients; } } @Nullable - private BackendApi create(final FeatureFlagEventType eventType) { - return this.directFallbackAvailable - ? this.backendApiFactory.createEvpProxyApi( - EVENT_PLATFORM, eventType.responseCompressionEnabled(), NEVER_RETRY) - : this.backendApiFactory.createEvpProxyApi( - EVENT_PLATFORM, eventType.responseCompressionEnabled()); + private synchronized Clients recover() { + if (this.clients != null) { + return this.clients; + } + if (this.directFallbackAvailable || this.nanoTime.getAsLong() - this.nextDiscoveryNanos < 0) { + return null; + } + this.clients = discover(true); + if (this.clients == null) { + scheduleDiscovery(); + } else { + LOGGER.info("Feature Flags event delivery recovered through the Datadog Agent event proxy"); + } + return this.clients; } + private synchronized void onDefinitiveRejection(final Clients failed) { + // Only the agentless proxy route without direct intake waits for a new endpoint. An in-flight + // send on an old endpoint cannot invalidate a newly recovered one. + if (this.agentless && !this.directFallbackAvailable && this.clients == failed) { + this.clients = null; + onUnavailable(); + } + } + + /** Waits for a new proxy endpoint when it is the only route. Runs once per outage. */ + private void onUnavailable() { + if (!this.directFallbackAvailable) { + scheduleDiscovery(); + LOGGER.warn( + "Feature Flags events are being dropped because no local EVP proxy or direct intake" + + " route is available. Make the Agent reachable, or configure DD_API_KEY and a" + + " valid DD_SITE."); + } + } + + private void scheduleDiscovery() { + this.nextDiscoveryNanos = this.nanoTime.getAsLong() + RECOVERY_INTERVAL_NANOS; + } + + @Nullable + private Clients discover(final boolean force) { + final String endpoint; + try { + endpoint = this.backendApiFactory.discoverEvpProxyEndpoint(force, IDENTITY_HEADER_NAMES); + } catch (final RuntimeException e) { + LOGGER.debug("Could not discover the Datadog Agent event proxy", e); + return null; + } + return endpoint == null ? null : clients(NEVER_RETRY, endpoint); + } + + private Clients clients(final HttpRetryPolicy.Factory retryPolicy, final String endpoint) { + return new Clients( + this.backendApiFactory.createEvpProxyApiForEndpoint( + EVENT_PLATFORM, EXPOSURE.responseCompressionEnabled(), retryPolicy, endpoint), + this.backendApiFactory.createEvpProxyApiForEndpoint( + EVENT_PLATFORM, FLAG_EVALUATION.responseCompressionEnabled(), retryPolicy, endpoint)); + } + + private static Map identityHeaders() { + final Map headers = new HashMap<>(2); + headers.put(ORIGIN_HEADER, JAVA_TRACING_LIBRARY); + headers.put(ORIGIN_VERSION_HEADER, TracerVersion.TRACER_VERSION); + return unmodifiableMap(headers); + } + + /** + * Tells whether the proxy definitively did not receive a payload, so it can be replayed through + * direct intake. + * + * @param exception the post failure. + * @return whether the failure is a connection refusal or a missing proxy route. + */ static boolean isDefinitiveRejection(final IOException exception) { if (exception instanceof ConnectException) { return true; } if (exception instanceof HttpResponseException) { final int statusCode = ((HttpResponseException) exception).getStatusCode(); - return statusCode == 403 || statusCode == 404 || statusCode == 405; + return statusCode == 404 || statusCode == 405; } return false; } + + /** The proxy clients of both routes, for one endpoint. */ + private static final class Clients { + final BackendApi exposures; + final BackendApi flagEvaluations; + + Clients(final BackendApi exposures, final BackendApi flagEvaluations) { + this.exposures = exposures; + this.flagEvaluations = flagEvaluations; + } + } + + private static final class PreSendRetryPolicy extends HttpRetryPolicy { + private int attemptsLeft = 3; + + private PreSendRetryPolicy() { + super(3, 100, 2.0, false); + } + + @Override + public boolean shouldRetry(final Exception exception) { + if (exception instanceof ConnectException && this.attemptsLeft > 0) { + this.attemptsLeft--; + return true; + } + return false; + } + + @Override + public boolean shouldRetry(@Nullable final Response response) { + return false; + } + } } diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FeatureFlagsBackend.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FeatureFlagsBackend.java index 0fdc3e3d712..10d2d69837f 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FeatureFlagsBackend.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FeatureFlagsBackend.java @@ -13,11 +13,17 @@ import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.telemetry.CoreMetricCollector; import java.io.IOException; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** The agent services exposed to the Feature Flags SDK instrumentation. */ public final class FeatureFlagsBackend implements FeatureFlaggingGateway.Backend, AutoCloseable { + private static final Logger LOGGER = LoggerFactory.getLogger(FeatureFlagsBackend.class); private static final String API_KEY = "api-key"; private static final String SITE = "site"; private static final String ENV = "env"; @@ -29,18 +35,33 @@ public final class FeatureFlagsBackend implements FeatureFlaggingGateway.Backend private final EventProxy eventProxy; private final SpanEnrichmentWriter spanEnrichmentWriter; private final CoreMetricCollector metrics; + private final List shutdownListeners = new CopyOnWriteArrayList<>(); + @Nullable private final Runnable onFirstShutdownSubscription; + private final AtomicBoolean shutdownSubscribed = new AtomicBoolean(); private RemoteConfigServiceImpl remoteConfig; - public FeatureFlagsBackend(final SharedCommunicationObjects sco, final Config config) { + /** + * @param sco the shared communication objects. + * @param config the agent configuration. + * @param onFirstShutdownSubscription the action to run when the SDK first subscribes to the + * shutdown, or {@code null} for none. + */ + public FeatureFlagsBackend( + final SharedCommunicationObjects sco, + final Config config, + @Nullable final Runnable onFirstShutdownSubscription) { this( sco, config, new EventProxy( - new BackendApiFactory(config, sco), + new BackendApiFactory(config, sco, EventProxy.IDENTITY_HEADERS, true), + config.getFeatureFlaggingConfigurationSource() == AGENTLESS, config.getFeatureFlaggingConfigurationSource() == AGENTLESS - && config.getApiKey() != null), + && config.getApiKey() != null + && !config.getApiKey().isEmpty()), SpanEnrichmentWriter.getInstance(), - CoreMetricCollector.getInstance()); + CoreMetricCollector.getInstance(), + onFirstShutdownSubscription); } FeatureFlagsBackend( @@ -48,12 +69,14 @@ public FeatureFlagsBackend(final SharedCommunicationObjects sco, final Config co final Config config, final EventProxy eventProxy, final SpanEnrichmentWriter spanEnrichmentWriter, - final CoreMetricCollector metrics) { + final CoreMetricCollector metrics, + @Nullable final Runnable onFirstShutdownSubscription) { this.sco = sco; this.config = config; this.eventProxy = eventProxy; this.spanEnrichmentWriter = spanEnrichmentWriter; this.metrics = metrics; + this.onFirstShutdownSubscription = onFirstShutdownSubscription; } /** Registers the Remote Configuration product early when it is the configuration source. */ @@ -136,7 +159,34 @@ public void enrichRuntimeDefault(final String flagKey, @Nullable final Object de } @Override - public synchronized void close() { + public AutoCloseable subscribeShutdown(final Runnable listener) { + this.shutdownListeners.add(listener); + if (this.onFirstShutdownSubscription != null + && this.shutdownSubscribed.compareAndSet(false, true)) { + this.onFirstShutdownSubscription.run(); + } + return () -> this.shutdownListeners.remove(listener); + } + + /** + * Notifies the shutdown listeners, flushing the SDK while the agent services are still running, + * then releases the backend resources. + */ + @Override + public void close() { + // Run outside the lock: the SDK flush posts events and counts metrics through this backend. + for (final Runnable listener : this.shutdownListeners) { + try { + listener.run(); + } catch (final RuntimeException | LinkageError e) { + LOGGER.debug("Feature Flags shutdown listener failed", e); + } + } + this.shutdownListeners.clear(); + closeResources(); + } + + private synchronized void closeResources() { if (this.remoteConfig != null) { this.remoteConfig.close(); this.remoteConfig = null; diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/EventProxyTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/EventProxyTest.java index c0492e04819..5d44674e4f3 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/EventProxyTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/EventProxyTest.java @@ -1,85 +1,196 @@ package com.datadog.featureflag; +import static com.datadog.featureflag.EventProxy.IDENTITY_HEADERS; +import static com.datadog.featureflag.EventProxy.IDENTITY_HEADER_NAMES; +import static com.datadog.featureflag.EventProxy.PRE_SEND_RETRY; +import static com.datadog.featureflag.EventProxy.RECOVERY_INTERVAL_NANOS; +import static datadog.communication.EvpProxy.ORIGIN_HEADER; +import static datadog.communication.EvpProxy.ORIGIN_VERSION_HEADER; +import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V2_EVP_PROXY_ENDPOINT; +import static datadog.communication.http.HttpRetryPolicy.Factory.NEVER_RETRY; +import static datadog.trace.api.intake.Intake.EVENT_PLATFORM; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Arrays.asList; +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.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyIterable; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import datadog.communication.BackendApi; import datadog.communication.BackendApiFactory; import datadog.communication.HttpResponseException; +import datadog.communication.ddagent.TracerVersion; import datadog.communication.http.HttpRetryPolicy; -import datadog.trace.api.intake.Intake; import java.io.IOException; import java.net.ConnectException; +import java.util.concurrent.atomic.AtomicLong; +import okhttp3.Response; import org.junit.jupiter.api.Test; import org.tabletest.junit.TableTest; class EventProxyTest { private static final byte[] PAYLOAD = "{}".getBytes(UTF_8); + private static final String ENDPOINT = "evp_proxy/v4/"; + + private final BackendApiFactory factory = mock(BackendApiFactory.class); + private final BackendApi exposures = mock(BackendApi.class); + private final BackendApi flagEvaluations = mock(BackendApi.class); + private final AtomicLong now = new AtomicLong(); + + @Test + void sendsRemoteConfigurationEventsToTheFixedV2EndpointWithPreSendRetry() throws IOException { + clientsFor(V2_EVP_PROXY_ENDPOINT); + final EventProxy proxy = proxy(false, false); + + assertTrue(proxy.isAvailable()); + assertTrue(proxy.post("exposures", PAYLOAD)); + assertTrue(proxy.post("flagevaluation", PAYLOAD)); + + verify(this.factory) + .createEvpProxyApiForEndpoint(EVENT_PLATFORM, true, PRE_SEND_RETRY, V2_EVP_PROXY_ENDPOINT); + verify(this.factory) + .createEvpProxyApiForEndpoint(EVENT_PLATFORM, false, PRE_SEND_RETRY, V2_EVP_PROXY_ENDPOINT); + verify(this.factory, never()).discoverEvpProxyEndpoint(anyBoolean(), anyIterable()); + verify(this.exposures).post(eq("exposures"), any(), any(), any(), eq(false)); + verify(this.flagEvaluations).post(eq("flagevaluation"), any(), any(), any(), eq(false)); + } + + @Test + void sendsAgentlessEventsOnceThroughTheDiscoveredEndpoint() throws IOException { + when(this.factory.discoverEvpProxyEndpoint(false, IDENTITY_HEADER_NAMES)).thenReturn(ENDPOINT); + clientsFor(ENDPOINT); + final EventProxy proxy = proxy(true, true); + + assertTrue(proxy.isAvailable()); + assertTrue(proxy.post("exposures", PAYLOAD)); + + verify(this.factory).createEvpProxyApiForEndpoint(EVENT_PLATFORM, true, NEVER_RETRY, ENDPOINT); + verify(this.factory).createEvpProxyApiForEndpoint(EVENT_PLATFORM, false, NEVER_RETRY, ENDPOINT); + verify(this.factory, times(1)).discoverEvpProxyEndpoint(anyBoolean(), anyIterable()); + } @Test - void isUnavailableWithoutAgentProxySupport() throws IOException { - final EventProxy proxy = new EventProxy(mock(BackendApiFactory.class), false); + void leavesAgentlessEventsToDirectIntakeWithoutTheAgentProxy() throws IOException { + final EventProxy proxy = proxy(true, true); assertFalse(proxy.isAvailable()); + this.now.addAndGet(RECOVERY_INTERVAL_NANOS); + assertFalse(proxy.post("exposures", PAYLOAD)); + + verify(this.factory, times(1)).discoverEvpProxyEndpoint(anyBoolean(), anyIterable()); + } + + @Test + void recoversTheAgentProxyWhenItIsTheOnlyRoute() throws IOException { + final EventProxy proxy = proxy(true, false); + + assertTrue(proxy.isAvailable(), "the proxy route stays in place to recover"); assertFalse(proxy.post("exposures", PAYLOAD)); + verify(this.factory, never()).discoverEvpProxyEndpoint(eq(true), anyIterable()); + + this.now.addAndGet(RECOVERY_INTERVAL_NANOS); + when(this.factory.discoverEvpProxyEndpoint(true, IDENTITY_HEADER_NAMES)).thenReturn(ENDPOINT); + clientsFor(ENDPOINT); + + assertTrue(proxy.post("exposures", PAYLOAD)); + verify(this.exposures).post(eq("exposures"), any(), any(), any(), eq(false)); } @Test - void postsEachRouteThroughItsOwnClient() throws IOException { - final BackendApiFactory factory = mock(BackendApiFactory.class); - final BackendApi exposures = mock(BackendApi.class); - final BackendApi flagEvaluations = mock(BackendApi.class); - when(factory.createEvpProxyApi(Intake.EVENT_PLATFORM, true)).thenReturn(exposures); - when(factory.createEvpProxyApi(Intake.EVENT_PLATFORM, false)).thenReturn(flagEvaluations); - final EventProxy proxy = new EventProxy(factory, false); + void failedDiscoveryRetriesAtMostOncePerRecoveryInterval() throws IOException { + when(this.factory.discoverEvpProxyEndpoint(false, IDENTITY_HEADER_NAMES)) + .thenThrow(new IllegalStateException("discovery failed")); + final EventProxy proxy = proxy(true, false); assertTrue(proxy.isAvailable()); + assertFalse(proxy.post("exposures", PAYLOAD)); + this.now.addAndGet(RECOVERY_INTERVAL_NANOS); + assertFalse(proxy.post("exposures", PAYLOAD)); + verify(this.factory, times(1)).discoverEvpProxyEndpoint(eq(true), anyIterable()); + + this.now.addAndGet(RECOVERY_INTERVAL_NANOS - 1); + assertFalse(proxy.post("exposures", PAYLOAD)); + verify(this.factory, times(1)).discoverEvpProxyEndpoint(eq(true), anyIterable()); + + this.now.incrementAndGet(); + when(this.factory.discoverEvpProxyEndpoint(true, IDENTITY_HEADER_NAMES)).thenReturn(ENDPOINT); + clientsFor(ENDPOINT); assertTrue(proxy.post("exposures", PAYLOAD)); - assertTrue(proxy.post("flagevaluation", PAYLOAD)); - verify(exposures).post(eq("exposures"), any(), any(), any(), eq(false)); - verify(flagEvaluations).post(eq("flagevaluation"), any(), any(), any(), eq(false)); + verify(this.factory, times(2)).discoverEvpProxyEndpoint(eq(true), anyIterable()); + verify(this.exposures).post(eq("exposures"), any(), any(), any(), eq(false)); } @Test - void doesNotRetryWhenDirectFallbackIsAvailable() { - final BackendApiFactory factory = mock(BackendApiFactory.class); - final EventProxy proxy = new EventProxy(factory, true); + void doesNotRediscoverAfterRejectionWhenDirectFallbackIsAvailable() throws IOException { + when(this.factory.discoverEvpProxyEndpoint(false, IDENTITY_HEADER_NAMES)).thenReturn(ENDPOINT); + clientsFor(ENDPOINT); + when(this.exposures.post(any(), any(), any(), any(), anyBoolean())) + .thenThrow(new ConnectException("refused")) + .thenReturn(null); + final EventProxy proxy = proxy(true, true); + + assertFalse(proxy.post("exposures", PAYLOAD)); + assertTrue(proxy.isAvailable()); + assertTrue(proxy.post("exposures", PAYLOAD)); - proxy.isAvailable(); + verify(this.factory, times(1)).discoverEvpProxyEndpoint(anyBoolean(), anyIterable()); + verify(this.exposures, times(2)).post(any(), any(), any(), any(), eq(false)); + } + + @Test + void propagatesAmbiguousTransportFailuresWithoutRetryOrRediscovery() throws IOException { + when(this.factory.discoverEvpProxyEndpoint(false, IDENTITY_HEADER_NAMES)).thenReturn(ENDPOINT); + clientsFor(ENDPOINT); + final IOException failure = new IOException("connection reset after sending"); + when(this.exposures.post(any(), any(), any(), any(), anyBoolean())).thenThrow(failure); + final EventProxy proxy = proxy(true, false); + + assertSame(failure, assertThrows(IOException.class, () -> proxy.post("exposures", PAYLOAD))); + assertTrue(proxy.isAvailable()); - verify(factory) - .createEvpProxyApi(Intake.EVENT_PLATFORM, true, HttpRetryPolicy.Factory.NEVER_RETRY); - verify(factory) - .createEvpProxyApi(Intake.EVENT_PLATFORM, false, HttpRetryPolicy.Factory.NEVER_RETRY); + verify(this.factory, times(1)).discoverEvpProxyEndpoint(anyBoolean(), anyIterable()); + verify(this.exposures).post(any(), any(), any(), any(), eq(false)); } @Test - void reportsConnectionFailuresAsUnavailable() throws IOException { - final BackendApi api = proxyApi(new ConnectException("refused")); + void waitsForANewEndpointAfterADefinitiveRejectionWhenItIsTheOnlyRoute() throws IOException { + when(this.factory.discoverEvpProxyEndpoint(false, IDENTITY_HEADER_NAMES)).thenReturn(ENDPOINT); + clientsFor(ENDPOINT); + when(this.exposures.post(any(), any(), any(), any(), anyBoolean())) + .thenThrow(new ConnectException("refused")); + final EventProxy proxy = proxy(true, false); + + assertFalse(proxy.post("exposures", PAYLOAD)); + assertFalse(proxy.post("exposures", PAYLOAD)); - assertFalse(new EventProxy(factoryOf(api), false).post("exposures", PAYLOAD)); + verify(this.exposures, times(1)).post(any(), any(), any(), any(), anyBoolean()); } @TableTest({ "Scenario | Status | Definitive", - "forbidden | 403 | true ", + "forbidden | 403 | false ", "not found | 404 | true ", "not allowed | 405 | true ", "server error | 500 | false ", "rate limited | 429 | false " }) void classifiesProxyRejections(final int status, final boolean definitive) throws IOException { - final BackendApi api = proxyApi(new HttpResponseException(status, "rejected")); - final EventProxy proxy = new EventProxy(factoryOf(api), false); + clientsFor(V2_EVP_PROXY_ENDPOINT); + when(this.exposures.post(any(), any(), any(), any(), anyBoolean())) + .thenThrow(new HttpResponseException(status, "rejected")); + final EventProxy proxy = proxy(false, false); if (definitive) { assertFalse(proxy.post("exposures", PAYLOAD)); @@ -88,15 +199,52 @@ void classifiesProxyRejections(final int status, final boolean definitive) throw } } - private static BackendApi proxyApi(final IOException failure) throws IOException { - final BackendApi api = mock(BackendApi.class); - when(api.post(any(), any(), any(), any(), anyBoolean())).thenThrow(failure); - return api; + @Test + void reportsConnectionRefusalAsDefinitive() throws IOException { + clientsFor(V2_EVP_PROXY_ENDPOINT); + when(this.exposures.post(any(), any(), any(), any(), anyBoolean())) + .thenThrow(new ConnectException("refused")); + + assertFalse(proxy(false, false).post("exposures", PAYLOAD)); + } + + @Test + void identifiesTheJavaTracingLibrary() { + assertEquals("dd-trace-java", IDENTITY_HEADERS.get(ORIGIN_HEADER)); + assertEquals(TracerVersion.TRACER_VERSION, IDENTITY_HEADERS.get(ORIGIN_VERSION_HEADER)); + assertNotEquals("0.0.0", IDENTITY_HEADERS.get(ORIGIN_VERSION_HEADER)); + } + + @Test + void discoversOnlyAgentlessEndpointsForwardingTheIdentity() { + proxy(true, true).isAvailable(); + + verify(this.factory) + .discoverEvpProxyEndpoint(false, asList(ORIGIN_HEADER, ORIGIN_VERSION_HEADER)); + } + + @Test + void preSendRetryRetriesConnectionRefusalOnly() { + final HttpRetryPolicy policy = PRE_SEND_RETRY.create(); + + assertFalse(policy.shouldRetry(new IOException("reset"))); + assertFalse(policy.shouldRetry((Response) null)); + assertTrue(policy.shouldRetry(new ConnectException("refused"))); + assertTrue(policy.shouldRetry(new ConnectException("refused"))); + assertTrue(policy.shouldRetry(new ConnectException("refused"))); + assertFalse(policy.shouldRetry(new ConnectException("refused")), "at most three retries"); + } + + private EventProxy proxy(final boolean agentless, final boolean directFallbackAvailable) { + return new EventProxy(this.factory, agentless, directFallbackAvailable, this.now::get); } - private static BackendApiFactory factoryOf(final BackendApi api) { - final BackendApiFactory factory = mock(BackendApiFactory.class); - when(factory.createEvpProxyApi(eq(Intake.EVENT_PLATFORM), anyBoolean())).thenReturn(api); - return factory; + private void clientsFor(final String endpoint) { + when(this.factory.createEvpProxyApiForEndpoint( + eq(EVENT_PLATFORM), eq(true), any(), eq(endpoint))) + .thenReturn(this.exposures); + when(this.factory.createEvpProxyApiForEndpoint( + eq(EVENT_PLATFORM), eq(false), any(), eq(endpoint))) + .thenReturn(this.flagEvaluations); } } diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagsBackendTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagsBackendTest.java index 0ba9c1a50e4..ed5c042adbb 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagsBackendTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagsBackendTest.java @@ -3,6 +3,7 @@ import static datadog.trace.api.config.FeatureFlaggingConfig.FEATURE_FLAGS_CONFIGURATION_SOURCE; import static datadog.trace.api.config.FeatureFlaggingConfig.FEATURE_FLAGS_ENABLED; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Arrays.asList; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -24,7 +25,10 @@ import datadog.trace.bootstrap.config.provider.ConfigProvider; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.test.junit.utils.config.WithConfigExtension; +import java.util.List; import java.util.Properties; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.tabletest.junit.TableTest; @@ -41,7 +45,7 @@ class FeatureFlagsBackendTest { private final CoreMetricCollector metrics = mock(CoreMetricCollector.class); private final FeatureFlagsBackend backend = new FeatureFlagsBackend( - this.sco, this.config, this.eventProxy, this.spanEnrichmentWriter, this.metrics); + this.sco, this.config, this.eventProxy, this.spanEnrichmentWriter, this.metrics, null); private static AgentSpan rootSpan() { final AgentSpan root = mock(AgentSpan.class); @@ -126,11 +130,48 @@ void createsTheBackendFromTheAgentConfiguration( when(this.config.getFeatureFlaggingConfigurationSource()).thenReturn(source); when(this.config.getApiKey()).thenReturn(apiKey); - final FeatureFlagsBackend created = new FeatureFlagsBackend(this.sco, this.config); + final FeatureFlagsBackend created = new FeatureFlagsBackend(this.sco, this.config, null); assertEquals(apiKey, created.setting("api-key")); } + @Test + void runsTheFirstShutdownSubscriptionActionOnce() { + final AtomicInteger firstSubscriptions = new AtomicInteger(); + final FeatureFlagsBackend backend = + new FeatureFlagsBackend( + this.sco, + this.config, + this.eventProxy, + this.spanEnrichmentWriter, + this.metrics, + firstSubscriptions::incrementAndGet); + + backend.subscribeShutdown(() -> {}); + backend.subscribeShutdown(() -> {}); + + assertEquals(1, firstSubscriptions.get()); + } + + @Test + void closeNotifiesTheShutdownListenersBeforeReleasingResources() throws Exception { + final List calls = new CopyOnWriteArrayList<>(); + this.backend.subscribeShutdown(() -> calls.add("first")); + this.backend.subscribeShutdown( + () -> { + throw new IllegalStateException("failing listener"); + }); + final AutoCloseable unregistered = + this.backend.subscribeShutdown(() -> calls.add("unregistered")); + this.backend.subscribeShutdown(() -> calls.add("last")); + unregistered.close(); + + this.backend.close(); + this.backend.close(); + + assertEquals(asList("first", "last"), calls); + } + @Test void closeWithoutRemoteConfigClearsSpanEnrichment() { this.backend.enrichSerialId(1, false, null); From 47e5c87a8494a4b8afb87c3a10c666c6c2ecbb0a Mon Sep 17 00:00:00 2001 From: Sarah Chen Date: Thu, 22 Oct 2026 14:56:02 -0500 Subject: [PATCH 02/16] fix(ffe): Guard OpenFeature agent bridge against incompatible SDK interfaces --- .../openfeature/JavaAgentBridge.java | 39 +++- .../DDOpenFeatureClassLoaderTest.java | 197 ++++++++++++++++++ ...ureInstrumentationNoTracingForkedTest.java | 6 + ...nFeatureIntegrationDisabledForkedTest.java | 26 +++ 4 files changed, 267 insertions(+), 1 deletion(-) create mode 100644 dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureClassLoaderTest.java create mode 100644 dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentationNoTracingForkedTest.java create mode 100644 dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureIntegrationDisabledForkedTest.java diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java index 4bb9b0f351b..9c5bbd47e79 100644 --- a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java +++ b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java @@ -1,5 +1,8 @@ package datadog.trace.instrumentation.openfeature; +import static java.lang.reflect.Modifier.isAbstract; +import static java.lang.reflect.Modifier.isStatic; + import com.datadog.openfeature.internal.bridge.v1.AgentBridge; import com.datadog.openfeature.internal.bridge.v1.ConfigurationSource; import com.datadog.openfeature.internal.bridge.v1.EventTransport; @@ -8,10 +11,21 @@ import com.datadog.openfeature.internal.bridge.v1.SpanEnricher; import datadog.trace.api.InstrumenterConfig; import datadog.trace.api.featureflag.FeatureFlaggingGateway; +import java.lang.reflect.Method; import javax.annotation.Nullable; /** The bridge backed by the agent Feature Flags backend. */ public final class JavaAgentBridge implements AgentBridge { + // Muzzle checks calls, but not the SPI methods implemented by these helpers. + private static final boolean SPI_COMPATIBLE = + implementsSpi(AgentBridge.class, JavaAgentBridge.class) + && implementsSpi(ConfigurationSource.class, AgentConfigurationSource.class) + && implementsSpi(EventTransport.class, AgentEventProxy.class) + && implementsSpi(HealthMetrics.class, AgentHealthMetrics.class) + && implementsSpi(SpanEnricher.class, AgentSpanEnricher.class) + && implementsSpi(ShutdownSource.class, AgentShutdownSource.class) + && implementsSpi(AgentBridge.class, DisabledAgentBridge.class); + private final FeatureFlaggingGateway.Backend backend; private final HealthMetrics healthMetrics; private final SpanEnricher spanEnricher; @@ -25,11 +39,15 @@ public final class JavaAgentBridge implements AgentBridge { /** * Connects the SDK to the agent backend. * - * @param fallback the bridge to use if the agent backend is not started. + * @param fallback the bridge to use if the agent backend is not started or the SDK SPI is + * incompatible. * @return the agent bridge, the disabled one if the agent configuration disables Feature Flags, * or the fallback one. */ public static AgentBridge connect(final AgentBridge fallback) { + if (!SPI_COMPATIBLE) { + return fallback; + } if (!InstrumenterConfig.get().isFeatureFlaggingEnabled()) { return DisabledAgentBridge.INSTANCE; } @@ -37,6 +55,25 @@ public static AgentBridge connect(final AgentBridge fallback) { return backend == null ? fallback : new JavaAgentBridge(backend); } + private static boolean implementsSpi(final Class spi, final Class implementation) { + try { + for (final Method method : spi.getMethods()) { + if (!isStatic(method.getModifiers())) { + final Method implemented = + implementation.getMethod(method.getName(), method.getParameterTypes()); + if (isAbstract(implemented.getModifiers()) + || isStatic(implemented.getModifiers()) + || implemented.getReturnType() != method.getReturnType()) { + return false; + } + } + } + return true; + } catch (final NoSuchMethodException e) { + return false; + } + } + @Nullable @Override public String setting(final String key) { diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureClassLoaderTest.java b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureClassLoaderTest.java new file mode 100644 index 00000000000..2c9999641c2 --- /dev/null +++ b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureClassLoaderTest.java @@ -0,0 +1,197 @@ +package datadog.trace.instrumentation.openfeature; + +import static java.nio.file.Files.createDirectories; +import static java.nio.file.Files.write; +import static net.bytebuddy.jar.asm.Opcodes.ACC_ABSTRACT; +import static net.bytebuddy.jar.asm.Opcodes.ACC_PUBLIC; +import static net.bytebuddy.jar.asm.Opcodes.ACONST_NULL; +import static net.bytebuddy.jar.asm.Opcodes.ARETURN; +import static net.bytebuddy.jar.asm.Opcodes.ASM9; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.datadog.openfeature.internal.bridge.v1.AgentBridge; +import datadog.trace.agent.test.AbstractInstrumentationTest; +import datadog.trace.api.featureflag.FeatureFlaggingGateway; +import java.io.IOException; +import java.io.InputStream; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.Path; +import java.util.function.UnaryOperator; +import net.bytebuddy.jar.asm.ClassReader; +import net.bytebuddy.jar.asm.ClassVisitor; +import net.bytebuddy.jar.asm.ClassWriter; +import net.bytebuddy.jar.asm.MethodVisitor; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.tabletest.junit.TableTest; + +class DDOpenFeatureClassLoaderTest extends AbstractInstrumentationTest { + @TempDir Path sdkOverrides; + + @AfterEach + void unregisterBackend() { + FeatureFlaggingGateway.register(null); + } + + @Test + void injectsIntoAnIsolatedSdkClassLoader() throws Exception { + final FeatureFlaggingGateway.Backend backend = mock(FeatureFlaggingGateway.Backend.class); + when(backend.setting("site")).thenReturn("datadoghq.eu"); + FeatureFlaggingGateway.register(backend); + + try (URLClassLoader loader = sdkClassLoader()) { + final Object bridge = detect(loader); + assertSame(loader, bridge.getClass().getClassLoader()); + assertEquals(JavaAgentBridge.class.getName(), bridge.getClass().getName()); + assertEquals( + "datadoghq.eu", + loader + .loadClass(AgentBridge.class.getName()) + .getMethod("setting", String.class) + .invoke(bridge, "site")); + } + } + + @TableTest({ + "Scenario | SPI | Method | Return type ", + "settings changed | AgentBridge | setting | ", + "configuration changed | ConfigurationSource | subscribe | ", + "transport changed | EventTransport | post | ", + "metrics changed | HealthMetrics | count | ", + "enrichment changed | SpanEnricher | serialId | ", + "shutdown changed | ShutdownSource | subscribe | ", + "return type changed | AgentBridge | setting | Ljava/lang/Object;" + }) + void keepsTheStandaloneBridgeWhenAnSpiMethodChanges( + final String spi, final String method, final String returnType) throws Exception { + rewriteSpi( + spi, + bytes -> { + final ClassWriter writer = new ClassWriter(0); + new ClassReader(bytes) + .accept( + new ClassVisitor(ASM9, writer) { + @Override + public MethodVisitor visitMethod( + int access, + String name, + String descriptor, + String signature, + String[] exceptions) { + if (name.equals(method)) { + if (returnType == null) { + name = "renamed"; + } else { + descriptor = + descriptor.substring(0, descriptor.indexOf(')') + 1) + returnType; + } + } + return super.visitMethod(access, name, descriptor, signature, exceptions); + } + }, + 0); + return writer.toByteArray(); + }); + FeatureFlaggingGateway.register(mock(FeatureFlaggingGateway.Backend.class)); + + try (URLClassLoader loader = sdkClassLoader()) { + // Implemented methods are not calls, so Muzzle alone accepts this incompatible SPI. + assertTrue(new DDOpenFeatureInstrumentation().getInstrumentationMuzzle().matches(loader)); + assertSame( + loader.loadClass(AgentBridge.class.getName()).getField("NONE").get(null), detect(loader)); + } + } + + @TableTest({ + "Scenario | Abstract | Static conflict", + "new abstract method | true | false ", + "new default method | false | false ", + "abstract conflicts with static | true | true ", + "default conflicts with static | false | true " + }) + void checksNewSpiMethods(final boolean isAbstract, final boolean staticConflict) + throws Exception { + rewriteSpi( + "AgentBridge", + bytes -> { + final ClassWriter writer = new ClassWriter(0); + new ClassReader(bytes) + .accept( + new ClassVisitor(ASM9, writer) { + @Override + public void visitEnd() { + final MethodVisitor method = + super.visitMethod( + ACC_PUBLIC | (isAbstract ? ACC_ABSTRACT : 0), + staticConflict ? "connect" : "additionalMethod", + staticConflict + ? "(Lcom/datadog/openfeature/internal/bridge/v1/AgentBridge;)" + + "Lcom/datadog/openfeature/internal/bridge/v1/AgentBridge;" + : "()Ljava/lang/String;", + null, + null); + if (!isAbstract) { + method.visitCode(); + if (staticConflict) { + method.visitInsn(ACONST_NULL); + } else { + method.visitLdcInsn("compatible"); + } + method.visitInsn(ARETURN); + method.visitMaxs(1, staticConflict ? 2 : 1); + } + method.visitEnd(); + super.visitEnd(); + } + }, + 0); + return writer.toByteArray(); + }); + FeatureFlaggingGateway.register(mock(FeatureFlaggingGateway.Backend.class)); + + try (URLClassLoader loader = sdkClassLoader()) { + final Class contract = loader.loadClass(AgentBridge.class.getName()); + final Object bridge = detect(loader); + if (isAbstract || staticConflict) { + assertSame(contract.getField("NONE").get(null), bridge); + } else { + assertEquals(JavaAgentBridge.class.getName(), bridge.getClass().getName()); + assertEquals("compatible", contract.getMethod("additionalMethod").invoke(bridge)); + } + } + } + + private URLClassLoader sdkClassLoader() throws IOException { + return new URLClassLoader( + new URL[] { + sdkOverrides.toUri().toURL(), + AgentBridge.class.getProtectionDomain().getCodeSource().getLocation() + }, + null); + } + + private static Object detect(final ClassLoader loader) throws Exception { + return loader + .loadClass("com.datadog.openfeature.internal.bridge.v1.AgentBridges") + .getMethod("detect") + .invoke(null); + } + + private void rewriteSpi(final String spi, final UnaryOperator rewrite) + throws IOException { + final String resource = "com/datadog/openfeature/internal/bridge/v1/" + spi + ".class"; + final Path target = sdkOverrides.resolve(resource); + createDirectories(target.getParent()); + try (InputStream input = AgentBridge.class.getClassLoader().getResourceAsStream(resource)) { + assertNotNull(input); + write(target, rewrite.apply(input.readAllBytes())); + } + } +} diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentationNoTracingForkedTest.java b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentationNoTracingForkedTest.java new file mode 100644 index 00000000000..31485765b7d --- /dev/null +++ b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureInstrumentationNoTracingForkedTest.java @@ -0,0 +1,6 @@ +package datadog.trace.instrumentation.openfeature; + +import datadog.trace.test.junit.utils.config.WithConfig; + +@WithConfig(key = "trace.enabled", value = "false") +class DDOpenFeatureInstrumentationNoTracingForkedTest extends DDOpenFeatureInstrumentationTest {} diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureIntegrationDisabledForkedTest.java b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureIntegrationDisabledForkedTest.java new file mode 100644 index 00000000000..d7a52e5a6c1 --- /dev/null +++ b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureIntegrationDisabledForkedTest.java @@ -0,0 +1,26 @@ +package datadog.trace.instrumentation.openfeature; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.mock; + +import com.datadog.openfeature.internal.bridge.v1.AgentBridge; +import com.datadog.openfeature.internal.bridge.v1.AgentBridges; +import datadog.trace.agent.test.AbstractInstrumentationTest; +import datadog.trace.api.featureflag.FeatureFlaggingGateway; +import datadog.trace.test.junit.utils.config.WithConfig; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +@WithConfig(key = "trace.openfeature.enabled", value = "false") +class DDOpenFeatureIntegrationDisabledForkedTest extends AbstractInstrumentationTest { + @AfterEach + void unregisterBackend() { + FeatureFlaggingGateway.register(null); + } + + @Test + void doesNotInstrumentWhenTheIntegrationIsDisabled() { + FeatureFlaggingGateway.register(mock(FeatureFlaggingGateway.Backend.class)); + assertSame(AgentBridge.NONE, AgentBridges.detect()); + } +} From 14909def3c446be55cdc9caa6b9f21b16fb916c1 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Fri, 23 Oct 2026 16:20:17 +0200 Subject: [PATCH 03/16] fix(ffe): Report rejected Remote Configuration flag documents as apply errors The SDK dropped malformed Remote Configuration documents and the agent swallowed subscriber failures, so the poller acknowledged documents the SDK never applied. The SDK now rejects malformed documents by throwing, and the agent rethrows the first subscriber failure once every subscriber is notified, so the poller reports an apply error as before the switch. Co-Authored-By: Claude Opus 5.5 --- .../bridge/v1/ConfigurationSource.java | 3 +- .../delivery/RemoteConfigurationService.java | 4 +- .../internal/FeatureFlagsRuntimeTest.java | 4 +- .../delivery/DeliveryDiagnosticsTest.java | 4 +- .../RemoteConfigurationServiceTest.java | 77 +++++++++++++++++++ .../featureflag/FeatureFlaggingGateway.java | 3 +- .../featureflag/RemoteConfigServiceImpl.java | 26 ++++++- .../RemoteConfigServiceImplTest.java | 51 ++++++++++++ 8 files changed, 165 insertions(+), 7 deletions(-) create mode 100644 dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/RemoteConfigurationServiceTest.java diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/bridge/v1/ConfigurationSource.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/bridge/v1/ConfigurationSource.java index 5cda6a93009..79505f916c1 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/bridge/v1/ConfigurationSource.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/bridge/v1/ConfigurationSource.java @@ -8,7 +8,8 @@ public interface ConfigurationSource { * Subscribes to configuration updates. * * @param listener the listener receiving raw UFC JSON documents, or {@code null} when the - * configuration is removed. + * configuration is removed. It throws a {@link RuntimeException} to reject a malformed + * document, which the source reports as not applied. * @return the handle to close to unsubscribe. */ AutoCloseable subscribe(Consumer listener); diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/RemoteConfigurationService.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/RemoteConfigurationService.java index f0179ab3b81..ec63935162a 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/RemoteConfigurationService.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/delivery/RemoteConfigurationService.java @@ -41,7 +41,9 @@ public synchronized void start(final Consumer listener) { "Remote Configuration returned a malformed flag configuration ({}); retaining the current configuration", e.getClass().getSimpleName(), e); - return; + // Rejects the document so the agent reports it as not applied. + throw new IllegalArgumentException( + "Malformed flag configuration: " + e.getMessage(), e); } if (configuration != null) { listener.accept(configuration); diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/FeatureFlagsRuntimeTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/FeatureFlagsRuntimeTest.java index f6924b3b4a9..7821bfa07ef 100644 --- a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/FeatureFlagsRuntimeTest.java +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/FeatureFlagsRuntimeTest.java @@ -129,7 +129,9 @@ void remoteConfigurationDeliversParsedConfigurationsAndReplaysToLateListeners() runtime.start(); try { source.listener.accept(EMPTY_UFC); - source.listener.accept("{not json".getBytes(StandardCharsets.UTF_8)); + assertThrows( + IllegalArgumentException.class, + () -> source.listener.accept("{not json".getBytes(StandardCharsets.UTF_8))); final List late = new CopyOnWriteArrayList<>(); runtime.addConfigurationListener(late::add); diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/DeliveryDiagnosticsTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/DeliveryDiagnosticsTest.java index d8e8e46abf3..440810e6077 100644 --- a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/DeliveryDiagnosticsTest.java +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/DeliveryDiagnosticsTest.java @@ -173,7 +173,9 @@ void remoteConfigurationReportsItsSubscriptionAndMalformedDocuments() { service.start(configuration -> {}); connector.listener.accept("{\"environment\":{\"name\":\"Test\"},\"flags\":{}}".getBytes(UTF_8)); - connector.listener.accept("{not json".getBytes(UTF_8)); + assertThrows( + IllegalArgumentException.class, + () -> connector.listener.accept("{not json".getBytes(UTF_8))); connector.listener.accept(null); assertLogged("subscribing to Remote Configuration"); diff --git a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/RemoteConfigurationServiceTest.java b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/RemoteConfigurationServiceTest.java new file mode 100644 index 00000000000..8706b279348 --- /dev/null +++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/delivery/RemoteConfigurationServiceTest.java @@ -0,0 +1,77 @@ +package com.datadog.openfeature.internal.delivery; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +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 com.datadog.openfeature.internal.bridge.v1.ConfigurationSource; +import com.datadog.openfeature.internal.ufc.ServerConfiguration; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Consumer; +import org.junit.jupiter.api.Test; + +class RemoteConfigurationServiceTest { + @Test + void forwardsParsedConfigurationsAndRemovals() { + final CapturingSource source = new CapturingSource(); + final List received = new ArrayList<>(); + final RemoteConfigurationService service = new RemoteConfigurationService(source); + service.start(received::add); + + source.listener.accept("{\"flags\":{}}".getBytes(UTF_8)); + source.listener.accept(null); + + assertEquals(2, received.size()); + assertNotNull(received.get(0)); + assertNull(received.get(1)); + } + + @Test + void rejectsMalformedDocuments() { + final CapturingSource source = new CapturingSource(); + final List received = new ArrayList<>(); + final RemoteConfigurationService service = new RemoteConfigurationService(source); + service.start(received::add); + + final IllegalArgumentException rejection = + assertThrows( + IllegalArgumentException.class, + () -> source.listener.accept("{\"flags\":[]}".getBytes(UTF_8))); + + assertTrue(rejection.getCause() instanceof IOException); + assertTrue(rejection.getMessage().startsWith("Malformed flag configuration: ")); + assertEquals(0, received.size()); + } + + @Test + void closeUnsubscribesOnce() { + final CapturingSource source = new CapturingSource(); + final RemoteConfigurationService service = new RemoteConfigurationService(source); + service.start(configuration -> {}); + service.start(configuration -> {}); + + service.close(); + service.close(); + + assertEquals(1, source.subscriptions); + assertEquals(1, source.closes); + } + + private static final class CapturingSource implements ConfigurationSource { + private Consumer listener; + private int subscriptions; + private int closes; + + @Override + public AutoCloseable subscribe(final Consumer listener) { + this.listener = listener; + this.subscriptions++; + return () -> this.closes++; + } + } +} diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java index a20b965a96c..e5b4760c1b2 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java @@ -52,7 +52,8 @@ public interface Backend { * Subscribes to the Feature Flags Remote Configuration product. * * @param listener the listener receiving raw configuration documents, or {@code null} when the - * configuration is removed. + * configuration is removed. It throws a {@link RuntimeException} to reject a malformed + * document, reported to Remote Configuration as an apply error. * @return the handle to close to unsubscribe. */ AutoCloseable subscribeRemoteConfig(Consumer listener); diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java index a0aae50ed9a..bbae91bf8ff 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java @@ -59,7 +59,12 @@ public AutoCloseable subscribe(final Consumer subscriber) { this.subscribers.add(subscriber); final byte[] document = this.current; if (document != null) { - subscriber.accept(document); + try { + subscriber.accept(document); + } catch (final RuntimeException e) { + // Already reported to Remote Configuration when the document was received. + LOGGER.debug("Feature Flags configuration subscriber rejected the current document", e); + } } } return () -> this.subscribers.remove(subscriber); @@ -78,6 +83,15 @@ public synchronized void close() { this.current = null; } + /** + * Forwards a configuration document to every subscriber. + * + * @param configKey the Remote Configuration key. + * @param content the raw document, or {@code null} when the configuration is removed. + * @param pollingRateHinter the polling rate hinter. + * @throws RuntimeException the first subscriber failure, once every subscriber is notified, so + * the poller reports the document as not applied. + */ @Override public void accept( final String configKey, @@ -88,12 +102,17 @@ public void accept( this.current = content; snapshot = this.subscribers.iterator(); } + RuntimeException failure = null; while (snapshot.hasNext()) { final Consumer subscriber = snapshot.next(); try { subscriber.accept(content); } catch (final RuntimeException e) { - LOGGER.debug("Feature Flags configuration subscriber failed", e); + if (failure == null) { + failure = e; + } else { + failure.addSuppressed(e); + } } } // Logged last: a diagnostic must not get in the way of the delivery. @@ -105,5 +124,8 @@ public void accept( configKey, content.length); } + if (failure != null) { + throw failure; + } } } 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 ec70db6d9f8..51c3683aa73 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 @@ -7,6 +7,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -79,6 +80,56 @@ void forwardsDocumentsAndRemovalsToAllSubscribers() { assertEquals(2, second.size()); } + @Test + void reportsRejectedDocumentsOnceEverySubscriberIsNotified() { + final RemoteConfigServiceImpl service = + new RemoteConfigServiceImpl(mock(ConfigurationPoller.class)); + final IllegalArgumentException rejection = new IllegalArgumentException("malformed"); + final IllegalStateException otherRejection = new IllegalStateException("malformed too"); + final List received = new ArrayList<>(); + service.subscribe( + content -> { + throw rejection; + }); + service.subscribe(received::add); + service.subscribe( + content -> { + throw otherRejection; + }); + + final IllegalArgumentException reported = + assertThrows( + IllegalArgumentException.class, + () -> service.accept("key", DOCUMENT, mock(PollingRateHinter.class))); + + assertSame(rejection, reported); + assertArrayEquals(new Throwable[] {otherRejection}, reported.getSuppressed()); + assertEquals(1, received.size()); + assertArrayEquals(DOCUMENT, received.get(0)); + } + + @Test + void lateSubscribersRejectingTheCurrentDocumentStaySubscribed() throws Exception { + final RemoteConfigServiceImpl service = + new RemoteConfigServiceImpl(mock(ConfigurationPoller.class)); + service.accept("key", DOCUMENT, mock(PollingRateHinter.class)); + final List received = new ArrayList<>(); + + final AutoCloseable subscription = + service.subscribe( + content -> { + if (content != null) { + throw new IllegalArgumentException("malformed"); + } + received.add(null); + }); + service.accept("key", null, mock(PollingRateHinter.class)); + subscription.close(); + + assertEquals(1, received.size()); + assertNull(received.get(0)); + } + @Test void replaysTheCurrentDocumentToLateSubscribers() { final RemoteConfigServiceImpl service = From 18b770416dcbdd25b84025708368f67c3efb54fa Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Mon, 26 Oct 2026 10:53:58 +0100 Subject: [PATCH 04/16] fix(ffe): Release the Remote Configuration poller when closing Feature Flags The poller reference-counts start and stop calls, but closing the Feature Flags Remote Configuration service no longer stopped it, keeping it polling once Feature Flags closed and leaking a reference on each restart. Co-Authored-By: Claude Opus 5.5 --- .../featureflag/RemoteConfigServiceImpl.java | 6 +++++- .../featureflag/RemoteConfigServiceImplTest.java | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java index bbae91bf8ff..acbc89e33cd 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java @@ -70,7 +70,10 @@ public AutoCloseable subscribe(final Consumer subscriber) { return () -> this.subscribers.remove(subscriber); } - /** Unregisters the Feature Flags product and drops the subscribers. */ + /** + * Unregisters the Feature Flags product, releases the poller started by {@link #start()} and + * drops the subscribers. + */ public synchronized void close() { if (!this.started) { return; @@ -79,6 +82,7 @@ public synchronized void close() { LOGGER.debug("Feature Flagging remote_config configuration source stopped"); this.configurationPoller.removeCapabilities(CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); this.configurationPoller.removeListeners(FFE_FLAGS); + this.configurationPoller.stop(); this.subscribers.clear(); this.current = null; } 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 51c3683aa73..4871d2c98a6 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 @@ -236,10 +236,24 @@ void closeUnregistersTheProductAndDropsSubscribers() { verify(poller, times(1)) .removeCapabilities(Capabilities.CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); verify(poller, times(1)).removeListeners(Product.FFE_FLAGS); - verify(poller, never()).stop(); + verify(poller, times(1)).stop(); assertEquals(0, received.size()); } + @Test + void restartsBalancePollerReferences() { + final ConfigurationPoller poller = mock(ConfigurationPoller.class); + final RemoteConfigServiceImpl service = new RemoteConfigServiceImpl(poller); + + service.start(); + service.close(); + service.subscribe(content -> {}); + service.close(); + + verify(poller, times(2)).start(); + verify(poller, times(2)).stop(); + } + @Nested class Diagnostics { private final Logger logger = (Logger) LoggerFactory.getLogger(RemoteConfigServiceImpl.class); From b8030d10bbef5802a112d77a1dd88d8e563d1ce0 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Tue, 27 Oct 2026 10:49:28 +0100 Subject: [PATCH 05/16] fix(ffe): Flush Feature Flags at exit when the shutdown instrumentation is disabled The Feature Flags shutdown hook was only registered when tracing was disabled, but the tracing shutdown path also relies on the shutdown instrumentation, which can be disabled on its own. The hook is now always registered once the SDK subscribes to the shutdown; stopping is idempotent, so it does nothing after the tracing shutdown path already stopped Feature Flags. Co-Authored-By: Claude Opus 5.5 --- .../com/datadog/featureflag/FeatureFlaggingSystem.java | 9 ++++----- .../datadog/featureflag/FeatureFlaggingSystemTest.java | 5 +++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/products/feature-flagging/feature-flagging-agent/src/main/java/com/datadog/featureflag/FeatureFlaggingSystem.java b/products/feature-flagging/feature-flagging-agent/src/main/java/com/datadog/featureflag/FeatureFlaggingSystem.java index 63590036b16..7df0ec92c7b 100644 --- a/products/feature-flagging/feature-flagging-agent/src/main/java/com/datadog/featureflag/FeatureFlaggingSystem.java +++ b/products/feature-flagging/feature-flagging-agent/src/main/java/com/datadog/featureflag/FeatureFlaggingSystem.java @@ -38,12 +38,11 @@ public static synchronized void start(final SharedCommunicationObjects sco) { return; } LOGGER.debug("Feature Flagging enabled; env={}", config.getEnv()); - // Tracing owns the shutdown path running before the JVM shutdown hooks. Without it, stop from a - // shutdown hook once the SDK subscribes to the shutdown, so it still flushes its pending - // events. + // The tracing shutdown path stops the system before the JVM shutdown hooks, but it is missing + // when tracing or its shutdown instrumentation is disabled. Stop from a shutdown hook as well + // once the SDK subscribes to the shutdown, so it always flushes its pending events. final FeatureFlagsBackend backend = - new FeatureFlagsBackend( - sco, config, config.isTraceEnabled() ? null : FeatureFlaggingSystem::addShutdownHook); + new FeatureFlagsBackend(sco, config, FeatureFlaggingSystem::addShutdownHook); try { backend.start(); } catch (final RuntimeException | Error e) { diff --git a/products/feature-flagging/feature-flagging-agent/src/test/java/com/datadog/featureflag/FeatureFlaggingSystemTest.java b/products/feature-flagging/feature-flagging-agent/src/test/java/com/datadog/featureflag/FeatureFlaggingSystemTest.java index c2a265544d1..6bffeb1101e 100644 --- a/products/feature-flagging/feature-flagging-agent/src/test/java/com/datadog/featureflag/FeatureFlaggingSystemTest.java +++ b/products/feature-flagging/feature-flagging-agent/src/test/java/com/datadog/featureflag/FeatureFlaggingSystemTest.java @@ -118,10 +118,11 @@ void lateShutdownSubscriptionDoesNotRegisterAHookAfterStop() throws Exception { } @Test - void leavesTheShutdownToTracingWhenEnabled() { + void keepsAShutdownHookWhenTracingIsEnabled() { + // The tracing shutdown instrumentation can be disabled on its own. FeatureFlaggingSystem.start(mock(SharedCommunicationObjects.class)); FeatureFlaggingGateway.backend().subscribeShutdown(() -> {}); - assertFalse(FeatureFlaggingSystem.isShutdownHookRegistered()); + assertTrue(FeatureFlaggingSystem.isShutdownHookRegistered()); } @Test From 23554d969ccb164ac16337d62aad2612413e5c0e Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Tue, 27 Oct 2026 11:47:20 +0100 Subject: [PATCH 06/16] test(ffe): Drop the agent bridge class loader test The test rewrote the SPI classes to check the runtime SPI guard. The japicmp bridge SPI check covers SPI compatibility once the SPI is released. Co-Authored-By: Claude Opus 5.5 --- .../DDOpenFeatureClassLoaderTest.java | 197 ------------------ 1 file changed, 197 deletions(-) delete mode 100644 dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureClassLoaderTest.java diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureClassLoaderTest.java b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureClassLoaderTest.java deleted file mode 100644 index 2c9999641c2..00000000000 --- a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/test/java/datadog/trace/instrumentation/openfeature/DDOpenFeatureClassLoaderTest.java +++ /dev/null @@ -1,197 +0,0 @@ -package datadog.trace.instrumentation.openfeature; - -import static java.nio.file.Files.createDirectories; -import static java.nio.file.Files.write; -import static net.bytebuddy.jar.asm.Opcodes.ACC_ABSTRACT; -import static net.bytebuddy.jar.asm.Opcodes.ACC_PUBLIC; -import static net.bytebuddy.jar.asm.Opcodes.ACONST_NULL; -import static net.bytebuddy.jar.asm.Opcodes.ARETURN; -import static net.bytebuddy.jar.asm.Opcodes.ASM9; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import com.datadog.openfeature.internal.bridge.v1.AgentBridge; -import datadog.trace.agent.test.AbstractInstrumentationTest; -import datadog.trace.api.featureflag.FeatureFlaggingGateway; -import java.io.IOException; -import java.io.InputStream; -import java.net.URL; -import java.net.URLClassLoader; -import java.nio.file.Path; -import java.util.function.UnaryOperator; -import net.bytebuddy.jar.asm.ClassReader; -import net.bytebuddy.jar.asm.ClassVisitor; -import net.bytebuddy.jar.asm.ClassWriter; -import net.bytebuddy.jar.asm.MethodVisitor; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; -import org.tabletest.junit.TableTest; - -class DDOpenFeatureClassLoaderTest extends AbstractInstrumentationTest { - @TempDir Path sdkOverrides; - - @AfterEach - void unregisterBackend() { - FeatureFlaggingGateway.register(null); - } - - @Test - void injectsIntoAnIsolatedSdkClassLoader() throws Exception { - final FeatureFlaggingGateway.Backend backend = mock(FeatureFlaggingGateway.Backend.class); - when(backend.setting("site")).thenReturn("datadoghq.eu"); - FeatureFlaggingGateway.register(backend); - - try (URLClassLoader loader = sdkClassLoader()) { - final Object bridge = detect(loader); - assertSame(loader, bridge.getClass().getClassLoader()); - assertEquals(JavaAgentBridge.class.getName(), bridge.getClass().getName()); - assertEquals( - "datadoghq.eu", - loader - .loadClass(AgentBridge.class.getName()) - .getMethod("setting", String.class) - .invoke(bridge, "site")); - } - } - - @TableTest({ - "Scenario | SPI | Method | Return type ", - "settings changed | AgentBridge | setting | ", - "configuration changed | ConfigurationSource | subscribe | ", - "transport changed | EventTransport | post | ", - "metrics changed | HealthMetrics | count | ", - "enrichment changed | SpanEnricher | serialId | ", - "shutdown changed | ShutdownSource | subscribe | ", - "return type changed | AgentBridge | setting | Ljava/lang/Object;" - }) - void keepsTheStandaloneBridgeWhenAnSpiMethodChanges( - final String spi, final String method, final String returnType) throws Exception { - rewriteSpi( - spi, - bytes -> { - final ClassWriter writer = new ClassWriter(0); - new ClassReader(bytes) - .accept( - new ClassVisitor(ASM9, writer) { - @Override - public MethodVisitor visitMethod( - int access, - String name, - String descriptor, - String signature, - String[] exceptions) { - if (name.equals(method)) { - if (returnType == null) { - name = "renamed"; - } else { - descriptor = - descriptor.substring(0, descriptor.indexOf(')') + 1) + returnType; - } - } - return super.visitMethod(access, name, descriptor, signature, exceptions); - } - }, - 0); - return writer.toByteArray(); - }); - FeatureFlaggingGateway.register(mock(FeatureFlaggingGateway.Backend.class)); - - try (URLClassLoader loader = sdkClassLoader()) { - // Implemented methods are not calls, so Muzzle alone accepts this incompatible SPI. - assertTrue(new DDOpenFeatureInstrumentation().getInstrumentationMuzzle().matches(loader)); - assertSame( - loader.loadClass(AgentBridge.class.getName()).getField("NONE").get(null), detect(loader)); - } - } - - @TableTest({ - "Scenario | Abstract | Static conflict", - "new abstract method | true | false ", - "new default method | false | false ", - "abstract conflicts with static | true | true ", - "default conflicts with static | false | true " - }) - void checksNewSpiMethods(final boolean isAbstract, final boolean staticConflict) - throws Exception { - rewriteSpi( - "AgentBridge", - bytes -> { - final ClassWriter writer = new ClassWriter(0); - new ClassReader(bytes) - .accept( - new ClassVisitor(ASM9, writer) { - @Override - public void visitEnd() { - final MethodVisitor method = - super.visitMethod( - ACC_PUBLIC | (isAbstract ? ACC_ABSTRACT : 0), - staticConflict ? "connect" : "additionalMethod", - staticConflict - ? "(Lcom/datadog/openfeature/internal/bridge/v1/AgentBridge;)" - + "Lcom/datadog/openfeature/internal/bridge/v1/AgentBridge;" - : "()Ljava/lang/String;", - null, - null); - if (!isAbstract) { - method.visitCode(); - if (staticConflict) { - method.visitInsn(ACONST_NULL); - } else { - method.visitLdcInsn("compatible"); - } - method.visitInsn(ARETURN); - method.visitMaxs(1, staticConflict ? 2 : 1); - } - method.visitEnd(); - super.visitEnd(); - } - }, - 0); - return writer.toByteArray(); - }); - FeatureFlaggingGateway.register(mock(FeatureFlaggingGateway.Backend.class)); - - try (URLClassLoader loader = sdkClassLoader()) { - final Class contract = loader.loadClass(AgentBridge.class.getName()); - final Object bridge = detect(loader); - if (isAbstract || staticConflict) { - assertSame(contract.getField("NONE").get(null), bridge); - } else { - assertEquals(JavaAgentBridge.class.getName(), bridge.getClass().getName()); - assertEquals("compatible", contract.getMethod("additionalMethod").invoke(bridge)); - } - } - } - - private URLClassLoader sdkClassLoader() throws IOException { - return new URLClassLoader( - new URL[] { - sdkOverrides.toUri().toURL(), - AgentBridge.class.getProtectionDomain().getCodeSource().getLocation() - }, - null); - } - - private static Object detect(final ClassLoader loader) throws Exception { - return loader - .loadClass("com.datadog.openfeature.internal.bridge.v1.AgentBridges") - .getMethod("detect") - .invoke(null); - } - - private void rewriteSpi(final String spi, final UnaryOperator rewrite) - throws IOException { - final String resource = "com/datadog/openfeature/internal/bridge/v1/" + spi + ".class"; - final Path target = sdkOverrides.resolve(resource); - createDirectories(target.getParent()); - try (InputStream input = AgentBridge.class.getClassLoader().getResourceAsStream(resource)) { - assertNotNull(input); - write(target, rewrite.apply(input.readAllBytes())); - } - } -} From 652cd42040a97063f51ce54d2a181559c2408b13 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Tue, 27 Oct 2026 11:47:22 +0100 Subject: [PATCH 07/16] chore(ffe): Mark the runtime bridge SPI check for removal after 1.69.0 Once dd-openfeature 1.69.0 is released, the japicmpBridgeV1 task checks the frozen bridge.v1 SPI, so the runtime check of the helpers becomes redundant. Co-Authored-By: Claude Opus 5.5 --- .../trace/instrumentation/openfeature/JavaAgentBridge.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java index 9c5bbd47e79..eab5e524928 100644 --- a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java +++ b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/JavaAgentBridge.java @@ -17,6 +17,8 @@ /** The bridge backed by the agent Feature Flags backend. */ public final class JavaAgentBridge implements AgentBridge { // Muzzle checks calls, but not the SPI methods implemented by these helpers. + // TODO APMLP-1512 Drop this check once dd-openfeature 1.69.0 is released: the japicmpBridgeV1 + // task then keeps the frozen bridge.v1 SPI compatible with the helpers implementing it. private static final boolean SPI_COMPATIBLE = implementsSpi(AgentBridge.class, JavaAgentBridge.class) && implementsSpi(ConfigurationSource.class, AgentConfigurationSource.class) From 6b1ea1f1ec21629bf6c7de856e5301acf24549e8 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Tue, 27 Oct 2026 11:47:22 +0100 Subject: [PATCH 08/16] chore(ffe): Track the checks to enable after the dd-openfeature 1.69.0 release Reference APMLP-1512 in the TODOs waiting for the first SDK release with the agent bridge, and prepare the muzzle pass directive of the bridge instrumentation, to enable once 1.69.0 is published. Co-Authored-By: Claude Opus 5.5 --- .../openfeature/dd-openfeature-1.69/build.gradle | 10 +++++++++- dd-openfeature/build.gradle.kts | 6 ++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/build.gradle b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/build.gradle index 900c517f4b8..ce6b586c0c1 100644 --- a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/build.gradle +++ b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/build.gradle @@ -2,7 +2,15 @@ plugins { id 'dd-trace-java.module.instrumentation' } -// TODO Add a muzzle pass directive on com.datadoghq:dd-openfeature once the SDK is published. +// The SDK releases with the agent bridge. +// TODO APMLP-1512 Enable once dd-openfeature 1.69.0 is published. +// muzzle { +// pass { +// group = 'com.datadoghq' +// module = 'dd-openfeature' +// versions = '[1.69.0,)' +// } +// } tracerJava { addSourceSetFor(JavaVersion.VERSION_11) diff --git a/dd-openfeature/build.gradle.kts b/dd-openfeature/build.gradle.kts index 9a8e13e1b68..3d9772ec925 100644 --- a/dd-openfeature/build.gradle.kts +++ b/dd-openfeature/build.gradle.kts @@ -62,7 +62,8 @@ tasks.named("jar") { // The agent bridge SPI versions, with the first SDK release shipping each of them. A released // version is frozen (only default methods can be added): check it against that release. -// TODO Set the v1 baseline once the first SDK release with the agent bridge is published. +// TODO APMLP-1512 Set the v1 baseline to 1.69.0, the first SDK release with the agent bridge, +// once it is published. val bridgeSpiBaselines = mapOf( "v1" to null, ) @@ -101,7 +102,8 @@ val japicmpPublicApi = tasks.register("japicmpPublicApi") { packageIncludes.addAll("com.datadog.openfeature", "datadog.trace.api.openfeature") failOnModification.set(true) failOnSourceIncompatibility.set(true) - // TODO Remove once the baseline is 1.69.0 or later: earlier releases link the agent bootstrap API. + // TODO APMLP-1512 Remove once the baseline is 1.69.0 or later: earlier releases link the agent + // bootstrap API. ignoreMissingClasses.set(true) txtOutputFile.set(layout.buildDirectory.file("reports/japicmp/public-api.txt")) } From 1919f3216604448184a3b68d4d4308d487df6c48 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Tue, 27 Oct 2026 11:47:23 +0100 Subject: [PATCH 09/16] chore(ffe): Drop the TODO about Remote Configuration without the agent Remote Configuration without the Datadog Java agent is out of scope. Co-Authored-By: Claude Opus 5.5 --- .../com/datadog/openfeature/internal/FeatureFlagsRuntime.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/FeatureFlagsRuntime.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/FeatureFlagsRuntime.java index b06f5386133..ca9517a1800 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/FeatureFlagsRuntime.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/FeatureFlagsRuntime.java @@ -105,7 +105,6 @@ static FeatureFlagsRuntime create(final AgentBridge bridge, final Settings setti final ConfigurationService source; final EventTransport transport; if (remoteConfig) { - // TODO Support Remote Configuration without the Datadog Java agent. final ConfigurationSource remoteConfiguration; try { remoteConfiguration = bridge.remoteConfiguration(); From 840895666d7eca8f5e6ca8a2a733f221ca98fd3e Mon Sep 17 00:00:00 2001 From: Sarah Chen Date: Tue, 27 Oct 2026 13:51:16 -0400 Subject: [PATCH 10/16] fix(ffe): Keep event proxy available with blank API key (cherry picked from commit ef95143bf0b6002f31b2bb0a59ca2a26e426223d) --- .../datadog/featureflag/FeatureFlagsBackend.java | 2 +- .../featureflag/FeatureFlagsBackendTest.java | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FeatureFlagsBackend.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FeatureFlagsBackend.java index 10d2d69837f..d6f9a72cb78 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FeatureFlagsBackend.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/FeatureFlagsBackend.java @@ -58,7 +58,7 @@ public FeatureFlagsBackend( config.getFeatureFlaggingConfigurationSource() == AGENTLESS, config.getFeatureFlaggingConfigurationSource() == AGENTLESS && config.getApiKey() != null - && !config.getApiKey().isEmpty()), + && !config.getApiKey().trim().isEmpty()), SpanEnrichmentWriter.getInstance(), CoreMetricCollector.getInstance(), onFirstShutdownSubscription); diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagsBackendTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagsBackendTest.java index ed5c042adbb..9fcbdc11b91 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagsBackendTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/FeatureFlagsBackendTest.java @@ -16,6 +16,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import datadog.communication.ddagent.DDAgentFeaturesDiscovery; import datadog.communication.ddagent.SharedCommunicationObjects; import datadog.remoteconfig.ConfigurationPoller; import datadog.remoteconfig.Product; @@ -135,6 +136,20 @@ void createsTheBackendFromTheAgentConfiguration( assertEquals(apiKey, created.setting("api-key")); } + @Test + void keepsTheEventProxyAvailableWithABlankApiKey() { + when(this.config.getFeatureFlaggingConfigurationSource()) + .thenReturn(FeatureFlaggingSource.AGENTLESS); + when(this.config.getApiKey()).thenReturn(" \t "); + when(this.sco.featuresDiscovery(this.config)).thenReturn(mock(DDAgentFeaturesDiscovery.class)); + + final FeatureFlagsBackend created = new FeatureFlagsBackend(this.sco, this.config, null); + + assertTrue( + created.isEventProxyAvailable(), + "the proxy stays available for recovery when direct intake has no usable API key"); + } + @Test void runsTheFirstShutdownSubscriptionActionOnce() { final AtomicInteger firstSubscriptions = new AtomicInteger(); From 8442221523e398ff6632630f4005f3805bdb5245 Mon Sep 17 00:00:00 2001 From: Sarah Chen Date: Tue, 27 Oct 2026 14:20:47 -0400 Subject: [PATCH 11/16] fix(ffe): Defer RC until an SDK subscriber is validated (cherry picked from commit dc27b62bc1b0222d812b6fe120605af43f9ec293) --- .../feature-flagging-lib/build.gradle.kts | 1 + .../featureflag/RemoteConfigServiceImpl.java | 7 +- .../RemoteConfigServiceImplTest.java | 70 ++++++++++++++++++- 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/products/feature-flagging/feature-flagging-lib/build.gradle.kts b/products/feature-flagging/feature-flagging-lib/build.gradle.kts index 4a355fa19be..a5768dee190 100644 --- a/products/feature-flagging/feature-flagging-lib/build.gradle.kts +++ b/products/feature-flagging/feature-flagging-lib/build.gradle.kts @@ -10,6 +10,7 @@ dependencies { api(libs.slf4j) api(project(":communication")) implementation(project(":internal-api")) + implementation(project(":remote-config:remote-config-core")) api(project(":products:feature-flagging:feature-flagging-bootstrap")) // Platform JSON writer for the ffe_* tag values. diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java index acbc89e33cd..48a1228b726 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java @@ -6,6 +6,7 @@ import datadog.remoteconfig.ConfigurationChangesTypedListener; import datadog.remoteconfig.ConfigurationPoller; import datadog.remoteconfig.PollingRateHinter; +import datadog.remoteconfig.ReportableException; import java.util.Iterator; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -62,7 +63,7 @@ public AutoCloseable subscribe(final Consumer subscriber) { try { subscriber.accept(document); } catch (final RuntimeException e) { - // Already reported to Remote Configuration when the document was received. + // The poller reports rejections during document delivery. LOGGER.debug("Feature Flags configuration subscriber rejected the current document", e); } } @@ -93,6 +94,7 @@ public synchronized void close() { * @param configKey the Remote Configuration key. * @param content the raw document, or {@code null} when the configuration is removed. * @param pollingRateHinter the polling rate hinter. + * @throws ReportableException if no SDK subscriber is available to validate the document. * @throws RuntimeException the first subscriber failure, once every subscriber is notified, so * the poller reports the document as not applied. */ @@ -103,6 +105,9 @@ public void accept( final PollingRateHinter pollingRateHinter) { final Iterator> snapshot; synchronized (this.subscribers) { + if (content != null && this.subscribers.isEmpty()) { + throw new ReportableException("Feature Flags configuration is awaiting an SDK subscriber"); + } this.current = content; snapshot = this.subscribers.iterator(); } 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 4871d2c98a6..a1105bc8baf 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,8 @@ package com.datadog.featureflag; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.jupiter.api.Assertions.assertArrayEquals; @@ -16,6 +18,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; @@ -26,6 +29,13 @@ import datadog.remoteconfig.ConfigurationPoller; import datadog.remoteconfig.PollingRateHinter; import datadog.remoteconfig.Product; +import datadog.remoteconfig.ReportableException; +import datadog.remoteconfig.state.ParsedConfigKey; +import datadog.remoteconfig.state.ProductState; +import datadog.remoteconfig.state.SimpleProductListener; +import datadog.remoteconfig.tuf.RemoteConfigRequest.ClientInfo.ClientState.ConfigState; +import datadog.remoteconfig.tuf.RemoteConfigResponse.Targets.ConfigTarget; +import datadog.remoteconfig.tuf.RemoteConfigResponse; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -41,6 +51,7 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.slf4j.LoggerFactory; +import org.tabletest.junit.TableTest; class RemoteConfigServiceImplTest { private static final byte[] DOCUMENT = "{\"flags\":{}}".getBytes(UTF_8); @@ -108,10 +119,60 @@ void reportsRejectedDocumentsOnceEverySubscriberIsNotified() { assertArrayEquals(DOCUMENT, received.get(0)); } + @TableTest({ + "Scenario | Rejection | Apply state", + "valid | | 2 ", + "malformed | malformed | 3 " + }) + void defersAcknowledgementUntilASubscriberValidatesTheDocument( + final String rejection, final int applyState) throws Exception { + final RemoteConfigServiceImpl service = + new RemoteConfigServiceImpl(mock(ConfigurationPoller.class)); + final ProductState state = new ProductState(Product.FFE_FLAGS); + state.addProductListener(new SimpleProductListener(service::accept)); + final ParsedConfigKey key = ParsedConfigKey.parse("org/FFE_FLAGS/config1/foo"); + final ConfigTarget target = new ConfigTarget(); + target.hashes = singletonMap("sha256", "hash1"); + target.length = DOCUMENT.length; + target.custom = new ConfigTarget.ConfigTargetCustom(); + target.custom.version = 1; + final RemoteConfigResponse response = mock(RemoteConfigResponse.class); + when(response.getTarget(key.toString())).thenReturn(target); + when(response.getFileContents(key.toString())).thenReturn(DOCUMENT); + + state.apply(response, singletonList(key), null); + + assertTrue(state.hasError()); + assertTrue(state.getConfigStates().isEmpty()); + assertTrue(state.getCachedTargetFiles().isEmpty()); + + final List received = new ArrayList<>(); + service.subscribe( + content -> { + received.add(content); + if (rejection != null) { + throw new IllegalArgumentException(rejection); + } + }); + assertTrue(received.isEmpty()); + state.apply(response, singletonList(key), null); + + assertEquals(1, received.size()); + assertArrayEquals(DOCUMENT, received.get(0)); + assertFalse(state.hasError()); + assertEquals(1, state.getConfigStates().size()); + final ConfigState configState = state.getConfigStates().iterator().next(); + assertEquals(applyState, configState.applyState); + assertEquals(rejection, configState.applyError); + assertEquals(1, state.getCachedTargetFiles().size()); + verify(response, times(2)).getFileContents(key.toString()); + } + @Test void lateSubscribersRejectingTheCurrentDocumentStaySubscribed() throws Exception { final RemoteConfigServiceImpl service = new RemoteConfigServiceImpl(mock(ConfigurationPoller.class)); + service.subscribe(content -> {}); service.accept("key", DOCUMENT, mock(PollingRateHinter.class)); final List received = new ArrayList<>(); @@ -148,6 +209,7 @@ void replaysTheCurrentDocumentToLateSubscribers() { void concurrentRemovalIsDeliveredAfterTheInitialReplay() throws Exception { final RemoteConfigServiceImpl service = new RemoteConfigServiceImpl(mock(ConfigurationPoller.class)); + service.subscribe(content -> {}); service.accept("key", DOCUMENT, null); final List received = new CopyOnWriteArrayList<>(); final CountDownLatch replayStarted = new CountDownLatch(1); @@ -216,7 +278,9 @@ void unsubscribedSubscribersStopReceivingDocuments() throws Exception { final AutoCloseable subscription = service.subscribe(received::add); subscription.close(); - service.accept("key", DOCUMENT, mock(PollingRateHinter.class)); + assertThrows( + ReportableException.class, + () -> service.accept("key", DOCUMENT, mock(PollingRateHinter.class))); assertEquals(0, received.size()); verify(poller, never()).removeListeners(any()); @@ -231,7 +295,9 @@ void closeUnregistersTheProductAndDropsSubscribers() { service.close(); service.close(); - service.accept("key", DOCUMENT, mock(PollingRateHinter.class)); + assertThrows( + ReportableException.class, + () -> service.accept("key", DOCUMENT, mock(PollingRateHinter.class))); verify(poller, times(1)) .removeCapabilities(Capabilities.CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); From d1eaee596bc3dd9e6e6f7db2e2346d116fa99017 Mon Sep 17 00:00:00 2001 From: Sarah Chen Date: Tue, 27 Oct 2026 17:13:00 -0400 Subject: [PATCH 12/16] fix(ffe): Restore RC before SDK subscriber is validated (cherry picked from commit c8ab5f6a862121580ace3b33c6a8e57fd4dc4dd3) --- .../feature-flagging-lib/build.gradle.kts | 1 - .../feature-flagging-lib/gradle.lockfile | 2 +- .../featureflag/RemoteConfigServiceImpl.java | 7 +- .../RemoteConfigServiceImplTest.java | 70 +------------------ 4 files changed, 4 insertions(+), 76 deletions(-) diff --git a/products/feature-flagging/feature-flagging-lib/build.gradle.kts b/products/feature-flagging/feature-flagging-lib/build.gradle.kts index a5768dee190..4a355fa19be 100644 --- a/products/feature-flagging/feature-flagging-lib/build.gradle.kts +++ b/products/feature-flagging/feature-flagging-lib/build.gradle.kts @@ -10,7 +10,6 @@ dependencies { api(libs.slf4j) api(project(":communication")) implementation(project(":internal-api")) - implementation(project(":remote-config:remote-config-core")) api(project(":products:feature-flagging:feature-flagging-bootstrap")) // Platform JSON writer for the ffe_* tag values. diff --git a/products/feature-flagging/feature-flagging-lib/gradle.lockfile b/products/feature-flagging/feature-flagging-lib/gradle.lockfile index 5ebb041416f..17cf71fb002 100644 --- a/products/feature-flagging/feature-flagging-lib/gradle.lockfile +++ b/products/feature-flagging/feature-flagging-lib/gradle.lockfile @@ -60,7 +60,7 @@ org.apache.commons:commons-lang3:3.20.0=spotbugs org.apache.commons:commons-text:1.15.0=spotbugs org.apache.logging.log4j:log4j-api:2.26.1=spotbugs org.apache.logging.log4j:log4j-core:2.26.1=spotbugs -org.apiguardian:apiguardian-api:1.1.2=testCompileClasspath +org.apiguardian:apiguardian-api:1.1.2=testCompileClasspath,testRuntimeClasspath org.codehaus.groovy:groovy-ant:3.0.23=codenarc org.codehaus.groovy:groovy-docgenerator:3.0.23=codenarc org.codehaus.groovy:groovy-groovydoc:3.0.23=codenarc diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java index 48a1228b726..acbc89e33cd 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java @@ -6,7 +6,6 @@ import datadog.remoteconfig.ConfigurationChangesTypedListener; import datadog.remoteconfig.ConfigurationPoller; import datadog.remoteconfig.PollingRateHinter; -import datadog.remoteconfig.ReportableException; import java.util.Iterator; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -63,7 +62,7 @@ public AutoCloseable subscribe(final Consumer subscriber) { try { subscriber.accept(document); } catch (final RuntimeException e) { - // The poller reports rejections during document delivery. + // Already reported to Remote Configuration when the document was received. LOGGER.debug("Feature Flags configuration subscriber rejected the current document", e); } } @@ -94,7 +93,6 @@ public synchronized void close() { * @param configKey the Remote Configuration key. * @param content the raw document, or {@code null} when the configuration is removed. * @param pollingRateHinter the polling rate hinter. - * @throws ReportableException if no SDK subscriber is available to validate the document. * @throws RuntimeException the first subscriber failure, once every subscriber is notified, so * the poller reports the document as not applied. */ @@ -105,9 +103,6 @@ public void accept( final PollingRateHinter pollingRateHinter) { final Iterator> snapshot; synchronized (this.subscribers) { - if (content != null && this.subscribers.isEmpty()) { - throw new ReportableException("Feature Flags configuration is awaiting an SDK subscriber"); - } this.current = content; snapshot = this.subscribers.iterator(); } 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 a1105bc8baf..4871d2c98a6 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,8 +1,6 @@ package com.datadog.featureflag; import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.Collections.singletonList; -import static java.util.Collections.singletonMap; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.jupiter.api.Assertions.assertArrayEquals; @@ -18,7 +16,6 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; @@ -29,13 +26,6 @@ import datadog.remoteconfig.ConfigurationPoller; import datadog.remoteconfig.PollingRateHinter; import datadog.remoteconfig.Product; -import datadog.remoteconfig.ReportableException; -import datadog.remoteconfig.state.ParsedConfigKey; -import datadog.remoteconfig.state.ProductState; -import datadog.remoteconfig.state.SimpleProductListener; -import datadog.remoteconfig.tuf.RemoteConfigRequest.ClientInfo.ClientState.ConfigState; -import datadog.remoteconfig.tuf.RemoteConfigResponse.Targets.ConfigTarget; -import datadog.remoteconfig.tuf.RemoteConfigResponse; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -51,7 +41,6 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.slf4j.LoggerFactory; -import org.tabletest.junit.TableTest; class RemoteConfigServiceImplTest { private static final byte[] DOCUMENT = "{\"flags\":{}}".getBytes(UTF_8); @@ -119,60 +108,10 @@ void reportsRejectedDocumentsOnceEverySubscriberIsNotified() { assertArrayEquals(DOCUMENT, received.get(0)); } - @TableTest({ - "Scenario | Rejection | Apply state", - "valid | | 2 ", - "malformed | malformed | 3 " - }) - void defersAcknowledgementUntilASubscriberValidatesTheDocument( - final String rejection, final int applyState) throws Exception { - final RemoteConfigServiceImpl service = - new RemoteConfigServiceImpl(mock(ConfigurationPoller.class)); - final ProductState state = new ProductState(Product.FFE_FLAGS); - state.addProductListener(new SimpleProductListener(service::accept)); - final ParsedConfigKey key = ParsedConfigKey.parse("org/FFE_FLAGS/config1/foo"); - final ConfigTarget target = new ConfigTarget(); - target.hashes = singletonMap("sha256", "hash1"); - target.length = DOCUMENT.length; - target.custom = new ConfigTarget.ConfigTargetCustom(); - target.custom.version = 1; - final RemoteConfigResponse response = mock(RemoteConfigResponse.class); - when(response.getTarget(key.toString())).thenReturn(target); - when(response.getFileContents(key.toString())).thenReturn(DOCUMENT); - - state.apply(response, singletonList(key), null); - - assertTrue(state.hasError()); - assertTrue(state.getConfigStates().isEmpty()); - assertTrue(state.getCachedTargetFiles().isEmpty()); - - final List received = new ArrayList<>(); - service.subscribe( - content -> { - received.add(content); - if (rejection != null) { - throw new IllegalArgumentException(rejection); - } - }); - assertTrue(received.isEmpty()); - state.apply(response, singletonList(key), null); - - assertEquals(1, received.size()); - assertArrayEquals(DOCUMENT, received.get(0)); - assertFalse(state.hasError()); - assertEquals(1, state.getConfigStates().size()); - final ConfigState configState = state.getConfigStates().iterator().next(); - assertEquals(applyState, configState.applyState); - assertEquals(rejection, configState.applyError); - assertEquals(1, state.getCachedTargetFiles().size()); - verify(response, times(2)).getFileContents(key.toString()); - } - @Test void lateSubscribersRejectingTheCurrentDocumentStaySubscribed() throws Exception { final RemoteConfigServiceImpl service = new RemoteConfigServiceImpl(mock(ConfigurationPoller.class)); - service.subscribe(content -> {}); service.accept("key", DOCUMENT, mock(PollingRateHinter.class)); final List received = new ArrayList<>(); @@ -209,7 +148,6 @@ void replaysTheCurrentDocumentToLateSubscribers() { void concurrentRemovalIsDeliveredAfterTheInitialReplay() throws Exception { final RemoteConfigServiceImpl service = new RemoteConfigServiceImpl(mock(ConfigurationPoller.class)); - service.subscribe(content -> {}); service.accept("key", DOCUMENT, null); final List received = new CopyOnWriteArrayList<>(); final CountDownLatch replayStarted = new CountDownLatch(1); @@ -278,9 +216,7 @@ void unsubscribedSubscribersStopReceivingDocuments() throws Exception { final AutoCloseable subscription = service.subscribe(received::add); subscription.close(); - assertThrows( - ReportableException.class, - () -> service.accept("key", DOCUMENT, mock(PollingRateHinter.class))); + service.accept("key", DOCUMENT, mock(PollingRateHinter.class)); assertEquals(0, received.size()); verify(poller, never()).removeListeners(any()); @@ -295,9 +231,7 @@ void closeUnregistersTheProductAndDropsSubscribers() { service.close(); service.close(); - assertThrows( - ReportableException.class, - () -> service.accept("key", DOCUMENT, mock(PollingRateHinter.class))); + service.accept("key", DOCUMENT, mock(PollingRateHinter.class)); verify(poller, times(1)) .removeCapabilities(Capabilities.CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); From 9817b27324e3c503f56635d94121cbc5ea611a4e Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Wed, 28 Oct 2026 10:27:35 +0100 Subject: [PATCH 13/16] fix(ffe): Report a rejected Remote Configuration document received before the SDK The agent registers the Feature Flags product at startup, so the first document usually arrives before any SDK subscribes and is acknowledged without validation. When the first subscriber rejects it on replay, the service now registers the product again, which drops its cached documents, so the next poll delivers the document again and reports the rejection as an apply error, without depending on the Remote Configuration core. Co-Authored-By: Claude Opus 5.5 --- .../featureflag/RemoteConfigServiceImpl.java | 36 ++++++++++++- .../RemoteConfigServiceImplTest.java | 54 +++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java index acbc89e33cd..8922955d373 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/RemoteConfigServiceImpl.java @@ -24,6 +24,13 @@ public class RemoteConfigServiceImpl implements ConfigurationChangesTypedListene private final ConfigurationPoller configurationPoller; private final List> subscribers = new CopyOnWriteArrayList<>(); private volatile byte[] current; + + /** + * Whether the current document was received before any subscriber, so the poller acknowledged it + * without validation. Guarded by the subscribers lock. + */ + private boolean unvalidated; + private boolean started; public RemoteConfigServiceImpl(final ConfigurationPoller configurationPoller) { @@ -41,9 +48,13 @@ public synchronized void start() { this.started = true; LOGGER.debug("Feature Flagging starting remote_config configuration source"); this.configurationPoller.addCapabilities(CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); + listen(); + this.configurationPoller.start(); + } + + private void listen() { // Listen as serialized event to prevent depending on the deserializer from agent this.configurationPoller.addListener(FFE_FLAGS, content -> content, this); - this.configurationPoller.start(); } /** @@ -54,6 +65,7 @@ public synchronized void start() { */ public AutoCloseable subscribe(final Consumer subscriber) { start(); + boolean requestAgain = false; // Keep registration and replay atomic with notification snapshots. synchronized (this.subscribers) { this.subscribers.add(subscriber); @@ -62,14 +74,33 @@ public AutoCloseable subscribe(final Consumer subscriber) { try { subscriber.accept(document); } catch (final RuntimeException e) { - // Already reported to Remote Configuration when the document was received. + // A rejection of a document received with subscribers is already reported. + requestAgain = this.unvalidated; LOGGER.debug("Feature Flags configuration subscriber rejected the current document", e); } + this.unvalidated = false; } } + if (requestAgain) { + requestAgain(); + } return () -> this.subscribers.remove(subscriber); } + /** + * Makes the poller deliver the current document again, so it reports the rejection of a document + * acknowledged before any subscriber validated it. Registering the product again drops its cached + * documents. Runs outside the subscribers lock, as the poller notifies under its own lock. + */ + private synchronized void requestAgain() { + if (!this.started) { + return; + } + LOGGER.debug("Requesting the rejected Feature Flags configuration again to report it"); + this.configurationPoller.removeListeners(FFE_FLAGS); + listen(); + } + /** * Unregisters the Feature Flags product, releases the poller started by {@link #start()} and * drops the subscribers. @@ -104,6 +135,7 @@ public void accept( final Iterator> snapshot; synchronized (this.subscribers) { this.current = content; + this.unvalidated = content != null && this.subscribers.isEmpty(); snapshot = this.subscribers.iterator(); } RuntimeException failure = null; 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 4871d2c98a6..f4083331c6c 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 @@ -130,6 +130,60 @@ void lateSubscribersRejectingTheCurrentDocumentStaySubscribed() throws Exception assertNull(received.get(0)); } + @Test + @SuppressWarnings("unchecked") + void requestsADocumentReceivedBeforeAnySubscriberAgainWhenRejected() { + final ConfigurationPoller poller = mock(ConfigurationPoller.class); + final RemoteConfigServiceImpl service = new RemoteConfigServiceImpl(poller); + service.start(); + service.accept("key", DOCUMENT, mock(PollingRateHinter.class)); + + service.subscribe( + content -> { + throw new IllegalArgumentException("malformed"); + }); + service.subscribe( + content -> { + throw new IllegalArgumentException("malformed"); + }); + + verify(poller, times(1)).removeListeners(Product.FFE_FLAGS); + verify(poller, times(2)) + .addListener(eq(Product.FFE_FLAGS), any(ConfigurationDeserializer.class), eq(service)); + } + + @Test + void keepsADocumentReceivedBeforeAnySubscriberWhenAccepted() { + final ConfigurationPoller poller = mock(ConfigurationPoller.class); + final RemoteConfigServiceImpl service = new RemoteConfigServiceImpl(poller); + service.start(); + service.accept("key", DOCUMENT, mock(PollingRateHinter.class)); + + service.subscribe(content -> {}); + + verify(poller, never()).removeListeners(any()); + } + + @Test + void doesNotRequestAgainADocumentAlreadyRejectedOnDelivery() { + final ConfigurationPoller poller = mock(ConfigurationPoller.class); + final RemoteConfigServiceImpl service = new RemoteConfigServiceImpl(poller); + service.subscribe( + content -> { + throw new IllegalArgumentException("malformed"); + }); + assertThrows( + IllegalArgumentException.class, + () -> service.accept("key", DOCUMENT, mock(PollingRateHinter.class))); + + service.subscribe( + content -> { + throw new IllegalArgumentException("malformed"); + }); + + verify(poller, never()).removeListeners(any()); + } + @Test void replaysTheCurrentDocumentToLateSubscribers() { final RemoteConfigServiceImpl service = From 25f652fe7a6aa0353bc7a5673c2e4944840c2052 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Wed, 28 Oct 2026 10:27:37 +0100 Subject: [PATCH 14/16] test(ffe): Check a malformed first Remote Configuration document is reported The smoke application can now register its provider on request instead of at startup, so the test delivers a malformed flag configuration before the SDK subscribes, then checks the agent reports it as not applied once the SDK rejects it. Co-Authored-By: Claude Opus 5.5 --- .../openfeature/OpenFeatureConfiguration.java | 13 ++- .../openfeature/OpenFeatureController.java | 16 +++- .../openfeature/ProviderRegistration.java | 36 +++++++ ...atureMalformedRemoteConfigSmokeTest.groovy | 95 +++++++++++++++++++ 4 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/ProviderRegistration.java create mode 100644 dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureMalformedRemoteConfigSmokeTest.groovy diff --git a/dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/OpenFeatureConfiguration.java b/dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/OpenFeatureConfiguration.java index 8ed5c9083a4..b1b41c0f9be 100644 --- a/dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/OpenFeatureConfiguration.java +++ b/dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/OpenFeatureConfiguration.java @@ -3,16 +3,25 @@ import com.datadog.openfeature.Provider; import dev.openfeature.sdk.Client; import dev.openfeature.sdk.OpenFeatureAPI; +import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @Configuration public class OpenFeatureConfiguration { + /** + * @param lazyProvider whether to wait for {@code POST /openfeature/provider} to register the + * provider, instead of registering it at startup. + * @return the OpenFeature client. + */ @Bean - public Client openFeatureClient() { + public Client openFeatureClient( + @Value("${openfeature.lazy-provider:false}") final boolean lazyProvider) { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProviderAndWait(new Provider()); + if (!lazyProvider) { + api.setProviderAndWait(new Provider()); + } return api.getClient(); } } diff --git a/dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/OpenFeatureController.java b/dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/OpenFeatureController.java index 47b5e6b18ab..9599854cbbf 100644 --- a/dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/OpenFeatureController.java +++ b/dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/OpenFeatureController.java @@ -26,9 +26,23 @@ public class OpenFeatureController { private static final Logger LOGGER = LoggerFactory.getLogger(OpenFeatureController.class); private final Client client; + private final ProviderRegistration providerRegistration; - public OpenFeatureController(final Client client) { + public OpenFeatureController( + final Client client, final ProviderRegistration providerRegistration) { this.client = client; + this.providerRegistration = providerRegistration; + } + + /** + * Registers the provider without waiting for its initialization. + * + * @return an accepted response. + */ + @PostMapping("/provider") + public ResponseEntity registerProvider() { + this.providerRegistration.request(); + return ResponseEntity.accepted().build(); } @PostMapping( diff --git a/dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/ProviderRegistration.java b/dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/ProviderRegistration.java new file mode 100644 index 00000000000..840243d547e --- /dev/null +++ b/dd-smoke-tests/openfeature/application/src/main/java/datadog/smoketest/springboot/openfeature/ProviderRegistration.java @@ -0,0 +1,36 @@ +package datadog.smoketest.springboot.openfeature; + +import com.datadog.openfeature.Provider; +import dev.openfeature.sdk.OpenFeatureAPI; +import java.util.concurrent.CountDownLatch; +import org.springframework.stereotype.Component; + +/** + * Registers the provider on request, from a thread started outside any request trace, so the + * provider initialization does not capture the trace context of the request. + */ +@Component +public class ProviderRegistration { + private final CountDownLatch requested = new CountDownLatch(1); + + public ProviderRegistration() { + final Thread thread = + new Thread(this::registerWhenRequested, "openfeature-provider-registration"); + thread.setDaemon(true); + thread.start(); + } + + /** Requests the provider registration, without waiting for its initialization. */ + public void request() { + this.requested.countDown(); + } + + private void registerWhenRequested() { + try { + this.requested.await(); + OpenFeatureAPI.getInstance().setProvider(new Provider()); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + } +} diff --git a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureMalformedRemoteConfigSmokeTest.groovy b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureMalformedRemoteConfigSmokeTest.groovy new file mode 100644 index 00000000000..3108fe0bb8a --- /dev/null +++ b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureMalformedRemoteConfigSmokeTest.groovy @@ -0,0 +1,95 @@ +package datadog.smoketest.springboot + +import datadog.smoketest.AbstractServerSmokeTest +import okhttp3.Request +import okhttp3.RequestBody + +/** + * A malformed flag configuration that Remote Configuration delivers before the SDK subscribes is + * acknowledged by the agent, then reported as not applied once the SDK rejects it. + */ +class OpenFeatureMalformedRemoteConfigSmokeTest extends AbstractServerSmokeTest { + private static final String CONFIG_PATH = 'datadog/2/FFE_FLAGS/1/config' + private static final String MALFORMED_CONFIG = '{"flags":[]}' + private static final int APPLY_STATE_ACKNOWLEDGED = 2 + private static final int APPLY_STATE_ERROR = 3 + + @Override + ProcessBuilder createProcessBuilder() { + setRemoteConfig(CONFIG_PATH, MALFORMED_CONFIG) + + final springBootShadowJar = System.getProperty("datadog.smoketest.springboot.shadowJar.path") + final command = [javaPath()] + command.addAll(defaultJavaProperties) + command.add('-Ddd.remote_config.enabled=true') + command.add("-Ddd.remote_config.url=http://localhost:${server.address.port}/v0.7/config".toString()) + command.addAll([ + '-jar', + springBootShadowJar, + "--server.port=${httpPort}".toString(), + '--openfeature.lazy-provider=true' + ]) + final builder = new ProcessBuilder(command).directory(new File(buildDirectory)) + builder.environment().put('DD_FEATURE_FLAGS_CONFIGURATION_SOURCE', 'remote_config') + return builder + } + + /** The provider never gets a valid configuration, so the OpenFeature SDK logs its failed initialization. */ + @Override + boolean isErrorLog(final String line) { + return super.isErrorLog(line) && !line.contains(' d.o.sdk.') && !line.contains(' dev.openfeature.sdk.') + } + + void 'test malformed configuration received before the SDK is reported as not applied'() { + setup: + // The base resets the Remote Configuration response before each test. + setRemoteConfig(CONFIG_PATH, MALFORMED_CONFIG) + final served = rcClientMessages.size() + + when: 'the agent receives the configuration while no SDK has subscribed' + final acknowledged = waitForRequestAfter(served) { applyState(it) == APPLY_STATE_ACKNOWLEDGED } + final response = client.newCall(new Request.Builder() + .url("http://localhost:${httpPort}/openfeature/provider") + .post(RequestBody.create(null, new byte[0])) + .build()).execute() + + then: 'the SDK rejects it and the agent reports it as not applied' + response.code() == 202 + final rejected = waitForRequestAfter(acknowledged) { applyState(it) == APPLY_STATE_ERROR } + (configState(rejected).apply_error as String).contains('Malformed flag configuration') + + cleanup: + response?.close() + } + + /** + * Waits for an agent Remote Configuration request matching the predicate. + * + * @param from the index of the first request to consider. + * @param predicate the predicate to match. + * @return the index of the first matching request. + */ + private int waitForRequestAfter(final int from, final Closure predicate) { + int index = -1 + defaultPoll.eventually { + index = rcClientMessages.findIndexOf(from, predicate) + assert index >= 0 + } + return index + } + + private Map configState(final int index) { + final request = rcClientMessages[index] + final states = request.client?.state?.config_states as List> + return states?.find { + it.product == 'FFE_FLAGS' + } + } + + private static Integer applyState(final Map request) { + final states = request.client?.state?.config_states as List> + return states?.find { + it.product == 'FFE_FLAGS' + }?.apply_state as Integer + } +} From 3b8f42086f9ca9a1a0e5f33aff74f79d5edcae72 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Thu, 29 Oct 2026 09:41:45 +0100 Subject: [PATCH 15/16] docs(ffe): Qualify when the agent shutdown notifies the SDK The agent shutdown notifies the SDK before the JVM shutdown hooks only when the tracing shutdown instrumentation runs, otherwise from a JVM shutdown hook. Document what always holds: the notification comes before the agent releases its Feature Flags resources. Co-Authored-By: Claude Opus 5.5 --- .../instrumentation/openfeature/AgentShutdownSource.java | 6 +++++- .../datadog/openfeature/internal/FeatureFlagsRuntime.java | 4 ++-- .../datadog/openfeature/internal/bridge/v1/AgentBridge.java | 5 +++-- .../trace/api/featureflag/FeatureFlaggingGateway.java | 4 +++- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/AgentShutdownSource.java b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/AgentShutdownSource.java index 9e73869ded4..9643ee4a99a 100644 --- a/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/AgentShutdownSource.java +++ b/dd-java-agent/instrumentation/datadog/openfeature/dd-openfeature-1.69/src/main/java11/datadog/trace/instrumentation/openfeature/AgentShutdownSource.java @@ -3,7 +3,11 @@ import com.datadog.openfeature.internal.bridge.v1.ShutdownSource; import datadog.trace.api.featureflag.FeatureFlaggingGateway; -/** Notifies the agent shutdown, before the JVM shutdown hooks run. */ +/** + * Notifies the agent shutdown, before the agent releases its Feature Flags resources: before the + * JVM shutdown hooks when the tracing shutdown instrumentation runs, otherwise from a JVM shutdown + * hook. + */ public final class AgentShutdownSource implements ShutdownSource { private final FeatureFlaggingGateway.Backend backend; diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/FeatureFlagsRuntime.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/FeatureFlagsRuntime.java index ca9517a1800..1eaebd45014 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/FeatureFlagsRuntime.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/FeatureFlagsRuntime.java @@ -31,8 +31,8 @@ /** * The Feature Flags runtime: configuration delivery and event pipelines. A single runtime is shared * by all the providers of a class loader, and stopped when the last one releases it, or when the - * application shuts down: by the Datadog Java agent before the JVM shutdown hooks run when - * attached, or by a JVM shutdown hook otherwise. + * application shuts down: by the Datadog Java agent shutdown when attached, or by a JVM shutdown + * hook otherwise. */ public final class FeatureFlagsRuntime { private static final Logger LOGGER = LoggerFactory.getLogger(FeatureFlagsRuntime.class); diff --git a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/bridge/v1/AgentBridge.java b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/bridge/v1/AgentBridge.java index 22531167beb..a48670f71fd 100644 --- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/bridge/v1/AgentBridge.java +++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/bridge/v1/AgentBridge.java @@ -40,8 +40,9 @@ public interface AgentBridge { SpanEnricher spanEnricher(); /** - * @return the agent shutdown notifications, sent before the JVM shutdown hooks run, or {@code - * null} if not available, so the SDK must flush from its own JVM shutdown hook. + * @return the agent shutdown notifications, sent before the agent releases its Feature Flags + * resources, or {@code null} if not available, so the SDK must flush from its own JVM + * shutdown hook. */ @Nullable ShutdownSource shutdown(); diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java index e5b4760c1b2..7b6d9730548 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/FeatureFlaggingGateway.java @@ -101,7 +101,9 @@ public interface Backend { void enrichRuntimeDefault(String flagKey, @Nullable Object defaultValue); /** - * Subscribes to the agent shutdown, notified before the JVM shutdown hooks run. + * Subscribes to the agent shutdown, notified before the agent releases its Feature Flags + * resources: before the JVM shutdown hooks when the tracing shutdown instrumentation runs, + * otherwise from a JVM shutdown hook. * * @param listener the listener run at shutdown, typically flushing the SDK pending events. * @return the handle to close to unsubscribe. From de707acb6e2f54a0228fe0b8cdc65e504b149f39 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Thu, 29 Oct 2026 09:41:46 +0100 Subject: [PATCH 16/16] test(ffe): Check the shutdown flush with a single smoke test The Feature Flagging shutdown hook is now always registered once the SDK subscribes, whether tracing is enabled or not, so the tracing disabled variant tested the same behavior. Keep one test, with the default setup. Co-Authored-By: Claude Opus 5.5 --- ...bstractOpenFeatureShutdownSmokeTest.groovy | 135 ------------------ .../OpenFeatureShutdownSmokeTest.groovy | 129 ++++++++++++++++- ...atureTraceDisabledShutdownSmokeTest.groovy | 13 -- 3 files changed, 125 insertions(+), 152 deletions(-) delete mode 100644 dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/AbstractOpenFeatureShutdownSmokeTest.groovy delete mode 100644 dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureTraceDisabledShutdownSmokeTest.groovy diff --git a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/AbstractOpenFeatureShutdownSmokeTest.groovy b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/AbstractOpenFeatureShutdownSmokeTest.groovy deleted file mode 100644 index 240131577d1..00000000000 --- a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/AbstractOpenFeatureShutdownSmokeTest.groovy +++ /dev/null @@ -1,135 +0,0 @@ -package datadog.smoketest.springboot - -import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer -import static java.util.concurrent.TimeUnit.SECONDS - -import datadog.smoketest.AbstractSmokeTest -import datadog.trace.agent.test.server.http.TestHttpServer -import datadog.trace.agent.test.server.http.TestHttpServer.HandlerApi.RequestApi -import datadog.trace.agent.test.utils.OkHttpUtils -import datadog.trace.agent.test.utils.PortUtils -import groovy.json.JsonOutput -import groovy.json.JsonSlurper -import okhttp3.MediaType -import okhttp3.OkHttpClient -import okhttp3.Request -import okhttp3.RequestBody -import spock.lang.AutoCleanup -import spock.lang.Shared -import spock.util.concurrent.PollingConditions - -/** The SDK with the Datadog Java agent flushes its pending events when the application exits. */ -abstract class AbstractOpenFeatureShutdownSmokeTest extends AbstractSmokeTest { - - @Shared - protected int httpPort = PortUtils.randomOpenPort() - - protected OkHttpClient client = OkHttpUtils.client() - - @Shared - protected final ufcConfig = new JsonSlurper().parse(fetchResource("ffe-system-test-data/ufc-config.json")) as Map - - /** Serves the flag configuration as the Datadog managed CDN does. */ - @Shared - @AutoCleanup - protected TestHttpServer cdn = httpServer { - handlers { - prefix("/api/v2/feature-flagging/config/rules-based/server") { - response.status(200).send(JsonOutput.toJson([ - data: [type: 'universal-flag-configuration', attributes: ufcConfig] - ])) - } - } - } - - /** @return whether the application runs with tracing enabled. */ - abstract boolean tracing() - - def setupSpec() { - PortUtils.waitForPortToOpen(httpPort, 240, SECONDS, testedProcess) - } - - @Override - ProcessBuilder createProcessBuilder() { - final springBootShadowJar = System.getProperty("datadog.smoketest.springboot.shadowJar.path") - final command = [javaPath()] - command.addAll(defaultJavaProperties) - command.add("-Ddd.trace.enabled=${tracing()}".toString()) - command.addAll(['-jar', springBootShadowJar, "--server.port=${httpPort}".toString()]) - final builder = new ProcessBuilder(command).directory(new File(buildDirectory)) - builder.environment().put('DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL', "http://localhost:${cdn.address.port}".toString()) - return builder - } - - @Override - boolean testTelemetry() { - return false - } - - @Override - Closure decodedTracesCallback() { - return {} - } - - @Override - Closure decodedEvpProxyMessageCallback() { - return { String path, RequestApi request -> - if (!path.contains('api/v2/flagevaluation')) { - return null - } - return [ - origin: request.getHeader('DD-EVP-ORIGIN'), - originVersion: request.getHeader('DD-EVP-ORIGIN-VERSION'), - body: new JsonSlurper().parse(request.body) - ] - } - } - - void 'test pending flag evaluations are flushed at application exit'() { - when: - // The first evaluation is flushed within the 10 seconds flush interval, which then restarts: - // the next evaluation stays pending until the application exits. - evaluate('test-case-boolean-false-assignment.json') - new PollingConditions(timeout: 20).eventually { - assert evaluatedFlags().contains('boolean-false-assignment') - } - evaluate('test-case-boolean-one-of-matches.json') - - then: - !evaluatedFlags().contains('boolean-one-of-matches') - - when: - testedProcess.destroy() - - then: - testedProcess.waitFor(30, SECONDS) - evaluatedFlags().contains('boolean-one-of-matches') - evaluationRequests().every { it.origin == 'dd-trace-java' && it.originVersion } - } - - protected void evaluate(final String testCaseFile) { - final testCase = (new JsonSlurper().parse(fetchResource("ffe-system-test-data/evaluation-cases/${testCaseFile}")) as List>).first() - final request = new Request.Builder() - .url("http://localhost:${httpPort}/openfeature/evaluate") - .post(RequestBody.create(MediaType.parse('application/json'), JsonOutput.toJson(testCase))) - .build() - final response = client.newCall(request).execute() - try { - assert response.code() == 200 - } finally { - response.close() - } - } - - protected List evaluatedFlags() { - return evaluationRequests()*.body*.flagEvaluations.flatten()*.flag*.key as List - } - - protected List> evaluationRequests() { - return evpProxyMessages*.getV2() as List> - } - - protected static URL fetchResource(final String name) { - return Thread.currentThread().getContextClassLoader().getResource(name) - } -} diff --git a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureShutdownSmokeTest.groovy b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureShutdownSmokeTest.groovy index 70192aa9331..55d79b006a7 100644 --- a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureShutdownSmokeTest.groovy +++ b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureShutdownSmokeTest.groovy @@ -1,10 +1,131 @@ package datadog.smoketest.springboot -/** The tracing shutdown hook flushes the SDK pending events. */ -class OpenFeatureShutdownSmokeTest extends AbstractOpenFeatureShutdownSmokeTest { +import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer +import static java.util.concurrent.TimeUnit.SECONDS + +import datadog.smoketest.AbstractSmokeTest +import datadog.trace.agent.test.server.http.TestHttpServer +import datadog.trace.agent.test.server.http.TestHttpServer.HandlerApi.RequestApi +import datadog.trace.agent.test.utils.OkHttpUtils +import datadog.trace.agent.test.utils.PortUtils +import groovy.json.JsonOutput +import groovy.json.JsonSlurper +import okhttp3.MediaType +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.RequestBody +import spock.lang.AutoCleanup +import spock.lang.Shared +import spock.util.concurrent.PollingConditions + +/** The SDK with the Datadog Java agent flushes its pending events when the application exits. */ +class OpenFeatureShutdownSmokeTest extends AbstractSmokeTest { + + @Shared + protected int httpPort = PortUtils.randomOpenPort() + + protected OkHttpClient client = OkHttpUtils.client() + + @Shared + protected final ufcConfig = new JsonSlurper().parse(fetchResource("ffe-system-test-data/ufc-config.json")) as Map + + /** Serves the flag configuration as the Datadog managed CDN does. */ + @Shared + @AutoCleanup + protected TestHttpServer cdn = httpServer { + handlers { + prefix("/api/v2/feature-flagging/config/rules-based/server") { + response.status(200).send(JsonOutput.toJson([ + data: [type: 'universal-flag-configuration', attributes: ufcConfig] + ])) + } + } + } + + def setupSpec() { + PortUtils.waitForPortToOpen(httpPort, 240, SECONDS, testedProcess) + } + + @Override + ProcessBuilder createProcessBuilder() { + final springBootShadowJar = System.getProperty("datadog.smoketest.springboot.shadowJar.path") + final command = [javaPath()] + command.addAll(defaultJavaProperties) + command.addAll(['-jar', springBootShadowJar, "--server.port=${httpPort}".toString()]) + final builder = new ProcessBuilder(command).directory(new File(buildDirectory)) + builder.environment().put('DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL', "http://localhost:${cdn.address.port}".toString()) + return builder + } @Override - boolean tracing() { - return true + boolean testTelemetry() { + return false + } + + @Override + Closure decodedTracesCallback() { + return {} + } + + @Override + Closure decodedEvpProxyMessageCallback() { + return { String path, RequestApi request -> + if (!path.contains('api/v2/flagevaluation')) { + return null + } + return [ + origin: request.getHeader('DD-EVP-ORIGIN'), + originVersion: request.getHeader('DD-EVP-ORIGIN-VERSION'), + body: new JsonSlurper().parse(request.body) + ] + } + } + + void 'test pending flag evaluations are flushed at application exit'() { + when: + // The first evaluation is flushed within the 10 seconds flush interval, which then restarts: + // the next evaluation stays pending until the application exits. + evaluate('test-case-boolean-false-assignment.json') + new PollingConditions(timeout: 20).eventually { + assert evaluatedFlags().contains('boolean-false-assignment') + } + evaluate('test-case-boolean-one-of-matches.json') + + then: + !evaluatedFlags().contains('boolean-one-of-matches') + + when: + testedProcess.destroy() + + then: + testedProcess.waitFor(30, SECONDS) + evaluatedFlags().contains('boolean-one-of-matches') + evaluationRequests().every { it.origin == 'dd-trace-java' && it.originVersion } + } + + protected void evaluate(final String testCaseFile) { + final testCase = (new JsonSlurper().parse(fetchResource("ffe-system-test-data/evaluation-cases/${testCaseFile}")) as List>).first() + final request = new Request.Builder() + .url("http://localhost:${httpPort}/openfeature/evaluate") + .post(RequestBody.create(MediaType.parse('application/json'), JsonOutput.toJson(testCase))) + .build() + final response = client.newCall(request).execute() + try { + assert response.code() == 200 + } finally { + response.close() + } + } + + protected List evaluatedFlags() { + return evaluationRequests()*.body*.flagEvaluations.flatten()*.flag*.key as List + } + + protected List> evaluationRequests() { + return evpProxyMessages*.getV2() as List> + } + + protected static URL fetchResource(final String name) { + return Thread.currentThread().getContextClassLoader().getResource(name) } } diff --git a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureTraceDisabledShutdownSmokeTest.groovy b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureTraceDisabledShutdownSmokeTest.groovy deleted file mode 100644 index c300962d260..00000000000 --- a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureTraceDisabledShutdownSmokeTest.groovy +++ /dev/null @@ -1,13 +0,0 @@ -package datadog.smoketest.springboot - -/** - * Without tracing, the Feature Flagging system registers its own shutdown hook to flush the SDK - * pending events. - */ -class OpenFeatureTraceDisabledShutdownSmokeTest extends AbstractOpenFeatureShutdownSmokeTest { - - @Override - boolean tracing() { - return false - } -}