Skip to content

Commit

Permalink
Remove exemption where we didn't defer if the custom logging manager
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mcculls committed Dec 27, 2024
1 parent 46b5986 commit 71886d7
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 83 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -1267,14 +1266,8 @@ private static boolean isAppUsingCustomLogManager(final EnumSet<Library> 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;
Expand Down Expand Up @@ -1305,14 +1298,8 @@ private static boolean isAppUsingCustomJMXBuilder(final EnumSet<Library> 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;
Expand Down
67 changes: 24 additions & 43 deletions dd-java-agent/src/test/java/jvmbootstraptest/LogManagerSetter.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,50 +39,31 @@ 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.");
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(
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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down

0 comments on commit 71886d7

Please sign in to comment.