WIP

DataDog · Dec 23, 2024 · d3ed23d · d3ed23d
1 parent 13d1c4b
commit d3ed23d
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 36 deletions.
diff --git 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<Runnable> 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
@@ -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,26 +496,30 @@ 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
           | IllegalAccessException
           | 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(500);
+        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
@@ -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
@@ -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
@@ -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-smoke-tests/log-injection/src/test/groovy/datadog/smoketest/LogInjectionSmokeTest.groovy b/dd-smoke-tests/log-injection/src/test/groovy/datadog/smoketest/LogInjectionSmokeTest.groovy
@@ -313,6 +313,10 @@ abstract class LogInjectionSmokeTest extends AbstractSmokeTest {
     }
 
     if (supportsDirectLogSubmission()) {
+      int expectedLogLineCount = 7
+      if ("IBM Corporation" == System.getProperty("java.vm.vendor")) {
+        expectedLogLineCount = 3
+      }
       assertParsedJsonLinesWithInjection(mockBackend.waitForLogs(7), firstTraceId, firstSpanId, secondTraceId, secondSpanId, forthTraceId, forthSpanId)
     }
   }

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
@@ -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 =