Add server tests

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();

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<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()
+      }
+    }
+  }
+}

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()));
+  }
+}

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)
+      }
+    })
+  }
+}

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]
+  }
+}

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)
+  }
+}