WIP

Author: mcculls

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java

@@ -1,7 +1,6 @@
 package datadog.trace.bootstrap;
 
 import static datadog.trace.api.ConfigDefaults.DEFAULT_STARTUP_LOGS_ENABLED;
-import static datadog.trace.api.Platform.getRuntimeVendor;
 import static datadog.trace.api.Platform.isJavaVersionAtLeast;
 import static datadog.trace.api.Platform.isOracleJDK8;
 import static datadog.trace.bootstrap.Library.WILDFLY;
@@ -329,7 +328,7 @@ public void run() {
       if (appUsingCustomJMXBuilder) {
         log.debug("Custom JMX builder detected. Delaying JMXFetch initialization.");
         registerMBeanServerBuilderCallback(new StartJmxCallback(jmxStartDelay));
-        // one minute fail-safe in case nothing touches JMX and and callback isn't triggered
+        // one minute fail-safe in case nothing touches JMX and callback isn't triggered
         scheduleJmxStart(60 + jmxStartDelay);
       } else if (appUsingCustomLogManager) {
         log.debug("Custom logger detected. Delaying JMXFetch initialization.");
@@ -339,20 +338,31 @@ public void run() {
       }
     }
 
-    boolean delayOkHttp = appUsingCustomLogManager && okHttpMayIndirectlyLoadJUL();
-
     /*
      * Similar thing happens with DatadogTracer on (at least) zulu-8 because it uses OkHttp which indirectly loads JFR
      * events which in turn loads LogManager. This is not a problem on newer JDKs because there JFR uses different
      * logging facility. Likewise on IBM JDKs OkHttp may indirectly load 'IBMSASL' which in turn loads LogManager.
      */
+    boolean delayOkHttp = !ciVisibilityEnabled && okHttpMayIndirectlyLoadJUL();
+    boolean waitForJUL = appUsingCustomLogManager && delayOkHttp;
+    int okHttpDelayMillis;
+    if (waitForJUL) {
+      okHttpDelayMillis = 1_000;
+    } else if (delayOkHttp) {
+      okHttpDelayMillis = 10;
+    } else {
+      okHttpDelayMillis = 0;
+    }
+
     InstallDatadogTracerCallback installDatadogTracerCallback =
-        new InstallDatadogTracerCallback(initTelemetry, inst, delayOkHttp);
-    if (delayOkHttp) {
+        new InstallDatadogTracerCallback(initTelemetry, inst, okHttpDelayMillis);
+    if (waitForJUL) {
       log.debug("Custom logger detected. Delaying Datadog Tracer initialization.");
       registerLogManagerCallback(installDatadogTracerCallback);
+    } else if (okHttpDelayMillis > 0) {
+      installDatadogTracerCallback.run(); // start on different thread (after premain)
     } else {
-      installDatadogTracerCallback.execute();
+      installDatadogTracerCallback.execute(); // start during premain
     }
 
     /*
@@ -362,7 +372,7 @@ public void run() {
     if (profilingEnabled && !isOracleJDK8()) {
       StaticEventLogger.begin("Profiling");
 
-      if (delayOkHttp) {
+      if (waitForJUL) {
         log.debug("Custom logger detected. Delaying Profiling initialization.");
         registerLogManagerCallback(new StartProfilingAgentCallback(inst));
       } else {
@@ -499,18 +509,18 @@ protected static class InstallDatadogTracerCallback extends ClassLoadCallBack {
     private final Instrumentation instrumentation;
     private final Object sco;
     private final Class<?> scoClass;
-    private final boolean delayOkHttp;
+    private final int okHttpDelayMillis;
 
     public InstallDatadogTracerCallback(
         InitializationTelemetry initTelemetry,
         Instrumentation instrumentation,
-        boolean delayOkHttp) {
-      this.delayOkHttp = delayOkHttp;
+        int okHttpDelayMillis) {
+      this.okHttpDelayMillis = okHttpDelayMillis;
       this.instrumentation = instrumentation;
       try {
         scoClass =
             AGENT_CLASSLOADER.loadClass("datadog.communication.ddagent.SharedCommunicationObjects");
-        sco = scoClass.getConstructor(boolean.class).newInstance(delayOkHttp);
+        sco = scoClass.getConstructor(boolean.class).newInstance(okHttpDelayMillis > 0);
       } catch (ClassNotFoundException
           | NoSuchMethodException
           | InstantiationException
@@ -521,6 +531,7 @@ public InstallDatadogTracerCallback(
 
       installDatadogTracer(initTelemetry, scoClass, sco);
       maybeInstallLogsIntake(scoClass, sco);
+      maybeStartIast(instrumentation);
     }
 
     @Override
@@ -530,12 +541,11 @@ public AgentThread agentThread() {
 
     @Override
     public void execute() {
-      if (delayOkHttp) {
+      if (okHttpDelayMillis > 0) {
         resumeRemoteComponents();
       }
 
       maybeStartAppSec(scoClass, sco);
-      maybeStartIast(instrumentation, scoClass, sco);
       maybeStartCiVisibility(instrumentation, scoClass, sco);
       // start debugger before remote config to subscribe to it before starting to poll
       maybeStartDebugger(instrumentation, scoClass, sco);
@@ -550,7 +560,7 @@ private void resumeRemoteComponents() {
       try {
         // remote components were paused for custom log-manager/jmx-builder
         // add small delay before resuming remote I/O to help stabilization
-        Thread.sleep(1_000);
+        Thread.sleep(okHttpDelayMillis);
         scoClass.getMethod("resume").invoke(sco);
       } catch (InterruptedException ignore) {
       } catch (Throwable e) {
@@ -837,14 +847,14 @@ private static boolean isSupportedAppSecArch() {
     return true;
   }
 
-  private static void maybeStartIast(Instrumentation instrumentation, Class<?> scoClass, Object o) {
+  private static void maybeStartIast(Instrumentation instrumentation) {
     if (iastEnabled || !iastFullyDisabled) {
 
       StaticEventLogger.begin("IAST");
 
       try {
         SubscriptionService ss = AgentTracer.get().getSubscriptionService(RequestContextSlot.IAST);
-        startIast(instrumentation, ss, scoClass, o);
+        startIast(instrumentation, ss);
       } catch (Exception e) {
         log.error("Error starting IAST subsystem", e);
       }
@@ -853,8 +863,7 @@ private static void maybeStartIast(Instrumentation instrumentation, Class<?> sco
     }
   }
 
-  private static void startIast(
-      Instrumentation instrumentation, SubscriptionService ss, Class<?> scoClass, Object sco) {
+  private static void startIast(Instrumentation instrumentation, SubscriptionService ss) {
     try {
       final Class<?> appSecSysClass = AGENT_CLASSLOADER.loadClass("com.datadog.iast.IastSystem");
       final Method iastInstallerMethod =
@@ -1339,15 +1348,25 @@ private static String ddGetEnv(final String sysProp) {
   }
 
   private static boolean okHttpMayIndirectlyLoadJUL() {
-    if ("IBM Corporation".equals(getRuntimeVendor())) {
-      return true; // IBM JDKs ship with 'IBMSASL' which will load JUL when OkHttp accesses TLS
+    if (isIBMSASLInstalled() || isACCPInstalled()) {
+      return true; // 'IBMSASL' and 'ACCP' crypto providers can load JUL when OkHttp accesses TLS
     }
     if (isJavaVersionAtLeast(9)) {
       return false; // JDKs since 9 have reworked JFR to use a different logging facility, not JUL
     }
     return isJFRSupported(); // assume OkHttp will indirectly load JUL via its JFR events
   }
 
+  private static boolean isIBMSASLInstalled() {
+    return ClassLoader.getSystemResource("com/ibm/security/sasl/IBMSASL.class") != null;
+  }
+
+  private static boolean isACCPInstalled() {
+    return ClassLoader.getSystemResource(
+            "com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.class")
+        != null;
+  }
+
   private static boolean isJFRSupported() {
     // FIXME: this is quite a hack because there maybe jfr classes on classpath somehow that have
     // nothing to do with JDK - but this should be safe because only thing this does is to delay

dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/IastSystemTest.groovy

@@ -136,7 +136,7 @@ class IastSystemTest extends DDSpecification {
     InstrumentationBridge.clearIastModules()
 
     when:
-    Agent.maybeStartIast(null, null, null)
+    Agent.maybeStartIast(null)
 
     then:
     InstrumentationBridge.iastModules.each {