From 8cfc2a343528e87de7e03e7e0ace8f95e435e6d3 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Mon, 4 Jan 2021 15:47:32 +0000 Subject: [PATCH 1/2] Move Constants#BOOTSTRAP_PACKAGE_PREFIXES to bootstrap so we can avoid bootstrap helper-injection in ClassloadingInstrumentation --- .../datadog/trace/bootstrap/Constants.java | 27 ++++++++++ .../trace/agent/tooling/Constants.java | 54 ------------------- .../tooling/context/ContextStoreUtils.java | 6 +-- .../ClassloadingInstrumentation.java | 7 +-- .../glassfish/GlassFishInstrumentation.java | 7 +-- .../agent/test/IntegrationTestUtils.java | 10 +--- .../datadog/trace/agent/test/SpockRunner.java | 2 +- .../test/groovy/AgentTestRunnerTest.groovy | 2 +- 8 files changed, 35 insertions(+), 80 deletions(-) create mode 100644 dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Constants.java delete mode 100644 dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Constants.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Constants.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Constants.java new file mode 100644 index 00000000000..a5996a285ef --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Constants.java @@ -0,0 +1,27 @@ +package datadog.trace.bootstrap; + +/** + * Some useful constants. + * + *

Idea here is to keep this class safe to inject into client's class loader. + */ +public final class Constants { + + /** + * packages which will be loaded on the bootstrap classloader + * + *

Updates should be mirrored in + * datadog.trace.agent.test.SpockRunner#BOOTSTRAP_PACKAGE_PREFIXES_COPY + */ + public static final String[] BOOTSTRAP_PACKAGE_PREFIXES = { + "datadog.slf4j", + "datadog.trace.api", + "datadog.trace.bootstrap", + "datadog.trace.context", + "datadog.trace.instrumentation.api", + "datadog.trace.logging", + "datadog.trace.util", + }; + + private Constants() {} +} diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Constants.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Constants.java deleted file mode 100644 index 47c951c0da7..00000000000 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Constants.java +++ /dev/null @@ -1,54 +0,0 @@ -package datadog.trace.agent.tooling; - -/** - * Some useful constants. - * - *

Idea here is to keep this class safe to inject into client's class loader. - */ -public final class Constants { - - /** - * packages which will be loaded on the bootstrap classloader - * - *

