Add resource naming instrumentation for jax-rs

dd-java-agent/instrumentation/jax-rs/jax-rs.gradle

@@ -0,0 +1,26 @@
+apply plugin: 'version-scan'
+
+versionScan {
+  group = "javax.ws.rs"
+  module = "jsr311-api"
+  versions = "(,)"
+}
+
+apply from: "${rootDir}/gradle/java.gradle"
+
+dependencies {
+  compileOnly group: 'javax.ws.rs', name: 'jsr311-api', version: '1.1.1'
+
+  compile deps.bytebuddy
+  compile deps.opentracing
+  compile deps.autoservice
+
+  compile project(':dd-trace-ot')
+  compile project(':dd-java-agent:tooling')
+
+  testCompile project(':dd-java-agent:testing')
+  testCompile group: 'com.sun.jersey', name: 'jersey-core', version: '1.19.4'
+  testCompile group: 'com.sun.jersey', name: 'jersey-servlet', version: '1.19.4'
+  testCompile group: 'io.dropwizard', name: 'dropwizard-testing', version: '0.7.1'
+  testCompile group: 'javax.xml.bind', name: 'jaxb-api', version: '2.2.3'
+}

dd-java-agent/instrumentation/jax-rs/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsInstrumentation.java

@@ -0,0 +1,100 @@
+package datadog.trace.instrumentation.jaxrs;
+
+import static net.bytebuddy.matcher.ElementMatchers.declaresMethod;
+import static net.bytebuddy.matcher.ElementMatchers.hasSuperType;
+import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith;
+import static net.bytebuddy.matcher.ElementMatchers.named;
+
+import com.google.auto.service.AutoService;
+import datadog.trace.agent.tooling.DDAdvice;
+import datadog.trace.agent.tooling.Instrumenter;
+import datadog.trace.api.DDTags;
+import io.opentracing.Scope;
+import io.opentracing.tag.Tags;
+import io.opentracing.util.GlobalTracer;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+import java.util.LinkedList;
+import javax.ws.rs.HttpMethod;
+import javax.ws.rs.Path;
+import net.bytebuddy.agent.builder.AgentBuilder;
+import net.bytebuddy.asm.Advice;
+
+@AutoService(Instrumenter.class)
+public final class JaxRsInstrumentation extends Instrumenter.Configurable {
+
+  public JaxRsInstrumentation() {
+    super("jax-rs", "jaxrs");
+  }
+
+  @Override
+  protected boolean defaultEnabled() {
+    return false;
+  }
+
+  @Override
+  protected AgentBuilder apply(final AgentBuilder agentBuilder) {
+    return agentBuilder
+        .type(
+            hasSuperType(
+                isAnnotatedWith(named("javax.ws.rs.Path"))
+                    .or(hasSuperType(declaresMethod(isAnnotatedWith(named("javax.ws.rs.Path")))))))
+        .transform(
+            DDAdvice.create()
+                .advice(
+                    isAnnotatedWith(
+                        named("javax.ws.rs.Path")
+                            .or(named("javax.ws.rs.DELETE"))
+                            .or(named("javax.ws.rs.GET"))
+                            .or(named("javax.ws.rs.HEAD"))
+                            .or(named("javax.ws.rs.OPTIONS"))
+                            .or(named("javax.ws.rs.POST"))
+                            .or(named("javax.ws.rs.PUT"))),
+                    JaxRsAdvice.class.getName()))
+        .asDecorator();
+  }
+
+  public static class JaxRsAdvice {
+
+    @Advice.OnMethodEnter(suppress = Throwable.class)
+    public static void nameSpan(@Advice.This final Object obj, @Advice.Origin final Method method) {
+      // TODO: do we need caching for this?
+
+      final LinkedList<Path> classPaths = new LinkedList<>();
+      Class<?> target = obj.getClass();
+      while (target != Object.class) {
+        final Path annotation = target.getAnnotation(Path.class);
+        if (annotation != null) {
+          classPaths.push(annotation);
+        }
+        target = target.getSuperclass();
+      }
+      final Path methodPath = method.getAnnotation(Path.class);
+      String httpMethod = null;
+      for (final Annotation ann : method.getDeclaredAnnotations()) {
+        if (ann.annotationType().getAnnotation(HttpMethod.class) != null) {
+          httpMethod = ann.annotationType().getSimpleName();
+        }
+      }
+
+      final StringBuilder resourceNameBuilder = new StringBuilder();
+      if (httpMethod != null) {
+        resourceNameBuilder.append(httpMethod);
+        resourceNameBuilder.append(" ");
+      }
+      for (final Path classPath : classPaths) {
+        resourceNameBuilder.append(classPath.value());
+      }
+      if (methodPath != null) {
+        resourceNameBuilder.append(methodPath.value());
+      }
+      final String resourceName = resourceNameBuilder.toString().trim();
+
+      final Scope scope = GlobalTracer.get().scopeManager().active();
+      if (scope != null && !resourceName.isEmpty()) {
+        scope.span().setTag(DDTags.RESOURCE_NAME, resourceName);
+        Tags.COMPONENT.set(scope.span(), "jax-rs");
+      }
+    }
+  }
+}

