From 00be4be0d66a080713e24af74e9fbb6260e21194 Mon Sep 17 00:00:00 2001 From: "ethan.romero" Date: Thu, 15 Oct 2026 19:06:30 +0000 Subject: [PATCH 1/7] Add debug logging for feature flag onboarding --- .../com/datadog/featureflag/FeatureFlaggingSystem.java | 3 +++ .../datadog/featureflag/RemoteConfigServiceImpl.java | 10 ++++++++++ 2 files changed, 13 insertions(+) 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 0b971d3f6f0..8bdd58f2be8 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 @@ -33,10 +33,13 @@ public static synchronized void start(final SharedCommunicationObjects sco) { LOGGER.debug("Feature Flagging system disabled"); return; } + LOGGER.debug("Feature Flagging enabled; env={}", config.getEnv()); final FeatureFlagsBackend backend = new FeatureFlagsBackend(sco, config); try { backend.start(); } catch (final RuntimeException | Error e) { + LOGGER.debug( + "Feature Flagging system initialization failed: {}", e.getClass().getSimpleName()); backend.close(); throw e; } 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 837e377dec4..894603557ef 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 @@ -39,6 +39,7 @@ public synchronized void start() { return; } this.started = true; + LOGGER.debug("Feature Flagging starting remote_config configuration source"); this.configurationPoller.addCapabilities(CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); // Listen as serialized event to prevent depending on the deserializer from agent this.configurationPoller.addListener(FFE_FLAGS, content -> content, this); @@ -70,6 +71,7 @@ public synchronized void close() { return; } this.started = false; + LOGGER.debug("Feature Flagging remote_config configuration source stopped"); this.configurationPoller.removeCapabilities(CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); this.configurationPoller.removeListeners(FFE_FLAGS); this.subscribers.clear(); @@ -81,6 +83,14 @@ public void accept( final String configKey, @Nullable final byte[] content, final PollingRateHinter pollingRateHinter) { + if (content == null) { + LOGGER.debug("Feature Flagging remote_config configuration removed: key={}", configKey); + } else { + LOGGER.debug( + "Feature Flagging remote_config configuration received: key={}, bytes={}", + configKey, + content.length); + } final Iterator> snapshot; synchronized (this.subscribers) { this.current = content; From 8b4712c942336e30d5562c3219b4a052dd2c009f Mon Sep 17 00:00:00 2001 From: "ethan.romero" Date: Wed, 21 Oct 2026 14:49:55 +0000 Subject: [PATCH 2/7] Complete feature flag evaluation and lifecycle diagnostics --- .../datadog/featureflag/DiagnosticLogger.java | 117 +++++++++++++++ .../SpanEnrichmentInterceptor.java | 5 +- .../featureflag/SpanEnrichmentWriter.java | 5 +- .../featureflag/DiagnosticIsolationTest.java | 136 ++++++++++++++++++ 4 files changed, 257 insertions(+), 6 deletions(-) create mode 100644 products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/DiagnosticLogger.java create mode 100644 products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticIsolationTest.java diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/DiagnosticLogger.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/DiagnosticLogger.java new file mode 100644 index 00000000000..82b37b4e34d --- /dev/null +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/DiagnosticLogger.java @@ -0,0 +1,117 @@ +package com.datadog.featureflag; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.Marker; + +/** Isolates diagnostic backend failures from feature flag behavior. */ +public final class DiagnosticLogger { + private final Logger delegate; + + private DiagnosticLogger(Logger delegate) { + this.delegate = delegate; + } + + public static DiagnosticLogger getLogger(Class type) { + Logger logger; + try { + logger = LoggerFactory.getLogger(type); + } catch (Throwable ignored) { + logger = null; + } + return new DiagnosticLogger(logger); + } + + /** The rate limiter's calls are guarded by its caller, including its own level checks. */ + Logger backend() { + return delegate == null ? org.slf4j.helpers.NOPLogger.NOP_LOGGER : delegate; + } + + public boolean isDebugEnabled() { + try { + return delegate != null && delegate.isDebugEnabled(); + } catch (Throwable ignored) { + return false; + } + } + + public void debug(String message) { + try { + if (delegate != null) { + delegate.debug(message); + } + } catch (Throwable ignored) { + // A diagnostic must never interrupt evaluation, readiness notification, or teardown. + } + } + + public void debug(String message, Object argument) { + try { + if (delegate != null) { + delegate.debug(message, argument); + } + } catch (Throwable ignored) { + // A diagnostic must never interrupt evaluation, readiness notification, or teardown. + } + } + + public void debug(String message, Object first, Object second) { + try { + if (delegate != null) { + delegate.debug(message, first, second); + } + } catch (Throwable ignored) { + // A diagnostic must never interrupt evaluation, readiness notification, or teardown. + } + } + + public void debug(String message, Object... arguments) { + try { + if (delegate != null) { + delegate.debug(message, arguments); + } + } catch (Throwable ignored) { + // A diagnostic must never interrupt evaluation, readiness notification, or teardown. + } + } + + public void warn(String message) { + try { + if (delegate != null) { + delegate.warn(message); + } + } catch (Throwable ignored) { + // A diagnostic must never interrupt evaluation, readiness notification, or teardown. + } + } + + public void warn(String message, Object... arguments) { + try { + if (delegate != null) { + delegate.warn(message, arguments); + } + } catch (Throwable ignored) { + // A diagnostic must never interrupt evaluation, readiness notification, or teardown. + } + } + + public void error(String message, Throwable error) { + try { + if (delegate != null) { + delegate.error(message, error); + } + } catch (Throwable ignored) { + // A diagnostic must never interrupt teardown. + } + } + + public void error(Marker marker, String message, Throwable error) { + try { + if (delegate != null) { + delegate.error(marker, message, error); + } + } catch (Throwable ignored) { + // A diagnostic must never interrupt evaluation, readiness notification, or teardown. + } + } +} diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentInterceptor.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentInterceptor.java index a82e194e248..9098ce17fa6 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentInterceptor.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentInterceptor.java @@ -5,8 +5,6 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import java.util.Collection; import java.util.Map; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * {@link TraceInterceptor} that writes the accumulated {@code ffe_*} tags onto the local root span @@ -46,7 +44,8 @@ */ final class SpanEnrichmentInterceptor implements TraceInterceptor { - private static final Logger log = LoggerFactory.getLogger(SpanEnrichmentInterceptor.class); + private static final DiagnosticLogger log = + DiagnosticLogger.getLogger(SpanEnrichmentInterceptor.class); /** * Unique priority in the "trace data enrichment" band, after {@code GIT_METADATA} (3) and before diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentWriter.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentWriter.java index e9c1200868e..342ceddcdf6 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentWriter.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentWriter.java @@ -4,8 +4,6 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.util.concurrent.atomic.AtomicBoolean; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Agent-side owner of APM feature-flag span enrichment. This is the WRITE tier of the @@ -31,7 +29,8 @@ */ public final class SpanEnrichmentWriter implements AutoCloseable { - private static final Logger log = LoggerFactory.getLogger(SpanEnrichmentWriter.class); + private static final DiagnosticLogger log = + DiagnosticLogger.getLogger(SpanEnrichmentWriter.class); public static SpanEnrichmentWriter getInstance() { return SingletonHolder.INSTANCE; diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticIsolationTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticIsolationTest.java new file mode 100644 index 00000000000..0f75b50ca2b --- /dev/null +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticIsolationTest.java @@ -0,0 +1,136 @@ +package com.datadog.featureflag; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.emptyMap; +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.turbo.TurboFilter; +import ch.qos.logback.core.spi.FilterReply; +import datadog.communication.BackendApi; +import datadog.communication.BackendApiFactory; +import datadog.communication.ddagent.SharedCommunicationObjects; +import datadog.remoteconfig.ConfigurationPoller; +import datadog.remoteconfig.Product; +import datadog.trace.api.Config; +import datadog.trace.api.featureflag.FeatureFlaggingGateway; +import datadog.trace.api.featureflag.exposure.Allocation; +import datadog.trace.api.featureflag.exposure.ExposureEvent; +import datadog.trace.api.featureflag.exposure.Flag; +import datadog.trace.api.featureflag.exposure.Subject; +import datadog.trace.api.featureflag.exposure.Variant; +import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; +import datadog.trace.api.intake.Intake; +import datadog.trace.test.util.PollingConditions; +import java.util.concurrent.ScheduledExecutorService; +import okhttp3.HttpUrl; +import org.slf4j.LoggerFactory; +import org.slf4j.Marker; +import org.tabletest.junit.TableTest; + +class DiagnosticIsolationTest { + @TableTest({ + "Scenario | Fail level check", + "backend throws | false ", + "level check throws | true " + }) + void preservesConfigurationAndDeliveryWhenLoggingFails(boolean failLevelCheck) throws Exception { + LoggerContext logging = (LoggerContext) LoggerFactory.getILoggerFactory(); + TurboFilter failure = + new TurboFilter() { + @Override + public FilterReply decide( + Marker marker, + Logger logger, + Level level, + String format, + Object[] params, + Throwable error) { + if (logger.getName().startsWith("com.datadog.featureflag") + && (failLevelCheck || format != null)) { + throw new AssertionError("broken logger"); + } + return FilterReply.NEUTRAL; + } + }; + failure.start(); + logging.addTurboFilter(failure); + FeatureFlaggingGateway.ConfigListener listener = + mock(FeatureFlaggingGateway.ConfigListener.class); + FeatureFlaggingGateway.addConfigListener(listener); + try { + Config config = mock(Config.class); + SharedCommunicationObjects sco = mock(SharedCommunicationObjects.class); + ConfigurationPoller poller = mock(ConfigurationPoller.class); + when(sco.configurationPoller(config)).thenReturn(poller); + try (RemoteConfigServiceImpl service = new RemoteConfigServiceImpl(sco, config)) { + service.init(); + ServerConfiguration parsed = + UniversalFlagConfigParser.INSTANCE.deserialize( + "{\"flags\":{\"bad\":42}}".getBytes(UTF_8)); + assertEquals(1, parsed.invalidFlags.size()); + service.accept("config", parsed, null); + verify(listener).accept(parsed); + } + verify(poller).removeListeners(Product.FFE_FLAGS); + verify(poller).stop(); + + AgentlessConfigurationSource.UfcHttpClient client = + mock(AgentlessConfigurationSource.UfcHttpClient.class); + ScheduledExecutorService executor = mock(ScheduledExecutorService.class); + HttpUrl endpoint = HttpUrl.get("http://localhost/config"); + when(client.fetch(eq(endpoint), eq(config), any())) + .thenReturn(new AgentlessConfigurationSource.UfcHttpResponse(503, null, null)) + .thenReturn( + new AgentlessConfigurationSource.UfcHttpResponse( + 200, + "v1", + "{\"data\":{\"type\":\"universal-flag-configuration\",\"attributes\":{\"flags\":{}}}}" + .getBytes(UTF_8))); + try (AgentlessConfigurationSource source = + new AgentlessConfigurationSource(endpoint, config, 1000, client, executor)) { + source.init(); + assertTrue(source.pollOnce()); + } + verify(client).cancel(); + verify(executor).shutdownNow(); + + BackendApi backend = mock(BackendApi.class); + BackendApiFactory backendFactory = mock(BackendApiFactory.class); + when(backendFactory.createEvpProxyApi(Intake.EVENT_PLATFORM, true)).thenReturn(backend); + FeatureFlagBackendApiFactory factory = + new FeatureFlagBackendApiFactory(config, backendFactory, FeatureFlagEventType.EXPOSURE); + ExposureWriterImpl writer = new ExposureWriterImpl(16, 1, MILLISECONDS, factory, config); + PollingConditions poll = new PollingConditions(5); + try { + writer.init(); + writer.accept( + new ExposureEvent( + 1, + new Allocation("allocation"), + new Flag("flag"), + new Variant("variant"), + new Subject("user", emptyMap()))); + poll.eventually(() -> verify(backend).post(any(), any(), any(), any(), eq(false))); + } finally { + writer.close(); + } + poll.eventually(() -> assertFalse(writer.isSerializerThreadAlive())); + } finally { + FeatureFlaggingGateway.removeConfigListener(listener); + FeatureFlaggingGateway.dispatch((ServerConfiguration) null); + logging.getTurboFilterList().remove(failure); + failure.stop(); + } + } +} From f2f51e9f3b5484d1c54f789e863b6e3f0788ea5b Mon Sep 17 00:00:00 2001 From: "ethan.romero" Date: Wed, 21 Oct 2026 17:04:37 +0000 Subject: [PATCH 3/7] Cover unavailable and failing feature flag loggers --- .../featureflag/DiagnosticLoggerTest.java | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticLoggerTest.java diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticLoggerTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticLoggerTest.java new file mode 100644 index 00000000000..47ef63e32a7 --- /dev/null +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticLoggerTest.java @@ -0,0 +1,88 @@ +package com.datadog.featureflag; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; +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.verify; +import static org.mockito.Mockito.when; + +import java.lang.reflect.Constructor; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.Marker; +import org.slf4j.helpers.NOPLogger; + +class DiagnosticLoggerTest { + private final Marker marker = mock(Marker.class); + private final Throwable error = new IllegalStateException("diagnostic failure"); + + @Test + void forwardsMessagesAndArgumentsToTheBackend() throws Exception { + Logger backend = mock(Logger.class); + when(backend.isDebugEnabled()).thenReturn(true); + DiagnosticLogger logger = loggerWithBackend(backend); + + assertTrue(logger.isDebugEnabled()); + assertSame(backend, logger.backend()); + logAllLevels(logger); + + verify(backend).debug("message"); + verify(backend).debug("one {}", "first"); + verify(backend).debug("two {} {}", "first", "second"); + verify(backend).debug("three {} {} {}", "first", "second", "third"); + verify(backend).warn("warning"); + verify(backend).warn("warning {}", new Object[] {"argument"}); + verify(backend).error("error", error); + verify(backend).error(marker, "marked error", error); + } + + @Test + void respectsTheBackendDebugLevel() throws Exception { + Logger backend = mock(Logger.class); + assertFalse(loggerWithBackend(backend).isDebugEnabled()); + verify(backend).isDebugEnabled(); + } + + @Test + void toleratesAnUnavailableBackend() throws Exception { + DiagnosticLogger logger = loggerWithBackend(null); + + assertFalse(logger.isDebugEnabled()); + assertSame(NOPLogger.NOP_LOGGER, logger.backend()); + assertDoesNotThrow(() -> logAllLevels(logger)); + } + + @Test + void containsFailuresFromEveryLoggingOverloadAndLevelCheck() throws Exception { + Logger backend = + mock( + Logger.class, + invocation -> { + throw new AssertionError("broken backend"); + }); + DiagnosticLogger logger = loggerWithBackend(backend); + + assertFalse(logger.isDebugEnabled()); + assertDoesNotThrow(() -> logAllLevels(logger)); + } + + private void logAllLevels(DiagnosticLogger logger) { + logger.debug("message"); + logger.debug("one {}", "first"); + logger.debug("two {} {}", "first", "second"); + logger.debug("three {} {} {}", "first", "second", "third"); + logger.warn("warning"); + logger.warn("warning {}", "argument"); + logger.error("error", error); + logger.error(marker, "marked error", error); + } + + private static DiagnosticLogger loggerWithBackend(Logger backend) throws Exception { + Constructor constructor = + DiagnosticLogger.class.getDeclaredConstructor(Logger.class); + constructor.setAccessible(true); + return constructor.newInstance(backend); + } +} From 7a5587fd61b544ce6366c3fe092028965395bf5a Mon Sep 17 00:00:00 2001 From: ethan-romero-dg Date: Thu, 22 Oct 2026 10:22:21 -0400 Subject: [PATCH 4/7] Simplify feature flag diagnostic logging Replace the DiagnosticLogger wrappers with plain SLF4J loggers. Agent-side code logs through the tracer's internal logger, so it needs no isolation wrapper. The provider keeps isolation where an application logging backend is involved: debug diagnostics go through debugQuietly and DebugLoggingHook, which cache DD_TRACE_DEBUG and swallow logging and formatting failures. - Warn at provider construction when DD_TRACE_DEBUG is enabled but the application logger blocks DEBUG for datadog.trace.api.openfeature. - Report flags=missing for Remote Config payloads without a flags member, separately from an empty configuration. - Drop the racy inFlight shutdown field and the duplicate parser summary. - Update the README to match. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5.5 --- .../datadog/featureflag/DiagnosticLogger.java | 117 --------------- .../SpanEnrichmentInterceptor.java | 5 +- .../featureflag/SpanEnrichmentWriter.java | 5 +- .../featureflag/DiagnosticIsolationTest.java | 136 ------------------ .../featureflag/DiagnosticLoggerTest.java | 88 ------------ 5 files changed, 6 insertions(+), 345 deletions(-) delete mode 100644 products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/DiagnosticLogger.java delete mode 100644 products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticIsolationTest.java delete mode 100644 products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticLoggerTest.java diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/DiagnosticLogger.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/DiagnosticLogger.java deleted file mode 100644 index 82b37b4e34d..00000000000 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/DiagnosticLogger.java +++ /dev/null @@ -1,117 +0,0 @@ -package com.datadog.featureflag; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.slf4j.Marker; - -/** Isolates diagnostic backend failures from feature flag behavior. */ -public final class DiagnosticLogger { - private final Logger delegate; - - private DiagnosticLogger(Logger delegate) { - this.delegate = delegate; - } - - public static DiagnosticLogger getLogger(Class type) { - Logger logger; - try { - logger = LoggerFactory.getLogger(type); - } catch (Throwable ignored) { - logger = null; - } - return new DiagnosticLogger(logger); - } - - /** The rate limiter's calls are guarded by its caller, including its own level checks. */ - Logger backend() { - return delegate == null ? org.slf4j.helpers.NOPLogger.NOP_LOGGER : delegate; - } - - public boolean isDebugEnabled() { - try { - return delegate != null && delegate.isDebugEnabled(); - } catch (Throwable ignored) { - return false; - } - } - - public void debug(String message) { - try { - if (delegate != null) { - delegate.debug(message); - } - } catch (Throwable ignored) { - // A diagnostic must never interrupt evaluation, readiness notification, or teardown. - } - } - - public void debug(String message, Object argument) { - try { - if (delegate != null) { - delegate.debug(message, argument); - } - } catch (Throwable ignored) { - // A diagnostic must never interrupt evaluation, readiness notification, or teardown. - } - } - - public void debug(String message, Object first, Object second) { - try { - if (delegate != null) { - delegate.debug(message, first, second); - } - } catch (Throwable ignored) { - // A diagnostic must never interrupt evaluation, readiness notification, or teardown. - } - } - - public void debug(String message, Object... arguments) { - try { - if (delegate != null) { - delegate.debug(message, arguments); - } - } catch (Throwable ignored) { - // A diagnostic must never interrupt evaluation, readiness notification, or teardown. - } - } - - public void warn(String message) { - try { - if (delegate != null) { - delegate.warn(message); - } - } catch (Throwable ignored) { - // A diagnostic must never interrupt evaluation, readiness notification, or teardown. - } - } - - public void warn(String message, Object... arguments) { - try { - if (delegate != null) { - delegate.warn(message, arguments); - } - } catch (Throwable ignored) { - // A diagnostic must never interrupt evaluation, readiness notification, or teardown. - } - } - - public void error(String message, Throwable error) { - try { - if (delegate != null) { - delegate.error(message, error); - } - } catch (Throwable ignored) { - // A diagnostic must never interrupt teardown. - } - } - - public void error(Marker marker, String message, Throwable error) { - try { - if (delegate != null) { - delegate.error(marker, message, error); - } - } catch (Throwable ignored) { - // A diagnostic must never interrupt evaluation, readiness notification, or teardown. - } - } -} diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentInterceptor.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentInterceptor.java index 9098ce17fa6..a82e194e248 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentInterceptor.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentInterceptor.java @@ -5,6 +5,8 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import java.util.Collection; import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * {@link TraceInterceptor} that writes the accumulated {@code ffe_*} tags onto the local root span @@ -44,8 +46,7 @@ */ final class SpanEnrichmentInterceptor implements TraceInterceptor { - private static final DiagnosticLogger log = - DiagnosticLogger.getLogger(SpanEnrichmentInterceptor.class); + private static final Logger log = LoggerFactory.getLogger(SpanEnrichmentInterceptor.class); /** * Unique priority in the "trace data enrichment" band, after {@code GIT_METADATA} (3) and before diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentWriter.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentWriter.java index 342ceddcdf6..e9c1200868e 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentWriter.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/SpanEnrichmentWriter.java @@ -4,6 +4,8 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.util.concurrent.atomic.AtomicBoolean; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Agent-side owner of APM feature-flag span enrichment. This is the WRITE tier of the @@ -29,8 +31,7 @@ */ public final class SpanEnrichmentWriter implements AutoCloseable { - private static final DiagnosticLogger log = - DiagnosticLogger.getLogger(SpanEnrichmentWriter.class); + private static final Logger log = LoggerFactory.getLogger(SpanEnrichmentWriter.class); public static SpanEnrichmentWriter getInstance() { return SingletonHolder.INSTANCE; diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticIsolationTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticIsolationTest.java deleted file mode 100644 index 0f75b50ca2b..00000000000 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticIsolationTest.java +++ /dev/null @@ -1,136 +0,0 @@ -package com.datadog.featureflag; - -import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.Collections.emptyMap; -import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import ch.qos.logback.classic.Level; -import ch.qos.logback.classic.Logger; -import ch.qos.logback.classic.LoggerContext; -import ch.qos.logback.classic.turbo.TurboFilter; -import ch.qos.logback.core.spi.FilterReply; -import datadog.communication.BackendApi; -import datadog.communication.BackendApiFactory; -import datadog.communication.ddagent.SharedCommunicationObjects; -import datadog.remoteconfig.ConfigurationPoller; -import datadog.remoteconfig.Product; -import datadog.trace.api.Config; -import datadog.trace.api.featureflag.FeatureFlaggingGateway; -import datadog.trace.api.featureflag.exposure.Allocation; -import datadog.trace.api.featureflag.exposure.ExposureEvent; -import datadog.trace.api.featureflag.exposure.Flag; -import datadog.trace.api.featureflag.exposure.Subject; -import datadog.trace.api.featureflag.exposure.Variant; -import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; -import datadog.trace.api.intake.Intake; -import datadog.trace.test.util.PollingConditions; -import java.util.concurrent.ScheduledExecutorService; -import okhttp3.HttpUrl; -import org.slf4j.LoggerFactory; -import org.slf4j.Marker; -import org.tabletest.junit.TableTest; - -class DiagnosticIsolationTest { - @TableTest({ - "Scenario | Fail level check", - "backend throws | false ", - "level check throws | true " - }) - void preservesConfigurationAndDeliveryWhenLoggingFails(boolean failLevelCheck) throws Exception { - LoggerContext logging = (LoggerContext) LoggerFactory.getILoggerFactory(); - TurboFilter failure = - new TurboFilter() { - @Override - public FilterReply decide( - Marker marker, - Logger logger, - Level level, - String format, - Object[] params, - Throwable error) { - if (logger.getName().startsWith("com.datadog.featureflag") - && (failLevelCheck || format != null)) { - throw new AssertionError("broken logger"); - } - return FilterReply.NEUTRAL; - } - }; - failure.start(); - logging.addTurboFilter(failure); - FeatureFlaggingGateway.ConfigListener listener = - mock(FeatureFlaggingGateway.ConfigListener.class); - FeatureFlaggingGateway.addConfigListener(listener); - try { - Config config = mock(Config.class); - SharedCommunicationObjects sco = mock(SharedCommunicationObjects.class); - ConfigurationPoller poller = mock(ConfigurationPoller.class); - when(sco.configurationPoller(config)).thenReturn(poller); - try (RemoteConfigServiceImpl service = new RemoteConfigServiceImpl(sco, config)) { - service.init(); - ServerConfiguration parsed = - UniversalFlagConfigParser.INSTANCE.deserialize( - "{\"flags\":{\"bad\":42}}".getBytes(UTF_8)); - assertEquals(1, parsed.invalidFlags.size()); - service.accept("config", parsed, null); - verify(listener).accept(parsed); - } - verify(poller).removeListeners(Product.FFE_FLAGS); - verify(poller).stop(); - - AgentlessConfigurationSource.UfcHttpClient client = - mock(AgentlessConfigurationSource.UfcHttpClient.class); - ScheduledExecutorService executor = mock(ScheduledExecutorService.class); - HttpUrl endpoint = HttpUrl.get("http://localhost/config"); - when(client.fetch(eq(endpoint), eq(config), any())) - .thenReturn(new AgentlessConfigurationSource.UfcHttpResponse(503, null, null)) - .thenReturn( - new AgentlessConfigurationSource.UfcHttpResponse( - 200, - "v1", - "{\"data\":{\"type\":\"universal-flag-configuration\",\"attributes\":{\"flags\":{}}}}" - .getBytes(UTF_8))); - try (AgentlessConfigurationSource source = - new AgentlessConfigurationSource(endpoint, config, 1000, client, executor)) { - source.init(); - assertTrue(source.pollOnce()); - } - verify(client).cancel(); - verify(executor).shutdownNow(); - - BackendApi backend = mock(BackendApi.class); - BackendApiFactory backendFactory = mock(BackendApiFactory.class); - when(backendFactory.createEvpProxyApi(Intake.EVENT_PLATFORM, true)).thenReturn(backend); - FeatureFlagBackendApiFactory factory = - new FeatureFlagBackendApiFactory(config, backendFactory, FeatureFlagEventType.EXPOSURE); - ExposureWriterImpl writer = new ExposureWriterImpl(16, 1, MILLISECONDS, factory, config); - PollingConditions poll = new PollingConditions(5); - try { - writer.init(); - writer.accept( - new ExposureEvent( - 1, - new Allocation("allocation"), - new Flag("flag"), - new Variant("variant"), - new Subject("user", emptyMap()))); - poll.eventually(() -> verify(backend).post(any(), any(), any(), any(), eq(false))); - } finally { - writer.close(); - } - poll.eventually(() -> assertFalse(writer.isSerializerThreadAlive())); - } finally { - FeatureFlaggingGateway.removeConfigListener(listener); - FeatureFlaggingGateway.dispatch((ServerConfiguration) null); - logging.getTurboFilterList().remove(failure); - failure.stop(); - } - } -} diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticLoggerTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticLoggerTest.java deleted file mode 100644 index 47ef63e32a7..00000000000 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/DiagnosticLoggerTest.java +++ /dev/null @@ -1,88 +0,0 @@ -package com.datadog.featureflag; - -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertFalse; -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.verify; -import static org.mockito.Mockito.when; - -import java.lang.reflect.Constructor; -import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.Marker; -import org.slf4j.helpers.NOPLogger; - -class DiagnosticLoggerTest { - private final Marker marker = mock(Marker.class); - private final Throwable error = new IllegalStateException("diagnostic failure"); - - @Test - void forwardsMessagesAndArgumentsToTheBackend() throws Exception { - Logger backend = mock(Logger.class); - when(backend.isDebugEnabled()).thenReturn(true); - DiagnosticLogger logger = loggerWithBackend(backend); - - assertTrue(logger.isDebugEnabled()); - assertSame(backend, logger.backend()); - logAllLevels(logger); - - verify(backend).debug("message"); - verify(backend).debug("one {}", "first"); - verify(backend).debug("two {} {}", "first", "second"); - verify(backend).debug("three {} {} {}", "first", "second", "third"); - verify(backend).warn("warning"); - verify(backend).warn("warning {}", new Object[] {"argument"}); - verify(backend).error("error", error); - verify(backend).error(marker, "marked error", error); - } - - @Test - void respectsTheBackendDebugLevel() throws Exception { - Logger backend = mock(Logger.class); - assertFalse(loggerWithBackend(backend).isDebugEnabled()); - verify(backend).isDebugEnabled(); - } - - @Test - void toleratesAnUnavailableBackend() throws Exception { - DiagnosticLogger logger = loggerWithBackend(null); - - assertFalse(logger.isDebugEnabled()); - assertSame(NOPLogger.NOP_LOGGER, logger.backend()); - assertDoesNotThrow(() -> logAllLevels(logger)); - } - - @Test - void containsFailuresFromEveryLoggingOverloadAndLevelCheck() throws Exception { - Logger backend = - mock( - Logger.class, - invocation -> { - throw new AssertionError("broken backend"); - }); - DiagnosticLogger logger = loggerWithBackend(backend); - - assertFalse(logger.isDebugEnabled()); - assertDoesNotThrow(() -> logAllLevels(logger)); - } - - private void logAllLevels(DiagnosticLogger logger) { - logger.debug("message"); - logger.debug("one {}", "first"); - logger.debug("two {} {}", "first", "second"); - logger.debug("three {} {} {}", "first", "second", "third"); - logger.warn("warning"); - logger.warn("warning {}", "argument"); - logger.error("error", error); - logger.error(marker, "marked error", error); - } - - private static DiagnosticLogger loggerWithBackend(Logger backend) throws Exception { - Constructor constructor = - DiagnosticLogger.class.getDeclaredConstructor(Logger.class); - constructor.setAccessible(true); - return constructor.newInstance(backend); - } -} From 3bb725fe58d7204b01f41d8760f4a103db991d94 Mon Sep 17 00:00:00 2001 From: "ethan.romero" Date: Tue, 27 Oct 2026 15:01:36 +0000 Subject: [PATCH 5/7] Cover feature flag remote config diagnostics Assert the debug logs that the Remote Configuration service emits when it starts, stops, receives a document and sees it removed. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5.5 --- .../RemoteConfigServiceImplTest.java | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) 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 6bac29d44f9..1dc96f1cf2b 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 @@ -15,6 +15,10 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import datadog.remoteconfig.Capabilities; import datadog.remoteconfig.ConfigurationDeserializer; import datadog.remoteconfig.ConfigurationPoller; @@ -28,8 +32,12 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeoutException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; +import org.slf4j.LoggerFactory; class RemoteConfigServiceImplTest { private static final byte[] DOCUMENT = "{\"flags\":{}}".getBytes(UTF_8); @@ -178,4 +186,57 @@ void closeUnregistersTheProductAndDropsSubscribers() { verify(poller, never()).stop(); assertEquals(0, received.size()); } + + @Nested + class Diagnostics { + private final Logger logger = (Logger) LoggerFactory.getLogger(RemoteConfigServiceImpl.class); + private final ListAppender appender = new ListAppender<>(); + private Level previousLevel; + private RemoteConfigServiceImpl service; + + @BeforeEach + void captureLogs() { + previousLevel = logger.getLevel(); + logger.setLevel(Level.DEBUG); + appender.start(); + logger.addAppender(appender); + service = new RemoteConfigServiceImpl(mock(ConfigurationPoller.class)); + } + + @AfterEach + void restoreLogging() { + logger.detachAppender(appender); + appender.stop(); + logger.setLevel(previousLevel); + } + + @Test + void reportsLifecycle() { + service.start(); + service.close(); + + assertLogged("starting remote_config configuration source"); + assertLogged("remote_config configuration source stopped"); + } + + @Test + void reportsReceivedConfiguration() { + service.accept("config", DOCUMENT, mock(PollingRateHinter.class)); + + assertLogged("configuration received: key=config, bytes=" + DOCUMENT.length); + } + + @Test + void reportsRemovedConfiguration() { + service.accept("config", null, mock(PollingRateHinter.class)); + + assertLogged("configuration removed: key=config"); + } + + private void assertLogged(final String text) { + assertTrue( + appender.list.stream().anyMatch(e -> e.getFormattedMessage().contains(text)), + "Missing log: " + text); + } + } } From 8d5e49d1de36cfb776d28f962b938a51fefe131a Mon Sep 17 00:00:00 2001 From: "ethan.romero" Date: Tue, 27 Oct 2026 19:04:24 +0000 Subject: [PATCH 6/7] Log remote config documents after delivering them Move the received and removed logs of the Remote Configuration service after the subscribers are notified, so that a failing logging backend cannot keep a configuration document from being delivered. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5.5 --- .../featureflag/RemoteConfigServiceImpl.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 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 894603557ef..a0aae50ed9a 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 @@ -83,14 +83,6 @@ public void accept( final String configKey, @Nullable final byte[] content, final PollingRateHinter pollingRateHinter) { - if (content == null) { - LOGGER.debug("Feature Flagging remote_config configuration removed: key={}", configKey); - } else { - LOGGER.debug( - "Feature Flagging remote_config configuration received: key={}, bytes={}", - configKey, - content.length); - } final Iterator> snapshot; synchronized (this.subscribers) { this.current = content; @@ -104,5 +96,14 @@ public void accept( LOGGER.debug("Feature Flags configuration subscriber failed", e); } } + // Logged last: a diagnostic must not get in the way of the delivery. + if (content == null) { + LOGGER.debug("Feature Flagging remote_config configuration removed: key={}", configKey); + } else { + LOGGER.debug( + "Feature Flagging remote_config configuration received: key={}, bytes={}", + configKey, + content.length); + } } } From e71a07a424e84a641b6703d705189099eea41ba6 Mon Sep 17 00:00:00 2001 From: "ethan.romero" Date: Tue, 27 Oct 2026 19:17:45 +0000 Subject: [PATCH 7/7] Check that remote config documents are logged after delivery Assert, with a subscriber, that the received log of the Remote Configuration service comes after the document is delivered. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5.5 --- .../RemoteConfigServiceImplTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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 1dc96f1cf2b..ec70db6d9f8 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 @@ -5,6 +5,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -32,6 +33,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; @@ -233,6 +235,21 @@ void reportsRemovedConfiguration() { assertLogged("configuration removed: key=config"); } + @Test + void reportsTheDocumentOnlyOnceItIsDelivered() { + final AtomicBoolean loggedBeforeDelivery = new AtomicBoolean(true); + service.subscribe( + content -> + loggedBeforeDelivery.set( + appender.list.stream() + .anyMatch(e -> e.getFormattedMessage().contains("configuration received")))); + + service.accept("config", DOCUMENT, mock(PollingRateHinter.class)); + + assertFalse(loggedBeforeDelivery.get(), "The log must not delay the delivery"); + assertLogged("configuration received: key=config, bytes=" + DOCUMENT.length); + } + private void assertLogged(final String text) { assertTrue( appender.list.stream().anyMatch(e -> e.getFormattedMessage().contains(text)),