Play 2.4 and 2.6 testing

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

@@ -30,6 +30,11 @@ public class ClassLoaderMatcher {
     return new ClassLoaderHasClassWithFieldMatcher(className, fieldName);
   }
 
+  public static ElementMatcher.Junction.AbstractBase<ClassLoader> classLoaderHasClassWithMethod(
+      final String className, final String methodName, final Class... methodArgs) {
+    return new ClassLoaderHasClassWithMethodMatcher(className, methodName, methodArgs);
+  }
+
   private static class SkipClassLoaderMatcher
       extends ElementMatcher.Junction.AbstractBase<ClassLoader> {
     public static final SkipClassLoaderMatcher INSTANCE = new SkipClassLoaderMatcher();
@@ -191,4 +196,50 @@ public class ClassLoaderMatcher {
       return false;
     }
   }
+
+  public static class ClassLoaderHasClassWithMethodMatcher
+      extends ElementMatcher.Junction.AbstractBase<ClassLoader> {
+
+    private final Map<ClassLoader, Boolean> cache =
+        Collections.synchronizedMap(new WeakHashMap<ClassLoader, Boolean>());
+
+    private final String className;
+    private final String methodName;
+    private final Class[] methodArgs;
+
+    private ClassLoaderHasClassWithMethodMatcher(
+        final String className, final String methodName, final Class... methodArgs) {
+      this.className = className;
+      this.methodName = methodName;
+      this.methodArgs = methodArgs;
+    }
+
+    @Override
+    public boolean matches(final ClassLoader target) {
+      if (target != null) {
+        synchronized (target) {
+          if (cache.containsKey(target)) {
+            return cache.get(target);
+          }
+          try {
+            final Class<?> aClass = Class.forName(className, false, target);
+            if (aClass.isInterface()) {
+              aClass.getMethod(methodName, methodArgs);
+            } else {
+              aClass.getDeclaredMethod(methodName, methodArgs);
+            }
+            cache.put(target, true);
+            return true;
+          } catch (final ClassNotFoundException e) {
+            cache.put(target, false);
+            return false;
+          } catch (final NoSuchMethodException e) {
+            cache.put(target, false);
+            return false;
+          }
+        }
+      }
+      return false;
+    }
+  }
 }

dd-java-agent/instrumentation/play-2.4/play-2.4.gradle

@@ -0,0 +1,26 @@
+apply from: "${rootDir}/gradle/java.gradle"
+apply from: "${rootDir}/gradle/test-with-scala.gradle"
+
+dependencies {
+  compileOnly group: 'com.typesafe.play', name: 'play_2.11', version: '2.4.0'
+
+  compile project(':dd-trace-api')
+  compile project(':dd-java-agent:agent-tooling')
+  compile deps.bytebuddy
+  compile deps.opentracing
+  compile deps.autoservice
+
+  testCompile group: 'org.scala-lang', name: 'scala-library', version: '2.11.12'
+  testCompile group: 'com.typesafe.play', name: 'play_2.11', version: '2.4.0'
+  testCompile group: 'com.typesafe.play', name: 'play-test_2.11', version: '2.4.0'
+  testCompile project(':dd-java-agent:testing')
+  testCompile project(':dd-java-agent:instrumentation:java-concurrent')
+  testCompile project(':dd-java-agent:instrumentation:trace-annotation')
+  testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0'
+}
+
+test {
+  if (JavaVersion.current().isJava8Compatible()) {
+    exclude '*Play*Test*'
+  }
+}

dd-java-agent/instrumentation/play-2.4/play-2.6-testing/play-2.6-testing.gradle

@@ -1,10 +1,8 @@
 apply from: "${rootDir}/gradle/java.gradle"
 apply from: "${rootDir}/gradle/test-with-scala.gradle"
 