dd-java-agent/instrumentation/jax-rs/src/test/groovy/JaxRsInstrumentationTest.groovy

@@ -0,0 +1,105 @@
+import datadog.opentracing.DDSpanContext
+import datadog.trace.agent.test.AgentTestRunner
+import io.opentracing.util.GlobalTracer
+import spock.lang.Unroll
+
+import javax.ws.rs.*
+
+class JaxRsInstrumentationTest extends AgentTestRunner {
+
+  static {
+    System.setProperty("dd.integration.jax-rs.enabled", "true")
+  }
+
+  @Unroll
+  def "span named '#resourceName' from annotations on class"() {
+    setup:
+    def scope = GlobalTracer.get().buildSpan("test").startActive(false)
+    DDSpanContext spanContext = scope.span().context()
+    obj.call()
+
+    expect:
+    spanContext.resourceName == resourceName
+
+    cleanup:
+    scope.close()
+
+    where:
+    resourceName                | obj
+    "test"                      | new Jax() {
+      // invalid because no annotations
+      void call() {}
+    }
+    "/a"                        | new Jax() {
+      @Path("/a")
+      void call() {}
+    }
+    "GET /b"                    | new Jax() {
+      @GET
+      @Path("/b")
+      void call() {}
+    }
+    "test"                      | new InterfaceWithPath() {
+      // invalid because no annotations
+      void call() {}
+    }
+    "POST /c"                   | new InterfaceWithPath() {
+      @POST
+      @Path("/c")
+      void call() {}
+    }
+    "HEAD"                      | new InterfaceWithPath() {
+      @HEAD
+      void call() {}
+    }
+    "test"                      | new AbstractClassWithPath() {
+      // invalid because no annotations
+      void call() {}
+    }
+    "POST /abstract/d"          | new AbstractClassWithPath() {
+      @POST
+      @Path("/d")
+      void call() {}
+    }
+    "PUT /abstract"             | new AbstractClassWithPath() {
+      @PUT
+      void call() {}
+    }
+    "test"                      | new ChildClassWithPath() {
+      // invalid because no annotations
+      void call() {}
+    }
+    "OPTIONS /abstract/child/e" | new ChildClassWithPath() {
+      @OPTIONS
+      @Path("/e")
+      void call() {}
+    }
+    "DELETE /abstract/child"    | new ChildClassWithPath() {
+      @DELETE
+      void call() {}
+    }
+    "POST /abstract/child"      | new ChildClassWithPath()
+  }
+
+  interface Jax {
+    void call()
+  }
+
+  @Path("/interface")
+  interface InterfaceWithPath extends Jax {
+    @GET
+    void call()
+  }
+
+  @Path("/abstract")
+  abstract class AbstractClassWithPath implements Jax {
+    @PUT
+    abstract void call()
+  }
+
+  @Path("/child")
+  class ChildClassWithPath extends AbstractClassWithPath {
+    @POST
+    void call() {}
+  }
+}

dd-java-agent/instrumentation/jax-rs/src/test/groovy/JerseyTest.groovy

