diff --git a/communication/build.gradle b/communication/build.gradle index 7cf72ea641a..2b621a99b4e 100644 --- a/communication/build.gradle +++ b/communication/build.gradle @@ -25,7 +25,7 @@ dependencies { } ext { - minimumBranchCoverage = 0.6 + minimumBranchCoverage = 0.5 minimumInstructionCoverage = 0.8 excludedClassesCoverage = [ 'datadog.communication.ddagent.ExternalAgentLauncher', diff --git a/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java b/communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java index 1a44228d98a..0309f5a38f1 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,32 @@ public void createRemaining(Config config) { } } + public void whenReady(Runnable callback) { + if (paused) { + synchronized (pausedComponents) { + if (paused) { + pausedComponents.add(callback); + return; + } + } + } + callback.run(); // not paused, run immediately + } + + 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 +139,16 @@ 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 + + if (paused) { + // defer remote discovery until remote I/O is allowed } else { - // avoid performing blocking I/O operation on application thread - AgentTaskScheduler.INSTANCE.execute(featuresDiscovery::discover); + 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 ec5ab19ca09..f3d124cff7c 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 @@ -10,7 +10,6 @@ import static datadog.trace.util.AgentThreadFactory.AgentThread.PROFILER_STARTUP; import static datadog.trace.util.AgentThreadFactory.AgentThread.TRACE_STARTUP; import static datadog.trace.util.AgentThreadFactory.newAgentThread; -import static datadog.trace.util.Strings.getResourceName; import static datadog.trace.util.Strings.propertyNameToSystemPropertyName; import static datadog.trace.util.Strings.toEnvVar; @@ -348,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); @@ -497,28 +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; - } - - @Override - public AgentThread agentThread() { - return TRACE_STARTUP; - } - - @Override - public void execute() { - Object sco; - Class scoClass; try { scoClass = AGENT_CLASSLOADER.loadClass("datadog.communication.ddagent.SharedCommunicationObjects"); - sco = scoClass.getConstructor().newInstance(); + sco = scoClass.getConstructor(boolean.class).newInstance(delayOkHttp); } catch (ClassNotFoundException | NoSuchMethodException | InstantiationException @@ -528,10 +520,23 @@ public void execute() { } installDatadogTracer(initTelemetry, scoClass, sco); + maybeInstallLogsIntake(scoClass, sco); + } + + @Override + public AgentThread agentThread() { + return TRACE_STARTUP; + } + + @Override + public void execute() { + if (delayOkHttp) { + resumeRemoteComponents(); + } + maybeStartAppSec(scoClass, sco); maybeStartIast(instrumentation, scoClass, sco); maybeStartCiVisibility(instrumentation, scoClass, sco); - maybeStartLogsIntake(scoClass, sco); // start debugger before remote config to subscribe to it before starting to poll maybeStartDebugger(instrumentation, scoClass, sco); maybeStartRemoteConfig(scoClass, sco); @@ -540,6 +545,18 @@ public void execute() { startTelemetry(instrumentation, scoClass, sco); } } + + 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); + scoClass.getMethod("resume").invoke(sco); + } catch (InterruptedException ignore) { + } catch (Throwable e) { + log.error("Error resuming remote components", e); + } + } } protected static class StartProfilingAgentCallback extends ClassLoadCallBack { @@ -866,17 +883,18 @@ private static void maybeStartCiVisibility(Instrumentation inst, Class scoCla } } - private static void maybeStartLogsIntake(Class scoClass, Object sco) { + private static void maybeInstallLogsIntake(Class scoClass, Object sco) { if (agentlessLogSubmissionEnabled) { StaticEventLogger.begin("Logs Intake"); try { final Class logsIntakeSystemClass = AGENT_CLASSLOADER.loadClass("datadog.trace.logging.intake.LogsIntakeSystem"); - final Method logsIntakeInstallerMethod = logsIntakeSystemClass.getMethod("start", scoClass); + final Method logsIntakeInstallerMethod = + logsIntakeSystemClass.getMethod("install", scoClass); logsIntakeInstallerMethod.invoke(null, sco); } catch (final Throwable e) { - log.warn("Not starting Logs Intake subsystem", e); + log.warn("Not installing Logs Intake subsystem", e); } StaticEventLogger.end("Logs Intake"); @@ -1267,14 +1285,8 @@ private static boolean isAppUsingCustomLogManager(final EnumSet librari final String logManagerProp = System.getProperty("java.util.logging.manager"); if (logManagerProp != null) { - final boolean onSysClasspath = - ClassLoader.getSystemResource(getResourceName(logManagerProp)) != null; log.debug("Prop - logging.manager: {}", logManagerProp); - log.debug("logging.manager on system classpath: {}", onSysClasspath); - // Some applications set java.util.logging.manager but never actually initialize the logger. - // Check to see if the configured manager is on the system classpath. - // If so, it should be safe to initialize jmxfetch which will setup the log manager. - return !onSysClasspath; + return true; } return false; @@ -1305,14 +1317,8 @@ private static boolean isAppUsingCustomJMXBuilder(final EnumSet librari final String jmxBuilderProp = System.getProperty("javax.management.builder.initial"); if (jmxBuilderProp != null) { - final boolean onSysClasspath = - ClassLoader.getSystemResource(getResourceName(jmxBuilderProp)) != null; log.debug("Prop - javax.management.builder.initial: {}", jmxBuilderProp); - log.debug("javax.management.builder.initial on system classpath: {}", onSysClasspath); - // Some applications set javax.management.builder.initial but never actually initialize JMX. - // Check to see if the configured JMX builder is on the system classpath. - // If so, it should be safe to initialize jmxfetch which will setup JMX. - return !onSysClasspath; + return true; } return false; diff --git a/dd-java-agent/agent-logs-intake/src/main/java/datadog/trace/logging/intake/LogsIntakeSystem.java b/dd-java-agent/agent-logs-intake/src/main/java/datadog/trace/logging/intake/LogsIntakeSystem.java index c33f2bdcc44..bea8cb802f2 100644 --- a/dd-java-agent/agent-logs-intake/src/main/java/datadog/trace/logging/intake/LogsIntakeSystem.java +++ b/dd-java-agent/agent-logs-intake/src/main/java/datadog/trace/logging/intake/LogsIntakeSystem.java @@ -1,6 +1,5 @@ package datadog.trace.logging.intake; -import datadog.communication.BackendApi; import datadog.communication.BackendApiFactory; import datadog.communication.ddagent.SharedCommunicationObjects; import datadog.trace.api.Config; @@ -12,7 +11,7 @@ public class LogsIntakeSystem { private static final Logger LOGGER = LoggerFactory.getLogger(LogsIntakeSystem.class); - public static void start(SharedCommunicationObjects sco) { + public static void install(SharedCommunicationObjects sco) { Config config = Config.get(); if (!config.isAgentlessLogSubmissionEnabled()) { LOGGER.debug("Agentless logs intake is disabled"); @@ -20,10 +19,8 @@ public static void start(SharedCommunicationObjects sco) { } BackendApiFactory apiFactory = new BackendApiFactory(config, sco); - BackendApi backendApi = apiFactory.createBackendApi(BackendApiFactory.Intake.LOGS); - LogsDispatcher dispatcher = new LogsDispatcher(backendApi); - LogsWriterImpl writer = new LogsWriterImpl(config, dispatcher); - writer.start(); + LogsWriterImpl writer = new LogsWriterImpl(config, apiFactory); + sco.whenReady(writer::start); LogsIntake.registerWriter(writer); } diff --git a/dd-java-agent/agent-logs-intake/src/main/java/datadog/trace/logging/intake/LogsWriterImpl.java b/dd-java-agent/agent-logs-intake/src/main/java/datadog/trace/logging/intake/LogsWriterImpl.java index 3eaacdc04a7..44ef25c7c00 100644 --- a/dd-java-agent/agent-logs-intake/src/main/java/datadog/trace/logging/intake/LogsWriterImpl.java +++ b/dd-java-agent/agent-logs-intake/src/main/java/datadog/trace/logging/intake/LogsWriterImpl.java @@ -2,6 +2,9 @@ import static datadog.trace.util.AgentThreadFactory.AGENT_THREAD_GROUP; +import datadog.communication.BackendApi; +import datadog.communication.BackendApiFactory; +import datadog.communication.BackendApiFactory.Intake; import datadog.trace.api.Config; import datadog.trace.api.logging.intake.LogsWriter; import datadog.trace.util.AgentThreadFactory; @@ -23,12 +26,12 @@ public class LogsWriterImpl implements LogsWriter { private static final int ENQUEUE_LOG_TIMEOUT_MILLIS = 1_000; private final Map commonTags; - private final LogsDispatcher logsDispatcher; + private final BackendApiFactory apiFactory; private final BlockingQueue> messageQueue; private final Thread messagePollingThread; - public LogsWriterImpl(Config config, LogsDispatcher logsDispatcher) { - this.logsDispatcher = logsDispatcher; + public LogsWriterImpl(Config config, BackendApiFactory apiFactory) { + this.apiFactory = apiFactory; commonTags = new HashMap<>(); commonTags.put("ddsource", "java"); @@ -84,6 +87,9 @@ public void log(Map message) { } private void logPollingLoop() { + BackendApi backendApi = apiFactory.createBackendApi(Intake.LOGS); + LogsDispatcher logsDispatcher = new LogsDispatcher(backendApi); + while (!Thread.currentThread().isInterrupted()) { try { List> batch = new ArrayList<>(); 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 c90d9d6000e..9d7b2ad1b26 100644 --- a/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java +++ b/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java @@ -39,55 +39,38 @@ 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"); - if (ClassLoader.getSystemResource( - System.getProperty("java.util.logging.manager").replaceAll("\\.", "/") + ".class") - == null) { - assertTraceInstallationDelayed( - "tracer install must be delayed when log manager system property is present."); - customAssert( - isJmxfetchStarted(false), - false, - "jmxfetch startup must be delayed when log manager system property is present."); - if (isJFRSupported()) { - assertProfilingStartupDelayed( - "profiling startup must be delayed when log manager system property is present."); - } - // Change back to a valid LogManager. - System.setProperty("java.util.logging.manager", CUSTOM_LOG_MANAGER_CLASS_NAME); - customAssert( - LogManager.getLogManager().getClass(), - LogManagerSetter.class - .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()) { - customAssert( - isProfilingStarted(true), true, "profiling should start after loading LogManager."); - } - } else { - customAssert( - isTracerInstalled(false), - true, - "tracer should be installed in premain when custom log manager found on classpath."); + customAssert( + isTracerInstalled(false), + true, + "tracer install is not delayed when log manager system property is present."); + customAssert( + isJmxfetchStarted(false), + false, + "jmxfetch startup must be delayed when log manager system property is present."); + if (isJFRSupported()) { + assertProfilingStartupDelayed( + "profiling startup must be delayed when log manager system property is present."); + } + // Change back to a valid LogManager. + System.setProperty("java.util.logging.manager", CUSTOM_LOG_MANAGER_CLASS_NAME); + customAssert( + LogManager.getLogManager().getClass(), + LogManagerSetter.class + .getClassLoader() + .loadClass(System.getProperty("java.util.logging.manager")), + "Javaagent should not prevent setting a custom log manager"); + customAssert( + isJmxfetchStarted(true), true, "jmxfetch should start after loading LogManager."); + if (isJFRSupported()) { customAssert( - isJmxfetchStarted(false), - true, - "jmxfetch should start in premain when custom log manager found on classpath."); - if (isJFRSupported()) { - customAssert( - isProfilingStarted(false), - true, - "profiling should start in premain when custom log manager found on classpath."); - } + isProfilingStarted(true), true, "profiling should start after loading LogManager."); } } 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, @@ -104,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, @@ -147,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); @@ -170,10 +138,13 @@ private static void assertProfilingStartupDelayed(final String message) { } private static boolean isThreadStarted(final String name, final boolean wait) { + System.out.println("Checking for thread " + name + "..."); + // Wait up to 10 seconds for thread to appear for (int i = 0; i < 20; i++) { for (final Thread thread : Thread.getAllStackTraces().keySet()) { if (name.equals(thread.getName())) { + System.out.println("...thread " + name + " has started"); return true; } } @@ -186,6 +157,7 @@ private static boolean isThreadStarted(final String name, final boolean wait) { e.printStackTrace(); } } + System.out.println("...thread " + name + " has not started"); return false; } @@ -198,9 +170,12 @@ private static boolean isProfilingStarted(final boolean wait) { } private static boolean isTracerInstalled(final boolean wait) { + System.out.println("Checking for tracer..."); + // Wait up to 10 seconds for tracer to get installed for (int i = 0; i < 20; i++) { if (AgentTracer.isRegistered()) { + System.out.println("...tracer is installed"); return true; } if (!wait) { @@ -212,6 +187,7 @@ private static boolean isTracerInstalled(final boolean wait) { e.printStackTrace(); } } + System.out.println("...tracer is not installed"); return false; } diff --git a/dd-java-agent/src/test/java/jvmbootstraptest/MBeanServerBuilderSetter.java b/dd-java-agent/src/test/java/jvmbootstraptest/MBeanServerBuilderSetter.java index 77771420c9c..190e71adaab 100644 --- a/dd-java-agent/src/test/java/jvmbootstraptest/MBeanServerBuilderSetter.java +++ b/dd-java-agent/src/test/java/jvmbootstraptest/MBeanServerBuilderSetter.java @@ -26,31 +26,19 @@ public static void main(final String... args) throws Exception { } else if (System.getProperty("javax.management.builder.initial") != null) { System.out.println("javax.management.builder.initial != null"); - if (ClassLoader.getSystemResource( - System.getProperty("javax.management.builder.initial").replaceAll("\\.", "/") - + ".class") - == null) { - customAssert( - isJmxfetchStarted(false), - false, - "jmxfetch startup must be delayed when management builder system property is present."); - // Change back to a valid MBeanServerBuilder. - System.setProperty( - "javax.management.builder.initial", "jvmbootstraptest.CustomMBeanServerBuilder"); - customAssert( - isCustomMBeanRegistered(), - true, - "Javaagent should not prevent setting a custom MBeanServerBuilder"); - customAssert( - isJmxfetchStarted(true), - true, - "jmxfetch should start after loading MBeanServerBuilder."); - } else { - customAssert( - isJmxfetchStarted(false), - true, - "jmxfetch should start in premain when custom MBeanServerBuilder found on classpath."); - } + customAssert( + isJmxfetchStarted(false), + false, + "jmxfetch startup must be delayed when management builder system property is present."); + // Change back to a valid MBeanServerBuilder. + System.setProperty( + "javax.management.builder.initial", "jvmbootstraptest.CustomMBeanServerBuilder"); + customAssert( + isCustomMBeanRegistered(), + true, + "Javaagent should not prevent setting a custom MBeanServerBuilder"); + customAssert( + isJmxfetchStarted(true), true, "jmxfetch should start after loading MBeanServerBuilder."); } else { System.out.println("No custom MBeanServerBuilder"); @@ -75,10 +63,13 @@ private static void customAssert( } private static boolean isThreadStarted(final String name, final boolean wait) { + System.out.println("Checking for thread " + name + "..."); + // Wait up to 10 seconds for thread to appear for (int i = 0; i < 20; i++) { for (final Thread thread : Thread.getAllStackTraces().keySet()) { if (name.equals(thread.getName())) { + System.out.println("...thread " + name + " has started"); return true; } } @@ -91,6 +82,7 @@ private static boolean isThreadStarted(final String name, final boolean wait) { e.printStackTrace(); } } + System.out.println("...thread " + name + " has not started"); return false; } 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 =