Merge pull request #942 from DataDog/tyler/http-server-testing

Jetty 8 test migration and ignoreParent

dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy

@@ -7,7 +7,7 @@ import io.opentracing.tag.Tags
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 
-abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<AkkaHttpServerDecorator> {
+abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<Object, AkkaHttpServerDecorator> {
 
   @Override
   AkkaHttpServerDecorator decorator() {
@@ -26,12 +26,12 @@ abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<AkkaHttp
 
 // FIXME: This doesn't work because we don't support bindAndHandle.
 //  @Override
-//  void startServer(int port) {
+//  def startServer(int port) {
 //    AkkaHttpTestWebServer.start(port)
 //  }
 //
 //  @Override
-//  void stopServer() {
+//  void stopServer(Object ignore) {
 //    AkkaHttpTestWebServer.stop()
 //  }
 
@@ -68,24 +68,24 @@ abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<AkkaHttp
 
 class AkkaHttpServerInstrumentationTestSync extends AkkaHttpServerInstrumentationTest {
   @Override
-  void startServer(int port) {
+  def startServer(int port) {
     AkkaHttpTestSyncWebServer.start(port)
   }
 
   @Override
-  void stopServer() {
+  void stopServer(Object ignore) {
     AkkaHttpTestSyncWebServer.stop()
   }
 }
 
 class AkkaHttpServerInstrumentationTestAsync extends AkkaHttpServerInstrumentationTest {
   @Override
-  void startServer(int port) {
+  def startServer(int port) {
     AkkaHttpTestAsyncWebServer.start(port)
   }
 
   @Override
-  void stopServer() {
+  void stopServer(Object ignore) {
     AkkaHttpTestAsyncWebServer.stop()
   }
 }

dd-java-agent/instrumentation/jetty-8/jetty-8.gradle

@@ -24,6 +24,8 @@ dependencies {
   testCompile(project(':dd-java-agent:testing')) {
     exclude group: 'org.eclipse.jetty', module: 'jetty-server'
   }
+  testCompile project(':dd-java-agent:instrumentation:java-concurrent')
+
   testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '8.0.0.v20110901'
   testCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '8.0.0.v20110901'
   testCompile group: 'org.eclipse.jetty', name: 'jetty-continuation', version: '8.0.0.v20110901'

dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/JettyHandlerAdvice.java

@@ -16,13 +16,14 @@ import javax.servlet.http.HttpServletResponse;
 import net.bytebuddy.asm.Advice;
 
 public class JettyHandlerAdvice {
+  public static final String SERVLET_SPAN = "datadog.servlet.span";
 
   @Advice.OnMethodEnter(suppress = Throwable.class)
   public static Scope startSpan(
       @Advice.This final Object source, @Advice.Argument(2) final HttpServletRequest req) {
 
-    if (GlobalTracer.get().activeSpan() != null) {
+    if (req.getAttribute(SERVLET_SPAN) != null) {
-      // Tracing might already be applied.  If so ignore this.
+      // Request already being traced elsewhere.
       return null;
     }
 
@@ -32,6 +33,7 @@ public class JettyHandlerAdvice {
     final Scope scope =
         GlobalTracer.get()
             .buildSpan("jetty.request")
+            .ignoreActiveSpan()
             .asChildOf(extractedContext)
             .withTag("span.origin.type", source.getClass().getName())
             .startActive(false);
@@ -46,6 +48,7 @@ public class JettyHandlerAdvice {
     if (scope instanceof TraceScope) {
       ((TraceScope) scope).setAsyncPropagation(true);
     }
+    req.setAttribute(SERVLET_SPAN, span);
     return scope;
   }
 

dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/JettyHandlerInstrumentation.java

@@ -16,9 +16,9 @@ import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
 
 @AutoService(Instrumenter.class)
-public final class HandlerInstrumentation extends Instrumenter.Default {
+public final class JettyHandlerInstrumentation extends Instrumenter.Default {
 
-  public HandlerInstrumentation() {
+  public JettyHandlerInstrumentation() {
     super("jetty", "jetty-8");
   }
 

dd-java-agent/instrumentation/jetty-8/src/test/groovy/JettyContinuationHandlerTest.groovy

@@ -0,0 +1,62 @@
+import javax.servlet.ServletException
+import javax.servlet.http.HttpServletRequest
+import javax.servlet.http.HttpServletResponse
+import org.eclipse.jetty.continuation.Continuation
+import org.eclipse.jetty.continuation.ContinuationSupport
+import org.eclipse.jetty.server.Request
+import org.eclipse.jetty.server.handler.AbstractHandler
+
+import java.util.concurrent.ExecutorService
+import java.util.concurrent.Executors
+
+// FIXME: We don't currently handle jetty continuations properly (at all).
+abstract class JettyContinuationHandlerTest extends JettyHandlerTest {
+
+  @Override
+  AbstractHandler handler() {
+    ContinuationTestHandler.INSTANCE
+  }
+
+  static class ContinuationTestHandler extends AbstractHandler {
+    static final ContinuationTestHandler INSTANCE = new ContinuationTestHandler()
+    final ExecutorService executorService = Executors.newSingleThreadExecutor()
+
+    @Override
+    void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
+      final Continuation continuation = ContinuationSupport.getContinuation(request)
+      if (continuation.initial) {
+        continuation.suspend()
+        executorService.execute {
+          continuation.resume()
+        }
+      } else {
+        handleRequest(baseRequest, response)
+      }
+      baseRequest.handled = true
+    }
+  }
+
+//  // This server seems to generate a TEST_SPAN twice... once for the initial request, and once for the continuation.
+//  void cleanAndAssertTraces(
+//    final int size,
+//    @ClosureParams(value = SimpleType, options = "datadog.trace.agent.test.asserts.ListWriterAssert")
+//    @DelegatesTo(value = ListWriterAssert, strategy = Closure.DELEGATE_FIRST)
+//    final Closure spec) {
+//
+//    // If this is failing, make sure HttpServerTestAdvice is applied correctly.
+//    TEST_WRITER.waitForTraces(size * 3)
+//    // TEST_WRITER is a CopyOnWriteArrayList, which doesn't support remove()
+//    def toRemove = TEST_WRITER.findAll {
+//      it.size() == 1 && it.get(0).operationName == "TEST_SPAN"
+//    }
+//    toRemove.each {
+//      assertTrace(it, 1) {
+//        basicSpan(it, 0, "TEST_SPAN", "ServerEntry")
+//      }
+//    }
+//    assert toRemove.size() == size * 2
+//    TEST_WRITER.removeAll(toRemove)
+//
+//    assertTraces(size, spec)
+//  }
+}

dd-java-agent/instrumentation/jetty-8/src/test/groovy/JettyHandlerTest.groovy

@@ -1,204 +1,144 @@
-import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.agent.test.asserts.TraceAssert
-import datadog.trace.agent.test.utils.OkHttpUtils
+import datadog.trace.agent.test.base.HttpServerTest
-import datadog.trace.agent.test.utils.PortUtils
 import datadog.trace.api.DDSpanTypes
-import okhttp3.OkHttpClient
+import datadog.trace.instrumentation.jetty8.JettyDecorator
-import org.eclipse.jetty.continuation.Continuation
+import io.opentracing.tag.Tags
-import org.eclipse.jetty.continuation.ContinuationSupport
-import org.eclipse.jetty.server.Handler
-import org.eclipse.jetty.server.Request
-import org.eclipse.jetty.server.Server
-import org.eclipse.jetty.server.handler.AbstractHandler
-
 import javax.servlet.DispatcherType
 import javax.servlet.ServletException
 import javax.servlet.http.HttpServletRequest
 import javax.servlet.http.HttpServletResponse
-import java.util.concurrent.atomic.AtomicBoolean
+import org.eclipse.jetty.server.Request
+import org.eclipse.jetty.server.Server
+import org.eclipse.jetty.server.handler.AbstractHandler
+import org.eclipse.jetty.server.handler.ErrorHandler
 
-class JettyHandlerTest extends AgentTestRunner {
+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.NOT_FOUND
+import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT
+import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
+
+class JettyHandlerTest extends HttpServerTest<Server, JettyDecorator> {
 
   static {
     System.setProperty("dd.integration.jetty.enabled", "true")
   }
 
-  int port = PortUtils.randomOpenPort()
+  static errorHandler = new ErrorHandler() {
-  Server server = new Server(port)
+    @Override
+    protected void handleErrorPage(HttpServletRequest request, Writer writer, int code, String message) throws IOException {
+      Throwable th = (Throwable) request.getAttribute("javax.servlet.error.exception")
+      message = th ? th.message : message
+      if (message) {
+        writer.write(message)
+      }
+    }
+  }
 
-  OkHttpClient client = OkHttpUtils.client()
+  @Override
+  Server startServer(int port) {
+    def server = new Server(port)
+    server.setHandler(handler())
+    server.addBean(errorHandler)
+    server.start()
+    return server
+  }
 
-  def cleanup() {
+  AbstractHandler handler() {
+    TestHandler.INSTANCE
+  }
+
+  @Override
+  void stopServer(Server server) {
     server.stop()
   }
 
-  def "call to jetty creates a trace"() {
+  @Override
-    setup:
+  JettyDecorator decorator() {
-    Handler handler = new AbstractHandler() {
+    return JettyDecorator.DECORATE
-      @Override
+  }
-      void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
-        response.setContentType("text/plain;charset=utf-8")
-        response.setStatus(HttpServletResponse.SC_OK)
-        baseRequest.setHandled(true)
-        response.getWriter().println("Hello World")
-      }
-    }
-    server.setHandler(handler)
-    server.start()
-    def request = new okhttp3.Request.Builder()
-      .url("http://localhost:$port/")
-      .get()
-      .build()
-    def response = client.newCall(request).execute()
 
-    expect:
+  @Override
-    response.body().string().trim() == "Hello World"
+  String expectedOperationName() {
+    return "jetty.request"
+  }
 
-    assertTraces(1) {
+  @Override
-      trace(0, 1) {
+  boolean testExceptionBody() {
-        span(0) {
+    false
-          serviceName "unnamed-java-app"
+  }
-          operationName "jetty.request"
+
-          resourceName "GET ${handler.class.name}"
+  static void handleRequest(Request request, HttpServletResponse response) {
-          spanType DDSpanTypes.HTTP_SERVER
+    ServerEndpoint endpoint = ServerEndpoint.forPath(request.requestURI)
-          errored false
+    controller(endpoint) {
-          parent()
+      response.contentType = "text/plain"
-          tags {
+      switch (endpoint) {
-            "http.url" "http://localhost:$port/"
+        case SUCCESS:
-            "http.method" "GET"
+          response.status = endpoint.status
-            "span.kind" "server"
+          response.writer.print(endpoint.body)
-            "component" "jetty-handler"
+          break
-            "span.origin.type" handler.class.name
+        case REDIRECT:
-            "http.status_code" 200
+          response.sendRedirect(endpoint.body)
-            "peer.hostname" "127.0.0.1"
+          break
-            "peer.ipv4" "127.0.0.1"
+        case ERROR:
-            "peer.port" Integer
+          response.sendError(endpoint.status, endpoint.body)
-            defaultTags()
+          break
-          }
+        case EXCEPTION:
-        }
+          throw new Exception(endpoint.body)
+        default:
+          response.status = NOT_FOUND.status
+          response.writer.print(NOT_FOUND.body)
+          break
       }
     }
   }
 
-  def "handler instrumentation clears state after async request"() {
+  static class TestHandler extends AbstractHandler {
-    setup:
+    static final TestHandler INSTANCE = new TestHandler()
-    Handler handler = new AbstractHandler() {
-      @Override
-      void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
-        final Continuation continuation = ContinuationSupport.getContinuation(request)
-        continuation.suspend(response)
-        // By the way, this is a terrible async server
-        new Thread() {
-          @Override
-          void run() {
-            continuation.getServletResponse().setContentType("text/plain;charset=utf-8")
-            continuation.getServletResponse().getWriter().println("Hello World")
-            continuation.complete()
-          }
-        }.start()
 
-        baseRequest.setHandled(true)
+    @Override
-      }
+    void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
-    }
+      if (baseRequest.dispatcherType != DispatcherType.ERROR) {
-    server.setHandler(handler)
+        handleRequest(baseRequest, response)
-    server.start()
+        baseRequest.handled = true
-    def request = new okhttp3.Request.Builder()
+      } else {
-      .url("http://localhost:$port/")
+        errorHandler.handle(target, baseRequest, response, response)
-      .get()
-      .build()
-    def numTraces = 10
-    for (int i = 0; i < numTraces; ++i) {
-      assert client.newCall(request).execute().body().string().trim() == "Hello World"
-    }
-
-    expect:
-    assertTraces(numTraces) {
-      for (int i = 0; i < numTraces; ++i) {
-        trace(i, 1) {
-          span(0) {
-            serviceName "unnamed-java-app"
-            operationName "jetty.request"
-            resourceName "GET ${handler.class.name}"
-            spanType DDSpanTypes.HTTP_SERVER
-          }
-        }
       }
     }
   }
 
-  def "call to jetty with error creates a trace"() {
+  @Override
-    setup:
+  void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
-    def errorHandlerCalled = new AtomicBoolean(false)
+    def handlerName = handler().class.name
-    Handler handler = new AbstractHandler() {
+    trace.span(index) {
-      @Override
+      serviceName expectedServiceName()
-      void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
+      operationName expectedOperationName()
-        if (baseRequest.dispatcherType == DispatcherType.ERROR) {
+      resourceName endpoint.status == 404 ? "404" : "$method $handlerName"
-          errorHandlerCalled.set(true)
+      spanType DDSpanTypes.HTTP_SERVER
-          baseRequest.setHandled(true)
+      errored endpoint.errored
-        } else {
+      if (parentID != null) {
-          throw new RuntimeException()
+        traceId traceID
-        }
+        parentId parentID
+      } else {
+        parent()
       }
-    }
+      tags {
-    server.setHandler(handler)
+        "span.origin.type" handlerName
-    server.start()
+        defaultTags(true)
-    def request = new okhttp3.Request.Builder()
+        "$Tags.COMPONENT.key" serverDecorator.component()
-      .url("http://localhost:$port/")
+        if (endpoint.errored) {
-      .get()
+          "$Tags.ERROR.key" endpoint.errored
-      .build()
+          "error.msg" { it == null || it == EXCEPTION.body }
-    def response = client.newCall(request).execute()
+          "error.type" { it == null || it == Exception.name }
-
+          "error.stack" { it == null || it instanceof String }
-    expect:
-    response.body().string().trim() == ""
-
-    assertTraces(errorHandlerCalled.get() ? 2 : 1) {
-      trace(0, 1) {
-        span(0) {
-          serviceName "unnamed-java-app"
-          operationName "jetty.request"
-          resourceName "GET ${handler.class.name}"
-          spanType DDSpanTypes.HTTP_SERVER
-          errored true
-          parent()
-          tags {
-            "http.url" "http://localhost:$port/"
-            "http.method" "GET"
-            "span.kind" "server"
-            "component" "jetty-handler"
-            "span.origin.type" handler.class.name
-            "http.status_code" 500
-            "peer.hostname" "127.0.0.1"
-            "peer.ipv4" "127.0.0.1"
-            "peer.port" Integer
-            errorTags RuntimeException
-            defaultTags()
-          }
-        }
-      }
-      if (errorHandlerCalled.get()) {
-        // FIXME: This doesn't ever seem to be called.
-        trace(1, 1) {
-          span(0) {
-            serviceName "unnamed-java-app"
-            operationName "jetty.request"
-            resourceName "GET ${handler.class.name}"
-            spanType DDSpanTypes.HTTP_SERVER
-            errored true
-            parent()
-            tags {
-              "http.url" "http://localhost:$port/"
-              "http.method" "GET"
-              "span.kind" "server"
-              "component" "jetty-handler"
-              "span.origin.type" handler.class.name
-              "http.status_code" 500
-              "peer.hostname" "127.0.0.1"
-              "peer.ipv4" "127.0.0.1"
-              "peer.port" Integer
-              "error" true
-              defaultTags()
-            }
-          }
         }
+        "$Tags.HTTP_STATUS.key" endpoint.status
+        "$Tags.HTTP_URL.key" "${endpoint.resolve(address)}"
+        "$Tags.PEER_HOSTNAME.key" { it == "localhost" || it == "127.0.0.1" }
+        "$Tags.PEER_PORT.key" Integer
+        "$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional
+        "$Tags.HTTP_METHOD.key" method
+        "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
       }
     }
   }

dd-java-agent/instrumentation/jetty-8/src/test/groovy/JettyTestInstrumentation.java

@@ -0,0 +1,33 @@
+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 JettyTestInstrumentation implements Instrumenter {
+
+  @Override
+  public AgentBuilder instrument(final AgentBuilder agentBuilder) {
+    return agentBuilder
+        // Jetty 8.0
+        .type(named("org.eclipse.jetty.server.HttpConnection"))
+        .transform(
+            new AgentBuilder.Transformer.ForAdvice()
+                .advice(
+                    named("handleRequest"), HttpServerTestAdvice.ServerEntryAdvice.class.getName()))
+        // Jetty 8.?
+        .type(named("org.eclipse.jetty.server.AbstractHttpConnection"))
+        .transform(
+            new AgentBuilder.Transformer.ForAdvice()
+                .advice(
+                    named("headerComplete"),
+                    HttpServerTestAdvice.ServerEntryAdvice.class.getName()))
+        // Jetty 9
+        .type(named("org.eclipse.jetty.server.HttpChannel"))
+        .transform(
+            new AgentBuilder.Transformer.ForAdvice()
+                .advice(named("handle"), HttpServerTestAdvice.ServerEntryAdvice.class.getName()));
+  }
+}

dd-java-agent/instrumentation/netty-4.0/src/test/groovy/Netty40ServerTest.groovy

@@ -12,6 +12,7 @@ import io.netty.channel.nio.NioEventLoopGroup
 import io.netty.channel.socket.nio.NioServerSocketChannel
 import io.netty.handler.codec.http.DefaultFullHttpResponse
 import io.netty.handler.codec.http.FullHttpResponse
+import io.netty.handler.codec.http.HttpHeaders
 import io.netty.handler.codec.http.HttpRequest
 import io.netty.handler.codec.http.HttpRequestDecoder
 import io.netty.handler.codec.http.HttpResponseEncoder
@@ -19,7 +20,6 @@ import io.netty.handler.codec.http.HttpResponseStatus
 import io.netty.handler.logging.LogLevel
 import io.netty.handler.logging.LoggingHandler
 import io.netty.util.CharsetUtil
-import spock.lang.Shared
 
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
@@ -31,13 +31,11 @@ import static io.netty.handler.codec.http.HttpHeaders.Names.CONTENT_TYPE
 import static io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR
 import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1
 
-class Netty40ServerTest extends HttpServerTest<NettyHttpServerDecorator> {
+class Netty40ServerTest extends HttpServerTest<EventLoopGroup, NettyHttpServerDecorator> {
-  @Shared
-  EventLoopGroup eventLoopGroup
 
   @Override
-  void startServer(int port) {
+  EventLoopGroup startServer(int port) {
-    eventLoopGroup = new NioEventLoopGroup()
+    def eventLoopGroup = new NioEventLoopGroup()
 
     ServerBootstrap bootstrap = new ServerBootstrap()
       .group(eventLoopGroup)
@@ -62,7 +60,7 @@ class Netty40ServerTest extends HttpServerTest<NettyHttpServerDecorator> {
                       break
                     case REDIRECT:
                       response = new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status))
-                      response.headers().set(HttpHeaderNames.LOCATION, endpoint.body)
+                      response.headers().set(HttpHeaders.Names.LOCATION, endpoint.body)
                       break
                     case EXCEPTION:
                       throw new Exception(endpoint.body)
@@ -91,11 +89,13 @@ class Netty40ServerTest extends HttpServerTest<NettyHttpServerDecorator> {
         }
       ] as ChannelInitializer).channel(NioServerSocketChannel)
     bootstrap.bind(port).sync()
+
+    return eventLoopGroup
   }
 
   @Override
-  void stopServer() {
+  void stopServer(EventLoopGroup server) {
-    eventLoopGroup?.shutdownGracefully()
+    server?.shutdownGracefully()
   }
 
   @Override

dd-java-agent/instrumentation/netty-4.1/src/test/groovy/Netty41ServerTest.groovy

@@ -19,7 +19,6 @@ import io.netty.handler.codec.http.HttpServerCodec
 import io.netty.handler.logging.LogLevel
 import io.netty.handler.logging.LoggingHandler
 import io.netty.util.CharsetUtil
-import spock.lang.Shared
 
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
@@ -31,13 +30,11 @@ import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE
 import static io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR
 import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1
 
-class Netty41ServerTest extends HttpServerTest<NettyHttpServerDecorator> {
+class Netty41ServerTest extends HttpServerTest<EventLoopGroup, NettyHttpServerDecorator> {
-  @Shared
-  EventLoopGroup eventLoopGroup
 
   @Override
-  void startServer(int port) {
+  EventLoopGroup startServer(int port) {
-    eventLoopGroup = new NioEventLoopGroup()
+    def eventLoopGroup = new NioEventLoopGroup()
 
     ServerBootstrap bootstrap = new ServerBootstrap()
       .group(eventLoopGroup)
@@ -91,11 +88,13 @@ class Netty41ServerTest extends HttpServerTest<NettyHttpServerDecorator> {
         }
       ] as ChannelInitializer).channel(NioServerSocketChannel)
     bootstrap.bind(port).sync()
+
+    return eventLoopGroup
   }
 
   @Override
-  void stopServer() {
+  void stopServer(EventLoopGroup server) {
-    eventLoopGroup?.shutdownGracefully()
+    server?.shutdownGracefully()
   }
 
   @Override

dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/StatusSavingHttpServletResponseWrapper.java

@@ -23,6 +23,12 @@ public class StatusSavingHttpServletResponseWrapper extends HttpServletResponseW
     super.sendError(status, message);
   }
 
+  @Override
+  public void sendRedirect(final String location) throws IOException {
+    status = 302;
+    super.sendRedirect(location);
+  }
+
   @Override
   public void setStatus(final int status) {
     this.status = status;

dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy

@@ -14,14 +14,13 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPT
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 
-class JettyServlet2Test extends HttpServerTest<Servlet2Decorator> {
+class JettyServlet2Test extends HttpServerTest<Server, Servlet2Decorator> {
 
   private static final CONTEXT = "ctx"
-  private Server jettyServer
 
   @Override
-  void startServer(int port) {
+  Server startServer(int port) {
-    jettyServer = new Server(port)
+    def jettyServer = new Server(port)
     jettyServer.connectors.each { it.resolveNames = true } // get localhost instead of 127.0.0.1
     ServletContextHandler servletContext = new ServletContextHandler(null, "/$CONTEXT")
     servletContext.errorHandler = new ErrorHandler() {
@@ -43,12 +42,14 @@ class JettyServlet2Test extends HttpServerTest<Servlet2Decorator> {
 
     jettyServer.setHandler(servletContext)
     jettyServer.start()
+    
+    return jettyServer
   }
 
   @Override
-  void stopServer() {
+  void stopServer(Server server) {
-    jettyServer.stop()
+    server.stop()
-    jettyServer.destroy()
+    server.destroy()
   }
 
   @Override

dd-java-agent/instrumentation/servlet-3/src/test/groovy/AbstractServlet3Test.groovy

@@ -13,7 +13,7 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPT
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 
-abstract class AbstractServlet3Test<CONTEXT> extends HttpServerTest<Servlet3Decorator> {
+abstract class AbstractServlet3Test<SERVER, CONTEXT> extends HttpServerTest<SERVER, Servlet3Decorator> {
   @Override
   URI buildAddress() {
     return new URI("http://localhost:$port/$context/")

dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy

@@ -10,7 +10,6 @@ import org.apache.catalina.core.ApplicationFilterChain
 import org.eclipse.jetty.server.Server
 import org.eclipse.jetty.server.handler.ErrorHandler
 import org.eclipse.jetty.servlet.ServletContextHandler
-import spock.lang.Shared
 
 import static datadog.trace.agent.test.asserts.TraceAssert.assertTrace
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED
@@ -20,10 +19,7 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRE
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
 
-abstract class JettyServlet3Test extends AbstractServlet3Test<ServletContextHandler> {
+abstract class JettyServlet3Test extends AbstractServlet3Test<Server, ServletContextHandler> {
-
-  @Shared
-  private Server jettyServer
 
   @Override
   boolean testNotFound() {
@@ -31,8 +27,8 @@ abstract class JettyServlet3Test extends AbstractServlet3Test<ServletContextHand
   }
 
   @Override
-  void startServer(int port) {
+  Server startServer(int port) {
-    jettyServer = new Server(port)
+    def jettyServer = new Server(port)
     jettyServer.connectors.each {
       if (it.hasProperty("resolveNames")) {
         it.resolveNames = true  // get localhost instead of 127.0.0.1
@@ -52,14 +48,13 @@ abstract class JettyServlet3Test extends AbstractServlet3Test<ServletContextHand
 
     jettyServer.start()
 
-    System.out.println(
+    return jettyServer
-      "Jetty server: http://localhost:" + port + "/")
   }
 
   @Override
-  void stopServer() {
+  void stopServer(Server server) {
-    jettyServer.stop()
+    server.stop()
-    jettyServer.destroy()
+    server.destroy()
   }
 
   @Override

dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy

@@ -15,7 +15,6 @@ import org.apache.catalina.startup.Tomcat
 import org.apache.catalina.valves.ErrorReportValve
 import org.apache.tomcat.JarScanFilter
 import org.apache.tomcat.JarScanType
-import spock.lang.Shared
 
 import static datadog.trace.agent.test.asserts.TraceAssert.assertTrace
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED
@@ -26,14 +25,11 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRE
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
 
-abstract class TomcatServlet3Test extends AbstractServlet3Test<Context> {
+abstract class TomcatServlet3Test extends AbstractServlet3Test<Tomcat, Context> {
-
-  @Shared
-  Tomcat tomcatServer
 
   @Override
-  void startServer(int port) {
+  Tomcat startServer(int port) {
-    tomcatServer = new Tomcat()
+    def tomcatServer = new Tomcat()
 
     def baseDir = Files.createTempDir()
     baseDir.deleteOnExit()
@@ -62,14 +58,14 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test<Context> {
     (tomcatServer.host as StandardHost).errorReportValveClass = ErrorHandlerValve.name
 
     tomcatServer.start()
-    System.out.println(
+    
-      "Tomcat server: http://" + tomcatServer.getHost().getName() + ":" + port + "/")
+    return tomcatServer
   }
 
   @Override
-  void stopServer() {
+  void stopServer(Tomcat server) {
-    tomcatServer.stop()
+    server.stop()
-    tomcatServer.destroy()
+    server.destroy()
   }
 
   @Override

dd-java-agent/instrumentation/spring-webflux/src/test/groovy/SpringWebfluxTest.groovy

@@ -34,7 +34,7 @@ class SpringWebfluxTest extends AgentTestRunner {
   @LocalServerPort
   private int port
 
-  OkHttpClient client = OkHttpUtils.client()
+  OkHttpClient client = OkHttpUtils.client(true)
 
   def "Basic GET test #testName"() {
     setup:

dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxHttpServerTest.groovy

@@ -12,7 +12,6 @@ import io.vertx.core.Vertx
 import io.vertx.core.VertxOptions
 import io.vertx.core.json.JsonObject
 import io.vertx.ext.web.Router
-import spock.lang.Shared
 
 import java.util.concurrent.CompletableFuture
 
@@ -21,15 +20,12 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPT
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 
-class VertxHttpServerTest extends HttpServerTest<NettyHttpServerDecorator> {
+class VertxHttpServerTest extends HttpServerTest<Vertx, NettyHttpServerDecorator> {
   public static final String CONFIG_HTTP_SERVER_PORT = "http.server.port"
 
-  @Shared
-  Vertx server
-
   @Override
-  void startServer(int port) {
+  Vertx startServer(int port) {
-    server = Vertx.vertx(new VertxOptions()
+    def server = Vertx.vertx(new VertxOptions()
     // Useful for debugging:
     // .setBlockedThreadCheckInterval(Integer.MAX_VALUE)
       .setClusterPort(port))
@@ -45,6 +41,7 @@ class VertxHttpServerTest extends HttpServerTest<NettyHttpServerDecorator> {
     }
 
     future.get()
+    return server
   }
 
   protected Class<io.vertx.reactivex.core.AbstractVerticle> verticle() {
@@ -52,7 +49,7 @@ class VertxHttpServerTest extends HttpServerTest<NettyHttpServerDecorator> {
   }
 
   @Override
-  void stopServer() {
+  void stopServer(Vertx server) {
     server.close()
   }
 
@@ -85,7 +82,7 @@ class VertxHttpServerTest extends HttpServerTest<NettyHttpServerDecorator> {
       }
       router.route(REDIRECT.path).handler { ctx ->
         controller(REDIRECT) {
-          ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body)
+          ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end()
         }
       }
       router.route(ERROR.path).handler { ctx ->

dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy

@@ -56,7 +56,7 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxHttpServerTest {
           }
           HttpServerTest.ServerEndpoint endpoint = it.result()
           controller(endpoint) {
-            ctx.response().setStatusCode(endpoint.status).putHeader("location", endpoint.body)
+            ctx.response().setStatusCode(endpoint.status).putHeader("location", endpoint.body).end()
           }
         }
       }

dd-java-agent/instrumentation/vertx/src/test/groovy/server/VertxRxHttpServerTest.groovy

@@ -31,7 +31,7 @@ class VertxRxHttpServerTest extends VertxHttpServerTest {
       }
       router.route(REDIRECT.path).handler { ctx ->
         controller(REDIRECT) {
-          ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body)
+          ctx.response().setStatusCode(REDIRECT.status).putHeader("location", REDIRECT.body).end()
         }
       }
       router.route(ERROR.path).handler { ctx ->

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy

@@ -24,14 +24,17 @@ import static datadog.trace.agent.test.asserts.TraceAssert.assertTrace
 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.NOT_FOUND
+import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT
 import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
 import static org.junit.Assume.assumeTrue
 
 @Unroll
-abstract class HttpServerTest<DECORATOR extends HttpServerDecorator> extends AgentTestRunner {
+abstract class HttpServerTest<SERVER, DECORATOR extends HttpServerDecorator> extends AgentTestRunner {
 
+  @Shared
+  SERVER server
   @Shared
   OkHttpClient client = OkHttpUtils.client()
   @Shared
@@ -47,18 +50,19 @@ abstract class HttpServerTest<DECORATOR extends HttpServerDecorator> extends Age
   DECORATOR serverDecorator = decorator()
 
   def setupSpec() {
-    startServer(port)
+    server = startServer(port)
-    println "Http server started at: http://localhost:$port/"
+    println getClass().name + " http server started at: http://localhost:$port/"
   }
 
-  abstract void startServer(int port)
+  abstract SERVER startServer(int port)
 
   def cleanupSpec() {
-    stopServer()
+    stopServer(server)
-    println "Http server stopped at: http://localhost:$port/"
+    server = null
+    println getClass().name + " http server stopped at: http://localhost:$port/"
   }
 
-  abstract void stopServer()
+  abstract void stopServer(SERVER server)
 
   abstract DECORATOR decorator()
 
@@ -78,7 +82,7 @@ abstract class HttpServerTest<DECORATOR extends HttpServerDecorator> extends Age
 
   enum ServerEndpoint {
     SUCCESS("success", 200, "success"),
-    REDIRECT("redirect", 302, null),
+    REDIRECT("redirect", 302, "/redirected"),
     ERROR("error", 500, "controller error"),
     EXCEPTION("exception", 500, "controller exception"),
     NOT_FOUND("notFound", 404, "not found"),
@@ -184,6 +188,30 @@ abstract class HttpServerTest<DECORATOR extends HttpServerDecorator> extends Age
     body = null
   }
 
+  def "test redirect"() {
+    setup:
+    def request = request(REDIRECT, method, body).build()
+    def response = client.newCall(request).execute()
+
+    expect:
+    response.code() == REDIRECT.status
+    response.header("location") == REDIRECT.body ||
+      response.header("location") == "${address.resolve(REDIRECT.body)}"
+    response.body().contentLength() < 1
+
+    and:
+    cleanAndAssertTraces(1) {
+      trace(0, 2) {
+        serverSpan(it, 0, null, null, method, REDIRECT)
+        controllerSpan(it, 1, span(0))
+      }
+    }
+
+    where:
+    method = "GET"
+    body = null
+  }
+
   def "test error"() {
     setup:
     def request = request(ERROR, method, body).build()

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/OkHttpUtils.groovy

@@ -14,7 +14,7 @@ class OkHttpUtils {
       .readTimeout(1, unit)
   }
 
-  static client() {
+  static client(boolean followRedirects = false) {
-    clientBuilder().build()
+    clientBuilder().followRedirects(followRedirects).build()
   }
 }