Migrate Play instrumentation to Decorators

This commit is contained in:
Tyler Benson 2019-03-11 08:48:18 -07:00
parent 0d23ab3169
commit b8affc472d
7 changed files with 251 additions and 325 deletions

View File

@ -38,6 +38,8 @@ dependencies {
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'
testCompile project(':dd-java-agent:testing')
testCompile project(':dd-java-agent:instrumentation:java-concurrent')
testCompile project(':dd-java-agent:instrumentation:trace-annotation')
@ -45,8 +47,9 @@ dependencies {
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.0'
latestDepTestCompile group: 'com.typesafe.play', name: 'play-test_2.11', version: '2.6.0'
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.+'
}
compileLatestDepTestGroovy {

View File

@ -8,12 +8,8 @@ import play.test.Helpers
import spock.lang.Shared
class Play26Test extends AgentTestRunner {
static {
System.setProperty("dd.integration.akka-http-server.enabled", "true")
}
@Shared
int port
int port = PortUtils.randomOpenPort()
@Shared
TestServer testServer
@ -21,7 +17,6 @@ class Play26Test extends AgentTestRunner {
def client = OkHttpUtils.client()
def setupSpec() {
port = PortUtils.randomOpenPort()
testServer = Helpers.testServer(port, Play26TestUtils.buildTestApp())
testServer.start()
}
@ -33,7 +28,7 @@ class Play26Test extends AgentTestRunner {
def "request traces"() {
setup:
def request = new Request.Builder()
.url("http://localhost:$port/helloplay/spock")
.url("http://localhost:$port/$path")
.header("x-datadog-trace-id", "123")
.header("x-datadog-parent-id", "456")
.get()
@ -41,200 +36,82 @@ class Play26Test extends AgentTestRunner {
def response = client.newCall(request).execute()
expect:
response.code() == 200
response.body().string() == "hello spock"
testServer != null
response.code() == status
if (body instanceof Class) {
body.isInstance(response.body())
} else {
response.body().string() == body
}
assertTraces(1) {
trace(0, 3) {
trace(0, extraSpans ? 3 : 2) {
span(0) {
traceId "123"
parentId "456"
serviceName "unnamed-java-app"
operationName "akka-http.request"
resourceName "GET /helloplay/:from"
resourceName status == 404 ? "404" : "GET $route"
spanType DDSpanTypes.HTTP_SERVER
errored false
errored isError
tags {
"http.status_code" status
"http.url" "http://localhost:$port/$path"
"http.method" "GET"
"peer.hostname" "localhost"
"peer.port" port
"span.kind" "server"
"component" "akka-http-server"
if (isError) {
"error" true
}
defaultTags(true)
"http.status_code" 200
"http.url" "http://localhost:$port/helloplay/spock"
}
}
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.hostname" "localhost"
"peer.port" port
"span.kind" "server"
"component" "akka-http-server"
}
}
span(1) {
childOf span(0)
operationName "play.request"
resourceName "GET /helloplay/:from"
spanType DDSpanTypes.HTTP_SERVER
tags {
defaultTags()
"http.status_code" 200
"http.url" "/helloplay/:from"
"http.method" "GET"
"span.kind" "server"
"component" "play-action"
if (isError) {
if (exception) {
errorTags(exception.class, exception.message)
} else {
"error" true
}
}
defaultTags()
}
}
span(2) {
childOf span(1)
operationName 'TracedWork$.doWork'
}
}
}
}
def "5xx errors trace"() {
setup:
def request = new Request.Builder()
.url("http://localhost:$port/make-error")
.get()
.build()
def response = client.newCall(request).execute()
expect:
response.code() == 500
assertTraces(1) {
trace(0, 2) {
span(0) {
serviceName "unnamed-java-app"
operationName "akka-http.request"
resourceName "GET /make-error"
spanType DDSpanTypes.HTTP_SERVER
errored true
tags {
defaultTags()
"http.status_code" 500
"http.url" "http://localhost:$port/make-error"
"http.method" "GET"
"peer.hostname" "localhost"
"peer.port" port
"span.kind" "server"
"component" "akka-http-server"
"error" true
}
}
span(1) {
childOf span(0)
operationName "play.request"
resourceName "GET /make-error"
spanType DDSpanTypes.HTTP_SERVER
errored true
tags {
defaultTags()
"http.status_code" 500
"http.url" "/make-error"
"http.method" "GET"
"span.kind" "server"
"component" "play-action"
"error" true
if (extraSpans) {
span(2) {
operationName "TracedWork\$.doWork"
childOf(span(1))
tags {
"component" "trace"
defaultTags()
}
}
}
}
}
}
def "error thrown in request"() {
setup:
def request = new Request.Builder()
.url("http://localhost:$port/exception")
.get()
.build()
def response = client.newCall(request).execute()
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
expect:
testServer != null
response.code() == 500
assertTraces(1) {
trace(0, 2) {
span(0) {
serviceName "unnamed-java-app"
operationName "akka-http.request"
resourceName "GET /exception"
spanType DDSpanTypes.HTTP_SERVER
errored true
tags {
defaultTags()
"http.status_code" 500
"http.url" "http://localhost:$port/exception"
"http.method" "GET"
"peer.hostname" "localhost"
"peer.port" port
"span.kind" "server"
"component" "akka-http-server"
"error" true
}
}
span(1) {
childOf span(0)
operationName "play.request"
resourceName "GET /exception"
spanType DDSpanTypes.HTTP_SERVER
errored true
tags {
defaultTags()
"http.status_code" 500
"http.url" "/exception"
"http.method" "GET"
"span.kind" "server"
"component" "play-action"
"error" true
"error.msg" "oh no"
"error.type" RuntimeException.getName()
"error.stack" String
}
}
}
}
}
def "4xx errors trace"() {
setup:
def request = new Request.Builder()
.url("http://localhost:$port/nowhere")
.get()
.build()
def response = client.newCall(request).execute()
expect:
response.code() == 404
assertTraces(1) {
trace(0, 2) {
span(0) {
serviceName "unnamed-java-app"
operationName "akka-http.request"
resourceName "404"
spanType DDSpanTypes.HTTP_SERVER
errored false
tags {
defaultTags()
"http.status_code" 404
"http.url" "http://localhost:$port/nowhere"
"http.method" "GET"
"peer.hostname" "localhost"
"peer.port" port
"span.kind" "server"
"component" "akka-http-server"
}
}
span(1) {
childOf span(0)
operationName "play.request"
resourceName "404"
spanType DDSpanTypes.HTTP_SERVER
errored false
tags {
defaultTags()
"http.status_code" 404
"http.method" "GET"
"span.kind" "server"
"component" "play-action"
}
}
}
}
extraSpans = !isError && status != 404
}
}

View File

@ -9,7 +9,6 @@ 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
@ -19,6 +18,9 @@ object Play26TestUtils {
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

View File

@ -0,0 +1,90 @@
package datadog.trace.instrumentation.play;
import datadog.trace.agent.decorator.HttpServerDecorator;
import datadog.trace.api.DDTags;
import io.opentracing.Span;
import io.opentracing.tag.Tags;
import java.net.URI;
import java.net.URISyntaxException;
import lombok.extern.slf4j.Slf4j;
import play.api.mvc.Request;
import play.api.mvc.Result;
import scala.Option;
@Slf4j
public class PlayHttpServerDecorator extends HttpServerDecorator<Request, Result> {
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 String url(final Request request) {
// FIXME: This code is similar to that from the netty integrations.
try {
URI uri = new URI(request.uri());
if ((uri.getHost() == null || uri.getHost().equals("")) && !request.host().isEmpty()) {
uri = new URI("http://" + request.host() + request.uri());
}
return new URI(uri.getScheme(), null, uri.getHost(), uri.getPort(), uri.getPath(), null, null)
.toString();
} catch (final URISyntaxException e) {
log.debug("Cannot parse uri: {}", request.uri());
return request.uri();
}
}
@Override
protected String hostname(final Request httpRequest) {
return httpRequest.domain();
}
@Override
protected Integer port(final Request httpRequest) {
final String[] split = httpRequest.host().split(":");
try {
return split.length == 2 ? Integer.valueOf(split[1]) : null;
} catch (final Exception e) {
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#router-tags-are-now-attributes
final Option pathOption = request.tags().get("ROUTE_PATTERN");
if (!pathOption.isEmpty()) {
final String path = (String) pathOption.get();
// scope.span().setTag(Tags.HTTP_URL.getKey(), path);
span.setTag(DDTags.RESOURCE_NAME, request.method() + " " + path);
}
}
return span;
}
@Override
public Span onError(final Span span, final Throwable throwable) {
Tags.HTTP_STATUS.set(span, 500);
return super.onError(span, throwable);
}
}

View File

@ -1,7 +1,7 @@
package datadog.trace.instrumentation.play;
import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
import static io.opentracing.log.Fields.ERROR_OBJECT;
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;
@ -10,7 +10,6 @@ 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.DDSpanTypes;
import datadog.trace.api.DDTags;
import datadog.trace.context.TraceScope;
import io.opentracing.Scope;
@ -28,7 +27,6 @@ import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
import org.slf4j.LoggerFactory;
import play.api.mvc.Action;
import play.api.mvc.Request;
import play.api.mvc.Result;
@ -36,7 +34,6 @@ import scala.Option;
import scala.Tuple2;
import scala.concurrent.Future;
@Slf4j
@AutoService(Instrumenter.class)
public final class PlayInstrumentation extends Instrumenter.Default {
@ -52,6 +49,10 @@ public final class PlayInstrumentation extends Instrumenter.Default {
@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"
@ -89,6 +90,7 @@ public final class PlayInstrumentation extends Instrumenter.Default {
// Do not extract the context.
scope = GlobalTracer.get().buildSpan("play.request").startActive(false);
}
DECORATE.afterStart(scope);
if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) {
((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(true);
@ -98,38 +100,36 @@ public final class PlayInstrumentation extends Instrumenter.Default {
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopTraceOnResponse(
@Advice.Enter final Scope scope,
@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<Result> responseFuture) {
// 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 (!pathOption.isEmpty()) {
final String path = (String) pathOption.get();
scope.span().setTag(Tags.HTTP_URL.getKey(), path);
scope.span().setTag(DDTags.RESOURCE_NAME, req.method() + " " + path);
}
final Span playControllerSpan = playControllerScope.span();
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.HTTP_SERVER);
scope.span().setTag(Tags.COMPONENT.getKey(), "play-action");
// Call onRequest on return after tags are populated.
DECORATE.onRequest(playControllerSpan, req);
if (throwable == null) {
responseFuture.onFailure(
new RequestError(scope.span()), ((Action) thisAction).executionContext());
new RequestError(playControllerSpan), ((Action) thisAction).executionContext());
responseFuture =
responseFuture.map(
new RequestCallback(scope.span()), ((Action) thisAction).executionContext());
new RequestCallback(playControllerSpan), ((Action) thisAction).executionContext());
} else {
RequestError.onError(scope.span(), throwable);
scope.span().finish();
DECORATE.onError(playControllerSpan, throwable);
Tags.HTTP_STATUS.set(playControllerSpan, 500);
DECORATE.beforeFinish(playControllerSpan);
playControllerSpan.finish();
}
scope.close();
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();
@ -163,6 +163,7 @@ public final class PlayInstrumentation extends Instrumenter.Default {
}
}
@Slf4j
public static class RequestError extends JavaPartialFunction<Throwable, Object> {
private final Span span;
@ -173,22 +174,18 @@ public final class PlayInstrumentation extends Instrumenter.Default {
@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);
}
onError(span, t);
} catch (final Throwable t2) {
LoggerFactory.getLogger(RequestCallback.class).debug("error in play instrumentation", t);
log.debug("error in play instrumentation", t);
} finally {
span.finish();
}
span.finish();
return null;
}
public static void onError(final Span span, final Throwable t) {
Tags.ERROR.set(span, Boolean.TRUE);
span.log(singletonMap(ERROR_OBJECT, t));
Tags.HTTP_STATUS.set(span, 500);
}
}
@Slf4j
@ -205,11 +202,13 @@ public final class PlayInstrumentation extends Instrumenter.Default {
((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(false);
}
try {
Tags.HTTP_STATUS.set(span, result.header().status());
DECORATE.onResponse(span, result);
DECORATE.beforeFinish(span);
} catch (final Throwable t) {
log.debug("error in play instrumentation", t);
} finally {
span.finish();
}
span.finish();
return result;
}
}

View File

@ -1,4 +1,3 @@
import datadog.opentracing.DDSpan
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.agent.test.utils.OkHttpUtils
import datadog.trace.agent.test.utils.PortUtils
@ -10,7 +9,7 @@ import spock.lang.Shared
class Play24Test extends AgentTestRunner {
@Shared
int port
int port = PortUtils.randomOpenPort()
@Shared
TestServer testServer
@ -18,8 +17,7 @@ class Play24Test extends AgentTestRunner {
def client = OkHttpUtils.client()
def setupSpec() {
port = PortUtils.randomOpenPort()
testServer = Helpers.testServer(port, Play24TestUtils.buildTestApp())
testServer = Helpers.testServer(port, Play24TestUtils.buildTestApp(port))
testServer.start()
}
@ -30,120 +28,69 @@ class Play24Test extends AgentTestRunner {
def "request traces"() {
setup:
def request = new Request.Builder()
.url("http://localhost:$port/helloplay/spock")
.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()
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"
response.code() == status
if (body instanceof Class) {
body.isInstance(response.body())
} else {
response.body().string() == body
}
// async work is linked to play trace
playTrace.size() == 2
playTrace[1].operationName == 'TracedWork$.doWork'
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.hostname" "localhost"
"peer.port" port
"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()
}
}
}
}
}
root.traceId == "123"
root.parentId == "456"
root.serviceName == "unnamed-java-app"
root.operationName == "play.request"
root.resourceName == "GET /helloplay/:from"
root.spanType == DDSpanTypes.HTTP_SERVER
!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"
}
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
def "5xx errors trace"() {
setup:
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 == "play.request"
root.resourceName == "GET /make-error"
root.spanType == DDSpanTypes.HTTP_SERVER
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:
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 == "play.request"
root.resourceName == "GET /exception"
root.spanType == DDSpanTypes.HTTP_SERVER
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:
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.spanType == DDSpanTypes.HTTP_SERVER
!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"
extraSpans = !isError && status != 404
}
}

View File

@ -12,18 +12,26 @@ import scala.concurrent.duration._
import scala.concurrent.{Await, Future}
object Play24TestUtils {
def buildTestApp(): play.Application = {
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...")