From 37a279069b87af184930d70209077a03b23b50f5 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Thu, 30 Jan 2020 15:23:26 -0500 Subject: [PATCH] Add server tests --- .../finatra/FinatraInstrumentation.java | 13 ++- .../src/test/groovy/FinatraServerTest.groovy | 91 +++++++++++++++++++ .../NettyServerTestInstrumentation.java | 20 ++++ .../src/test/scala/FinatraController.scala | 56 ++++++++++++ .../src/test/scala/FinatraServer.scala | 13 +++ .../ResponseSettingExceptionMapper.scala | 15 +++ 6 files changed, 205 insertions(+), 3 deletions(-) create mode 100644 dd-java-agent/instrumentation/finatra-2.9/src/test/groovy/FinatraServerTest.groovy create mode 100644 dd-java-agent/instrumentation/finatra-2.9/src/test/groovy/NettyServerTestInstrumentation.java create mode 100644 dd-java-agent/instrumentation/finatra-2.9/src/test/scala/FinatraController.scala create mode 100644 dd-java-agent/instrumentation/finatra-2.9/src/test/scala/FinatraServer.scala create mode 100644 dd-java-agent/instrumentation/finatra-2.9/src/test/scala/ResponseSettingExceptionMapper.scala diff --git a/dd-java-agent/instrumentation/finatra-2.9/src/main/java/datadog/trace/instrumentation/finatra/FinatraInstrumentation.java b/dd-java-agent/instrumentation/finatra-2.9/src/main/java/datadog/trace/instrumentation/finatra/FinatraInstrumentation.java index 2911bd58bf..3c426fbdde 100644 --- a/dd-java-agent/instrumentation/finatra-2.9/src/main/java/datadog/trace/instrumentation/finatra/FinatraInstrumentation.java +++ b/dd-java-agent/instrumentation/finatra-2.9/src/main/java/datadog/trace/instrumentation/finatra/FinatraInstrumentation.java @@ -19,6 +19,7 @@ import com.twitter.finagle.http.Response; import com.twitter.util.Future; import com.twitter.util.FutureEventListener; import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.api.Config; import datadog.trace.api.DDTags; import datadog.trace.instrumentation.api.AgentScope; import datadog.trace.instrumentation.api.AgentSpan; @@ -76,8 +77,6 @@ public class FinatraInstrumentation extends Instrumenter.Default { final AgentSpan span = startSpan("finatra.request"); DECORATE.afterStart(span); - DECORATE.onConnection(span, request); - DECORATE.onRequest(span, request); if (parent != null && "netty.request".equals(parent.getSpanName())) { parent.setTag(DDTags.RESOURCE_NAME, request.method().name() + " " + path); @@ -86,6 +85,9 @@ public class FinatraInstrumentation extends Instrumenter.Default { span.setSpanName("finatra.controller"); span.setTag(DDTags.RESOURCE_NAME, DECORATE.spanNameForClass(clazz)); + } else { + DECORATE.onConnection(span, request); + DECORATE.onRequest(span, request); } final AgentScope scope = activateSpan(span, false); @@ -125,7 +127,12 @@ public class FinatraInstrumentation extends Instrumenter.Default { @Override public void onSuccess(final Response response) { - DECORATE.onResponse(scope.span(), response); + if ("finatra.request".equals(scope.span().getSpanName())) { + DECORATE.onResponse(scope.span(), response); + } else if (Config.get().getHttpServerErrorStatuses().contains(DECORATE.status(response))) { + scope.span().setError(true); + } + DECORATE.beforeFinish(scope.span()); scope.span().finish(); scope.close(); diff --git a/dd-java-agent/instrumentation/finatra-2.9/src/test/groovy/FinatraServerTest.groovy b/dd-java-agent/instrumentation/finatra-2.9/src/test/groovy/FinatraServerTest.groovy new file mode 100644 index 0000000000..da891d5fc7 --- /dev/null +++ b/dd-java-agent/instrumentation/finatra-2.9/src/test/groovy/FinatraServerTest.groovy @@ -0,0 +1,91 @@ +import com.twitter.finatra.http.HttpServer +import com.twitter.util.Await +import com.twitter.util.Closable +import com.twitter.util.Duration +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.api.Tags +import datadog.trace.instrumentation.finatra.FinatraDecorator + +import java.util.concurrent.TimeoutException + +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.SUCCESS + +class FinatraServerTest extends HttpServerTest { + private static final Duration TIMEOUT = Duration.fromSeconds(5) + private static final long STARTUP_TIMEOUT = 20 * 1000 + + static closeAndWait(Closable closable) { + if (closable != null) { + Await.ready(closable.close(), TIMEOUT) + } + } + + @Override + HttpServer startServer(int port) { + HttpServer testServer = new FinatraServer() + + // Starting the server is blocking so start it in a separate thread + Thread startupThread = new Thread({ + testServer.main("-admin.port=:0", "-http.port=:" + port) + }) + startupThread.setDaemon(true) + startupThread.start() + + long startupDeadline = System.currentTimeMillis() + STARTUP_TIMEOUT + while (!testServer.started()) { + if (System.currentTimeMillis() > startupDeadline) { + throw new TimeoutException("Timed out waiting for server startup") + } + } + + return testServer + } + + @Override + boolean hasHandlerSpan() { + return true + } + + @Override + void stopServer(HttpServer httpServer) { + Await.ready(httpServer.close(), TIMEOUT) + } + + @Override + FinatraDecorator decorator() { + return FinatraDecorator.DECORATE + } + + @Override + String expectedOperationName() { + return "finatra.request" + } + + void handlerSpan(TraceAssert trace, int index, Object parent, ServerEndpoint endpoint = SUCCESS) { + def errorEndpoint = endpoint == EXCEPTION || endpoint == ERROR + trace.span(index) { + serviceName expectedServiceName() + operationName "finatra.controller" + resourceName "FinatraController" + spanType DDSpanTypes.HTTP_SERVER + errored errorEndpoint + childOf(parent as DDSpan) + tags { + "$Tags.COMPONENT" FinatraDecorator.DECORATE.component() + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + + // Finatra doesn't propagate the stack trace or exception to the instrumentation + // so the normal errorTags() method can't be used + if (errorEndpoint) { + "$Tags.ERROR" true + } + defaultTags() + } + } + } +} diff --git a/dd-java-agent/instrumentation/finatra-2.9/src/test/groovy/NettyServerTestInstrumentation.java b/dd-java-agent/instrumentation/finatra-2.9/src/test/groovy/NettyServerTestInstrumentation.java new file mode 100644 index 0000000000..224b08f4af --- /dev/null +++ b/dd-java-agent/instrumentation/finatra-2.9/src/test/groovy/NettyServerTestInstrumentation.java @@ -0,0 +1,20 @@ +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/finatra-2.9/src/test/scala/FinatraController.scala b/dd-java-agent/instrumentation/finatra-2.9/src/test/scala/FinatraController.scala new file mode 100644 index 0000000000..6a5503c8a4 --- /dev/null +++ b/dd-java-agent/instrumentation/finatra-2.9/src/test/scala/FinatraController.scala @@ -0,0 +1,56 @@ +import com.twitter.finagle.http.{Request, Response} +import com.twitter.finatra.http.Controller +import com.twitter.util.Future +import datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint._ +import datadog.trace.agent.test.base.HttpServerTest.controller +import groovy.lang.Closure + +class FinatraController extends Controller { + any(SUCCESS.getPath) { request: Request => + controller(SUCCESS, new Closure[Response](null) { + override def call(): Response = { + response.ok(SUCCESS.getBody) + } + }) + } + + any(ERROR.getPath) { request: Request => + controller(ERROR, new Closure[Response](null) { + override def call(): Response = { + response.internalServerError(ERROR.getBody) + } + }) + } + + any(NOT_FOUND.getPath) { request: Request => + controller(NOT_FOUND, new Closure[Response](null) { + override def call(): Response = { + response.notFound(NOT_FOUND.getBody) + } + }) + } + + any(QUERY_PARAM.getPath) { request: Request => + controller(QUERY_PARAM, new Closure[Response](null) { + override def call(): Response = { + response.ok(QUERY_PARAM.getBody) + } + }) + } + + any(EXCEPTION.getPath) { request: Request => + controller(EXCEPTION, new Closure[Future[Response]](null) { + override def call(): Future[Response] = { + throw new Exception(EXCEPTION.getBody) + } + }) + } + + any(REDIRECT.getPath) { request: Request => + controller(REDIRECT, new Closure[Response](null) { + override def call(): Response = { + response.found.location(REDIRECT.getBody) + } + }) + } +} diff --git a/dd-java-agent/instrumentation/finatra-2.9/src/test/scala/FinatraServer.scala b/dd-java-agent/instrumentation/finatra-2.9/src/test/scala/FinatraServer.scala new file mode 100644 index 0000000000..0bc035e017 --- /dev/null +++ b/dd-java-agent/instrumentation/finatra-2.9/src/test/scala/FinatraServer.scala @@ -0,0 +1,13 @@ +import com.twitter.finagle.http.Request +import com.twitter.finatra.http.HttpServer +import com.twitter.finatra.http.filters.ExceptionMappingFilter +import com.twitter.finatra.http.routing.HttpRouter + +class FinatraServer extends HttpServer { + override protected def configureHttp(router: HttpRouter): Unit = { + router + .filter[ExceptionMappingFilter[Request]] + .add[FinatraController] + .exceptionMapper[ResponseSettingExceptionMapper] + } +} diff --git a/dd-java-agent/instrumentation/finatra-2.9/src/test/scala/ResponseSettingExceptionMapper.scala b/dd-java-agent/instrumentation/finatra-2.9/src/test/scala/ResponseSettingExceptionMapper.scala new file mode 100644 index 0000000000..de5a31928d --- /dev/null +++ b/dd-java-agent/instrumentation/finatra-2.9/src/test/scala/ResponseSettingExceptionMapper.scala @@ -0,0 +1,15 @@ + + +import com.twitter.finagle.http.{Request, Response} +import com.twitter.finatra.http.exceptions.ExceptionMapper +import com.twitter.finatra.http.response.ResponseBuilder +import javax.inject.{Inject, Singleton} + +@Singleton +class ResponseSettingExceptionMapper @Inject()(response: ResponseBuilder) + extends ExceptionMapper[Exception] { + + override def toResponse(request: Request, exception: Exception): Response = { + response.internalServerError(exception.getMessage) + } +}