@@ -0,0 +1,33 @@
+import datadog.trace.agent.test.AgentTestRunner
+import io.dropwizard.testing.junit.ResourceTestRule
+import org.junit.ClassRule
+import spock.lang.Shared
+
+class JerseyTest extends AgentTestRunner {
+
+  static {
+    System.setProperty("dd.integration.jax-rs.enabled", "true")
+  }
+
+  @Shared
+  @ClassRule
+  ResourceTestRule resources = ResourceTestRule.builder().addResource(new TestResource()).build()
+
+  def "test resource"() {
+    setup:
+    // start a trace because the test doesn't go through any servlet or other instrumentation.
+    def scope = TEST_TRACER.buildSpan("test.span").startActive(true)
+    def response = resources.client().resource("/test/hello/bob").post(String)
+    scope.close()
+
+    expect:
+    response == "Hello bob!"
+    TEST_WRITER.waitForTraces(1)
+    TEST_WRITER.size() == 1
+
+    def trace = TEST_WRITER.firstTrace()
+    def span = trace[0]
+    span.resourceName == "POST /test/hello/{name}"
+    span.tags["component"] == "jax-rs"
+  }
+}

dd-java-agent/instrumentation/jax-rs/src/test/java/TestResource.java

@@ -0,0 +1,13 @@
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+
+// Originally had this as a groovy class but was getting some weird errors.
+@Path("/test")
+public class TestResource {
+  @POST
+  @Path("/hello/{name}")
+  public String addBook(@PathParam("name") final String name) {
+    return "Hello " + name + "!";
+  }
+}

dd-java-agent/instrumentation/kafka-clients-0.11/kafka-clients-0.11.gradle

@@ -25,5 +25,5 @@ dependencies {
   testCompile group: 'org.apache.kafka', name: 'kafka-clients', version: '0.11.0.0'
   testCompile group: 'org.springframework.kafka', name: 'spring-kafka', version: '1.3.3.RELEASE'
   testCompile group: 'org.springframework.kafka', name: 'spring-kafka-test', version: '1.3.3.RELEASE'
-  testCompile group: 'javax.xml.bind', name: 'jaxb-api', version: '2.3.0'
+  testCompile group: 'javax.xml.bind', name: 'jaxb-api', version: '2.2.3'
 }

dd-java-agent/instrumentation/kafka-clients-0.11/src/test/groovy/KafkaClientTest.groovy

@@ -1,9 +1,6 @@
-import ch.qos.logback.classic.Level
-import ch.qos.logback.classic.Logger
 import datadog.trace.agent.test.AgentTestRunner
 import org.apache.kafka.clients.consumer.ConsumerRecord
 import org.junit.ClassRule
-import org.slf4j.LoggerFactory
 import org.springframework.kafka.core.DefaultKafkaConsumerFactory
 import org.springframework.kafka.core.DefaultKafkaProducerFactory
 import org.springframework.kafka.core.KafkaTemplate
@@ -22,8 +19,6 @@ class KafkaClientTest extends AgentTestRunner {
   static final SHARED_TOPIC = "shared.topic"
 
   static {
-    ((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).setLevel(Level.WARN)
-    ((Logger) LoggerFactory.getLogger("datadog")).setLevel(Level.DEBUG)
     System.setProperty("dd.integration.kafka.enabled", "true")
   }
 

dd-java-agent/instrumentation/kafka-streams-0.11/kafka-streams-0.11.gradle

@@ -29,5 +29,5 @@ dependencies {
   testCompile group: 'org.apache.kafka', name: 'kafka-streams', version: '0.11.0.0'
   testCompile group: 'org.springframework.kafka', name: 'spring-kafka', version: '1.3.3.RELEASE'
   testCompile group: 'org.springframework.kafka', name: 'spring-kafka-test', version: '1.3.3.RELEASE'
-  testCompile group: 'javax.xml.bind', name: 'jaxb-api', version: '2.3.0'
+  testCompile group: 'javax.xml.bind', name: 'jaxb-api', version: '2.2.3'
 }

dd-java-agent/instrumentation/kafka-streams-0.11/src/test/groovy/KafkaStreamsTest.groovy

@@ -1,7 +1,6 @@
 import ch.qos.logback.classic.Level
 import ch.qos.logback.classic.Logger
 import datadog.trace.agent.test.AgentTestRunner
-import io.opentracing.util.GlobalTracer
 import org.apache.kafka.clients.consumer.ConsumerRecord
 import org.apache.kafka.common.serialization.Serdes
 import org.apache.kafka.streams.KafkaStreams
@@ -61,7 +60,7 @@ class KafkaStreamsTest extends AgentTestRunner {
       @Override
       void onMessage(ConsumerRecord<String, String> record) {
         WRITER_PHASER.arriveAndAwaitAdvance() // ensure consistent ordering of traces
-        GlobalTracer.get().activeSpan().setTag("testing", 123)
+        TEST_TRACER.activeSpan().setTag("testing", 123)
         records.add(record)
       }
     })
@@ -80,7 +79,7 @@ class KafkaStreamsTest extends AgentTestRunner {
       @Override
       String apply(String textLine) {
         WRITER_PHASER.arriveAndAwaitAdvance() // ensure consistent ordering of traces
-        GlobalTracer.get().activeSpan().setTag("asdf", "testing")
+        TEST_TRACER.activeSpan().setTag("asdf", "testing")
         return textLine.toLowerCase()
       }
     })

