Add server tests
MODIFY
dd-java-agent/instrumentation/finatra-2.9/src/main/java/datadog/trace/instrumentation/finatra/FinatraInstrumentation.java
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) {
+ 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();

ADD
dd-java-agent/instrumentation/finatra-2.9/src/test/groovy/FinatraServerTest.groovy
+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<HttpServer, FinatraDecorator> {
+ 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()
+ }
+ }
+ }
+}

ADD
dd-java-agent/instrumentation/finatra-2.9/src/test/groovy/NettyServerTestInstrumentation.java
+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()));
+ }
+}

ADD
dd-java-agent/instrumentation/finatra-2.9/src/test/scala/FinatraController.scala
+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)
+ }
+ })
+ }
+}

ADD
dd-java-agent/instrumentation/finatra-2.9/src/test/scala/FinatraServer.scala
+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]
+ }
+}

ADD
dd-java-agent/instrumentation/finatra-2.9/src/test/scala/ResponseSettingExceptionMapper.scala
+
+
+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)
+ }
+}

Add server tests