Review fixes

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

@@ -31,7 +31,7 @@ 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<NettyHttpServerDecorator> {
   @Shared
   EventLoopGroup eventLoopGroup
 
@@ -45,7 +45,6 @@ class Netty40ServerTest  extends HttpServerTest<NettyHttpServerDecorator> {
       .childHandler([
         initChannel: { ch ->
           ChannelPipeline pipeline = ch.pipeline()
-          // def handlers = [new HttpServerCodec()]
           def handlers = [new HttpRequestDecoder(), new HttpResponseEncoder()]
           handlers.each { pipeline.addLast(it) }
           pipeline.addLast([

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

@@ -46,7 +46,6 @@ class Netty41ServerTest extends HttpServerTest<NettyHttpServerDecorator> {
         initChannel: { ch ->
           ChannelPipeline pipeline = ch.pipeline()
           def handlers = [new HttpServerCodec()]
-          // def handlers = [new HttpRequestDecoder(), new HttpResponseEncoder()]
           handlers.each { pipeline.addLast(it) }
           pipeline.addLast([
             channelRead0       : { ctx, msg ->

dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java

@@ -85,11 +85,7 @@ public class Servlet3Advice {
             // exception is thrown in filter chain, but status code is incorrect
             Tags.HTTP_STATUS.set(span, 500);
           }
-          if (throwable.getCause() == null) {
+          DECORATE.onError(span, throwable);
-            DECORATE.onError(span, throwable);
-          } else {
-            DECORATE.onError(span, throwable.getCause());
-          }
           DECORATE.beforeFinish(span);
           req.removeAttribute(SERVLET_SPAN);
           span.finish(); // Finish the span manually since finishSpanOnClose was false

dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java

@@ -4,6 +4,7 @@ import datadog.trace.agent.decorator.HttpServerDecorator;
 import io.opentracing.Span;
 import java.net.URI;
 import java.net.URISyntaxException;
+import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
@@ -59,4 +60,14 @@ public class Servlet3Decorator
     }
     return super.onRequest(span, request);
   }
+
+  @Override
+  public Span onError(final Span span, final Throwable throwable) {
+    if (throwable instanceof ServletException && throwable.getCause() != null) {
+      super.onError(span, throwable.getCause());
+    } else {
+      super.onError(span, throwable);
+    }
+    return span;
+  }
 }

dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/TagSettingAsyncListener.java

@@ -8,7 +8,6 @@ import java.io.IOException;
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.servlet.AsyncEvent;
 import javax.servlet.AsyncListener;
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletResponse;
 
 public class TagSettingAsyncListener implements AsyncListener {
@@ -48,11 +47,7 @@ public class TagSettingAsyncListener implements AsyncListener {
         // exception is thrown in filter chain, but status code is incorrect
         Tags.HTTP_STATUS.set(span, 500);
       }
-      Throwable throwable = event.getThrowable();
+      DECORATE.onError(span, event.getThrowable());
-      if (throwable instanceof ServletException && throwable.getCause() != null) {
-        throwable = throwable.getCause();
-      }
-      DECORATE.onError(span, throwable);
       DECORATE.beforeFinish(span);
       span.finish();
     }

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

@@ -94,10 +94,6 @@ abstract class AbstractServlet3Test<CONTEXT> extends HttpServerTest<Servlet3Deco
         }
         "$Tags.HTTP_STATUS.key" endpoint.status
         "$Tags.HTTP_URL.key" "${endpoint.resolve(address)}"
-//        if (tagQueryString) {
-//          "$DDTags.HTTP_QUERY" uri.query
-//          "$DDTags.HTTP_FRAGMENT" { it == null || it == uri.fragment } // Optional
-//        }
         "$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

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

@@ -203,7 +203,7 @@ abstract class JettyDispatchTest extends JettyServlet3Test {
           resourceName endpoint.status == 404 ? "404" : "GET ${endpoint.resolve(address).path}"
           spanType DDSpanTypes.HTTP_SERVER
           errored endpoint.errored
-          // parent()
+          // we can't reliably assert parent or child relationship here since both are tested.
           tags {
             "servlet.context" "/$context"
             "servlet.dispatch" endpoint.path

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

@@ -250,7 +250,7 @@ abstract class TomcatDispatchTest extends TomcatServlet3Test {
           resourceName endpoint.status == 404 ? "404" : "GET ${endpoint.resolve(address).path}"
           spanType DDSpanTypes.HTTP_SERVER
           errored endpoint.errored
-          // parent()
+          // we can't reliably assert parent or child relationship here since both are tested.
           tags {
             "servlet.context" "/$context"
             "servlet.dispatch" endpoint.path