From 2279f72d88bed751a66f9f7d56f02cbddee0023c Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Sun, 22 Dec 2024 21:21:19 +0000 Subject: [PATCH] WIP --- .../ddagent/SharedCommunicationObjects.java | 50 +++++++++++++++++-- .../java/datadog/trace/bootstrap/Agent.java | 29 ++++++++--- .../trace/agent/CustomLogManagerTest.groovy | 2 +- .../agent/CustomMBeanServerBuilderTest.groovy | 2 +- .../jvmbootstraptest/LogManagerSetter.java | 29 +++-------- .../java/datadog/trace/core/CoreTracer.java | 5 +- 6 files changed, 81 insertions(+), 36 deletions(-) diff --git a/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java b/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java index 1a44228d98a..5ab46582b72 100644 --- a/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java +++ b/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java @@ -11,6 +11,8 @@ import datadog.remoteconfig.DefaultConfigurationPoller; import datadog.trace.api.Config; import datadog.trace.util.AgentTaskScheduler; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import okhttp3.HttpUrl; @@ -21,12 +23,23 @@ public class SharedCommunicationObjects { private static final Logger log = LoggerFactory.getLogger(SharedCommunicationObjects.class); + private final List pausedComponents = new ArrayList<>(); + private volatile boolean paused; + public OkHttpClient okHttpClient; public HttpUrl agentUrl; public Monitoring monitoring; private DDAgentFeaturesDiscovery featuresDiscovery; private ConfigurationPoller configurationPoller; + public SharedCommunicationObjects() { + this(false); + } + + public SharedCommunicationObjects(boolean paused) { + this.paused = paused; + } + public void createRemaining(Config config) { if (monitoring == null) { monitoring = Monitoring.DISABLED; @@ -46,6 +59,30 @@ public void createRemaining(Config config) { } } + public void whenReady(Runnable callback) { + synchronized (pausedComponents) { + if (paused) { + pausedComponents.add(callback); + } else { + callback.run(); + } + } + } + + public void resume() { + paused = false; + synchronized (pausedComponents) { + for (Runnable callback : pausedComponents) { + try { + callback.run(); + } catch (Throwable e) { + log.warn("Problem resuming remote component {}", callback, e); + } + } + pausedComponents.clear(); + } + } + private static HttpUrl parseAgentUrl(Config config) { String agentUrl = config.getAgentUrl(); if (agentUrl.startsWith("unix:")) { @@ -100,11 +137,14 @@ public DDAgentFeaturesDiscovery featuresDiscovery(Config config) { agentUrl, config.isTraceAgentV05Enabled(), config.isTracerMetricsEnabled()); - if (AGENT_THREAD_GROUP.equals(Thread.currentThread().getThreadGroup())) { - featuresDiscovery.discover(); // safe to run on same thread - } else { - // avoid performing blocking I/O operation on application thread - AgentTaskScheduler.INSTANCE.execute(featuresDiscovery::discover); + + if (!paused) { + if (AGENT_THREAD_GROUP.equals(Thread.currentThread().getThreadGroup())) { + featuresDiscovery.discover(); // safe to run on same thread + } else { + // avoid performing blocking I/O operation on application thread + AgentTaskScheduler.INSTANCE.execute(featuresDiscovery::discover); + } } } return featuresDiscovery; 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 61dce68b3b2..43f53ceeaf8 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 @@ -347,7 +347,7 @@ public void run() { * logging facility. Likewise on IBM JDKs OkHttp may indirectly load 'IBMSASL' which in turn loads LogManager. */ InstallDatadogTracerCallback installDatadogTracerCallback = - new InstallDatadogTracerCallback(initTelemetry, inst); + new InstallDatadogTracerCallback(initTelemetry, inst, delayOkHttp); if (delayOkHttp) { log.debug("Custom logger detected. Delaying Datadog Tracer initialization."); registerLogManagerCallback(installDatadogTracerCallback); @@ -496,19 +496,21 @@ public void execute() { } protected static class InstallDatadogTracerCallback extends ClassLoadCallBack { - private final InitializationTelemetry initTelemetry; private final Instrumentation instrumentation; private final Object sco; private final Class scoClass; + private final boolean delayOkHttp; public InstallDatadogTracerCallback( - InitializationTelemetry initTelemetry, Instrumentation instrumentation) { - this.initTelemetry = initTelemetry; + InitializationTelemetry initTelemetry, + Instrumentation instrumentation, + boolean delayOkHttp) { + this.delayOkHttp = delayOkHttp; this.instrumentation = instrumentation; try { scoClass = AGENT_CLASSLOADER.loadClass("datadog.communication.ddagent.SharedCommunicationObjects"); - sco = scoClass.getConstructor().newInstance(); + sco = scoClass.getConstructor(boolean.class).newInstance(delayOkHttp); } catch (ClassNotFoundException | NoSuchMethodException | InstantiationException @@ -516,6 +518,8 @@ public InstallDatadogTracerCallback( | InvocationTargetException e) { throw new UndeclaredThrowableException(e); } + + installDatadogTracer(initTelemetry, scoClass, sco); } @Override @@ -525,7 +529,10 @@ public AgentThread agentThread() { @Override public void execute() { - installDatadogTracer(initTelemetry, scoClass, sco); + if (delayOkHttp) { + resumeRemoteComponents(); + } + maybeStartAppSec(scoClass, sco); maybeStartIast(instrumentation, scoClass, sco); maybeStartCiVisibility(instrumentation, scoClass, sco); @@ -538,6 +545,16 @@ public void execute() { startTelemetry(instrumentation, scoClass, sco); } } + + private void resumeRemoteComponents() { + try { + Thread.sleep(1_000); + scoClass.getMethod("resume").invoke(sco); + } catch (InterruptedException ignore) { + } catch (Exception e) { + log.error("Error resuming remote components", e); + } + } } protected static class StartProfilingAgentCallback extends ClassLoadCallBack { diff --git a/dd-java-agent/src/test/groovy/datadog/trace/agent/CustomLogManagerTest.groovy b/dd-java-agent/src/test/groovy/datadog/trace/agent/CustomLogManagerTest.groovy index 9ff2d97e32d..89081e01a1b 100644 --- a/dd-java-agent/src/test/groovy/datadog/trace/agent/CustomLogManagerTest.groovy +++ b/dd-java-agent/src/test/groovy/datadog/trace/agent/CustomLogManagerTest.groovy @@ -27,7 +27,7 @@ class CustomLogManagerTest extends Specification { , true) == 0 } - def "agent services starts up in premain if configured log manager on system classpath"() { + def "agent services startup is delayed even if configured log manager on system classpath"() { expect: IntegrationTestUtils.runOnSeparateJvm(LogManagerSetter.getName() , [ diff --git a/dd-java-agent/src/test/groovy/datadog/trace/agent/CustomMBeanServerBuilderTest.groovy b/dd-java-agent/src/test/groovy/datadog/trace/agent/CustomMBeanServerBuilderTest.groovy index 6f2e85125b3..180c98d7d96 100644 --- a/dd-java-agent/src/test/groovy/datadog/trace/agent/CustomMBeanServerBuilderTest.groovy +++ b/dd-java-agent/src/test/groovy/datadog/trace/agent/CustomMBeanServerBuilderTest.groovy @@ -28,7 +28,7 @@ class CustomMBeanServerBuilderTest extends Specification { , true) == 0 } - def "JMXFetch starts up in premain if configured MBeanServerBuilder on system classpath"() { + def "JMXFetch startup is delayed even if configured MBeanServerBuilder on system classpath"() { expect: IntegrationTestUtils.runOnSeparateJvm(MBeanServerBuilderSetter.getName() , [ diff --git a/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java b/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java index f93ce2f8be9..169d488fe05 100644 --- a/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java +++ b/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java @@ -39,8 +39,10 @@ public static void main(final String... args) throws Exception { } else if (System.getProperty("java.util.logging.manager") != null) { System.out.println("java.util.logging.manager != null"); - assertTraceInstallationDelayed( - "tracer install must be delayed when log manager system property is present."); + customAssert( + isTracerInstalled(false), + true, + "tracer install is not delayed when log manager system property is present."); customAssert( isJmxfetchStarted(false), false, @@ -57,8 +59,6 @@ public static void main(final String... args) throws Exception { .getClassLoader() .loadClass(System.getProperty("java.util.logging.manager")), "Javaagent should not prevent setting a custom log manager"); - customAssert( - isTracerInstalled(true), true, "tracer should be installed after loading LogManager."); customAssert( isJmxfetchStarted(true), true, "jmxfetch should start after loading LogManager."); if (isJFRSupported()) { @@ -67,8 +67,10 @@ public static void main(final String... args) throws Exception { } } else if (System.getenv("JBOSS_HOME") != null) { System.out.println("JBOSS_HOME != null"); - assertTraceInstallationDelayed( - "tracer install must be delayed when JBOSS_HOME property is present."); + customAssert( + isTracerInstalled(false), + true, + "tracer install is not delayed when JBOSS_HOME property is present."); customAssert( isJmxfetchStarted(false), false, @@ -85,10 +87,6 @@ public static void main(final String... args) throws Exception { .getClassLoader() .loadClass(System.getProperty("java.util.logging.manager")), "Javaagent should not prevent setting a custom log manager"); - customAssert( - isTracerInstalled(true), - true, - "tracer should be installed after loading with JBOSS_HOME set."); customAssert( isJmxfetchStarted(true), true, @@ -128,17 +126,6 @@ private static void customAssert( } } - private static void assertTraceInstallationDelayed(final String message) { - if (okHttpMayIndirectlyLoadJUL()) { - customAssert(isTracerInstalled(false), false, message); - } else { - customAssert( - isTracerInstalled(false), - true, - "We can safely install tracer on java9+ since it doesn't indirectly trigger logger manager init"); - } - } - private static void assertProfilingStartupDelayed(final String message) { if (okHttpMayIndirectlyLoadJUL()) { customAssert(isProfilingStarted(false), false, message); 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 7d93d13c2a6..db63153921b 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 @@ -689,7 +689,7 @@ private CoreTracer( } pendingTraceBuffer.start(); - this.writer.start(); + sharedCommunicationObjects.whenReady(this.writer::start); metricsAggregator = createMetricsAggregator(config, sharedCommunicationObjects); // Schedule the metrics aggregator to begin reporting after a random delay of 1 to 10 seconds @@ -705,7 +705,8 @@ private CoreTracer( } else { this.dataStreamsMonitoring = dataStreamsMonitoring; } - this.dataStreamsMonitoring.start(); + + sharedCommunicationObjects.whenReady(this.dataStreamsMonitoring::start); // Create default extractor from config if not provided and decorate it with DSM extractor HttpCodec.Extractor builtExtractor =