-// TODO: Rename to play-{version}
-
 dependencies {
-  compileOnly group: 'com.typesafe.play', name: 'play_2.12', version: '2.6.0'
+  compileOnly group: 'com.typesafe.play', name: 'play_2.11', version: '2.6.0'
 
   compile project(':dd-trace-api')
   compile project(':dd-java-agent:agent-tooling')
@@ -12,10 +10,18 @@ dependencies {
   compile deps.opentracing
   compile deps.autoservice
 
-  testCompile group: 'com.typesafe.play', name: 'play_2.12', version: '2.6.0'
-  testCompile group: 'com.typesafe.play', name: 'play-test_2.12', version: '2.6.0'
+  testCompile group: 'org.scala-lang', name: 'scala-library', version: '2.11.12'
+  testCompile project(':dd-java-agent:instrumentation:play-2.4')
+  testCompile group: 'com.typesafe.play', name: 'play_2.11', version: '2.6.0'
+  testCompile group: 'com.typesafe.play', name: 'play-test_2.11', version: '2.6.0'
   testCompile project(':dd-java-agent:testing')
   testCompile project(':dd-java-agent:instrumentation:java-concurrent')
   testCompile project(':dd-java-agent:instrumentation:trace-annotation')
   testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0'
 }
+
+test {
+  if (JavaVersion.current().isJava8Compatible()) {
+    exclude '*Play*Test*'
+  }
+}

dd-java-agent/instrumentation/play-2.4/play-2.6-testing/src/test/groovy/Play26Test.groovy

@@ -7,7 +7,7 @@ import play.api.test.TestServer
 import play.test.Helpers
 import spock.lang.Shared
 
-class PlayTest extends AgentTestRunner {
+class Play26Test extends AgentTestRunner {
   static {
     System.setProperty("dd.integration.java_concurrent.enabled", "true")
     System.setProperty("dd.integration.play.enabled", "true")
@@ -19,7 +19,7 @@ class PlayTest extends AgentTestRunner {
   TestServer testServer
 
   def setupSpec() {
-    testServer = Helpers.testServer(port, PlayTestUtils.buildTestApp())
+    testServer = Helpers.testServer(port, Play26TestUtils.buildTestApp())
     testServer.start()
   }
 

dd-java-agent/instrumentation/play-2.4/play-2.6-testing/src/test/scala/Play26TestUtils.scala

@@ -5,12 +5,14 @@ import play.api.routing.sird._
 import java.lang.reflect.Field
 
 import datadog.trace.api.Trace
+import play.api.libs.typedmap.TypedKey
+import play.api.mvc.request.RequestAttrKey
 
 import scala.concurrent.{Await, Future}
 import scala.concurrent.duration._
 
 
-object PlayTestUtils {
+object Play26TestUtils {
   def buildTestApp(): play.Application = {
     // build play.api.Application with desired setting and pass into play.Application for testing
     val apiApp :play.api.Application = new play.api.inject.guice.GuiceApplicationBuilder()
@@ -56,9 +58,11 @@ object HandlerSetter {
   def setHandler(req: RequestHeader, path: String): Unit = {
     val f: Field = req.getClass().getDeclaredField("attrs")
     f.setAccessible(true)
-    f.set(req, req.attrs.updated(play.routing.Router.Attrs.HANDLER_DEF.underlying(), new HandlerDef(null, null, null, null, null, null, path, null, null)))
+    f.set(req, req.attrs
+      .updated(play.routing.Router.Attrs.HANDLER_DEF.underlying(), new HandlerDef(null, null, null, null, null, null, path, null, null))
+      .updated(RequestAttrKey.Tags, Map(play.routing.Router.Tags.ROUTE_PATTERN -> path)))
     f.setAccessible(false)
   }
 }
 
-class PlayTestUtils {}
+class Play26TestUtils {}

dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java

@@ -1,13 +1,12 @@
 package datadog.trace.instrumentation.play;
 
+import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClassWithMethod;
+import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses;
 import static net.bytebuddy.matcher.ElementMatchers.*;
 
 import akka.japi.JavaPartialFunction;
 import com.google.auto.service.AutoService;
-import datadog.trace.agent.tooling.DDAdvice;
-import datadog.trace.agent.tooling.DDTransformers;
-import datadog.trace.agent.tooling.HelperInjector;
-import datadog.trace.agent.tooling.Instrumenter;
+import datadog.trace.agent.tooling.*;
 import datadog.trace.api.DDSpanTypes;
 import datadog.trace.api.DDTags;
 import io.opentracing.Scope;
@@ -25,9 +24,6 @@ import org.slf4j.LoggerFactory;
 import play.api.mvc.Action;
 import play.api.mvc.Request;
 import play.api.mvc.Result;
-import play.routing.HandlerDef;
-import play.routing.Router;
-import scala.Function1;
 import scala.Option;
 import scala.Tuple2;
 import scala.concurrent.Future;
@@ -53,7 +49,16 @@ public final class PlayInstrumentation extends Instrumenter.Configurable {
   @Override
   public AgentBuilder apply(final AgentBuilder agentBuilder) {
     return agentBuilder
-        .type(hasSuperType(named("play.api.mvc.Action")))
+        .type(
+            hasSuperType(named("play.api.mvc.Action")),
+            classLoaderHasClasses(
+                    "akka.japi.JavaPartialFunction",
+                    "play.api.mvc.Action",
+                    "play.api.mvc.Result",
+                    "scala.Option",
+                    "scala.Tuple2",
+                    "scala.concurrent.Future")
+                .and(classLoaderHasClassWithMethod("play.api.mvc.Request", "tags")))
         .and(
             declaresMethod(
                 named("executionContext").and(returns(named("scala.concurrent.ExecutionContext")))))
@@ -102,13 +107,14 @@ public final class PlayInstrumentation extends Instrumenter.Configurable {
         @Advice.Argument(0) final Request req,
         @Advice.Return(readOnly = false) Future<Result> responseFuture) {
       // more about routes here: https://github.com/playframework/playframework/blob/master/documentation/manual/releases/release26/migration26/Migration26.md
-      final Option handlerOption = req.attrs().get(Router.Attrs.HANDLER_DEF.underlying());
-      if (!handlerOption.isEmpty()) {
-        final HandlerDef handlerDef = (HandlerDef) handlerOption.get();
-        scope.span().setTag(Tags.HTTP_URL.getKey(), handlerDef.path());
-        scope.span().setOperationName(handlerDef.path());
-        scope.span().setTag(DDTags.RESOURCE_NAME, req.method() + " " + handlerDef.path());
+      final Option pathOption = req.tags().get("ROUTE_PATTERN");
+      if (!pathOption.isEmpty()) {
+        final String path = (String) pathOption.get();
+        scope.span().setTag(Tags.HTTP_URL.getKey(), path);
+        scope.span().setOperationName(path);
+        scope.span().setTag(DDTags.RESOURCE_NAME, req.method() + " " + path);
       }
+
       scope.span().setTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER);
       scope.span().setTag(Tags.HTTP_METHOD.getKey(), req.method());
       scope.span().setTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET);
@@ -179,7 +185,7 @@ public final class PlayInstrumentation extends Instrumenter.Configurable {
   }
 
   @Slf4j
-  public static class RequestCallback implements Function1<Result, Result> {
+  public static class RequestCallback extends scala.runtime.AbstractFunction1<Result, Result> {
     private final Span span;
 
     public RequestCallback(Span span) {

dd-java-agent/instrumentation/play-2.4/src/test/groovy/Play24Test.groovy

@@ -0,0 +1,154 @@
+import datadog.opentracing.DDSpan
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.agent.test.TestUtils
+import okhttp3.OkHttpClient
+import okhttp3.Request
+import play.api.test.TestServer
+import play.test.Helpers
+import spock.lang.Shared
+
+class Play24Test extends AgentTestRunner {
+  static {
+    System.setProperty("dd.integration.java_concurrent.enabled", "true")
+    System.setProperty("dd.integration.play.enabled", "true")
+  }
+
+  @Shared
+  int port = TestUtils.randomOpenPort()
+  @Shared
+  TestServer testServer
+
+  def setupSpec() {
+    testServer = Helpers.testServer(port, Play24TestUtils.buildTestApp())
+    testServer.start()
+  }
+
+  def cleanupSpec() {
+    testServer.stop()
+  }
+
+  @Override
+  void afterTest() {
+    // Ignore failures to instrument sun proxy classes
+  }
+
+  def "request traces" () {
+    setup:
+    OkHttpClient client = new OkHttpClient.Builder().build()
+    def request = new Request.Builder()
+      .url("http://localhost:$port/helloplay/spock")
+      .header("x-datadog-trace-id", "123")
+      .header("x-datadog-parent-id", "456")
+      .get()
+      .build()
+    def response = client.newCall(request).execute()
+    TEST_WRITER.waitForTraces(1)
+    DDSpan[] playTrace = TEST_WRITER.get(0)
+    DDSpan root = playTrace[0]
+
+    expect:
+    testServer != null
+    response.code() == 200
+    response.body().string() == "hello spock"
+
+    // async work is linked to play trace
+    playTrace.size() == 2
+    playTrace[1].operationName == 'TracedWork$.doWork'
+
+    root.traceId == 123
+    root.parentId == 456
+    root.serviceName == "unnamed-java-app"
+    root.operationName == "/helloplay/:from"
+    root.resourceName == "GET /helloplay/:from"
+    !root.context().getErrorFlag()
+    root.context().tags["http.status_code"] == 200
+    root.context().tags["http.url"] == "/helloplay/:from"
+    root.context().tags["http.method"] == "GET"
+    root.context().tags["span.kind"] == "server"
+    root.context().tags["component"] == "play-action"
+  }
+
+  def "5xx errors trace" () {
+    setup:
+    OkHttpClient client = new OkHttpClient.Builder().build()
+    def request = new Request.Builder()
+      .url("http://localhost:$port/make-error")
+      .get()
+      .build()
+    def response = client.newCall(request).execute()
+    TEST_WRITER.waitForTraces(1)
+    DDSpan[] playTrace = TEST_WRITER.get(0)
+    DDSpan root = playTrace[0]
+
+    expect:
+    testServer != null
+    response.code() == 500
+
+    root.serviceName == "unnamed-java-app"
+    root.operationName == "/make-error"
+    root.resourceName == "GET /make-error"
+    root.context().getErrorFlag()
+    root.context().tags["http.status_code"] == 500
+    root.context().tags["http.url"] == "/make-error"
+    root.context().tags["http.method"] == "GET"
+    root.context().tags["span.kind"] == "server"
+    root.context().tags["component"] == "play-action"
+  }
+
+  def "error thrown in request" () {
+    setup:
+    OkHttpClient client = new OkHttpClient.Builder().build()
+    def request = new Request.Builder()
+      .url("http://localhost:$port/exception")
+      .get()
+      .build()
+    def response = client.newCall(request).execute()
+    TEST_WRITER.waitForTraces(1)
+    DDSpan[] playTrace = TEST_WRITER.get(0)
+    DDSpan root = playTrace[0]
+
+    expect:
+    testServer != null
+    response.code() == 500
+
+    root.context().getErrorFlag()
+    root.context().tags["error.msg"] == "oh no"
+    root.context().tags["error.type"] == RuntimeException.getName()
+
+    root.serviceName == "unnamed-java-app"
+    root.operationName == "/exception"
+    root.resourceName == "GET /exception"
+    root.context().tags["http.status_code"] == 500
+    root.context().tags["http.url"] == "/exception"
+    root.context().tags["http.method"] == "GET"
+    root.context().tags["span.kind"] == "server"
+    root.context().tags["component"] == "play-action"
+  }
+
+  def "4xx errors trace" () {
+    setup:
+    OkHttpClient client = new OkHttpClient.Builder().build()
+    def request = new Request.Builder()
+      .url("http://localhost:$port/nowhere")
+      .get()
+      .build()
+    def response = client.newCall(request).execute()
+    TEST_WRITER.waitForTraces(1)
+    DDSpan[] playTrace = TEST_WRITER.get(0)
+    DDSpan root = playTrace[0]
+
+    expect:
+    testServer != null
+    response.code() == 404
+
+    root.serviceName == "unnamed-java-app"
+    root.operationName == "play.request"
+    root.resourceName == "404"
+    !root.context().getErrorFlag()
+    root.context().tags["http.status_code"] == 404
+    root.context().tags["http.url"] == null
+    root.context().tags["http.method"] == "GET"
+    root.context().tags["span.kind"] == "server"
+    root.context().tags["component"] == "play-action"
+  }
+}

dd-java-agent/instrumentation/play-2.4/src/test/scala/Play24TestUtils.scala

@@ -0,0 +1,76 @@
+import java.lang.reflect.Field
+
+import play.api.mvc.Action
+import play.api.routing.Router
+import play.api.mvc._
+import play.api.routing.sird._
+import datadog.trace.api.Trace
+import play.inject.DelegateInjector
+
+import scala.concurrent.{Await, Future}
+import scala.concurrent.duration._
+import play.api.inject.bind
+
+import scala.concurrent.ExecutionContext.Implicits.global
+
+object Play24TestUtils {
+  def buildTestApp(): play.Application = {
+    // build play.api.Application with desired setting and pass into play.Application for testing
+    val apiApp :play.api.Application = new play.api.inject.guice.GuiceApplicationBuilder()
+        .overrides(bind[Router].toInstance(Router.from {
+          case GET(p"/helloplay/$from") => Action { req: RequestHeader =>
+            HandlerSetter.setHandler(req, "/helloplay/:from")
+            val f: Future[String] = Future[String] {
+              TracedWork.doWork()
+              from
+            }
+            Results.Ok(s"hello " + Await.result(f, 5 seconds))
+          }
+          case GET(p"/make-error") => Action { req: RequestHeader =>
+            HandlerSetter.setHandler(req, "/make-error")
+            Results.InternalServerError("Really sorry...")
+          }
+          case GET(p"/exception") => Action { req: RequestHeader =>
+            HandlerSetter.setHandler(req, "/exception")
+            if (System.currentTimeMillis() > 0) {
+              throw new RuntimeException("oh no")
+            }
+            Results.Ok("hello")
+          }
+          case _ => Action {
+            Results.NotFound("Sorry..")
+          }
+        }))
+      .build()
+
+
+    return new play.DefaultApplication(apiApp, new DelegateInjector(apiApp.injector))
+  }
+}
+
+object TracedWork {
+  @Trace
+  def doWork(): Unit = {
+  }
+}
+
+object HandlerSetter {
+  def setHandler(req: RequestHeader, path: String): Unit = {
+    val rh = getField(req, "rh$1")
+    val newTags: Map[String, String] = Map(Router.Tags.RoutePattern -> path)
+    val f: Field = rh.getClass().getDeclaredField("tags")
+    f.setAccessible(true)
+    f.set(rh, newTags)
+    f.setAccessible(false)
+  }
+
+  private def getField(o: Object, fieldName :String): Object = {
+    val f: Field = o.getClass().getDeclaredField(fieldName)
+    f.setAccessible(true)
+    val result: Object = f.get(o)
+    f.setAccessible(false)
+    return result
+  }
+}
+
+class Play24TestUtils {}

settings.gradle

@@ -30,7 +30,6 @@ include ':dd-java-agent:instrumentation:mongo-3.1'
 include ':dd-java-agent:instrumentation:mongo-async-3.3'
 include ':dd-java-agent:instrumentation:okhttp-3'
 include ':dd-java-agent:instrumentation:osgi-classloading'
-include ':dd-java-agent:instrumentation:play'
 include ':dd-java-agent:instrumentation:servlet-2'
 include ':dd-java-agent:instrumentation:servlet-3'
 include ':dd-java-agent:instrumentation:spring-web'
@@ -38,6 +37,10 @@ include ':dd-java-agent:instrumentation:trace-annotation'
 
 
 if (JavaVersion.current().isJava8Compatible()) {
+  // java 8 only instrumentation
+  include ':dd-java-agent:instrumentation:play-2.4'
+  include ':dd-java-agent:instrumentation:play-2.4:play-2.6-testing'
+
   // benchmark
   include ':dd-java-agent:benchmark'
   include ':dd-java-agent:benchmark-integration'