From 2940f6aa6e28b1d20d6d2e02fbef5168f25ecbae Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 17 Sep 2026 16:07:09 +0200 Subject: [PATCH 01/21] Expose OTel thread/process context without requiring profiling - Add Config.isDatadogProfilerSafeAndConfigured() as the raw ddprof env-safety/explicit-flag predicate, without the isProfilingEnabled() AND-prefix - Add Config.isOtelContextExposureEnabled(), defaulting to enabled when the profiler is safe/configured and either profiling is enabled or AppSec is fully enabled, with an explicit DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED override - Gate Agent.createProfilingContextIntegration()'s ddprof branch on the new flag (additive, ORed with the existing profiling gate) and reflectively register the process context even when profiling never starts - Add TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED to OtlpConfig and supported-configurations.json --- .../java/datadog/trace/bootstrap/Agent.java | 52 +++++--- .../profiling/agent/ProcessContextTest.java | 87 ++++++++++++ .../datadog/trace/api/config/OtlpConfig.java | 10 ++ .../main/java/datadog/trace/api/Config.java | 41 ++++++ .../api/ConfigOtelContextExposureTest.java | 124 ++++++++++++++++++ metadata/supported-configurations.json | 8 ++ 6 files changed, 303 insertions(+), 19 deletions(-) create mode 100644 internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index 1180c983990..dc6974ddf66 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -53,6 +53,7 @@ import datadog.trace.api.profiling.ProfilingEnablement; import datadog.trace.api.scopemanager.ScopeListener; import datadog.trace.bootstrap.benchmark.StaticEventLogger; +import datadog.trace.bootstrap.config.provider.ConfigProvider; import datadog.trace.bootstrap.config.provider.StableConfigSource; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI; @@ -1490,29 +1491,42 @@ public void withTracer(TracerAPI tracer) { * on JFR. */ private static ProfilingContextIntegration createProfilingContextIntegration() { - if (Config.get().isProfilingEnabled()) { - if (Config.get().isDatadogProfilerEnabled() && !OperatingSystem.isWindows()) { + Config config = Config.get(); + // isDatadogProfilerEnabled() is ORed in explicitly so an explicit + // DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED=false never disables ddprof for a user where real + // profiling already enabled it - the new flag is additive, not a replacement gate. + if ((config.isDatadogProfilerEnabled() || config.isOtelContextExposureEnabled()) + && !OperatingSystem.isWindows()) { + try { + ProfilingContextIntegration integration = + (ProfilingContextIntegration) + AGENT_CLASSLOADER + .loadClass("com.datadog.profiling.ddprof.DatadogProfilingIntegration") + .getDeclaredConstructor() + .newInstance(); try { - return (ProfilingContextIntegration) - AGENT_CLASSLOADER - .loadClass("com.datadog.profiling.ddprof.DatadogProfilingIntegration") - .getDeclaredConstructor() - .newInstance(); + AGENT_CLASSLOADER + .loadClass("com.datadog.profiling.agent.ProcessContext") + .getMethod("register", ConfigProvider.class) + .invoke(null, ConfigProvider.getInstance()); } catch (Throwable t) { - log.debug("ddprof-based profiling context labeling not available. {}", t.getMessage()); + log.debug("Process context registration not available. {}", t.getMessage()); } + return integration; + } catch (Throwable t) { + log.debug("ddprof-based profiling context labeling not available. {}", t.getMessage()); } - if (Config.get().isProfilingTimelineEventsEnabled()) { - // important: note that this will not initialise JFR until onStart is called - try { - return (ProfilingContextIntegration) - AGENT_CLASSLOADER - .loadClass("com.datadog.profiling.controller.openjdk.JFREventContextIntegration") - .getDeclaredConstructor() - .newInstance(); - } catch (Throwable t) { - log.debug("JFR event-based profiling context labeling not available. {}", t.getMessage()); - } + } + if (config.isProfilingEnabled() && config.isProfilingTimelineEventsEnabled()) { + // important: note that this will not initialise JFR until onStart is called + try { + return (ProfilingContextIntegration) + AGENT_CLASSLOADER + .loadClass("com.datadog.profiling.controller.openjdk.JFREventContextIntegration") + .getDeclaredConstructor() + .newInstance(); + } catch (Throwable t) { + log.debug("JFR event-based profiling context labeling not available. {}", t.getMessage()); } } return ProfilingContextIntegration.NoOp.INSTANCE; diff --git a/dd-java-agent/agent-profiling/src/test/java/com/datadog/profiling/agent/ProcessContextTest.java b/dd-java-agent/agent-profiling/src/test/java/com/datadog/profiling/agent/ProcessContextTest.java index da718cf0a4e..9e5013831e2 100644 --- a/dd-java-agent/agent-profiling/src/test/java/com/datadog/profiling/agent/ProcessContextTest.java +++ b/dd-java-agent/agent-profiling/src/test/java/com/datadog/profiling/agent/ProcessContextTest.java @@ -14,11 +14,25 @@ import datadog.trace.api.Config; import datadog.trace.api.config.ProfilingConfig; import datadog.trace.bootstrap.config.provider.ConfigProvider; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.Arrays; +import java.util.Collections; import java.util.LinkedHashSet; import org.junit.jupiter.api.Test; import org.mockito.MockedStatic; +/** + * Tests {@link ProcessContext#register(ConfigProvider)}, the entry point that publishes the process + * context attributes to the ddprof native {@link OTelContext}. + * + *

