Merge pull request #1199 from DataDog/landerson/shared-classloader

Shared classloader for agent and jmx-fetch

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java

@@ -38,10 +38,13 @@ public class Agent {
   }
 
   // fields must be managed under class lock
+  private static ClassLoader PARENT_CLASSLOADER = null;
+  private static ClassLoader BOOTSTRAP_PROXY = null;
   private static ClassLoader AGENT_CLASSLOADER = null;
   private static ClassLoader JMXFETCH_CLASSLOADER = null;
 
   public static void start(final Instrumentation inst, final URL bootstrapURL) {
+    createParentClassloader(bootstrapURL);
     startDatadogAgent(inst, bootstrapURL);
 
     final boolean appUsingCustomLogManager = isAppUsingCustomLogManager();
@@ -160,12 +163,38 @@ public class Agent {
     }
   }
 
+  private static synchronized void createParentClassloader(final URL bootstrapURL) {
+    if (PARENT_CLASSLOADER == null) {
+      try {
+        final Class<?> bootstrapProxyClass =
+            ClassLoader.getSystemClassLoader()
+                .loadClass("datadog.trace.bootstrap.DatadogClassLoader$BootstrapClassLoaderProxy");
+        final Constructor constructor = bootstrapProxyClass.getDeclaredConstructor(URL.class);
+        BOOTSTRAP_PROXY = (ClassLoader) constructor.newInstance(bootstrapURL);
+
+        final ClassLoader grandParent;
+        if (isJavaBefore9()) {
+          grandParent = null; // bootstrap
+        } else {
+          // platform classloader is parent of system in java 9+
+          grandParent = getPlatformClassLoader();
+        }
+
+        PARENT_CLASSLOADER = createDatadogClassLoader("shared.isolated", bootstrapURL, grandParent);
+      } catch (final Throwable ex) {
+        log.error("Throwable thrown creating parent classloader", ex);
+      }
+    }
+  }
+
   private static synchronized void startDatadogAgent(
       final Instrumentation inst, final URL bootstrapURL) {
     if (AGENT_CLASSLOADER == null) {
       try {
         final ClassLoader agentClassLoader =
-            createDatadogClassLoader("agent-tooling-and-instrumentation.isolated", bootstrapURL);
+            createDatadogClassLoader(
+                "agent-tooling-and-instrumentation.isolated", bootstrapURL, PARENT_CLASSLOADER);
+
         final Class<?> agentInstallerClass =
             agentClassLoader.loadClass("datadog.trace.agent.tooling.AgentInstaller");
         final Method agentInstallerMethod =
@@ -202,7 +231,7 @@ public class Agent {
       final ClassLoader contextLoader = Thread.currentThread().getContextClassLoader();
       try {
         final ClassLoader jmxFetchClassLoader =
-            createDatadogClassLoader("agent-jmxfetch.isolated", bootstrapURL);
+            createDatadogClassLoader("agent-jmxfetch.isolated", bootstrapURL, PARENT_CLASSLOADER);
         Thread.currentThread().setContextClassLoader(jmxFetchClassLoader);
         final Class<?> jmxFetchAgentClass =
             jmxFetchClassLoader.loadClass("datadog.trace.agent.jmxfetch.JMXFetch");
@@ -243,20 +272,16 @@ public class Agent {
    * @return Datadog Classloader
    */
   private static ClassLoader createDatadogClassLoader(
-      final String innerJarFilename, final URL bootstrapURL) throws Exception {
+      final String innerJarFilename, final URL bootstrapURL, final ClassLoader parent)
-    final ClassLoader agentParent;
+      throws Exception {
-    if (isJavaBefore9()) {
-      agentParent = null; // bootstrap
-    } else {
-      // platform classloader is parent of system in java 9+
-      agentParent = getPlatformClassLoader();
-    }
 
     final Class<?> loaderClass =
         ClassLoader.getSystemClassLoader().loadClass("datadog.trace.bootstrap.DatadogClassLoader");
     final Constructor constructor =
-        loaderClass.getDeclaredConstructor(URL.class, String.class, ClassLoader.class);
+        loaderClass.getDeclaredConstructor(
-    return (ClassLoader) constructor.newInstance(bootstrapURL, innerJarFilename, agentParent);
+            URL.class, String.class, ClassLoader.class, ClassLoader.class);
+    return (ClassLoader)
+        constructor.newInstance(bootstrapURL, innerJarFilename, BOOTSTRAP_PROXY, parent);
   }
 
   private static ClassLoader getPlatformClassLoader()

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/DatadogClassLoader.java

@@ -21,7 +21,7 @@ public class DatadogClassLoader extends URLClassLoader {
   // adds a jar to the bootstrap class lookup, but not to the resource lookup.
   // As a workaround, we keep a reference to the bootstrap jar
   // to use only for resource lookups.
-  private final BootstrapClassLoaderProxy bootstrapProxy;
+  private final ClassLoader bootstrapProxy;
   /**
    * Construct a new DatadogClassLoader
    *
@@ -31,14 +31,13 @@ public class DatadogClassLoader extends URLClassLoader {
    *     9+.
    */
   public DatadogClassLoader(
-      final URL bootstrapJarLocation, final String internalJarFileName, final ClassLoader parent) {
+      final URL bootstrapJarLocation,
+      final String internalJarFileName,
+      final ClassLoader bootstrapProxy,
+      final ClassLoader parent) {
     super(new URL[] {}, parent);
 
-    // some tests pass null
+    this.bootstrapProxy = bootstrapProxy;
-    bootstrapProxy =
-        bootstrapJarLocation == null
-            ? new BootstrapClassLoaderProxy(new URL[0])
-            : new BootstrapClassLoaderProxy(new URL[] {bootstrapJarLocation});
 
     try {
       // The fields of the URL are mostly dummy.  InternalJarURLHandler is the only important
@@ -78,7 +77,7 @@ public class DatadogClassLoader extends URLClassLoader {
     return findLoadedClass(className) != null;
   }
 
-  public BootstrapClassLoaderProxy getBootstrapProxy() {
+  public ClassLoader getBootstrapProxy() {
     return bootstrapProxy;
   }
 
@@ -93,8 +92,12 @@ public class DatadogClassLoader extends URLClassLoader {
       ClassLoader.registerAsParallelCapable();
     }
 
-    public BootstrapClassLoaderProxy(final URL[] urls) {
+    public BootstrapClassLoaderProxy(final URL url) {
-      super(urls, null);
+      super(new URL[] {url}, null);
+    }
+
+    public BootstrapClassLoaderProxy() {
+      super(new URL[0], null);
     }
 
     @Override

dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/DatadogClassLoaderTest.groovy

@@ -13,7 +13,10 @@ class DatadogClassLoaderTest extends Specification {
     def className1 = 'some/class/Name1'
     def className2 = 'some/class/Name2'
     final URL loc = getClass().getProtectionDomain().getCodeSource().getLocation()
-    final DatadogClassLoader ddLoader = new DatadogClassLoader(loc, null, null)
+    final DatadogClassLoader ddLoader = new DatadogClassLoader(loc,
+      null,
+      new DatadogClassLoader.BootstrapClassLoaderProxy(),
+      null)
     final Phaser threadHoldLockPhase = new Phaser(2)
     final Phaser acquireLockFromMainThreadPhase = new Phaser(2)
 

dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle

@@ -13,18 +13,12 @@ dependencies {
   compile project(':dd-trace-api')
 }
 
-configurations {
-  // exclude bootstrap dependencies from shadowJar
-  runtime.exclude module: deps.opentracing
-  runtime.exclude module: deps.slf4j
-  runtime.exclude group: 'org.slf4j'
-  runtime.exclude group: 'io.opentracing'
-}
-
 shadowJar {
+  dependencies deps.sharedInverse
   dependencies {
     exclude(project(':dd-java-agent:agent-bootstrap'))
     exclude(project(':dd-trace-api'))
+    exclude(dependency('org.slf4j::'))
   }
 }
 

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Utils.java

@@ -5,7 +5,6 @@ import static net.bytebuddy.matcher.ElementMatchers.named;
 import datadog.trace.bootstrap.DatadogClassLoader;
 import datadog.trace.bootstrap.DatadogClassLoader.BootstrapClassLoaderProxy;
 import java.lang.reflect.Method;
-import java.net.URL;
 import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDefinition;
 
@@ -15,7 +14,7 @@ public class Utils {
   private static Method findLoadedClassMethod = null;
 
   private static final BootstrapClassLoaderProxy unitTestBootstrapProxy =
-      new BootstrapClassLoaderProxy(new URL[0]);
+      new BootstrapClassLoaderProxy();
 
   static {
     try {
@@ -31,7 +30,7 @@ public class Utils {
   }
 
   /** Return a classloader which can be used to look up bootstrap resources. */
-  public static BootstrapClassLoaderProxy getBootstrapProxy() {
+  public static ClassLoader getBootstrapProxy() {
     if (getAgentClassLoader() instanceof DatadogClassLoader) {
       return ((DatadogClassLoader) getAgentClassLoader()).getBootstrapProxy();
     } else {
@@ -86,10 +85,10 @@ public class Utils {
 
   /** @return The current stack trace with multiple entries on new lines. */
   public static String getStackTraceAsString() {
-    StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
+    final StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
-    StringBuilder stringBuilder = new StringBuilder();
+    final StringBuilder stringBuilder = new StringBuilder();
-    String lineSeparator = System.getProperty("line.separator");
+    final String lineSeparator = System.getProperty("line.separator");
-    for (StackTraceElement element : stackTrace) {
+    for (final StackTraceElement element : stackTrace) {
       stringBuilder.append(element.toString());
       stringBuilder.append(lineSeparator);
     }

dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ClassLoaderMatcherTest.groovy

@@ -16,7 +16,7 @@ class ClassLoaderMatcherTest extends DDSpecification {
   def "skips agent classloader"() {
     setup:
     URL root = new URL("file://")
-    final URLClassLoader agentLoader = new DatadogClassLoader(root, null, null)
+    final URLClassLoader agentLoader = new DatadogClassLoader(root, null, new DatadogClassLoader.BootstrapClassLoaderProxy(), null)
     expect:
     ClassLoaderMatcher.skipClassLoader().matches(agentLoader)
   }

dd-java-agent/dd-java-agent.gradle

@@ -1,3 +1,5 @@
+import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar
+
 plugins {
   id "com.github.johnrengelman.shadow" version "5.2.0"
 }
@@ -9,27 +11,18 @@ apply from: "${rootDir}/gradle/publish.gradle"
 
 configurations {
   shadowInclude
+  sharedShadowInclude
 }
+
 /*
- * Include subproject's shadowJar in the dd-java-agent jar.
+ * 4 shadow jars are created
- * Note jarname must not end with '.jar', or its classes will be on the classpath of
+ * - The main "dd-java-agent" jar that also has the bootstrap project
- * the dd-java-agent jar.
+ * - 2 jars based on projects (jmxfetch, agent tooling)
+ * - 1 based on the shared dependencies
+ * This general config is shared by all of them
  */
 
-def includeShadowJar(subproject, jarname) {
+ext.generalShadowJarConfig = {
-  def agent_project = project
-  subproject.afterEvaluate {
-    agent_project.processResources {
-      from(zipTree(subproject.tasks.shadowJar.archiveFile)) {
-        into jarname
-        rename '(^.*)\\.class$', '$1.classdata'
-        // Rename LICENSE file since it clashes with license dir on non-case sensitive FSs (i.e. Mac)
-        rename '^LICENSE$', 'LICENSE.renamed'
-      }
-    }
-
-    agent_project.processResources.dependsOn subproject.tasks.shadowJar
-    subproject.shadowJar {
   mergeServiceFiles()
 
   exclude '**/module-info.class'
@@ -49,14 +42,37 @@ def includeShadowJar(subproject, jarname) {
     relocate 'datadog.opentracing', 'datadog.trace.agent.ot'
   }
 }
+
+def includeShadowJar(shadowJarTask, jarname) {
+  project.processResources {
+    from(zipTree(shadowJarTask.archiveFile)) {
+      into jarname + '.isolated'
+      rename '(^.*)\\.class$', '$1.classdata'
+      // Rename LICENSE file since it clashes with license dir on non-case sensitive FSs (i.e. Mac)
+      rename '^LICENSE$', 'LICENSE.renamed'
     }
   }
 
-includeShadowJar(project(':dd-java-agent:instrumentation'), 'agent-tooling-and-instrumentation.isolated')
+  project.processResources.dependsOn shadowJarTask
-includeShadowJar(project(':dd-java-agent:agent-jmxfetch'), 'agent-jmxfetch.isolated')
+  shadowJarTask.configure generalShadowJarConfig
+}
 
-jar {
+project(':dd-java-agent:instrumentation').afterEvaluate {
-  archiveClassifier = 'unbundled'
+  includeShadowJar(it.tasks.shadowJar, 'agent-tooling-and-instrumentation')
+}
+project(':dd-java-agent:agent-jmxfetch').afterEvaluate {
+  includeShadowJar(it.tasks.shadowJar, 'agent-jmxfetch')
+}
+
+task sharedShadowJar(type: ShadowJar) {
+  configurations = [project.configurations.sharedShadowInclude]
+}
+includeShadowJar(sharedShadowJar, 'shared')
+
+shadowJar generalShadowJarConfig >> {
+  configurations = [project.configurations.shadowInclude]
+
+  archiveClassifier = ''
 
   manifest {
     attributes(
@@ -69,31 +85,6 @@ jar {
   }
 }
 
-shadowJar {
-  configurations = [project.configurations.shadowInclude]
-
-  archiveClassifier = ''
-
-  mergeServiceFiles()
-
-  exclude '**/module-info.class'
-
-  dependencies {
-    exclude(dependency("org.projectlombok:lombok:$versions.lombok"))
-  }
-
-  // Prevents conflict with other SLF4J instances. Important for premain.
-  relocate 'org.slf4j', 'datadog.slf4j'
-  // rewrite dependencies calling Logger.getLogger
-  relocate 'java.util.logging.Logger', 'datadog.trace.bootstrap.PatchLogger'
-
-  if (!project.hasProperty("disableShadowRelocate") || !disableShadowRelocate) {
-    // shadow OT impl to prevent casts to implementation
-    relocate 'datadog.trace.common', 'datadog.trace.agent.common'
-    relocate 'datadog.opentracing', 'datadog.trace.agent.ot'
-  }
-}
-
 // We don't want bundled dependencies to show up in the pom.
 modifyPom {
   dependencies.removeAll { true }
@@ -109,7 +100,11 @@ dependencies {
   testCompile deps.testLogging
   testCompile deps.guava
 
+  // Includes for the top level shadow jar
   shadowInclude project(path: ':dd-java-agent:agent-bootstrap')
+
+  // Includes for the shared internal shadow jar
+  sharedShadowInclude deps.shared
 }
 
 tasks.withType(Test).configureEach {

dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy

@@ -121,7 +121,7 @@ class UrlConnectionTest extends AgentTestRunner {
 
   def "DatadogClassloader ClassNotFoundException doesn't create span"() {
     given:
-    ClassLoader datadogLoader = new DatadogClassLoader(null, null, null)
+    ClassLoader datadogLoader = new DatadogClassLoader(null, null, new DatadogClassLoader.BootstrapClassLoaderProxy(), null)
     ClassLoader childLoader = new URLClassLoader(new URL[0], datadogLoader)
 
     when:

dd-java-agent/instrumentation/instrumentation.gradle

@@ -76,16 +76,12 @@ dependencies {
   }
 }
 
-configurations {
-  // exclude bootstrap dependencies from shadowJar
-  runtime.exclude module: deps.slf4j
-  runtime.exclude group: 'org.slf4j'
-}
-
 shadowJar {
+  dependencies deps.sharedInverse
   dependencies {
     exclude(project(':dd-java-agent:agent-bootstrap'))
     exclude(project(':dd-trace-api'))
+    exclude(dependency('org.slf4j::'))
   }
 }
 

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/SpockRunner.java

@@ -2,6 +2,7 @@ package datadog.trace.agent.test;
 
 import com.google.common.reflect.ClassPath;
 import datadog.trace.agent.test.utils.ClasspathUtils;
+import datadog.trace.bootstrap.DatadogClassLoader.BootstrapClassLoaderProxy;
 import java.io.File;
 import java.io.IOException;
 import java.lang.reflect.Field;
@@ -142,7 +143,8 @@ public class SpockRunner extends Sputnik {
           .appendToBootstrapClassLoaderSearch(new JarFile(bootstrapJar));
       // Utils cannot be referenced before this line, as its static initializers load bootstrap
       // classes (for example, the bootstrap proxy).
-      datadog.trace.agent.tooling.Utils.getBootstrapProxy().addURL(bootstrapJar.toURI().toURL());
+      ((BootstrapClassLoaderProxy) datadog.trace.agent.tooling.Utils.getBootstrapProxy())
+          .addURL(bootstrapJar.toURI().toURL());
     } catch (final IOException e) {
       throw new RuntimeException(e);
     }

gradle/dependencies.gradle

@@ -62,5 +62,31 @@ ext {
     scala          : dependencies.create(group: 'org.scala-lang', name: 'scala-library', version: "${versions.scala}"),
     kotlin         : dependencies.create(group: 'org.jetbrains.kotlin', name: 'kotlin-stdlib', version: "${versions.kotlin}"),
     coroutines     : dependencies.create(group: 'org.jetbrains.kotlinx', name: 'kotlinx-coroutines-core', version: "${versions.coroutines}"),
+
+    // Shared between agent tooling and instrumentation and JMXFetch
+    shared         : [
+      dependencies.create(group: 'com.datadoghq', name: 'java-dogstatsd-client', version: '2.8'),
+      // "Explicit override jnr version because .22 caused issues" - mar-kolya
+      dependencies.create(group: 'com.github.jnr', name: 'jnr-unixsocket', version: '0.23'),
+      dependencies.create(group: 'com.google.guava', name: 'guava', version: "${versions.guava}")
+    ],
+
+    // Inverse of "shared".  These exclude directives are part of shadowJar's DSL
+    // which is similar but not exactly the same as the regular gradle dependency{} block
+    // Also, transitive dependencies have to be explicitly listed
+    sharedInverse  : (Closure) {
+      // dogstatsd and its transitives
+      exclude(dependency('com.datadoghq:java-dogstatsd-client'))
+      exclude(dependency('com.github.jnr::'))
+      exclude(dependency('org.ow2.asm::'))
+
+      // Guava and its transitives
+      exclude(dependency('com.google.guava::'))
+      exclude(dependency('com.google.code.findbugs::'))
+      exclude(dependency('com.google.errorprone::'))
+      exclude(dependency('com.google.j2objc::'))
+      exclude(dependency('org.codehaus.mojo::'))
+      exclude(dependency('org.checkerframework::'))
+    }
   ]
 }