From 92ca151af024891fb558c5890947c2b3c547a0aa Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Sat, 28 Dec 2024 02:15:12 +0000 Subject: [PATCH 1/8] Additional logging to help triage custom log-manager / jmx-builder test failures. --- .../src/test/java/jvmbootstraptest/LogManagerSetter.java | 8 ++++++++ .../java/jvmbootstraptest/MBeanServerBuilderSetter.java | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java b/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java index c90d9d6000e..a5a3d6ca4b9 100644 --- a/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java +++ b/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java @@ -170,10 +170,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 +189,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 +202,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 +219,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..1780a63c181 100644 --- a/dd-java-agent/src/test/java/jvmbootstraptest/MBeanServerBuilderSetter.java +++ b/dd-java-agent/src/test/java/jvmbootstraptest/MBeanServerBuilderSetter.java @@ -75,10 +75,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 +94,7 @@ private static boolean isThreadStarted(final String name, final boolean wait) { e.printStackTrace(); } } + System.out.println("...thread " + name + " has not started"); return false; } From 40c75838c85370afded89fe3ec58bb75680ea126 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Sun, 22 Dec 2024 15:32:52 +0000 Subject: [PATCH 2/8] Move SharedCommunicationObjects creation to InstallDatadogTracerCallback constructor --- .../java/datadog/trace/bootstrap/Agent.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 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 ec5ab19ca09..e673e0420a7 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 @@ -499,22 +499,13 @@ 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; public InstallDatadogTracerCallback( InitializationTelemetry initTelemetry, Instrumentation instrumentation) { this.initTelemetry = initTelemetry; 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"); @@ -526,7 +517,15 @@ public void execute() { | InvocationTargetException e) { throw new UndeclaredThrowableException(e); } + } + + @Override + public AgentThread agentThread() { + return TRACE_STARTUP; + } + @Override + public void execute() { installDatadogTracer(initTelemetry, scoClass, sco); maybeStartAppSec(scoClass, sco); maybeStartIast(instrumentation, scoClass, sco); From 84d2b6dcb2086083feb6e55558360e3f133caf20 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Sat, 28 Dec 2024 22:13:36 +0000 Subject: [PATCH 3/8] Add ability to pause remote components until SharedCommunicationObjects is ready --- .../ddagent/SharedCommunicationObjects.java | 52 +++++++++++++++++-- 1 file changed, 48 insertions(+), 4 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..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; From 9b97d7af2f827142e0d6c57952560f90659f4ff3 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Sat, 28 Dec 2024 22:25:30 +0000 Subject: [PATCH 4/8] Delay starting trace-writer and data-stream-monitoring until remote I/O is allowed --- .../src/main/java/datadog/trace/core/CoreTracer.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 = From 2cdc507d75b16a581c08788d393b44e96b0fd3f2 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Sat, 28 Dec 2024 22:37:48 +0000 Subject: [PATCH 5/8] Delay starting logs-intake backend until remote I/O is allowed --- .../src/main/java/datadog/trace/bootstrap/Agent.java | 9 +++++---- .../trace/logging/intake/LogsIntakeSystem.java | 9 +++------ .../datadog/trace/logging/intake/LogsWriterImpl.java | 12 +++++++++--- 3 files changed, 17 insertions(+), 13 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 e673e0420a7..4c0461ce769 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 @@ -530,7 +530,7 @@ public void execute() { maybeStartAppSec(scoClass, sco); maybeStartIast(instrumentation, scoClass, sco); maybeStartCiVisibility(instrumentation, scoClass, sco); - maybeStartLogsIntake(scoClass, sco); + maybeInstallLogsIntake(scoClass, sco); // start debugger before remote config to subscribe to it before starting to poll maybeStartDebugger(instrumentation, scoClass, sco); maybeStartRemoteConfig(scoClass, sco); @@ -865,17 +865,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"); 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<>(); From 9a09d98dcd54e4bbba88175478af704e6111e1ed Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Sat, 28 Dec 2024 22:57:03 +0000 Subject: [PATCH 6/8] Install tracer and logs-intake as soon as necessary, but defer any remote components when use of OkHttp should be delayed --- .../java/datadog/trace/bootstrap/Agent.java | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 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 4c0461ce769..0170819cf78 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 @@ -348,7 +348,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,19 +497,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 @@ -517,6 +519,9 @@ public InstallDatadogTracerCallback( | InvocationTargetException e) { throw new UndeclaredThrowableException(e); } + + installDatadogTracer(initTelemetry, scoClass, sco); + maybeInstallLogsIntake(scoClass, sco); } @Override @@ -526,11 +531,13 @@ 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); - maybeInstallLogsIntake(scoClass, sco); // start debugger before remote config to subscribe to it before starting to poll maybeStartDebugger(instrumentation, scoClass, sco); maybeStartRemoteConfig(scoClass, sco); @@ -539,6 +546,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 { From 9ee929b38087ed2bb1e898fabb48f2efbe42e3d5 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Sat, 28 Dec 2024 23:22:33 +0000 Subject: [PATCH 7/8] Remove exemption where we didn't defer if the custom logging manager or JMX builder was on the system classpath (because the main thread would find it there if OkHttp triggered initialization of JUL.). We now make OkHttp calls from our own background threads, which are isolated from the system classloader, not the main thread - so this exemption no longer makes sense. --- .../java/datadog/trace/bootstrap/Agent.java | 17 +--- .../trace/agent/CustomLogManagerTest.groovy | 2 +- .../agent/CustomMBeanServerBuilderTest.groovy | 2 +- .../jvmbootstraptest/LogManagerSetter.java | 88 ++++++------------- .../MBeanServerBuilderSetter.java | 38 +++----- 5 files changed, 45 insertions(+), 102 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 0170819cf78..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; @@ -1286,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; @@ -1324,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/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 a5a3d6ca4b9..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); diff --git a/dd-java-agent/src/test/java/jvmbootstraptest/MBeanServerBuilderSetter.java b/dd-java-agent/src/test/java/jvmbootstraptest/MBeanServerBuilderSetter.java index 1780a63c181..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"); From c9cf18f1498cbb46fd0f75961ef70d9d85fdfdb0 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Sat, 28 Dec 2024 23:53:56 +0000 Subject: [PATCH 8/8] Adjust minimumBranchCoverage to account for new double-checked lock in SharedCommunicationObjects --- communication/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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',