More classloading issues with Glassfish

Glassfish’s WebappClassLoader caches when a resource or class load fails, so we can’t load as a resource first to see if it is available.

Also add additional logging.

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

@@ -7,23 +7,23 @@ import net.bytebuddy.dynamic.ClassFileLocator;
 import net.bytebuddy.utility.JavaModule;
 
 /**
- * Locate resources with the loading classloader. If the loading classloader cannot find the desired
- * resource, check up the classloader hierarchy until a resource is found or the bootstrap loader is
- * reached.
+ * Locate resources with the loading classloader. Because of a quirk with the way classes appended
+ * to the bootstrap classpath work, we first check our bootstrap proxy. If the loading classloader
+ * cannot find the desired resource, check up the classloader hierarchy until a resource is found.
  */
 public class DDLocationStrategy implements AgentBuilder.LocationStrategy {
-  public ClassFileLocator classFileLocator(ClassLoader classLoader) {
+  public ClassFileLocator classFileLocator(final ClassLoader classLoader) {
     return classFileLocator(classLoader, null);
   }
 
   @Override
   public ClassFileLocator classFileLocator(ClassLoader classLoader, final JavaModule javaModule) {
     final List<ClassFileLocator> locators = new ArrayList<>();
+    locators.add(ClassFileLocator.ForClassLoader.of(Utils.getBootstrapProxy()));
     while (classLoader != null) {
       locators.add(ClassFileLocator.ForClassLoader.WeaklyReferenced.of(classLoader));
       classLoader = classLoader.getParent();
     }
-    locators.add(ClassFileLocator.ForClassLoader.of(Utils.getBootstrapProxy()));
     return new ClassFileLocator.Compound(locators.toArray(new ClassFileLocator[0]));
   }
 }

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

@@ -31,6 +31,9 @@ public class ExceptionHandlers {
 
             @Override
             public Size apply(final MethodVisitor mv, final Implementation.Context context) {
+              final String name = context.getInstrumentedType().getName();
+              final ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
+
               // writes the following bytecode:
               // try {
               //   org.slf4j.LoggerFactory.getLogger((Class)ExceptionLogger.class)
@@ -58,7 +61,11 @@ public class ExceptionHandlers {
                   "(Ljava/lang/Class;)L" + LOGGER_NAME + ";",
                   false);
               mv.visitInsn(Opcodes.SWAP); // stack: (top) throwable,logger
-              mv.visitLdcInsn("Failed to handle exception in instrumentation");
+              mv.visitLdcInsn(
+                  "Failed to handle exception in instrumentation for "
+                      + name
+                      + " on "
+                      + classLoader);
               mv.visitInsn(Opcodes.SWAP); // stack: (top) throwable,string,logger
               mv.visitMethodInsn(
                   Opcodes.INVOKEINTERFACE,

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

@@ -98,7 +98,7 @@ public interface Instrumenter {
         agentBuilder =
             agentBuilder.transform(
                 new AgentBuilder.Transformer.ForAdvice()
-                    .include(Utils.getAgentClassLoader())
+                    .include(Utils.getBootstrapProxy(), Utils.getAgentClassLoader())
                     .withExceptionHandler(ExceptionHandlers.defaultExceptionHandler())
                     .advice(entry.getKey(), entry.getValue()));
       }

dd-java-agent/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy

@@ -1,17 +1,15 @@
 package datadog.trace.agent.integration.classloading
 
+import static datadog.trace.agent.test.IntegrationTestUtils.createJarWithClasses
+
 import datadog.test.ClassToInstrument
 import datadog.test.ClassToInstrumentChild
-import datadog.trace.agent.test.IntegrationTestUtils
 import datadog.trace.api.Trace
 import datadog.trace.util.gc.GCUtils
+import java.lang.ref.WeakReference
 import spock.lang.Specification
 import spock.lang.Timeout
 
-import java.lang.ref.WeakReference
-
-import static datadog.trace.agent.test.IntegrationTestUtils.createJarWithClasses
-
 @Timeout(10)
 class ClassLoadingTest extends Specification {
 
@@ -94,8 +92,23 @@ class ClassLoadingTest extends Specification {
     loader1.count == loader2.count
   }
 
-  def "can find bootstrap resources"() {
+  def "can find classes but not resources loaded onto the bootstrap classpath"() {
     expect:
-    IntegrationTestUtils.getAgentClassLoader().getResources('datadog/trace/api/Trace.class') != null
+    Class.forName(name) != null
+
+    // Resources from bootstrap injected jars can't be loaded.
+    // https://github.com/raphw/byte-buddy/pull/496
+    if (onTestClasspath) {
+      assert ClassLoader.getSystemClassLoader().getResource(resource) != null
+    } else {
+      assert ClassLoader.getSystemClassLoader().getResource(resource) == null
+    }
+
+
+    where:
+    name                                                            | onTestClasspath
+    "datadog.trace.api.Trace"                                       | true
+    "datadog.trace.bootstrap.instrumentation.java.concurrent.State" | false
+    resource = name.replace(".", "/") + ".class"
   }
 }