From 46b776f76af51f57a361161cc0a70db7e1966144 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 5 Mar 2020 12:18:55 -0500 Subject: [PATCH] Include classloading instrumentation into all tests Classloading instrument core JDK classes so we should make sure this doesn't have bad effects on other instrumentations. --- .../AdditionalLibraryIgnoresMatcher.java | 38 +++++++++++++++---- .../instrumentation/instrumentation.gradle | 6 ++- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/AdditionalLibraryIgnoresMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/AdditionalLibraryIgnoresMatcher.java index 8938230d27..df441159b5 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/AdditionalLibraryIgnoresMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/AdditionalLibraryIgnoresMatcher.java @@ -29,7 +29,6 @@ public class AdditionalLibraryIgnoresMatcher if (name.startsWith("com.beust.jcommander.") || name.startsWith("com.fasterxml.classmate.") - || name.startsWith("com.fasterxml.jackson.") || name.startsWith("com.github.mustachejava.") || name.startsWith("com.jayway.jsonpath.") || name.startsWith("com.lightbend.lagom.") @@ -49,7 +48,6 @@ public class AdditionalLibraryIgnoresMatcher || name.startsWith("org.springframework.ejb.") || name.startsWith("org.springframework.expression.") || name.startsWith("org.springframework.format.") - || name.startsWith("org.springframework.instrument.") || name.startsWith("org.springframework.jca.") || name.startsWith("org.springframework.jdbc.") || name.startsWith("org.springframework.jms.") @@ -69,8 +67,11 @@ public class AdditionalLibraryIgnoresMatcher } if (name.startsWith("org.springframework.data.")) { - if (name.equals( - "org.springframework.data.repository.core.support.RepositoryFactorySupport")) { + if (name.equals("org.springframework.data.repository.core.support.RepositoryFactorySupport") + || name.startsWith( + "org.springframework.data.convert.ClassGeneratingEntityInstantiator$") + || name.equals( + "org.springframework.data.jpa.repository.config.InspectionClassLoader")) { return false; } return true; @@ -98,7 +99,9 @@ public class AdditionalLibraryIgnoresMatcher || name.startsWith("org.springframework.boot.autoconfigure.condition.OnClassCondition$") || name.startsWith("org.springframework.boot.web.embedded.netty.NettyWebServer$") || name.startsWith( - "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainer$")) { + "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainer$") + || name.equals( + "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedWebappClassLoader")) { return false; } return true; @@ -116,14 +119,25 @@ public class AdditionalLibraryIgnoresMatcher if (name.startsWith("org.springframework.context.")) { // More runnables to deal with - if (name.startsWith("org.springframework.context.support.AbstractApplicationContext$")) { + if (name.startsWith("org.springframework.context.support.AbstractApplicationContext$") + || name.equals("org.springframework.context.support.ContextTypeMatchClassLoader")) { return false; } return true; } if (name.startsWith("org.springframework.core.")) { - if (name.startsWith("org.springframework.core.task.")) { + if (name.startsWith("org.springframework.core.task.") + || name.equals("org.springframework.core.DecoratingClassLoader") + || name.equals("org.springframework.core.OverridingClassLoader")) { + return false; + } + return true; + } + + if (name.startsWith("org.springframework.instrument.")) { + if (name.equals("org.springframework.instrument.classloading.SimpleThrowawayClassLoader") + || name.equals("org.springframework.instrument.classloading.ShadowingClassLoader")) { return false; } return true; @@ -219,7 +233,8 @@ public class AdditionalLibraryIgnoresMatcher } if (name.startsWith("com.google.inject.")) { // We instrument Runnable there - if (name.startsWith("com.google.inject.internal.AbstractBindingProcessor$")) { + if (name.startsWith("com.google.inject.internal.AbstractBindingProcessor$") + || name.startsWith("com.google.inject.internal.BytecodeGen$")) { return false; } return true; @@ -252,6 +267,13 @@ public class AdditionalLibraryIgnoresMatcher return true; } + if (name.startsWith("com.fasterxml.jackson.")) { + if (name.equals("com.fasterxml.jackson.module.afterburner.util.MyClassLoader")) { + return false; + } + return true; + } + // kotlin, note we do not ignore kotlinx because we instrument coroutins code if (name.startsWith("kotlin.")) { return true; diff --git a/dd-java-agent/instrumentation/instrumentation.gradle b/dd-java-agent/instrumentation/instrumentation.gradle index 8b5325ca61..170de85760 100644 --- a/dd-java-agent/instrumentation/instrumentation.gradle +++ b/dd-java-agent/instrumentation/instrumentation.gradle @@ -56,8 +56,12 @@ subprojects { Project subProj -> annotationProcessor deps.autoservice implementation deps.autoservice - // Always include concurrent instrumentation dependency to make sure that all instrumentations are tested with async parts instrumented + // Include instrumentations instrumenting core JDK classes tp ensure interoperability with other instrumentation testCompile project(':dd-java-agent:instrumentation:java-concurrent') + // FIXME: we should enable this, but currently this fails tests for google http client + //testCompile project(':dd-java-agent:instrumentation:http-url-connection') + testCompile project(':dd-java-agent:instrumentation:classloading') + testCompile project(':dd-java-agent:testing') testAnnotationProcessor deps.autoservice testImplementation deps.autoservice