Updates should be mirrored in - * datadog.trace.agent.test.SpockRunner#BOOTSTRAP_PACKAGE_PREFIXES_COPY - */ - public static final String[] BOOTSTRAP_PACKAGE_PREFIXES = { - "datadog.slf4j", - "datadog.trace.api", - "datadog.trace.bootstrap", - "datadog.trace.context", - "datadog.trace.instrumentation.api", - "datadog.trace.logging", - "datadog.trace.util", - }; - - // This is used in IntegrationTestUtils.java - public static final String[] AGENT_PACKAGE_PREFIXES = { - "datadog.trace.common", - "datadog.trace.agent", - "datadog.trace.instrumentation", - // guava - "com.google.auto", - "com.google.common", - "com.google.thirdparty.publicsuffix", - // WeakConcurrentMap - "com.blogspot.mydailyjava.weaklockfree", - // bytebuddy - "net.bytebuddy", - // msgpack - "org.msgpack", - // disruptor - "com.lmax.disruptor", - // okHttp - "okhttp3", - "okio", - "jnr", - "org.objectweb.asm", - "com.kenai", - // Custom RxJava Utility - "rx.DDTracingUtil", - }; - - private Constants() {} -} diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/ContextStoreUtils.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/ContextStoreUtils.java index 4d9984ff086..360f5a15815 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/ContextStoreUtils.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/ContextStoreUtils.java @@ -74,9 +74,9 @@ public DynamicType.Builder transform( } /** - * Note: the value here has to be inside on of the prefixes in - * datadog.trace.agent.tooling.Constants#BOOTSTRAP_PACKAGE_PREFIXES. This ensures that 'isolating' - * (or 'module') classloaders like jboss and osgi see injected classes. This works because we + * Note: the value here has to be inside one of the prefixes in + * datadog.trace.bootstrap.Constants#BOOTSTRAP_PACKAGE_PREFIXES. This ensures that 'isolating' (or + * 'module') classloaders like jboss and osgi see injected classes. This works because we * instrument those classloaders to load everything inside bootstrap packages. */ private static final String DYNAMIC_CLASSES_PACKAGE = diff --git a/dd-java-agent/instrumentation/classloading/src/main/java/datadog/trace/instrumentation/classloading/ClassloadingInstrumentation.java b/dd-java-agent/instrumentation/classloading/src/main/java/datadog/trace/instrumentation/classloading/ClassloadingInstrumentation.java index 288436f4c4b..3366ddae672 100644 --- a/dd-java-agent/instrumentation/classloading/src/main/java/datadog/trace/instrumentation/classloading/ClassloadingInstrumentation.java +++ b/dd-java-agent/instrumentation/classloading/src/main/java/datadog/trace/instrumentation/classloading/ClassloadingInstrumentation.java @@ -13,9 +13,9 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Constants; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import datadog.trace.bootstrap.Constants; import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; @@ -46,11 +46,6 @@ public ElementMatcher typeMatcher() { .and(extendsClass(named("java.lang.ClassLoader"))); } - @Override - public String[] helperClassNames() { - return new String[] {Constants.class.getName()}; - } - @Override public Map, String> transformers() { return singletonMap( diff --git a/dd-java-agent/instrumentation/glassfish/src/main/java/datadog/trace/instrumentation/glassfish/GlassFishInstrumentation.java b/dd-java-agent/instrumentation/glassfish/src/main/java/datadog/trace/instrumentation/glassfish/GlassFishInstrumentation.java index 45e5660b88c..4e166c33a99 100644 --- a/dd-java-agent/instrumentation/glassfish/src/main/java/datadog/trace/instrumentation/glassfish/GlassFishInstrumentation.java +++ b/dd-java-agent/instrumentation/glassfish/src/main/java/datadog/trace/instrumentation/glassfish/GlassFishInstrumentation.java @@ -6,8 +6,8 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Constants; import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.Constants; import java.util.Map; import lombok.extern.slf4j.Slf4j; import net.bytebuddy.asm.Advice; @@ -31,11 +31,6 @@ public GlassFishInstrumentation() { super("glassfish"); } - @Override - public String[] helperClassNames() { - return new String[] {Constants.class.getName()}; - } - @Override public ElementMatcher typeMatcher() { return named("com.sun.enterprise.v3.server.APIClassLoaderServiceImpl$APIClassLoader"); 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 41b60d0df7a..3df6edcad99 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 @@ -131,19 +131,11 @@ public static String getResourceName(final String className) { public static String[] getBootstrapPackagePrefixes() throws Exception { final Field f = getAgentClassLoader() - .loadClass("datadog.trace.agent.tooling.Constants") + .loadClass("datadog.trace.bootstrap.Constants") .getField("BOOTSTRAP_PACKAGE_PREFIXES"); return (String[]) f.get(null); } - public static String[] getAgentPackagePrefixes() throws Exception { - final Field f = - getAgentClassLoader() - .loadClass("datadog.trace.agent.tooling.Constants") - .getField("AGENT_PACKAGE_PREFIXES"); - return (String[]) f.get(null); - } - private static String getAgentArgument() { final RuntimeMXBean runtimeMxBean = ManagementFactory.getRuntimeMXBean(); for (final String arg : runtimeMxBean.getInputArguments()) { diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/SpockRunner.java b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/SpockRunner.java index f793e63dbc1..688084be2b7 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/SpockRunner.java +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/SpockRunner.java @@ -26,7 +26,7 @@ */ public class SpockRunner extends Sputnik { /** - * An exact copy of {@link datadog.trace.agent.tooling.Constants#BOOTSTRAP_PACKAGE_PREFIXES}. + * An exact copy of {@link datadog.trace.bootstrap.Constants#BOOTSTRAP_PACKAGE_PREFIXES}. * *

This list is needed to initialize the bootstrap classpath because Utils' static initializer * references bootstrap classes (e.g. DatadogClassLoader). diff --git a/dd-java-agent/testing/src/test/groovy/AgentTestRunnerTest.groovy b/dd-java-agent/testing/src/test/groovy/AgentTestRunnerTest.groovy index 1f2a2edd0a9..12c2e7b6d3c 100644 --- a/dd-java-agent/testing/src/test/groovy/AgentTestRunnerTest.groovy +++ b/dd-java-agent/testing/src/test/groovy/AgentTestRunnerTest.groovy @@ -2,8 +2,8 @@ import com.google.common.reflect.ClassPath import datadog.trace.agent.test.AgentTestRunner import datadog.trace.agent.test.SpockRunner import datadog.trace.agent.test.utils.ClasspathUtils -import datadog.trace.agent.tooling.Constants import datadog.trace.api.GlobalTracer +import datadog.trace.bootstrap.Constants import datadog.trace.bootstrap.instrumentation.api.AgentScope import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.AgentTracer From c1b2bb09cbaecb9d255ae6c170ffc5dbee0e7f5b Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Mon, 4 Jan 2021 17:53:24 +0000 Subject: [PATCH 2/2] Update wording Co-authored-by: Richard Startin --- .../src/main/java/datadog/trace/bootstrap/Constants.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Constants.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Constants.java index a5996a285ef..eee93219cc7 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Constants.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Constants.java @@ -3,7 +3,7 @@ /** * Some useful constants. * - *

Idea here is to keep this class safe to inject into client's class loader. + *

The idea here is to keep this class safe to inject into client's class loader. */ public final class Constants {