From 5265a43c6d140a17dbef2234251fa7ef916a673f Mon Sep 17 00:00:00 2001 From: Lev Priima Date: Thu, 6 Feb 2020 22:18:23 -0500 Subject: [PATCH] Reduce use of Thread::setContextClassLoader --- CONTRIBUTING.md | 4 ++-- .../java/datadog/trace/bootstrap/Agent.java | 8 -------- .../trace/agent/tooling/AgentInstaller.java | 3 ++- .../trace/agent/test/AgentTestRunner.java | 17 ++++++----------- 4 files changed, 10 insertions(+), 22 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ffc85ece5f0..cec83b9b54f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,8 +20,8 @@ Other source files (Groovy, Scala, etc) should ideally be formatted by Intellij Suggested plugins and settings: * Editor > Code Style > Java/Groovy > Imports - * Class count to use import with '*': `50` (some number sufficiently large that is unlikely to matter) - * Names count to use static import with '*': `50` + * Class count to use import with '*': `9999` (some number sufficiently large that is unlikely to matter) + * Names count to use static import with '*': `9999` * With java use the following import layout (groovy should still use the default) to ensure consistency with google-java-format: ![import layout](https://user-images.githubusercontent.com/734411/43430811-28442636-94ae-11e8-86f1-f270ddcba023.png) * [Google Java Format](https://plugins.jetbrains.com/plugin/8527-google-java-format) 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 4cfd50a828d..512fcd0004d 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 @@ -163,11 +163,9 @@ public void execute() { private static synchronized void startDatadogAgent( final Instrumentation inst, final URL bootstrapURL) { if (AGENT_CLASSLOADER == null) { - final ClassLoader contextLoader = Thread.currentThread().getContextClassLoader(); try { final ClassLoader agentClassLoader = createDatadogClassLoader("agent-tooling-and-instrumentation.isolated", bootstrapURL); - Thread.currentThread().setContextClassLoader(agentClassLoader); final Class agentInstallerClass = agentClassLoader.loadClass("datadog.trace.agent.tooling.AgentInstaller"); final Method agentInstallerMethod = @@ -176,8 +174,6 @@ private static synchronized void startDatadogAgent( AGENT_CLASSLOADER = agentClassLoader; } catch (final Throwable ex) { log.error("Throwable thrown while installing the Datadog Agent", ex); - } finally { - Thread.currentThread().setContextClassLoader(contextLoader); } } } @@ -186,11 +182,9 @@ private static synchronized void installDatadogTracer() { if (AGENT_CLASSLOADER == null) { throw new IllegalStateException("Datadog agent should have been started already"); } - final ClassLoader contextLoader = Thread.currentThread().getContextClassLoader(); // TracerInstaller.installGlobalTracer can be called multiple times without any problem // so there is no need to have a 'datadogTracerInstalled' flag here. try { - Thread.currentThread().setContextClassLoader(AGENT_CLASSLOADER); // install global tracer final Class tracerInstallerClass = AGENT_CLASSLOADER.loadClass("datadog.trace.agent.tooling.TracerInstaller"); @@ -200,8 +194,6 @@ private static synchronized void installDatadogTracer() { logVersionInfoMethod.invoke(null); } catch (final Throwable ex) { log.error("Throwable thrown while installing the Datadog Tracer", ex); - } finally { - Thread.currentThread().setContextClassLoader(contextLoader); } } 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 8af04df97e5..0f466e1a498 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 @@ -143,7 +143,8 @@ public static ResettableClassFileTransformer installBytebuddyAgent( agentBuilder = agentBuilder.with(listener); } int numInstrumenters = 0; - for (final Instrumenter instrumenter : ServiceLoader.load(Instrumenter.class)) { + for (final Instrumenter instrumenter : + ServiceLoader.load(Instrumenter.class, AgentInstaller.class.getClassLoader())) { log.debug("Loading instrumentation {}", instrumenter.getClass().getName()); try { diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.java b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.java index 6c93f4cc669..82cc227e0b9 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.java +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.java @@ -139,20 +139,15 @@ protected boolean shouldTransformClass(final String className, final ClassLoader } @BeforeClass - public static synchronized void agentSetup() throws Exception { + public static synchronized void agentSetup() { if (null != activeTransformer) { throw new IllegalStateException("transformer already in place: " + activeTransformer); } - - final ClassLoader contextLoader = Thread.currentThread().getContextClassLoader(); - try { - Thread.currentThread().setContextClassLoader(AgentTestRunner.class.getClassLoader()); - assert ServiceLoader.load(Instrumenter.class).iterator().hasNext() - : "No instrumentation found"; - activeTransformer = AgentInstaller.installBytebuddyAgent(INSTRUMENTATION, TEST_LISTENER); - } finally { - Thread.currentThread().setContextClassLoader(contextLoader); - } + assert ServiceLoader.load(Instrumenter.class, AgentTestRunner.class.getClassLoader()) + .iterator() + .hasNext() + : "No instrumentation found"; + activeTransformer = AgentInstaller.installBytebuddyAgent(INSTRUMENTATION, TEST_LISTENER); INSTRUMENTATION_ERROR_COUNT.set(0); }