From b6118f0397b6ca61b6e1b224821d6ae7066cadeb Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Fri, 27 Jul 2018 10:29:17 -0400 Subject: [PATCH 01/12] Apache HTTP Client: add test for redirected request --- .../test/groovy/ApacheHttpClientTest.groovy | 270 ++++++++++-------- 1 file changed, 154 insertions(+), 116 deletions(-) diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/test/groovy/ApacheHttpClientTest.groovy b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/test/groovy/ApacheHttpClientTest.groovy index 88d2d01b1f..5fbc1d680f 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/test/groovy/ApacheHttpClientTest.groovy +++ b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/test/groovy/ApacheHttpClientTest.groovy @@ -1,10 +1,13 @@ import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.ListWriterAssert import datadog.trace.agent.test.RatpackUtils +import datadog.trace.agent.test.TraceAssert import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import io.opentracing.tag.Tags import org.apache.http.HttpResponse import org.apache.http.client.HttpClient +import org.apache.http.client.config.RequestConfig import org.apache.http.client.methods.HttpGet import org.apache.http.impl.client.HttpClientBuilder import org.apache.http.message.BasicHeader @@ -19,151 +22,186 @@ class ApacheHttpClientTest extends AgentTestRunner { @Shared def server = ratpack { handlers { - get { - RatpackUtils.handleDistributedRequest(context) + prefix("success") { + get { + RatpackUtils.handleDistributedRequest(context) - String msg = "

Hello test.

\n" - response.status(200).send(msg) + String msg = "

Hello test.

\n" + response.status(200).send(msg) + } + } + prefix("redirect") { + get { + RatpackUtils.handleDistributedRequest(context) + + redirect(server.address.resolve("/success").toURL().toString()) + } } } } @Shared - int port = server.getAddress().port + int port = server.address.port + @Shared + def successUrl = server.address.resolve("/success") + @Shared + def redirectUrl = server.address.resolve("/redirect") + + final HttpClientBuilder builder = HttpClientBuilder.create() + final HttpClient client = builder.build() def "trace request with propagation"() { setup: - final HttpClientBuilder builder = HttpClientBuilder.create() - - final HttpClient client = builder.build() runUnderTrace("someTrace") { - try { - HttpResponse response = client.execute(new HttpGet(server.getAddress())) - assert response.getStatusLine().getStatusCode() == 200 - } catch (Exception e) { - e.printStackTrace() - throw new RuntimeException(e) - } + HttpResponse response = client.execute(new HttpGet(successUrl)) + assert response.getStatusLine().getStatusCode() == 200 } expect: // one trace on the server, one trace on the client assertTraces(TEST_WRITER, 2) { - trace(0, 1) { - span(0) { - childOf TEST_WRITER[1][2] - serviceName "unnamed-java-app" - operationName "test-http-server" - resourceName "test-http-server" - errored false - tags { - defaultTags() - } - } - } + serverTrace(it, 0, TEST_WRITER[1][2]) trace(1, 3) { - span(0) { - parent() - serviceName "unnamed-java-app" - operationName "someTrace" - resourceName "someTrace" - errored false - tags { - defaultTags() - } - } - span(1) { - childOf span(0) - serviceName "unnamed-java-app" - operationName "apache.http" - resourceName "apache.http" - errored false - tags { - defaultTags() - "$Tags.COMPONENT.key" "apache-httpclient" - } - } - span(2) { - childOf span(1) - serviceName "unnamed-java-app" - operationName "http.request" - resourceName "GET /" - spanType DDSpanTypes.HTTP_CLIENT - errored false - tags { - defaultTags() - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "http://localhost:$port/" - "$Tags.PEER_HOSTNAME.key" "localhost" - "$Tags.PEER_PORT.key" server.getAddress().port - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT - "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT - } - } + outerSpan(it, 0) + clientParentSpan(it, 1, span(0)) + successClientSpan(it, 2, span(1)) + } + } + } + + def "trace redirected request with propagation many redirects allowed"() { + setup: + final RequestConfig.Builder requestConfigBuilder = new RequestConfig.Builder() + requestConfigBuilder.setMaxRedirects(10) + runUnderTrace("someTrace") { + HttpGet request = new HttpGet(redirectUrl) + request.setConfig(requestConfigBuilder.build()) + HttpResponse response = client.execute(request) + assert response.getStatusLine().getStatusCode() == 200 + } + + expect: + // two traces on the server, one trace on the client + assertTraces(TEST_WRITER, 3) { + serverTrace(it, 0, TEST_WRITER[2][3]) + serverTrace(it, 1, TEST_WRITER[2][2]) + trace(2, 4) { + outerSpan(it, 0) + clientParentSpan(it, 1, span(0)) + successClientSpan(it, 2, span(1)) + redirectClientSpan(it, 3, span(1)) + } + } + } + + def "trace redirected request with propagation 1 redirect allowed"() { + setup: + final RequestConfig.Builder requestConfigBuilder = new RequestConfig.Builder() + requestConfigBuilder.setMaxRedirects(1) + runUnderTrace("someTrace") { + HttpGet request = new HttpGet(redirectUrl) + request.setConfig(requestConfigBuilder.build()) + HttpResponse response = client.execute(request) + assert response.getStatusLine().getStatusCode() == 200 + } + + expect: + print(TEST_WRITER) + // two traces on the server, one trace on the client + assertTraces(TEST_WRITER, 3) { + serverTrace(it, 0, TEST_WRITER[2][3]) + serverTrace(it, 1, TEST_WRITER[2][1]) + trace(2, 4) { + outerSpan(it, 0) + // Note: this is kind of an weird order? + successClientSpan(it, 1, span(2)) + clientParentSpan(it, 2, span(0)) + redirectClientSpan(it, 3, span(2)) } } } def "trace request without propagation"() { setup: - final HttpClientBuilder builder = HttpClientBuilder.create() - - final HttpClient client = builder.build() runUnderTrace("someTrace") { - try { - HttpGet request = new HttpGet(server.getAddress()) - request.addHeader(new BasicHeader("is-dd-server", "false")) - HttpResponse response = client.execute(request) - assert response.getStatusLine().getStatusCode() == 200 - } catch (Exception e) { - e.printStackTrace() - throw new RuntimeException(e) - } + HttpGet request = new HttpGet(successUrl) + request.addHeader(new BasicHeader("is-dd-server", "false")) + HttpResponse response = client.execute(request) + assert response.getStatusLine().getStatusCode() == 200 } expect: // only one trace (client). assertTraces(TEST_WRITER, 1) { trace(0, 3) { - span(0) { - parent() - serviceName "unnamed-java-app" - operationName "someTrace" - resourceName "someTrace" - errored false - tags { - defaultTags() - } - } - span(1) { - childOf span(0) - serviceName "unnamed-java-app" - operationName "apache.http" - resourceName "apache.http" - errored false - tags { - defaultTags() - "$Tags.COMPONENT.key" "apache-httpclient" - } - } - span(2) { - childOf span(1) - serviceName "unnamed-java-app" - operationName "http.request" - resourceName "GET /" - spanType DDSpanTypes.HTTP_CLIENT - errored false - tags { - defaultTags() - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "http://localhost:$port/" - "$Tags.PEER_HOSTNAME.key" "localhost" - "$Tags.PEER_PORT.key" server.getAddress().port - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT - "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT - } + outerSpan(it, 0) + clientParentSpan(it, 1, span(0)) + successClientSpan(it, 2, span(1)) + } + } + } + + def serverTrace(ListWriterAssert writer, index, parent) { + writer.trace(index, 1) { + span(0) { + childOf parent + serviceName "unnamed-java-app" + operationName "test-http-server" + resourceName "test-http-server" + errored false + tags { + defaultTags() } } } } + + def outerSpan(TraceAssert trace, index) { + trace.span(index) { + parent() + serviceName "unnamed-java-app" + operationName "someTrace" + resourceName "someTrace" + errored false + tags { + defaultTags() + } + } + } + + def clientParentSpan(TraceAssert trace, index, parent) { + trace.span(index) { + childOf parent + serviceName "unnamed-java-app" + operationName "apache.http" + resourceName "apache.http" + errored false + tags { + defaultTags() + "$Tags.COMPONENT.key" "apache-httpclient" + } + } + } + + def successClientSpan(TraceAssert trace, index, parent, status = 200, route = "success") { + trace.span(index) { + childOf parent + serviceName "unnamed-java-app" + operationName "http.request" + resourceName "GET /$route" + errored false + tags { + defaultTags() + "$Tags.HTTP_STATUS.key" status + "$Tags.HTTP_URL.key" "http://localhost:$port/$route" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.getAddress().port + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT + } + } + } + + def redirectClientSpan(TraceAssert trace, index, parent) { + successClientSpan(trace, index, parent, 302, "redirect") + } } From 4ae9263e1c39e53a6e44b72018bd34311b79e73f Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 30 Jul 2018 10:05:51 -0400 Subject: [PATCH 02/12] Set POOL_ONLY DescriptionStrategy for ByteBuddy This makes sure no classes are loaded during instrumentation transformation which allows us to safely instrument depndent classes. --- .../java/datadog/trace/agent/tooling/AgentInstaller.java | 1 + .../trace/agent/tooling/muzzle/ReferenceMatcher.java | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java index 3a9aa0be71..059feb96bd 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java @@ -42,6 +42,7 @@ public class AgentInstaller { new AgentBuilder.Default() .disableClassFormatChanges() .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION) + .with(AgentBuilder.DescriptionStrategy.Default.POOL_ONLY) .with(new LoggingListener()) .with(new DDLocationStrategy()) .ignore(any(), skipClassLoader()) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceMatcher.java index 807af39d6f..3a6a25bf5f 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceMatcher.java @@ -21,11 +21,11 @@ public class ReferenceMatcher { private final Reference[] references; private final Set helperClassNames; - public ReferenceMatcher(Reference... references) { + public ReferenceMatcher(final Reference... references) { this(new String[0], references); } - public ReferenceMatcher(String[] helperClassNames, Reference[] references) { + public ReferenceMatcher(final String[] helperClassNames, final Reference[] references) { this.references = references; this.helperClassNames = new HashSet<>(Arrays.asList(helperClassNames)); } @@ -34,7 +34,7 @@ public class ReferenceMatcher { * @param loader Classloader to validate against (or null for bootstrap) * @return true if all references match the classpath of loader */ - public boolean matches(ClassLoader loader) { + public boolean matches(final ClassLoader loader) { return getMismatchedReferenceSources(loader).size() == 0; } @@ -52,7 +52,7 @@ public class ReferenceMatcher { mismatches = mismatchCache.get(loader); if (null == mismatches) { mismatches = new ArrayList<>(0); - for (Reference reference : references) { + for (final Reference reference : references) { // Don't reference-check helper classes. // They will be injected by the instrumentation's HelperInjector. if (!helperClassNames.contains(reference.getClassName())) { From bae79514c05675d610c2a13b7523a0ae2b3d0210 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 30 Jul 2018 13:27:32 -0400 Subject: [PATCH 03/12] Fix DDLocationStrategy to use DataDog ClassLoader This allows ByteBuddy to properly find classes injected into the agent. Thanks @realark for providing a fix! --- .../datadog/trace/agent/tooling/DDLocationStrategy.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/DDLocationStrategy.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/DDLocationStrategy.java index 15ef5d1c99..dfa5c07b5f 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/DDLocationStrategy.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/DDLocationStrategy.java @@ -12,16 +12,14 @@ import net.bytebuddy.utility.JavaModule; * reached. */ public class DDLocationStrategy implements AgentBuilder.LocationStrategy { - private static final ClassLoader BOOTSTRAP_RESOURCE_LOCATOR = new ClassLoader(null) {}; - @Override - public ClassFileLocator classFileLocator(ClassLoader classLoader, JavaModule javaModule) { - List locators = new ArrayList(); + public ClassFileLocator classFileLocator(ClassLoader classLoader, final JavaModule javaModule) { + final List locators = new ArrayList<>(); while (classLoader != null) { locators.add(ClassFileLocator.ForClassLoader.of(classLoader)); classLoader = classLoader.getParent(); } - locators.add(ClassFileLocator.ForClassLoader.of(BOOTSTRAP_RESOURCE_LOCATOR)); + locators.add(ClassFileLocator.ForClassLoader.of(Utils.getBootstrapProxy())); return new ClassFileLocator.Compound(locators.toArray(new ClassFileLocator[0])); } } From 1ebe4732f8e65eec52f467632e0430733ae74187 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 30 Jul 2018 13:33:43 -0400 Subject: [PATCH 04/12] Stop using `failSafe` matcher It should not be necessary after we jave fixed class location issue for ByteBuddy --- .../instrumentation/jaxrs/JaxRsClientInstrumentation.java | 3 +-- .../instrumentation/jdbc/ConnectionInstrumentation.java | 5 ++--- .../jms1/JMS1MessageListenerInstrumentation.java | 5 ++--- .../jms1/JMS1MessageProducerInstrumentation.java | 5 ++--- .../jms2/JMS2MessageConsumerInstrumentation.java | 5 ++--- .../jms2/JMS2MessageListenerInstrumentation.java | 5 ++--- .../jms2/JMS2MessageProducerInstrumentation.java | 5 ++--- .../servlet2/FilterChain2Instrumentation.java | 3 +-- .../servlet2/HttpServlet2Instrumentation.java | 3 +-- .../servlet3/AbstractServlet3Instrumentation.java | 3 +-- .../servlet3/FilterChain3Instrumentation.java | 3 +-- .../servlet3/HttpServlet3Instrumentation.java | 3 +-- .../springweb/SpringWebInstrumentation.java | 7 +++---- 13 files changed, 21 insertions(+), 34 deletions(-) diff --git a/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java b/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java index 1315114efd..c896aa1c64 100644 --- a/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java @@ -1,6 +1,5 @@ package datadog.trace.instrumentation.jaxrs; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.returns; @@ -22,7 +21,7 @@ public final class JaxRsClientInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return failSafe(hasSuperType(named("javax.ws.rs.client.ClientBuilder"))); + return hasSuperType(named("javax.ws.rs.client.ClientBuilder")); } @Override diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java index e5c21a4c70..25877f74f1 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java @@ -1,6 +1,5 @@ package datadog.trace.instrumentation.jdbc; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; @@ -27,12 +26,12 @@ public final class ConnectionInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(failSafe(isSubTypeOf(Connection.class))); + return not(isInterface()).and(isSubTypeOf(Connection.class)); } @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( nameStartsWith("prepare") .and(takesArgument(0, String.class)) diff --git a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java index c8837ed721..21f84f6b6b 100644 --- a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java @@ -3,7 +3,6 @@ package datadog.trace.instrumentation.jms1; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -40,7 +39,7 @@ public final class JMS1MessageListenerInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(failSafe(hasSuperType(named("javax.jms.MessageListener")))); + return not(isInterface()).and(hasSuperType(named("javax.jms.MessageListener"))); } @Override @@ -55,7 +54,7 @@ public final class JMS1MessageListenerInstrumentation extends Instrumenter.Defau @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( named("onMessage").and(takesArgument(0, named("javax.jms.Message"))).and(isPublic()), MessageListenerAdvice.class.getName()); diff --git a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java index 9cb37ff2b5..97efb4d3f2 100644 --- a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java @@ -3,7 +3,6 @@ package datadog.trace.instrumentation.jms1; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -40,7 +39,7 @@ public final class JMS1MessageProducerInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(failSafe(hasSuperType(named("javax.jms.MessageProducer")))); + return not(isInterface()).and(hasSuperType(named("javax.jms.MessageProducer"))); } @Override @@ -55,7 +54,7 @@ public final class JMS1MessageProducerInstrumentation extends Instrumenter.Defau @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( named("send").and(takesArgument(0, named("javax.jms.Message"))).and(isPublic()), ProducerAdvice.class.getName()); diff --git a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java index 73e9d86571..e0dca6ef9d 100644 --- a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java @@ -3,7 +3,6 @@ package datadog.trace.instrumentation.jms2; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -47,7 +46,7 @@ public final class JMS2MessageConsumerInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(failSafe(hasSuperType(named("javax.jms.MessageConsumer")))); + return not(isInterface()).and(hasSuperType(named("javax.jms.MessageConsumer"))); } @Override @@ -62,7 +61,7 @@ public final class JMS2MessageConsumerInstrumentation extends Instrumenter.Defau @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( named("receive").and(takesArguments(0).or(takesArguments(1))).and(isPublic()), ConsumerAdvice.class.getName()); diff --git a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java index 5d457f25a4..2ccfe83f47 100644 --- a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java @@ -3,7 +3,6 @@ package datadog.trace.instrumentation.jms2; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -40,7 +39,7 @@ public final class JMS2MessageListenerInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(failSafe(hasSuperType(named("javax.jms.MessageListener")))); + return not(isInterface()).and(hasSuperType(named("javax.jms.MessageListener"))); } @Override @@ -55,7 +54,7 @@ public final class JMS2MessageListenerInstrumentation extends Instrumenter.Defau @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( named("onMessage").and(takesArgument(0, named("javax.jms.Message"))).and(isPublic()), MessageListenerAdvice.class.getName()); diff --git a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java index f8d41483e4..01546ab226 100644 --- a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java @@ -3,7 +3,6 @@ package datadog.trace.instrumentation.jms2; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -40,7 +39,7 @@ public final class JMS2MessageProducerInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(failSafe(hasSuperType(named("javax.jms.MessageProducer")))); + return not(isInterface()).and(hasSuperType(named("javax.jms.MessageProducer"))); } @Override @@ -55,7 +54,7 @@ public final class JMS2MessageProducerInstrumentation extends Instrumenter.Defau @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( named("send").and(takesArgument(0, named("javax.jms.Message"))).and(isPublic()), ProducerAdvice.class.getName()); diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java index 5bbb6065fb..64f8103a64 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.servlet2; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -24,7 +23,7 @@ public final class FilterChain2Instrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(failSafe(hasSuperType(named("javax.servlet.FilterChain")))); + return not(isInterface()).and(hasSuperType(named("javax.servlet.FilterChain"))); } @Override diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java index 7f9609933e..f58c348ce4 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.servlet2; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isProtected; @@ -29,7 +28,7 @@ public final class HttpServlet2Instrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(failSafe(hasSuperType(named("javax.servlet.http.HttpServlet")))); + return not(isInterface()).and(hasSuperType(named("javax.servlet.http.HttpServlet"))); } @Override diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java index 808c42a491..e7c6bc0198 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.servlet3; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -22,7 +21,7 @@ public class AbstractServlet3Instrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(failSafe(hasSuperType(named("javax.servlet.FilterChain")))); + return not(isInterface()).and(hasSuperType(named("javax.servlet.FilterChain"))); } @Override diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java index 459ac401fe..15e6c236f1 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java @@ -1,6 +1,5 @@ package datadog.trace.instrumentation.servlet3; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -19,7 +18,7 @@ public final class FilterChain3Instrumentation extends AbstractServlet3Instrumen @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(failSafe(hasSuperType(named("javax.servlet.FilterChain")))); + return not(isInterface()).and(hasSuperType(named("javax.servlet.FilterChain"))); } @Override diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java index c4f6f023d3..61f2b1288e 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java @@ -1,6 +1,5 @@ package datadog.trace.instrumentation.servlet3; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isProtected; @@ -19,7 +18,7 @@ public final class HttpServlet3Instrumentation extends AbstractServlet3Instrumen @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(failSafe(hasSuperType(named("javax.servlet.http.HttpServlet")))); + return not(isInterface()).and(hasSuperType(named("javax.servlet.http.HttpServlet"))); } @Override diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java index be53bc2091..632a6e3742 100644 --- a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java @@ -2,7 +2,6 @@ package datadog.trace.instrumentation.springweb; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClassWithField; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -39,7 +38,7 @@ public final class SpringWebInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { return not(isInterface()) - .and(failSafe(hasSuperType(named("org.springframework.web.servlet.HandlerAdapter")))); + .and(hasSuperType(named("org.springframework.web.servlet.HandlerAdapter"))); } @Override @@ -50,7 +49,7 @@ public final class SpringWebInstrumentation extends Instrumenter.Default { @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( isMethod() .and(isPublic()) @@ -74,7 +73,7 @@ public final class SpringWebInstrumentation extends Instrumenter.Default { @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( isMethod() .and(isProtected()) From deec4a795c42258c028bc136111953f36149c439 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 30 Jul 2018 10:31:14 -0400 Subject: [PATCH 05/12] Improve Apache HTTP client Properly open and close outer span in multi-request cases --- .../ApacheHttpClientInstrumentation.java | 57 +++++-- .../apachehttpclient/DDTracingClientExec.java | 120 +++---------- .../test/groovy/ApacheHttpClientTest.groovy | 161 ++++++++++-------- 3 files changed, 157 insertions(+), 181 deletions(-) diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java index 1efe58cd48..f3e05c6ed7 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java @@ -1,18 +1,25 @@ package datadog.trace.instrumentation.apachehttpclient; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; +import static io.opentracing.log.Fields.ERROR_OBJECT; +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isAbstract; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.Tracer; +import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; -import org.apache.http.impl.client.DefaultRedirectStrategy; import org.apache.http.impl.execchain.ClientExecChain; @AutoService(Instrumenter.class) @@ -23,8 +30,9 @@ public class ApacheHttpClientInstrumentation extends Instrumenter.Default { } @Override - public ElementMatcher typeMatcher() { - return named("org.apache.http.impl.client.HttpClientBuilder"); + public ElementMatcher typeMatcher() { + return named("org.apache.http.impl.client.HttpClientBuilder") + .or(hasSuperType(named("org.apache.http.impl.client.CloseableHttpClient"))); } @Override @@ -38,7 +46,9 @@ public class ApacheHttpClientInstrumentation extends Instrumenter.Default { "org.apache.http.client.methods.HttpRequestWrapper", "org.apache.http.client.protocol.HttpClientContext", "org.apache.http.conn.routing.HttpRoute", - "org.apache.http.impl.execchain.ClientExecChain"); + "org.apache.http.impl.execchain.ClientExecChain", + "org.apache.http.impl.client.CloseableHttpClient", + "org.apache.http.impl.client.InternalHttpClient"); } @Override @@ -51,19 +61,42 @@ public class ApacheHttpClientInstrumentation extends Instrumenter.Default { @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( - isMethod().and(named("decorateProtocolExec")), ApacheHttpClientAdvice.class.getName()); + isMethod().and(not(isAbstract())).and(named("doExecute")), ClientAdvice.class.getName()); + transformers.put( + isMethod().and(named("decorateProtocolExec")), ClientExecAdvice.class.getName()); return transformers; } - public static class ApacheHttpClientAdvice { - /** Strategy: add our tracing exec to the apache exec chain. */ + public static class ClientAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Scope methodEnter() { + final Tracer.SpanBuilder spanBuilder = + GlobalTracer.get() + .buildSpan(DDTracingClientExec.OPERATION_NAME) + .withTag(Tags.COMPONENT.getKey(), DDTracingClientExec.COMPONENT_NAME); + return spanBuilder.startActive(true); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void methodExit( + @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) { + final Span span = scope.span(); + if (throwable != null) { + Tags.ERROR.set(span, Boolean.TRUE); + span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + span.finish(); + } + scope.close(); + } + } + + public static class ClientExecAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void addTracingExec(@Advice.Return(readOnly = false) ClientExecChain execChain) { - execChain = - new DDTracingClientExec( - execChain, DefaultRedirectStrategy.INSTANCE, false, GlobalTracer.get()); + execChain = new DDTracingClientExec(execChain, GlobalTracer.get()); } } } diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/DDTracingClientExec.java b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/DDTracingClientExec.java index 3a34eb4ded..f8d3aef521 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/DDTracingClientExec.java +++ b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/DDTracingClientExec.java @@ -18,7 +18,6 @@ import java.util.Map; import lombok.extern.slf4j.Slf4j; import org.apache.http.HttpException; import org.apache.http.HttpRequest; -import org.apache.http.client.RedirectStrategy; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpExecutionAware; import org.apache.http.client.methods.HttpRequestWrapper; @@ -33,36 +32,15 @@ import org.apache.http.impl.execchain.ClientExecChain; */ @Slf4j public class DDTracingClientExec implements ClientExecChain { - private static final String COMPONENT_NAME = "apache-httpclient"; - private static final String OPERATION_NAME = "http.request"; - /** - * Id of {@link HttpClientContext#setAttribute(String, Object)} representing span associated with - * the current client processing. Referenced span is local span not a span representing HTTP - * communication. - */ - private static final String ACTIVE_SPAN = DDTracingClientExec.class.getName() + ".activeSpan"; - /** - * Tracing {@link ClientExecChain} is executed after redirect exec, so on redirects it is called - * multiple times. This is used as an id for {@link HttpClientContext#setAttribute(String, - * Object)} to store number of redirects. - */ - private static final String REDIRECT_COUNT = - DDTracingClientExec.class.getName() + ".redirectCount"; + static final String COMPONENT_NAME = "apache-httpclient"; + static final String OPERATION_NAME = "http.request"; - private final RedirectStrategy redirectStrategy; private final ClientExecChain requestExecutor; - private final boolean redirectHandlingDisabled; private final Tracer tracer; - public DDTracingClientExec( - final ClientExecChain clientExecChain, - final RedirectStrategy redirectStrategy, - final boolean redirectHandlingDisabled, - final Tracer tracer) { - this.requestExecutor = clientExecChain; - this.redirectStrategy = redirectStrategy; - this.redirectHandlingDisabled = redirectHandlingDisabled; + public DDTracingClientExec(final ClientExecChain clientExecChain, final Tracer tracer) { + requestExecutor = clientExecChain; this.tracer = tracer; } @@ -73,91 +51,37 @@ public class DDTracingClientExec implements ClientExecChain { final HttpClientContext clientContext, final HttpExecutionAware execAware) throws IOException, HttpException { - - Scope localScope = clientContext.getAttribute(ACTIVE_SPAN, Scope.class); - CloseableHttpResponse response = null; - try { - if (localScope == null) { - localScope = createLocalScope(request, clientContext); - } - - return (response = createNetworkSpan(localScope, route, request, clientContext, execAware)); - } catch (final Exception e) { - localScope.close(); - throw e; - } finally { - if (response != null) { - /** - * This exec runs after {@link org.apache.http.impl.execchain.RedirectExec} which loops - * until there is no redirect or reaches max redirect count. {@link RedirectStrategy} is - * used to decide whether localScope should be finished or not. If there is a redirect - * localScope is not finished and redirect is logged. - */ - Integer redirectCount = clientContext.getAttribute(REDIRECT_COUNT, Integer.class); - if (!redirectHandlingDisabled - && clientContext.getRequestConfig().isRedirectsEnabled() - && redirectStrategy.isRedirected(request, response, clientContext) - && ++redirectCount < clientContext.getRequestConfig().getMaxRedirects()) { - clientContext.setAttribute(REDIRECT_COUNT, redirectCount); - } else { - localScope.close(); - } - } - } - } - - private Scope createLocalScope( - final HttpRequest httpRequest, final HttpClientContext clientContext) { - final Tracer.SpanBuilder spanBuilder = - tracer.buildSpan(OPERATION_NAME).withTag(Tags.COMPONENT.getKey(), COMPONENT_NAME); - - final Scope scope = spanBuilder.startActive(true); - clientContext.setAttribute(ACTIVE_SPAN, scope); - clientContext.setAttribute(REDIRECT_COUNT, 0); - return scope; - } - - private CloseableHttpResponse createNetworkSpan( - final Scope parentScope, - final HttpRoute route, - final HttpRequestWrapper request, - final HttpClientContext clientContext, - final HttpExecutionAware execAware) - throws IOException, HttpException { - Scope networkScope = null; - Span networkSpan = null; + Scope scope = null; + Span span = null; try { // This handlers runs untrapped in the client code // so we must ensure any unexpected agent errors are caught. try { - networkScope = + scope = tracer .buildSpan(OPERATION_NAME) .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT) .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT) - .asChildOf(parentScope.span()) .startActive(true); - networkSpan = networkScope.span(); + span = scope.span(); final boolean awsClientCall = request.getHeaders("amz-sdk-invocation-id").length > 0; // AWS calls are often signed, so we can't add headers without breaking the signature. if (!awsClientCall) { tracer.inject( - networkSpan.context(), - Format.Builtin.HTTP_HEADERS, - new HttpHeadersInjectAdapter(request)); + span.context(), Format.Builtin.HTTP_HEADERS, new HttpHeadersInjectAdapter(request)); } // request tags - Tags.HTTP_METHOD.set(networkSpan, request.getRequestLine().getMethod()); - Tags.HTTP_URL.set(networkSpan, request.getRequestLine().getUri()); + Tags.HTTP_METHOD.set(span, request.getRequestLine().getMethod()); + Tags.HTTP_URL.set(span, request.getRequestLine().getUri()); final URI uri = request.getURI(); // zuul users have encountered cases where getURI returns null if (null != uri) { - Tags.PEER_PORT.set(networkSpan, uri.getPort() == -1 ? 80 : uri.getPort()); - Tags.PEER_HOSTNAME.set(networkSpan, uri.getHost()); + Tags.PEER_PORT.set(span, uri.getPort() == -1 ? 80 : uri.getPort()); + Tags.PEER_HOSTNAME.set(span, uri.getHost()); } } catch (final Exception e) { - log.debug("failed to create network span", e); + log.debug("failed to create span", e); } final CloseableHttpResponse response = @@ -165,23 +89,23 @@ public class DDTracingClientExec implements ClientExecChain { try { // response tags - if (null != networkSpan) { - Tags.HTTP_STATUS.set(networkSpan, response.getStatusLine().getStatusCode()); + if (null != span) { + Tags.HTTP_STATUS.set(span, response.getStatusLine().getStatusCode()); } } catch (final Exception e) { - log.debug("failed to set network span status", e); + log.debug("failed to set span status", e); } return response; - } catch (IOException | HttpException | RuntimeException e) { + } catch (final IOException | HttpException | RuntimeException e) { // error tags - Tags.ERROR.set(networkSpan, Boolean.TRUE); - networkSpan.log(Collections.singletonMap(ERROR_OBJECT, e)); + Tags.ERROR.set(span, Boolean.TRUE); + span.log(Collections.singletonMap(ERROR_OBJECT, e)); throw e; } finally { - if (null != networkScope) { - networkScope.close(); + if (null != scope) { + scope.close(); } } } diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/test/groovy/ApacheHttpClientTest.groovy b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/test/groovy/ApacheHttpClientTest.groovy index 5fbc1d680f..a93d2aa128 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/test/groovy/ApacheHttpClientTest.groovy +++ b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/test/groovy/ApacheHttpClientTest.groovy @@ -1,3 +1,4 @@ +import datadog.opentracing.DDSpan import datadog.trace.agent.test.AgentTestRunner import datadog.trace.agent.test.ListWriterAssert import datadog.trace.agent.test.RatpackUtils @@ -6,6 +7,7 @@ import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import io.opentracing.tag.Tags import org.apache.http.HttpResponse +import org.apache.http.client.ClientProtocolException import org.apache.http.client.HttpClient import org.apache.http.client.config.RequestConfig import org.apache.http.client.methods.HttpGet @@ -14,7 +16,6 @@ import org.apache.http.message.BasicHeader import spock.lang.Shared import static datadog.trace.agent.test.ListWriterAssert.assertTraces -import static datadog.trace.agent.test.TestUtils.runUnderTrace import static ratpack.groovy.test.embed.GroovyEmbeddedApp.ratpack class ApacheHttpClientTest extends AgentTestRunner { @@ -37,6 +38,13 @@ class ApacheHttpClientTest extends AgentTestRunner { redirect(server.address.resolve("/success").toURL().toString()) } } + prefix("another-redirect") { + get { + RatpackUtils.handleDistributedRequest(context) + + redirect(server.address.resolve("/redirect").toURL().toString()) + } + } } } @Shared @@ -45,25 +53,24 @@ class ApacheHttpClientTest extends AgentTestRunner { def successUrl = server.address.resolve("/success") @Shared def redirectUrl = server.address.resolve("/redirect") + @Shared + def twoRedirectsUrl = server.address.resolve("/another-redirect") final HttpClientBuilder builder = HttpClientBuilder.create() final HttpClient client = builder.build() def "trace request with propagation"() { - setup: - runUnderTrace("someTrace") { - HttpResponse response = client.execute(new HttpGet(successUrl)) - assert response.getStatusLine().getStatusCode() == 200 - } + when: + HttpResponse response = client.execute(new HttpGet(successUrl)) - expect: + then: + response.getStatusLine().getStatusCode() == 200 // one trace on the server, one trace on the client assertTraces(TEST_WRITER, 2) { - serverTrace(it, 0, TEST_WRITER[1][2]) - trace(1, 3) { - outerSpan(it, 0) - clientParentSpan(it, 1, span(0)) - successClientSpan(it, 2, span(1)) + serverTrace(it, 0, TEST_WRITER[1][1]) + trace(1, 2) { + clientParentSpan(it, 0) + successClientSpan(it, 1, span(0)) } } } @@ -72,23 +79,23 @@ class ApacheHttpClientTest extends AgentTestRunner { setup: final RequestConfig.Builder requestConfigBuilder = new RequestConfig.Builder() requestConfigBuilder.setMaxRedirects(10) - runUnderTrace("someTrace") { - HttpGet request = new HttpGet(redirectUrl) - request.setConfig(requestConfigBuilder.build()) - HttpResponse response = client.execute(request) - assert response.getStatusLine().getStatusCode() == 200 - } - expect: + HttpGet request = new HttpGet(redirectUrl) + request.setConfig(requestConfigBuilder.build()) + + when: + HttpResponse response = client.execute(request) + + then: + response.getStatusLine().getStatusCode() == 200 // two traces on the server, one trace on the client assertTraces(TEST_WRITER, 3) { - serverTrace(it, 0, TEST_WRITER[2][3]) - serverTrace(it, 1, TEST_WRITER[2][2]) - trace(2, 4) { - outerSpan(it, 0) - clientParentSpan(it, 1, span(0)) - successClientSpan(it, 2, span(1)) - redirectClientSpan(it, 3, span(1)) + serverTrace(it, 0, TEST_WRITER[2][2]) + serverTrace(it, 1, TEST_WRITER[2][1]) + trace(2, 3) { + clientParentSpan(it, 0) + successClientSpan(it, 1, span(0)) + redirectClientSpan(it, 2, span(0)) } } } @@ -97,49 +104,71 @@ class ApacheHttpClientTest extends AgentTestRunner { setup: final RequestConfig.Builder requestConfigBuilder = new RequestConfig.Builder() requestConfigBuilder.setMaxRedirects(1) - runUnderTrace("someTrace") { - HttpGet request = new HttpGet(redirectUrl) - request.setConfig(requestConfigBuilder.build()) - HttpResponse response = client.execute(request) - assert response.getStatusLine().getStatusCode() == 200 - } + HttpGet request = new HttpGet(redirectUrl) + request.setConfig(requestConfigBuilder.build()) - expect: - print(TEST_WRITER) + when: + HttpResponse response = client.execute(request) + + then: + response.getStatusLine().getStatusCode() == 200 // two traces on the server, one trace on the client assertTraces(TEST_WRITER, 3) { - serverTrace(it, 0, TEST_WRITER[2][3]) + serverTrace(it, 0, TEST_WRITER[2][2]) serverTrace(it, 1, TEST_WRITER[2][1]) - trace(2, 4) { - outerSpan(it, 0) - // Note: this is kind of an weird order? - successClientSpan(it, 1, span(2)) - clientParentSpan(it, 2, span(0)) - redirectClientSpan(it, 3, span(2)) + trace(2, 3) { + clientParentSpan(it, 0) + successClientSpan(it, 1, span(0)) + redirectClientSpan(it, 2, span(0)) + } + } + } + + def "trace redirected request with propagation too many redirects"() { + setup: + final RequestConfig.Builder requestConfigBuilder = new RequestConfig.Builder() + requestConfigBuilder.setMaxRedirects(1) + + HttpGet request = new HttpGet(twoRedirectsUrl) + request.setConfig(requestConfigBuilder.build()) + + when: + client.execute(request) + + then: + def exception = thrown(ClientProtocolException) + // two traces on the server, one trace on the client + assertTraces(TEST_WRITER, 3) { + serverTrace(it, 0, TEST_WRITER[2][2]) + serverTrace(it, 1, TEST_WRITER[2][1]) + trace(2, 3) { + clientParentSpan(it, 0, exception) + redirectClientSpan(it, 1, span(0)) + redirectClientSpan(it, 2, span(0), "another-redirect") } } } def "trace request without propagation"() { setup: - runUnderTrace("someTrace") { - HttpGet request = new HttpGet(successUrl) - request.addHeader(new BasicHeader("is-dd-server", "false")) - HttpResponse response = client.execute(request) - assert response.getStatusLine().getStatusCode() == 200 - } - expect: + HttpGet request = new HttpGet(successUrl) + request.addHeader(new BasicHeader("is-dd-server", "false")) + + when: + HttpResponse response = client.execute(request) + + then: + response.getStatusLine().getStatusCode() == 200 // only one trace (client). assertTraces(TEST_WRITER, 1) { - trace(0, 3) { - outerSpan(it, 0) - clientParentSpan(it, 1, span(0)) - successClientSpan(it, 2, span(1)) + trace(0, 2) { + clientParentSpan(it, 0) + successClientSpan(it, 1, span(0)) } } } - def serverTrace(ListWriterAssert writer, index, parent) { + def serverTrace(ListWriterAssert writer, int index, DDSpan parent) { writer.trace(index, 1) { span(0) { childOf parent @@ -154,34 +183,24 @@ class ApacheHttpClientTest extends AgentTestRunner { } } - def outerSpan(TraceAssert trace, index) { + def clientParentSpan(TraceAssert trace, int index, Throwable exception = null) { trace.span(index) { parent() serviceName "unnamed-java-app" - operationName "someTrace" - resourceName "someTrace" - errored false - tags { - defaultTags() - } - } - } - - def clientParentSpan(TraceAssert trace, index, parent) { - trace.span(index) { - childOf parent - serviceName "unnamed-java-app" operationName "apache.http" resourceName "apache.http" - errored false + errored exception != null tags { defaultTags() "$Tags.COMPONENT.key" "apache-httpclient" + if (exception) { + errorTags(exception.class) + } } } } - def successClientSpan(TraceAssert trace, index, parent, status = 200, route = "success") { + def successClientSpan(TraceAssert trace, int index, DDSpan parent, status = 200, route = "success") { trace.span(index) { childOf parent serviceName "unnamed-java-app" @@ -201,7 +220,7 @@ class ApacheHttpClientTest extends AgentTestRunner { } } - def redirectClientSpan(TraceAssert trace, index, parent) { - successClientSpan(trace, index, parent, 302, "redirect") + def redirectClientSpan(TraceAssert trace, int index, DDSpan parent, route = "redirect") { + successClientSpan(trace, index, parent, 302, route) } } From 6bc1d1ab8ef49b6a6efd5d5e3a20b5f74021e03c Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 30 Jul 2018 16:20:55 -0400 Subject: [PATCH 06/12] Implement `saveHasSuperType` and use it instead of `hasSuperType` The idea is to just 'trim' type hierarchy 'up-trees' that we cannot resolve dring instrumentation instead of failing to instrument completely. --- .../tooling/ByteBuddyElementMatchers.java | 132 ++++++++++++++++++ .../ApacheHttpClientInstrumentation.java | 4 +- ...search2TransportClientInstrumentation.java | 2 +- ...search5TransportClientInstrumentation.java | 2 +- ...search6TransportClientInstrumentation.java | 2 +- .../HystrixCommandInstrumentation.java | 4 +- .../concurrent/ExecutorInstrumentation.java | 4 +- .../concurrent/FutureInstrumentation.java | 4 +- .../JaxRsAnnotationsInstrumentation.java | 11 +- .../jaxrs/JaxRsClientInstrumentation.java | 6 +- .../jetty8/HandlerInstrumentation.java | 4 +- .../JMS1MessageConsumerInstrumentation.java | 6 +- .../JMS1MessageListenerInstrumentation.java | 4 +- .../JMS1MessageProducerInstrumentation.java | 4 +- .../JMS2MessageConsumerInstrumentation.java | 4 +- .../JMS2MessageListenerInstrumentation.java | 4 +- .../JMS2MessageProducerInstrumentation.java | 4 +- .../jsp/JSPInstrumentation.java | 10 +- .../NettyChannelPipelineInstrumentation.java | 8 +- .../NettyChannelPipelineInstrumentation.java | 8 +- .../play/PlayInstrumentation.java | 4 +- .../RatpackHttpClientInstrumentation.java | 6 +- .../ratpack/RatpackInstrumentation.java | 6 +- .../servlet2/FilterChain2Instrumentation.java | 4 +- .../servlet2/HttpServlet2Instrumentation.java | 4 +- .../AbstractServlet3Instrumentation.java | 4 +- .../servlet3/FilterChain3Instrumentation.java | 4 +- .../servlet3/HttpServlet3Instrumentation.java | 4 +- .../springweb/SpringWebInstrumentation.java | 4 +- .../TraceAnnotationsInstrumentation.java | 6 +- .../TraceConfigInstrumentation.java | 10 +- 31 files changed, 208 insertions(+), 75 deletions(-) create mode 100644 dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java 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 new file mode 100644 index 0000000000..c934ee1e64 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java @@ -0,0 +1,132 @@ +package datadog.trace.agent.tooling; + +import static net.bytebuddy.matcher.ElementMatchers.erasure; + +import java.util.HashSet; +import java.util.Set; +import lombok.extern.slf4j.Slf4j; +import net.bytebuddy.build.HashCodeAndEqualsPlugin; +import net.bytebuddy.description.type.TypeDefinition; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.description.type.TypeList; +import net.bytebuddy.matcher.ElementMatcher; +import net.bytebuddy.matcher.ElementMatchers; + +/** + * This class provides some custom ByteBuddy element matchers to use when applying instrumentation + */ +@Slf4j +public class ByteBuddyElementMatchers { + + /** + * Matches any type description that declares a super type that matches the provided matcher. + * Exceptions during matching process are logged and ignored. + * + * @param matcher The type to be checked for being a super type of the matched type. + * @param The type of the matched object. + * @return A matcher that matches any type description that declares a super type that matches the + * provided matcher. + * @see ElementMatchers#hasSuperType(net.bytebuddy.matcher.ElementMatcher) + */ + public static ElementMatcher.Junction safeHasSuperType( + final ElementMatcher matcher) { + return safeHasGenericSuperType(erasure(matcher)); + } + + /** + * Matches any type description that declares a super type that matches the provided matcher. + * Exceptions during matching process are logged and ignored. + * + * @param matcher The type to be checked for being a super type of the matched type. + * @param The type of the matched object. + * @return A matcher that matches any type description that declares a super type that matches the + * provided matcher. + * @see ElementMatchers#hasGenericSuperType(net.bytebuddy.matcher.ElementMatcher) + */ + public static ElementMatcher.Junction safeHasGenericSuperType( + final ElementMatcher matcher) { + return new SafeHasSuperTypeMatcher<>(matcher); + } + + /** + * An element matcher that matches a super type. Exceptions during matching process are logged and + * ignored. + * + * @param The type of the matched entity. + * @see net.bytebuddy.matcher.HasSuperTypeMatcher + */ + @HashCodeAndEqualsPlugin.Enhance + public static class SafeHasSuperTypeMatcher + extends ElementMatcher.Junction.AbstractBase { + + /** The matcher to apply to any super type of the matched type. */ + private final ElementMatcher matcher; + + /** + * Creates a new matcher for a super type. + * + * @param matcher The matcher to apply to any super type of the matched type. + */ + public SafeHasSuperTypeMatcher(final ElementMatcher matcher) { + this.matcher = matcher; + } + + @Override + public boolean matches(final T target) { + final Set checkedInterfaces = new HashSet<>(); + // We do not use foreach loop and iterator interface here because we need to catch exceptions + // in {@code getSuperClass} calls + TypeDefinition typeDefinition = target; + while (typeDefinition != null) { + if (matcher.matches(typeDefinition.asGenericType()) + || hasInterface(typeDefinition, checkedInterfaces)) { + return true; + } + typeDefinition = safeGetSuperClass(typeDefinition); + } + return false; + } + + private TypeDefinition safeGetSuperClass(final TypeDefinition typeDefinition) { + try { + return typeDefinition.getSuperClass(); + } catch (final Exception e) { + log.info("Exception trying to get next type definition:", e); + return null; + } + } + + /** + * Matches a type's interfaces against the provided matcher. + * + * @param typeDefinition The type for which to check all implemented interfaces. + * @param checkedInterfaces The interfaces that have already been checked. + * @return {@code true} if any interface matches the supplied matcher. + */ + private boolean hasInterface( + final TypeDefinition typeDefinition, final Set checkedInterfaces) { + for (final TypeDefinition interfaceType : safeGetInterfaces(typeDefinition)) { + if (checkedInterfaces.add(interfaceType.asErasure()) + && (matcher.matches(interfaceType.asGenericType()) + || hasInterface(interfaceType, checkedInterfaces))) { + return true; + } + } + return false; + } + + private TypeList.Generic safeGetInterfaces(final TypeDefinition typeDefinition) { + try { + return typeDefinition.getInterfaces(); + } catch (final Exception e) { + log.info("Exception trying to get interfaces:", e); + return new TypeList.Generic.Empty(); + } + } + + @Override + public String toString() { + return "safeHasSuperType(" + matcher + ")"; + } + } +} diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java index f3e05c6ed7..23d71954b6 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java @@ -1,8 +1,8 @@ package datadog.trace.instrumentation.apachehttpclient; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isAbstract; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -32,7 +32,7 @@ public class ApacheHttpClientInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { return named("org.apache.http.impl.client.HttpClientBuilder") - .or(hasSuperType(named("org.apache.http.impl.client.CloseableHttpClient"))); + .or(safeHasSuperType(named("org.apache.http.impl.client.CloseableHttpClient"))); } @Override diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/Elasticsearch2TransportClientInstrumentation.java b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/Elasticsearch2TransportClientInstrumentation.java index 7ae20d78b7..b607f603bd 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/Elasticsearch2TransportClientInstrumentation.java +++ b/dd-java-agent/instrumentation/elasticsearch-transport-2/src/main/java/datadog/trace/instrumentation/elasticsearch2/Elasticsearch2TransportClientInstrumentation.java @@ -41,7 +41,7 @@ public class Elasticsearch2TransportClientInstrumentation extends Instrumenter.D @Override public ElementMatcher typeMatcher() { // If we want to be more generic, we could instrument the interface instead: - // .and(hasSuperType(named("org.elasticsearch.client.ElasticsearchClient")))) + // .and(safeHasSuperType(named("org.elasticsearch.client.ElasticsearchClient")))) return not(isInterface()).and(named("org.elasticsearch.client.support.AbstractClient")); } diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/Elasticsearch5TransportClientInstrumentation.java b/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/Elasticsearch5TransportClientInstrumentation.java index 22401d276a..f138adfaa2 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/Elasticsearch5TransportClientInstrumentation.java +++ b/dd-java-agent/instrumentation/elasticsearch-transport-5/src/main/java/datadog/trace/instrumentation/elasticsearch5/Elasticsearch5TransportClientInstrumentation.java @@ -41,7 +41,7 @@ public class Elasticsearch5TransportClientInstrumentation extends Instrumenter.D @Override public ElementMatcher typeMatcher() { // If we want to be more generic, we could instrument the interface instead: - // .and(hasSuperType(named("org.elasticsearch.client.ElasticsearchClient")))) + // .and(safeHasSuperType(named("org.elasticsearch.client.ElasticsearchClient")))) return not(isInterface()).and(named("org.elasticsearch.client.support.AbstractClient")); } diff --git a/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/Elasticsearch6TransportClientInstrumentation.java b/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/Elasticsearch6TransportClientInstrumentation.java index 27bf0c158f..6f9b925941 100644 --- a/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/Elasticsearch6TransportClientInstrumentation.java +++ b/dd-java-agent/instrumentation/elasticsearch-transport-6/src/main/java/datadog/trace/instrumentation/elasticsearch6/Elasticsearch6TransportClientInstrumentation.java @@ -45,7 +45,7 @@ public class Elasticsearch6TransportClientInstrumentation extends Instrumenter.D @Override public ElementMatcher typeMatcher() { // If we want to be more generic, we could instrument the interface instead: - // .and(hasSuperType(named("org.elasticsearch.client.ElasticsearchClient")))) + // .and(safeHasSuperType(named("org.elasticsearch.client.ElasticsearchClient")))) return not(isInterface()).and(named("org.elasticsearch.client.support.AbstractClient")); } diff --git a/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixCommandInstrumentation.java b/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixCommandInstrumentation.java index bb723c565b..0514e4c951 100644 --- a/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixCommandInstrumentation.java +++ b/dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixCommandInstrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.hystrix; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -31,7 +31,7 @@ public class HystrixCommandInstrumentation extends Instrumenter.Default { public ElementMatcher typeMatcher() { // Not adding a version restriction because this should work with any version and add some // benefit. - return not(isInterface()).and(hasSuperType(named("com.netflix.hystrix.HystrixCommand"))); + return not(isInterface()).and(safeHasSuperType(named("com.netflix.hystrix.HystrixCommand"))); } @Override diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/ExecutorInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/ExecutorInstrumentation.java index 4bc99e0463..c7e6fff610 100644 --- a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/ExecutorInstrumentation.java +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/ExecutorInstrumentation.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.java.concurrent; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.nameMatches; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -95,7 +95,7 @@ public final class ExecutorInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { return not(isInterface()) - .and(hasSuperType(named(Executor.class.getName()))) + .and(safeHasSuperType(named(Executor.class.getName()))) .and( new ElementMatcher() { @Override diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java index 3f6a4a0504..4c56a7e6cf 100644 --- a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.java.concurrent; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; @@ -70,7 +70,7 @@ public final class FutureInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { return not(isInterface()) - .and(hasSuperType(named(Future.class.getName()))) + .and(safeHasSuperType(named(Future.class.getName()))) .and( new ElementMatcher() { @Override diff --git a/dd-java-agent/instrumentation/jax-rs-annotations/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsAnnotationsInstrumentation.java b/dd-java-agent/instrumentation/jax-rs-annotations/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsAnnotationsInstrumentation.java index 8ae0bd7de8..ee4755ece1 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsAnnotationsInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs-annotations/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsAnnotationsInstrumentation.java @@ -1,8 +1,7 @@ package datadog.trace.instrumentation.jaxrs; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static net.bytebuddy.matcher.ElementMatchers.declaresMethod; -import static net.bytebuddy.matcher.ElementMatchers.failSafe; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -31,16 +30,14 @@ public final class JaxRsAnnotationsInstrumentation extends Instrumenter.Default @Override public ElementMatcher typeMatcher() { - return hasSuperType( + return safeHasSuperType( isAnnotatedWith(named("javax.ws.rs.Path")) - .or( - failSafe( - hasSuperType(declaresMethod(isAnnotatedWith(named("javax.ws.rs.Path"))))))); + .or(safeHasSuperType(declaresMethod(isAnnotatedWith(named("javax.ws.rs.Path")))))); } @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( isAnnotatedWith( named("javax.ws.rs.Path") diff --git a/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java b/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java index c896aa1c64..e0025d06df 100644 --- a/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.jaxrs; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.returns; @@ -21,7 +21,7 @@ public final class JaxRsClientInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return hasSuperType(named("javax.ws.rs.client.ClientBuilder")); + return safeHasSuperType(named("javax.ws.rs.client.ClientBuilder")); } @Override @@ -37,7 +37,7 @@ public final class JaxRsClientInstrumentation extends Instrumenter.Default { public Map transformers() { final Map transformers = new HashMap<>(); transformers.put( - named("build").and(returns(hasSuperType(named("javax.ws.rs.client.Client")))), + named("build").and(returns(safeHasSuperType(named("javax.ws.rs.client.Client")))), ClientBuilderAdvice.class.getName()); return transformers; } diff --git a/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/HandlerInstrumentation.java b/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/HandlerInstrumentation.java index f1cd07214c..f829f23400 100644 --- a/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/HandlerInstrumentation.java +++ b/dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/HandlerInstrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.jetty8; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -30,7 +30,7 @@ public final class HandlerInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { return not(isInterface()) - .and(hasSuperType(named("org.eclipse.jetty.server.Handler"))) + .and(safeHasSuperType(named("org.eclipse.jetty.server.Handler"))) .and(not(named("org.eclipse.jetty.server.handler.HandlerWrapper"))); } diff --git a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageConsumerInstrumentation.java b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageConsumerInstrumentation.java index c9bc5896ac..41339cc8f5 100644 --- a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageConsumerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageConsumerInstrumentation.java @@ -1,9 +1,9 @@ package datadog.trace.instrumentation.jms1; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -47,7 +47,7 @@ public final class JMS1MessageConsumerInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.jms.MessageConsumer"))); + return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageConsumer"))); } @Override @@ -62,7 +62,7 @@ public final class JMS1MessageConsumerInstrumentation extends Instrumenter.Defau @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( named("receive").and(takesArguments(0).or(takesArguments(1))).and(isPublic()), ConsumerAdvice.class.getName()); diff --git a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java index 21f84f6b6b..2ff94ef531 100644 --- a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java @@ -1,9 +1,9 @@ package datadog.trace.instrumentation.jms1; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -39,7 +39,7 @@ public final class JMS1MessageListenerInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.jms.MessageListener"))); + return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageListener"))); } @Override diff --git a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java index 97efb4d3f2..c4f54b22e8 100644 --- a/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java @@ -1,9 +1,9 @@ package datadog.trace.instrumentation.jms1; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -39,7 +39,7 @@ public final class JMS1MessageProducerInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.jms.MessageProducer"))); + return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageProducer"))); } @Override diff --git a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java index e0dca6ef9d..d48b050397 100644 --- a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java @@ -1,9 +1,9 @@ package datadog.trace.instrumentation.jms2; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -46,7 +46,7 @@ public final class JMS2MessageConsumerInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.jms.MessageConsumer"))); + return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageConsumer"))); } @Override diff --git a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java index 2ccfe83f47..ceae43d9d3 100644 --- a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java @@ -1,9 +1,9 @@ package datadog.trace.instrumentation.jms2; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -39,7 +39,7 @@ public final class JMS2MessageListenerInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.jms.MessageListener"))); + return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageListener"))); } @Override diff --git a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java index 01546ab226..6a8f12e0a0 100644 --- a/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java +++ b/dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java @@ -1,9 +1,9 @@ package datadog.trace.instrumentation.jms2; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -39,7 +39,7 @@ public final class JMS2MessageProducerInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.jms.MessageProducer"))); + return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageProducer"))); } @Override diff --git a/dd-java-agent/instrumentation/jsp-2.3/src/main/java/datadog/trace/instrumentation/jsp/JSPInstrumentation.java b/dd-java-agent/instrumentation/jsp-2.3/src/main/java/datadog/trace/instrumentation/jsp/JSPInstrumentation.java index ed11deeb25..41daeedabd 100644 --- a/dd-java-agent/instrumentation/jsp-2.3/src/main/java/datadog/trace/instrumentation/jsp/JSPInstrumentation.java +++ b/dd-java-agent/instrumentation/jsp-2.3/src/main/java/datadog/trace/instrumentation/jsp/JSPInstrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.jsp; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -39,12 +39,12 @@ public final class JSPInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.servlet.jsp.HttpJspPage"))); + return not(isInterface()).and(safeHasSuperType(named("javax.servlet.jsp.HttpJspPage"))); } @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( named("_jspService") .and(takesArgument(0, named("javax.servlet.http.HttpServletRequest"))) @@ -68,14 +68,14 @@ public final class JSPInstrumentation extends Instrumenter.Default { final Span span = scope.span(); // get the JSP file name being rendered in an include action - Object includeServletPath = req.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); + final Object includeServletPath = req.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); String resourceName = req.getServletPath(); if (includeServletPath != null && includeServletPath instanceof String) { resourceName = includeServletPath.toString(); } span.setTag(DDTags.RESOURCE_NAME, resourceName); - Object forwardOrigin = req.getAttribute(RequestDispatcher.FORWARD_SERVLET_PATH); + final Object forwardOrigin = req.getAttribute(RequestDispatcher.FORWARD_SERVLET_PATH); if (forwardOrigin != null && forwardOrigin instanceof String) { span.setTag("jsp.forwardOrigin", forwardOrigin.toString()); } diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java index c64986f48b..5cb8977ef3 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/NettyChannelPipelineInstrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.netty40; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; @@ -48,7 +48,7 @@ public class NettyChannelPipelineInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("io.netty.channel.ChannelPipeline"))); + return not(isInterface()).and(safeHasSuperType(named("io.netty.channel.ChannelPipeline"))); } @Override @@ -99,7 +99,9 @@ public class NettyChannelPipelineInstrumentation extends Instrumenter.Default { @Advice.Enter final int depth, @Advice.This final ChannelPipeline pipeline, @Advice.Argument(2) final ChannelHandler handler) { - if (depth > 0) return; + if (depth > 0) { + return; + } try { // Server pipeline handlers diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java index 7677f2addb..3ae8fd5024 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/NettyChannelPipelineInstrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.netty41; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; @@ -48,7 +48,7 @@ public class NettyChannelPipelineInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("io.netty.channel.ChannelPipeline"))); + return not(isInterface()).and(safeHasSuperType(named("io.netty.channel.ChannelPipeline"))); } @Override @@ -99,7 +99,9 @@ public class NettyChannelPipelineInstrumentation extends Instrumenter.Default { @Advice.Enter final int depth, @Advice.This final ChannelPipeline pipeline, @Advice.Argument(2) final ChannelHandler handler) { - if (depth > 0) return; + if (depth > 0) { + return; + } try { // Server pipeline handlers diff --git a/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java b/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java index 801fa19940..7f72c276e1 100644 --- a/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java +++ b/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java @@ -1,9 +1,9 @@ package datadog.trace.instrumentation.play; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClassWithMethod; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -46,7 +46,7 @@ public final class PlayInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return hasSuperType(named("play.api.mvc.Action")); + return safeHasSuperType(named("play.api.mvc.Action")); } @Override diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackHttpClientInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackHttpClientInstrumentation.java index b62f4ca880..5ac3701acc 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackHttpClientInstrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.ratpack; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.instrumentation.ratpack.RatpackInstrumentation.CLASSLOADER_CONTAINS_RATPACK_1_4_OR_ABOVE; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; @@ -35,7 +35,7 @@ public final class RatpackHttpClientInstrumentation extends Instrumenter.Default @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("ratpack.http.client.HttpClient"))); + return not(isInterface()).and(safeHasSuperType(named("ratpack.http.client.HttpClient"))); } @Override @@ -63,7 +63,7 @@ public final class RatpackHttpClientInstrumentation extends Instrumenter.Default @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( named("request") .and( diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackInstrumentation.java index da4c15bc74..466ba6b732 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackInstrumentation.java +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackInstrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.ratpack; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClassWithMethod; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isStatic; @@ -91,7 +91,7 @@ public final class RatpackInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("ratpack.exec.ExecStarter"))); + return not(isInterface()).and(safeHasSuperType(named("ratpack.exec.ExecStarter"))); } @Override @@ -136,7 +136,7 @@ public final class RatpackInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { return named("ratpack.exec.Execution") - .or(not(isInterface()).and(hasSuperType(named("ratpack.exec.Execution")))); + .or(not(isInterface()).and(safeHasSuperType(named("ratpack.exec.Execution")))); } @Override diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java index 64f8103a64..37e5b47728 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/FilterChain2Instrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.servlet2; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -23,7 +23,7 @@ public final class FilterChain2Instrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.servlet.FilterChain"))); + return not(isInterface()).and(safeHasSuperType(named("javax.servlet.FilterChain"))); } @Override diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java index f58c348ce4..b294e5ffd1 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServlet2Instrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.servlet2; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isProtected; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -28,7 +28,7 @@ public final class HttpServlet2Instrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.servlet.http.HttpServlet"))); + return not(isInterface()).and(safeHasSuperType(named("javax.servlet.http.HttpServlet"))); } @Override diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java index e7c6bc0198..8f3de24845 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.servlet3; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -21,7 +21,7 @@ public class AbstractServlet3Instrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.servlet.FilterChain"))); + return not(isInterface()).and(safeHasSuperType(named("javax.servlet.FilterChain"))); } @Override diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java index 15e6c236f1..41b6e537c9 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.servlet3; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -18,7 +18,7 @@ public final class FilterChain3Instrumentation extends AbstractServlet3Instrumen @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.servlet.FilterChain"))); + return not(isInterface()).and(safeHasSuperType(named("javax.servlet.FilterChain"))); } @Override diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java index 61f2b1288e..b6d72be6c4 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.servlet3; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isProtected; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -18,7 +18,7 @@ public final class HttpServlet3Instrumentation extends AbstractServlet3Instrumen @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(hasSuperType(named("javax.servlet.http.HttpServlet"))); + return not(isInterface()).and(safeHasSuperType(named("javax.servlet.http.HttpServlet"))); } @Override diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java index 632a6e3742..d0ad15571f 100644 --- a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/SpringWebInstrumentation.java @@ -1,8 +1,8 @@ package datadog.trace.instrumentation.springweb; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClassWithField; import static io.opentracing.log.Fields.ERROR_OBJECT; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isProtected; @@ -38,7 +38,7 @@ public final class SpringWebInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { return not(isInterface()) - .and(hasSuperType(named("org.springframework.web.servlet.HandlerAdapter"))); + .and(safeHasSuperType(named("org.springframework.web.servlet.HandlerAdapter"))); } @Override diff --git a/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAnnotationsInstrumentation.java b/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAnnotationsInstrumentation.java index 3d77dfe3c4..965e05f520 100644 --- a/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAnnotationsInstrumentation.java +++ b/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAnnotationsInstrumentation.java @@ -1,8 +1,8 @@ package datadog.trace.instrumentation.trace_annotation; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.instrumentation.trace_annotation.TraceConfigInstrumentation.PACKAGE_CLASS_NAME_REGEX; import static net.bytebuddy.matcher.ElementMatchers.declaresMethod; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.is; import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -79,12 +79,12 @@ public final class TraceAnnotationsInstrumentation extends Instrumenter.Default @Override public ElementMatcher typeMatcher() { - return hasSuperType(declaresMethod(isAnnotatedWith(methodTraceMatcher))); + return safeHasSuperType(declaresMethod(isAnnotatedWith(methodTraceMatcher))); } @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put(isAnnotatedWith(methodTraceMatcher), TraceAdvice.class.getName()); return transformers; } diff --git a/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceConfigInstrumentation.java b/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceConfigInstrumentation.java index 79744ec1c4..02e71c8171 100644 --- a/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceConfigInstrumentation.java +++ b/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceConfigInstrumentation.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.trace_annotation; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static net.bytebuddy.matcher.ElementMatchers.named; import com.google.auto.service.AutoService; @@ -92,7 +92,7 @@ public class TraceConfigInstrumentation implements Instrumenter { } for (final Map.Entry> entry : classMethodsToTrace.entrySet()) { - TracerClassInstrumentation tracerConfigClass = + final TracerClassInstrumentation tracerConfigClass = new TracerClassInstrumentation(entry.getKey(), entry.getValue()); agentBuilder = tracerConfigClass.instrument(agentBuilder); } @@ -109,7 +109,7 @@ public class TraceConfigInstrumentation implements Instrumenter { this("noop", Collections.singleton("noop")); } - public TracerClassInstrumentation(String className, Set methodNames) { + public TracerClassInstrumentation(final String className, final Set methodNames) { super("trace", "trace-config"); this.className = className; this.methodNames = methodNames; @@ -117,7 +117,7 @@ public class TraceConfigInstrumentation implements Instrumenter { @Override public ElementMatcher typeMatcher() { - return hasSuperType(named(className)); + return safeHasSuperType(named(className)); } @Override @@ -131,7 +131,7 @@ public class TraceConfigInstrumentation implements Instrumenter { } } - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put(methodMatchers, TraceAdvice.class.getName()); return transformers; } From 6693a93485e2a307ee6686a796e2c10550162f4c Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 30 Jul 2018 16:23:07 -0400 Subject: [PATCH 07/12] Remove onInstrumentationError from LagomTest We not longer need it since our instrumentation can handle underlying loading problem. --- .../src/lagomTest/groovy/LagomTest.groovy | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy b/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy index 533965ccf8..0ac6d5e4a5 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy @@ -6,7 +6,6 @@ import datadog.trace.agent.test.AgentTestRunner import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags import io.opentracing.tag.Tags -import net.bytebuddy.utility.JavaModule import play.inject.guice.GuiceApplicationBuilder import spock.lang.Shared @@ -25,20 +24,6 @@ class LagomTest extends AgentTestRunner { @Shared private TestServer server - @Override - protected boolean onInstrumentationError(String typeName, ClassLoader classLoader, JavaModule module, boolean loaded, Throwable throwable) { - if (throwable.getMessage().contains('Cannot resolve type description for akka.stream.impl.VirtualProcessor$WrappedSubscription$$SubscriptionState')) { - // 'akka/stream/impl/VirtualProcessor$WrappedSubscription$PassThrough$.class' declares - // itself an implementation of 'VirtualProcessor$WrappedSubscription$$SubscriptionState', - // but this interface does not exist on the classpath. - // The closest thing on the classpath is 'VirtualProcessor$WrappedSubscription$SubscriptionState' (only one $). - - // Looks like a compiler/packaging issue on akka's end. Or maybe this interface is dynamically generated. - return false - } - return super.onInstrumentationError(typeName, classLoader, module, loaded, throwable) - } - def setupSpec() { server = startServer(defaultSetup() .withCluster(false) From c66bd24d3ad0d9e9b5d1462840bffa43d3382d15 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 30 Jul 2018 16:37:18 -0400 Subject: [PATCH 08/12] Use `safeHasSuperType` instead of `isSubType` `isSubType` may fail on certain class lookup problems, even on classes unrelated to given instrumentation, preventing instrumentation from being applied. --- .../classloaders/ClassLoaderInstrumentation.java | 5 ++--- .../HttpUrlConnectionInstrumentation.java | 12 ++++++------ .../jdbc/ConnectionInstrumentation.java | 6 +++--- .../jdbc/PreparedStatementInstrumentation.java | 5 +++-- .../jdbc/StatementInstrumentation.java | 5 +++-- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/ClassLoaderInstrumentation.java b/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/ClassLoaderInstrumentation.java index 396c5f70b4..a7e99e63b5 100644 --- a/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/ClassLoaderInstrumentation.java +++ b/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/ClassLoaderInstrumentation.java @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.classloaders; import static net.bytebuddy.matcher.ElementMatchers.isConstructor; -import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import com.google.auto.service.AutoService; import datadog.opentracing.DDTracer; @@ -28,12 +27,12 @@ public final class ClassLoaderInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return isSubTypeOf(ClassLoader.class); + return safeHasSuperType(named("java.lang.ClassLoader")); } @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put(isConstructor(), ClassloaderAdvice.class.getName()); return transformers; } diff --git a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java index 1592c77c83..c5e82c4d88 100644 --- a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java +++ b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java @@ -1,9 +1,9 @@ package datadog.trace.instrumentation.http_url_connection; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; @@ -43,7 +43,7 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return isSubTypeOf(HttpURLConnection.class) + return safeHasSuperType(named("java.net.HttpURLConnection")) // This class is a simple delegator. Skip because it does not update its `connected` field. .and(not(named("sun.net.www.protocol.https.HttpsURLConnectionImpl"))); } @@ -58,7 +58,7 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( isMethod() .and(isPublic()) @@ -177,7 +177,7 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { private static final Map STATE_MAP = Collections.synchronizedMap(new WeakHashMap()); - public static HttpURLState get(HttpURLConnection connection) { + public static HttpURLState get(final HttpURLConnection connection) { HttpURLState state = STATE_MAP.get(connection); if (state == null) { // not thread-safe, but neither is HttpURLConnection @@ -195,8 +195,8 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { return hasDoneIO; } - public void setHasDoneIO(boolean value) { - this.hasDoneIO = value; + public void setHasDoneIO(final boolean value) { + hasDoneIO = value; } } } diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java index 25877f74f1..d7f8d5de4d 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java @@ -1,8 +1,9 @@ package datadog.trace.instrumentation.jdbc; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; -import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -10,7 +11,6 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.bootstrap.JDBCMaps; -import java.sql.Connection; import java.sql.PreparedStatement; import java.util.HashMap; import java.util.Map; @@ -26,7 +26,7 @@ public final class ConnectionInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(isSubTypeOf(Connection.class)); + return not(isInterface()).and(safeHasSuperType(named("java.sql.Connection"))); } @Override diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java index 8756e89a44..d15ac53689 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -1,10 +1,11 @@ package datadog.trace.instrumentation.jdbc; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; @@ -36,7 +37,7 @@ public final class PreparedStatementInstrumentation extends Instrumenter.Default @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(isSubTypeOf(PreparedStatement.class)); + return not(isInterface()).and(safeHasSuperType(named("java.sql.PreparedStatement"))); } @Override diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java index dffd5b1354..65795a781e 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java @@ -1,10 +1,11 @@ package datadog.trace.instrumentation.jdbc; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -37,7 +38,7 @@ public final class StatementInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(isSubTypeOf(Statement.class)); + return not(isInterface()).and(safeHasSuperType(named("java.sql.Statement"))); } @Override From 78e6a9c336ae0d364e2d0577ca849b8c293dec92 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 30 Jul 2018 20:40:41 -0400 Subject: [PATCH 09/12] Get rid of DDAdvice Newer ByteBuddy api simplifies things. --- .../datadog/trace/agent/tooling/DDAdvice.java | 33 ------- .../trace/agent/tooling/HelperInjector.java | 7 +- .../trace/agent/tooling/Instrumenter.java | 97 +++++++++++-------- 3 files changed, 62 insertions(+), 75 deletions(-) delete mode 100644 dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/DDAdvice.java diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/DDAdvice.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/DDAdvice.java deleted file mode 100644 index 442e635c36..0000000000 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/DDAdvice.java +++ /dev/null @@ -1,33 +0,0 @@ -package datadog.trace.agent.tooling; - -import net.bytebuddy.agent.builder.AgentBuilder; -import net.bytebuddy.agent.builder.AgentBuilder.LocationStrategy; -import net.bytebuddy.dynamic.ClassFileLocator; - -/** A bytebuddy advice builder with default DataDog settings. */ -public class DDAdvice extends AgentBuilder.Transformer.ForAdvice { - private static final ClassFileLocator AGENT_CLASS_LOCATOR = - ClassFileLocator.ForClassLoader.of(Utils.getAgentClassLoader()); - /** Location strategy for resolving classes in the agent's jar. */ - private static final LocationStrategy AGENT_CLASS_LOCATION_STRATEGY = - new AgentBuilder.LocationStrategy.Simple(AGENT_CLASS_LOCATOR); - - /** - * Create bytebuddy advice with default datadog settings. - * - * @return the bytebuddy advice - */ - public static AgentBuilder.Transformer.ForAdvice create() { - return create(true); - } - - public static AgentBuilder.Transformer.ForAdvice create(final boolean includeExceptionHandler) { - ForAdvice advice = new DDAdvice().with(AGENT_CLASS_LOCATION_STRATEGY); - if (includeExceptionHandler) { - advice = advice.withExceptionHandler(ExceptionHandlers.defaultExceptionHandler()); - } - return advice; - } - - private DDAdvice() {} -} diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java index 4ce3bfb22b..051f2b265a 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java @@ -30,9 +30,10 @@ public class HelperInjector implements Transformer { * Construct HelperInjector. * * @param helperClassNames binary names of the helper classes to inject. These class names must be - * resolvable by the classloader returned by DDAdvice#getAgentClassLoader(). Classes are - * injected in the order provided. This is important if there is interdependency between - * helper classes that requires them to be injected in a specific order. + * resolvable by the classloader returned by + * datadog.trace.agent.tooling.Utils#getAgentClassLoader(). Classes are injected in the order + * provided. This is important if there is interdependency between helper classes that + * requires them to be injected in a specific order. */ public HelperInjector(final String... helperClassNames) { this.helperClassNames = new LinkedHashSet<>(Arrays.asList(helperClassNames)); diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java index ed6d6689ca..d4b2a8e4d0 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java @@ -72,56 +72,75 @@ public interface Instrumenter { } @Override - public AgentBuilder instrument(final AgentBuilder agentBuilder) { + public AgentBuilder instrument(final AgentBuilder parentAgentBuilder) { if (!enabled) { log.debug("Instrumentation {} is disabled", this); - return agentBuilder; + return parentAgentBuilder; } - AgentBuilder.Identified.Extendable advice = - agentBuilder + AgentBuilder.Identified.Extendable agentBuilder = + parentAgentBuilder .type(typeMatcher(), classLoaderMatcher()) - .and( - new AgentBuilder.RawMatcher() { - @Override - public boolean matches( - final TypeDescription typeDescription, - final ClassLoader classLoader, - final JavaModule module, - final Class classBeingRedefined, - final ProtectionDomain protectionDomain) { - /* Optimization: calling getInstrumentationMuzzle() inside this method - * prevents unnecessary loading of muzzle references during agentBuilder - * setup. - */ - final ReferenceMatcher muzzle = getInstrumentationMuzzle(); - if (null != muzzle) { - final List mismatches = - muzzle.getMismatchedReferenceSources(classLoader); - if (mismatches.size() > 0) { - log.debug( - "Instrumentation muzzled: {} -- {} on {}", - instrumentationPrimaryName, - this.getClass().getName(), - classLoader); - } - for (final Reference.Mismatch mismatch : mismatches) { - log.debug("-- {}", mismatch); - } - return mismatches.size() == 0; - } - return true; - } - }) + .and(new MuzzleMatcher()) .transform(DDTransformers.defaultTransformers()); + agentBuilder = injectHelperClasses(agentBuilder); + agentBuilder = applyInstrumentationTransformers(agentBuilder); + return agentBuilder.asDecorator(); + } + + private AgentBuilder.Identified.Extendable injectHelperClasses( + AgentBuilder.Identified.Extendable agentBuilder) { final String[] helperClassNames = helperClassNames(); if (helperClassNames.length > 0) { - advice = advice.transform(new HelperInjector(helperClassNames)); + agentBuilder = agentBuilder.transform(new HelperInjector(helperClassNames)); } + return agentBuilder; + } + + private AgentBuilder.Identified.Extendable applyInstrumentationTransformers( + AgentBuilder.Identified.Extendable agentBuilder) { for (final Map.Entry entry : transformers().entrySet()) { - advice = advice.transform(DDAdvice.create().advice(entry.getKey(), entry.getValue())); + agentBuilder = + agentBuilder.transform( + new AgentBuilder.Transformer.ForAdvice() + .include(Utils.getAgentClassLoader()) + .withExceptionHandler(ExceptionHandlers.defaultExceptionHandler()) + .advice(entry.getKey(), entry.getValue())); + } + return agentBuilder; + } + + /** Matches classes for which instrumentation is not muzzled. */ + private class MuzzleMatcher implements AgentBuilder.RawMatcher { + @Override + public boolean matches( + final TypeDescription typeDescription, + final ClassLoader classLoader, + final JavaModule module, + final Class classBeingRedefined, + final ProtectionDomain protectionDomain) { + /* Optimization: calling getInstrumentationMuzzle() inside this method + * prevents unnecessary loading of muzzle references during agentBuilder + * setup. + */ + final ReferenceMatcher muzzle = getInstrumentationMuzzle(); + if (null != muzzle) { + final List mismatches = + muzzle.getMismatchedReferenceSources(classLoader); + if (mismatches.size() > 0) { + log.debug( + "Instrumentation muzzled: {} -- {} on {}", + instrumentationPrimaryName, + this.getClass().getName(), + classLoader); + } + for (final Reference.Mismatch mismatch : mismatches) { + log.debug("-- {}", mismatch); + } + return mismatches.size() == 0; + } + return true; } - return advice.asDecorator(); } /** From 2bfb7b93ec4a0e05a3a9bbd7a7e48a43746681e9 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 30 Jul 2018 21:41:10 -0400 Subject: [PATCH 10/12] Disable Lagom circuit breaker It looks like they fail tests from time to time --- .../akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy | 2 +- .../akka-http-10.0/src/lagomTest/resource/application.conf | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/resource/application.conf diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy b/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy index 0ac6d5e4a5..f7bcb01815 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/groovy/LagomTest.groovy @@ -30,7 +30,7 @@ class LagomTest extends AgentTestRunner { .withPersistence(false) .withCassandra(false) .withJdbc(false) - .withConfigureBuilder( + .configureBuilder( new Function() { @Override GuiceApplicationBuilder apply(GuiceApplicationBuilder builder) { diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/resource/application.conf b/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/resource/application.conf new file mode 100644 index 0000000000..b36b0fa482 --- /dev/null +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/lagomTest/resource/application.conf @@ -0,0 +1,6 @@ +lagom.circuit-breaker { + // Disable Lagom circuit-breaker + default { + enabled = off + } +} From bb2126bd9a7ac3d13bf1695a3454b3c51ff947fd Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 30 Jul 2018 21:52:16 -0400 Subject: [PATCH 11/12] Improve ByteBuddyElementMatchers javadoc --- .../agent/tooling/ByteBuddyElementMatchers.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) 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 c934ee1e64..8588bbae7d 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 @@ -49,8 +49,19 @@ public class ByteBuddyElementMatchers { } /** - * An element matcher that matches a super type. Exceptions during matching process are logged and - * ignored. + * An element matcher that matches a super type. This is different from {@link + * net.bytebuddy.matcher.HasSuperTypeMatcher} in the following way: + * + *
    + *
  • Exceptions are logged + *
  • When exception happens the rest of the inheritance subtree is discarded (since ByteBuddy + * cannot load/parse type information for it) but search in other subtrees continues + *
+ * + *

This is useful because this allows us to see when matcher's check is not complete (i.e. part + * of it fails), at the same time it makes best effort instead of failing quickly (like {@code + * failSafe(hasSuperType(...))} does) which means the code is more resilient to classpath + * inconsistencies * * @param The type of the matched entity. * @see net.bytebuddy.matcher.HasSuperTypeMatcher From 7ad93059276ad0ee8392db4d3f0679e4d678d5e0 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Wed, 1 Aug 2018 14:14:25 -0400 Subject: [PATCH 12/12] Add some integration tests to check ByteBuddy's behavoir on class loading and parsing --- .../classloading/ClassLoadingTest.groovy | 62 ++++++++++++++++++- .../classloading/ClassToInstrument.java | 8 --- .../java/datadog/test/ClassToInstrument.java | 9 +++ .../datadog/test/ClassToInstrumentChild.java | 4 ++ .../agent/test/IntegrationTestUtils.java | 11 ++++ .../trace/agent/tooling/Instrumenter.java | 4 +- 6 files changed, 87 insertions(+), 11 deletions(-) delete mode 100644 dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassToInstrument.java create mode 100644 dd-java-agent-ittests/src/test/java/datadog/test/ClassToInstrument.java create mode 100644 dd-java-agent-ittests/src/test/java/datadog/test/ClassToInstrumentChild.java diff --git a/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy b/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy index 721471c4e6..ef7e1aeeb9 100644 --- a/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy +++ b/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy @@ -1,17 +1,22 @@ package datadog.trace.agent.integration.classloading +import datadog.test.ClassToInstrument +import datadog.test.ClassToInstrumentChild import datadog.trace.agent.test.IntegrationTestUtils import datadog.trace.api.Trace import spock.lang.Specification +import java.lang.ref.WeakReference + import static datadog.trace.agent.test.IntegrationTestUtils.createJarWithClasses class ClassLoadingTest extends Specification { + final URL[] classpath = [createJarWithClasses(ClassToInstrument, ClassToInstrumentChild, Trace)] + /** Assert that we can instrument classloaders which cannot resolve agent advice classes. */ def "instrument classloader without agent classes"() { setup: - final URL[] classpath = [createJarWithClasses(ClassToInstrument, Trace)] final URLClassLoader loader = new URLClassLoader(classpath, (ClassLoader) null) when: @@ -25,6 +30,61 @@ class ClassLoadingTest extends Specification { instrumentedClass.getClassLoader() == loader } + def "make sure ByteBuddy does not hold strong references to ClassLoader"() { + setup: + final URLClassLoader loader = new URLClassLoader(classpath, (ClassLoader) null) + final WeakReference ref = new WeakReference<>(loader) + + when: + loader.loadClass(ClassToInstrument.getName()) + loader = null + + IntegrationTestUtils.awaitGC() + + then: + null == ref.get() + } + + // We are doing this because Grovy cannot properly resolve constructor argument types in anonymous classes + static class CountingClassLoader extends URLClassLoader { + public int count = 0 + + CountingClassLoader(URL[] urls) { + super(urls, (ClassLoader) null) + } + + @Override + URL getResource(String name) { + count++ + } + } + + def "make sure that ByteBuddy reads classes's bytes only once"() { + setup: + final CountingClassLoader loader = new CountingClassLoader(classpath) + + when: + loader.loadClass(ClassToInstrument.getName()) + loader.loadClass(ClassToInstrumentChild.getName()) + + then: + loader.count == 2 + } + + def "make sure that ByteBuddy doesn't resue cached type descriptions between different classloaders"() { + setup: + final CountingClassLoader loader1 = new CountingClassLoader(classpath) + final CountingClassLoader loader2 = new CountingClassLoader(classpath) + + when: + loader1.loadClass(ClassToInstrument.getName()) + loader2.loadClass(ClassToInstrument.getName()) + + then: + loader1.count == 1 + loader2.count == 1 + } + def "can find bootstrap resources"() { expect: IntegrationTestUtils.getAgentClassLoader().getResources('datadog/trace/api/Trace.class') != null diff --git a/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassToInstrument.java b/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassToInstrument.java deleted file mode 100644 index 17a666f3a4..0000000000 --- a/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassToInstrument.java +++ /dev/null @@ -1,8 +0,0 @@ -package datadog.trace.agent.integration.classloading; - -import datadog.trace.api.Trace; - -class ClassToInstrument { - @Trace - public static void someMethod() {} -} diff --git a/dd-java-agent-ittests/src/test/java/datadog/test/ClassToInstrument.java b/dd-java-agent-ittests/src/test/java/datadog/test/ClassToInstrument.java new file mode 100644 index 0000000000..52f085751f --- /dev/null +++ b/dd-java-agent-ittests/src/test/java/datadog/test/ClassToInstrument.java @@ -0,0 +1,9 @@ +package datadog.test; + +import datadog.trace.api.Trace; + +/** Note: this has to stay in 'datadog.test' package to be considered for instrumentation */ +public class ClassToInstrument { + @Trace + public static void someMethod() {} +} diff --git a/dd-java-agent-ittests/src/test/java/datadog/test/ClassToInstrumentChild.java b/dd-java-agent-ittests/src/test/java/datadog/test/ClassToInstrumentChild.java new file mode 100644 index 0000000000..5025e5316e --- /dev/null +++ b/dd-java-agent-ittests/src/test/java/datadog/test/ClassToInstrumentChild.java @@ -0,0 +1,4 @@ +package datadog.test; + +/** Note: this has to stay in 'datadog.test' package to be considered for instrumentation */ +public class ClassToInstrumentChild extends ClassToInstrument {} diff --git a/dd-java-agent-ittests/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java index c953a1c801..34dcaa8b42 100644 --- a/dd-java-agent-ittests/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java +++ b/dd-java-agent-ittests/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java @@ -7,6 +7,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.lang.ref.WeakReference; import java.lang.reflect.Field; import java.net.URL; import java.util.UUID; @@ -156,4 +157,14 @@ public class IntegrationTestUtils { .getField("AGENT_PACKAGE_PREFIXES"); return (String[]) f.get(null); } + + public static void awaitGC() { + System.gc(); // For good measure. + Object obj = new Object(); + final WeakReference ref = new WeakReference<>(obj); + obj = null; + while (ref.get() != null) { + System.gc(); + } + } } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java index d4b2a8e4d0..61de26c1f1 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java @@ -51,7 +51,7 @@ public interface Instrumenter { protected final boolean enabled; public Default(final String instrumentationName, final String... additionalNames) { - this.instrumentationNames = new HashSet<>(Arrays.asList(additionalNames)); + instrumentationNames = new HashSet<>(Arrays.asList(additionalNames)); instrumentationNames.add(instrumentationName); instrumentationPrimaryName = instrumentationName; @@ -131,7 +131,7 @@ public interface Instrumenter { log.debug( "Instrumentation muzzled: {} -- {} on {}", instrumentationPrimaryName, - this.getClass().getName(), + getClass().getName(), classLoader); } for (final Reference.Mismatch mismatch : mismatches) {