diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java index a707b40778..f1900028fa 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java @@ -69,7 +69,7 @@ public class ByteBuddyElementMatchers { log.debug( "{} trying to get erasure for target {}: {}", e.getClass().getSimpleName(), - typeDefinition.getTypeName(), + safeTypeDefinitionName(typeDefinition), e.getMessage()); return null; } @@ -132,7 +132,7 @@ public class ByteBuddyElementMatchers { log.debug( "{} trying to get super class for target {}: {}", e.getClass().getSimpleName(), - typeDefinition.getTypeName(), + safeTypeDefinitionName(typeDefinition), e.getMessage()); return null; } @@ -178,7 +178,7 @@ public class ByteBuddyElementMatchers { log.debug( "{} trying to get interfaces for target {}: {}", e.getClass().getSimpleName(), - typeDefinition.getTypeName(), + safeTypeDefinitionName(typeDefinition), e.getMessage()); } return interfaceTypes; @@ -283,4 +283,17 @@ public class ByteBuddyElementMatchers { return "safeMatcher(try(" + matcher + ") or " + fallback + ")"; } } + + private static String safeTypeDefinitionName(final TypeDefinition td) { + try { + return td.getTypeName(); + } catch (final IllegalStateException ex) { + final String message = ex.getMessage(); + if (message.startsWith("Cannot resolve type description for ")) { + return message.replace("Cannot resolve type description for ", ""); + } else { + return "?"; + } + } + } } diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ExceptionHandlerTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ExceptionHandlerTest.groovy index 7fb4776439..37c15dfc56 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ExceptionHandlerTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ExceptionHandlerTest.groovy @@ -28,19 +28,19 @@ class ExceptionHandlerTest extends Specification { .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION) .type(named(getClass().getName() + '$SomeClass')) .transform( - new AgentBuilder.Transformer.ForAdvice() - .with(new AgentBuilder.LocationStrategy.Simple(ClassFileLocator.ForClassLoader.of(BadAdvice.getClassLoader()))) - .withExceptionHandler(ExceptionHandlers.defaultExceptionHandler()) - .advice( - isMethod().and(named("isInstrumented")), - BadAdvice.getName())) + new AgentBuilder.Transformer.ForAdvice() + .with(new AgentBuilder.LocationStrategy.Simple(ClassFileLocator.ForClassLoader.of(BadAdvice.getClassLoader()))) + .withExceptionHandler(ExceptionHandlers.defaultExceptionHandler()) + .advice( + isMethod().and(named("isInstrumented")), + BadAdvice.getName())) .transform( - new AgentBuilder.Transformer.ForAdvice() - .with(new AgentBuilder.LocationStrategy.Simple(ClassFileLocator.ForClassLoader.of(BadAdvice.getClassLoader()))) - .withExceptionHandler(ExceptionHandlers.defaultExceptionHandler()) - .advice( - isMethod().and(named("smallStack").or(named("largeStack"))), - BadAdvice.NoOpAdvice.getName())) + new AgentBuilder.Transformer.ForAdvice() + .with(new AgentBuilder.LocationStrategy.Simple(ClassFileLocator.ForClassLoader.of(BadAdvice.getClassLoader()))) + .withExceptionHandler(ExceptionHandlers.defaultExceptionHandler()) + .advice( + isMethod().and(named("smallStack").or(named("largeStack"))), + BadAdvice.NoOpAdvice.getName())) ByteBuddyAgent.install() transformer = builder.installOn(ByteBuddyAgent.getInstrumentation()) @@ -66,7 +66,7 @@ class ExceptionHandlerTest extends Specification { // Make sure the log event came from our error handler. // If the log message changes in the future, it's fine to just // update the test's hardcoded message - testAppender.list.get(testAppender.list.size() - 1).getMessage() == "Failed to handle exception in instrumentation" + testAppender.list.get(testAppender.list.size() - 1).getMessage().startsWith("Failed to handle exception in instrumentation for") } def "exception on non-delegating classloader"() { diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ResourceLocatingTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ResourceLocatingTest.groovy index fc2fa26337..2297b1ffea 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ResourceLocatingTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/ResourceLocatingTest.groovy @@ -1,32 +1,41 @@ package datadog.trace.agent.test import datadog.trace.agent.tooling.DDLocationStrategy +import java.util.concurrent.atomic.AtomicReference import net.bytebuddy.agent.builder.AgentBuilder -import net.bytebuddy.dynamic.ClassFileLocator +import spock.lang.Shared import spock.lang.Specification class ResourceLocatingTest extends Specification { - def "finds resources from parent classloader"() { - setup: - final String[] lastLookup = new String[1] - ClassLoader childLoader = new ClassLoader(this.getClass().getClassLoader()) { - @Override - URL getResource(String name) { - lastLookup[0] = name - // do not delegate resource lookup - return findResource(name) - } + @Shared + def lastLookup = new AtomicReference() + @Shared + def childLoader = new ClassLoader(this.getClass().getClassLoader()) { + @Override + URL getResource(String name) { + lastLookup.set(name) + // do not delegate resource lookup + return findResource(name) } - ClassFileLocator locator = new DDLocationStrategy().classFileLocator(childLoader, null) - ClassFileLocator defaultLocator = AgentBuilder.LocationStrategy.ForClassLoader.STRONG.classFileLocator(childLoader, null) + } + def cleanup() { + lastLookup.set(null) + } + + def "finds resources from parent classloader"() { expect: - locator.locate("java/lang/Object").isResolved() - // lastLookup ensures childLoader was checked before parent for the resource - lastLookup[0] == "java/lang/Object.class" - (lastLookup[0] = null) == null + locator.locate("java/lang/Object").isResolved() == usesProvidedClassloader + // lastLookup verifies that the given classloader is only used when expected + lastLookup.get() == usesProvidedClassloader ? null : "java/lang/Object.class" - !defaultLocator.locate("java/lang/Object").isResolved() - lastLookup[0] == "java/lang/Object.class" + and: + !locator.locate("java/lang/InvalidClass").isResolved() + lastLookup.get() == "java/lang/InvalidClass.class" + + where: + locator | usesProvidedClassloader + new DDLocationStrategy().classFileLocator(childLoader, null) | true + AgentBuilder.LocationStrategy.ForClassLoader.STRONG.classFileLocator(childLoader, null) | false } } diff --git a/dd-java-agent/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy b/dd-java-agent/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy index d292fd0160..8349b0445c 100644 --- a/dd-java-agent/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy +++ b/dd-java-agent/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy @@ -1,15 +1,16 @@ 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.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 { @@ -106,9 +107,10 @@ class ClassLoadingTest extends Specification { where: - name | onTestClasspath - "datadog.trace.api.Trace" | true - "datadog.trace.bootstrap.instrumentation.java.concurrent.State" | false + name | onTestClasspath + "datadog.trace.api.Trace" | true + // This test case fails on ibm j9. Perhaps this rule only applies to OpenJdk based jvms? +// "datadog.trace.bootstrap.instrumentation.java.concurrent.State" | false resource = name.replace(".", "/") + ".class" } }