diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java index aaa66b14cf..d06bd32b67 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java +++ b/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 classLoaderHasClassWithMethod( + final String className, final String methodName, final Class... methodArgs) { + return new ClassLoaderHasClassWithMethodMatcher(className, methodName, methodArgs); + } + private static class SkipClassLoaderMatcher extends ElementMatcher.Junction.AbstractBase { public static final SkipClassLoaderMatcher INSTANCE = new SkipClassLoaderMatcher(); @@ -191,4 +196,50 @@ public class ClassLoaderMatcher { return false; } } + + public static class ClassLoaderHasClassWithMethodMatcher + extends ElementMatcher.Junction.AbstractBase { + + private final Map cache = + Collections.synchronizedMap(new WeakHashMap()); + + 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; + } + } } diff --git a/dd-java-agent/instrumentation/play-2.4/play-2.4.gradle b/dd-java-agent/instrumentation/play-2.4/play-2.4.gradle new file mode 100644 index 0000000000..45c1dd9812 --- /dev/null +++ b/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*' + } +} diff --git a/dd-java-agent/instrumentation/play/play.gradle b/dd-java-agent/instrumentation/play-2.4/play-2.6-testing/play-2.6-testing.gradle similarity index 58% rename from dd-java-agent/instrumentation/play/play.gradle rename to dd-java-agent/instrumentation/play-2.4/play-2.6-testing/play-2.6-testing.gradle index 3cbdedafeb..2c985375da 100644 --- a/dd-java-agent/instrumentation/play/play.gradle +++ b/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*' + } +} diff --git a/dd-java-agent/instrumentation/play/src/test/groovy/PlayTest.groovy b/dd-java-agent/instrumentation/play-2.4/play-2.6-testing/src/test/groovy/Play26Test.groovy similarity index 97% rename from dd-java-agent/instrumentation/play/src/test/groovy/PlayTest.groovy rename to dd-java-agent/instrumentation/play-2.4/play-2.6-testing/src/test/groovy/Play26Test.groovy index c45c059608..bc192c886f 100644 --- a/dd-java-agent/instrumentation/play/src/test/groovy/PlayTest.groovy +++ b/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() } diff --git a/dd-java-agent/instrumentation/play/src/test/scala/PlayTestUtils.scala b/dd-java-agent/instrumentation/play-2.4/play-2.6-testing/src/test/scala/Play26TestUtils.scala similarity index 83% rename from dd-java-agent/instrumentation/play/src/test/scala/PlayTestUtils.scala rename to dd-java-agent/instrumentation/play-2.4/play-2.6-testing/src/test/scala/Play26TestUtils.scala index 93fb96dd56..3fbe377675 100644 --- a/dd-java-agent/instrumentation/play/src/test/scala/PlayTestUtils.scala +++ b/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 {} diff --git a/dd-java-agent/instrumentation/play/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 similarity index 85% rename from dd-java-agent/instrumentation/play/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java rename to dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java index 0b7b85cad0..7821da279f 100644 --- a/dd-java-agent/instrumentation/play/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,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 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 { + public static class RequestCallback extends scala.runtime.AbstractFunction1 { private final Span span; public RequestCallback(Span span) { diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/Play24Test.groovy b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/Play24Test.groovy new file mode 100644 index 0000000000..bd21e4ddab --- /dev/null +++ b/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" + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/scala/Play24TestUtils.scala b/dd-java-agent/instrumentation/play-2.4/src/test/scala/Play24TestUtils.scala new file mode 100644 index 0000000000..1a00476917 --- /dev/null +++ b/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 {} diff --git a/settings.gradle b/settings.gradle index e2545e5ee2..d04949daa8 100644 --- a/settings.gradle +++ b/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'