From aa031cc7084198e0d237ae1555db701fe4855ad0 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 28 Feb 2019 10:02:24 -0500 Subject: [PATCH] Add instrumentation for Tomcat webapp classloading --- .../TomcatClassloadingInstrumentation.java | 68 +++++++++++++++++++ .../test/groovy/TomcatClassloadingTest.groovy | 30 ++++++++ .../tomcat-classloading.gradle | 34 ++++++++++ settings.gradle | 1 + 4 files changed, 133 insertions(+) create mode 100644 dd-java-agent/instrumentation/tomcat-classloading/src/main/java/datadog/trace/instrumentation/tomcat/TomcatClassloadingInstrumentation.java create mode 100644 dd-java-agent/instrumentation/tomcat-classloading/src/test/groovy/TomcatClassloadingTest.groovy create mode 100644 dd-java-agent/instrumentation/tomcat-classloading/tomcat-classloading.gradle diff --git a/dd-java-agent/instrumentation/tomcat-classloading/src/main/java/datadog/trace/instrumentation/tomcat/TomcatClassloadingInstrumentation.java b/dd-java-agent/instrumentation/tomcat-classloading/src/main/java/datadog/trace/instrumentation/tomcat/TomcatClassloadingInstrumentation.java new file mode 100644 index 0000000000..b61c8a6982 --- /dev/null +++ b/dd-java-agent/instrumentation/tomcat-classloading/src/main/java/datadog/trace/instrumentation/tomcat/TomcatClassloadingInstrumentation.java @@ -0,0 +1,68 @@ +package datadog.trace.instrumentation.tomcat; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +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.api.GlobalTracer; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +/** + * Instrument Tomcat's web app classloader so it loads Datadog's bootstrap classes from parent + * classloader. Without this change web apps get their oen versions of Datadog's classes leading to + * there being multiple {@link GlobalTracer}s in existance, some of them not configured properly. + * This is really the same idea we have for OSGi and JBoss. + */ +@AutoService(Instrumenter.class) +public final class TomcatClassloadingInstrumentation extends Instrumenter.Default { + public TomcatClassloadingInstrumentation() { + super("tomcat-classloading"); + } + + @Override + public ElementMatcher typeMatcher() { + return safeHasSuperType(named("org.apache.catalina.loader.WebappClassLoaderBase")); + } + + @Override + public String[] helperClassNames() { + return new String[] {Constants.class.getName()}; + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod() + .and(named("filter")) + .and(takesArgument(0, String.class)) + // Older versions have 1 argument method, newer versions have two arguments + .and(takesArguments(2).or(takesArguments(1))), + WebappClassLoaderAdvice.class.getName()); + } + + public static class WebappClassLoaderAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void methodExit( + @Advice.Argument(0) final String name, @Advice.Return(readOnly = false) boolean result) { + if (result) { + return; + } + for (final String prefix : Constants.BOOTSTRAP_PACKAGE_PREFIXES) { + if (name.startsWith(prefix)) { + result = true; + break; + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/tomcat-classloading/src/test/groovy/TomcatClassloadingTest.groovy b/dd-java-agent/instrumentation/tomcat-classloading/src/test/groovy/TomcatClassloadingTest.groovy new file mode 100644 index 0000000000..8ddf66f9c8 --- /dev/null +++ b/dd-java-agent/instrumentation/tomcat-classloading/src/test/groovy/TomcatClassloadingTest.groovy @@ -0,0 +1,30 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.GlobalTracer +import org.apache.catalina.WebResource +import org.apache.catalina.WebResourceRoot +import org.apache.catalina.loader.ParallelWebappClassLoader + +class TomcatClassloadingTest extends AgentTestRunner { + + WebResourceRoot resources = Mock(WebResourceRoot) { + getResource(_) >> Mock(WebResource) + listResources(_) >> [] + // Looks like 9.x.x needs this one: + getResources(_) >> [] + } + ParallelWebappClassLoader classloader = new ParallelWebappClassLoader(null) + + def "tomcat class loading delegates to parent for Datadog classes"() { + setup: + classloader.setResources(resources) + classloader.init() + classloader.start() + + when: + // If instrumentation didn't work this would blow up with NPE due to incomplete resources mocking + def clazz = classloader.loadClass("datadog.trace.api.GlobalTracer") + + then: + clazz == GlobalTracer + } +} diff --git a/dd-java-agent/instrumentation/tomcat-classloading/tomcat-classloading.gradle b/dd-java-agent/instrumentation/tomcat-classloading/tomcat-classloading.gradle new file mode 100644 index 0000000000..b4862224b2 --- /dev/null +++ b/dd-java-agent/instrumentation/tomcat-classloading/tomcat-classloading.gradle @@ -0,0 +1,34 @@ + +muzzle { + pass { + group = "org.apache.tomcat" + module = 'tomcat-catalina' + versions = "[3.0.14,)" + assertInverse = true + } +} + +apply from: "${rootDir}/gradle/java.gradle" + +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' + } +} + +dependencies { + compile project(':dd-java-agent:agent-tooling') + compile deps.bytebuddy + annotationProcessor deps.autoservice + implementation deps.autoservice + + testCompile project(':dd-java-agent:testing') + + //This seems to be the earliest version that has org.apache.catalina.loader.WebappClassLoaderBase + //Older versions would require slightly different instrumentation. + testCompile group: 'org.apache.tomcat', name: 'tomcat-catalina', version: '8.0.14' + + latestDepTestCompile group: 'org.apache.tomcat', name: 'tomcat-catalina', version: '+' +} diff --git a/settings.gradle b/settings.gradle index 0d93861bd3..1a3038cb40 100644 --- a/settings.gradle +++ b/settings.gradle @@ -77,6 +77,7 @@ include ':dd-java-agent:instrumentation:sparkjava-2.3' include ':dd-java-agent:instrumentation:spring-web' include ':dd-java-agent:instrumentation:spring-webflux' include ':dd-java-agent:instrumentation:spymemcached-2.12' +include ':dd-java-agent:instrumentation:tomcat-classloading' include ':dd-java-agent:instrumentation:trace-annotation' include ':dd-java-agent:instrumentation:vertx'