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 b97dfcf6802..7245b2d70a4 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 @@ -1,5 +1,6 @@ package datadog.trace.agent.tooling.bytebuddy.matcher; +import java.util.regex.Pattern; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -14,6 +15,9 @@ public class AdditionalLibraryIgnoresMatcher extends ElementMatcher.Junction.AbstractBase { + private static final Pattern COM_MCHANGE_PROXY = + Pattern.compile("com\\.mchange\\.v2\\.c3p0\\..*Proxy"); + public static Junction additionalLibraryIgnoresMatcher() { return new AdditionalLibraryIgnoresMatcher<>(); } @@ -28,26 +32,39 @@ public boolean matches(final T target) { final String name = target.getActualName(); if (name.startsWith("com.beust.jcommander.") - || name.startsWith("com.carrotsearch.hppc.") - || name.startsWith("com.couchbase.client.deps.") || 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") + || name.startsWith("com.lightbend.lagom.") + || name.startsWith("com.netflix.hystrix.") // test this + || name.startsWith("io.micrometer.") + || name.startsWith("io.github.classgraph.") || name.startsWith("javax.el.") + || name.startsWith("javax.crypto.") + || name.startsWith("javax.cache.") + || name.startsWith("javax.management.") + || name.startsWith("javax.persistence.") || name.startsWith("net.sf.cglib.") - || name.startsWith("org.apache.lucene") - || name.startsWith("org.apache.tartarus") - || name.startsWith("org.json.simple") - || name.startsWith("org.objectweb.asm.") - || name.startsWith("org.yaml.snakeyaml")) { + || name.startsWith("nonapi.io.github.classgraph.") + || name.startsWith("org.apache.commons.") + || name.startsWith("org.apache.coyote.") + || name.startsWith("org.apache.el.") + || name.startsWith("org.apache.juli.") + || name.startsWith("org.apache.lucene.") + || name.startsWith("org.apache.naming.") + || name.startsWith("org.apache.tartarus.") + || name.startsWith("org.ehcache.") + || name.startsWith("org.hibernate.") // test this + || name.startsWith("org.json.simple.") + || name.startsWith("org.msgpack.") + || name.startsWith("org.thymeleaf.") + || name.startsWith("org.yaml.snakeyaml.")) { return true; } if (name.startsWith("org.springframework.")) { if (name.startsWith("org.springframework.aop.") - || name.startsWith("org.springframework.asm.") || name.startsWith("org.springframework.cache.") || name.startsWith("org.springframework.dao.") || name.startsWith("org.springframework.ejb.") @@ -60,16 +77,15 @@ public boolean matches(final T target) { || name.startsWith("org.springframework.jmx.") || name.startsWith("org.springframework.jndi.") || name.startsWith("org.springframework.lang.") + || name.startsWith("org.springframework.mail.") || name.startsWith("org.springframework.messaging.") || name.startsWith("org.springframework.objenesis.") || name.startsWith("org.springframework.orm.") || name.startsWith("org.springframework.remoting.") - || name.startsWith("org.springframework.scheduling.annotation") || name.startsWith("org.springframework.scripting.") || name.startsWith("org.springframework.stereotype.") || name.startsWith("org.springframework.transaction.") || name.startsWith("org.springframework.ui.") - || name.startsWith("org.springframework.util.") || name.startsWith("org.springframework.validation.")) { return true; } @@ -101,7 +117,10 @@ public boolean matches(final T target) { if (name.startsWith("org.springframework.boot.")) { // More runnables to deal with if (name.startsWith("org.springframework.boot.autoconfigure.BackgroundPreinitializer$") - || name.startsWith("org.springframework.boot.web.embedded.netty.NettyWebServer$")) { + || 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$")) { return false; } return true; @@ -140,9 +159,17 @@ public boolean matches(final T target) { return true; } + if (name.startsWith("org.springframework.util.")) { + if (name.startsWith("org.springframework.util.concurrent.")) { + return false; + } + return true; + } + if (name.startsWith("org.springframework.web.")) { if (name.startsWith("org.springframework.web.servlet.") - || name.startsWith("org.springframework.web.reactive.")) { + || name.startsWith("org.springframework.web.reactive.") + || name.startsWith("org.springframework.web.context.request.async.")) { return false; } return true; @@ -161,7 +188,10 @@ public boolean matches(final T target) { || name.startsWith("org.apache.xerces.") || name.startsWith("org.apache.xml.") || name.startsWith("org.apache.xpath.") - || name.startsWith("org.xml.")) { + || name.startsWith("org.xml.") + || name.startsWith("com.sun.org.apache.xerces.") + || name.startsWith("com.sun.org.apache.xalan.") + || name.startsWith("com.sun.xml.")) { return true; } @@ -182,8 +212,12 @@ public boolean matches(final T target) { return true; } - if (name.startsWith("com.datastax.driver.")) { - if (name.startsWith("com.datastax.driver.core.Cluster$")) { + if (name.startsWith("com.couchbase.client.deps.")) { + // Couchbase library includes some packaged dependencies, unfortunately some of them are + // instrumented by java-concurrent instrumentation + if (name.startsWith("com.couchbase.client.deps.io.netty.") + || name.startsWith("com.couchbase.client.deps.org.LatencyUtils.") + || name.startsWith("com.couchbase.client.deps.com.lmax.disruptor.")) { return false; } return true; @@ -216,7 +250,7 @@ public boolean matches(final T target) { return true; } if (name.startsWith("com.google.api.")) { - if (name.equals("com.google.api.client.http.HttpRequest")) { + if (name.startsWith("com.google.api.client.http.HttpRequest")) { return false; } return true; @@ -227,8 +261,66 @@ public boolean matches(final T target) { || name.startsWith("org.h2.jdbc.") || name.startsWith("org.h2.jdbcx.") // Some runnables that get instrumented + || name.equals("org.h2.util.Task") || name.equals("org.h2.store.FileLock") - || name.equals("org.h2.engine.DatabaseCloser")) { + || name.equals("org.h2.engine.DatabaseCloser") + || name.equals("org.h2.engine.OnExitDatabaseCloser")) { + return false; + } + return true; + } + + if (name.startsWith("com.carrotsearch.hppc.")) { + if (name.startsWith("com.carrotsearch.hppc.HashOrderMixing$")) { + return false; + } + return true; + } + + if (name.startsWith("okio.")) { + if (name.equals("okio.AsyncTimeout$Watchdog")) { + return false; + } + return true; + } + + if (name.startsWith("org.hsqldb.")) { + if (name.startsWith("org.hsqldb.jdbc.")) { + return false; + } + return true; + } + + if (name.startsWith("org.apache.catalina.")) { + if (name.startsWith("org.apache.catalina.connector.") + || name.startsWith("org.apache.catalina.core.") + || name.startsWith("org.apache.catalina.servlets.")) { + return false; + } + return true; + } + + if (name.startsWith("org.apache.logging.")) { + // We instrument single class in the whole library + // But unfortunately we also instrument some runnables + if (name.equals("org.apache.logging.log4j.ThreadContext") + || name.equals("org.apache.logging.log4j.core.util.DefaultShutdownCallbackRegistry")) { + return false; + } + return true; + } + + if (name.startsWith("org.apache.tomcat.")) { + if (name.startsWith("org.apache.tomcat.jdbc.")) { + return false; + } + return true; + } + + if (name.startsWith("io.grpc.")) { + // We instrument two specific classes + if (name.startsWith("io.grpc.internal.AbstractManagedChannelImplBuilder") + || name.startsWith("io.grpc.internal.AbstractServerImplBuilder")) { return false; } return true; @@ -239,6 +331,10 @@ public boolean matches(final T target) { return true; } + if (COM_MCHANGE_PROXY.matcher(name).matches()) { + return true; + } + return false; } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/GlobalIgnoresMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/GlobalIgnoresMatcher.java index eada68fd9c4..750846a6c71 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/GlobalIgnoresMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/GlobalIgnoresMatcher.java @@ -1,6 +1,5 @@ package datadog.trace.agent.tooling.bytebuddy.matcher; -import java.util.regex.Pattern; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -23,9 +22,6 @@ public class GlobalIgnoresMatcher extends ElementMatcher.Junction.AbstractBase { - private static final Pattern COM_MCHANGE_PROXY = - Pattern.compile("com\\.mchange\\.v2\\.c3p0\\..*Proxy"); - public static ElementMatcher.Junction globalIgnoresMatcher() { return new GlobalIgnoresMatcher<>(); } @@ -144,10 +140,6 @@ public boolean matches(final T target) { return true; } - if (COM_MCHANGE_PROXY.matcher(name).matches()) { - return true; - } - if (additionalLibraryIgnoreMatcher.matches(target)) { return true; } diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/client/TracingClientInterceptor.java b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/client/TracingClientInterceptor.java index 5102dda936d..c99f0e50120 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/client/TracingClientInterceptor.java +++ b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/client/TracingClientInterceptor.java @@ -33,7 +33,6 @@ public ClientCall interceptCall( startSpan("grpc.client").setTag(DDTags.RESOURCE_NAME, method.getFullMethodName()); try (final AgentScope scope = activateSpan(span, false)) { DECORATE.afterStart(span); - scope.setAsyncPropagation(true); final ClientCall result; try { @@ -64,7 +63,6 @@ public void start(final Listener responseListener, final Metadata headers propagate().inject(span, headers, SETTER); try (final AgentScope scope = activateSpan(span, false)) { - scope.setAsyncPropagation(true); super.start(new TracingClientCallListener<>(span, responseListener), headers); } catch (final Throwable e) { DECORATE.onError(span, e); @@ -77,7 +75,6 @@ public void start(final Listener responseListener, final Metadata headers @Override public void sendMessage(final ReqT message) { try (final AgentScope scope = activateSpan(span, false)) { - scope.setAsyncPropagation(true); super.sendMessage(message); } catch (final Throwable e) { DECORATE.onError(span, e); @@ -104,7 +101,6 @@ public void onMessage(final RespT message) { .setTag("message.type", message.getClass().getName()); DECORATE.afterStart(messageSpan); final AgentScope scope = activateSpan(messageSpan, true); - scope.setAsyncPropagation(true); try { delegate().onMessage(message); } catch (final Throwable e) { @@ -121,7 +117,6 @@ public void onClose(final Status status, final Metadata trailers) { DECORATE.onClose(span, status); // Finishes span. try (final AgentScope scope = activateSpan(span, false)) { - scope.setAsyncPropagation(true); delegate().onClose(status, trailers); } catch (final Throwable e) { DECORATE.onError(span, e); @@ -135,7 +130,6 @@ public void onClose(final Status status, final Metadata trailers) { @Override public void onReady() { try (final AgentScope scope = activateSpan(span, false)) { - scope.setAsyncPropagation(true); delegate().onReady(); } catch (final Throwable e) { DECORATE.onError(span, e); diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java index 57cb0e8cc7e..7335c733a2a 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java +++ b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/TracingServerInterceptor.java @@ -37,7 +37,6 @@ public ServerCall.Listener interceptCall( DECORATE.afterStart(span); final AgentScope scope = activateSpan(span, false); - scope.setAsyncPropagation(true); final ServerCall.Listener result; try { @@ -53,7 +52,6 @@ public ServerCall.Listener interceptCall( span.finish(); throw e; } finally { - scope.setAsyncPropagation(false); scope.close(); } @@ -74,9 +72,7 @@ static final class TracingServerCall public void close(final Status status, final Metadata trailers) { DECORATE.onClose(span, status); try (final AgentScope scope = activateSpan(span, false)) { - scope.setAsyncPropagation(true); delegate().close(status, trailers); - scope.setAsyncPropagation(false); } catch (final Throwable e) { DECORATE.onError(span, e); throw e; @@ -100,7 +96,6 @@ public void onMessage(final ReqT message) { .setTag("message.type", message.getClass().getName()); DECORATE.afterStart(span); final AgentScope scope = activateSpan(span, true); - scope.setAsyncPropagation(true); try { delegate().onMessage(message); } catch (final Throwable e) { @@ -109,7 +104,6 @@ public void onMessage(final ReqT message) { this.span.finish(); throw e; } finally { - scope.setAsyncPropagation(false); DECORATE.beforeFinish(span); scope.close(); } @@ -118,9 +112,7 @@ public void onMessage(final ReqT message) { @Override public void onHalfClose() { try (final AgentScope scope = activateSpan(span, false)) { - scope.setAsyncPropagation(true); delegate().onHalfClose(); - scope.setAsyncPropagation(false); } catch (final Throwable e) { DECORATE.onError(span, e); DECORATE.beforeFinish(span); @@ -133,10 +125,8 @@ public void onHalfClose() { public void onCancel() { // Finishes span. try (final AgentScope scope = activateSpan(span, false)) { - scope.setAsyncPropagation(true); delegate().onCancel(); span.setTag("canceled", true); - scope.setAsyncPropagation(false); } catch (final Throwable e) { DECORATE.onError(span, e); throw e; @@ -150,9 +140,7 @@ public void onCancel() { public void onComplete() { // Finishes span. try (final AgentScope scope = activateSpan(span, false)) { - scope.setAsyncPropagation(true); delegate().onComplete(); - scope.setAsyncPropagation(false); } catch (final Throwable e) { DECORATE.onError(span, e); throw e; @@ -165,9 +153,7 @@ public void onComplete() { @Override public void onReady() { try (final AgentScope scope = activateSpan(span, false)) { - scope.setAsyncPropagation(true); delegate().onReady(); - scope.setAsyncPropagation(false); } catch (final Throwable e) { DECORATE.onError(span, e); DECORATE.beforeFinish(span); diff --git a/dd-java-agent/instrumentation/instrumentation.gradle b/dd-java-agent/instrumentation/instrumentation.gradle index 12e443dbf87..8b5325ca61d 100644 --- a/dd-java-agent/instrumentation/instrumentation.gradle +++ b/dd-java-agent/instrumentation/instrumentation.gradle @@ -56,6 +56,8 @@ 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 + testCompile project(':dd-java-agent:instrumentation:java-concurrent') testCompile project(':dd-java-agent:testing') testAnnotationProcessor deps.autoservice testImplementation deps.autoservice diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/RunnableCallableInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/CallableInstrumentation.java similarity index 69% rename from dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/RunnableCallableInstrumentation.java rename to dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/CallableInstrumentation.java index 4795056c849..bced21fffca 100644 --- a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/RunnableCallableInstrumentation.java +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/CallableInstrumentation.java @@ -24,15 +24,15 @@ /** Instrument {@link Runnable} and {@Callable} */ @Slf4j @AutoService(Instrumenter.class) -public final class RunnableCallableInstrumentation extends Instrumenter.Default { +public final class CallableInstrumentation extends Instrumenter.Default { - public RunnableCallableInstrumentation() { + public CallableInstrumentation() { super(AbstractExecutorInstrumentation.EXEC_NAME); } @Override public ElementMatcher typeMatcher() { - return implementsInterface(named(Runnable.class.getName()).or(named(Callable.class.getName()))); + return implementsInterface(named(Callable.class.getName())); } @Override @@ -45,7 +45,6 @@ public String[] helperClassNames() { @Override public Map contextStore() { final Map map = new HashMap<>(); - map.put(Runnable.class.getName(), State.class.getName()); map.put(Callable.class.getName(), State.class.getName()); return Collections.unmodifiableMap(map); } @@ -53,30 +52,12 @@ public Map contextStore() { @Override public Map, String> transformers() { final Map, String> transformers = new HashMap<>(); - transformers.put( - named("run").and(takesArguments(0)).and(isPublic()), - RunnableCallableInstrumentation.class.getName() + "$RunnableAdvice"); transformers.put( named("call").and(takesArguments(0)).and(isPublic()), - RunnableCallableInstrumentation.class.getName() + "$CallableAdvice"); + CallableInstrumentation.class.getName() + "$CallableAdvice"); return transformers; } - public static class RunnableAdvice { - - @Advice.OnMethodEnter(suppress = Throwable.class) - public static TraceScope enter(@Advice.This final Runnable thiz) { - final ContextStore contextStore = - InstrumentationContext.get(Runnable.class, State.class); - return AdviceUtils.startTaskScope(contextStore, thiz); - } - - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void exit(@Advice.Enter final TraceScope scope) { - AdviceUtils.endTaskScope(scope); - } - } - public static class CallableAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/RunnableInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/RunnableInstrumentation.java new file mode 100644 index 00000000000..2ae88a96f74 --- /dev/null +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/RunnableInstrumentation.java @@ -0,0 +1,74 @@ +package datadog.trace.instrumentation.java.concurrent; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.implementsInterface; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.ContextStore; +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.bootstrap.instrumentation.java.concurrent.State; +import datadog.trace.context.TraceScope; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import lombok.extern.slf4j.Slf4j; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +/** Instrument {@link Runnable} and {@Callable} */ +@Slf4j +@AutoService(Instrumenter.class) +public final class RunnableInstrumentation extends Instrumenter.Default { + + public RunnableInstrumentation() { + super(AbstractExecutorInstrumentation.EXEC_NAME); + } + + @Override + public ElementMatcher typeMatcher() { + return implementsInterface(named(Runnable.class.getName())); + } + + @Override + public String[] helperClassNames() { + return new String[] { + AdviceUtils.class.getName(), + }; + } + + @Override + public Map contextStore() { + final Map map = new HashMap<>(); + map.put(Runnable.class.getName(), State.class.getName()); + return Collections.unmodifiableMap(map); + } + + @Override + public Map, String> transformers() { + final Map, String> transformers = new HashMap<>(); + transformers.put( + named("run").and(takesArguments(0)).and(isPublic()), + RunnableInstrumentation.class.getName() + "$RunnableAdvice"); + return transformers; + } + + public static class RunnableAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static TraceScope enter(@Advice.This final Runnable thiz) { + final ContextStore contextStore = + InstrumentationContext.get(Runnable.class, State.class); + return AdviceUtils.startTaskScope(contextStore, thiz); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void exit(@Advice.Enter final TraceScope scope) { + AdviceUtils.endTaskScope(scope); + } + } +} diff --git a/dd-java-agent/instrumentation/log4j2/src/main/java/datadog/trace/instrumentation/log4j2/ThreadContextInstrumentation.java b/dd-java-agent/instrumentation/log4j2/src/main/java/datadog/trace/instrumentation/log4j2/ThreadContextInstrumentation.java index 88048bc1fad..27f2b85d1b2 100644 --- a/dd-java-agent/instrumentation/log4j2/src/main/java/datadog/trace/instrumentation/log4j2/ThreadContextInstrumentation.java +++ b/dd-java-agent/instrumentation/log4j2/src/main/java/datadog/trace/instrumentation/log4j2/ThreadContextInstrumentation.java @@ -29,6 +29,8 @@ protected boolean defaultEnabled() { return Config.get().isLogsInjectionEnabled(); } + // We instrument single class in the whole library, the rest is ignored by + // AdditionalLibraryIgnoresMatcher @Override public ElementMatcher typeMatcher() { return named("org.apache.logging.log4j.ThreadContext");