From c49d4f6dd8268984a05111b1dc70503c71b49b3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Gr=C3=B8ndal?= Date: Wed, 7 Mar 2018 08:46:31 +0100 Subject: [PATCH] fix: add jetty handler instrumentaiton. remove filter3instrumentation. add sparkjava routematch instrumentation --- .../instrumentation/jetty-9/jetty-9.gradle | 37 ++++ .../jetty9/HandlerInstrumentation.java} | 54 +++--- .../sparkjava/sparkjava.gradle | 18 +- .../sparkjava/Filter3Instrumentation.java | 183 ------------------ .../sparkjava/RoutesInstrumentation.java | 54 ++++++ settings.gradle | 1 + 6 files changed, 128 insertions(+), 219 deletions(-) create mode 100644 dd-java-agent/instrumentation/jetty-9/jetty-9.gradle rename dd-java-agent/instrumentation/{servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Filter3Instrumentation.java => jetty-9/src/main/java/datadog/trace/instrumentation/jetty9/HandlerInstrumentation.java} (78%) delete mode 100644 dd-java-agent/instrumentation/sparkjava/src/main/java/datadog/trace/instrumentation/sparkjava/Filter3Instrumentation.java create mode 100644 dd-java-agent/instrumentation/sparkjava/src/main/java/datadog/trace/instrumentation/sparkjava/RoutesInstrumentation.java diff --git a/dd-java-agent/instrumentation/jetty-9/jetty-9.gradle b/dd-java-agent/instrumentation/jetty-9/jetty-9.gradle new file mode 100644 index 0000000000..4a25c0779d --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-9/jetty-9.gradle @@ -0,0 +1,37 @@ +apply plugin: 'version-scan' + +versionScan { + group = "javax.servlet" + module = 'javax.servlet-api' + legacyModule = "servlet-api" + versions = "[3.0,)" + verifyPresent = [ + "javax.servlet.AsyncEvent" : null, + "javax.servlet.AsyncListener": null, + ] +} + +apply from: "${rootDir}/gradle/java.gradle" + +dependencies { + compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1' + compile('io.opentracing.contrib:opentracing-web-servlet-filter:0.1.0') { + transitive = false + } + + compile project(':dd-trace-ot') + compile project(':dd-java-agent:agent-tooling') + + compile deps.bytebuddy + compile deps.opentracing + compile deps.autoservice + + testCompile project(':dd-java-agent:testing') + testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '8.2.0.v20160908' + testCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '8.2.0.v20160908' + testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '8.0.41' + testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '8.0.41' + + testCompile project(':dd-java-agent:instrumentation:okhttp-3') // used in the tests + testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0' +} diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Filter3Instrumentation.java b/dd-java-agent/instrumentation/jetty-9/src/main/java/datadog/trace/instrumentation/jetty9/HandlerInstrumentation.java similarity index 78% rename from dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Filter3Instrumentation.java rename to dd-java-agent/instrumentation/jetty-9/src/main/java/datadog/trace/instrumentation/jetty9/HandlerInstrumentation.java index 33468dfeb5..0a0da32889 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Filter3Instrumentation.java +++ b/dd-java-agent/instrumentation/jetty-9/src/main/java/datadog/trace/instrumentation/jetty9/HandlerInstrumentation.java @@ -1,4 +1,4 @@ -package datadog.trace.instrumentation.servlet3; +package datadog.trace.instrumentation.jetty9; import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static io.opentracing.log.Fields.ERROR_OBJECT; @@ -36,18 +36,18 @@ import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.asm.Advice; @AutoService(Instrumenter.class) -public final class Filter3Instrumentation extends Instrumenter.Configurable { - public static final String SERVLET_OPERATION_NAME = "servlet.request"; +public final class HandlerInstrumentation extends Instrumenter.Configurable { + public static final String SERVLET_OPERATION_NAME = "jetty.request"; - public Filter3Instrumentation() { - super("servlet", "servlet-3"); + public HandlerInstrumentation() { + super("jetty", "jetty-9"); } @Override public AgentBuilder apply(final AgentBuilder agentBuilder) { return agentBuilder .type( - not(isInterface()).and(hasSuperType(named("javax.servlet.Filter"))), + not(isInterface()).and(hasSuperType(named("org.eclipse.jetty.server.Handler"))), classLoaderHasClasses("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener")) .transform( new HelperInjector( @@ -57,59 +57,50 @@ public final class Filter3Instrumentation extends Instrumenter.Configurable { "io.opentracing.contrib.web.servlet.filter.ServletFilterSpanDecorator$1", "io.opentracing.contrib.web.servlet.filter.TracingFilter", "io.opentracing.contrib.web.servlet.filter.TracingFilter$1", - FilterChain3Advice.class.getName() + "$TagSettingAsyncListener")) + HandlerInstrumentationAdvice.class.getName() + "$TagSettingAsyncListener")) .transform( DDAdvice.create() .advice( - named("doFilter") - .and(takesArgument(0, named("javax.servlet.ServletRequest"))) - .and(takesArgument(1, named("javax.servlet.ServletResponse"))) - .and(takesArgument(2, named("javax.servlet.ServletResponse"))) + named("handle") + .and(takesArgument(0, named("String"))) + .and(takesArgument(1, named("org.eclipse.jetty.server.Request"))) + .and(takesArgument(2, named("javax.servlet.HttpServletRequest"))) + .and(takesArgument(3, named("javax.servlet.HttpServletResponse"))) .and(isPublic()), - FilterChain3Advice.class.getName())) + HandlerInstrumentationAdvice.class.getName())) .asDecorator(); } - public static class FilterChain3Advice { + public static class HandlerInstrumentationAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static Scope startSpan(@Advice.Argument(0) final ServletRequest req) { - if (GlobalTracer.get().activeSpan() != null || !(req instanceof HttpServletRequest)) { - // Tracing might already be applied by the FilterChain. If so ignore this. - return null; - } + public static Scope startSpan(@Advice.Argument(0) final String target, @Advice.Argument(2) final HttpServletRequest req) { final SpanContext extractedContext = - GlobalTracer.get() - .extract( - Format.Builtin.HTTP_HEADERS, - new HttpServletRequestExtractAdapter((HttpServletRequest) req)); - + GlobalTracer.get().extract(Format.Builtin.HTTP_HEADERS, new HttpServletRequestExtractAdapter(req)); + final String resourceName = req.getMethod() + target; final Scope scope = GlobalTracer.get() .buildSpan(SERVLET_OPERATION_NAME) .asChildOf(extractedContext) .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER) .withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET) + //.withTag("span.origin.type", statement.getClass().getName()) + .withTag(DDTags.RESOURCE_NAME, resourceName) .startActive(false); - ServletFilterSpanDecorator.STANDARD_TAGS.onRequest((HttpServletRequest) req, scope.span()); + ServletFilterSpanDecorator.STANDARD_TAGS.onRequest(req, scope.span()); return scope; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( - @Advice.Argument(0) final ServletRequest request, - @Advice.Argument(1) final ServletResponse response, + @Advice.Argument(2) final HttpServletRequest req, + @Advice.Argument(3) final HttpServletResponse resp, @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) { - if (scope != null) { - if (request instanceof HttpServletRequest && response instanceof HttpServletResponse) { - final HttpServletRequest req = (HttpServletRequest) request; - final HttpServletResponse resp = (HttpServletResponse) response; final Span span = scope.span(); - if (throwable != null) { ServletFilterSpanDecorator.STANDARD_TAGS.onError(req, resp, throwable, span); span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); @@ -124,7 +115,6 @@ public final class Filter3Instrumentation extends Instrumenter.Configurable { scope.close(); scope.span().finish(); // Finish the span manually since finishSpanOnClose was false } - } } } diff --git a/dd-java-agent/instrumentation/sparkjava/sparkjava.gradle b/dd-java-agent/instrumentation/sparkjava/sparkjava.gradle index ac5fabe3ec..fa41dc046d 100644 --- a/dd-java-agent/instrumentation/sparkjava/sparkjava.gradle +++ b/dd-java-agent/instrumentation/sparkjava/sparkjava.gradle @@ -1,5 +1,16 @@ apply from: "${rootDir}/gradle/java.gradle" +if (JavaVersion.current() != JavaVersion.VERSION_1_8) { + sourceSets { + test { + groovy { + // Sparkjava is not compatible with < Java 8 + exclude '**/SparkJavaBasedTest.groovy' + } + } + } +} + dependencies { compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.1.0' compile('io.opentracing.contrib:opentracing-web-servlet-filter:0.1.0') { @@ -11,13 +22,12 @@ dependencies { compile deps.bytebuddy compile deps.opentracing compile deps.autoservice + compile group: 'com.sparkjava', name: 'spark-core', version: '2.7.1' testCompile project(':dd-java-agent:testing') - // Include servlet instrumentation for verifying the tomcat requests - testCompile project(':dd-java-agent:instrumentation:servlet-3') - - testCompile group: 'com.sparkjava', name: 'spark-core', version: '2.7.1' + // Uses jetty-9 instrumentation for requests + testCompile project(':dd-java-agent:instrumentation:jetty-9') testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0' } diff --git a/dd-java-agent/instrumentation/sparkjava/src/main/java/datadog/trace/instrumentation/sparkjava/Filter3Instrumentation.java b/dd-java-agent/instrumentation/sparkjava/src/main/java/datadog/trace/instrumentation/sparkjava/Filter3Instrumentation.java deleted file mode 100644 index 33468dfeb5..0000000000 --- a/dd-java-agent/instrumentation/sparkjava/src/main/java/datadog/trace/instrumentation/sparkjava/Filter3Instrumentation.java +++ /dev/null @@ -1,183 +0,0 @@ -package datadog.trace.instrumentation.servlet3; - -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.isInterface; -import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.not; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.DDAdvice; -import datadog.trace.agent.tooling.HelperInjector; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; -import io.opentracing.Scope; -import io.opentracing.Span; -import io.opentracing.SpanContext; -import io.opentracing.contrib.web.servlet.filter.HttpServletRequestExtractAdapter; -import io.opentracing.contrib.web.servlet.filter.ServletFilterSpanDecorator; -import io.opentracing.propagation.Format; -import io.opentracing.tag.Tags; -import io.opentracing.util.GlobalTracer; -import java.io.IOException; -import java.util.Collections; -import java.util.concurrent.atomic.AtomicBoolean; -import javax.servlet.AsyncEvent; -import javax.servlet.AsyncListener; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import net.bytebuddy.agent.builder.AgentBuilder; -import net.bytebuddy.asm.Advice; - -@AutoService(Instrumenter.class) -public final class Filter3Instrumentation extends Instrumenter.Configurable { - public static final String SERVLET_OPERATION_NAME = "servlet.request"; - - public Filter3Instrumentation() { - super("servlet", "servlet-3"); - } - - @Override - public AgentBuilder apply(final AgentBuilder agentBuilder) { - return agentBuilder - .type( - not(isInterface()).and(hasSuperType(named("javax.servlet.Filter"))), - classLoaderHasClasses("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener")) - .transform( - new HelperInjector( - "io.opentracing.contrib.web.servlet.filter.HttpServletRequestExtractAdapter", - "io.opentracing.contrib.web.servlet.filter.HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator", - "io.opentracing.contrib.web.servlet.filter.ServletFilterSpanDecorator", - "io.opentracing.contrib.web.servlet.filter.ServletFilterSpanDecorator$1", - "io.opentracing.contrib.web.servlet.filter.TracingFilter", - "io.opentracing.contrib.web.servlet.filter.TracingFilter$1", - FilterChain3Advice.class.getName() + "$TagSettingAsyncListener")) - .transform( - DDAdvice.create() - .advice( - named("doFilter") - .and(takesArgument(0, named("javax.servlet.ServletRequest"))) - .and(takesArgument(1, named("javax.servlet.ServletResponse"))) - .and(takesArgument(2, named("javax.servlet.ServletResponse"))) - .and(isPublic()), - FilterChain3Advice.class.getName())) - .asDecorator(); - } - - public static class FilterChain3Advice { - - @Advice.OnMethodEnter(suppress = Throwable.class) - public static Scope startSpan(@Advice.Argument(0) final ServletRequest req) { - if (GlobalTracer.get().activeSpan() != null || !(req instanceof HttpServletRequest)) { - // Tracing might already be applied by the FilterChain. If so ignore this. - return null; - } - - final SpanContext extractedContext = - GlobalTracer.get() - .extract( - Format.Builtin.HTTP_HEADERS, - new HttpServletRequestExtractAdapter((HttpServletRequest) req)); - - final Scope scope = - GlobalTracer.get() - .buildSpan(SERVLET_OPERATION_NAME) - .asChildOf(extractedContext) - .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER) - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET) - .startActive(false); - - ServletFilterSpanDecorator.STANDARD_TAGS.onRequest((HttpServletRequest) req, scope.span()); - return scope; - } - - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void stopSpan( - @Advice.Argument(0) final ServletRequest request, - @Advice.Argument(1) final ServletResponse response, - @Advice.Enter final Scope scope, - @Advice.Thrown final Throwable throwable) { - - if (scope != null) { - if (request instanceof HttpServletRequest && response instanceof HttpServletResponse) { - final HttpServletRequest req = (HttpServletRequest) request; - final HttpServletResponse resp = (HttpServletResponse) response; - final Span span = scope.span(); - - if (throwable != null) { - ServletFilterSpanDecorator.STANDARD_TAGS.onError(req, resp, throwable, span); - span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); - scope.close(); - scope.span().finish(); // Finish the span manually since finishSpanOnClose was false - } else if (req.isAsyncStarted()) { - final AtomicBoolean activated = new AtomicBoolean(false); - // what if async is already finished? This would not be called - req.getAsyncContext().addListener(new TagSettingAsyncListener(activated, span)); - } else { - ServletFilterSpanDecorator.STANDARD_TAGS.onResponse(req, resp, span); - scope.close(); - scope.span().finish(); // Finish the span manually since finishSpanOnClose was false - } - } - } - } - - public static class TagSettingAsyncListener implements AsyncListener { - private final AtomicBoolean activated; - private final Span span; - - public TagSettingAsyncListener(final AtomicBoolean activated, final Span span) { - this.activated = activated; - this.span = span; - } - - @Override - public void onComplete(final AsyncEvent event) throws IOException { - if (activated.compareAndSet(false, true)) { - try (Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { - ServletFilterSpanDecorator.STANDARD_TAGS.onResponse( - (HttpServletRequest) event.getSuppliedRequest(), - (HttpServletResponse) event.getSuppliedResponse(), - span); - } - } - } - - @Override - public void onTimeout(final AsyncEvent event) throws IOException { - if (activated.compareAndSet(false, true)) { - try (Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { - ServletFilterSpanDecorator.STANDARD_TAGS.onTimeout( - (HttpServletRequest) event.getSuppliedRequest(), - (HttpServletResponse) event.getSuppliedResponse(), - event.getAsyncContext().getTimeout(), - span); - } - } - } - - @Override - public void onError(final AsyncEvent event) throws IOException { - if (event.getThrowable() != null && activated.compareAndSet(false, true)) { - try (Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { - ServletFilterSpanDecorator.STANDARD_TAGS.onError( - (HttpServletRequest) event.getSuppliedRequest(), - (HttpServletResponse) event.getSuppliedResponse(), - event.getThrowable(), - span); - span.log(Collections.singletonMap(ERROR_OBJECT, event.getThrowable())); - } - } - } - - @Override - public void onStartAsync(final AsyncEvent event) throws IOException {} - } - } -} diff --git a/dd-java-agent/instrumentation/sparkjava/src/main/java/datadog/trace/instrumentation/sparkjava/RoutesInstrumentation.java b/dd-java-agent/instrumentation/sparkjava/src/main/java/datadog/trace/instrumentation/sparkjava/RoutesInstrumentation.java new file mode 100644 index 0000000000..05e3c1465a --- /dev/null +++ b/dd-java-agent/instrumentation/sparkjava/src/main/java/datadog/trace/instrumentation/sparkjava/RoutesInstrumentation.java @@ -0,0 +1,54 @@ +package datadog.trace.instrumentation.sparkjava; + +import datadog.trace.agent.tooling.DDAdvice; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.api.DDTags; +import io.opentracing.Scope; +import io.opentracing.Span; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; +import spark.route.HttpMethod; +import spark.routematch.RouteMatch; + +import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; +import static net.bytebuddy.matcher.ElementMatchers.*; + +public class RoutesInstrumentation extends Instrumenter.Configurable { + + public RoutesInstrumentation() { + super("sparkjava", "sparkjava-2"); + } + + @Override + public AgentBuilder apply(final AgentBuilder agentBuilder) { + return agentBuilder + .type( + is(named("spark.route.Routes")), + classLoaderHasClasses("spark.embeddedserver.jetty.EmbeddedJettyServer")) + .transform( + DDAdvice.create() + .advice( + named("find") // HttpMethod httpMethod, String path, String acceptType + .and(takesArgument(0, named("spark.route.HttpMethod"))) + .and(takesArgument(1, named("String"))) + .and(takesArgument(2, named("String"))) + .and(isPublic()), + RoutesInstrumentationAdvice.class.getName())) + .asDecorator(); + } + + public static class RoutesInstrumentationAdvice { + + @Advice.OnMethodExit() + public static void routeMatchEnricher( + @Advice.Argument(0) final HttpMethod method, + @Advice.Enter final Scope scope, + @Advice.Return final RouteMatch routeMatch) { + if (scope != null && routeMatch != null) { + final Span span = scope.span(); + final String resourceName = method.name() + " " + routeMatch.getMatchUri(); + span.setTag(DDTags.RESOURCE_NAME, resourceName); + } + } + } +} diff --git a/settings.gradle b/settings.gradle index be84868432..da19ebf8b2 100644 --- a/settings.gradle +++ b/settings.gradle @@ -21,6 +21,7 @@ include ':dd-java-agent:instrumentation:java-concurrent:akka-testing' include ':dd-java-agent:instrumentation:jboss-classloading' include ':dd-java-agent:instrumentation:jdbc' include ':dd-java-agent:instrumentation:jedis-1.4' +include ':dd-java-agent:instrumentation:jetty-9' include ':dd-java-agent:instrumentation:jms-1' include ':dd-java-agent:instrumentation:jms-2' include ':dd-java-agent:instrumentation:kafka-clients-0.11'