dd-java-agent/src/test/resources/logback.xml

@@ -13,6 +13,6 @@
     <appender-ref ref="console"/>
   </root>
 
-  <logger name="com.datadoghq" level="debug"/>
+  <logger name="datadog" level="debug"/>
 
 </configuration>

dd-java-agent/testing/src/main/java/datadog/trace/agent/test/AgentTestRunner.java

@@ -1,5 +1,7 @@
 package datadog.trace.agent.test;
 
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
 import datadog.opentracing.DDSpan;
 import datadog.opentracing.DDTracer;
 import datadog.opentracing.decorators.AbstractDecorator;
@@ -12,10 +14,18 @@ import java.lang.instrument.ClassFileTransformer;
 import java.lang.instrument.Instrumentation;
 import java.util.List;
 import java.util.concurrent.Phaser;
+import java.util.concurrent.atomic.AtomicInteger;
+import lombok.extern.slf4j.Slf4j;
 import net.bytebuddy.agent.ByteBuddyAgent;
+import net.bytebuddy.agent.builder.AgentBuilder;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.dynamic.DynamicType;
+import net.bytebuddy.utility.JavaModule;
+import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
+import org.slf4j.LoggerFactory;
 import org.spockframework.runtime.model.SpecMetadata;
 import spock.lang.Specification;
 
@@ -34,6 +44,7 @@ import spock.lang.Specification;
  *       in an initialized state.
  * </ul>
  */
