fix: add jetty handler instrumentaiton. remove filter3instrumentation. add sparkjava routematch instrumentation

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'
+}

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 final class HandlerInstrumentation extends Instrumenter.Configurable {
-  public static final String SERVLET_OPERATION_NAME = "servlet.request";
+  public static final String SERVLET_OPERATION_NAME = "jetty.request";
 
-  public Filter3Instrumentation() {
+  public HandlerInstrumentation() {
-    super("servlet", "servlet-3");
+    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")
+            named("handle")
-              .and(takesArgument(0, named("javax.servlet.ServletRequest")))
+              .and(takesArgument(0, named("String")))
-              .and(takesArgument(1, named("javax.servlet.ServletResponse")))
+              .and(takesArgument(1, named("org.eclipse.jetty.server.Request")))
-              .and(takesArgument(2, named("javax.servlet.ServletResponse")))
+              .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) {
+    public static Scope startSpan(@Advice.Argument(0) final String target, @Advice.Argument(2) final HttpServletRequest 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()
+        GlobalTracer.get().extract(Format.Builtin.HTTP_HEADERS, new HttpServletRequestExtractAdapter(req));
-          .extract(
+      final String resourceName = req.getMethod() + target;
-            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)
+          //.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(2) final HttpServletRequest req,
-      @Advice.Argument(1) final ServletResponse response,
+      @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));
@@ -126,7 +117,6 @@ public final class Filter3Instrumentation extends Instrumenter.Configurable {
           }
       }
     }
-    }
 
     public static class TagSettingAsyncListener implements AsyncListener {
       private final AtomicBoolean activated;

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
+  // Uses jetty-9 instrumentation for requests
-  testCompile project(':dd-java-agent:instrumentation:servlet-3')
+  testCompile project(':dd-java-agent:instrumentation:jetty-9')
-
-  testCompile group: 'com.sparkjava', name: 'spark-core', version: '2.7.1'
   testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0'
 
 }

dd-java-agent/instrumentation/sparkjava/src/main/java/datadog/trace/instrumentation/sparkjava/Filter3Instrumentation.java

@@ -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 {}
-    }
-  }
-}

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);
+      }
+    }
+  }
+}

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'