From 7f5b270c67099324736f1687b3f7b9210ca94c54 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Tue, 5 Nov 2019 12:55:31 -0500 Subject: [PATCH 1/6] Call class loading callbacks from onComplete This should make things more clear --- .../trace/agent/tooling/AgentInstaller.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java index 036f8acc6c..5533f2e622 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java @@ -270,13 +270,7 @@ public class AgentInstaller { final String typeName, final ClassLoader classLoader, final JavaModule javaModule, - final boolean b) { - for (final Map.Entry entry : classLoadCallbacks.entrySet()) { - if (entry.getKey().equals(typeName)) { - entry.getValue().run(); - } - } - } + final boolean b) {} @Override public void onTransformation( @@ -303,10 +297,16 @@ public class AgentInstaller { @Override public void onComplete( - final String s, + final String typeName, final ClassLoader classLoader, final JavaModule javaModule, - final boolean b) {} + final boolean b) { + for (final Map.Entry entry : classLoadCallbacks.entrySet()) { + if (entry.getKey().equals(typeName)) { + entry.getValue().run(); + } + } + } } private AgentInstaller() {} From acfa861e4a1b0e950fd3c2b08621a48b9e7548c9 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Tue, 5 Nov 2019 12:56:24 -0500 Subject: [PATCH 2/6] Fix some typos --- .../src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java | 2 +- .../src/main/java/datadog/trace/agent/TracingAgent.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java index ad0c948e58..680160ba71 100644 --- a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java +++ b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java @@ -108,7 +108,7 @@ public class JMXFetch { } catch (final InterruptedException e) { // It looks like JMXFetch itself eats up InterruptedException, so we will do // same here for consistency - log.error("JMXFetch was interupted, ignoring", e); + log.error("JMXFetch was interrupted, ignoring", e); } } } 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 4f375adbc0..d249bb4fd8 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 @@ -54,7 +54,7 @@ public class TracingAgent { * * JMXFetch uses jre bootstrap classes which touch this class. This means applications which require a custom log manager may not have a chance to set the global log manager if jmxfetch runs first. JMXFetch will incorrectly set the global log manager in cases where the app sets the log manager system property or when the log manager class is not on the system classpath. * - * Our solution is to delay the initilization of jmxfetch when we detect a custom log manager being used. + * Our solution is to delay the initialization of jmxfetch when we detect a custom log manager being used. * * Once we see the LogManager class loading, it's safe to start jmxfetch because the application is already setting the global log manager and jmxfetch won't be able to touch it due to classloader locking. */ From f236c0e8e0fa0116e1d15bb1fa3e034859e7e095 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Tue, 5 Nov 2019 13:00:47 -0500 Subject: [PATCH 3/6] Reformat some comments --- .../src/main/java/datadog/trace/agent/TracingAgent.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 d249bb4fd8..eff494b214 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 @@ -52,11 +52,15 @@ public class TracingAgent { /* * java.util.logging.LogManager maintains a final static LogManager, which is created during class initialization. * - * JMXFetch uses jre bootstrap classes which touch this class. This means applications which require a custom log manager may not have a chance to set the global log manager if jmxfetch runs first. JMXFetch will incorrectly set the global log manager in cases where the app sets the log manager system property or when the log manager class is not on the system classpath. + * JMXFetch uses jre bootstrap classes which touch this class. This means applications which require a custom log + * manager may not have a chance to set the global log manager if jmxfetch runs first. JMXFetch will incorrectly + * set the global log manager in cases where the app sets the log manager system property or when the log manager + * class is not on the system classpath. * * Our solution is to delay the initialization of jmxfetch when we detect a custom log manager being used. * - * Once we see the LogManager class loading, it's safe to start jmxfetch because the application is already setting the global log manager and jmxfetch won't be able to touch it due to classloader locking. + * Once we see the LogManager class loading, it's safe to start jmxfetch because the application is already setting + * the global log manager and jmxfetch won't be able to touch it due to classloader locking. */ final Class agentInstallerClass = AGENT_CLASSLOADER.loadClass("datadog.trace.agent.tooling.AgentInstaller"); From b31778a05e324e49a0d9ea39ec27df4fcfca7fca Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Tue, 5 Nov 2019 13:01:42 -0500 Subject: [PATCH 4/6] Do not use `printStackTrace()` --- .../src/main/java/datadog/trace/agent/TracingAgent.java | 1 - 1 file changed, 1 deletion(-) 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 eff494b214..4f79e78d35 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 @@ -87,7 +87,6 @@ public class TracingAgent { try { startJmxFetch(inst, bootstrapURL); } catch (final Exception e) { - e.printStackTrace(); throw new RuntimeException(e); } } From 415a875dfc615dc6ac4c7c35c34f3a4902fc1783 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Tue, 5 Nov 2019 13:02:18 -0500 Subject: [PATCH 5/6] Add generic is-before-java9 check --- .../src/main/java/datadog/trace/agent/TracingAgent.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 4f79e78d35..b2c09c67fd 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 @@ -268,8 +268,7 @@ public class TracingAgent { private static ClassLoader createDatadogClassLoader( final String innerJarFilename, final URL bootstrapURL) throws Exception { final ClassLoader agentParent; - final String javaVersion = System.getProperty("java.version"); - if (javaVersion.startsWith("1.7") || javaVersion.startsWith("1.8")) { + if (isJavaBefore9()) { agentParent = null; // bootstrap } else { // platform classloader is parent of system in java 9+ @@ -375,6 +374,10 @@ public class TracingAgent { return false; } + private static boolean isJavaBefore9() { + return System.getProperty("java.version").startsWith("1."); + } + /** * Main entry point. * From 0b4b07b3f8ea973c7abd6a7c856070035e55d59b Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 5 Nov 2019 14:35:48 -0800 Subject: [PATCH 6/6] Disable zulu8 test for time being due to test incompatibility OkHttp seems to be inadvertently loading the java log manager. --- .circleci/config.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6932ad2a45..3839851471 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -107,10 +107,10 @@ jobs: environment: - TEST_TASK: testJavaIBM8 - test_zulu8: - <<: *default_test_job - environment: - - TEST_TASK: testJavaZULU8 +# test_zulu8: +# <<: *default_test_job +# environment: +# - TEST_TASK: testJavaZULU8 test_9: <<: *default_test_job @@ -283,12 +283,12 @@ workflows: filters: tags: only: /.*/ - - test_zulu8: - requires: - - build - filters: - tags: - only: /.*/ +# - test_zulu8: +# requires: +# - build +# filters: +# tags: +# only: /.*/ - test_9: requires: - build @@ -353,7 +353,7 @@ workflows: - test_8 - test_latest8 - test_ibm8 - - test_zulu8 +# - test_zulu8 - test_9 - test_10 - test_11 @@ -374,7 +374,7 @@ workflows: - test_8 - test_latest8 - test_ibm8 - - test_zulu8 +# - test_zulu8 - test_9 - test_10 - test_11