diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy index 696d153a67..b37d0c169d 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy @@ -27,10 +27,10 @@ class AkkaHttpClientInstrumentationTest extends HttpClientTest - // FIXME: Callback should be here instead. - // callback?.call() - //} + //.whenComplete { result, error -> + // FIXME: Callback should be here instead. + // callback?.call() + //} .toCompletableFuture() .get() } finally { @@ -51,6 +51,7 @@ class AkkaHttpClientInstrumentationTest extends HttpClientTest { @Shared def clientConfig = DefaultAsyncHttpClientConfig.Builder.newInstance().setRequestTimeout(TimeUnit.SECONDS.toMillis(10).toInteger()) @Shared + @AutoCleanup AsyncHttpClient asyncHttpClient = asyncHttpClient(clientConfig) @Override 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 index 3b6ec2e4c3..a5dbe8dbe5 100644 --- a/dd-java-agent/instrumentation/play-2.4/play-2.4.gradle +++ b/dd-java-agent/instrumentation/play-2.4/play-2.4.gradle @@ -1,50 +1,58 @@ -// Set properties before any plugins get loaded ext { minJavaVersionForTests = JavaVersion.VERSION_1_8 -} - -apply from: "${rootDir}/gradle/java.gradle" -apply from: "${rootDir}/gradle/test-with-scala.gradle" - -apply plugin: 'org.unbroken-dome.test-sets' - -testSets { - latestDepTest + // Play doesn't work with Java 9+ until 2.6.12 + maxJavaVersionForTests = JavaVersion.VERSION_1_8 } muzzle { pass { group = 'com.typesafe.play' module = 'play_2.11' - versions = '[2.4.0,2.7.0-M1)' + versions = '[2.4.0,2.6)' + assertInverse = true } pass { group = 'com.typesafe.play' module = 'play_2.12' - versions = '[2.4.0,2.7.0-M1)' + versions = '[2.4.0,2.6)' + assertInverse = true + } + pass { + group = 'com.typesafe.play' + module = 'play_2.13' + versions = '[2.4.0,2.6)' + assertInverse = true + } +} + +apply from: "${rootDir}/gradle/java.gradle" + +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' } } dependencies { - compileOnly group: 'com.typesafe.play', name: 'play_2.11', version: '2.4.0' - - testCompile deps.scala - 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 group: 'com.typesafe.play', name: 'play-ws_2.11', version: '2.4.0' + main_java8Compile group: 'com.typesafe.play', name: 'play_2.11', version: '2.4.0' testCompile project(':dd-java-agent:instrumentation:java-concurrent') - testCompile project(':dd-java-agent:instrumentation:trace-annotation') + testCompile project(':dd-java-agent:instrumentation:netty-4.0') + testCompile project(':dd-java-agent:instrumentation:netty-4.1') testCompile project(':dd-java-agent:instrumentation:akka-http-10.0') - testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0' - latestDepTestCompile deps.scala - latestDepTestCompile group: 'com.typesafe.play', name: 'play_2.11', version: '2.6.+' - latestDepTestCompile group: 'com.typesafe.play', name: 'play-test_2.11', version: '2.6.+' - latestDepTestCompile group: 'com.typesafe.play', name: 'play-ws_2.11', version: '2.6.+' -} + // Before 2.5, play used netty 3.x which isn't supported, so for better test consistency, we test with just 2.5 + testCompile group: 'com.typesafe.play', name: 'play-java_2.11', version: '2.5.0' + testCompile group: 'com.typesafe.play', name: 'play-java-ws_2.11', version: '2.5.0' + testCompile (group: 'com.typesafe.play', name: 'play-test_2.11', version: '2.5.0') { + exclude group: 'org.eclipse.jetty.websocket', module: 'websocket-client' + } -compileLatestDepTestGroovy { - classpath = classpath.plus(files(compileLatestDepTestScala.destinationDir)) - dependsOn compileLatestDepTestScala + latestDepTestCompile group: 'com.typesafe.play', name: 'play-java_2.11', version: '2.5.+' + latestDepTestCompile group: 'com.typesafe.play', name: 'play-java-ws_2.11', version: '2.5.+' + latestDepTestCompile (group: 'com.typesafe.play', name: 'play-test_2.11', version: '2.5.+') { + exclude group: 'org.eclipse.jetty.websocket', module: 'websocket-client' + } } diff --git a/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/groovy/Play26Test.groovy b/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/groovy/Play26Test.groovy deleted file mode 100644 index 60eec3f825..0000000000 --- a/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/groovy/Play26Test.groovy +++ /dev/null @@ -1,114 +0,0 @@ -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.agent.test.utils.OkHttpUtils -import datadog.trace.agent.test.utils.PortUtils -import datadog.trace.api.DDSpanTypes -import okhttp3.Request -import play.api.test.TestServer -import play.test.Helpers -import spock.lang.Shared - -class Play26Test extends AgentTestRunner { - @Shared - int port = PortUtils.randomOpenPort() - @Shared - TestServer testServer - - @Shared - def client = OkHttpUtils.client() - - def setupSpec() { - testServer = Helpers.testServer(port, Play26TestUtils.buildTestApp()) - testServer.start() - } - - def cleanupSpec() { - testServer.stop() - } - - def "request traces"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/$path") - .header("x-datadog-trace-id", "123") - .header("x-datadog-parent-id", "456") - .get() - .build() - def response = client.newCall(request).execute() - - expect: - testServer != null - response.code() == status - if (body instanceof Class) { - body.isInstance(response.body()) - } else { - response.body().string() == body - } - - assertTraces(1) { - trace(0, extraSpans ? 3 : 2) { - span(0) { - traceId "123" - parentId "456" - serviceName "unnamed-java-app" - operationName "akka-http.request" - resourceName status == 404 ? "404" : "GET $route" - spanType DDSpanTypes.HTTP_SERVER - errored isError - tags { - "http.status_code" status - "http.url" "http://localhost:$port/$path" - "http.method" "GET" - "span.kind" "server" - "component" "akka-http-server" - if (isError) { - "error" true - } - defaultTags(true) - } - } - span(1) { - operationName "play.request" - resourceName status == 404 ? "404" : "GET $route" - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - errored isError - tags { - "http.status_code" status - "http.url" "http://localhost:$port/$path" - "http.method" "GET" - "peer.ipv4" "127.0.0.1" - "span.kind" "server" - "component" "play-action" - if (isError) { - if (exception) { - errorTags(exception.class, exception.message) - } else { - "error" true - } - } - defaultTags() - } - } - if (extraSpans) { - span(2) { - operationName "TracedWork\$.doWork" - childOf(span(1)) - tags { - "component" "trace" - defaultTags() - } - } - } - } - } - - where: - path | route | body | status | isError | exception - "helloplay/spock" | "/helloplay/:from" | "hello spock" | 200 | false | null - "make-error" | "/make-error" | "Really sorry..." | 500 | true | null - "exception" | "/exception" | String | 500 | true | new RuntimeException("oh no") - "nowhere" | "/nowhere" | "Really sorry..." | 404 | false | null - - extraSpans = !isError && status != 404 - } -} diff --git a/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/scala/Play26TestUtils.scala b/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/scala/Play26TestUtils.scala deleted file mode 100644 index d24b3f7c07..0000000000 --- a/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/scala/Play26TestUtils.scala +++ /dev/null @@ -1,68 +0,0 @@ -import java.lang.reflect.Field - -import datadog.trace.api.Trace -import play.api.mvc.request.RequestAttrKey -import play.api.mvc.{Action, _} -import play.api.routing.sird._ -import play.api.routing.{HandlerDef, Router} - -import scala.concurrent.duration._ -import scala.concurrent.{Await, Future} - -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() - .requireAtInjectOnConstructors(true) - .router( - Router.from { - case GET(p"/helloplay/$from") => Action { req: RequestHeader => - HandlerSetter.setHandler(req, "/helloplay/:from") - // FIXME: Add WS request for testing. - // implicit val application = Play.current - // val wsRequest = WS.url("http://localhost:" + port).get() - val f: Future[String] = Future[String] { - TracedWork.doWork() - from - }(Action.executionContext) - 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 play.inject.guice.GuiceApplicationBuilder().build().injector()) - } -} - -object TracedWork { - @Trace - def doWork(): Unit = { - } -} - -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)) - .updated(RequestAttrKey.Tags, Map(play.routing.Router.Tags.ROUTE_PATTERN -> path))) - f.setAccessible(false) - } -} - -class Play26TestUtils {} diff --git a/dd-java-agent/instrumentation/play-2.4/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 deleted file mode 100644 index 661f1b77b8..0000000000 --- a/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java +++ /dev/null @@ -1,216 +0,0 @@ -package datadog.trace.instrumentation.play; - -import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static datadog.trace.instrumentation.play.PlayHttpServerDecorator.DECORATE; -import static java.util.Collections.singletonMap; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.returns; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; - -import akka.japi.JavaPartialFunction; -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.api.DDTags; -import datadog.trace.context.TraceScope; -import io.opentracing.Scope; -import io.opentracing.Span; -import io.opentracing.SpanContext; -import io.opentracing.propagation.Format; -import io.opentracing.propagation.TextMap; -import io.opentracing.tag.Tags; -import io.opentracing.util.GlobalTracer; -import java.util.HashMap; -import java.util.Iterator; -import java.util.Map; -import lombok.extern.slf4j.Slf4j; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; -import play.api.mvc.Action; -import play.api.mvc.Request; -import play.api.mvc.Result; -import scala.Option; -import scala.Tuple2; -import scala.concurrent.Future; - -@AutoService(Instrumenter.class) -public final class PlayInstrumentation extends Instrumenter.Default { - - public PlayInstrumentation() { - super("play"); - } - - @Override - public ElementMatcher typeMatcher() { - return safeHasSuperType(named("play.api.mvc.Action")); - } - - @Override - public String[] helperClassNames() { - return new String[] { - "datadog.trace.agent.decorator.BaseDecorator", - "datadog.trace.agent.decorator.ServerDecorator", - "datadog.trace.agent.decorator.HttpServerDecorator", - packageName + ".PlayHttpServerDecorator", - PlayInstrumentation.class.getName() + "$RequestCallback", - PlayInstrumentation.class.getName() + "$RequestError", - PlayInstrumentation.class.getName() + "$PlayHeaders" - }; - } - - @Override - public Map, String> transformers() { - return singletonMap( - named("apply") - .and(takesArgument(0, named("play.api.mvc.Request"))) - .and(returns(named("scala.concurrent.Future"))), - PlayAdvice.class.getName()); - } - - public static class PlayAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static Scope startSpan(@Advice.Argument(0) final Request req) { - final Scope scope; - if (GlobalTracer.get().activeSpan() == null) { - final SpanContext extractedContext; - if (GlobalTracer.get().scopeManager().active() == null) { - extractedContext = - GlobalTracer.get().extract(Format.Builtin.HTTP_HEADERS, new PlayHeaders(req)); - } else { - extractedContext = null; - } - scope = - GlobalTracer.get() - .buildSpan("play.request") - .asChildOf(extractedContext) - .startActive(false); - } else { - // An upstream framework (e.g. akka-http, netty) has already started the span. - // Do not extract the context. - scope = GlobalTracer.get().buildSpan("play.request").startActive(false); - } - DECORATE.afterStart(scope); - DECORATE.onConnection(scope.span(), req); - - if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { - ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(true); - } - return scope; - } - - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void stopTraceOnResponse( - @Advice.Enter final Scope playControllerScope, - @Advice.This final Object thisAction, - @Advice.Thrown final Throwable throwable, - @Advice.Argument(0) final Request req, - @Advice.Return(readOnly = false) Future responseFuture) { - final Span playControllerSpan = playControllerScope.span(); - - // Call onRequest on return after tags are populated. - DECORATE.onRequest(playControllerSpan, req); - - if (throwable == null) { - responseFuture.onFailure( - new RequestError(playControllerSpan), ((Action) thisAction).executionContext()); - responseFuture = - responseFuture.map( - new RequestCallback(playControllerSpan), ((Action) thisAction).executionContext()); - } else { - DECORATE.onError(playControllerSpan, throwable); - Tags.HTTP_STATUS.set(playControllerSpan, 500); - DECORATE.beforeFinish(playControllerSpan); - playControllerSpan.finish(); - } - playControllerScope.close(); - - final Span rootSpan = GlobalTracer.get().activeSpan(); - - // more about routes here: - // https://github.com/playframework/playframework/blob/master/documentation/manual/releases/release26/migration26/Migration26.md - - final Option pathOption = req.tags().get("ROUTE_PATTERN"); - if (rootSpan != null && !pathOption.isEmpty()) { - // set the resource name on the upstream akka/netty span - final String path = (String) pathOption.get(); - rootSpan.setTag(DDTags.RESOURCE_NAME, req.method() + " " + path); - } - } - } - - public static class PlayHeaders implements TextMap { - private final Request request; - - public PlayHeaders(final Request request) { - this.request = request; - } - - @Override - public Iterator> iterator() { - final scala.collection.Map scalaMap = request.headers().toSimpleMap(); - final Map javaMap = new HashMap<>(scalaMap.size()); - final scala.collection.Iterator> scalaIterator = scalaMap.iterator(); - while (scalaIterator.hasNext()) { - final Tuple2 tuple = scalaIterator.next(); - javaMap.put(tuple._1(), tuple._2()); - } - return javaMap.entrySet().iterator(); - } - - @Override - public void put(final String s, final String s1) { - throw new IllegalStateException("play headers can only be extracted"); - } - } - - @Slf4j - public static class RequestError extends JavaPartialFunction { - private final Span span; - - public RequestError(final Span span) { - this.span = span; - } - - @Override - public Object apply(final Throwable t, final boolean isCheck) throws Exception { - try { - DECORATE.onError(span, t); - DECORATE.beforeFinish(span); - if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { - ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(false); - } - } catch (final Throwable t2) { - log.debug("error in play instrumentation", t); - } finally { - span.finish(); - } - return null; - } - } - - @Slf4j - public static class RequestCallback extends scala.runtime.AbstractFunction1 { - private final Span span; - - public RequestCallback(final Span span) { - this.span = span; - } - - @Override - public Result apply(final Result result) { - if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { - ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(false); - } - try { - DECORATE.onResponse(span, result); - DECORATE.beforeFinish(span); - } catch (final Throwable t) { - log.debug("error in play instrumentation", t); - } finally { - span.finish(); - } - return result; - } - } -} diff --git a/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play24/PlayInstrumentation.java b/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play24/PlayInstrumentation.java new file mode 100644 index 0000000000..950fe72d60 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play24/PlayInstrumentation.java @@ -0,0 +1,48 @@ +package datadog.trace.instrumentation.play24; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Map; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class PlayInstrumentation extends Instrumenter.Default { + + public PlayInstrumentation() { + super("play"); + } + + @Override + public ElementMatcher typeMatcher() { + return safeHasSuperType(named("play.api.mvc.Action")); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".PlayHttpServerDecorator", + packageName + ".RequestCompleteCallback", + packageName + ".PlayHeaders", + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("apply") + .and(takesArgument(0, named("play.api.mvc.Request"))) + .and(returns(named("scala.concurrent.Future"))), + packageName + ".PlayAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayAdvice.java b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayAdvice.java new file mode 100644 index 0000000000..ca5ca34d24 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayAdvice.java @@ -0,0 +1,82 @@ +package datadog.trace.instrumentation.play24; + +import static datadog.trace.instrumentation.play24.PlayHttpServerDecorator.DECORATE; + +import datadog.trace.context.TraceScope; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.SpanContext; +import io.opentracing.propagation.Format; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import net.bytebuddy.asm.Advice; +import play.api.mvc.Action; +import play.api.mvc.Request; +import play.api.mvc.Result; +import scala.concurrent.Future; + +public class PlayAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Scope startSpan(@Advice.Argument(0) final Request req) { + final Scope scope; + if (GlobalTracer.get().activeSpan() == null) { + final SpanContext extractedContext; + if (GlobalTracer.get().scopeManager().active() == null) { + extractedContext = + GlobalTracer.get().extract(Format.Builtin.HTTP_HEADERS, new PlayHeaders(req)); + } else { + extractedContext = null; + } + scope = + GlobalTracer.get() + .buildSpan("play.request") + .asChildOf(extractedContext) + .startActive(false); + } else { + // An upstream framework (e.g. akka-http, netty) has already started the span. + // Do not extract the context. + scope = GlobalTracer.get().buildSpan("play.request").startActive(false); + } + DECORATE.afterStart(scope); + DECORATE.onConnection(scope.span(), req); + + if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { + ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(true); + } + return scope; + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopTraceOnResponse( + @Advice.Enter final Scope playControllerScope, + @Advice.This final Object thisAction, + @Advice.Thrown final Throwable throwable, + @Advice.Argument(0) final Request req, + @Advice.Return(readOnly = false) final Future responseFuture) { + final Span playControllerSpan = playControllerScope.span(); + + // Call onRequest on return after tags are populated. + DECORATE.onRequest(playControllerSpan, req); + + if (throwable == null) { + responseFuture.onComplete( + new RequestCompleteCallback(playControllerSpan), + ((Action) thisAction).executionContext()); + } else { + DECORATE.onError(playControllerSpan, throwable); + Tags.HTTP_STATUS.set(playControllerSpan, 500); + DECORATE.beforeFinish(playControllerSpan); + playControllerSpan.finish(); + } + playControllerScope.close(); + + final Span rootSpan = GlobalTracer.get().activeSpan(); + // set the resource name on the upstream akka/netty span + DECORATE.onRequest(rootSpan, req); + } + + // Unused method for muzzle to allow only 2.4-2.5 + public static void muzzleCheck() { + play.libs.Akka.system(); + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHeaders.java b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHeaders.java new file mode 100644 index 0000000000..5c20eeb2c9 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHeaders.java @@ -0,0 +1,33 @@ +package datadog.trace.instrumentation.play24; + +import io.opentracing.propagation.TextMap; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import play.api.mvc.Request; +import scala.Tuple2; + +public class PlayHeaders implements TextMap { + private final Request request; + + public PlayHeaders(final Request request) { + this.request = request; + } + + @Override + public Iterator> iterator() { + final scala.collection.Map scalaMap = request.headers().toSimpleMap(); + final Map javaMap = new HashMap<>(scalaMap.size()); + final scala.collection.Iterator> scalaIterator = scalaMap.iterator(); + while (scalaIterator.hasNext()) { + final Tuple2 tuple = scalaIterator.next(); + javaMap.put(tuple._1(), tuple._2()); + } + return javaMap.entrySet().iterator(); + } + + @Override + public void put(final String s, final String s1) { + throw new IllegalStateException("play headers can only be extracted"); + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayHttpServerDecorator.java b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java similarity index 74% rename from dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayHttpServerDecorator.java rename to dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java index 5bbfed9a85..e6e0468490 100644 --- a/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java @@ -1,9 +1,11 @@ -package datadog.trace.instrumentation.play; +package datadog.trace.instrumentation.play24; import datadog.trace.agent.decorator.HttpServerDecorator; import datadog.trace.api.DDTags; import io.opentracing.Span; import io.opentracing.tag.Tags; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.UndeclaredThrowableException; import java.net.URI; import java.net.URISyntaxException; import lombok.extern.slf4j.Slf4j; @@ -64,7 +66,6 @@ public class PlayHttpServerDecorator extends HttpServerDecorator, Object> { + private final Span span; + + public RequestCompleteCallback(final Span span) { + this.span = span; + } + + @Override + public Object apply(final Try result) { + try { + if (result.isFailure()) { + DECORATE.onError(span, result.failed().get()); + } else { + DECORATE.onResponse(span, result.get()); + } + DECORATE.beforeFinish(span); + if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { + ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(false); + } + } catch (final Throwable t) { + log.debug("error in play instrumentation", t); + } finally { + span.finish(); + } + return null; + } +} 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 deleted file mode 100644 index e10450885e..0000000000 --- a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/Play24Test.groovy +++ /dev/null @@ -1,95 +0,0 @@ -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.agent.test.utils.OkHttpUtils -import datadog.trace.agent.test.utils.PortUtils -import datadog.trace.api.DDSpanTypes -import okhttp3.Request -import play.api.test.TestServer -import play.test.Helpers -import spock.lang.Shared - -class Play24Test extends AgentTestRunner { - @Shared - int port = PortUtils.randomOpenPort() - @Shared - TestServer testServer - - @Shared - def client = OkHttpUtils.client() - - def setupSpec() { - testServer = Helpers.testServer(port, Play24TestUtils.buildTestApp(port)) - testServer.start() - } - - def cleanupSpec() { - testServer.stop() - } - - def "request traces"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/$path") - .header("x-datadog-trace-id", "123") - .header("x-datadog-parent-id", "456") - .get() - .build() - def response = client.newCall(request).execute() - - expect: - testServer != null - response.code() == status - if (body instanceof Class) { - body.isInstance(response.body()) - } else { - response.body().string() == body - } - - assertTraces(1) { - trace(0, extraSpans ? 2 : 1) { - span(0) { - traceId "123" - parentId "456" - operationName "play.request" - resourceName status == 404 ? "404" : "GET $route" - spanType DDSpanTypes.HTTP_SERVER - errored isError - tags { - "http.status_code" status - "http.url" "http://localhost:$port/$path" - "http.method" "GET" - "peer.ipv4" "127.0.0.1" - "span.kind" "server" - "component" "play-action" - if (isError) { - if (exception) { - errorTags(exception.class, exception.message) - } else { - "error" true - } - } - defaultTags(true) - } - } - if (extraSpans) { - span(1) { - operationName "TracedWork\$.doWork" - childOf(span(0)) - tags { - "component" "trace" - defaultTags() - } - } - } - } - } - - where: - path | route | body | status | isError | exception - "helloplay/spock" | "/helloplay/:from" | "hello spock" | 200 | false | null - "make-error" | "/make-error" | "Really sorry..." | 500 | true | null - "exception" | "/exception" | String | 500 | true | new RuntimeException("oh no") - "nowhere" | "/nowhere" | "Really sorry..." | 404 | false | null - - extraSpans = !isError && status != 404 - } -} diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/client/PlayWSClientTest.groovy b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/client/PlayWSClientTest.groovy new file mode 100644 index 0000000000..86768658bf --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/client/PlayWSClientTest.groovy @@ -0,0 +1,53 @@ +package client + +import datadog.trace.agent.test.base.HttpClientTest +import datadog.trace.instrumentation.netty40.client.NettyHttpClientDecorator +import play.libs.ws.WS +import spock.lang.AutoCleanup +import spock.lang.Shared +import spock.lang.Subject + +// Play 2.6+ uses a separately versioned client that shades the underlying dependency +// This means our built in instrumentation won't work. +class PlayWSClientTest extends HttpClientTest { + @Subject + @Shared + @AutoCleanup + def client = WS.newClient(-1) + + @Override + int doRequest(String method, URI uri, Map headers, Closure callback) { + def request = client.url(uri.toString()) + headers.entrySet().each { + request.setHeader(it.key, it.value) + } + + def status = request.execute(method).thenApply { + callback?.call() + it + }.thenApply { + it.status + } + return status.toCompletableFuture().get() + } + + @Override + NettyHttpClientDecorator decorator() { + return NettyHttpClientDecorator.DECORATE + } + + @Override + String expectedOperationName() { + return "netty.client.request" + } + + @Override + boolean testRedirects() { + false + } + + @Override + boolean testConnectionFailure() { + false + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/NettyServerTestInstrumentation.java b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/NettyServerTestInstrumentation.java new file mode 100644 index 0000000000..61cb150a54 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/NettyServerTestInstrumentation.java @@ -0,0 +1,22 @@ +package server; + +import static net.bytebuddy.matcher.ElementMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.test.base.HttpServerTestAdvice; +import datadog.trace.agent.tooling.Instrumenter; +import net.bytebuddy.agent.builder.AgentBuilder; + +@AutoService(Instrumenter.class) +public class NettyServerTestInstrumentation implements Instrumenter { + + @Override + public AgentBuilder instrument(final AgentBuilder agentBuilder) { + return agentBuilder + .type(named("io.netty.handler.codec.ByteToMessageDecoder")) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice( + named("channelRead"), HttpServerTestAdvice.ServerEntryAdvice.class.getName())); + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayAsyncServerTest.groovy b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayAsyncServerTest.groovy new file mode 100644 index 0000000000..a89348d1a6 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayAsyncServerTest.groovy @@ -0,0 +1,52 @@ +package server + +import play.libs.concurrent.HttpExecution +import play.mvc.Results +import play.routing.RoutingDsl +import play.server.Server + +import java.util.concurrent.CompletableFuture +import java.util.function.Supplier + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +class PlayAsyncServerTest extends PlayServerTest { + @Override + Server startServer(int port) { + def router = + new RoutingDsl() + .GET(SUCCESS.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(SUCCESS) { + Results.status(SUCCESS.getStatus(), SUCCESS.getBody()) + } + }, HttpExecution.defaultContext()) + } as Supplier) + .GET(REDIRECT.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(REDIRECT) { + Results.found(REDIRECT.getBody()) + } + }, HttpExecution.defaultContext()) + } as Supplier) + .GET(ERROR.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(ERROR) { + Results.status(ERROR.getStatus(), ERROR.getBody()) + } + }, HttpExecution.defaultContext()) + } as Supplier) + .GET(EXCEPTION.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(EXCEPTION) { + throw new Exception(EXCEPTION.getBody()) + } + }, HttpExecution.defaultContext()) + } as Supplier) + + return Server.forRouter(router.build(), port) + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayServerTest.groovy new file mode 100644 index 0000000000..334b2478fd --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayServerTest.groovy @@ -0,0 +1,105 @@ +package server + +import datadog.opentracing.DDSpan +import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.api.DDSpanTypes +import datadog.trace.instrumentation.netty40.server.NettyHttpServerDecorator +import datadog.trace.instrumentation.play24.PlayHttpServerDecorator +import io.opentracing.tag.Tags +import play.mvc.Results +import play.routing.RoutingDsl +import play.server.Server + +import java.util.function.Supplier + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +class PlayServerTest extends HttpServerTest { + @Override + Server startServer(int port) { + def router = + new RoutingDsl() + .GET(SUCCESS.getPath()).routeTo({ + controller(SUCCESS) { + Results.status(SUCCESS.getStatus(), SUCCESS.getBody()) + } + } as Supplier) + .GET(REDIRECT.getPath()).routeTo({ + controller(REDIRECT) { + Results.found(REDIRECT.getBody()) + } + } as Supplier) + .GET(ERROR.getPath()).routeTo({ + controller(ERROR) { + Results.status(ERROR.getStatus(), ERROR.getBody()) + } + } as Supplier) + .GET(EXCEPTION.getPath()).routeTo({ + controller(EXCEPTION) { + throw new Exception(EXCEPTION.getBody()) + } + } as Supplier) + + return Server.forRouter(router.build(), port) + } + + @Override + void stopServer(Server server) { + server.stop() + } + + @Override + NettyHttpServerDecorator decorator() { + return NettyHttpServerDecorator.DECORATE + } + + @Override + String expectedOperationName() { + return "netty.request" + } + + @Override + boolean hasHandlerSpan() { + true + } + + @Override + // Return the handler span's name + String reorderHandlerSpan() { + "play.request" + } + + boolean testExceptionBody() { + // I can't figure out how to set a proper exception handler to customize the response body. + false + } + + @Override + void handlerSpan(TraceAssert trace, int index, Object parent, ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + serviceName expectedServiceName() + operationName "play.request" + spanType DDSpanTypes.HTTP_SERVER + errored endpoint == ERROR || endpoint == EXCEPTION + childOf(parent as DDSpan) + tags { + "$Tags.COMPONENT.key" PlayHttpServerDecorator.DECORATE.component() + "$Tags.HTTP_STATUS.key" Integer + "$Tags.HTTP_URL.key" String + "$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional + "$Tags.HTTP_METHOD.key" String + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + defaultTags() + if (endpoint == ERROR) { + "$Tags.ERROR.key" true + } else if (endpoint == EXCEPTION) { + errorTags(Exception, EXCEPTION.body) + } + } + } + } +} 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 deleted file mode 100644 index 44045973c5..0000000000 --- a/dd-java-agent/instrumentation/play-2.4/src/test/scala/Play24TestUtils.scala +++ /dev/null @@ -1,82 +0,0 @@ -import java.lang.reflect.Field - -import datadog.trace.api.Trace -import play.api.inject.bind -import play.api.mvc.{Action, _} -import play.api.routing.Router -import play.api.routing.sird._ -import play.inject.DelegateInjector - -import scala.concurrent.ExecutionContext.Implicits.global -import scala.concurrent.duration._ -import scala.concurrent.{Await, Future} - -object Play24TestUtils { - def buildTestApp(port: Int): 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") - // FIXME: Add WS request for testing. - // implicit val application = Play.current - // val wsRequest = WS.url("http://localhost:" + port).get() - val f: Future[String] = Future[String] { - TracedWork.doWork() - from - } - // Await.result(wsRequest, 5 seconds) - Results.Ok(s"hello " + Await.result(f, 5 seconds)) - } - case GET(p"/") => Action { req: RequestHeader => - TracedWork.doWork() - Results.Ok(s"hello") - } - 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/dd-java-agent/instrumentation/play-2.6/.gitignore b/dd-java-agent/instrumentation/play-2.6/.gitignore new file mode 100644 index 0000000000..5292519a25 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/.gitignore @@ -0,0 +1 @@ +logs/ \ No newline at end of file diff --git a/dd-java-agent/instrumentation/play-2.6/play-2.6.gradle b/dd-java-agent/instrumentation/play-2.6/play-2.6.gradle new file mode 100644 index 0000000000..35d0977232 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/play-2.6.gradle @@ -0,0 +1,59 @@ +ext { + minJavaVersionForTests = JavaVersion.VERSION_1_8 + // Play doesn't work with Java 9+ until 2.6.12 + maxJavaVersionForTests = JavaVersion.VERSION_1_8 +} + +muzzle { + pass { + group = 'com.typesafe.play' + module = 'play_$scalaVersion' + versions = '[2.6.0,)' + assertInverse = true + } + pass { + group = 'com.typesafe.play' + module = 'play_2.12' + versions = '[2.6.0,)' + assertInverse = true + } + pass { + group = 'com.typesafe.play' + module = 'play_2.13' + versions = '[2.6.0,)' + assertInverse = true + } +} + +apply from: "${rootDir}/gradle/java.gradle" + +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' + } +} + +def scalaVersion = '2.11' +def playVersion = '2.6.0' + +dependencies { + main_java8Compile group: 'com.typesafe.play', name: "play_$scalaVersion", version: playVersion + + testCompile project(':dd-java-agent:instrumentation:java-concurrent') + testCompile project(':dd-java-agent:instrumentation:netty-4.0') + testCompile project(':dd-java-agent:instrumentation:netty-4.1') + testCompile project(':dd-java-agent:instrumentation:akka-http-10.0') + + testCompile group: 'com.typesafe.play', name: "play-java_$scalaVersion", version: playVersion + // TODO: Play WS is a separately versioned library starting with 2.6 and needs separate instrumentation. + testCompile (group: 'com.typesafe.play', name: "play-test_$scalaVersion", version: playVersion) { + exclude group: 'org.eclipse.jetty.websocket', module: 'websocket-client' + } + + latestDepTestCompile group: 'com.typesafe.play', name: "play-java_$scalaVersion", version: '2.+' + latestDepTestCompile (group: 'com.typesafe.play', name: "play-test_$scalaVersion", version: '2.+') { + exclude group: 'org.eclipse.jetty.websocket', module: 'websocket-client' + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayInstrumentation.java b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayInstrumentation.java new file mode 100644 index 0000000000..6b0dee65ad --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayInstrumentation.java @@ -0,0 +1,48 @@ +package datadog.trace.instrumentation.play26; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Map; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class PlayInstrumentation extends Instrumenter.Default { + + public PlayInstrumentation() { + super("play"); + } + + @Override + public ElementMatcher typeMatcher() { + return safeHasSuperType(named("play.api.mvc.Action")); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".PlayHttpServerDecorator", + packageName + ".RequestCompleteCallback", + packageName + ".PlayHeaders", + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("apply") + .and(takesArgument(0, named("play.api.mvc.Request"))) + .and(returns(named("scala.concurrent.Future"))), + packageName + ".PlayAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayAdvice.java b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayAdvice.java new file mode 100644 index 0000000000..15e11a7a62 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayAdvice.java @@ -0,0 +1,77 @@ +package datadog.trace.instrumentation.play26; + +import static datadog.trace.instrumentation.play26.PlayHttpServerDecorator.DECORATE; + +import datadog.trace.context.TraceScope; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.SpanContext; +import io.opentracing.propagation.Format; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import net.bytebuddy.asm.Advice; +import play.api.mvc.Action; +import play.api.mvc.Request; +import play.api.mvc.Result; +import scala.concurrent.Future; + +public class PlayAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Scope startSpan(@Advice.Argument(0) final Request req) { + final Scope scope; + if (GlobalTracer.get().activeSpan() == null) { + final SpanContext extractedContext; + if (GlobalTracer.get().scopeManager().active() == null) { + extractedContext = + GlobalTracer.get().extract(Format.Builtin.HTTP_HEADERS, new PlayHeaders(req)); + } else { + extractedContext = null; + } + scope = + GlobalTracer.get() + .buildSpan("play.request") + .asChildOf(extractedContext) + .startActive(false); + } else { + // An upstream framework (e.g. akka-http, netty) has already started the span. + // Do not extract the context. + scope = GlobalTracer.get().buildSpan("play.request").startActive(false); + } + DECORATE.afterStart(scope); + DECORATE.onConnection(scope.span(), req); + + if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { + ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(true); + } + return scope; + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopTraceOnResponse( + @Advice.Enter final Scope playControllerScope, + @Advice.This final Object thisAction, + @Advice.Thrown final Throwable throwable, + @Advice.Argument(0) final Request req, + @Advice.Return(readOnly = false) final Future responseFuture) { + final Span playControllerSpan = playControllerScope.span(); + + // Call onRequest on return after tags are populated. + DECORATE.onRequest(playControllerSpan, req); + + if (throwable == null) { + responseFuture.onComplete( + new RequestCompleteCallback(playControllerSpan), + ((Action) thisAction).executionContext()); + } else { + DECORATE.onError(playControllerSpan, throwable); + Tags.HTTP_STATUS.set(playControllerSpan, 500); + DECORATE.beforeFinish(playControllerSpan); + playControllerSpan.finish(); + } + playControllerScope.close(); + + final Span rootSpan = GlobalTracer.get().activeSpan(); + // set the resource name on the upstream akka/netty span + DECORATE.onRequest(rootSpan, req); + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHeaders.java b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHeaders.java new file mode 100644 index 0000000000..c888581d7b --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHeaders.java @@ -0,0 +1,33 @@ +package datadog.trace.instrumentation.play26; + +import io.opentracing.propagation.TextMap; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import play.api.mvc.Request; +import scala.Tuple2; + +public class PlayHeaders implements TextMap { + private final Request request; + + public PlayHeaders(final Request request) { + this.request = request; + } + + @Override + public Iterator> iterator() { + final scala.collection.Map scalaMap = request.headers().toSimpleMap(); + final Map javaMap = new HashMap<>(scalaMap.size()); + final scala.collection.Iterator> scalaIterator = scalaMap.iterator(); + while (scalaIterator.hasNext()) { + final Tuple2 tuple = scalaIterator.next(); + javaMap.put(tuple._1(), tuple._2()); + } + return javaMap.entrySet().iterator(); + } + + @Override + public void put(final String s, final String s1) { + throw new IllegalStateException("play headers can only be extracted"); + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHttpServerDecorator.java b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHttpServerDecorator.java new file mode 100644 index 0000000000..72338bdf32 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHttpServerDecorator.java @@ -0,0 +1,94 @@ +package datadog.trace.instrumentation.play26; + +import datadog.trace.agent.decorator.HttpServerDecorator; +import datadog.trace.api.DDTags; +import io.opentracing.Span; +import io.opentracing.tag.Tags; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.UndeclaredThrowableException; +import java.net.URI; +import java.net.URISyntaxException; +import lombok.extern.slf4j.Slf4j; +import play.api.mvc.Request; +import play.api.mvc.Result; +import play.api.routing.HandlerDef; +import play.routing.Router; +import scala.Option; + +@Slf4j +public class PlayHttpServerDecorator extends HttpServerDecorator { + public static final PlayHttpServerDecorator DECORATE = new PlayHttpServerDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"play"}; + } + + @Override + protected String component() { + return "play-action"; + } + + @Override + protected String method(final Request httpRequest) { + return httpRequest.method(); + } + + @Override + protected URI url(final Request request) throws URISyntaxException { + return new URI(request.secure() ? "https://" : "http://" + request.host() + request.uri()); + } + + @Override + protected String peerHostname(final Request request) { + return null; + } + + @Override + protected String peerHostIP(final Request request) { + return request.remoteAddress(); + } + + @Override + protected Integer peerPort(final Request request) { + return null; + } + + @Override + protected Integer status(final Result httpResponse) { + return httpResponse.header().status(); + } + + @Override + public Span onRequest(final Span span, final Request request) { + super.onRequest(span, request); + if (request != null) { + // more about routes here: + // https://github.com/playframework/playframework/blob/master/documentation/manual/releases/release26/migration26/Migration26.md + final Option defOption = + request.attrs().get(Router.Attrs.HANDLER_DEF.underlying()); + if (!defOption.isEmpty()) { + final String path = defOption.get().path(); + span.setTag(DDTags.RESOURCE_NAME, request.method() + " " + path); + } + } + return span; + } + + @Override + public Span onError(final Span span, Throwable throwable) { + Tags.HTTP_STATUS.set(span, 500); + if (throwable != null + // This can be moved to instanceof check when using Java 8. + && throwable.getClass().getName().equals("java.util.concurrent.CompletionException") + && throwable.getCause() != null) { + throwable = throwable.getCause(); + } + while ((throwable instanceof InvocationTargetException + || throwable instanceof UndeclaredThrowableException) + && throwable.getCause() != null) { + throwable = throwable.getCause(); + } + return super.onError(span, throwable); + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/RequestCompleteCallback.java b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/RequestCompleteCallback.java new file mode 100644 index 0000000000..d79339fba8 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/RequestCompleteCallback.java @@ -0,0 +1,39 @@ +package datadog.trace.instrumentation.play26; + +import static datadog.trace.instrumentation.play26.PlayHttpServerDecorator.DECORATE; + +import datadog.trace.context.TraceScope; +import io.opentracing.Span; +import io.opentracing.util.GlobalTracer; +import lombok.extern.slf4j.Slf4j; +import play.api.mvc.Result; +import scala.util.Try; + +@Slf4j +public class RequestCompleteCallback extends scala.runtime.AbstractFunction1, Object> { + private final Span span; + + public RequestCompleteCallback(final Span span) { + this.span = span; + } + + @Override + public Object apply(final Try result) { + try { + if (result.isFailure()) { + DECORATE.onError(span, result.failed().get()); + } else { + DECORATE.onResponse(span, result.get()); + } + DECORATE.beforeFinish(span); + if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { + ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(false); + } + } catch (final Throwable t) { + log.debug("error in play instrumentation", t); + } finally { + span.finish(); + } + return null; + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/AkkaHttpTestInstrumentation.java b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/AkkaHttpTestInstrumentation.java new file mode 100644 index 0000000000..cd10a5c4ca --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/AkkaHttpTestInstrumentation.java @@ -0,0 +1,19 @@ +import static net.bytebuddy.matcher.ElementMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.test.base.HttpServerTestAdvice; +import datadog.trace.agent.tooling.Instrumenter; +import net.bytebuddy.agent.builder.AgentBuilder; + +@AutoService(Instrumenter.class) +public class AkkaHttpTestInstrumentation implements Instrumenter { + + @Override + public AgentBuilder instrument(final AgentBuilder agentBuilder) { + return agentBuilder + .type(named("akka.http.impl.engine.server.HttpServerBluePrint$PrepareRequests$$anon$1")) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice(named("onPush"), HttpServerTestAdvice.ServerEntryAdvice.class.getName())); + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy new file mode 100644 index 0000000000..1695438450 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy @@ -0,0 +1,64 @@ +package server + +import play.BuiltInComponents +import play.Mode +import play.libs.concurrent.HttpExecution +import play.mvc.Results +import play.routing.RoutingDsl +import play.server.Server +import spock.lang.Shared + +import java.util.concurrent.CompletableFuture +import java.util.concurrent.Executors +import java.util.function.Supplier + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +class PlayAsyncServerTest extends PlayServerTest { + @Shared + def executor = Executors.newCachedThreadPool() + + def cleanupSpec() { + executor.shutdown() + } + + @Override + Server startServer(int port) { + def execContext = HttpExecution.fromThread(executor) + return Server.forRouter(Mode.TEST, port) { BuiltInComponents components -> + RoutingDsl.fromComponents(components) + .GET(SUCCESS.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(SUCCESS) { + Results.status(SUCCESS.getStatus(), SUCCESS.getBody()) + } + }, execContext) + } as Supplier) + .GET(REDIRECT.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(REDIRECT) { + Results.found(REDIRECT.getBody()) + } + }, execContext) + } as Supplier) + .GET(ERROR.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(ERROR) { + Results.status(ERROR.getStatus(), ERROR.getBody()) + } + }, execContext) + } as Supplier) + .GET(EXCEPTION.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(EXCEPTION) { + throw new Exception(EXCEPTION.getBody()) + } + }, execContext) + } as Supplier) + .build() + } + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy new file mode 100644 index 0000000000..f38e407118 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy @@ -0,0 +1,137 @@ +package server + +import datadog.opentracing.DDSpan +import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.api.DDSpanTypes +import datadog.trace.instrumentation.akkahttp.AkkaHttpServerDecorator +import datadog.trace.instrumentation.play26.PlayHttpServerDecorator +import io.opentracing.tag.Tags +import play.BuiltInComponents +import play.Mode +import play.mvc.Results +import play.routing.RoutingDsl +import play.server.Server + +import java.util.function.Supplier + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +class PlayServerTest extends HttpServerTest { + @Override + Server startServer(int port) { + return Server.forRouter(Mode.TEST, port) { BuiltInComponents components -> + RoutingDsl.fromComponents(components) + .GET(SUCCESS.getPath()).routeTo({ + controller(SUCCESS) { + Results.status(SUCCESS.getStatus(), SUCCESS.getBody()) + } + } as Supplier) + .GET(REDIRECT.getPath()).routeTo({ + controller(REDIRECT) { + Results.found(REDIRECT.getBody()) + } + } as Supplier) + .GET(ERROR.getPath()).routeTo({ + controller(ERROR) { + Results.status(ERROR.getStatus(), ERROR.getBody()) + } + } as Supplier) + .GET(EXCEPTION.getPath()).routeTo({ + controller(EXCEPTION) { + throw new Exception(EXCEPTION.getBody()) + } + } as Supplier) + .build() + } + } + + @Override + void stopServer(Server server) { + server.stop() + } + + @Override + AkkaHttpServerDecorator decorator() { + return AkkaHttpServerDecorator.DECORATE + } + + @Override + String expectedOperationName() { + return "akka-http.request" + } + + @Override + boolean hasHandlerSpan() { + true + } + + @Override + // Return the handler span's name + String reorderHandlerSpan() { + "play.request" + } + + boolean testExceptionBody() { + // I can't figure out how to set a proper exception handler to customize the response body. + false + } + + @Override + void handlerSpan(TraceAssert trace, int index, Object parent, ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + serviceName expectedServiceName() + operationName "play.request" + spanType DDSpanTypes.HTTP_SERVER + errored endpoint == ERROR || endpoint == EXCEPTION + childOf(parent as DDSpan) + tags { + "$Tags.COMPONENT.key" PlayHttpServerDecorator.DECORATE.component() + "$Tags.HTTP_STATUS.key" Integer + "$Tags.HTTP_URL.key" String + "$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional + "$Tags.HTTP_METHOD.key" String + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + defaultTags() + if (endpoint == ERROR) { + "$Tags.ERROR.key" true + } else if (endpoint == EXCEPTION) { + errorTags(Exception, EXCEPTION.body) + } + } + } + } + + void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + serviceName expectedServiceName() + operationName expectedOperationName() + resourceName endpoint.status == 404 ? "404" : "$method ${endpoint.resolve(address).path}" + spanType DDSpanTypes.HTTP_SERVER + errored endpoint.errored + if (parentID != null) { + traceId traceID + parentId parentID + } else { + parent() + } + tags { + defaultTags(true) + "$Tags.COMPONENT.key" serverDecorator.component() + if (endpoint.errored) { + "$Tags.ERROR.key" endpoint.errored + "error.msg" { it == null || it == EXCEPTION.body } + "error.type" { it == null || it == Exception.name } + "error.stack" { it == null || it instanceof String } + } + "$Tags.HTTP_STATUS.key" endpoint.status + "$Tags.HTTP_URL.key" "${endpoint.resolve(address)}" + "$Tags.HTTP_METHOD.key" method + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + } + } + } +} diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index 004c06921c..984b8f43bc 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -57,6 +57,10 @@ abstract class HttpServerTest ext abstract SERVER startServer(int port) def cleanupSpec() { + if (server == null) { + println getClass().name + " can't stop null server" + return + } stopServer(server) server = null println getClass().name + " http server stopped at: http://localhost:$port/" @@ -76,8 +80,13 @@ abstract class HttpServerTest ext false } + // Return the handler span's name + String reorderHandlerSpan() { + null + } + boolean reorderControllerSpan() { - true + false } boolean testNotFound() { @@ -356,7 +365,18 @@ abstract class HttpServerTest ext assert toRemove.size() == size TEST_WRITER.removeAll(toRemove) - if(reorderControllerSpan()) { + if (reorderHandlerSpan()) { + TEST_WRITER.each { + def controllerSpan = it.find { + it.operationName == reorderHandlerSpan() + } + if (controllerSpan) { + it.remove(controllerSpan) + it.add(controllerSpan) + } + } + } + if (reorderControllerSpan() || reorderHandlerSpan()) { // Some frameworks close the handler span before the controller returns, so we need to manually reorder it. TEST_WRITER.each { def controllerSpan = it.find { diff --git a/settings.gradle b/settings.gradle index e5f4eb2adc..69802bcc9c 100644 --- a/settings.gradle +++ b/settings.gradle @@ -80,6 +80,7 @@ include ':dd-java-agent:instrumentation:netty-4.1' include ':dd-java-agent:instrumentation:okhttp-3' include ':dd-java-agent:instrumentation:osgi-classloading' include ':dd-java-agent:instrumentation:play-2.4' +include ':dd-java-agent:instrumentation:play-2.6' include ':dd-java-agent:instrumentation:rabbitmq-amqp-2.7' include ':dd-java-agent:instrumentation:ratpack-1.4' include ':dd-java-agent:instrumentation:reactor-core-3.1'