This method has two independent callers: {@code ProfilingAgent.run()} (the historical caller, + * only reachable when profiling is enabled) and {@code Agent.createProfilingContextIntegration()}, + * which now invokes it reflectively whenever OTel context exposure is enabled - including the + * AppSec-only, profiling-disabled case where {@code ProfilingAgent.run()} never executes. The + * signature {@code register(ConfigProvider)} is pinned by a GraalVM {@code @Substitute} and by that + * reflective lookup, so it must not change. + */ class ProcessContextTest { @Test @@ -91,6 +105,79 @@ void testEnabledByDefault() { assertTrue(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED_DEFAULT); } + /** + * Covers the call path introduced for AppSec-only deployments: {@code + * Agent.createProfilingContextIntegration()} reflectively calls {@code register(ConfigProvider)} + * when OTel context exposure is enabled, even though {@code profiling.enabled} is {@code false} + * and {@code ProfilingAgent.run()} is therefore never executed. + * + *

The process context gate is {@code profiling.process.context.enabled} alone: {@code + * register} never reads {@code Config#isProfilingEnabled()}, so it is already profiling-agnostic. + * This test pins that property by stubbing {@code isProfilingEnabled()} to {@code false} and + * asserting the native context is still fully initialized, so a future change that made process + * context depend on the profiler being enabled would break the new caller here rather than + * silently in production. + */ + @Test + void testRegisterWorksIndependentlyOfProfilingEnabledState() { + ConfigProvider configProvider = mock(ConfigProvider.class); + when(configProvider.getBoolean( + eq(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED), + eq(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED_DEFAULT))) + .thenReturn(true); + when(configProvider.getSet(eq(ProfilingConfig.PROFILING_CONTEXT_ATTRIBUTES), any())) + .thenReturn(new LinkedHashSet<>(Collections.singletonList("http.route"))); + + Config config = mock(Config.class); + when(config.isProfilingEnabled()).thenReturn(false); + when(config.getEnv()).thenReturn("appsec-env"); + when(config.getHostName()).thenReturn("appsec-host"); + when(config.getRuntimeId()).thenReturn("appsec-runtime-id"); + when(config.getServiceName()).thenReturn("appsec-service"); + when(config.getRuntimeVersion()).thenReturn("appsec-runtime-version"); + when(config.getVersion()).thenReturn("appsec-version"); + + OTelContext otelContext = mock(OTelContext.class); + DdprofLibraryLoader.OTelContextHolder holder = + mock(DdprofLibraryLoader.OTelContextHolder.class); + when(holder.getReasonNotLoaded()).thenReturn(null); + when(holder.getComponent()).thenReturn(otelContext); + + try (MockedStatic configMock = mockStatic(Config.class); + MockedStatic ddprofMock = mockStatic(DdprofLibraryLoader.class)) { + + configMock.when(Config::get).thenReturn(config); + ddprofMock.when(DdprofLibraryLoader::otelContext).thenReturn(holder); + + ProcessContext.register(configProvider); + + verify(otelContext) + .initializeAllContext( + eq("appsec-env"), + eq("appsec-host"), + eq("appsec-runtime-id"), + eq("appsec-service"), + eq("appsec-runtime-version"), + eq("appsec-version"), + aryEq(new String[] {"http.route"})); + } + } + + /** + * The reflective call site in {@code Agent.createProfilingContextIntegration()} looks up {@code + * register} by the exact signature {@code register(ConfigProvider)} and is unable to fail at + * compile time if that signature changes. The same signature is pinned by a GraalVM + * {@code @Substitute}. This test fails fast if the method is renamed or its parameter type + * changes. + */ + @Test + void testRegisterSignatureIsStableForReflectiveLookup() throws NoSuchMethodException { + Method register = ProcessContext.class.getMethod("register", ConfigProvider.class); + + assertTrue(Modifier.isStatic(register.getModifiers())); + assertTrue(Modifier.isPublic(register.getModifiers())); + } + @Test void testRegisterHandlesLibraryLoadFailure() { ConfigProvider configProvider = mock(ConfigProvider.class); diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/OtlpConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/OtlpConfig.java index 90fd9fd046f..3f664d604fc 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/OtlpConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/OtlpConfig.java @@ -36,6 +36,16 @@ public final class OtlpConfig { public static final String TRACE_OTEL_ENABLED = "trace.otel.enabled"; public static final String TRACE_OTEL_EXPORTER = "trace.otel.exporter"; + /** + * Enables exposing the OpenTelemetry thread and process context to external consumers (eBPF/CWS) + * through the Datadog profiler native library, independently of profiling being enabled. + * + *

Environment variable: {@code DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED}. When unset, the value + * is computed dynamically from the profiling and AppSec activation levels. + */ + public static final String TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED = + "trace.otel.context-exposure.enabled"; + public static final String OTLP_TRACES_ENDPOINT = "otlp.traces.endpoint"; public static final String OTLP_TRACES_HEADERS = "otlp.traces.headers"; public static final String OTLP_TRACES_PROTOCOL = "otlp.traces.protocol"; diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 20d9d0509e5..a930d08bc24 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -508,6 +508,7 @@ import static datadog.trace.api.config.OtlpConfig.OTLP_TRACES_HEADERS; import static datadog.trace.api.config.OtlpConfig.OTLP_TRACES_PROTOCOL; import static datadog.trace.api.config.OtlpConfig.OTLP_TRACES_TIMEOUT; +import static datadog.trace.api.config.OtlpConfig.TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED; import static datadog.trace.api.config.OtlpConfig.TRACE_OTEL_EXPORTER; import static datadog.trace.api.config.ProfilingConfig.PROFILING_AGENTLESS; import static datadog.trace.api.config.ProfilingConfig.PROFILING_AGENTLESS_DEFAULT; @@ -1080,6 +1081,7 @@ public static String getHostName() { private final ProfilingEnablement profilingEnabled; private final boolean profilingAgentless; private final boolean isDatadogProfilerEnabled; + private final boolean otelContextExposureEnabled; @Deprecated private final String profilingUrl; private final Map profilingTags; private final int profilingStartDelay; @@ -2642,6 +2644,21 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) ? traceResourceRenamingExplicit : instrumenterConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED; + // OpenTelemetry thread/process context exposure configuration + // Default: enabled when the Datadog profiler is safe and configured, and either profiling is + // enabled or AppSec is fully enabled + // Can be explicitly overridden by setting DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED, but an + // explicit true still requires isDatadogProfilerSafeAndConfigured(): the native-image/J9/JDK8 + // exclusions it carries must never be bypassable by a user-set flag. + Boolean otelContextExposureExplicit = + configProvider.getBoolean(TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED); + this.otelContextExposureEnabled = + otelContextExposureExplicit != null + ? otelContextExposureExplicit && isDatadogProfilerSafeAndConfigured() + : isDatadogProfilerSafeAndConfigured() + && (isProfilingEnabled() + || instrumenterConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED); + this.traceResourceRenamingAlwaysSimplifiedEndpoint = configProvider.getBoolean(TRACE_RESOURCE_RENAMING_ALWAYS_SIMPLIFIED_ENDPOINT, false); @@ -4240,6 +4257,28 @@ public boolean isDatadogProfilerEnabled() { return isProfilingEnabled() && isDatadogProfilerEnabled; } + /** + * The raw Datadog-profiler env-safety and explicit-flag predicate, without the {@link + * #isProfilingEnabled()} AND-prefix applied by {@link #isDatadogProfilerEnabled()}. Exposed as + * its own getter so other call sites (for example {@link #isOtelContextExposureEnabled()}) can + * reuse the same native-image/J9/JDK8 exclusions without re-deriving them or accidentally + * depending on {@code isProfilingEnabled()}. + */ + public boolean isDatadogProfilerSafeAndConfigured() { + return isDatadogProfilerEnabled; + } + + /** + * Whether the OpenTelemetry thread and process context should be exposed through the Datadog + * profiler native library, so external consumers (for example eBPF/CWS) can read the span context + * of a JVM. Defaults to enabled when the Datadog profiler is safe and configured and either + * profiling is enabled or AppSec is fully enabled; can be explicitly overridden with {@code + * DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED}. + */ + public boolean isOtelContextExposureEnabled() { + return otelContextExposureEnabled; + } + public static boolean isDatadogProfilerEnablementOverridden() { // old non-LTS versions without important backports // also, we have no windows binaries @@ -6780,6 +6819,8 @@ public String toString() { + profilingExceptionHistogramMaxCollectionSize + ", profilingExcludeAgentThreads=" + profilingExcludeAgentThreads + + ", otelContextExposureEnabled=" + + otelContextExposureEnabled + ", crashTrackingTags=" + crashTrackingTags + ", crashTrackingAgentless=" diff --git a/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java new file mode 100644 index 00000000000..c7a2c343287 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java @@ -0,0 +1,124 @@ +package datadog.trace.api; + +import static datadog.trace.api.config.AppSecConfig.APPSEC_ENABLED; +import static datadog.trace.api.config.OtlpConfig.TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_DATADOG_PROFILER_ENABLED; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_ENABLED; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import datadog.trace.test.junit.utils.config.WithConfig; +import datadog.trace.test.junit.utils.config.WithConfigExtension; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +/** Tests the resolution of {@link Config#isOtelContextExposureEnabled()}. */ +@ExtendWith(WithConfigExtension.class) +class ConfigOtelContextExposureTest { + + /** + * The Datadog profiler raw predicate is vetoed outright on platforms and JVM versions that cannot + * run it, regardless of any explicit opt-in. Tests that expect the feature to be enabled are only + * meaningful on a JVM where that veto does not apply. + */ + private static void assumeDatadogProfilerNotVetoed() { + assumeTrue( + !Config.isDatadogProfilerEnablementOverridden(), + "Datadog profiler is unavailable on this platform/JVM version"); + } + + @Test + void disabledByDefault() { + assertFalse(Config.get().isOtelContextExposureEnabled()); + } + + @Test + @WithConfig(key = TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED, value = "false") + @WithConfig(key = PROFILING_ENABLED, value = "true") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") + void explicitFalseOverridesConditionsThatWouldEnableIt() { + assertFalse(Config.get().isOtelContextExposureEnabled()); + } + + @Test + @WithConfig(key = TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED, value = "true") + @WithConfig(key = PROFILING_ENABLED, value = "false") + @WithConfig(key = APPSEC_ENABLED, value = "false") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") + void explicitTrueOverridesProfilingAndAppSecActivationLevel() { + assumeDatadogProfilerNotVetoed(); + + assertTrue(Config.get().isOtelContextExposureEnabled()); + } + + /** + * An explicit {@code true} overrides the profiling/AppSec activation-level conditions, but it + * must never bypass {@link Config#isDatadogProfilerSafeAndConfigured()} - that predicate carries + * the native-image/J9/JDK8-aarch64 exclusions, and a user-set flag must not be able to force + * ddprof context labeling on an environment where the Datadog profiler cannot run safely. + */ + @Test + @WithConfig(key = TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED, value = "true") + @WithConfig(key = PROFILING_ENABLED, value = "false") + @WithConfig(key = APPSEC_ENABLED, value = "false") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "false") + void explicitTrueDoesNotBypassDatadogProfilerSafetyPredicate() { + assertFalse(Config.get().isOtelContextExposureEnabled()); + } + + @Test + @WithConfig(key = PROFILING_ENABLED, value = "true") + @WithConfig(key = APPSEC_ENABLED, value = "false") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") + void enabledWhenProfilingIsEnabled() { + assumeDatadogProfilerNotVetoed(); + + assertTrue(Config.get().isOtelContextExposureEnabled()); + } + + @Test + @WithConfig(key = APPSEC_ENABLED, value = "true") + @WithConfig(key = PROFILING_ENABLED, value = "false") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") + void enabledWhenAppSecIsFullyEnabledWithoutProfiling() { + assumeDatadogProfilerNotVetoed(); + + Config config = Config.get(); + assertFalse(config.isProfilingEnabled()); + assertTrue(config.isOtelContextExposureEnabled()); + } + + @Test + @WithConfig(key = APPSEC_ENABLED, value = "inactive") + @WithConfig(key = PROFILING_ENABLED, value = "false") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") + void disabledWhenAppSecIsOnlyEnabledInactive() { + assertFalse(Config.get().isOtelContextExposureEnabled()); + } + + @Test + @WithConfig(key = APPSEC_ENABLED, value = "true") + @WithConfig(key = PROFILING_ENABLED, value = "false") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "false") + void disabledWhenDatadogProfilerIsExplicitlyDisabled() { + assertFalse(Config.get().isOtelContextExposureEnabled()); + } + + /** + * An environment where the Datadog profiler cannot run (unsupported JVM version, Windows, GraalVM + * native image) makes the raw ddprof predicate {@code false}. That environment detection reads + * real, cached JVM and OS state that a unit test running on a normal JVM cannot fake, and there + * is no existing test helper in this module to stub it. This test therefore drives the identical + * boolean short-circuit through the {@code DD_PROFILING_DDPROF_ENABLED=false} env variable: from + * {@link Config}'s point of view an environment-detected "unsafe" and an explicit "false" + * collapse into the same raw-predicate value, so the downstream effect on {@link + * Config#isOtelContextExposureEnabled()} is the same. + */ + @Test + @WithConfig(key = "APPSEC_ENABLED", value = "true", env = true) + @WithConfig(key = "PROFILING_DDPROF_ENABLED", value = "false", env = true) + void disabledInAnEnvironmentWhereTheDatadogProfilerIsUnsafe() { + assertFalse(Config.get().isOtelContextExposureEnabled()); + } +} diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index b443c1fbd66..7e2c95b254c 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -8812,6 +8812,14 @@ "aliases": ["DD_TRACE_INTEGRATION_OSGI_ENABLED", "DD_INTEGRATION_OSGI_ENABLED"] } ], + "DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED": [ + { + "version": "A", + "type": "boolean", + "default": null, + "aliases": [] + } + ], "DD_TRACE_OTEL_ENABLED": [ { "version": "A", From 113eaf9776c52590d10b24cafaade2fc1bdf355f Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 18 Sep 2026 09:26:01 +0200 Subject: [PATCH 02/21] Defer ddprof context integration construction past premain for AppSec-only trigger Constructing DatadogProfilingIntegration touches java.nio.file (via TempLocationManager) and loads the ddprof native library, which must not happen on the primordial premain thread. Users with the Datadog profiler enabled were unaffected (they already ran this synchronously), but the new AppSec-only trigger reached this construction from premain for the first time. DeferredProfilingContextIntegration wraps the real integration behind a NoOp delegate until AgentTaskScheduler runs the deferred construction off the premain thread, then swaps it in. The profiler-enabled path keeps the exact synchronous behavior it had before, since profiling accuracy needs every scope from the first one. Addresses a P1 finding from the Codex review on this PR. --- .../java/datadog/trace/bootstrap/Agent.java | 71 +++++-- .../DeferredProfilingContextIntegration.java | 146 ++++++++++++++ ...ferredProfilingContextIntegrationTest.java | 189 ++++++++++++++++++ 3 files changed, 389 insertions(+), 17 deletions(-) create mode 100644 dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java create mode 100644 dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index dc6974ddf66..6fb411077ee 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -73,6 +73,7 @@ import java.net.URL; import java.security.CodeSource; import java.util.EnumSet; +import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.regex.PatternSyntaxException; @@ -1497,24 +1498,15 @@ private static ProfilingContextIntegration createProfilingContextIntegration() { // profiling already enabled it - the new flag is additive, not a replacement gate. if ((config.isDatadogProfilerEnabled() || config.isOtelContextExposureEnabled()) && !OperatingSystem.isWindows()) { - try { - ProfilingContextIntegration integration = - (ProfilingContextIntegration) - AGENT_CLASSLOADER - .loadClass("com.datadog.profiling.ddprof.DatadogProfilingIntegration") - .getDeclaredConstructor() - .newInstance(); - try { - AGENT_CLASSLOADER - .loadClass("com.datadog.profiling.agent.ProcessContext") - .getMethod("register", ConfigProvider.class) - .invoke(null, ConfigProvider.getInstance()); - } catch (Throwable t) { - log.debug("Process context registration not available. {}", t.getMessage()); - } + // When the ddprof integration is triggered by context exposure alone (profiling disabled), + // its construction is deferred off the premain thread: it loads the ddprof native library + // and touches java.nio.file, which must not happen on the primordial premain thread. Users + // with the profiler actually enabled keep the synchronous path, since profiling accuracy + // requires seeing every scope from the very first one. + ProfilingContextIntegration integration = + createDdprofContextIntegration(AGENT_CLASSLOADER, !config.isDatadogProfilerEnabled()); + if (integration != null) { return integration; - } catch (Throwable t) { - log.debug("ddprof-based profiling context labeling not available. {}", t.getMessage()); } } if (config.isProfilingEnabled() && config.isProfilingTimelineEventsEnabled()) { @@ -1532,6 +1524,51 @@ private static ProfilingContextIntegration createProfilingContextIntegration() { return ProfilingContextIntegration.NoOp.INSTANCE; } + /** + * Creates the ddprof-based profiling context integration, either synchronously or deferred off + * the calling thread. + * + * @param classLoader the agent class loader used to reach the profiling classes. + * @param deferInitialization when true, the integration (and the process context registration + * that follows it) is constructed on an {@link AgentTaskScheduler} thread instead of the + * caller's, which during premain is the JVM's primordial thread. + * @return the integration, or {@code null} if a synchronous construction failed, in which case + * the caller falls back to the other integrations. + */ + static ProfilingContextIntegration createDdprofContextIntegration( + final ClassLoader classLoader, final boolean deferInitialization) { + Callable factory = + () -> { + ProfilingContextIntegration integration = + (ProfilingContextIntegration) + classLoader + .loadClass("com.datadog.profiling.ddprof.DatadogProfilingIntegration") + .getDeclaredConstructor() + .newInstance(); + try { + classLoader + .loadClass("com.datadog.profiling.agent.ProcessContext") + .getMethod("register", ConfigProvider.class) + .invoke(null, ConfigProvider.getInstance()); + } catch (Throwable t) { + log.debug("Process context registration not available. {}", t.getMessage()); + } + return integration; + }; + if (deferInitialization) { + DeferredProfilingContextIntegration deferred = + new DeferredProfilingContextIntegration("ddprof", factory); + deferred.scheduleInitialization(); + return deferred; + } + try { + return factory.call(); + } catch (Throwable t) { + log.debug("ddprof-based profiling context labeling not available. {}", t.getMessage()); + return null; + } + } + private static boolean startProfilingAgent( final boolean earlyStart, final boolean firstAttempt, Instrumentation inst) { if (isAwsLambdaRuntime()) { diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java new file mode 100644 index 00000000000..9d9efa851e8 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java @@ -0,0 +1,146 @@ +package datadog.trace.bootstrap; + +import datadog.trace.api.EndpointTracker; +import datadog.trace.api.Stateful; +import datadog.trace.api.profiling.ProfilingContextAttribute; +import datadog.trace.api.profiling.ProfilingScope; +import datadog.trace.api.profiling.Timing; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.ProfilerContext; +import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration; +import datadog.trace.util.AgentTaskScheduler; +import java.util.concurrent.Callable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A {@link ProfilingContextIntegration} that can be handed out synchronously during {@code premain} + * while the real integration is constructed later, off the premain thread. + * + *

Constructing the ddprof-based integration loads the ddprof native library and touches {@code + * java.nio.file} (through {@code TempLocationManager}), which must not happen on the JVM's + * primordial premain thread: it can lock in the default filesystem provider before the application + * has a chance to configure one in {@code main}. This wrapper keeps the premain thread free of that + * work by delegating to {@link ProfilingContextIntegration.NoOp} until the deferred construction + * completes, then swapping in the real integration. + * + *

Scope events happening before the swap are silently dropped. That is acceptable for context + * exposure (eBPF/CWS reading the current span off a thread), but not for profiling + * accuracy, so users with the Datadog profiler actually enabled keep the synchronous construction + * path. + */ +final class DeferredProfilingContextIntegration implements ProfilingContextIntegration { + private static final Logger log = + LoggerFactory.getLogger(DeferredProfilingContextIntegration.class); + + private final String name; + private final Callable factory; + + /** + * Swapped from {@link ProfilingContextIntegration.NoOp} to the real integration once the deferred + * construction succeeds. Volatile because application threads may already be running scopes when + * the swap happens. + */ + private volatile ProfilingContextIntegration delegate = ProfilingContextIntegration.NoOp.INSTANCE; + + /** + * @param name the name reported by {@link #name()}, i.e. the name of the integration being + * deferred. + * @param factory creates the real integration; invoked at most once, off the premain thread. + */ + DeferredProfilingContextIntegration( + final String name, final Callable factory) { + this.name = name; + this.factory = factory; + } + + /** Schedules the deferred construction so that it runs off the calling (premain) thread. */ + void scheduleInitialization() { + AgentTaskScheduler.get().execute(this::initialize); + } + + /** + * Runs the deferred construction. On failure this instance keeps behaving as {@link + * ProfilingContextIntegration.NoOp} forever; a background failure must never propagate. + */ + void initialize() { + try { + final ProfilingContextIntegration integration = factory.call(); + if (integration != null) { + delegate = integration; + } + } catch (final Throwable t) { + log.debug("Deferred {} profiling context labeling not available. {}", name, t.getMessage()); + } + } + + /** + * The name of the deferred integration, not of the current delegate: it is read once when the + * tracer is built, which may happen before the deferred construction completes, and it must + * describe the integration that is being installed. + */ + @Override + public String name() { + return name; + } + + @Override + public void onStart() { + delegate.onStart(); + } + + @Override + public void onAttach() { + delegate.onAttach(); + } + + @Override + public void onDetach() { + delegate.onDetach(); + } + + @Override + public Stateful newScopeState(final ProfilerContext profilerContext) { + return delegate.newScopeState(profilerContext); + } + + @Override + public int encode(final CharSequence constant) { + return delegate.encode(constant); + } + + @Override + public int encodeOperationName(final CharSequence constant) { + return delegate.encodeOperationName(constant); + } + + @Override + public int encodeResourceName(final CharSequence constant) { + return delegate.encodeResourceName(constant); + } + + @Override + public ProfilingContextAttribute createContextAttribute(final String attribute) { + return delegate.createContextAttribute(attribute); + } + + @Override + public ProfilingScope newScope() { + return delegate.newScope(); + } + + @Override + public void onRootSpanFinished(final AgentSpan rootSpan, final EndpointTracker tracker) { + delegate.onRootSpanFinished(rootSpan, tracker); + } + + @Override + public EndpointTracker onRootSpanStarted(final AgentSpan rootSpan) { + return delegate.onRootSpanStarted(rootSpan); + } + + @Override + public Timing start(final TimerType type) { + return delegate.start(type); + } +} diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java new file mode 100644 index 00000000000..458eb9dea56 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java @@ -0,0 +1,189 @@ +package datadog.trace.bootstrap; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.api.EndpointTracker; +import datadog.trace.api.Stateful; +import datadog.trace.api.profiling.ProfilingContextAttribute; +import datadog.trace.api.profiling.ProfilingScope; +import datadog.trace.api.profiling.Timer.TimerType; +import datadog.trace.api.profiling.Timing; +import datadog.trace.bootstrap.config.provider.ConfigProvider; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.ProfilerContext; +import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Covers the premain-timing contract of the ddprof profiling context integration: the AppSec-only + * trigger must not construct it (nor register the process context) on the calling thread, while the + * profiler-enabled path must keep doing exactly that. + */ +class DeferredProfilingContextIntegrationTest { + + @BeforeEach + void reset() { + FakeDatadogProfilingIntegration.constructions.set(0); + FakeDatadogProfilingIntegration.constructionThread.set(null); + FakeProcessContext.registrations.set(0); + FakeProcessContext.registered = new CountDownLatch(1); + FakeDatadogProfilingIntegration.gate = new CountDownLatch(0); + } + + @Test + void deferredConstructionDoesNotRunOnTheCallingThread() throws Exception { + // hold the deferred construction so the "not done synchronously" assertions cannot race with it + FakeDatadogProfilingIntegration.gate = new CountDownLatch(1); + + ProfilingContextIntegration integration = + Agent.createDdprofContextIntegration(fakeProfilingClassLoader(), true); + + // nothing was constructed synchronously on this (premain) thread + assertNotNull(integration); + assertEquals(0, FakeDatadogProfilingIntegration.constructions.get()); + assertEquals(0, FakeProcessContext.registrations.get()); + assertEquals("ddprof", integration.name()); + // ... and before the swap the wrapper behaves as a no-op + assertSame(Stateful.DEFAULT, integration.newScopeState(null)); + assertSame(ProfilingScope.NO_OP, integration.newScope()); + assertSame(Timing.NoOp.INSTANCE, integration.start(TimerType.QUEUEING)); + + FakeDatadogProfilingIntegration.gate.countDown(); + assertTrue( + FakeProcessContext.registered.await(30, TimeUnit.SECONDS), + "deferred construction never ran"); + // the deferred work happened, and it happened on another thread + assertEquals(1, FakeDatadogProfilingIntegration.constructions.get()); + assertEquals(1, FakeProcessContext.registrations.get()); + assertNotSame(Thread.currentThread(), FakeDatadogProfilingIntegration.constructionThread.get()); + assertSame(FakeDatadogProfilingIntegration.STATE, integration.newScopeState(null)); + } + + @Test + void synchronousConstructionKeepsRunningOnTheCallingThread() { + ProfilingContextIntegration integration = + Agent.createDdprofContextIntegration(fakeProfilingClassLoader(), false); + + assertTrue(integration instanceof FakeDatadogProfilingIntegration); + assertEquals(1, FakeDatadogProfilingIntegration.constructions.get()); + assertEquals(1, FakeProcessContext.registrations.get()); + assertSame(Thread.currentThread(), FakeDatadogProfilingIntegration.constructionThread.get()); + } + + @Test + void delegatesToTheRealIntegrationOnceInitialized() { + DeferredProfilingContextIntegration deferred = + new DeferredProfilingContextIntegration("ddprof", FakeDatadogProfilingIntegration::new); + + assertSame(Stateful.DEFAULT, deferred.newScopeState(null)); + + deferred.initialize(); + + assertSame(FakeDatadogProfilingIntegration.STATE, deferred.newScopeState(null)); + assertEquals("ddprof", deferred.name()); + } + + @Test + void staysNoOpWhenTheDeferredConstructionFails() { + DeferredProfilingContextIntegration deferred = + new DeferredProfilingContextIntegration( + "ddprof", + () -> { + throw new UnsatisfiedLinkError("no native library here"); + }); + + deferred.initialize(); + + assertSame(Stateful.DEFAULT, deferred.newScopeState(null)); + assertSame(ProfilingScope.NO_OP, deferred.newScope()); + assertSame(ProfilingContextAttribute.NoOp.INSTANCE, deferred.createContextAttribute("tag")); + assertSame(EndpointTracker.NO_OP, deferred.onRootSpanStarted(null)); + assertEquals(0, deferred.encode("something")); + assertEquals("ddprof", deferred.name()); + } + + private static ClassLoader fakeProfilingClassLoader() { + return new ClassLoader(null) { + @Override + public Class loadClass(final String name) throws ClassNotFoundException { + if ("com.datadog.profiling.ddprof.DatadogProfilingIntegration".equals(name)) { + return FakeDatadogProfilingIntegration.class; + } + if ("com.datadog.profiling.agent.ProcessContext".equals(name)) { + return FakeProcessContext.class; + } + return super.loadClass(name); + } + }; + } + + public static final class FakeProcessContext { + static final AtomicInteger registrations = new AtomicInteger(); + static volatile CountDownLatch registered = new CountDownLatch(1); + + public static void register(final ConfigProvider configProvider) { + registrations.incrementAndGet(); + registered.countDown(); + } + } + + public static final class FakeDatadogProfilingIntegration implements ProfilingContextIntegration { + static final Stateful STATE = + new Stateful() { + @Override + public void close() {} + + @Override + public void activate(final Object context) {} + }; + + static final AtomicInteger constructions = new AtomicInteger(); + static final AtomicReference constructionThread = new AtomicReference<>(); + static volatile CountDownLatch gate = new CountDownLatch(0); + + public FakeDatadogProfilingIntegration() { + try { + if (!gate.await(30, TimeUnit.SECONDS)) { + throw new IllegalStateException("construction gate was never released"); + } + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException(e); + } + constructions.incrementAndGet(); + constructionThread.set(Thread.currentThread()); + } + + @Override + public Stateful newScopeState(final ProfilerContext profilerContext) { + return STATE; + } + + @Override + public String name() { + return "ddprof"; + } + + @Override + public void onRootSpanFinished(final AgentSpan rootSpan, final EndpointTracker tracker) {} + + @Override + public EndpointTracker onRootSpanStarted(final AgentSpan rootSpan) { + return EndpointTracker.NO_OP; + } + + @Override + public Timing start(final TimerType type) { + return Timing.NoOp.INSTANCE; + } + } +} From 28ef43472a10ce8f1cee7978afc717d985a39e22 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 18 Sep 2026 11:14:03 +0200 Subject: [PATCH 03/21] Drop dedicated OTel context exposure config flag, derive purely from profiling/AppSec isOtelContextExposureEnabled() no longer has its own explicit override. It mirrors isProfilingEnabled(), which has no dedicated sub-flag either: the kill switch is disabling DD_PROFILING_ENABLED and DD_APPSEC_ENABLED, the same flags that already drive the derivation. This removes the DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED public config entirely, along with its metadata/supported-configurations.json entry - so there is no new config requiring Feature Parity Dashboard registration, which was causing the config-inversion-local-validation.py CI job to fail. --- .../java/datadog/trace/bootstrap/Agent.java | 6 ++-- .../datadog/trace/api/config/OtlpConfig.java | 10 ------ .../main/java/datadog/trace/api/Config.java | 27 ++++++-------- .../api/ConfigOtelContextExposureTest.java | 35 ------------------- metadata/supported-configurations.json | 8 ----- 5 files changed, 14 insertions(+), 72 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index 6fb411077ee..881f68ffc4c 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -1493,9 +1493,9 @@ public void withTracer(TracerAPI tracer) { */ private static ProfilingContextIntegration createProfilingContextIntegration() { Config config = Config.get(); - // isDatadogProfilerEnabled() is ORed in explicitly so an explicit - // DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED=false never disables ddprof for a user where real - // profiling already enabled it - the new flag is additive, not a replacement gate. + // isDatadogProfilerEnabled() is ORed in explicitly so a user with real profiling enabled keeps + // ddprof regardless of the AppSec activation level that otherwise drives + // isOtelContextExposureEnabled() - additive, not a replacement gate. if ((config.isDatadogProfilerEnabled() || config.isOtelContextExposureEnabled()) && !OperatingSystem.isWindows()) { // When the ddprof integration is triggered by context exposure alone (profiling disabled), diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/OtlpConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/OtlpConfig.java index 3f664d604fc..90fd9fd046f 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/OtlpConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/OtlpConfig.java @@ -36,16 +36,6 @@ public final class OtlpConfig { public static final String TRACE_OTEL_ENABLED = "trace.otel.enabled"; public static final String TRACE_OTEL_EXPORTER = "trace.otel.exporter"; - /** - * Enables exposing the OpenTelemetry thread and process context to external consumers (eBPF/CWS) - * through the Datadog profiler native library, independently of profiling being enabled. - * - *

Environment variable: {@code DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED}. When unset, the value - * is computed dynamically from the profiling and AppSec activation levels. - */ - public static final String TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED = - "trace.otel.context-exposure.enabled"; - public static final String OTLP_TRACES_ENDPOINT = "otlp.traces.endpoint"; public static final String OTLP_TRACES_HEADERS = "otlp.traces.headers"; public static final String OTLP_TRACES_PROTOCOL = "otlp.traces.protocol"; diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index a930d08bc24..4c8567d114d 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -508,7 +508,6 @@ import static datadog.trace.api.config.OtlpConfig.OTLP_TRACES_HEADERS; import static datadog.trace.api.config.OtlpConfig.OTLP_TRACES_PROTOCOL; import static datadog.trace.api.config.OtlpConfig.OTLP_TRACES_TIMEOUT; -import static datadog.trace.api.config.OtlpConfig.TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED; import static datadog.trace.api.config.OtlpConfig.TRACE_OTEL_EXPORTER; import static datadog.trace.api.config.ProfilingConfig.PROFILING_AGENTLESS; import static datadog.trace.api.config.ProfilingConfig.PROFILING_AGENTLESS_DEFAULT; @@ -2645,19 +2644,14 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) : instrumenterConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED; // OpenTelemetry thread/process context exposure configuration - // Default: enabled when the Datadog profiler is safe and configured, and either profiling is - // enabled or AppSec is fully enabled - // Can be explicitly overridden by setting DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED, but an - // explicit true still requires isDatadogProfilerSafeAndConfigured(): the native-image/J9/JDK8 - // exclusions it carries must never be bypassable by a user-set flag. - Boolean otelContextExposureExplicit = - configProvider.getBoolean(TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED); + // No dedicated flag: enabled whenever the Datadog profiler is safe and configured and either + // profiling is enabled or AppSec is fully enabled. A user who wants this off already has a + // kill switch through the underlying flags - DD_PROFILING_ENABLED and DD_APPSEC_ENABLED - + // the same way isProfilingEnabled() itself has no dedicated override beyond its own flag. this.otelContextExposureEnabled = - otelContextExposureExplicit != null - ? otelContextExposureExplicit && isDatadogProfilerSafeAndConfigured() - : isDatadogProfilerSafeAndConfigured() - && (isProfilingEnabled() - || instrumenterConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED); + isDatadogProfilerSafeAndConfigured() + && (isProfilingEnabled() + || instrumenterConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED); this.traceResourceRenamingAlwaysSimplifiedEndpoint = configProvider.getBoolean(TRACE_RESOURCE_RENAMING_ALWAYS_SIMPLIFIED_ENDPOINT, false); @@ -4271,9 +4265,10 @@ public boolean isDatadogProfilerSafeAndConfigured() { /** * Whether the OpenTelemetry thread and process context should be exposed through the Datadog * profiler native library, so external consumers (for example eBPF/CWS) can read the span context - * of a JVM. Defaults to enabled when the Datadog profiler is safe and configured and either - * profiling is enabled or AppSec is fully enabled; can be explicitly overridden with {@code - * DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED}. + * of a JVM. Enabled when the Datadog profiler is safe and configured and either profiling is + * enabled or AppSec is fully enabled. No dedicated override: disabling profiling and AppSec + * already disables this, the same way {@link #isProfilingEnabled()} has no override of its own + * beyond {@code DD_PROFILING_ENABLED}. */ public boolean isOtelContextExposureEnabled() { return otelContextExposureEnabled; diff --git a/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java index c7a2c343287..198f3f6ac59 100644 --- a/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java +++ b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java @@ -1,7 +1,6 @@ package datadog.trace.api; import static datadog.trace.api.config.AppSecConfig.APPSEC_ENABLED; -import static datadog.trace.api.config.OtlpConfig.TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED; import static datadog.trace.api.config.ProfilingConfig.PROFILING_DATADOG_PROFILER_ENABLED; import static datadog.trace.api.config.ProfilingConfig.PROFILING_ENABLED; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -33,40 +32,6 @@ void disabledByDefault() { assertFalse(Config.get().isOtelContextExposureEnabled()); } - @Test - @WithConfig(key = TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED, value = "false") - @WithConfig(key = PROFILING_ENABLED, value = "true") - @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") - void explicitFalseOverridesConditionsThatWouldEnableIt() { - assertFalse(Config.get().isOtelContextExposureEnabled()); - } - - @Test - @WithConfig(key = TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED, value = "true") - @WithConfig(key = PROFILING_ENABLED, value = "false") - @WithConfig(key = APPSEC_ENABLED, value = "false") - @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") - void explicitTrueOverridesProfilingAndAppSecActivationLevel() { - assumeDatadogProfilerNotVetoed(); - - assertTrue(Config.get().isOtelContextExposureEnabled()); - } - - /** - * An explicit {@code true} overrides the profiling/AppSec activation-level conditions, but it - * must never bypass {@link Config#isDatadogProfilerSafeAndConfigured()} - that predicate carries - * the native-image/J9/JDK8-aarch64 exclusions, and a user-set flag must not be able to force - * ddprof context labeling on an environment where the Datadog profiler cannot run safely. - */ - @Test - @WithConfig(key = TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED, value = "true") - @WithConfig(key = PROFILING_ENABLED, value = "false") - @WithConfig(key = APPSEC_ENABLED, value = "false") - @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "false") - void explicitTrueDoesNotBypassDatadogProfilerSafetyPredicate() { - assertFalse(Config.get().isOtelContextExposureEnabled()); - } - @Test @WithConfig(key = PROFILING_ENABLED, value = "true") @WithConfig(key = APPSEC_ENABLED, value = "false") diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index 7e2c95b254c..b443c1fbd66 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -8812,14 +8812,6 @@ "aliases": ["DD_TRACE_INTEGRATION_OSGI_ENABLED", "DD_INTEGRATION_OSGI_ENABLED"] } ], - "DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED": [ - { - "version": "A", - "type": "boolean", - "default": null, - "aliases": [] - } - ], "DD_TRACE_OTEL_ENABLED": [ { "version": "A", From 385c0292faf2b76c7b442bfb124ae894e1e88778 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 18 Sep 2026 14:38:59 +0200 Subject: [PATCH 04/21] review: pre-PR checks - Add OtelContextExposureSmokeTest verifying OTel process context registration follows AppSec activation, not profiling - Extend DeferredProfilingContextIntegrationTest to cover all delegate pass-through methods (onAttach/onDetach/encodeOperationName/encodeResourceName/onRootSpanFinished), not just newScopeState/name --- ...ferredProfilingContextIntegrationTest.java | 41 ++- .../OtelContextExposureSmokeTest.groovy | 59 ++++ decisions.md | 303 ++++++++++++++++++ .../main/java/datadog/trace/api/Config.java | 7 + 4 files changed, 409 insertions(+), 1 deletion(-) create mode 100644 dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy create mode 100644 decisions.md diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java index 458eb9dea56..b110b7d6864 100644 --- a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java @@ -37,6 +37,9 @@ void reset() { FakeProcessContext.registrations.set(0); FakeProcessContext.registered = new CountDownLatch(1); FakeDatadogProfilingIntegration.gate = new CountDownLatch(0); + FakeDatadogProfilingIntegration.onAttachCalls.set(0); + FakeDatadogProfilingIntegration.onDetachCalls.set(0); + FakeDatadogProfilingIntegration.onRootSpanFinishedCalls.set(0); } @Test @@ -90,6 +93,17 @@ void delegatesToTheRealIntegrationOnceInitialized() { assertSame(FakeDatadogProfilingIntegration.STATE, deferred.newScopeState(null)); assertEquals("ddprof", deferred.name()); + + // every other pass-through method must reach the swapped-in delegate too, not just + // newScopeState/name — each is a distinct code path in DeferredProfilingContextIntegration. + deferred.onAttach(); + deferred.onDetach(); + assertEquals(1, FakeDatadogProfilingIntegration.onAttachCalls.get()); + assertEquals(1, FakeDatadogProfilingIntegration.onDetachCalls.get()); + assertEquals(42, deferred.encodeOperationName("op")); + assertEquals(43, deferred.encodeResourceName("resource")); + deferred.onRootSpanFinished(null, EndpointTracker.NO_OP); + assertEquals(1, FakeDatadogProfilingIntegration.onRootSpanFinishedCalls.get()); } @Test @@ -149,6 +163,9 @@ public void activate(final Object context) {} static final AtomicInteger constructions = new AtomicInteger(); static final AtomicReference constructionThread = new AtomicReference<>(); static volatile CountDownLatch gate = new CountDownLatch(0); + static final AtomicInteger onAttachCalls = new AtomicInteger(); + static final AtomicInteger onDetachCalls = new AtomicInteger(); + static final AtomicInteger onRootSpanFinishedCalls = new AtomicInteger(); public FakeDatadogProfilingIntegration() { try { @@ -174,7 +191,9 @@ public String name() { } @Override - public void onRootSpanFinished(final AgentSpan rootSpan, final EndpointTracker tracker) {} + public void onRootSpanFinished(final AgentSpan rootSpan, final EndpointTracker tracker) { + onRootSpanFinishedCalls.incrementAndGet(); + } @Override public EndpointTracker onRootSpanStarted(final AgentSpan rootSpan) { @@ -185,5 +204,25 @@ public EndpointTracker onRootSpanStarted(final AgentSpan rootSpan) { public Timing start(final TimerType type) { return Timing.NoOp.INSTANCE; } + + @Override + public void onAttach() { + onAttachCalls.incrementAndGet(); + } + + @Override + public void onDetach() { + onDetachCalls.incrementAndGet(); + } + + @Override + public int encodeOperationName(final CharSequence constant) { + return 42; + } + + @Override + public int encodeResourceName(final CharSequence constant) { + return 43; + } } } diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy new file mode 100644 index 00000000000..41cbaaaa122 --- /dev/null +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy @@ -0,0 +1,59 @@ +package datadog.smoketest.appsec + +import spock.util.concurrent.PollingConditions + +/** + * Verifies that the OTel thread/process context integration ({@code + * datadog.trace.bootstrap.Agent#createProfilingContextIntegration}) is driven purely by AppSec + * activation, independently of profiling: {@code defaultAppSecProperties} always sets {@code + * -Ddd.profiling.enabled=false}, and this module runs twice in CI (the {@code test} and {@code + * testRuntimeActivation} Gradle tasks), once with AppSec fully enabled and once with it inactive. + */ +class OtelContextExposureSmokeTest extends AbstractAppSecServerSmokeTest { + + private static final String PROCESS_CONTEXT_LOG_LINE = 'Registering process context for OTel profiler' + + @Override + def logLevel() { + 'DEBUG' + } + + @Override + ProcessBuilder createProcessBuilder() { + String springBootShadowJar = System.getProperty("datadog.smoketest.appsec.springboot.shadowJar.path") + + List command = new ArrayList<>() + command.add(javaPath()) + command.addAll(defaultJavaProperties) + command.addAll(defaultAppSecProperties) + command.addAll((String[]) ["-jar", springBootShadowJar, "--server.port=${httpPort}"]) + + ProcessBuilder processBuilder = new ProcessBuilder(command) + processBuilder.directory(new File(buildDirectory)) + } + + // TODO(APPSEC-70254): the `testRuntimeActivation` Gradle task (which sets + // `smoke_test.appsec.enabled=inactive`) does not discover this or any other Spock spec today, + // so the `appSecFullyEnabled == false` branch below is not actually exercised in CI. See the + // ticket for the pre-existing root cause (missing `useJUnitPlatform()` on that task). + void 'OTel process context registration follows AppSec activation, not profiling'() { + given: + boolean appSecFullyEnabled = System.getProperty('smoke_test.appsec.enabled') != 'inactive' + PollingConditions conditions = new PollingConditions(timeout: 30, initialDelay: 1, factor: 1.25) + + expect: + if (appSecFullyEnabled) { + conditions.eventually { + assert new File(logFilePath).text.contains(PROCESS_CONTEXT_LOG_LINE) + } + } else { + // Give the agent the same startup time as the positive case before asserting absence, + // so a slow-starting agent can't produce a false negative. + conditions.eventually { + assert new File(logFilePath).length() > 0 + } + sleep(5_000) + assert !new File(logFilePath).text.contains(PROCESS_CONTEXT_LOG_LINE) + } + } +} diff --git a/decisions.md b/decisions.md new file mode 100644 index 00000000000..8e9ce4ab01d --- /dev/null +++ b/decisions.md @@ -0,0 +1,303 @@ +# Decisions + +Design decisions made during this session — the "why A over B" reasoning. +Injected into context on every user message. Propagated to feature manifest at PR open. + +## Metis adversarial review (Paso 4.9, 2026-09-17) + +Findings that revise the draft plan before the checkpoint: + +- BLOCKING: AppSec-only default path must AND in the ddprof environment-safety predicate + (`!isDatadogProfilerEnablementOverridden() && isDatadogProfilerSafeInCurrentEnvironment() + && !Platform.isNativeImage()`), not just `!OperatingSystem.isWindows()` — otherwise ships + native-image/J9/JDK8-aarch64 crash-class regressions. `isDatadogProfilerEnabled()` is NOT a + usable "raw check" (it already ANDs `isProfilingEnabled()`); need a new raw getter. +- Test rule: no `ConfigTest.groovy` case — project CLAUDE.md mandates JUnit 5 Java for new tests. +- `ProcessContext.register()` double-invocation is pre-existing behavior today (via + `ProfilingAgent.run()`'s documented reentrancy for early-start), not an open question — do not + add a naive `AtomicBoolean` guard without confirming the second call is redundant first. +- New `createProfilingContextIntegration()` call-site placement (`installDatadogTracer`, called + from 2 places, "can be called multiple times") risks its own multi-invocation + moves native + lib load/hostname resolution earlier into premain — unvalidated, needs explicit handling. +- Two-way interaction gaps to resolve explicitly: (a) explicit `DD_PROFILING_DDPROF_ENABLED=false` + must still veto even when AppSec triggers the new flag; (b) new flag = false for a profiling + user must not silently fall through to JFR/NoOp and drop ddprof context labels. +- Naming: `TRACE_OTEL_CONTEXT_ENABLED`/`DD_TRACE_OTEL_CTX_ENABLED`/`isOtelContextPropagationEnabled()` + are inconsistent ("propagation" is the wrong word - this is exposure, not propagation). Also: + computed default (false for plain tracing) diverges from dd-trace-py's default-true semantics + under the same env var name - decide explicitly, don't inherit accidentally. +- `_dd.profiling.ctz` tag will now appear on AppSec-only users' spans with no profile behind it - + explicit accept/reject decision needed, not a footnote. +- Step 9 (system-tests smoke run) is not executable on this darwin dev machine (ddprof is + Linux-only) - must be reframed as a CI/Linux-host step. +- Step 7 must add `spotlessApply`/`spotlessCheck` and exercise the config-inversion check + (`ConfigInversionExtension`) for the new metadata entry. + +## Cross-tracer precedent added by user (checkpoint, 2026-09-17) + +User: PHP tracer's equivalent is "automatically enabled when `DD_APPSEC_ENABLED=true`" — a plain +boolean, not an activation-level nuance (PHP has no FULLY_ENABLED/ENABLED_INACTIVE distinction). +Closest Java analog to a bare "AppSec is on" boolean is `ProductActivation.FULLY_ENABLED` +(`ENABLED_INACTIVE` is the Java/dd-trace-java-specific "could be remote-config-activated later" +state PHP doesn't model) — this supports the FULLY_ENABLED-only trigger level already proposed +via the `traceResourceRenamingEnabled` precedent, now with two independent cross-tracer votes +(dd-trace-py's decoupled flag + PHP's AppSec-boolean trigger). + +## Checkpoint decisions confirmed by user (2026-09-17) + +1. **AppSec trigger level: `ProductActivation.FULLY_ENABLED` only** (cross-tracer precedent: + dd-trace-py's decoupled flag + PHP's `DD_APPSEC_ENABLED=true` boolean trigger). +2. **Explicit `DD_PROFILING_DDPROF_ENABLED=false` vetoes ddprof integration even when AppSec would + otherwise trigger the new flag.** The new AppSec-only default path must still respect an + explicit false on the ddprof raw flag. +3. **The new flag is additive (OR), never a replacement gate for existing profiling users.** + `isProfilingEnabled() && isDatadogProfilerEnabled()` remains sufficient on its own; the new + flag only adds the AppSec-only path. Setting the new flag to false must NOT disable ddprof for + a user where real profiling already enabled it (no silent JFR/NoOp fallthrough for profiling + users). +4. **`_dd.profiling.ctx` appearing on AppSec-only users' spans is accepted as-is** — pre-existing + side effect of instantiating `DatadogProfilingIntegration`, not worth special-casing. +5. **Naming**: `OtlpConfig.TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED = "trace.otel.context-exposure.enabled"`, + env `DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED`, getter `Config.isOtelContextExposureEnabled()`. + Deliberately NOT reusing dd-trace-py's `DD_TRACE_OTEL_CTX_ENABLED` name, since the Java default + is conditional (AppSec-or-profiling-driven) vs Python's unconditional default-true — same name + with different semantics would be a cross-tracer trap. +6. **Config file placement**: `OtlpConfig.java` (groups with existing `TRACE_OTEL_ENABLED`). +7. **Call-site placement**: `ProcessContext.register()` for the AppSec-only path is added inside + `Agent.createProfilingContextIntegration()` (same single wiring point as the rest of the ddprof + branch), NOT in `installDatadogTracer()` — avoids the multi-invocation and premain-timing risks + Metis flagged for that call site. + +## Final gating design (synthesized from decisions 1-4, resolves Metis findings 1-2) + +New raw getter `Config.isDatadogProfilerSafeAndConfigured()` = the existing raw +`isDatadogProfilerEnabled` field logic (env-safety predicate + explicit-flag respect, native-image +excluded) WITHOUT the `isProfilingEnabled()` AND-prefix — i.e., exactly today's private field +expression, now exposed on its own. + +`isOtelContextExposureEnabled()` = explicit override via `configProvider.getBoolean(...)` if set, +else: +``` +isDatadogProfilerSafeAndConfigured() + && (isProfilingEnabled() || getAppSecActivation() == ProductActivation.FULLY_ENABLED) +``` +This is additive/OR (decision 3: never disables ddprof for existing profiling users), respects an +explicit `DD_PROFILING_DDPROF_ENABLED=false` (decision 2: baked into the raw predicate), and never +bypasses the native-image/env-safety exclusions (fixes Metis finding 1). + +## Idempotency verified with evidence (resolves Metis finding 4, 2026-09-17) + +Read `com/datadoghq/profiler/OTelContext.java` directly from the ddprof sources jar +(`~/.gradle/caches/modules-2/files-2.1/com.datadoghq/ddprof/1.50.0/.../ddprof-1.50.0-sources.jar`). +`initializeAllContext()`'s own Javadoc states: "Calling this method multiple times will replace +the previous context with the new values" — confirmed idempotent/reentrant by design, additionally +guarded internally by a `ReentrantReadWriteLock` write lock around the native `setProcessCtx0` +call. **No `AtomicBoolean` or other double-invocation guard is needed** in `ProcessContext.register()` +or at the new AppSec-only call site. + +## Remaining scope decisions (2026-09-17) + +- System-tests `THREAD_CONTEXT_SHARING` validation: **out of this PR**, documented as a manual/CI + follow-up (command + preconditions), not tracked as an executable TODO in `task_plan.md` (not + runnable on this darwin dev machine; ddprof is Linux-only). + +## TODO-10 — /techdebt overreach reverted (2026-09-17) + +The `/techdebt` review agent removed `Config.isDatadogProfilerSafeAndConfigured()` (added in +TODO-2) and inlined the raw `isDatadogProfilerEnabled` field at its single call site, reasoning it +was an unnecessary one-use abstraction. This directly undoes an explicit, adversarially-reviewed +design decision (see `.claude-invariants.md` invariant #8 and "Final gating design" above): the +getter exists specifically so any *future* AppSec-only trigger path reuses the raw ddprof +env-safety predicate correctly, without falling back to `isDatadogProfilerEnabled()` (which already +ANDs `isProfilingEnabled()` and would silently reintroduce the native-image/J9/JDK8 regression +Metis flagged). The `/techdebt` agent had no access to this task's `.claude-invariants.md`/ +`decisions.md` context, so it could not see why the getter was deliberate rather than incidental. + +**Resolution:** reverted the removal — restored `isDatadogProfilerSafeAndConfigured()` with an +expanded Javadoc explaining the reuse rationale, and restored its use in the `otelContextExposureEnabled` +computation. Recompiled and re-ran `:internal-api:test`/`spotlessApply`/`spotlessCheck` — all pass, +no behavior change (the inlined and getter-based forms were computationally identical; only the +discoverability/reuse-safety property was at stake). + +## TODO-11 — Manual/CI follow-up: `THREAD_CONTEXT_SHARING` system-tests validation (2026-09-17) + +Out of scope for this PR (invariant #20): cannot be executed from this darwin dev machine, since +ddprof and the eBPF/system-probe consumer are Linux-only. Documented here as a follow-up step for +whoever validates this change on a Linux host / in CI, not tracked as an in-repo executable TODO. + +**Scenario:** `tests/cws/test_thread_context_sharing.py::Test_ThreadContextSharing` +(`THREAD_CONTEXT_SHARING` scenario in `system-tests`, introduced in system-tests PR #7617). + +**What it validates:** that a JVM running with this change's new AppSec-only trigger path +(`Config.isOtelContextExposureEnabled()` true via `ProductActivation.FULLY_ENABLED` with profiling +disabled) exposes both the thread-local span context (native TLS write via +`DatadogProfilingIntegration`) and the process-wide descriptor (`ProcessContext.register()` -> +`OTelContext.initializeAllContext(...)`) so that an eBPF/CWS consumer running alongside the JVM can +read the current span context off a traced thread. + +**Preconditions:** +- Linux host (the ddprof native library and the eBPF/system-probe consumer are Linux-only; this + scenario cannot run on macOS/darwin). +- Datadog Agent >= 7.84.0-devel (the version that ships the eBPF/CWS-side consumer for this + context-sharing mechanism). +- `DD_RUNTIME_SECURITY_CONFIG_ENABLED=true` (or the scenario's documented equivalent) on the traced + JVM's Agent, so the Datadog Agent's system-probe/eBPF component is active. +- System-probe/eBPF support available in the test environment (typically requires elevated + privileges / a kernel with the needed eBPF features — see `system-tests`' own scenario + preconditions for `THREAD_CONTEXT_SHARING` in `utils/_context/_scenarios/__init__.py`). + +**How to run (on a Linux host with system-tests set up, from the `system-tests` repo root):** +```bash +./run.sh THREAD_CONTEXT_SHARING +``` +(Standard system-tests scenario invocation; substitute the dd-trace-java build under test per the +system-tests library-injection instructions if validating this branch specifically, e.g. via a +locally built `dd-java-agent` jar per `docs/how_to_smoke_test.md`/system-tests' Java onboarding +docs.) + +**Specific assertion this PR's change should newly satisfy:** with AppSec `FULLY_ENABLED` and +profiling disabled, the scenario's checks for both thread-context (TLS) and process-context +(`OTelContext` descriptor) presence should now pass, where before this change they would have been +absent (both gates were previously collapsed into `isProfilingEnabled()`, per invariant #2). + +## Premain-timing fix for the AppSec-only ddprof path (Codex P1 on PR #12546, 2026-09-17) + +**Problem (Codex review, inline on `Agent.java:1506`):** with AppSec `FULLY_ENABLED` and profiling +disabled, the widened ddprof branch is reached from `InstallDatadogTracerCallback.execute()`, i.e. +on the JVM's primordial premain thread. Constructing `DatadogProfilingIntegration` initializes +`DatadogProfiler`, whose constructor goes through `TempLocationManager` and `java.nio.file.Files`, +which can lock in the default filesystem provider before the application configures one in `main` +— a direct violation of invariant #11 / `AGENTS.md`'s bootstrap constraints. This premain-time NIO +touch already exists today for real-profiling users (accepted, pre-existing), but this PR extended +it to a brand-new population that previously never loaded ddprof that early. + +**Design chosen:** new package-private wrapper +`dd-java-agent/agent-bootstrap/.../DeferredProfilingContextIntegration` implementing +`ProfilingContextIntegration`. It is returned synchronously (the caller needs a non-null integration +immediately) while the real reflective construction of `DatadogProfilingIntegration` *and* the +subsequent `ProcessContext.register(ConfigProvider)` call run on `AgentTaskScheduler.get().execute(...)` +— the same "defer past premain" primitive already used by `startCrashTracking()`. Until the swap it +delegates to `ProfilingContextIntegration.NoOp.INSTANCE` via a single `volatile` delegate field +(volatile, not `AtomicReference`: a plain publish/read is all the swap needs and it matches the +existing lazy-publish style in this module); after the swap every interface method delegates to the +real integration. A failing deferred construction logs at `log.debug(...)` and leaves the instance +behaving as `NoOp` forever — a background task never propagates a failure. No idempotency guard was +added around `ProcessContext.register()` (invariant #14 still holds). + +**Why the real-profiling path is untouched:** dropping the first few scope events is fine for +context *exposure* (eBPF/CWS reads whatever the current span is when it looks) but not for profiling +accuracy, and "no behavior change for existing profiling users" is a hard constraint of this PR. So +the branch selection is `deferInitialization = !config.isDatadogProfilerEnabled()`: when the Datadog +profiler is enabled the construction stays synchronous, byte-for-byte the current behavior; only the +AppSec-only-only trigger (`!isDatadogProfilerEnabled() && isOtelContextExposureEnabled()`) defers. + +**Deviations / judgment calls:** +- Extracted the ddprof construction into a package-private `Agent.createDdprofContextIntegration(ClassLoader, boolean)` + seam (mirroring the existing `Agent.shutdownFeatureFlagging(ClassLoader)` test seam) so the + deferral is unit-testable with a fake class loader instead of requiring the real native library. + `null` return means "synchronous construction failed", preserving the existing fall-through to the + JFR/NoOp branches. +- `name()` returns the constant `"ddprof"` rather than the current delegate's name. `CoreTracer` + reads it exactly once when the tracer is built (`_dd.profiling.ctx` tag), which may happen before + the swap; returning the delegate's name would make that tag race between `"none"` and `"ddprof"`. + Residual risk: if the deferred construction later fails, the tag reads `"ddprof"` optimistically. +- Narrow edge case accepted: for the (unusual) combination of an explicit + `DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED=true` with profiling enabled but the ddprof raw predicate + false, a *failing* ddprof construction no longer falls through to the JFR/timeline branch, because + the failure is now only known after premain. Every other path keeps its current fallback. +- Tests: `DeferredProfilingContextIntegrationTest` (JUnit 5, Java) asserts that with + `deferInitialization=true` neither the integration constructor nor `ProcessContext.register` runs + on the calling thread (gated by a latch so the assertion cannot race the scheduler), that they do + run afterwards on another thread, that `deferInitialization=false` still constructs synchronously + on the calling thread, and that a failing factory leaves the wrapper NoOp-equivalent. + +### Dropped the dedicated `DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED` config flag (2026-09-18) + +**Problem statement:** the original design added a new public config +(`OtlpConfig.TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED`) with an explicit-override branch in +`Config.isOtelContextExposureEnabled()`, on top of the derived default. This surfaced a real cost +during review: any new entry in `metadata/supported-configurations.json` needs manual registration +on the external Feature Parity Dashboard (`docs/add_new_configurations.md` Step 8) before the +`config-inversion-local-validation.py` CI job passes - and this was never done, causing a CI failure +on PR #12546. + +**Question raised:** did this feature actually need a dedicated flag, or could it be a pure +derivation from the two conditions that already drive it (profiling enabled, AppSec fully enabled)? + +**Precedent checked:** `isProfilingEnabled()` itself has no dedicated override for the *composed* +behavior it exposes - it is driven by `DD_PROFILING_ENABLED` (a `ProfilingEnablement` tri-state) with +no separate "disable profiling context integration but keep profiling" escape hatch. A user who wants +ddprof context labeling off already has a kill switch: disable `DD_PROFILING_ENABLED` and disable +`DD_APPSEC_ENABLED` (or drop it to `inactive`). Composing `isOtelContextExposureEnabled()` from those +two existing, independently-overridable flags gives the same practical kill-switch coverage as +`isProfilingEnabled()` has for itself, without introducing a third, unregistered flag. + +**Design chosen:** removed `OtlpConfig.TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED` entirely (constant, +`metadata/supported-configurations.json` entry, and the explicit-override branch in `Config.java`). +`isOtelContextExposureEnabled()` is now a pure derivation: +`isDatadogProfilerSafeAndConfigured() && (isProfilingEnabled() || getAppSecActivation() == +ProductActivation.FULLY_ENABLED)`. No public config added by this PR - the +`config-inversion-local-validation.py` / Feature Parity Dashboard registration problem is moot. + +**What was explicitly considered and rejected as a mismatch:** using `isAppSecScaEnabled()` +(`DD_APPSEC_SCA_ENABLED`) as the AppSec signal instead of `getAppSecActivation() == +ProductActivation.FULLY_ENABLED`. SCA (Software Composition Analysis, dependency/vulnerability +telemetry) is a different product from AppSec's runtime protection (WAF/RASP) and is not what CWS/eBPF +context exposure needs - a user could have AppSec fully protecting requests with SCA off, and would +wrongly lose context exposure under that substitution. + +**Trade-off accepted:** no way for a user to say "profiling and/or AppSec FULLY_ENABLED, but I +specifically don't want context exposure" without disabling one of the two underlying features. Judged +acceptable - `isProfilingEnabled()` has the same limitation, and no support/rollback case has come up +that needs finer granularity than that. + +**Tests updated:** removed the three `ConfigOtelContextExposureTest` cases that asserted explicit +override behavior (`explicitFalseOverridesConditionsThatWouldEnableIt`, +`explicitTrueOverridesProfilingAndAppSecActivationLevel`, +`explicitTrueDoesNotBypassDatadogProfilerSafetyPredicate`) - that behavior no longer exists. The +remaining derivation tests (`disabledByDefault`, `enabledWhenProfilingIsEnabled`, +`enabledWhenAppSecIsFullyEnabledWithoutProfiling`, `disabledWhenAppSecIsOnlyEnabledInactive`, +`disabledWhenDatadogProfilerIsExplicitlyDisabled`, +`disabledInAnEnvironmentWhereTheDatadogProfilerIsUnsafe`) are unchanged and still pass. + +### Why `Agent.java` still reflectively loads `DatadogProfilingIntegration`/`ProcessContext` instead of depending on `agent-profiling` directly (2026-09-18) + +Raised during `/pr-deep-review` of `Config.java`: does `isOtelContextExposureEnabled()` truly not +depend on profiling, given `Agent.createProfilingContextIntegration()` still reaches into +`com.datadog.profiling.ddprof.DatadogProfilingIntegration` and +`com.datadog.profiling.agent.ProcessContext` via `AGENT_CLASSLOADER.loadClass(...)`? Anticipated +reviewer question: why not move that code somewhere reusable instead of loading it via reflection. + +**Verified:** +- The reflective `loadClass(...)` mechanism predates this PR. On `origin/master`, + `createProfilingContextIntegration()` already loaded `DatadogProfilingIntegration` this exact way + for the `isProfilingEnabled() && isDatadogProfilerEnabled()` branch, and still loads + `JFREventContextIntegration` the same way for the JFR branch, untouched by this PR. +- `dd-java-agent/agent-bootstrap/build.gradle` has no `project(':dd-java-agent:agent-profiling...')` + dependency - confirmed by grep, empty result. `agent-bootstrap` runs in premain under strict + bootstrap constraints (no `java.nio.file`, no JMX - see + `docs/bootstrap_design_guidelines.md`) and intentionally has no compile-time dependency on the + much heavier `agent-profiling` module (ddprof native bindings, JFR controllers). +- `DatadogProfilingIntegration.java` and `ProcessContext.java` live under + `dd-java-agent/agent-profiling/...`. They are always present in the single shaded agent jar + regardless of `DD_PROFILING_ENABLED` - "profiling enabled" is a runtime flag deciding whether to + *instantiate* these classes, not whether the jar contains them. So `isDatadogProfilerSafeAndConfigured()` + correctly has no dependency on profiling being active: it only checks JVM/platform safety + (`isDatadogProfilerEnablementOverridden()`, `isDatadogProfilerSafeInCurrentEnvironment()`) plus the + `PROFILING_DATADOG_PROFILER_ENABLED` sub-flag, whose own default (`isDatadogProfilerSafeInCurrentEnvironment()`) + is likewise independent of `DD_PROFILING_ENABLED` - verified across all its usages + (`Config.java`, `OpenJdkController.java`; no other read site exists). + +**Decision:** keep the reflective load. Moving `DatadogProfilingIntegration`/`ProcessContext` to a +shared module to avoid reflection would mean redesigning the intentional bootstrap/profiling module +boundary - much higher risk and blast radius than reusing an established, working pattern this PR +only extends (new OR condition + deferred construction), not invents. + +**Follow-up naming note:** `isDatadogProfilerEnabled()` (the pre-existing getter, unchanged by this +PR) is a false friend - it sounds like "the profiler is currently recording" but is actually +`isProfilingEnabled() && isDatadogProfilerSafeAndConfigured()`. Documented with a clarifying Javadoc +on the getter in this PR. A full rename (`isDatadogProfilerEnabled()` → +`isDatadogProfilerActive()`, raw field → `ddprofEngineAllowed`) was considered but rejected for this +PR: the getter has wide call-site exposure (`StatusLogger`, `ProfilerFlareReporter`, +`CompositeController`, `ProfilerSettingsSupport`, JFR controllers) and renaming it would mix an +unrelated wide rename into a PR already touching bootstrap/premain. diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 4c8567d114d..997cabbf764 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -4247,6 +4247,13 @@ public boolean isProfilingRecordExceptionMessage() { return profilingRecordExceptionMessage; } + /** + * Despite the name, this does NOT mean "the Datadog profiler engine is currently recording" - it + * means "profiling is enabled AND the ddprof engine is allowed to run" ({@link + * #isProfilingEnabled()} AND {@link #isDatadogProfilerSafeAndConfigured()}). The underlying + * {@code isDatadogProfilerEnabled} field is itself independent of profiling: see {@link + * #isDatadogProfilerSafeAndConfigured()}. + */ public boolean isDatadogProfilerEnabled() { return isProfilingEnabled() && isDatadogProfilerEnabled; } From 060ff1f2d6744c70c2a012f7aad8bac7729470cd Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 18 Sep 2026 14:44:29 +0200 Subject: [PATCH 05/21] review: remove decisions.md from tracked files decisions.md is a session-local planning artifact and should not ship as part of the PR diff; it is now added to the global gitignore alongside progress.md/task_plan.md. --- decisions.md | 303 --------------------------------------------------- 1 file changed, 303 deletions(-) delete mode 100644 decisions.md diff --git a/decisions.md b/decisions.md deleted file mode 100644 index 8e9ce4ab01d..00000000000 --- a/decisions.md +++ /dev/null @@ -1,303 +0,0 @@ -# Decisions - -Design decisions made during this session — the "why A over B" reasoning. -Injected into context on every user message. Propagated to feature manifest at PR open. - -## Metis adversarial review (Paso 4.9, 2026-09-17) - -Findings that revise the draft plan before the checkpoint: - -- BLOCKING: AppSec-only default path must AND in the ddprof environment-safety predicate - (`!isDatadogProfilerEnablementOverridden() && isDatadogProfilerSafeInCurrentEnvironment() - && !Platform.isNativeImage()`), not just `!OperatingSystem.isWindows()` — otherwise ships - native-image/J9/JDK8-aarch64 crash-class regressions. `isDatadogProfilerEnabled()` is NOT a - usable "raw check" (it already ANDs `isProfilingEnabled()`); need a new raw getter. -- Test rule: no `ConfigTest.groovy` case — project CLAUDE.md mandates JUnit 5 Java for new tests. -- `ProcessContext.register()` double-invocation is pre-existing behavior today (via - `ProfilingAgent.run()`'s documented reentrancy for early-start), not an open question — do not - add a naive `AtomicBoolean` guard without confirming the second call is redundant first. -- New `createProfilingContextIntegration()` call-site placement (`installDatadogTracer`, called - from 2 places, "can be called multiple times") risks its own multi-invocation + moves native - lib load/hostname resolution earlier into premain — unvalidated, needs explicit handling. -- Two-way interaction gaps to resolve explicitly: (a) explicit `DD_PROFILING_DDPROF_ENABLED=false` - must still veto even when AppSec triggers the new flag; (b) new flag = false for a profiling - user must not silently fall through to JFR/NoOp and drop ddprof context labels. -- Naming: `TRACE_OTEL_CONTEXT_ENABLED`/`DD_TRACE_OTEL_CTX_ENABLED`/`isOtelContextPropagationEnabled()` - are inconsistent ("propagation" is the wrong word - this is exposure, not propagation). Also: - computed default (false for plain tracing) diverges from dd-trace-py's default-true semantics - under the same env var name - decide explicitly, don't inherit accidentally. -- `_dd.profiling.ctz` tag will now appear on AppSec-only users' spans with no profile behind it - - explicit accept/reject decision needed, not a footnote. -- Step 9 (system-tests smoke run) is not executable on this darwin dev machine (ddprof is - Linux-only) - must be reframed as a CI/Linux-host step. -- Step 7 must add `spotlessApply`/`spotlessCheck` and exercise the config-inversion check - (`ConfigInversionExtension`) for the new metadata entry. - -## Cross-tracer precedent added by user (checkpoint, 2026-09-17) - -User: PHP tracer's equivalent is "automatically enabled when `DD_APPSEC_ENABLED=true`" — a plain -boolean, not an activation-level nuance (PHP has no FULLY_ENABLED/ENABLED_INACTIVE distinction). -Closest Java analog to a bare "AppSec is on" boolean is `ProductActivation.FULLY_ENABLED` -(`ENABLED_INACTIVE` is the Java/dd-trace-java-specific "could be remote-config-activated later" -state PHP doesn't model) — this supports the FULLY_ENABLED-only trigger level already proposed -via the `traceResourceRenamingEnabled` precedent, now with two independent cross-tracer votes -(dd-trace-py's decoupled flag + PHP's AppSec-boolean trigger). - -## Checkpoint decisions confirmed by user (2026-09-17) - -1. **AppSec trigger level: `ProductActivation.FULLY_ENABLED` only** (cross-tracer precedent: - dd-trace-py's decoupled flag + PHP's `DD_APPSEC_ENABLED=true` boolean trigger). -2. **Explicit `DD_PROFILING_DDPROF_ENABLED=false` vetoes ddprof integration even when AppSec would - otherwise trigger the new flag.** The new AppSec-only default path must still respect an - explicit false on the ddprof raw flag. -3. **The new flag is additive (OR), never a replacement gate for existing profiling users.** - `isProfilingEnabled() && isDatadogProfilerEnabled()` remains sufficient on its own; the new - flag only adds the AppSec-only path. Setting the new flag to false must NOT disable ddprof for - a user where real profiling already enabled it (no silent JFR/NoOp fallthrough for profiling - users). -4. **`_dd.profiling.ctx` appearing on AppSec-only users' spans is accepted as-is** — pre-existing - side effect of instantiating `DatadogProfilingIntegration`, not worth special-casing. -5. **Naming**: `OtlpConfig.TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED = "trace.otel.context-exposure.enabled"`, - env `DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED`, getter `Config.isOtelContextExposureEnabled()`. - Deliberately NOT reusing dd-trace-py's `DD_TRACE_OTEL_CTX_ENABLED` name, since the Java default - is conditional (AppSec-or-profiling-driven) vs Python's unconditional default-true — same name - with different semantics would be a cross-tracer trap. -6. **Config file placement**: `OtlpConfig.java` (groups with existing `TRACE_OTEL_ENABLED`). -7. **Call-site placement**: `ProcessContext.register()` for the AppSec-only path is added inside - `Agent.createProfilingContextIntegration()` (same single wiring point as the rest of the ddprof - branch), NOT in `installDatadogTracer()` — avoids the multi-invocation and premain-timing risks - Metis flagged for that call site. - -## Final gating design (synthesized from decisions 1-4, resolves Metis findings 1-2) - -New raw getter `Config.isDatadogProfilerSafeAndConfigured()` = the existing raw -`isDatadogProfilerEnabled` field logic (env-safety predicate + explicit-flag respect, native-image -excluded) WITHOUT the `isProfilingEnabled()` AND-prefix — i.e., exactly today's private field -expression, now exposed on its own. - -`isOtelContextExposureEnabled()` = explicit override via `configProvider.getBoolean(...)` if set, -else: -``` -isDatadogProfilerSafeAndConfigured() - && (isProfilingEnabled() || getAppSecActivation() == ProductActivation.FULLY_ENABLED) -``` -This is additive/OR (decision 3: never disables ddprof for existing profiling users), respects an -explicit `DD_PROFILING_DDPROF_ENABLED=false` (decision 2: baked into the raw predicate), and never -bypasses the native-image/env-safety exclusions (fixes Metis finding 1). - -## Idempotency verified with evidence (resolves Metis finding 4, 2026-09-17) - -Read `com/datadoghq/profiler/OTelContext.java` directly from the ddprof sources jar -(`~/.gradle/caches/modules-2/files-2.1/com.datadoghq/ddprof/1.50.0/.../ddprof-1.50.0-sources.jar`). -`initializeAllContext()`'s own Javadoc states: "Calling this method multiple times will replace -the previous context with the new values" — confirmed idempotent/reentrant by design, additionally -guarded internally by a `ReentrantReadWriteLock` write lock around the native `setProcessCtx0` -call. **No `AtomicBoolean` or other double-invocation guard is needed** in `ProcessContext.register()` -or at the new AppSec-only call site. - -## Remaining scope decisions (2026-09-17) - -- System-tests `THREAD_CONTEXT_SHARING` validation: **out of this PR**, documented as a manual/CI - follow-up (command + preconditions), not tracked as an executable TODO in `task_plan.md` (not - runnable on this darwin dev machine; ddprof is Linux-only). - -## TODO-10 — /techdebt overreach reverted (2026-09-17) - -The `/techdebt` review agent removed `Config.isDatadogProfilerSafeAndConfigured()` (added in -TODO-2) and inlined the raw `isDatadogProfilerEnabled` field at its single call site, reasoning it -was an unnecessary one-use abstraction. This directly undoes an explicit, adversarially-reviewed -design decision (see `.claude-invariants.md` invariant #8 and "Final gating design" above): the -getter exists specifically so any *future* AppSec-only trigger path reuses the raw ddprof -env-safety predicate correctly, without falling back to `isDatadogProfilerEnabled()` (which already -ANDs `isProfilingEnabled()` and would silently reintroduce the native-image/J9/JDK8 regression -Metis flagged). The `/techdebt` agent had no access to this task's `.claude-invariants.md`/ -`decisions.md` context, so it could not see why the getter was deliberate rather than incidental. - -**Resolution:** reverted the removal — restored `isDatadogProfilerSafeAndConfigured()` with an -expanded Javadoc explaining the reuse rationale, and restored its use in the `otelContextExposureEnabled` -computation. Recompiled and re-ran `:internal-api:test`/`spotlessApply`/`spotlessCheck` — all pass, -no behavior change (the inlined and getter-based forms were computationally identical; only the -discoverability/reuse-safety property was at stake). - -## TODO-11 — Manual/CI follow-up: `THREAD_CONTEXT_SHARING` system-tests validation (2026-09-17) - -Out of scope for this PR (invariant #20): cannot be executed from this darwin dev machine, since -ddprof and the eBPF/system-probe consumer are Linux-only. Documented here as a follow-up step for -whoever validates this change on a Linux host / in CI, not tracked as an in-repo executable TODO. - -**Scenario:** `tests/cws/test_thread_context_sharing.py::Test_ThreadContextSharing` -(`THREAD_CONTEXT_SHARING` scenario in `system-tests`, introduced in system-tests PR #7617). - -**What it validates:** that a JVM running with this change's new AppSec-only trigger path -(`Config.isOtelContextExposureEnabled()` true via `ProductActivation.FULLY_ENABLED` with profiling -disabled) exposes both the thread-local span context (native TLS write via -`DatadogProfilingIntegration`) and the process-wide descriptor (`ProcessContext.register()` -> -`OTelContext.initializeAllContext(...)`) so that an eBPF/CWS consumer running alongside the JVM can -read the current span context off a traced thread. - -**Preconditions:** -- Linux host (the ddprof native library and the eBPF/system-probe consumer are Linux-only; this - scenario cannot run on macOS/darwin). -- Datadog Agent >= 7.84.0-devel (the version that ships the eBPF/CWS-side consumer for this - context-sharing mechanism). -- `DD_RUNTIME_SECURITY_CONFIG_ENABLED=true` (or the scenario's documented equivalent) on the traced - JVM's Agent, so the Datadog Agent's system-probe/eBPF component is active. -- System-probe/eBPF support available in the test environment (typically requires elevated - privileges / a kernel with the needed eBPF features — see `system-tests`' own scenario - preconditions for `THREAD_CONTEXT_SHARING` in `utils/_context/_scenarios/__init__.py`). - -**How to run (on a Linux host with system-tests set up, from the `system-tests` repo root):** -```bash -./run.sh THREAD_CONTEXT_SHARING -``` -(Standard system-tests scenario invocation; substitute the dd-trace-java build under test per the -system-tests library-injection instructions if validating this branch specifically, e.g. via a -locally built `dd-java-agent` jar per `docs/how_to_smoke_test.md`/system-tests' Java onboarding -docs.) - -**Specific assertion this PR's change should newly satisfy:** with AppSec `FULLY_ENABLED` and -profiling disabled, the scenario's checks for both thread-context (TLS) and process-context -(`OTelContext` descriptor) presence should now pass, where before this change they would have been -absent (both gates were previously collapsed into `isProfilingEnabled()`, per invariant #2). - -## Premain-timing fix for the AppSec-only ddprof path (Codex P1 on PR #12546, 2026-09-17) - -**Problem (Codex review, inline on `Agent.java:1506`):** with AppSec `FULLY_ENABLED` and profiling -disabled, the widened ddprof branch is reached from `InstallDatadogTracerCallback.execute()`, i.e. -on the JVM's primordial premain thread. Constructing `DatadogProfilingIntegration` initializes -`DatadogProfiler`, whose constructor goes through `TempLocationManager` and `java.nio.file.Files`, -which can lock in the default filesystem provider before the application configures one in `main` -— a direct violation of invariant #11 / `AGENTS.md`'s bootstrap constraints. This premain-time NIO -touch already exists today for real-profiling users (accepted, pre-existing), but this PR extended -it to a brand-new population that previously never loaded ddprof that early. - -**Design chosen:** new package-private wrapper -`dd-java-agent/agent-bootstrap/.../DeferredProfilingContextIntegration` implementing -`ProfilingContextIntegration`. It is returned synchronously (the caller needs a non-null integration -immediately) while the real reflective construction of `DatadogProfilingIntegration` *and* the -subsequent `ProcessContext.register(ConfigProvider)` call run on `AgentTaskScheduler.get().execute(...)` -— the same "defer past premain" primitive already used by `startCrashTracking()`. Until the swap it -delegates to `ProfilingContextIntegration.NoOp.INSTANCE` via a single `volatile` delegate field -(volatile, not `AtomicReference`: a plain publish/read is all the swap needs and it matches the -existing lazy-publish style in this module); after the swap every interface method delegates to the -real integration. A failing deferred construction logs at `log.debug(...)` and leaves the instance -behaving as `NoOp` forever — a background task never propagates a failure. No idempotency guard was -added around `ProcessContext.register()` (invariant #14 still holds). - -**Why the real-profiling path is untouched:** dropping the first few scope events is fine for -context *exposure* (eBPF/CWS reads whatever the current span is when it looks) but not for profiling -accuracy, and "no behavior change for existing profiling users" is a hard constraint of this PR. So -the branch selection is `deferInitialization = !config.isDatadogProfilerEnabled()`: when the Datadog -profiler is enabled the construction stays synchronous, byte-for-byte the current behavior; only the -AppSec-only-only trigger (`!isDatadogProfilerEnabled() && isOtelContextExposureEnabled()`) defers. - -**Deviations / judgment calls:** -- Extracted the ddprof construction into a package-private `Agent.createDdprofContextIntegration(ClassLoader, boolean)` - seam (mirroring the existing `Agent.shutdownFeatureFlagging(ClassLoader)` test seam) so the - deferral is unit-testable with a fake class loader instead of requiring the real native library. - `null` return means "synchronous construction failed", preserving the existing fall-through to the - JFR/NoOp branches. -- `name()` returns the constant `"ddprof"` rather than the current delegate's name. `CoreTracer` - reads it exactly once when the tracer is built (`_dd.profiling.ctx` tag), which may happen before - the swap; returning the delegate's name would make that tag race between `"none"` and `"ddprof"`. - Residual risk: if the deferred construction later fails, the tag reads `"ddprof"` optimistically. -- Narrow edge case accepted: for the (unusual) combination of an explicit - `DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED=true` with profiling enabled but the ddprof raw predicate - false, a *failing* ddprof construction no longer falls through to the JFR/timeline branch, because - the failure is now only known after premain. Every other path keeps its current fallback. -- Tests: `DeferredProfilingContextIntegrationTest` (JUnit 5, Java) asserts that with - `deferInitialization=true` neither the integration constructor nor `ProcessContext.register` runs - on the calling thread (gated by a latch so the assertion cannot race the scheduler), that they do - run afterwards on another thread, that `deferInitialization=false` still constructs synchronously - on the calling thread, and that a failing factory leaves the wrapper NoOp-equivalent. - -### Dropped the dedicated `DD_TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED` config flag (2026-09-18) - -**Problem statement:** the original design added a new public config -(`OtlpConfig.TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED`) with an explicit-override branch in -`Config.isOtelContextExposureEnabled()`, on top of the derived default. This surfaced a real cost -during review: any new entry in `metadata/supported-configurations.json` needs manual registration -on the external Feature Parity Dashboard (`docs/add_new_configurations.md` Step 8) before the -`config-inversion-local-validation.py` CI job passes - and this was never done, causing a CI failure -on PR #12546. - -**Question raised:** did this feature actually need a dedicated flag, or could it be a pure -derivation from the two conditions that already drive it (profiling enabled, AppSec fully enabled)? - -**Precedent checked:** `isProfilingEnabled()` itself has no dedicated override for the *composed* -behavior it exposes - it is driven by `DD_PROFILING_ENABLED` (a `ProfilingEnablement` tri-state) with -no separate "disable profiling context integration but keep profiling" escape hatch. A user who wants -ddprof context labeling off already has a kill switch: disable `DD_PROFILING_ENABLED` and disable -`DD_APPSEC_ENABLED` (or drop it to `inactive`). Composing `isOtelContextExposureEnabled()` from those -two existing, independently-overridable flags gives the same practical kill-switch coverage as -`isProfilingEnabled()` has for itself, without introducing a third, unregistered flag. - -**Design chosen:** removed `OtlpConfig.TRACE_OTEL_CONTEXT_EXPOSURE_ENABLED` entirely (constant, -`metadata/supported-configurations.json` entry, and the explicit-override branch in `Config.java`). -`isOtelContextExposureEnabled()` is now a pure derivation: -`isDatadogProfilerSafeAndConfigured() && (isProfilingEnabled() || getAppSecActivation() == -ProductActivation.FULLY_ENABLED)`. No public config added by this PR - the -`config-inversion-local-validation.py` / Feature Parity Dashboard registration problem is moot. - -**What was explicitly considered and rejected as a mismatch:** using `isAppSecScaEnabled()` -(`DD_APPSEC_SCA_ENABLED`) as the AppSec signal instead of `getAppSecActivation() == -ProductActivation.FULLY_ENABLED`. SCA (Software Composition Analysis, dependency/vulnerability -telemetry) is a different product from AppSec's runtime protection (WAF/RASP) and is not what CWS/eBPF -context exposure needs - a user could have AppSec fully protecting requests with SCA off, and would -wrongly lose context exposure under that substitution. - -**Trade-off accepted:** no way for a user to say "profiling and/or AppSec FULLY_ENABLED, but I -specifically don't want context exposure" without disabling one of the two underlying features. Judged -acceptable - `isProfilingEnabled()` has the same limitation, and no support/rollback case has come up -that needs finer granularity than that. - -**Tests updated:** removed the three `ConfigOtelContextExposureTest` cases that asserted explicit -override behavior (`explicitFalseOverridesConditionsThatWouldEnableIt`, -`explicitTrueOverridesProfilingAndAppSecActivationLevel`, -`explicitTrueDoesNotBypassDatadogProfilerSafetyPredicate`) - that behavior no longer exists. The -remaining derivation tests (`disabledByDefault`, `enabledWhenProfilingIsEnabled`, -`enabledWhenAppSecIsFullyEnabledWithoutProfiling`, `disabledWhenAppSecIsOnlyEnabledInactive`, -`disabledWhenDatadogProfilerIsExplicitlyDisabled`, -`disabledInAnEnvironmentWhereTheDatadogProfilerIsUnsafe`) are unchanged and still pass. - -### Why `Agent.java` still reflectively loads `DatadogProfilingIntegration`/`ProcessContext` instead of depending on `agent-profiling` directly (2026-09-18) - -Raised during `/pr-deep-review` of `Config.java`: does `isOtelContextExposureEnabled()` truly not -depend on profiling, given `Agent.createProfilingContextIntegration()` still reaches into -`com.datadog.profiling.ddprof.DatadogProfilingIntegration` and -`com.datadog.profiling.agent.ProcessContext` via `AGENT_CLASSLOADER.loadClass(...)`? Anticipated -reviewer question: why not move that code somewhere reusable instead of loading it via reflection. - -**Verified:** -- The reflective `loadClass(...)` mechanism predates this PR. On `origin/master`, - `createProfilingContextIntegration()` already loaded `DatadogProfilingIntegration` this exact way - for the `isProfilingEnabled() && isDatadogProfilerEnabled()` branch, and still loads - `JFREventContextIntegration` the same way for the JFR branch, untouched by this PR. -- `dd-java-agent/agent-bootstrap/build.gradle` has no `project(':dd-java-agent:agent-profiling...')` - dependency - confirmed by grep, empty result. `agent-bootstrap` runs in premain under strict - bootstrap constraints (no `java.nio.file`, no JMX - see - `docs/bootstrap_design_guidelines.md`) and intentionally has no compile-time dependency on the - much heavier `agent-profiling` module (ddprof native bindings, JFR controllers). -- `DatadogProfilingIntegration.java` and `ProcessContext.java` live under - `dd-java-agent/agent-profiling/...`. They are always present in the single shaded agent jar - regardless of `DD_PROFILING_ENABLED` - "profiling enabled" is a runtime flag deciding whether to - *instantiate* these classes, not whether the jar contains them. So `isDatadogProfilerSafeAndConfigured()` - correctly has no dependency on profiling being active: it only checks JVM/platform safety - (`isDatadogProfilerEnablementOverridden()`, `isDatadogProfilerSafeInCurrentEnvironment()`) plus the - `PROFILING_DATADOG_PROFILER_ENABLED` sub-flag, whose own default (`isDatadogProfilerSafeInCurrentEnvironment()`) - is likewise independent of `DD_PROFILING_ENABLED` - verified across all its usages - (`Config.java`, `OpenJdkController.java`; no other read site exists). - -**Decision:** keep the reflective load. Moving `DatadogProfilingIntegration`/`ProcessContext` to a -shared module to avoid reflection would mean redesigning the intentional bootstrap/profiling module -boundary - much higher risk and blast radius than reusing an established, working pattern this PR -only extends (new OR condition + deferred construction), not invents. - -**Follow-up naming note:** `isDatadogProfilerEnabled()` (the pre-existing getter, unchanged by this -PR) is a false friend - it sounds like "the profiler is currently recording" but is actually -`isProfilingEnabled() && isDatadogProfilerSafeAndConfigured()`. Documented with a clarifying Javadoc -on the getter in this PR. A full rename (`isDatadogProfilerEnabled()` → -`isDatadogProfilerActive()`, raw field → `ddprofEngineAllowed`) was considered but rejected for this -PR: the getter has wide call-site exposure (`StatusLogger`, `ProfilerFlareReporter`, -`CompositeController`, `ProfilerSettingsSupport`, JFR controllers) and renaming it would mix an -unrelated wide rename into a PR already touching bootstrap/premain. From 8652de23ed3ff59f097d3598711ceb36dc489ed3 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 21 Sep 2026 12:08:34 +0200 Subject: [PATCH 06/21] review: defer ddprof context construction with a startup delay Mitigates a P1 Autotest finding (PR #12546 discussion): the deferred construction was only moved off the premain thread, not delayed past premain/main startup, so it could still race with an application setting java.nio.file.spi.DefaultFileSystemProvider in main. Schedule it with the same delay magnitude Agent already uses for the analogous OkHttp/JUL startup race. --- .../DeferredProfilingContextIntegration.java | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java index 9d9efa851e8..1d59ca0aaa2 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java @@ -1,5 +1,7 @@ package datadog.trace.bootstrap; +import static java.util.concurrent.TimeUnit.MILLISECONDS; + import datadog.trace.api.EndpointTracker; import datadog.trace.api.Stateful; import datadog.trace.api.profiling.ProfilingContextAttribute; @@ -24,6 +26,13 @@ * work by delegating to {@link ProfilingContextIntegration.NoOp} until the deferred construction * completes, then swapping in the real integration. * + *

Moving the work to another thread is not enough on its own, because that thread would still + * run concurrently with the rest of {@code premain}, i.e. still before {@code main} gets to set + * {@code java.nio.file.spi.DefaultFileSystemProvider}. The construction is therefore also delayed + * by {@link #INITIALIZATION_DELAY_MILLIS}. That delay is a mitigation, not a guarantee: the JVM + * offers no hook for "the application has entered {@code main}", so an application whose start-up + * is slower than the delay can still be racing with it. + * *

Scope events happening before the swap are silently dropped. That is acceptable for context * exposure (eBPF/CWS reading the current span off a thread), but not for profiling * accuracy, so users with the Datadog profiler actually enabled keep the synchronous construction @@ -33,6 +42,19 @@ final class DeferredProfilingContextIntegration implements ProfilingContextInteg private static final Logger log = LoggerFactory.getLogger(DeferredProfilingContextIntegration.class); + /** + * How long to wait before running the deferred construction, so that the rest of {@code premain} + * has returned and the application has had a chance to run the top of {@code main} (where an + * application that cares about it installs its own {@code + * java.nio.file.spi.DefaultFileSystemProvider}). + * + *

Same magnitude as the longest delay {@code Agent} already applies for the analogous "let the + * application get there first" problem with OkHttp and a custom log manager, and hardcoded for + * the same reason: this is context exposure for eBPF/CWS consumers, where losing the + * first second of thread context is not observable, so there is nothing for a user to tune. + */ + private static final long INITIALIZATION_DELAY_MILLIS = 1_000; + private final String name; private final Callable factory; @@ -54,9 +76,16 @@ final class DeferredProfilingContextIntegration implements ProfilingContextInteg this.factory = factory; } - /** Schedules the deferred construction so that it runs off the calling (premain) thread. */ + /** + * Schedules the deferred construction so that it runs off the calling (premain) thread, after + * {@link #INITIALIZATION_DELAY_MILLIS}. + * + *

The delay is the same order of magnitude as the one {@code Agent} already uses to let the + * application reach a given point before starting OkHttp when a custom log manager is in play. It + * is a heuristic, not a handshake: nothing here observes {@code main} actually starting. + */ void scheduleInitialization() { - AgentTaskScheduler.get().execute(this::initialize); + AgentTaskScheduler.get().schedule(this::initialize, INITIALIZATION_DELAY_MILLIS, MILLISECONDS); } /** From 68e2e09758370fa9e450fc0b4ceb66015f830b80 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 21 Sep 2026 15:18:41 +0200 Subject: [PATCH 07/21] review: defer profiling context engine tag until deferred construction succeeds - DeferredProfilingContextIntegration now exposes whenAvailable(Runnable), queuing callbacks until the real ddprof integration swaps in and never running them if construction fails. Default implementation runs inline, so the synchronous profiling/JFR path is unchanged. - CoreTracer stamps the _dd.profiling.ctx.engine tag through that callback instead of an identity check, so the tag is only ever set once the deferred integration actually becomes available, never unconditionally. - Bumped the deferred construction failure log to info, since it's the only signal that requested context exposure silently didn't happen. - Added useJUnitPlatform() to the springboot smoke test's testRuntimeActivation task, fixing the AppSec-inactive branch of OtelContextExposureSmokeTest never running in CI. --- .../DeferredProfilingContextIntegration.java | 50 ++++++++++- ...ferredProfilingContextIntegrationTest.java | 33 ++++++++ dd-smoke-tests/appsec/springboot/build.gradle | 3 + .../OtelContextExposureSmokeTest.groovy | 4 - .../java/datadog/trace/core/CoreTracer.java | 44 +++++++--- .../datadog/trace/core/CoreTracerTest.java | 83 +++++++++++++++++++ .../api/ProfilingContextIntegration.java | 15 ++++ 7 files changed, 215 insertions(+), 17 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java index 1d59ca0aaa2..716caeb5477 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java @@ -11,6 +11,8 @@ import datadog.trace.bootstrap.instrumentation.api.ProfilerContext; import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration; import datadog.trace.util.AgentTaskScheduler; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.Callable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,6 +39,11 @@ * exposure (eBPF/CWS reading the current span off a thread), but not for profiling * accuracy, so users with the Datadog profiler actually enabled keep the synchronous construction * path. + * + *

Because the deferred construction can also fail outright, this wrapper never claims to be the + * real engine until it is: {@link #whenAvailable(Runnable)} only fires after a successful swap, so + * consumers (such as the tracer stamping the {@code _dd.profiling.ctx} tag) do not advertise an + * engine that never materialized. */ final class DeferredProfilingContextIntegration implements ProfilingContextIntegration { private static final Logger log = @@ -65,6 +72,13 @@ final class DeferredProfilingContextIntegration implements ProfilingContextInteg */ private volatile ProfilingContextIntegration delegate = ProfilingContextIntegration.NoOp.INSTANCE; + /** + * Callbacks registered through {@link #whenAvailable(Runnable)} before the swap happened, to be + * run once it does. Guarded by {@code this}, together with the {@link #delegate} write, so that a + * callback registered concurrently with the swap is neither run twice nor dropped. + */ + private final List pendingAvailabilityCallbacks = new ArrayList<>(1); + /** * @param name the name reported by {@link #name()}, i.e. the name of the integration being * deferred. @@ -95,12 +109,44 @@ void scheduleInitialization() { void initialize() { try { final ProfilingContextIntegration integration = factory.call(); - if (integration != null) { + if (integration == null) { + return; + } + final List callbacks; + synchronized (this) { delegate = integration; + callbacks = new ArrayList<>(pendingAvailabilityCallbacks); + pendingAvailabilityCallbacks.clear(); + } + for (final Runnable callback : callbacks) { + try { + callback.run(); + } catch (final Throwable t) { + log.debug("Availability callback for {} profiling context failed.", name, t); + } } } catch (final Throwable t) { - log.debug("Deferred {} profiling context labeling not available. {}", name, t.getMessage()); + // Reaching this point means context exposure was requested and is silently not happening, + // and there is no other signal for it. The throwable is rendered with toString() because + // the common failures here (UnsatisfiedLinkError and friends) carry no message. + log.info("Deferred {} profiling context labeling not available. {}", name, t.toString()); + } + } + + /** + * Runs {@code callback} once the real integration has been swapped in, or immediately if that + * already happened. If the deferred construction fails, the callback is never run: consumers must + * treat "not yet available" and "never available" the same way. + */ + @Override + public void whenAvailable(final Runnable callback) { + synchronized (this) { + if (delegate == ProfilingContextIntegration.NoOp.INSTANCE) { + pendingAvailabilityCallbacks.add(callback); + return; + } } + callback.run(); } /** diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java index b110b7d6864..e06fb6ccb9d 100644 --- a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java @@ -125,6 +125,39 @@ void staysNoOpWhenTheDeferredConstructionFails() { assertEquals("ddprof", deferred.name()); } + @Test + void availabilityCallbacksRunOnlyOnceTheRealIntegrationIsIn() { + DeferredProfilingContextIntegration deferred = + new DeferredProfilingContextIntegration("ddprof", FakeDatadogProfilingIntegration::new); + AtomicInteger callbacks = new AtomicInteger(); + + deferred.whenAvailable(callbacks::incrementAndGet); + assertEquals(0, callbacks.get()); + + deferred.initialize(); + assertEquals(1, callbacks.get()); + + // registering after the swap runs the callback straight away, without waiting for anything + deferred.whenAvailable(callbacks::incrementAndGet); + assertEquals(2, callbacks.get()); + } + + @Test + void availabilityCallbacksNeverRunWhenTheDeferredConstructionFails() { + DeferredProfilingContextIntegration deferred = + new DeferredProfilingContextIntegration( + "ddprof", + () -> { + throw new UnsatisfiedLinkError("no native library here"); + }); + AtomicInteger callbacks = new AtomicInteger(); + + deferred.whenAvailable(callbacks::incrementAndGet); + deferred.initialize(); + + assertEquals(0, callbacks.get()); + } + private static ClassLoader fakeProfilingClassLoader() { return new ClassLoader(null) { @Override diff --git a/dd-smoke-tests/appsec/springboot/build.gradle b/dd-smoke-tests/appsec/springboot/build.gradle index cef43d450b5..a8ce97071a1 100644 --- a/dd-smoke-tests/appsec/springboot/build.gradle +++ b/dd-smoke-tests/appsec/springboot/build.gradle @@ -54,6 +54,9 @@ tasks.withType(Test).configureEach { tasks.register('testRuntimeActivation', Test) { def shadowJarTask = tasks.named('shadowJar', ShadowJar) + // Without this, the task keeps Gradle's default JUnit 4 runner, which discovers none of the + // Spock specs in this module, so the task passes without running anything. + useJUnitPlatform() jvmArgs '-Dsmoke_test.appsec.enabled=inactive' jvmArgumentProviders.add(new CommandLineArgumentProvider() { @Override diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy index 41cbaaaa122..db896911404 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy @@ -32,10 +32,6 @@ class OtelContextExposureSmokeTest extends AbstractAppSecServerSmokeTest { processBuilder.directory(new File(buildDirectory)) } - // TODO(APPSEC-70254): the `testRuntimeActivation` Gradle task (which sets - // `smoke_test.appsec.enabled=inactive`) does not discover this or any other Spock spec today, - // so the `appSecFullyEnabled == false` branch below is not actually exercised in CI. See the - // ticket for the pre-existing root cause (missing `useJUnitPlatform()` on that task). void 'OTel process context registration follows AppSec activation, not profiling'() { given: boolean appSecFullyEnabled = System.getProperty('smoke_test.appsec.enabled') != 'inactive' diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index e9e130847b9..14517e13fb7 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -200,10 +200,17 @@ public static CoreTracerBuilder builder() { /** Maintains dynamic configuration associated with the tracer */ private final DynamicConfig dynamicConfig; - /** A set of tags that are added only to the application's root span */ - private final TagMap localRootSpanTags; + /** + * A set of tags that are added only to the application's root span. + * + *

Written once in the constructor and, for a profiling context integration whose construction + * is deferred, once more when that construction succeeds (see {@link + * #stampProfilingContextEngine()}), hence volatile. The map itself is always frozen, so readers + * only ever see a fully built, immutable snapshot. + */ + private volatile TagMap localRootSpanTags; - private final boolean localRootSpanTagsNeedIntercept; + private volatile boolean localRootSpanTagsNeedIntercept; /** * When {@code false}, every exported span is stamped with the {@code _dd.apm.enabled:0} billing @@ -924,16 +931,15 @@ private CoreTracer( this.injectLinksAsTags = injectLinksAsTags; this.flushOnClose = flushOnClose; this.allowInferredServices = SpanNaming.instance().namingSchema().allowInferredServices(); - if (profilingContextIntegration != ProfilingContextIntegration.NoOp.INSTANCE) { - TagMap tmp = TagMap.fromMap(localRootSpanTags); - tmp.set(PROFILING_CONTEXT_ENGINE, profilingContextIntegration.name()); - this.localRootSpanTags = tmp.freeze(); - } else { - this.localRootSpanTags = TagMap.fromMapImmutable(localRootSpanTags); - } - + this.localRootSpanTags = TagMap.fromMapImmutable(localRootSpanTags); this.localRootSpanTagsNeedIntercept = this.tagInterceptor.needsIntercept(this.localRootSpanTags); + if (profilingContextIntegration != ProfilingContextIntegration.NoOp.INSTANCE) { + // The engine tag is stamped only once the integration can really label context. Integrations + // that are ready when they are handed out run this inline, right here; an integration whose + // construction is deferred runs it later, and not at all if that construction fails. + profilingContextIntegration.whenAvailable(this::stampProfilingContextEngine); + } if (serviceDiscoveryFactory != null) { AgentTaskScheduler.get() .schedule( @@ -951,6 +957,22 @@ private CoreTracer( } } + /** + * Adds the profiling context engine tag to the local root span tags. + * + *

Runs at most once per tracer, either inline from the constructor (integration already + * available) or on the thread that completes a deferred integration's construction. The tag is + * kept in the pre-frozen {@link #localRootSpanTags} rather than evaluated per span, so root span + * creation pays nothing beyond the volatile read it already does. + */ + private void stampProfilingContextEngine() { + TagMap tags = TagMap.fromMap(this.localRootSpanTags); + tags.set(PROFILING_CONTEXT_ENGINE, this.profilingContextIntegration.name()); + this.localRootSpanTags = tags.freeze(); + this.localRootSpanTagsNeedIntercept = + this.tagInterceptor.needsIntercept(this.localRootSpanTags); + } + private void startMetricsAggregation(Config config, SharedCommunicationObjects sco) { metricsAggregator = createMetricsAggregator(config, sco, this.healthMetrics); // Schedule the metrics aggregator to begin reporting after a random delay of diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index db850470da0..96828c9114c 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -24,12 +24,16 @@ import datadog.remoteconfig.state.ProductListener; import datadog.trace.api.Config; import datadog.trace.api.DDTags; +import datadog.trace.api.EndpointTracker; import datadog.trace.api.config.GeneralConfig; import datadog.trace.api.config.TracerConfig; +import datadog.trace.api.profiling.Timer.TimerType; +import datadog.trace.api.profiling.Timing; import datadog.trace.api.remoteconfig.ServiceNameCollector; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.time.ControllableTimeSource; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration; import datadog.trace.bootstrap.instrumentation.api.ServiceNameSources; import datadog.trace.common.sampling.AllSampler; import datadog.trace.common.sampling.RateByServiceTraceSampler; @@ -59,6 +63,8 @@ @Timeout(value = 10, unit = TimeUnit.SECONDS) public class CoreTracerTest extends DDCoreJavaSpecification { + private static final String FAKE_ENGINE = "fake-engine"; + @BeforeAll static void checkJvm() { assumeFalse( @@ -344,6 +350,39 @@ void rootTagsAppliedOnlyToRootSpans() { } } + @Test + void profilingContextEngineTagStampedWhenTheIntegrationIsAlreadyAvailable() { + CoreTracer tracer = + tracerBuilder().profilingContextIntegration(new FakeContextIntegration()).build(); + AgentSpan root = tracer.buildSpan("datadog", "my_root").start(); + try { + assertEquals(FAKE_ENGINE, root.getTags().get(DDTags.PROFILING_CONTEXT_ENGINE)); + } finally { + root.finish(); + tracer.close(); + } + } + + @Test + void profilingContextEngineTagWithheldUntilTheIntegrationBecomesAvailable() { + FakeContextIntegration integration = new FakeContextIntegration(); + integration.deferAvailability = true; + CoreTracer tracer = tracerBuilder().profilingContextIntegration(integration).build(); + try { + AgentSpan beforeSwap = tracer.buildSpan("datadog", "before").start(); + assertFalse(beforeSwap.getTags().containsKey(DDTags.PROFILING_CONTEXT_ENGINE)); + beforeSwap.finish(); + + integration.becomeAvailable(); + + AgentSpan afterSwap = tracer.buildSpan("datadog", "after").start(); + assertEquals(FAKE_ENGINE, afterSwap.getTags().get(DDTags.PROFILING_CONTEXT_ENGINE)); + afterSwap.finish(); + } finally { + tracer.close(); + } + } + @Test void prioritySamplingWhenSpanFinishes() throws Exception { ListWriter writer = new ListWriter(); @@ -756,6 +795,50 @@ private static Map buildStringMap(String... keyValues) { // --- inner classes --- + /** + * A profiling context integration whose availability can be released after the tracer has been + * built, the way an integration whose construction is deferred off the premain thread does. + */ + static class FakeContextIntegration implements ProfilingContextIntegration { + boolean deferAvailability; + private Runnable availabilityCallback; + + @Override + public String name() { + return FAKE_ENGINE; + } + + @Override + public void onRootSpanFinished(AgentSpan rootSpan, EndpointTracker tracker) {} + + @Override + public EndpointTracker onRootSpanStarted(AgentSpan rootSpan) { + return EndpointTracker.NO_OP; + } + + @Override + public Timing start(TimerType type) { + return Timing.NoOp.INSTANCE; + } + + @Override + public void whenAvailable(Runnable callback) { + if (deferAvailability) { + this.availabilityCallback = callback; + } else { + callback.run(); + } + } + + void becomeAvailable() { + Runnable callback = this.availabilityCallback; + this.availabilityCallback = null; + if (callback != null) { + callback.run(); + } + } + } + static class WriterWithExplicitFlush implements datadog.trace.common.writer.Writer { final List> writtenTraces = new CopyOnWriteArrayList<>(); final List> flushedTraces = new CopyOnWriteArrayList<>(); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ProfilingContextIntegration.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ProfilingContextIntegration.java index 20b41f4d24e..75d11aa6752 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ProfilingContextIntegration.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ProfilingContextIntegration.java @@ -40,6 +40,21 @@ default int encodeResourceName(CharSequence constant) { String name(); + /** + * Registers a one-shot callback to run once this integration is actually able to label context. + * + *

Implementations that are fully built by the time they are handed out (the common case) are + * available immediately and run the callback inline. An implementation whose construction is + * deferred runs it later, on the thread that completes that construction, and never runs it if + * the construction fails. Consumers use this to publish metadata about the engine (such as the + * {@code _dd.profiling.ctx} tag) only when there really is an engine behind it. + * + * @param callback invoked at most once, possibly on an arbitrary thread. + */ + default void whenAvailable(Runnable callback) { + callback.run(); + } + final class NoOp implements ProfilingContextIntegration { public static final ProfilingContextIntegration INSTANCE = From 271e8cb096a9e6002ee173d96912f3c2fb66dfc2 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Mon, 21 Sep 2026 19:15:03 +0200 Subject: [PATCH 08/21] review: limit testRuntimeActivation to the OTel exposure spec useJUnitPlatform() made the task discover every Spock spec in the source set, but it runs with AppSec forced inactive, which the other specs (e.g. SpringBootSmokeTest's 403 blocking assertions) don't tolerate. Filter the task to OtelContextExposureSmokeTest only. --- dd-smoke-tests/appsec/springboot/build.gradle | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dd-smoke-tests/appsec/springboot/build.gradle b/dd-smoke-tests/appsec/springboot/build.gradle index a8ce97071a1..cd0b7407d97 100644 --- a/dd-smoke-tests/appsec/springboot/build.gradle +++ b/dd-smoke-tests/appsec/springboot/build.gradle @@ -57,6 +57,12 @@ tasks.register('testRuntimeActivation', Test) { // Without this, the task keeps Gradle's default JUnit 4 runner, which discovers none of the // Spock specs in this module, so the task passes without running anything. useJUnitPlatform() + // Limit to the exposure spec: the other specs in this source set expect AppSec active (e.g. + // SpringBootSmokeTest asserts 403 blocking responses), which is incompatible with the inactive + // mode this task runs under. + filter { + includeTestsMatching 'datadog.smoketest.appsec.OtelContextExposureSmokeTest' + } jvmArgs '-Dsmoke_test.appsec.enabled=inactive' jvmArgumentProviders.add(new CommandLineArgumentProvider() { @Override From 6b1c10df6fe82d0c9cff2ed784e3e277ff241778 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 22 Sep 2026 16:13:08 +0200 Subject: [PATCH 09/21] review: fix thread-safety race in CoreTracer local root span tags - Merge localRootSpanTags/localRootSpanTagsNeedIntercept into a single immutable LocalRootSpanTags holder published through one volatile reference, so a concurrent startSpan() never observes the new tag map paired with the stale intercept flag (introduced by this PR's deferred profiling context construction) --- .../java/datadog/trace/bootstrap/Agent.java | 111 +++++++++++++----- .../DeferredProfilingContextIntegration.java | 5 + ...ferredProfilingContextIntegrationTest.java | 49 +++++++- .../java/com/datadog/appsec/AppSecSystem.java | 4 +- .../OtelContextExposureSmokeTest.groovy | 2 + .../java/datadog/trace/core/CoreTracer.java | 40 ++++--- .../main/java/datadog/trace/api/Config.java | 24 ++++ .../trace/bootstrap/ActiveSubsystems.java | 68 +++++++++++ .../api/ConfigOtelContextExposureTest.java | 40 +++++++ .../trace/bootstrap/ActiveSubsystemsTest.java | 91 ++++++++++++++ 10 files changed, 386 insertions(+), 48 deletions(-) create mode 100644 internal-api/src/test/java/datadog/trace/bootstrap/ActiveSubsystemsTest.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index 881f68ffc4c..7ee49241782 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -1493,20 +1493,23 @@ public void withTracer(TracerAPI tracer) { */ private static ProfilingContextIntegration createProfilingContextIntegration() { Config config = Config.get(); - // isDatadogProfilerEnabled() is ORed in explicitly so a user with real profiling enabled keeps - // ddprof regardless of the AppSec activation level that otherwise drives - // isOtelContextExposureEnabled() - additive, not a replacement gate. - if ((config.isDatadogProfilerEnabled() || config.isOtelContextExposureEnabled()) - && !OperatingSystem.isWindows()) { - // When the ddprof integration is triggered by context exposure alone (profiling disabled), - // its construction is deferred off the premain thread: it loads the ddprof native library - // and touches java.nio.file, which must not happen on the primordial premain thread. Users - // with the profiler actually enabled keep the synchronous path, since profiling accuracy - // requires seeing every scope from the very first one. - ProfilingContextIntegration integration = - createDdprofContextIntegration(AGENT_CLASSLOADER, !config.isDatadogProfilerEnabled()); - if (integration != null) { - return integration; + if (!OperatingSystem.isWindows()) { + // isDatadogProfilerEnabled() is ORed in explicitly so a user with real profiling enabled + // keeps ddprof regardless of the AppSec activation level that otherwise drives + // isOtelContextExposureEnabled() - additive, not a replacement gate. + if (config.isDatadogProfilerEnabled() || config.isOtelContextExposureEnabled()) { + // When the ddprof integration is triggered by context exposure alone (profiling disabled), + // its construction is deferred off the premain thread: it loads the ddprof native library + // and touches java.nio.file, which must not happen on the primordial premain thread. Users + // with the profiler actually enabled keep the synchronous path, since profiling accuracy + // requires seeing every scope from the very first one. + ProfilingContextIntegration integration = + createDdprofContextIntegration(AGENT_CLASSLOADER, !config.isDatadogProfilerEnabled()); + if (integration != null) { + return integration; + } + } else if (config.isOtelContextExposurePendingAppSecActivation()) { + return createAppSecActivatedDdprofContextIntegration(AGENT_CLASSLOADER); } } if (config.isProfilingEnabled() && config.isProfilingTimelineEventsEnabled()) { @@ -1537,24 +1540,12 @@ private static ProfilingContextIntegration createProfilingContextIntegration() { */ static ProfilingContextIntegration createDdprofContextIntegration( final ClassLoader classLoader, final boolean deferInitialization) { + // deferInitialization is exactly "the profiler itself is not running", which is also exactly + // when nobody else registers the process context: ProfilingAgent.run() already does it when + // the profiler starts. Registering it here as well in the profiler-enabled case would log and + // call into the native library twice for every user that has profiling on today. Callable factory = - () -> { - ProfilingContextIntegration integration = - (ProfilingContextIntegration) - classLoader - .loadClass("com.datadog.profiling.ddprof.DatadogProfilingIntegration") - .getDeclaredConstructor() - .newInstance(); - try { - classLoader - .loadClass("com.datadog.profiling.agent.ProcessContext") - .getMethod("register", ConfigProvider.class) - .invoke(null, ConfigProvider.getInstance()); - } catch (Throwable t) { - log.debug("Process context registration not available. {}", t.getMessage()); - } - return integration; - }; + ddprofContextIntegrationFactory(classLoader, deferInitialization); if (deferInitialization) { DeferredProfilingContextIntegration deferred = new DeferredProfilingContextIntegration("ddprof", factory); @@ -1569,6 +1560,64 @@ static ProfilingContextIntegration createDdprofContextIntegration( } } + /** + * Creates a ddprof context integration that stays a no-op until AppSec is activated at runtime + * through remote config, and only then builds the real one. + * + *

This covers {@code DD_APPSEC_ENABLED=inactive}, the "one-click" activation flow, where the + * boot-time activation level stays {@link datadog.trace.api.ProductActivation#ENABLED_INACTIVE} + * forever and only a runtime flag flips. Profiling is off in this case (otherwise the caller took + * the branch above), so the profiler never registers the process context either and the deferred + * construction is responsible for it. + * + *

The activation callback runs on the remote-config poller thread, so it only schedules the + * construction rather than doing it inline. Deactivation is deliberately not handled: the context + * exposure is a one-time process-wide registration, and tearing the native context down when + * AppSec is switched back off is out of scope. + * + * @param classLoader the agent class loader used to reach the profiling classes. + * @return the integration, which is never {@code null}: nothing can fail synchronously here. + */ + static ProfilingContextIntegration createAppSecActivatedDdprofContextIntegration( + final ClassLoader classLoader) { + DeferredProfilingContextIntegration deferred = + new DeferredProfilingContextIntegration( + "ddprof", ddprofContextIntegrationFactory(classLoader, true)); + ActiveSubsystems.whenAppSecActivated(deferred::scheduleInitialization); + return deferred; + } + + /** + * Builds the ddprof integration reflectively, optionally registering the OTel process context + * alongside it. + * + * @param classLoader the agent class loader used to reach the profiling classes. + * @param registerProcessContext whether this factory also has to register the process context, + * i.e. whether the profiler agent, which registers it on its own, is not going to start. + */ + private static Callable ddprofContextIntegrationFactory( + final ClassLoader classLoader, final boolean registerProcessContext) { + return () -> { + ProfilingContextIntegration integration = + (ProfilingContextIntegration) + classLoader + .loadClass("com.datadog.profiling.ddprof.DatadogProfilingIntegration") + .getDeclaredConstructor() + .newInstance(); + if (registerProcessContext) { + try { + classLoader + .loadClass("com.datadog.profiling.agent.ProcessContext") + .getMethod("register", ConfigProvider.class) + .invoke(null, ConfigProvider.getInstance()); + } catch (Throwable t) { + log.debug("Process context registration not available. {}", t.getMessage()); + } + } + return integration; + }; + } + private static boolean startProfilingAgent( final boolean earlyStart, final boolean firstAttempt, Instrumentation inst) { if (isAwsLambdaRuntime()) { diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java index 716caeb5477..81543816c22 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java @@ -35,6 +35,11 @@ * offers no hook for "the application has entered {@code main}", so an application whose start-up * is slower than the delay can still be racing with it. * + *

The same wrapper also covers the other case where the integration cannot exist yet at {@code + * premain} time: AppSec started as {@code inactive} and only activated later through remote config. + * There the construction is not scheduled up front but when the activation arrives, which may be + * minutes into the run. + * *

Scope events happening before the swap are silently dropped. That is acceptable for context * exposure (eBPF/CWS reading the current span off a thread), but not for profiling * accuracy, so users with the Datadog profiler actually enabled keep the synchronous construction diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java index e06fb6ccb9d..ae69dd75d4b 100644 --- a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java @@ -26,7 +26,9 @@ /** * Covers the premain-timing contract of the ddprof profiling context integration: the AppSec-only * trigger must not construct it (nor register the process context) on the calling thread, while the - * profiler-enabled path must keep doing exactly that. + * profiler-enabled path must keep constructing it there and must leave the process context + * registration to the profiler agent. Also covers the AppSec runtime-activation trigger, where the + * construction only happens once remote config turns AppSec on. */ class DeferredProfilingContextIntegrationTest { @@ -78,10 +80,53 @@ void synchronousConstructionKeepsRunningOnTheCallingThread() { assertTrue(integration instanceof FakeDatadogProfilingIntegration); assertEquals(1, FakeDatadogProfilingIntegration.constructions.get()); - assertEquals(1, FakeProcessContext.registrations.get()); assertSame(Thread.currentThread(), FakeDatadogProfilingIntegration.constructionThread.get()); } + /** + * The synchronous path is only taken when the Datadog profiler is actually enabled, and in that + * case {@code ProfilingAgent.run()} registers the process context on its own, as it always has. + * Registering it here as well would log "Registering process context for OTel profiler" twice and + * call into the native library twice for every user that already has profiling on. + */ + @Test + void synchronousConstructionLeavesTheProcessContextToTheProfilerAgent() { + Agent.createDdprofContextIntegration(fakeProfilingClassLoader(), false); + + assertEquals(0, FakeProcessContext.registrations.get()); + } + + @Test + void appSecActivatedConstructionWaitsForTheRuntimeActivation() throws Exception { + boolean originalAppSecActive = ActiveSubsystems.APPSEC_ACTIVE; + ActiveSubsystems.APPSEC_ACTIVE = false; + try { + ProfilingContextIntegration integration = + Agent.createAppSecActivatedDdprofContextIntegration(fakeProfilingClassLoader()); + + // AppSec is only "inactive-enabled" so far: nothing may be constructed yet + assertNotNull(integration); + assertEquals("ddprof", integration.name()); + assertEquals(0, FakeDatadogProfilingIntegration.constructions.get()); + assertEquals(0, FakeProcessContext.registrations.get()); + assertSame(Stateful.DEFAULT, integration.newScopeState(null)); + + ActiveSubsystems.setAppSecActive(true); + + assertTrue( + FakeProcessContext.registered.await(30, TimeUnit.SECONDS), + "the activation never triggered the deferred construction"); + assertEquals(1, FakeDatadogProfilingIntegration.constructions.get()); + // nothing else registers the process context here, because the profiler never starts + assertEquals(1, FakeProcessContext.registrations.get()); + assertNotSame( + Thread.currentThread(), FakeDatadogProfilingIntegration.constructionThread.get()); + assertSame(FakeDatadogProfilingIntegration.STATE, integration.newScopeState(null)); + } finally { + ActiveSubsystems.APPSEC_ACTIVE = originalAppSecActive; + } + } + @Test void delegatesToTheRealIntegrationOnceInitialized() { DeferredProfilingContextIntegration deferred = diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java index d7d780123ea..dc1f7ee01bb 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java @@ -118,7 +118,9 @@ public static boolean isActive() { } public static void setActive(boolean status) { - ActiveSubsystems.APPSEC_ACTIVE = status; + // Goes through the setter rather than the field so that components outside the AppSec module, + // which cannot see this class, still observe a runtime (remote-config driven) activation. + ActiveSubsystems.setAppSecActive(status); // Report to the product change via telemetry log.debug("AppSec is now {}", status ? "active" : "inactive"); ProductChangeCollector.get() diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy index db896911404..6fb19d9efc3 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy @@ -43,6 +43,8 @@ class OtelContextExposureSmokeTest extends AbstractAppSecServerSmokeTest { assert new File(logFilePath).text.contains(PROCESS_CONTEXT_LOG_LINE) } } else { + // AppSec is only "inactive-enabled" here and no remote config ever activates it, so the + // integration stays armed and never registers anything. // Give the agent the same startup time as the positive case before asserting absence, // so a slow-starting agent can't produce a false negative. conditions.eventually { diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 14517e13fb7..333e9993007 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -201,16 +201,26 @@ public static CoreTracerBuilder builder() { private final DynamicConfig dynamicConfig; /** - * A set of tags that are added only to the application's root span. + * A set of tags that are added only to the application's root span, paired with whether that set + * needs interception. * *

Written once in the constructor and, for a profiling context integration whose construction * is deferred, once more when that construction succeeds (see {@link - * #stampProfilingContextEngine()}), hence volatile. The map itself is always frozen, so readers - * only ever see a fully built, immutable snapshot. + * #stampProfilingContextEngine()}). The pair is replaced together as a single immutable holder, + * read through one volatile reference, so a concurrent root span creation never observes the new + * tag map alongside the stale intercept flag (or vice versa). */ - private volatile TagMap localRootSpanTags; + private static final class LocalRootSpanTags { + final TagMap tags; + final boolean needsIntercept; - private volatile boolean localRootSpanTagsNeedIntercept; + LocalRootSpanTags(final TagMap tags, final boolean needsIntercept) { + this.tags = tags; + this.needsIntercept = needsIntercept; + } + } + + private volatile LocalRootSpanTags localRootSpanTags; /** * When {@code false}, every exported span is stamped with the {@code _dd.apm.enabled:0} billing @@ -931,9 +941,10 @@ private CoreTracer( this.injectLinksAsTags = injectLinksAsTags; this.flushOnClose = flushOnClose; this.allowInferredServices = SpanNaming.instance().namingSchema().allowInferredServices(); - this.localRootSpanTags = TagMap.fromMapImmutable(localRootSpanTags); - this.localRootSpanTagsNeedIntercept = - this.tagInterceptor.needsIntercept(this.localRootSpanTags); + final TagMap frozenLocalRootSpanTags = TagMap.fromMapImmutable(localRootSpanTags); + this.localRootSpanTags = + new LocalRootSpanTags( + frozenLocalRootSpanTags, this.tagInterceptor.needsIntercept(frozenLocalRootSpanTags)); if (profilingContextIntegration != ProfilingContextIntegration.NoOp.INSTANCE) { // The engine tag is stamped only once the integration can really label context. Integrations // that are ready when they are handed out run this inline, right here; an integration whose @@ -966,11 +977,11 @@ private CoreTracer( * creation pays nothing beyond the volatile read it already does. */ private void stampProfilingContextEngine() { - TagMap tags = TagMap.fromMap(this.localRootSpanTags); + TagMap tags = TagMap.fromMap(this.localRootSpanTags.tags); tags.set(PROFILING_CONTEXT_ENGINE, this.profilingContextIntegration.name()); - this.localRootSpanTags = tags.freeze(); - this.localRootSpanTagsNeedIntercept = - this.tagInterceptor.needsIntercept(this.localRootSpanTags); + TagMap frozenTags = tags.freeze(); + this.localRootSpanTags = + new LocalRootSpanTags(frozenTags, this.tagInterceptor.needsIntercept(frozenTags)); } private void startMetricsAggregation(Config config, SharedCommunicationObjects sco) { @@ -2224,8 +2235,9 @@ protected static final DDSpanContext buildSpanContext( ciVisibilityContextData = null; } - rootSpanTags = tracer.localRootSpanTags; - rootSpanTagsNeedsIntercept = tracer.localRootSpanTagsNeedIntercept; + LocalRootSpanTags localRootSpanTags = tracer.localRootSpanTags; + rootSpanTags = localRootSpanTags.tags; + rootSpanTagsNeedsIntercept = localRootSpanTags.needsIntercept; parentTraceCollector = tracer.createTraceCollector(traceId, traceConfig); diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 997cabbf764..449f08e960d 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -1081,6 +1081,7 @@ public static String getHostName() { private final boolean profilingAgentless; private final boolean isDatadogProfilerEnabled; private final boolean otelContextExposureEnabled; + private final boolean otelContextExposurePendingAppSecActivation; @Deprecated private final String profilingUrl; private final Map profilingTags; private final int profilingStartDelay; @@ -2653,6 +2654,14 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) && (isProfilingEnabled() || instrumenterConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED); + // AppSec started as ENABLED_INACTIVE is the standard "one-click" flow: the activation level + // itself never changes afterwards, only the runtime flag AppSec flips when remote config + // turns it on. Context exposure therefore cannot be decided here, it can only be armed. + this.otelContextExposurePendingAppSecActivation = + isDatadogProfilerSafeAndConfigured() + && !otelContextExposureEnabled + && instrumenterConfig.getAppSecActivation() == ProductActivation.ENABLED_INACTIVE; + this.traceResourceRenamingAlwaysSimplifiedEndpoint = configProvider.getBoolean(TRACE_RESOURCE_RENAMING_ALWAYS_SIMPLIFIED_ENDPOINT, false); @@ -4281,6 +4290,19 @@ public boolean isOtelContextExposureEnabled() { return otelContextExposureEnabled; } + /** + * Whether OpenTelemetry context exposure is not enabled at boot but must be armed so it can start + * later, when AppSec is activated at runtime through remote config. + * + *

{@link #isOtelContextExposureEnabled()} can only look at {@code getAppSecActivation()}, + * which is read once from {@code InstrumenterConfig} and never changes. The "one-click" AppSec + * activation flow leaves it at {@link ProductActivation#ENABLED_INACTIVE} forever and flips a + * separate runtime flag instead, so that flow would never be observed without this. + */ + public boolean isOtelContextExposurePendingAppSecActivation() { + return otelContextExposurePendingAppSecActivation; + } + public static boolean isDatadogProfilerEnablementOverridden() { // old non-LTS versions without important backports // also, we have no windows binaries @@ -6823,6 +6845,8 @@ public String toString() { + profilingExcludeAgentThreads + ", otelContextExposureEnabled=" + otelContextExposureEnabled + + ", otelContextExposurePendingAppSecActivation=" + + otelContextExposurePendingAppSecActivation + ", crashTrackingTags=" + crashTrackingTags + ", crashTrackingAgentless=" diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/ActiveSubsystems.java b/internal-api/src/main/java/datadog/trace/bootstrap/ActiveSubsystems.java index 2a43c32c3f3..9d52a4500e4 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/ActiveSubsystems.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/ActiveSubsystems.java @@ -1,5 +1,73 @@ package datadog.trace.bootstrap; +import java.util.ArrayList; +import java.util.List; + +/** Runtime activation state of the subsystems that can be switched on and off after start-up. */ public class ActiveSubsystems { + /** + * Whether AppSec is currently active. Unlike {@code Config.getAppSecActivation()}, which is fixed + * at boot, this flag follows the remote-config "one-click" activation flow, so it can flip at any + * time while the application runs. + * + *

Kept public and directly writable because it is read from hot paths and from tests that set + * it up by hand. Production code that owns the transition must go through {@link + * #setAppSecActive(boolean)} instead, so the activation callbacks below are honoured. + */ public static volatile boolean APPSEC_ACTIVE; + + /** + * One-shot callbacks waiting for AppSec to become active, registered through {@link + * #whenAppSecActivated(Runnable)}. Guarded by its own monitor, which also guards the read of + * {@link #APPSEC_ACTIVE} on the registration side, so a callback registered concurrently with an + * activation is neither run twice nor dropped. + */ + private static final List APPSEC_ACTIVATION_CALLBACKS = new ArrayList<>(1); + + /** + * Updates {@link #APPSEC_ACTIVE} and, on a transition into the active state, runs the pending + * activation callbacks. + * + *

Callbacks fire on the caller's thread, which for the remote-config flow is the configuration + * poller thread, so they must not block. They fire at most once for the lifetime of the process: + * AppSec can be activated and deactivated repeatedly through remote config, but the consumers + * here are one-time initializations. + */ + public static void setAppSecActive(final boolean active) { + APPSEC_ACTIVE = active; + if (!active) { + return; + } + final List callbacks; + synchronized (APPSEC_ACTIVATION_CALLBACKS) { + if (APPSEC_ACTIVATION_CALLBACKS.isEmpty()) { + return; + } + callbacks = new ArrayList<>(APPSEC_ACTIVATION_CALLBACKS); + APPSEC_ACTIVATION_CALLBACKS.clear(); + } + for (final Runnable callback : callbacks) { + try { + callback.run(); + } catch (final Throwable ignored) { + // A failing callback must never prevent AppSec from becoming active, nor stop the + // remaining callbacks. There is deliberately no logger here: this class is loaded very + // early and from hot paths, and each callback is expected to report its own failures. + } + } + } + + /** + * Runs {@code callback} once AppSec becomes active, or immediately if it already is. The callback + * is run at most once, and is never run if AppSec never becomes active. + */ + public static void whenAppSecActivated(final Runnable callback) { + synchronized (APPSEC_ACTIVATION_CALLBACKS) { + if (!APPSEC_ACTIVE) { + APPSEC_ACTIVATION_CALLBACKS.add(callback); + return; + } + } + callback.run(); + } } diff --git a/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java index 198f3f6ac59..9704da837e5 100644 --- a/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java +++ b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java @@ -62,6 +62,46 @@ void disabledWhenAppSecIsOnlyEnabledInactive() { assertFalse(Config.get().isOtelContextExposureEnabled()); } + @Test + @WithConfig(key = APPSEC_ENABLED, value = "inactive") + @WithConfig(key = PROFILING_ENABLED, value = "false") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") + void pendingWhenAppSecIsOnlyEnabledInactive() { + assumeDatadogProfilerNotVetoed(); + + // "inactive" is the one-click flow: nothing is exposed yet, but it has to be armed, because + // the activation level itself never changes once remote config turns AppSec on. + assertTrue(Config.get().isOtelContextExposurePendingAppSecActivation()); + } + + @Test + @WithConfig(key = APPSEC_ENABLED, value = "true") + @WithConfig(key = PROFILING_ENABLED, value = "false") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") + void notPendingWhenAlreadyEnabledAtBoot() { + assumeDatadogProfilerNotVetoed(); + + Config config = Config.get(); + assertTrue(config.isOtelContextExposureEnabled()); + assertFalse(config.isOtelContextExposurePendingAppSecActivation()); + } + + @Test + @WithConfig(key = APPSEC_ENABLED, value = "false") + @WithConfig(key = PROFILING_ENABLED, value = "false") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") + void notPendingWhenAppSecIsFullyDisabled() { + assertFalse(Config.get().isOtelContextExposurePendingAppSecActivation()); + } + + @Test + @WithConfig(key = APPSEC_ENABLED, value = "inactive") + @WithConfig(key = PROFILING_ENABLED, value = "false") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "false") + void notPendingWhenDatadogProfilerIsExplicitlyDisabled() { + assertFalse(Config.get().isOtelContextExposurePendingAppSecActivation()); + } + @Test @WithConfig(key = APPSEC_ENABLED, value = "true") @WithConfig(key = PROFILING_ENABLED, value = "false") diff --git a/internal-api/src/test/java/datadog/trace/bootstrap/ActiveSubsystemsTest.java b/internal-api/src/test/java/datadog/trace/bootstrap/ActiveSubsystemsTest.java new file mode 100644 index 00000000000..61b94c12075 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/bootstrap/ActiveSubsystemsTest.java @@ -0,0 +1,91 @@ +package datadog.trace.bootstrap; + +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 java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Covers the AppSec runtime-activation callbacks, the only signal other subsystems get for the + * remote-config "one-click" activation flow, where the boot-time activation level never changes. + */ +class ActiveSubsystemsTest { + + private boolean originalAppSecActive; + + @BeforeEach + void saveState() { + originalAppSecActive = ActiveSubsystems.APPSEC_ACTIVE; + ActiveSubsystems.APPSEC_ACTIVE = false; + } + + @AfterEach + void restoreState() { + ActiveSubsystems.APPSEC_ACTIVE = originalAppSecActive; + } + + @Test + void callbackRunsWhenAppSecBecomesActive() { + AtomicInteger runs = new AtomicInteger(); + + ActiveSubsystems.whenAppSecActivated(runs::incrementAndGet); + assertEquals(0, runs.get()); + + ActiveSubsystems.setAppSecActive(true); + + assertTrue(ActiveSubsystems.APPSEC_ACTIVE); + assertEquals(1, runs.get()); + } + + @Test + void callbackRunsAtMostOnceAcrossRepeatedToggles() { + AtomicInteger runs = new AtomicInteger(); + ActiveSubsystems.whenAppSecActivated(runs::incrementAndGet); + + ActiveSubsystems.setAppSecActive(true); + ActiveSubsystems.setAppSecActive(false); + ActiveSubsystems.setAppSecActive(true); + + assertEquals(1, runs.get()); + } + + @Test + void callbackRunsImmediatelyWhenAppSecIsAlreadyActive() { + AtomicInteger runs = new AtomicInteger(); + ActiveSubsystems.setAppSecActive(true); + + ActiveSubsystems.whenAppSecActivated(runs::incrementAndGet); + + assertEquals(1, runs.get()); + } + + @Test + void callbackNeverRunsWhileAppSecStaysInactive() { + AtomicInteger runs = new AtomicInteger(); + + ActiveSubsystems.whenAppSecActivated(runs::incrementAndGet); + ActiveSubsystems.setAppSecActive(false); + + assertFalse(ActiveSubsystems.APPSEC_ACTIVE); + assertEquals(0, runs.get()); + } + + @Test + void aFailingCallbackNeitherBreaksActivationNorTheOtherCallbacks() { + AtomicInteger runs = new AtomicInteger(); + ActiveSubsystems.whenAppSecActivated( + () -> { + throw new IllegalStateException("boom"); + }); + ActiveSubsystems.whenAppSecActivated(runs::incrementAndGet); + + ActiveSubsystems.setAppSecActive(true); + + assertTrue(ActiveSubsystems.APPSEC_ACTIVE); + assertEquals(1, runs.get()); + } +} From b1d3bbb5630f3e25fa88ae06ff7a188173248f50 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 10:28:51 +0200 Subject: [PATCH 10/21] revert: remove AppSec runtime-activation trigger for OTel context exposure Confirmed with product that no remote-config/one-click activation path is needed for this feature: CADR relies only on SSI and fleet (startup config), never on AppSec turning on later via remote config. Removes ActiveSubsystems.setAppSecActive/whenAppSecActivated and the associated callback list, Config.isOtelContextExposurePendingAppSecActivation, and Agent.createAppSecActivatedDdprofContextIntegration, plus their tests. The fix for the separate double process-context registration issue (Agent.ddprofContextIntegrationFactory's registerProcessContext param) is unrelated and stays untouched. --- .../java/datadog/trace/bootstrap/Agent.java | 29 ------ ...ferredProfilingContextIntegrationTest.java | 34 +------ .../java/com/datadog/appsec/AppSecSystem.java | 4 +- .../main/java/datadog/trace/api/Config.java | 24 ----- .../trace/bootstrap/ActiveSubsystems.java | 67 -------------- .../api/ConfigOtelContextExposureTest.java | 40 -------- .../trace/bootstrap/ActiveSubsystemsTest.java | 91 ------------------- 7 files changed, 2 insertions(+), 287 deletions(-) delete mode 100644 internal-api/src/test/java/datadog/trace/bootstrap/ActiveSubsystemsTest.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index 7ee49241782..387ab57f480 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -1508,8 +1508,6 @@ private static ProfilingContextIntegration createProfilingContextIntegration() { if (integration != null) { return integration; } - } else if (config.isOtelContextExposurePendingAppSecActivation()) { - return createAppSecActivatedDdprofContextIntegration(AGENT_CLASSLOADER); } } if (config.isProfilingEnabled() && config.isProfilingTimelineEventsEnabled()) { @@ -1560,33 +1558,6 @@ static ProfilingContextIntegration createDdprofContextIntegration( } } - /** - * Creates a ddprof context integration that stays a no-op until AppSec is activated at runtime - * through remote config, and only then builds the real one. - * - *

This covers {@code DD_APPSEC_ENABLED=inactive}, the "one-click" activation flow, where the - * boot-time activation level stays {@link datadog.trace.api.ProductActivation#ENABLED_INACTIVE} - * forever and only a runtime flag flips. Profiling is off in this case (otherwise the caller took - * the branch above), so the profiler never registers the process context either and the deferred - * construction is responsible for it. - * - *

The activation callback runs on the remote-config poller thread, so it only schedules the - * construction rather than doing it inline. Deactivation is deliberately not handled: the context - * exposure is a one-time process-wide registration, and tearing the native context down when - * AppSec is switched back off is out of scope. - * - * @param classLoader the agent class loader used to reach the profiling classes. - * @return the integration, which is never {@code null}: nothing can fail synchronously here. - */ - static ProfilingContextIntegration createAppSecActivatedDdprofContextIntegration( - final ClassLoader classLoader) { - DeferredProfilingContextIntegration deferred = - new DeferredProfilingContextIntegration( - "ddprof", ddprofContextIntegrationFactory(classLoader, true)); - ActiveSubsystems.whenAppSecActivated(deferred::scheduleInitialization); - return deferred; - } - /** * Builds the ddprof integration reflectively, optionally registering the OTel process context * alongside it. diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java index ae69dd75d4b..ec9522bcc42 100644 --- a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java @@ -27,8 +27,7 @@ * Covers the premain-timing contract of the ddprof profiling context integration: the AppSec-only * trigger must not construct it (nor register the process context) on the calling thread, while the * profiler-enabled path must keep constructing it there and must leave the process context - * registration to the profiler agent. Also covers the AppSec runtime-activation trigger, where the - * construction only happens once remote config turns AppSec on. + * registration to the profiler agent. */ class DeferredProfilingContextIntegrationTest { @@ -96,37 +95,6 @@ void synchronousConstructionLeavesTheProcessContextToTheProfilerAgent() { assertEquals(0, FakeProcessContext.registrations.get()); } - @Test - void appSecActivatedConstructionWaitsForTheRuntimeActivation() throws Exception { - boolean originalAppSecActive = ActiveSubsystems.APPSEC_ACTIVE; - ActiveSubsystems.APPSEC_ACTIVE = false; - try { - ProfilingContextIntegration integration = - Agent.createAppSecActivatedDdprofContextIntegration(fakeProfilingClassLoader()); - - // AppSec is only "inactive-enabled" so far: nothing may be constructed yet - assertNotNull(integration); - assertEquals("ddprof", integration.name()); - assertEquals(0, FakeDatadogProfilingIntegration.constructions.get()); - assertEquals(0, FakeProcessContext.registrations.get()); - assertSame(Stateful.DEFAULT, integration.newScopeState(null)); - - ActiveSubsystems.setAppSecActive(true); - - assertTrue( - FakeProcessContext.registered.await(30, TimeUnit.SECONDS), - "the activation never triggered the deferred construction"); - assertEquals(1, FakeDatadogProfilingIntegration.constructions.get()); - // nothing else registers the process context here, because the profiler never starts - assertEquals(1, FakeProcessContext.registrations.get()); - assertNotSame( - Thread.currentThread(), FakeDatadogProfilingIntegration.constructionThread.get()); - assertSame(FakeDatadogProfilingIntegration.STATE, integration.newScopeState(null)); - } finally { - ActiveSubsystems.APPSEC_ACTIVE = originalAppSecActive; - } - } - @Test void delegatesToTheRealIntegrationOnceInitialized() { DeferredProfilingContextIntegration deferred = diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java index dc1f7ee01bb..d7d780123ea 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java @@ -118,9 +118,7 @@ public static boolean isActive() { } public static void setActive(boolean status) { - // Goes through the setter rather than the field so that components outside the AppSec module, - // which cannot see this class, still observe a runtime (remote-config driven) activation. - ActiveSubsystems.setAppSecActive(status); + ActiveSubsystems.APPSEC_ACTIVE = status; // Report to the product change via telemetry log.debug("AppSec is now {}", status ? "active" : "inactive"); ProductChangeCollector.get() diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 449f08e960d..997cabbf764 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -1081,7 +1081,6 @@ public static String getHostName() { private final boolean profilingAgentless; private final boolean isDatadogProfilerEnabled; private final boolean otelContextExposureEnabled; - private final boolean otelContextExposurePendingAppSecActivation; @Deprecated private final String profilingUrl; private final Map profilingTags; private final int profilingStartDelay; @@ -2654,14 +2653,6 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) && (isProfilingEnabled() || instrumenterConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED); - // AppSec started as ENABLED_INACTIVE is the standard "one-click" flow: the activation level - // itself never changes afterwards, only the runtime flag AppSec flips when remote config - // turns it on. Context exposure therefore cannot be decided here, it can only be armed. - this.otelContextExposurePendingAppSecActivation = - isDatadogProfilerSafeAndConfigured() - && !otelContextExposureEnabled - && instrumenterConfig.getAppSecActivation() == ProductActivation.ENABLED_INACTIVE; - this.traceResourceRenamingAlwaysSimplifiedEndpoint = configProvider.getBoolean(TRACE_RESOURCE_RENAMING_ALWAYS_SIMPLIFIED_ENDPOINT, false); @@ -4290,19 +4281,6 @@ public boolean isOtelContextExposureEnabled() { return otelContextExposureEnabled; } - /** - * Whether OpenTelemetry context exposure is not enabled at boot but must be armed so it can start - * later, when AppSec is activated at runtime through remote config. - * - *

{@link #isOtelContextExposureEnabled()} can only look at {@code getAppSecActivation()}, - * which is read once from {@code InstrumenterConfig} and never changes. The "one-click" AppSec - * activation flow leaves it at {@link ProductActivation#ENABLED_INACTIVE} forever and flips a - * separate runtime flag instead, so that flow would never be observed without this. - */ - public boolean isOtelContextExposurePendingAppSecActivation() { - return otelContextExposurePendingAppSecActivation; - } - public static boolean isDatadogProfilerEnablementOverridden() { // old non-LTS versions without important backports // also, we have no windows binaries @@ -6845,8 +6823,6 @@ public String toString() { + profilingExcludeAgentThreads + ", otelContextExposureEnabled=" + otelContextExposureEnabled - + ", otelContextExposurePendingAppSecActivation=" - + otelContextExposurePendingAppSecActivation + ", crashTrackingTags=" + crashTrackingTags + ", crashTrackingAgentless=" diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/ActiveSubsystems.java b/internal-api/src/main/java/datadog/trace/bootstrap/ActiveSubsystems.java index 9d52a4500e4..eb811a678fb 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/ActiveSubsystems.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/ActiveSubsystems.java @@ -1,73 +1,6 @@ package datadog.trace.bootstrap; -import java.util.ArrayList; -import java.util.List; - /** Runtime activation state of the subsystems that can be switched on and off after start-up. */ public class ActiveSubsystems { - /** - * Whether AppSec is currently active. Unlike {@code Config.getAppSecActivation()}, which is fixed - * at boot, this flag follows the remote-config "one-click" activation flow, so it can flip at any - * time while the application runs. - * - *

Kept public and directly writable because it is read from hot paths and from tests that set - * it up by hand. Production code that owns the transition must go through {@link - * #setAppSecActive(boolean)} instead, so the activation callbacks below are honoured. - */ public static volatile boolean APPSEC_ACTIVE; - - /** - * One-shot callbacks waiting for AppSec to become active, registered through {@link - * #whenAppSecActivated(Runnable)}. Guarded by its own monitor, which also guards the read of - * {@link #APPSEC_ACTIVE} on the registration side, so a callback registered concurrently with an - * activation is neither run twice nor dropped. - */ - private static final List APPSEC_ACTIVATION_CALLBACKS = new ArrayList<>(1); - - /** - * Updates {@link #APPSEC_ACTIVE} and, on a transition into the active state, runs the pending - * activation callbacks. - * - *

Callbacks fire on the caller's thread, which for the remote-config flow is the configuration - * poller thread, so they must not block. They fire at most once for the lifetime of the process: - * AppSec can be activated and deactivated repeatedly through remote config, but the consumers - * here are one-time initializations. - */ - public static void setAppSecActive(final boolean active) { - APPSEC_ACTIVE = active; - if (!active) { - return; - } - final List callbacks; - synchronized (APPSEC_ACTIVATION_CALLBACKS) { - if (APPSEC_ACTIVATION_CALLBACKS.isEmpty()) { - return; - } - callbacks = new ArrayList<>(APPSEC_ACTIVATION_CALLBACKS); - APPSEC_ACTIVATION_CALLBACKS.clear(); - } - for (final Runnable callback : callbacks) { - try { - callback.run(); - } catch (final Throwable ignored) { - // A failing callback must never prevent AppSec from becoming active, nor stop the - // remaining callbacks. There is deliberately no logger here: this class is loaded very - // early and from hot paths, and each callback is expected to report its own failures. - } - } - } - - /** - * Runs {@code callback} once AppSec becomes active, or immediately if it already is. The callback - * is run at most once, and is never run if AppSec never becomes active. - */ - public static void whenAppSecActivated(final Runnable callback) { - synchronized (APPSEC_ACTIVATION_CALLBACKS) { - if (!APPSEC_ACTIVE) { - APPSEC_ACTIVATION_CALLBACKS.add(callback); - return; - } - } - callback.run(); - } } diff --git a/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java index 9704da837e5..198f3f6ac59 100644 --- a/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java +++ b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java @@ -62,46 +62,6 @@ void disabledWhenAppSecIsOnlyEnabledInactive() { assertFalse(Config.get().isOtelContextExposureEnabled()); } - @Test - @WithConfig(key = APPSEC_ENABLED, value = "inactive") - @WithConfig(key = PROFILING_ENABLED, value = "false") - @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") - void pendingWhenAppSecIsOnlyEnabledInactive() { - assumeDatadogProfilerNotVetoed(); - - // "inactive" is the one-click flow: nothing is exposed yet, but it has to be armed, because - // the activation level itself never changes once remote config turns AppSec on. - assertTrue(Config.get().isOtelContextExposurePendingAppSecActivation()); - } - - @Test - @WithConfig(key = APPSEC_ENABLED, value = "true") - @WithConfig(key = PROFILING_ENABLED, value = "false") - @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") - void notPendingWhenAlreadyEnabledAtBoot() { - assumeDatadogProfilerNotVetoed(); - - Config config = Config.get(); - assertTrue(config.isOtelContextExposureEnabled()); - assertFalse(config.isOtelContextExposurePendingAppSecActivation()); - } - - @Test - @WithConfig(key = APPSEC_ENABLED, value = "false") - @WithConfig(key = PROFILING_ENABLED, value = "false") - @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") - void notPendingWhenAppSecIsFullyDisabled() { - assertFalse(Config.get().isOtelContextExposurePendingAppSecActivation()); - } - - @Test - @WithConfig(key = APPSEC_ENABLED, value = "inactive") - @WithConfig(key = PROFILING_ENABLED, value = "false") - @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "false") - void notPendingWhenDatadogProfilerIsExplicitlyDisabled() { - assertFalse(Config.get().isOtelContextExposurePendingAppSecActivation()); - } - @Test @WithConfig(key = APPSEC_ENABLED, value = "true") @WithConfig(key = PROFILING_ENABLED, value = "false") diff --git a/internal-api/src/test/java/datadog/trace/bootstrap/ActiveSubsystemsTest.java b/internal-api/src/test/java/datadog/trace/bootstrap/ActiveSubsystemsTest.java deleted file mode 100644 index 61b94c12075..00000000000 --- a/internal-api/src/test/java/datadog/trace/bootstrap/ActiveSubsystemsTest.java +++ /dev/null @@ -1,91 +0,0 @@ -package datadog.trace.bootstrap; - -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 java.util.concurrent.atomic.AtomicInteger; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -/** - * Covers the AppSec runtime-activation callbacks, the only signal other subsystems get for the - * remote-config "one-click" activation flow, where the boot-time activation level never changes. - */ -class ActiveSubsystemsTest { - - private boolean originalAppSecActive; - - @BeforeEach - void saveState() { - originalAppSecActive = ActiveSubsystems.APPSEC_ACTIVE; - ActiveSubsystems.APPSEC_ACTIVE = false; - } - - @AfterEach - void restoreState() { - ActiveSubsystems.APPSEC_ACTIVE = originalAppSecActive; - } - - @Test - void callbackRunsWhenAppSecBecomesActive() { - AtomicInteger runs = new AtomicInteger(); - - ActiveSubsystems.whenAppSecActivated(runs::incrementAndGet); - assertEquals(0, runs.get()); - - ActiveSubsystems.setAppSecActive(true); - - assertTrue(ActiveSubsystems.APPSEC_ACTIVE); - assertEquals(1, runs.get()); - } - - @Test - void callbackRunsAtMostOnceAcrossRepeatedToggles() { - AtomicInteger runs = new AtomicInteger(); - ActiveSubsystems.whenAppSecActivated(runs::incrementAndGet); - - ActiveSubsystems.setAppSecActive(true); - ActiveSubsystems.setAppSecActive(false); - ActiveSubsystems.setAppSecActive(true); - - assertEquals(1, runs.get()); - } - - @Test - void callbackRunsImmediatelyWhenAppSecIsAlreadyActive() { - AtomicInteger runs = new AtomicInteger(); - ActiveSubsystems.setAppSecActive(true); - - ActiveSubsystems.whenAppSecActivated(runs::incrementAndGet); - - assertEquals(1, runs.get()); - } - - @Test - void callbackNeverRunsWhileAppSecStaysInactive() { - AtomicInteger runs = new AtomicInteger(); - - ActiveSubsystems.whenAppSecActivated(runs::incrementAndGet); - ActiveSubsystems.setAppSecActive(false); - - assertFalse(ActiveSubsystems.APPSEC_ACTIVE); - assertEquals(0, runs.get()); - } - - @Test - void aFailingCallbackNeitherBreaksActivationNorTheOtherCallbacks() { - AtomicInteger runs = new AtomicInteger(); - ActiveSubsystems.whenAppSecActivated( - () -> { - throw new IllegalStateException("boom"); - }); - ActiveSubsystems.whenAppSecActivated(runs::incrementAndGet); - - ActiveSubsystems.setAppSecActive(true); - - assertTrue(ActiveSubsystems.APPSEC_ACTIVE); - assertEquals(1, runs.get()); - } -} From ba4757f3751584129b199caaa2cc2d0420a35960 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 10:58:34 +0200 Subject: [PATCH 11/21] fix: unwrap LocalRootSpanTags in DDTracerAPITest reflection CoreTracer.localRootSpanTags moved from a plain Map to a private LocalRootSpanTags{tags, needsIntercept} wrapper in 6b1c10df6f, so the test's cast to java.util.Map now throws ClassCastException. Reflect into the wrapper's tags field before casting. --- .../src/test/java/datadog/opentracing/DDTracerAPITest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dd-trace-ot/src/test/java/datadog/opentracing/DDTracerAPITest.java b/dd-trace-ot/src/test/java/datadog/opentracing/DDTracerAPITest.java index a95cb2b360d..a15dd3ae44f 100644 --- a/dd-trace-ot/src/test/java/datadog/opentracing/DDTracerAPITest.java +++ b/dd-trace-ot/src/test/java/datadog/opentracing/DDTracerAPITest.java @@ -32,7 +32,8 @@ void verifySamplerWriterConstructor() throws Exception { assertSame(sampler, getField(tracer, "initialSampler")); assertSame(writer, getField(tracer, "writer")); - Object localRootSpanTags = getField(tracer, "localRootSpanTags"); + Object localRootSpanTagsHolder = getField(tracer, "localRootSpanTags"); + Object localRootSpanTags = getField(localRootSpanTagsHolder, "tags"); assertNotNull(localRootSpanTags.toString()); // Verify runtime-id and language tags are populated assertTrue( From 92339b3acd5b460899cfe38bf1ce3a9616ba5863 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 11:06:23 +0200 Subject: [PATCH 12/21] docs: remove stale remote-config activation paragraph from DeferredProfilingContextIntegration javadoc The runtime-activation path this described was reverted in b1d3bbb563; createDdprofContextIntegration() is now the only caller of scheduleInitialization(). --- .../trace/bootstrap/DeferredProfilingContextIntegration.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java index 81543816c22..716caeb5477 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java @@ -35,11 +35,6 @@ * offers no hook for "the application has entered {@code main}", so an application whose start-up * is slower than the delay can still be racing with it. * - *

The same wrapper also covers the other case where the integration cannot exist yet at {@code - * premain} time: AppSec started as {@code inactive} and only activated later through remote config. - * There the construction is not scheduled up front but when the activation arrives, which may be - * minutes into the run. - * *

Scope events happening before the swap are silently dropped. That is acceptable for context * exposure (eBPF/CWS reading the current span off a thread), but not for profiling * accuracy, so users with the Datadog profiler actually enabled keep the synchronous construction From 33352ec55ca6fa90091c4f6c37bbe2f193c3ae7e Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 11:34:46 +0200 Subject: [PATCH 13/21] fix: forward virtual-thread context binding in deferred wrapper; tighten smoke test assertion DeferredProfilingContextIntegration forwarded most methods to its delegate but fell back to the interface defaults for isThreadContextBindingRequired() and setContext(Context), so virtual-thread context rebinding silently never happened on the AppSec-only path even after the deferred ddprof integration was swapped in. OtelContextExposureSmokeTest only asserted the 'Registering process context...' log line, which is emitted unconditionally before the actual registration outcome is known. Also assert the failure log line is absent. --- .../DeferredProfilingContextIntegration.java | 11 +++++ ...ferredProfilingContextIntegrationTest.java | 41 +++++++++++++++++++ .../OtelContextExposureSmokeTest.groovy | 6 +++ 3 files changed, 58 insertions(+) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java index 716caeb5477..e4b313ce285 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java @@ -2,6 +2,7 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; +import datadog.context.Context; import datadog.trace.api.EndpointTracker; import datadog.trace.api.Stateful; import datadog.trace.api.profiling.ProfilingContextAttribute; @@ -174,6 +175,16 @@ public void onDetach() { delegate.onDetach(); } + @Override + public boolean isThreadContextBindingRequired() { + return delegate.isThreadContextBindingRequired(); + } + + @Override + public void setContext(final Context context) { + delegate.setContext(context); + } + @Override public Stateful newScopeState(final ProfilerContext profilerContext) { return delegate.newScopeState(profilerContext); diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java index ec9522bcc42..1b055d0aafd 100644 --- a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java @@ -1,11 +1,15 @@ package datadog.trace.bootstrap; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import datadog.context.Context; +import datadog.context.ContextKey; import datadog.trace.api.EndpointTracker; import datadog.trace.api.Stateful; import datadog.trace.api.profiling.ProfilingContextAttribute; @@ -41,6 +45,7 @@ void reset() { FakeDatadogProfilingIntegration.onAttachCalls.set(0); FakeDatadogProfilingIntegration.onDetachCalls.set(0); FakeDatadogProfilingIntegration.onRootSpanFinishedCalls.set(0); + FakeDatadogProfilingIntegration.lastBoundContext.set(null); } @Test @@ -119,6 +124,31 @@ void delegatesToTheRealIntegrationOnceInitialized() { assertEquals(1, FakeDatadogProfilingIntegration.onRootSpanFinishedCalls.get()); } + /** + * Virtual thread mount/unmount goes through {@link + * ProfilingContextIntegration#isThreadContextBindingRequired()} and {@link + * ProfilingContextIntegration#setContext(Context)}. Without explicit forwarding the wrapper would + * keep answering with the interface defaults ({@code false} / no-op) even after the real + * integration is swapped in, so virtual-thread rebinding would silently never happen. + */ + @Test + void forwardsVirtualThreadContextBindingOnceInitialized() { + DeferredProfilingContextIntegration deferred = + new DeferredProfilingContextIntegration("ddprof", FakeDatadogProfilingIntegration::new); + + // before the swap the wrapper uses the interface defaults + assertFalse(deferred.isThreadContextBindingRequired()); + deferred.setContext(Context.root()); + assertNull(FakeDatadogProfilingIntegration.lastBoundContext.get()); + + deferred.initialize(); + + assertTrue(deferred.isThreadContextBindingRequired()); + Context context = Context.root().with(ContextKey.named("test"), "value"); + deferred.setContext(context); + assertSame(context, FakeDatadogProfilingIntegration.lastBoundContext.get()); + } + @Test void staysNoOpWhenTheDeferredConstructionFails() { DeferredProfilingContextIntegration deferred = @@ -212,6 +242,7 @@ public void activate(final Object context) {} static final AtomicInteger onAttachCalls = new AtomicInteger(); static final AtomicInteger onDetachCalls = new AtomicInteger(); static final AtomicInteger onRootSpanFinishedCalls = new AtomicInteger(); + static final AtomicReference lastBoundContext = new AtomicReference<>(); public FakeDatadogProfilingIntegration() { try { @@ -270,5 +301,15 @@ public int encodeOperationName(final CharSequence constant) { public int encodeResourceName(final CharSequence constant) { return 43; } + + @Override + public boolean isThreadContextBindingRequired() { + return true; + } + + @Override + public void setContext(final Context context) { + lastBoundContext.set(context); + } } } diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy index 6fb19d9efc3..54a01e132e7 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy @@ -12,6 +12,7 @@ import spock.util.concurrent.PollingConditions class OtelContextExposureSmokeTest extends AbstractAppSecServerSmokeTest { private static final String PROCESS_CONTEXT_LOG_LINE = 'Registering process context for OTel profiler' + private static final String PROCESS_CONTEXT_FAILURE_LOG_LINE = 'Failed to register process context for OTel profiler' @Override def logLevel() { @@ -42,6 +43,11 @@ class OtelContextExposureSmokeTest extends AbstractAppSecServerSmokeTest { conditions.eventually { assert new File(logFilePath).text.contains(PROCESS_CONTEXT_LOG_LINE) } + // The "Registering..." line is logged before the native library is loaded and the OTel + // context is initialized, so on its own it only proves the attempt. Give the registration + // time to complete (or fail) and then assert it did not fail. + sleep(5_000) + assert !new File(logFilePath).text.contains(PROCESS_CONTEXT_FAILURE_LOG_LINE) } else { // AppSec is only "inactive-enabled" here and no remote config ever activates it, so the // integration stays armed and never registers anything. From a75de833ee72ebb0ab8fd5b93002a462a4ae3fbb Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Wed, 23 Sep 2026 12:18:57 +0200 Subject: [PATCH 14/21] fix: exclude AWS Lambda from ddprof context-exposure path; cover factory-level registration failure in smoke test --- .../java/datadog/trace/bootstrap/Agent.java | 6 ++- ...AgentLambdaProfilingContextForkedTest.java | 48 +++++++++++++++++++ .../OtelContextExposureSmokeTest.groovy | 6 ++- 3 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/AgentLambdaProfilingContextForkedTest.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index 387ab57f480..d9f184d7c5b 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -1491,9 +1491,11 @@ public void withTracer(TracerAPI tracer) { * {@see com.datadog.profiling.ddprof.DatadogProfilingIntegration} must not be modified to depend * on JFR. */ - private static ProfilingContextIntegration createProfilingContextIntegration() { + static ProfilingContextIntegration createProfilingContextIntegration() { Config config = Config.get(); - if (!OperatingSystem.isWindows()) { + // AWS Lambda is excluded for the same reason startProfilingAgent() excludes it: the ddprof + // native library is not supported there, and loading it would only add cold-start overhead. + if (!OperatingSystem.isWindows() && !isAwsLambdaRuntime()) { // isDatadogProfilerEnabled() is ORed in explicitly so a user with real profiling enabled // keeps ddprof regardless of the AppSec activation level that otherwise drives // isOtelContextExposureEnabled() - additive, not a replacement gate. diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/AgentLambdaProfilingContextForkedTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/AgentLambdaProfilingContextForkedTest.java new file mode 100644 index 00000000000..c49308a63cc --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/AgentLambdaProfilingContextForkedTest.java @@ -0,0 +1,48 @@ +package datadog.trace.bootstrap; + +import static datadog.trace.api.config.AppSecConfig.APPSEC_ENABLED; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_DATADOG_PROFILER_ENABLED; +import static datadog.trace.api.config.ProfilingConfig.PROFILING_ENABLED; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import datadog.trace.api.Config; +import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration; +import datadog.trace.test.junit.utils.config.WithConfig; +import datadog.trace.test.junit.utils.config.WithConfigExtension; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * Profiling is explicitly unsupported in AWS Lambda runtimes ({@code Agent#startProfilingAgent} + * bails out there). The AppSec-driven OTel context exposure path must honour the same exclusion, so + * that enabling AppSec inside a Lambda function never loads the ddprof native library. + * + *

Forked because {@link WithConfigExtension} swaps the process-wide environment variable + * provider. + */ +@ExtendWith(WithConfigExtension.class) +class AgentLambdaProfilingContextForkedTest { + + @Test + @WithConfig(key = APPSEC_ENABLED, value = "true") + @WithConfig(key = PROFILING_ENABLED, value = "false") + @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") + @WithConfig( + key = "AWS_LAMBDA_FUNCTION_NAME", + value = "my-function", + env = true, + addPrefix = false) + void doesNotCreateTheDdprofIntegrationInAwsLambda() { + // The exclusion is only observable when the configuration would otherwise have triggered the + // ddprof context integration; the Datadog profiler is vetoed on some platforms and JVMs. + assumeTrue( + Config.get().isOtelContextExposureEnabled(), + "OTel context exposure is unavailable on this platform/JVM version"); + + // AGENT_CLASSLOADER is null in this unit test, so reaching the ddprof branch at all would fail + // loudly rather than silently return the no-op integration. + assertSame( + ProfilingContextIntegration.NoOp.INSTANCE, Agent.createProfilingContextIntegration()); + } +} diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy index 54a01e132e7..f1c684c4c59 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy @@ -13,6 +13,8 @@ class OtelContextExposureSmokeTest extends AbstractAppSecServerSmokeTest { private static final String PROCESS_CONTEXT_LOG_LINE = 'Registering process context for OTel profiler' private static final String PROCESS_CONTEXT_FAILURE_LOG_LINE = 'Failed to register process context for OTel profiler' + /** Logged by Agent#ddprofContextIntegrationFactory when the reflective registration call itself fails. */ + private static final String PROCESS_CONTEXT_UNAVAILABLE_LOG_LINE = 'Process context registration not available' @Override def logLevel() { @@ -47,7 +49,9 @@ class OtelContextExposureSmokeTest extends AbstractAppSecServerSmokeTest { // context is initialized, so on its own it only proves the attempt. Give the registration // time to complete (or fail) and then assert it did not fail. sleep(5_000) - assert !new File(logFilePath).text.contains(PROCESS_CONTEXT_FAILURE_LOG_LINE) + String logContent = new File(logFilePath).text + assert !logContent.contains(PROCESS_CONTEXT_FAILURE_LOG_LINE) + assert !logContent.contains(PROCESS_CONTEXT_UNAVAILABLE_LOG_LINE) } else { // AppSec is only "inactive-enabled" here and no remote config ever activates it, so the // integration stays armed and never registers anything. From 6d290fcb163d965818d4008eea5fe2fc82b546ca Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 24 Sep 2026 10:21:34 +0200 Subject: [PATCH 15/21] docs: trim verbose Javadocs and inline comments per reviewer feedback Reduce multi-paragraph Javadocs to 1-2 lines across the OTel context exposure changes, keeping only non-obvious rationale. --- .../java/datadog/trace/bootstrap/Agent.java | 36 +++------ .../DeferredProfilingContextIntegration.java | 76 +++++-------------- ...ferredProfilingContextIntegrationTest.java | 19 ++--- .../OtelContextExposureSmokeTest.groovy | 18 +---- .../java/datadog/trace/core/CoreTracer.java | 22 ++---- .../main/java/datadog/trace/api/Config.java | 28 ++----- .../api/ProfilingContextIntegration.java | 11 +-- 7 files changed, 53 insertions(+), 157 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index d9f184d7c5b..d3ef52bcfb3 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -1493,18 +1493,12 @@ public void withTracer(TracerAPI tracer) { */ static ProfilingContextIntegration createProfilingContextIntegration() { Config config = Config.get(); - // AWS Lambda is excluded for the same reason startProfilingAgent() excludes it: the ddprof - // native library is not supported there, and loading it would only add cold-start overhead. + // AWS Lambda has no ddprof native library support, same as startProfilingAgent(). if (!OperatingSystem.isWindows() && !isAwsLambdaRuntime()) { - // isDatadogProfilerEnabled() is ORed in explicitly so a user with real profiling enabled - // keeps ddprof regardless of the AppSec activation level that otherwise drives - // isOtelContextExposureEnabled() - additive, not a replacement gate. + // ORed explicitly so real profiling keeps ddprof regardless of AppSec activation level. if (config.isDatadogProfilerEnabled() || config.isOtelContextExposureEnabled()) { - // When the ddprof integration is triggered by context exposure alone (profiling disabled), - // its construction is deferred off the premain thread: it loads the ddprof native library - // and touches java.nio.file, which must not happen on the primordial premain thread. Users - // with the profiler actually enabled keep the synchronous path, since profiling accuracy - // requires seeing every scope from the very first one. + // Deferred unless profiling is enabled: construction loads the ddprof native library and + // touches java.nio.file, which must not happen on the primordial premain thread. ProfilingContextIntegration integration = createDdprofContextIntegration(AGENT_CLASSLOADER, !config.isDatadogProfilerEnabled()); if (integration != null) { @@ -1529,21 +1523,13 @@ static ProfilingContextIntegration createProfilingContextIntegration() { /** * Creates the ddprof-based profiling context integration, either synchronously or deferred off - * the calling thread. - * - * @param classLoader the agent class loader used to reach the profiling classes. - * @param deferInitialization when true, the integration (and the process context registration - * that follows it) is constructed on an {@link AgentTaskScheduler} thread instead of the - * caller's, which during premain is the JVM's primordial thread. - * @return the integration, or {@code null} if a synchronous construction failed, in which case - * the caller falls back to the other integrations. + * the calling thread onto an {@link AgentTaskScheduler} thread. Returns {@code null} if a + * synchronous construction fails, so the caller can fall back to another integration. */ static ProfilingContextIntegration createDdprofContextIntegration( final ClassLoader classLoader, final boolean deferInitialization) { - // deferInitialization is exactly "the profiler itself is not running", which is also exactly - // when nobody else registers the process context: ProfilingAgent.run() already does it when - // the profiler starts. Registering it here as well in the profiler-enabled case would log and - // call into the native library twice for every user that has profiling on today. + // deferInitialization means the profiler itself isn't running, so nothing else registers the + // process context here — ProfilingAgent.run() already does it when the profiler starts. Callable factory = ddprofContextIntegrationFactory(classLoader, deferInitialization); if (deferInitialization) { @@ -1562,11 +1548,7 @@ static ProfilingContextIntegration createDdprofContextIntegration( /** * Builds the ddprof integration reflectively, optionally registering the OTel process context - * alongside it. - * - * @param classLoader the agent class loader used to reach the profiling classes. - * @param registerProcessContext whether this factory also has to register the process context, - * i.e. whether the profiler agent, which registers it on its own, is not going to start. + * alongside it (when the profiler agent isn't going to register it itself). */ private static Callable ddprofContextIntegrationFactory( final ClassLoader classLoader, final boolean registerProcessContext) { diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java index e4b313ce285..a4f8a012d67 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java @@ -19,47 +19,21 @@ import org.slf4j.LoggerFactory; /** - * A {@link ProfilingContextIntegration} that can be handed out synchronously during {@code premain} - * while the real integration is constructed later, off the premain thread. - * - *

Constructing the ddprof-based integration loads the ddprof native library and touches {@code - * java.nio.file} (through {@code TempLocationManager}), which must not happen on the JVM's - * primordial premain thread: it can lock in the default filesystem provider before the application - * has a chance to configure one in {@code main}. This wrapper keeps the premain thread free of that - * work by delegating to {@link ProfilingContextIntegration.NoOp} until the deferred construction - * completes, then swapping in the real integration. - * - *

Moving the work to another thread is not enough on its own, because that thread would still - * run concurrently with the rest of {@code premain}, i.e. still before {@code main} gets to set - * {@code java.nio.file.spi.DefaultFileSystemProvider}. The construction is therefore also delayed - * by {@link #INITIALIZATION_DELAY_MILLIS}. That delay is a mitigation, not a guarantee: the JVM - * offers no hook for "the application has entered {@code main}", so an application whose start-up - * is slower than the delay can still be racing with it. - * - *

Scope events happening before the swap are silently dropped. That is acceptable for context - * exposure (eBPF/CWS reading the current span off a thread), but not for profiling - * accuracy, so users with the Datadog profiler actually enabled keep the synchronous construction - * path. - * - *

Because the deferred construction can also fail outright, this wrapper never claims to be the - * real engine until it is: {@link #whenAvailable(Runnable)} only fires after a successful swap, so - * consumers (such as the tracer stamping the {@code _dd.profiling.ctx} tag) do not advertise an - * engine that never materialized. + * A {@link ProfilingContextIntegration} handed out synchronously during {@code premain} while the + * real ddprof-based integration is constructed later, off the premain thread, to avoid loading the + * ddprof native library (and touching {@code java.nio.file}) before {@code main} gets a chance to + * set its own {@code java.nio.file.spi.DefaultFileSystemProvider}. Delegates to {@link + * ProfilingContextIntegration.NoOp} until the swap happens; stays a no-op forever if construction + * fails. */ final class DeferredProfilingContextIntegration implements ProfilingContextIntegration { private static final Logger log = LoggerFactory.getLogger(DeferredProfilingContextIntegration.class); /** - * How long to wait before running the deferred construction, so that the rest of {@code premain} - * has returned and the application has had a chance to run the top of {@code main} (where an - * application that cares about it installs its own {@code - * java.nio.file.spi.DefaultFileSystemProvider}). - * - *

Same magnitude as the longest delay {@code Agent} already applies for the analogous "let the - * application get there first" problem with OkHttp and a custom log manager, and hardcoded for - * the same reason: this is context exposure for eBPF/CWS consumers, where losing the - * first second of thread context is not observable, so there is nothing for a user to tune. + * Delay before the deferred construction runs, giving {@code main} a chance to install its own + * {@code java.nio.file.spi.DefaultFileSystemProvider} first; not user-tunable since losing the + * first second of context exposure is not observable. */ private static final long INITIALIZATION_DELAY_MILLIS = 1_000; @@ -67,16 +41,14 @@ final class DeferredProfilingContextIntegration implements ProfilingContextInteg private final Callable factory; /** - * Swapped from {@link ProfilingContextIntegration.NoOp} to the real integration once the deferred - * construction succeeds. Volatile because application threads may already be running scopes when - * the swap happens. + * Swapped to the real integration once construction succeeds; volatile since scopes may already + * be running when the swap happens. */ private volatile ProfilingContextIntegration delegate = ProfilingContextIntegration.NoOp.INSTANCE; /** - * Callbacks registered through {@link #whenAvailable(Runnable)} before the swap happened, to be - * run once it does. Guarded by {@code this}, together with the {@link #delegate} write, so that a - * callback registered concurrently with the swap is neither run twice nor dropped. + * Callbacks queued via {@link #whenAvailable(Runnable)} before the swap; guarded by {@code this} + * together with the {@link #delegate} write so none is run twice or dropped. */ private final List pendingAvailabilityCallbacks = new ArrayList<>(1); @@ -92,12 +64,8 @@ final class DeferredProfilingContextIntegration implements ProfilingContextInteg } /** - * Schedules the deferred construction so that it runs off the calling (premain) thread, after - * {@link #INITIALIZATION_DELAY_MILLIS}. - * - *

The delay is the same order of magnitude as the one {@code Agent} already uses to let the - * application reach a given point before starting OkHttp when a custom log manager is in play. It - * is a heuristic, not a handshake: nothing here observes {@code main} actually starting. + * Schedules the deferred construction to run off this (premain) thread, after {@link + * #INITIALIZATION_DELAY_MILLIS}. */ void scheduleInitialization() { AgentTaskScheduler.get().schedule(this::initialize, INITIALIZATION_DELAY_MILLIS, MILLISECONDS); @@ -127,17 +95,14 @@ void initialize() { } } } catch (final Throwable t) { - // Reaching this point means context exposure was requested and is silently not happening, - // and there is no other signal for it. The throwable is rendered with toString() because - // the common failures here (UnsatisfiedLinkError and friends) carry no message. + // toString() because failures here (UnsatisfiedLinkError etc.) often carry no message. log.info("Deferred {} profiling context labeling not available. {}", name, t.toString()); } } /** - * Runs {@code callback} once the real integration has been swapped in, or immediately if that - * already happened. If the deferred construction fails, the callback is never run: consumers must - * treat "not yet available" and "never available" the same way. + * Runs {@code callback} once the real integration is swapped in, or immediately if it already is; + * never runs it if the deferred construction failed. */ @Override public void whenAvailable(final Runnable callback) { @@ -151,9 +116,8 @@ public void whenAvailable(final Runnable callback) { } /** - * The name of the deferred integration, not of the current delegate: it is read once when the - * tracer is built, which may happen before the deferred construction completes, and it must - * describe the integration that is being installed. + * The name of the deferred integration, not of the current delegate — read once at tracer build + * time, possibly before the deferred construction completes. */ @Override public String name() { diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java index 1b055d0aafd..2616af6fa72 100644 --- a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java @@ -28,10 +28,8 @@ import org.junit.jupiter.api.Test; /** - * Covers the premain-timing contract of the ddprof profiling context integration: the AppSec-only - * trigger must not construct it (nor register the process context) on the calling thread, while the - * profiler-enabled path must keep constructing it there and must leave the process context - * registration to the profiler agent. + * Covers the premain-timing contract of the ddprof profiling context integration: deferred vs. + * synchronous construction, and process context registration ownership. */ class DeferredProfilingContextIntegrationTest { @@ -88,10 +86,8 @@ void synchronousConstructionKeepsRunningOnTheCallingThread() { } /** - * The synchronous path is only taken when the Datadog profiler is actually enabled, and in that - * case {@code ProfilingAgent.run()} registers the process context on its own, as it always has. - * Registering it here as well would log "Registering process context for OTel profiler" twice and - * call into the native library twice for every user that already has profiling on. + * {@code ProfilingAgent.run()} registers the process context on its own when profiling is + * enabled; registering it here too would double-register. */ @Test void synchronousConstructionLeavesTheProcessContextToTheProfilerAgent() { @@ -125,11 +121,8 @@ void delegatesToTheRealIntegrationOnceInitialized() { } /** - * Virtual thread mount/unmount goes through {@link - * ProfilingContextIntegration#isThreadContextBindingRequired()} and {@link - * ProfilingContextIntegration#setContext(Context)}. Without explicit forwarding the wrapper would - * keep answering with the interface defaults ({@code false} / no-op) even after the real - * integration is swapped in, so virtual-thread rebinding would silently never happen. + * Without explicit forwarding, virtual-thread context rebinding would silently never happen after + * the real integration is swapped in. */ @Test void forwardsVirtualThreadContextBindingOnceInitialized() { diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy index f1c684c4c59..4eaaa8a7c80 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy @@ -2,13 +2,8 @@ package datadog.smoketest.appsec import spock.util.concurrent.PollingConditions -/** - * Verifies that the OTel thread/process context integration ({@code - * datadog.trace.bootstrap.Agent#createProfilingContextIntegration}) is driven purely by AppSec - * activation, independently of profiling: {@code defaultAppSecProperties} always sets {@code - * -Ddd.profiling.enabled=false}, and this module runs twice in CI (the {@code test} and {@code - * testRuntimeActivation} Gradle tasks), once with AppSec fully enabled and once with it inactive. - */ +/** Verifies OTel thread/process context integration is driven by AppSec activation, independently + * of profiling ({@code defaultAppSecProperties} always sets {@code -Ddd.profiling.enabled=false}). */ class OtelContextExposureSmokeTest extends AbstractAppSecServerSmokeTest { private static final String PROCESS_CONTEXT_LOG_LINE = 'Registering process context for OTel profiler' @@ -45,18 +40,13 @@ class OtelContextExposureSmokeTest extends AbstractAppSecServerSmokeTest { conditions.eventually { assert new File(logFilePath).text.contains(PROCESS_CONTEXT_LOG_LINE) } - // The "Registering..." line is logged before the native library is loaded and the OTel - // context is initialized, so on its own it only proves the attempt. Give the registration - // time to complete (or fail) and then assert it did not fail. + // "Registering..." only proves the attempt; give it time to complete or fail. sleep(5_000) String logContent = new File(logFilePath).text assert !logContent.contains(PROCESS_CONTEXT_FAILURE_LOG_LINE) assert !logContent.contains(PROCESS_CONTEXT_UNAVAILABLE_LOG_LINE) } else { - // AppSec is only "inactive-enabled" here and no remote config ever activates it, so the - // integration stays armed and never registers anything. - // Give the agent the same startup time as the positive case before asserting absence, - // so a slow-starting agent can't produce a false negative. + // Same startup time as the positive case, so a slow-starting agent isn't a false negative. conditions.eventually { assert new File(logFilePath).length() > 0 } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 78263f76b81..cdd72eacbf9 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -200,14 +200,8 @@ public static CoreTracerBuilder builder() { private final DynamicConfig dynamicConfig; /** - * A set of tags that are added only to the application's root span, paired with whether that set - * needs interception. - * - *

Written once in the constructor and, for a profiling context integration whose construction - * is deferred, once more when that construction succeeds (see {@link - * #stampProfilingContextEngine()}). The pair is replaced together as a single immutable holder, - * read through one volatile reference, so a concurrent root span creation never observes the new - * tag map alongside the stale intercept flag (or vice versa). + * Tags added only to the application's root span, paired with whether they need interception; + * replaced as one immutable holder so a concurrent root span creation never sees a torn read. */ private static final class LocalRootSpanTags { final TagMap tags; @@ -945,9 +939,7 @@ private CoreTracer( new LocalRootSpanTags( frozenLocalRootSpanTags, this.tagInterceptor.needsIntercept(frozenLocalRootSpanTags)); if (profilingContextIntegration != ProfilingContextIntegration.NoOp.INSTANCE) { - // The engine tag is stamped only once the integration can really label context. Integrations - // that are ready when they are handed out run this inline, right here; an integration whose - // construction is deferred runs it later, and not at all if that construction fails. + // Deferred integrations run this later (or never, if construction fails). profilingContextIntegration.whenAvailable(this::stampProfilingContextEngine); } if (serviceDiscoveryFactory != null) { @@ -968,12 +960,8 @@ private CoreTracer( } /** - * Adds the profiling context engine tag to the local root span tags. - * - *

Runs at most once per tracer, either inline from the constructor (integration already - * available) or on the thread that completes a deferred integration's construction. The tag is - * kept in the pre-frozen {@link #localRootSpanTags} rather than evaluated per span, so root span - * creation pays nothing beyond the volatile read it already does. + * Adds the profiling context engine tag to {@link #localRootSpanTags}; runs at most once per + * tracer. */ private void stampProfilingContextEngine() { TagMap tags = TagMap.fromMap(this.localRootSpanTags.tags); diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 7b73ec34170..095f2d17f4b 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -2646,11 +2646,7 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) ? traceResourceRenamingExplicit : instrumenterConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED; - // OpenTelemetry thread/process context exposure configuration - // No dedicated flag: enabled whenever the Datadog profiler is safe and configured and either - // profiling is enabled or AppSec is fully enabled. A user who wants this off already has a - // kill switch through the underlying flags - DD_PROFILING_ENABLED and DD_APPSEC_ENABLED - - // the same way isProfilingEnabled() itself has no dedicated override beyond its own flag. + // No dedicated flag: kill switch is DD_PROFILING_ENABLED / DD_APPSEC_ENABLED. this.otelContextExposureEnabled = isDatadogProfilerSafeAndConfigured() && (isProfilingEnabled() @@ -4253,34 +4249,24 @@ public boolean isProfilingRecordExceptionMessage() { } /** - * Despite the name, this does NOT mean "the Datadog profiler engine is currently recording" - it - * means "profiling is enabled AND the ddprof engine is allowed to run" ({@link - * #isProfilingEnabled()} AND {@link #isDatadogProfilerSafeAndConfigured()}). The underlying - * {@code isDatadogProfilerEnabled} field is itself independent of profiling: see {@link - * #isDatadogProfilerSafeAndConfigured()}. + * Means "profiling is enabled AND the ddprof engine is allowed to run", not "currently + * recording". */ public boolean isDatadogProfilerEnabled() { return isProfilingEnabled() && isDatadogProfilerEnabled; } /** - * The raw Datadog-profiler env-safety and explicit-flag predicate, without the {@link - * #isProfilingEnabled()} AND-prefix applied by {@link #isDatadogProfilerEnabled()}. Exposed as - * its own getter so other call sites (for example {@link #isOtelContextExposureEnabled()}) can - * reuse the same native-image/J9/JDK8 exclusions without re-deriving them or accidentally - * depending on {@code isProfilingEnabled()}. + * The raw ddprof env-safety/explicit-flag predicate, without the {@link #isProfilingEnabled()} + * AND-prefix — reused by {@link #isOtelContextExposureEnabled()}. */ public boolean isDatadogProfilerSafeAndConfigured() { return isDatadogProfilerEnabled; } /** - * Whether the OpenTelemetry thread and process context should be exposed through the Datadog - * profiler native library, so external consumers (for example eBPF/CWS) can read the span context - * of a JVM. Enabled when the Datadog profiler is safe and configured and either profiling is - * enabled or AppSec is fully enabled. No dedicated override: disabling profiling and AppSec - * already disables this, the same way {@link #isProfilingEnabled()} has no override of its own - * beyond {@code DD_PROFILING_ENABLED}. + * Whether the OTel thread/process context should be exposed through the Datadog profiler native + * library for external consumers such as eBPF/CWS. */ public boolean isOtelContextExposureEnabled() { return otelContextExposureEnabled; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ProfilingContextIntegration.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ProfilingContextIntegration.java index e5b442c8a4a..f479a956973 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ProfilingContextIntegration.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ProfilingContextIntegration.java @@ -58,15 +58,8 @@ default int encodeResourceName(CharSequence constant) { String name(); /** - * Registers a one-shot callback to run once this integration is actually able to label context. - * - *

Implementations that are fully built by the time they are handed out (the common case) are - * available immediately and run the callback inline. An implementation whose construction is - * deferred runs it later, on the thread that completes that construction, and never runs it if - * the construction fails. Consumers use this to publish metadata about the engine (such as the - * {@code _dd.profiling.ctx} tag) only when there really is an engine behind it. - * - * @param callback invoked at most once, possibly on an arbitrary thread. + * Registers a one-shot callback to run once this integration is actually able to label context; + * runs inline if already available, or never if a deferred construction fails. */ default void whenAvailable(Runnable callback) { callback.run(); From 7c502c23393022fb7269e4d90cb37b79f919b0d9 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 24 Sep 2026 10:39:14 +0200 Subject: [PATCH 16/21] refactor: self-document OTel context config naming per reviewer feedback Rename isOtelContextExposureEnabled() to isOtelThreadContextEnabled() and the internal isDatadogProfilerEnabled field to isDatadogProfilerSafeAndConfigured so each name matches what it actually represents. Also replace remaining em dashes introduced by the previous comment-trimming commit. --- .../java/datadog/trace/bootstrap/Agent.java | 4 ++-- .../DeferredProfilingContextIntegration.java | 2 +- ...AgentLambdaProfilingContextForkedTest.java | 2 +- ...ferredProfilingContextIntegrationTest.java | 2 +- .../main/java/datadog/trace/api/Config.java | 22 +++++++++---------- .../api/ConfigOtelContextExposureTest.java | 16 +++++++------- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index d3ef52bcfb3..412a727e0a7 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -1496,7 +1496,7 @@ static ProfilingContextIntegration createProfilingContextIntegration() { // AWS Lambda has no ddprof native library support, same as startProfilingAgent(). if (!OperatingSystem.isWindows() && !isAwsLambdaRuntime()) { // ORed explicitly so real profiling keeps ddprof regardless of AppSec activation level. - if (config.isDatadogProfilerEnabled() || config.isOtelContextExposureEnabled()) { + if (config.isDatadogProfilerEnabled() || config.isOtelThreadContextEnabled()) { // Deferred unless profiling is enabled: construction loads the ddprof native library and // touches java.nio.file, which must not happen on the primordial premain thread. ProfilingContextIntegration integration = @@ -1529,7 +1529,7 @@ static ProfilingContextIntegration createProfilingContextIntegration() { static ProfilingContextIntegration createDdprofContextIntegration( final ClassLoader classLoader, final boolean deferInitialization) { // deferInitialization means the profiler itself isn't running, so nothing else registers the - // process context here — ProfilingAgent.run() already does it when the profiler starts. + // process context here: ProfilingAgent.run() already does it when the profiler starts. Callable factory = ddprofContextIntegrationFactory(classLoader, deferInitialization); if (deferInitialization) { diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java index a4f8a012d67..4e54a35cc54 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java @@ -116,7 +116,7 @@ public void whenAvailable(final Runnable callback) { } /** - * The name of the deferred integration, not of the current delegate — read once at tracer build + * The name of the deferred integration, not of the current delegate: read once at tracer build * time, possibly before the deferred construction completes. */ @Override diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/AgentLambdaProfilingContextForkedTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/AgentLambdaProfilingContextForkedTest.java index c49308a63cc..a68a29a782f 100644 --- a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/AgentLambdaProfilingContextForkedTest.java +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/AgentLambdaProfilingContextForkedTest.java @@ -37,7 +37,7 @@ void doesNotCreateTheDdprofIntegrationInAwsLambda() { // The exclusion is only observable when the configuration would otherwise have triggered the // ddprof context integration; the Datadog profiler is vetoed on some platforms and JVMs. assumeTrue( - Config.get().isOtelContextExposureEnabled(), + Config.get().isOtelThreadContextEnabled(), "OTel context exposure is unavailable on this platform/JVM version"); // AGENT_CLASSLOADER is null in this unit test, so reaching the ddprof branch at all would fail diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java index 2616af6fa72..3eb2bea986f 100644 --- a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java @@ -109,7 +109,7 @@ void delegatesToTheRealIntegrationOnceInitialized() { assertEquals("ddprof", deferred.name()); // every other pass-through method must reach the swapped-in delegate too, not just - // newScopeState/name — each is a distinct code path in DeferredProfilingContextIntegration. + // newScopeState/name: each is a distinct code path in DeferredProfilingContextIntegration. deferred.onAttach(); deferred.onDetach(); assertEquals(1, FakeDatadogProfilingIntegration.onAttachCalls.get()); diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 095f2d17f4b..4058a5161a0 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -1081,8 +1081,8 @@ public static String getHostName() { private final ProfilingEnablement profilingEnabled; private final boolean profilingAgentless; - private final boolean isDatadogProfilerEnabled; - private final boolean otelContextExposureEnabled; + private final boolean isDatadogProfilerSafeAndConfigured; + private final boolean otelThreadContextEnabled; @Deprecated private final String profilingUrl; private final Map profilingTags; private final int profilingStartDelay; @@ -2401,7 +2401,7 @@ && isMetricsOtelEnabled() profilingEnabled = ProfilingEnablement.of(value); profilingAgentless = configProvider.getBoolean(PROFILING_AGENTLESS, PROFILING_AGENTLESS_DEFAULT); - isDatadogProfilerEnabled = + isDatadogProfilerSafeAndConfigured = !isDatadogProfilerEnablementOverridden() && configProvider.getBoolean( PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) @@ -2647,7 +2647,7 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) : instrumenterConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED; // No dedicated flag: kill switch is DD_PROFILING_ENABLED / DD_APPSEC_ENABLED. - this.otelContextExposureEnabled = + this.otelThreadContextEnabled = isDatadogProfilerSafeAndConfigured() && (isProfilingEnabled() || instrumenterConfig.getAppSecActivation() == ProductActivation.FULLY_ENABLED); @@ -4253,23 +4253,23 @@ public boolean isProfilingRecordExceptionMessage() { * recording". */ public boolean isDatadogProfilerEnabled() { - return isProfilingEnabled() && isDatadogProfilerEnabled; + return isProfilingEnabled() && isDatadogProfilerSafeAndConfigured; } /** * The raw ddprof env-safety/explicit-flag predicate, without the {@link #isProfilingEnabled()} - * AND-prefix — reused by {@link #isOtelContextExposureEnabled()}. + * AND-prefix: reused by {@link #isOtelThreadContextEnabled()}. */ public boolean isDatadogProfilerSafeAndConfigured() { - return isDatadogProfilerEnabled; + return isDatadogProfilerSafeAndConfigured; } /** * Whether the OTel thread/process context should be exposed through the Datadog profiler native * library for external consumers such as eBPF/CWS. */ - public boolean isOtelContextExposureEnabled() { - return otelContextExposureEnabled; + public boolean isOtelThreadContextEnabled() { + return otelThreadContextEnabled; } public static boolean isDatadogProfilerEnablementOverridden() { @@ -6821,8 +6821,8 @@ public String toString() { + profilingExceptionHistogramMaxCollectionSize + ", profilingExcludeAgentThreads=" + profilingExcludeAgentThreads - + ", otelContextExposureEnabled=" - + otelContextExposureEnabled + + ", otelThreadContextEnabled=" + + otelThreadContextEnabled + ", crashTrackingTags=" + crashTrackingTags + ", crashTrackingAgentless=" diff --git a/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java index 198f3f6ac59..77ee10d3962 100644 --- a/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java +++ b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java @@ -12,7 +12,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -/** Tests the resolution of {@link Config#isOtelContextExposureEnabled()}. */ +/** Tests the resolution of {@link Config#isOtelThreadContextEnabled()}. */ @ExtendWith(WithConfigExtension.class) class ConfigOtelContextExposureTest { @@ -29,7 +29,7 @@ private static void assumeDatadogProfilerNotVetoed() { @Test void disabledByDefault() { - assertFalse(Config.get().isOtelContextExposureEnabled()); + assertFalse(Config.get().isOtelThreadContextEnabled()); } @Test @@ -39,7 +39,7 @@ void disabledByDefault() { void enabledWhenProfilingIsEnabled() { assumeDatadogProfilerNotVetoed(); - assertTrue(Config.get().isOtelContextExposureEnabled()); + assertTrue(Config.get().isOtelThreadContextEnabled()); } @Test @@ -51,7 +51,7 @@ void enabledWhenAppSecIsFullyEnabledWithoutProfiling() { Config config = Config.get(); assertFalse(config.isProfilingEnabled()); - assertTrue(config.isOtelContextExposureEnabled()); + assertTrue(config.isOtelThreadContextEnabled()); } @Test @@ -59,7 +59,7 @@ void enabledWhenAppSecIsFullyEnabledWithoutProfiling() { @WithConfig(key = PROFILING_ENABLED, value = "false") @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "true") void disabledWhenAppSecIsOnlyEnabledInactive() { - assertFalse(Config.get().isOtelContextExposureEnabled()); + assertFalse(Config.get().isOtelThreadContextEnabled()); } @Test @@ -67,7 +67,7 @@ void disabledWhenAppSecIsOnlyEnabledInactive() { @WithConfig(key = PROFILING_ENABLED, value = "false") @WithConfig(key = PROFILING_DATADOG_PROFILER_ENABLED, value = "false") void disabledWhenDatadogProfilerIsExplicitlyDisabled() { - assertFalse(Config.get().isOtelContextExposureEnabled()); + assertFalse(Config.get().isOtelThreadContextEnabled()); } /** @@ -78,12 +78,12 @@ void disabledWhenDatadogProfilerIsExplicitlyDisabled() { * boolean short-circuit through the {@code DD_PROFILING_DDPROF_ENABLED=false} env variable: from * {@link Config}'s point of view an environment-detected "unsafe" and an explicit "false" * collapse into the same raw-predicate value, so the downstream effect on {@link - * Config#isOtelContextExposureEnabled()} is the same. + * Config#isOtelThreadContextEnabled()} is the same. */ @Test @WithConfig(key = "APPSEC_ENABLED", value = "true", env = true) @WithConfig(key = "PROFILING_DDPROF_ENABLED", value = "false", env = true) void disabledInAnEnvironmentWhereTheDatadogProfilerIsUnsafe() { - assertFalse(Config.get().isOtelContextExposureEnabled()); + assertFalse(Config.get().isOtelThreadContextEnabled()); } } From c7c52f921970ecdea6a6bff2abd63a92336c81ea Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Thu, 24 Sep 2026 11:23:42 +0200 Subject: [PATCH 17/21] refactor: split ddprof context integration into load/defer paths per reviewer feedback Replace the single createDdprofContextIntegration/ddprofContextIntegrationFactory pair, which reused one boolean flag for two unrelated purposes (defer-vs-sync construction, and process-context registration), with four dedicated methods: loadDdprofContextIntegration, deferDdprofContextIntegration, and the shared private helpers newDdprofContextIntegration and registerProcessContext. Also apply double-checked locking to DeferredProfilingContextIntegration#whenAvailable, now safe since delegate is volatile, and clarify the initialize() Javadoc contract. --- .../java/datadog/trace/bootstrap/Agent.java | 94 ++++++++++--------- .../DeferredProfilingContextIntegration.java | 14 ++- ...ferredProfilingContextIntegrationTest.java | 6 +- .../OtelContextExposureSmokeTest.groovy | 2 +- 4 files changed, 61 insertions(+), 55 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index 412a727e0a7..44fa21b5eec 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -73,7 +73,6 @@ import java.net.URL; import java.security.CodeSource; import java.util.EnumSet; -import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.regex.PatternSyntaxException; @@ -1495,15 +1494,18 @@ static ProfilingContextIntegration createProfilingContextIntegration() { Config config = Config.get(); // AWS Lambda has no ddprof native library support, same as startProfilingAgent(). if (!OperatingSystem.isWindows() && !isAwsLambdaRuntime()) { - // ORed explicitly so real profiling keeps ddprof regardless of AppSec activation level. - if (config.isDatadogProfilerEnabled() || config.isOtelThreadContextEnabled()) { - // Deferred unless profiling is enabled: construction loads the ddprof native library and - // touches java.nio.file, which must not happen on the primordial premain thread. - ProfilingContextIntegration integration = - createDdprofContextIntegration(AGENT_CLASSLOADER, !config.isDatadogProfilerEnabled()); + if (config.isDatadogProfilerEnabled()) { + // The profiler itself is running: load ddprof now, and let ProfilingAgent.run() register + // the process context as it always has. + ProfilingContextIntegration integration = loadDdprofContextIntegration(AGENT_CLASSLOADER); if (integration != null) { return integration; } + } else if (config.isOtelThreadContextEnabled()) { + // No profiler, we only want the context exposed: loading ddprof pulls in the native + // library and touches java.nio.file, which must not happen on the primordial premain + // thread, so it is deferred. + return deferDdprofContextIntegration(AGENT_CLASSLOADER); } } if (config.isProfilingEnabled() && config.isProfilingTimelineEventsEnabled()) { @@ -1522,24 +1524,13 @@ static ProfilingContextIntegration createProfilingContextIntegration() { } /** - * Creates the ddprof-based profiling context integration, either synchronously or deferred off - * the calling thread onto an {@link AgentTaskScheduler} thread. Returns {@code null} if a - * synchronous construction fails, so the caller can fall back to another integration. + * Loads the ddprof-based profiling context integration on the calling thread, for when the + * Datadog profiler is running. Returns {@code null} when it isn't available, so the caller can + * fall back to another integration. */ - static ProfilingContextIntegration createDdprofContextIntegration( - final ClassLoader classLoader, final boolean deferInitialization) { - // deferInitialization means the profiler itself isn't running, so nothing else registers the - // process context here: ProfilingAgent.run() already does it when the profiler starts. - Callable factory = - ddprofContextIntegrationFactory(classLoader, deferInitialization); - if (deferInitialization) { - DeferredProfilingContextIntegration deferred = - new DeferredProfilingContextIntegration("ddprof", factory); - deferred.scheduleInitialization(); - return deferred; - } + static ProfilingContextIntegration loadDdprofContextIntegration(final ClassLoader classLoader) { try { - return factory.call(); + return newDdprofContextIntegration(classLoader); } catch (Throwable t) { log.debug("ddprof-based profiling context labeling not available. {}", t.getMessage()); return null; @@ -1547,30 +1538,41 @@ static ProfilingContextIntegration createDdprofContextIntegration( } /** - * Builds the ddprof integration reflectively, optionally registering the OTel process context - * alongside it (when the profiler agent isn't going to register it itself). + * Returns a placeholder integration that loads the ddprof-based one off the calling thread and + * registers the OTel process context alongside it. Only used when the profiler isn't running: + * otherwise {@code ProfilingAgent.run()} registers the process context itself. */ - private static Callable ddprofContextIntegrationFactory( - final ClassLoader classLoader, final boolean registerProcessContext) { - return () -> { - ProfilingContextIntegration integration = - (ProfilingContextIntegration) - classLoader - .loadClass("com.datadog.profiling.ddprof.DatadogProfilingIntegration") - .getDeclaredConstructor() - .newInstance(); - if (registerProcessContext) { - try { - classLoader - .loadClass("com.datadog.profiling.agent.ProcessContext") - .getMethod("register", ConfigProvider.class) - .invoke(null, ConfigProvider.getInstance()); - } catch (Throwable t) { - log.debug("Process context registration not available. {}", t.getMessage()); - } - } - return integration; - }; + static ProfilingContextIntegration deferDdprofContextIntegration(final ClassLoader classLoader) { + DeferredProfilingContextIntegration deferred = + new DeferredProfilingContextIntegration( + "ddprof", + () -> { + ProfilingContextIntegration integration = newDdprofContextIntegration(classLoader); + registerProcessContext(classLoader); + return integration; + }); + deferred.scheduleInitialization(); + return deferred; + } + + private static ProfilingContextIntegration newDdprofContextIntegration( + final ClassLoader classLoader) throws ReflectiveOperationException { + return (ProfilingContextIntegration) + classLoader + .loadClass("com.datadog.profiling.ddprof.DatadogProfilingIntegration") + .getDeclaredConstructor() + .newInstance(); + } + + private static void registerProcessContext(final ClassLoader classLoader) { + try { + classLoader + .loadClass("com.datadog.profiling.agent.ProcessContext") + .getMethod("register", ConfigProvider.class) + .invoke(null, ConfigProvider.getInstance()); + } catch (Throwable t) { + log.debug("Process context registration not available. {}", t.getMessage()); + } } private static boolean startProfilingAgent( diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java index 4e54a35cc54..db9b52d19ec 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DeferredProfilingContextIntegration.java @@ -72,7 +72,8 @@ void scheduleInitialization() { } /** - * Runs the deferred construction. On failure this instance keeps behaving as {@link + * Runs the deferred construction; called exactly once per instance, from {@link + * #scheduleInitialization()}. On failure this instance keeps behaving as {@link * ProfilingContextIntegration.NoOp} forever; a background failure must never propagate. */ void initialize() { @@ -106,10 +107,13 @@ void initialize() { */ @Override public void whenAvailable(final Runnable callback) { - synchronized (this) { - if (delegate == ProfilingContextIntegration.NoOp.INSTANCE) { - pendingAvailabilityCallbacks.add(callback); - return; + // double-checked: delegate is volatile, so a post-swap caller never takes the lock + if (delegate == ProfilingContextIntegration.NoOp.INSTANCE) { + synchronized (this) { + if (delegate == ProfilingContextIntegration.NoOp.INSTANCE) { + pendingAvailabilityCallbacks.add(callback); + return; + } } } callback.run(); diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java index 3eb2bea986f..19db2b76af1 100644 --- a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java @@ -52,7 +52,7 @@ void deferredConstructionDoesNotRunOnTheCallingThread() throws Exception { FakeDatadogProfilingIntegration.gate = new CountDownLatch(1); ProfilingContextIntegration integration = - Agent.createDdprofContextIntegration(fakeProfilingClassLoader(), true); + Agent.deferDdprofContextIntegration(fakeProfilingClassLoader()); // nothing was constructed synchronously on this (premain) thread assertNotNull(integration); @@ -78,7 +78,7 @@ void deferredConstructionDoesNotRunOnTheCallingThread() throws Exception { @Test void synchronousConstructionKeepsRunningOnTheCallingThread() { ProfilingContextIntegration integration = - Agent.createDdprofContextIntegration(fakeProfilingClassLoader(), false); + Agent.loadDdprofContextIntegration(fakeProfilingClassLoader()); assertTrue(integration instanceof FakeDatadogProfilingIntegration); assertEquals(1, FakeDatadogProfilingIntegration.constructions.get()); @@ -91,7 +91,7 @@ void synchronousConstructionKeepsRunningOnTheCallingThread() { */ @Test void synchronousConstructionLeavesTheProcessContextToTheProfilerAgent() { - Agent.createDdprofContextIntegration(fakeProfilingClassLoader(), false); + Agent.loadDdprofContextIntegration(fakeProfilingClassLoader()); assertEquals(0, FakeProcessContext.registrations.get()); } diff --git a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy index 4eaaa8a7c80..ff09ba67fe1 100644 --- a/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy +++ b/dd-smoke-tests/appsec/springboot/src/test/groovy/datadog/smoketest/appsec/OtelContextExposureSmokeTest.groovy @@ -8,7 +8,7 @@ class OtelContextExposureSmokeTest extends AbstractAppSecServerSmokeTest { private static final String PROCESS_CONTEXT_LOG_LINE = 'Registering process context for OTel profiler' private static final String PROCESS_CONTEXT_FAILURE_LOG_LINE = 'Failed to register process context for OTel profiler' - /** Logged by Agent#ddprofContextIntegrationFactory when the reflective registration call itself fails. */ + /** Logged by Agent#registerProcessContext when the reflective registration call itself fails. */ private static final String PROCESS_CONTEXT_UNAVAILABLE_LOG_LINE = 'Process context registration not available' @Override From 7d3d3e74aacd9c6886add549e8478a2b11ad4a6e Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 25 Sep 2026 09:10:20 +0200 Subject: [PATCH 18/21] refactor: drop redundant Windows check in createProfilingContextIntegration Config.isDatadogProfilerEnabled() and Config.isOtelThreadContextEnabled() already resolve to false on Windows via isDatadogProfilerSafeAndConfigured (isDatadogProfilerEnablementOverridden() checks OperatingSystem.isWindows()), so the local OperatingSystem.isWindows() check here was a second, independently-maintained exclusion. AWS Lambda has no equivalent Config-side guard, so !isAwsLambdaRuntime() stays. --- .../src/main/java/datadog/trace/bootstrap/Agent.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index 44fa21b5eec..acf1a797f19 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -1492,8 +1492,10 @@ public void withTracer(TracerAPI tracer) { */ static ProfilingContextIntegration createProfilingContextIntegration() { Config config = Config.get(); - // AWS Lambda has no ddprof native library support, same as startProfilingAgent(). - if (!OperatingSystem.isWindows() && !isAwsLambdaRuntime()) { + // Windows is already excluded by Config (isDatadogProfilerSafeAndConfigured), so only AWS + // Lambda needs to be excluded here: it has no ddprof native library support, same as + // startProfilingAgent(). + if (!isAwsLambdaRuntime()) { if (config.isDatadogProfilerEnabled()) { // The profiler itself is running: load ddprof now, and let ProfilingAgent.run() register // the process context as it always has. From ae334b4e5730f1f46aa94e84069ec67dc5786a2f Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 25 Sep 2026 10:33:16 +0200 Subject: [PATCH 19/21] test: cover null-returning factory in DeferredProfilingContextIntegration initialize() already guards against a null integration from the factory, but no test constructed one, so a mutation deleting that guard would have survived (delegate=null, NPE on any pass-through call after initialize()). --- .../DeferredProfilingContextIntegrationTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java index 19db2b76af1..671188e913d 100644 --- a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/DeferredProfilingContextIntegrationTest.java @@ -161,6 +161,20 @@ void staysNoOpWhenTheDeferredConstructionFails() { assertEquals("ddprof", deferred.name()); } + @Test + void staysNoOpWhenTheFactoryReturnsNull() { + DeferredProfilingContextIntegration deferred = + new DeferredProfilingContextIntegration("ddprof", () -> null); + AtomicInteger callbacks = new AtomicInteger(); + + deferred.whenAvailable(callbacks::incrementAndGet); + deferred.initialize(); + + assertEquals(0, callbacks.get()); + assertSame(Stateful.DEFAULT, deferred.newScopeState(null)); + assertSame(ProfilingScope.NO_OP, deferred.newScope()); + } + @Test void availabilityCallbacksRunOnlyOnceTheRealIntegrationIsIn() { DeferredProfilingContextIntegration deferred = From 3100f8a603ba50924397276a9280af232276a41a Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 25 Sep 2026 11:07:33 +0200 Subject: [PATCH 20/21] test: rename disabledInAnEnvironmentWhereTheDatadogProfilerIsUnsafe The method name overclaimed what the test covers: it drives the same raw-predicate short-circuit as the explicit-disable test via an env var, not a genuine environment-detection veto (already explained in the javadoc). Renamed for honesty; no behavior change. --- .../java/datadog/trace/api/ConfigOtelContextExposureTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java index 77ee10d3962..a095fa72553 100644 --- a/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java +++ b/internal-api/src/test/java/datadog/trace/api/ConfigOtelContextExposureTest.java @@ -83,7 +83,7 @@ void disabledWhenDatadogProfilerIsExplicitlyDisabled() { @Test @WithConfig(key = "APPSEC_ENABLED", value = "true", env = true) @WithConfig(key = "PROFILING_DDPROF_ENABLED", value = "false", env = true) - void disabledInAnEnvironmentWhereTheDatadogProfilerIsUnsafe() { + void disabledWhenDatadogProfilerIsUnsafeOrExplicitlyDisabledViaEnv() { assertFalse(Config.get().isOtelThreadContextEnabled()); } } From 19885f93f1b47fb2c9ef775e39167a09260272bb Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Fri, 25 Sep 2026 14:15:44 +0200 Subject: [PATCH 21/21] Add explicit profiling guard and needsIntercept regression test - Make the !isProfilingEnabled() invariant on the ddprof context-only branch explicit in Agent.createProfilingContextIntegration(), per reviewer feedback that there was no local guard against skipping the JFR-events fallback. - Add a regression test pinning the needsIntercept() recomputation in CoreTracer.stampProfilingContextEngine(), covering the trace.split-by-tags scenario raised in review. --- .../java/datadog/trace/bootstrap/Agent.java | 5 +++- .../datadog/trace/core/CoreTracerTest.java | 29 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index acf1a797f19..e58dfc8b228 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -1503,10 +1503,13 @@ static ProfilingContextIntegration createProfilingContextIntegration() { if (integration != null) { return integration; } - } else if (config.isOtelThreadContextEnabled()) { + } else if (!config.isProfilingEnabled() && config.isOtelThreadContextEnabled()) { // No profiler, we only want the context exposed: loading ddprof pulls in the native // library and touches java.nio.file, which must not happen on the primordial premain // thread, so it is deferred. + // The explicit !isProfilingEnabled() guard (redundant with isOtelThreadContextEnabled()'s + // own isDatadogProfilerSafeAndConfigured() factor) keeps this branch provably unreachable + // whenever profiling is enabled, so the JFR-events fallback below is never skipped. return deferDdprofContextIntegration(AGENT_CLASSLOADER); } } diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java index 96828c9114c..666431d8f1f 100644 --- a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +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.assertTrue; @@ -383,6 +384,34 @@ void profilingContextEngineTagWithheldUntilTheIntegrationBecomesAvailable() { } } + /** + * Pins the needsIntercept half of the {@code LocalRootSpanTags} swap: {@code + * stampProfilingContextEngine()} recomputes {@code tagInterceptor.needsIntercept()} on the frozen + * tag map, so a root span started after the swap must apply interception rules (here, {@code + * trace.split-by-tags}) to the newly-stamped {@code _dd.profiling.ctx} tag exactly like any other + * tag present at span-start time, while one started before the swap must not. + */ + @Test + @WithConfig(key = TracerConfig.SPLIT_BY_TAGS, value = DDTags.PROFILING_CONTEXT_ENGINE) + void needsInterceptRecomputationAppliesSplitByTagsOnceProfilingContextEngineTagIsStamped() { + FakeContextIntegration integration = new FakeContextIntegration(); + integration.deferAvailability = true; + CoreTracer tracer = tracerBuilder().profilingContextIntegration(integration).build(); + try { + DDSpan beforeSwap = (DDSpan) tracer.buildSpan("datadog", "before").start(); + assertNotEquals(FAKE_ENGINE, beforeSwap.getServiceName()); + beforeSwap.finish(); + + integration.becomeAvailable(); + + DDSpan afterSwap = (DDSpan) tracer.buildSpan("datadog", "after").start(); + assertEquals(FAKE_ENGINE, afterSwap.getServiceName()); + afterSwap.finish(); + } finally { + tracer.close(); + } + } + @Test void prioritySamplingWhenSpanFinishes() throws Exception { ListWriter writer = new ListWriter();