diff --git a/dd-java-agent/src/main/java/datadog/trace/agent/TracingAgent.java b/dd-java-agent/src/main/java/datadog/trace/agent/TracingAgent.java index f137712ce4..ecc51e8b0d 100644 --- a/dd-java-agent/src/main/java/datadog/trace/agent/TracingAgent.java +++ b/dd-java-agent/src/main/java/datadog/trace/agent/TracingAgent.java @@ -276,7 +276,7 @@ public class TracingAgent { final String logManagerProp = System.getProperty("java.util.logging.manager"); if (logManagerProp != null) { - final boolean onSysClasspath = ClassLoader.getSystemResource(logManagerProp) == null; + final boolean onSysClasspath = ClassLoader.getSystemResource(logManagerProp) != null; if (debugEnabled) { System.out.println("Prop - logging.manager: " + logManagerProp); System.out.println("logging.manager on system classpath: " + onSysClasspath); 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 226f8a231e..c40c5655d1 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 @@ -30,14 +30,25 @@ class CustomLogManagerTest extends Specification { IntegrationTestUtils.runOnSeparateJvm(LogManagerSetter.getName() , [agentArg, "-Ddd.jmxfetch.enabled=true", "-Ddd.jmxfetch.refresh-beans-period=1", "-Ddatadog.slf4j.simpleLogger.defaultLogLevel=off"] as String[] , "" as String[] + , [:] + , true) == 0 + } + + def "jmxfetch starts up in premain if configured log manager on system classpath"() { + expect: + IntegrationTestUtils.runOnSeparateJvm(LogManagerSetter.getName() + , [agentArg, "-Ddd.jmxfetch.enabled=true", "-Ddd.jmxfetch.refresh-beans-period=1", "-Ddatadog.slf4j.simpleLogger.defaultLogLevel=off", "-Djava.util.logging.manager=jvmbootstraptest.CustomLogManager"] as String[] + , "" as String[] + , [:] , true) == 0 } def "jmxfetch startup is delayed with java.util.logging.manager sysprop"() { expect: IntegrationTestUtils.runOnSeparateJvm(LogManagerSetter.getName() - , [agentArg, "-Ddd.jmxfetch.enabled=true", "-Ddd.jmxfetch.refresh-beans-period=1", "-Ddatadog.slf4j.simpleLogger.defaultLogLevel=off", "-Djava.util.logging.manager=jvmbootstraptest.CustomLogManager"] as String[] + , [agentArg, "-Ddd.jmxfetch.enabled=true", "-Ddd.jmxfetch.refresh-beans-period=1", "-Ddatadog.slf4j.simpleLogger.defaultLogLevel=off", "-Djava.util.logging.manager=jvmbootstraptest.MissingLogManager"] as String[] , "" as String[] + , [:] , true) == 0 } @@ -46,6 +57,16 @@ class CustomLogManagerTest extends Specification { IntegrationTestUtils.runOnSeparateJvm(LogManagerSetter.getName() , [agentArg, "-Ddd.jmxfetch.enabled=true", "-Ddd.jmxfetch.refresh-beans-period=1", "-Ddatadog.slf4j.simpleLogger.defaultLogLevel=off", "-Ddd.app.customlogmanager=true"] as String[] , "" as String[] + , [:] + , true) == 0 + } + + def "jmxfetch startup delayed with JBOSS_HOME environment variable"() { + expect: + IntegrationTestUtils.runOnSeparateJvm(LogManagerSetter.getName() + , [agentArg, "-Ddd.jmxfetch.enabled=true", "-Ddd.jmxfetch.refresh-beans-period=1", "-Ddatadog.slf4j.simpleLogger.defaultLogLevel=off", "-Ddd.app.customlogmanager=true"] as String[] + , "" as String[] + , ["JBOSS_HOME": "/"] , true) == 0 } } diff --git a/dd-java-agent/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java b/dd-java-agent/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java index 7f9f856e36..af5c22b872 100644 --- a/dd-java-agent/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java +++ b/dd-java-agent/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java @@ -16,6 +16,7 @@ import java.net.URL; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.UUID; import java.util.concurrent.Callable; import java.util.jar.Attributes; @@ -198,12 +199,13 @@ public class IntegrationTestUtils { final String mainClassName, final String[] jvmArgs, final String[] mainMethodArgs, + final Map envVars, final boolean printOutputStreams) throws Exception { final String separator = System.getProperty("file.separator"); final String classpath = System.getProperty("java.class.path"); final String path = System.getProperty("java.home") + separator + "bin" + separator + "java"; - final List commands = new ArrayList(); + final List commands = new ArrayList<>(); commands.add(path); commands.addAll(Arrays.asList(jvmArgs)); commands.add("-cp"); @@ -211,6 +213,7 @@ public class IntegrationTestUtils { commands.add(mainClassName); commands.addAll(Arrays.asList(mainMethodArgs)); final ProcessBuilder processBuilder = new ProcessBuilder(commands.toArray(new String[0])); + processBuilder.environment().putAll(envVars); final Process process = processBuilder.start(); final int result = process.waitFor(); diff --git a/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java b/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java index f1d95a7b08..94aad7be17 100644 --- a/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java +++ b/dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java @@ -3,20 +3,38 @@ package jvmbootstraptest; import java.util.logging.LogManager; public class LogManagerSetter { - public static void main(String... args) throws Exception { + public static void main(final String... args) throws Exception { if (System.getProperty("java.util.logging.manager") != null) { - customAssert( - isJmxfetchStarted(), - false, - "jmxfetch startup must be delayed when log manager system property is present."); + if (ClassLoader.getSystemResource(System.getProperty("java.util.logging.manager")) == null) { + customAssert( + isJmxfetchStarted(), + false, + "jmxfetch startup must be delayed when log manager system property is present."); + // Change back to a valid LogManager. + System.setProperty("java.util.logging.manager", CustomLogManager.class.getName()); + 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, "jmxfetch should start after loading LogManager."); + } else { + customAssert( + isJmxfetchStarted(), + true, + "jmxfetch should start in premain when custom log manager found on classpath."); + } + } else if (System.getProperty("dd.app.customlogmanager") != null) { + System.setProperty("java.util.logging.manager", CustomLogManager.class.getName()); 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, "jmxfetch should start after loading LogManager."); - } else if (System.getProperty("dd.app.customlogmanager") != null) { + + } else if (System.getenv("JBOSS_HOME") != null) { System.setProperty("java.util.logging.manager", CustomLogManager.class.getName()); customAssert( LogManager.getLogManager().getClass(), @@ -33,7 +51,8 @@ public class LogManagerSetter { } } - private static void customAssert(Object got, Object expected, String assertionMessage) { + private static void customAssert( + final Object got, final Object expected, final String assertionMessage) { if ((null == got && got != expected) || !got.equals(expected)) { throw new RuntimeException( "Assertion failed. Expected <" + expected + "> got <" + got + "> " + assertionMessage); @@ -41,7 +60,7 @@ public class LogManagerSetter { } private static boolean isJmxfetchStarted() { - for (Thread thread : Thread.getAllStackTraces().keySet()) { + for (final Thread thread : Thread.getAllStackTraces().keySet()) { if ("dd-jmx-collector".equals(thread.getName())) { return true; }