+@Slf4j
 @SpecMetadata(filename = "AgentTestRunner.java", line = 0)
 public abstract class AgentTestRunner extends Specification {
   /**
@@ -43,13 +54,18 @@ public abstract class AgentTestRunner extends Specification {
    */
   public static final ListWriter TEST_WRITER;
 
-  private static final Tracer TEST_TRACER;
+  protected static final Tracer TEST_TRACER;
+  private static final AtomicInteger INSTRUMENTATION_ERROR_COUNT = new AtomicInteger();
+
   private static final Instrumentation instrumentation;
   private static ClassFileTransformer activeTransformer = null;
 
   protected static final Phaser WRITER_PHASER = new Phaser();
 
   static {
+    ((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).setLevel(Level.WARN);
+    ((Logger) LoggerFactory.getLogger("datadog")).setLevel(Level.DEBUG);
+
     WRITER_PHASER.register();
     TEST_WRITER =
         new ListWriter() {
@@ -75,13 +91,22 @@ public abstract class AgentTestRunner extends Specification {
     if (null != activeTransformer) {
       throw new IllegalStateException("transformer already in place: " + activeTransformer);
     }
-    activeTransformer = AgentInstaller.installBytebuddyAgent(instrumentation);
+
+    activeTransformer =
+        AgentInstaller.installBytebuddyAgent(instrumentation, new ErrorCountingListener());
     TestUtils.registerOrReplaceGlobalTracer(TEST_TRACER);
   }
 
   @Before
   public void beforeTest() {
     TEST_WRITER.start();
+    INSTRUMENTATION_ERROR_COUNT.set(0);
+    assert TEST_TRACER.activeSpan() == null;
+  }
+
+  @After
+  public void afterTest() {
+    assert INSTRUMENTATION_ERROR_COUNT.get() == 0;
   }
 
   @AfterClass
@@ -89,4 +114,45 @@ public abstract class AgentTestRunner extends Specification {
     instrumentation.removeTransformer(activeTransformer);
     activeTransformer = null;
   }
+
+  private static class ErrorCountingListener implements AgentBuilder.Listener {
+    @Override
+    public void onDiscovery(
+        final String typeName,
+        final ClassLoader classLoader,
+        final JavaModule module,
+        final boolean loaded) {}
+
+    @Override
+    public void onTransformation(
+        final TypeDescription typeDescription,
+        final ClassLoader classLoader,
+        final JavaModule module,
+        final boolean loaded,
+        final DynamicType dynamicType) {}
+
+    @Override
+    public void onIgnored(
+        final TypeDescription typeDescription,
+        final ClassLoader classLoader,
+        final JavaModule module,
+        final boolean loaded) {}
+
+    @Override
+    public void onError(
+        final String typeName,
+        final ClassLoader classLoader,
+        final JavaModule module,
+        final boolean loaded,
+        final Throwable throwable) {
+      INSTRUMENTATION_ERROR_COUNT.incrementAndGet();
+    }
+
+    @Override
+    public void onComplete(
+        final String typeName,
+        final ClassLoader classLoader,
+        final JavaModule module,
+        final boolean loaded) {}
+  }
 }

dd-java-agent/testing/testing.gradle

@@ -6,6 +6,7 @@ dependencies {
   compile deps.slf4j
   compile deps.opentracing
   compile deps.spock
+  compile deps.testLogging
 
   compile project(':dd-trace-ot')
   compile project(':dd-java-agent:tooling')

dd-java-agent/tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java

@@ -26,7 +26,8 @@ public class AgentInstaller {
    * @param inst Java Instrumentation used to install bytebuddy
    * @return the agent's class transformer
    */
-  public static ResettableClassFileTransformer installBytebuddyAgent(final Instrumentation inst) {
+  public static ResettableClassFileTransformer installBytebuddyAgent(
+      final Instrumentation inst, final AgentBuilder.Listener... listeners) {
     AgentBuilder agentBuilder =
         new AgentBuilder.Default()
             .disableClassFormatChanges()
@@ -53,6 +54,9 @@ public class AgentInstaller {
                     .or(
                         classLoaderWithName(
                             "org.codehaus.groovy.runtime.callsite.CallSiteClassLoader")));
+    for (final AgentBuilder.Listener listener : listeners) {
+      agentBuilder = agentBuilder.with(listener);
+    }
     int numInstrumenters = 0;
     for (final Instrumenter instrumenter : ServiceLoader.load(Instrumenter.class)) {
       log.debug("Loading instrumentation {}", instrumenter);

dd-trace-ot/src/main/java/datadog/trace/common/writer/ListWriter.java

@@ -7,6 +7,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 /** List writer used by tests mostly */
 public class ListWriter extends CopyOnWriteArrayList<List<DDSpan>> implements Writer {
@@ -30,7 +32,7 @@ public class ListWriter extends CopyOnWriteArrayList<List<DDSpan>> implements Wr
     }
   }
 
-  public void waitForTraces(final int number) throws InterruptedException {
+  public void waitForTraces(final int number) throws InterruptedException, TimeoutException {
     final CountDownLatch latch = new CountDownLatch(number);
     synchronized (latches) {
       if (size() >= number) {
@@ -38,7 +40,9 @@ public class ListWriter extends CopyOnWriteArrayList<List<DDSpan>> implements Wr
       }
       latches.add(latch);
     }
-    latch.await();
+    if (!latch.await(5, TimeUnit.SECONDS)) {
+      throw new TimeoutException("Timeout waiting for " + number + " trace(s).");
+    }
   }
 
   @Override

settings.gradle

@@ -11,6 +11,7 @@ include ':dd-trace-api'
 include ':dd-java-agent:instrumentation:apache-httpclient-4.3'
 include ':dd-java-agent:instrumentation:aws-sdk'
 include ':dd-java-agent:instrumentation:datastax-cassandra-3.2'
+include ':dd-java-agent:instrumentation:jax-rs'
 include ':dd-java-agent:instrumentation:jdbc'
 include ':dd-java-agent:instrumentation:jms-1'
 include ':dd-java-agent:instrumentation:jms-2'