diff --git a/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy b/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy index 6e7e6f0343..3ef33133a3 100644 --- a/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy +++ b/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy @@ -98,6 +98,8 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/$jspFileName" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER @@ -177,6 +179,8 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/getQuery.jsp" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER @@ -253,6 +257,8 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/post.jsp" "http.method" "POST" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER @@ -326,6 +332,8 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/$jspFileName" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER @@ -413,6 +421,8 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/includes/includeHtml.jsp" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER @@ -485,6 +495,8 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/includes/includeMulti.jsp" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER @@ -625,6 +637,8 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/$jspFileName" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER diff --git a/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy b/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy index 6a285ccf28..a29ef46aa8 100644 --- a/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy +++ b/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy @@ -97,6 +97,8 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/$forwardFromFileName" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER @@ -209,6 +211,8 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/forwards/forwardToHtml.jsp" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER @@ -281,6 +285,8 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/forwards/forwardToIncludeMulti.jsp" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER @@ -458,6 +464,8 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/forwards/forwardToJspForward.jsp" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER @@ -600,6 +608,8 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/forwards/forwardToCompileError.jsp" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER @@ -694,6 +704,8 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { tags { "http.url" "http://localhost:$port/$jspWebappContext/forwards/forwardToNonExistent.jsp" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "component" "java-web-servlet" "span.type" DDSpanTypes.HTTP_SERVER diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java index 75a82b5a74..cb2ed723a1 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java @@ -21,8 +21,12 @@ public abstract class AbstractServlet2Instrumentation extends Instrumenter.Defau @Override public String[] helperClassNames() { return new String[] { - "datadog.trace.instrumentation.servlet2.HttpServletRequestExtractAdapter", - "datadog.trace.instrumentation.servlet2.HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator" + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".HttpServletRequestExtractAdapter", + packageName + ".HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator", + packageName + ".Servlet2Decorator", }; } } diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java index 47bafaa35c..a809e93916 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java @@ -1,19 +1,15 @@ package datadog.trace.instrumentation.servlet2; -import static io.opentracing.log.Fields.ERROR_OBJECT; - -import datadog.trace.api.DDSpanTypes; import datadog.trace.api.DDTags; import datadog.trace.context.TraceScope; import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.propagation.Format; -import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import java.security.Principal; -import java.util.Collections; import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import net.bytebuddy.asm.Advice; @@ -38,15 +34,12 @@ public class Servlet2Advice { GlobalTracer.get() .buildSpan("servlet.request") .asChildOf(extractedContext) - .withTag(Tags.COMPONENT.getKey(), "java-web-servlet") - .withTag(Tags.HTTP_METHOD.getKey(), httpServletRequest.getMethod()) - .withTag(Tags.HTTP_URL.getKey(), httpServletRequest.getRequestURL().toString()) - .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER) - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET) .withTag("span.origin.type", servlet.getClass().getName()) - .withTag("servlet.context", httpServletRequest.getContextPath()) .startActive(true); + Servlet2Decorator.INSTANCE.afterStart(scope.span()); + Servlet2Decorator.INSTANCE.onRequest(scope.span(), httpServletRequest); + if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(true); } @@ -55,14 +48,15 @@ public class Servlet2Advice { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( + @Advice.Argument(0) final ServletRequest request, + @Advice.Argument(1) final ServletResponse response, @Advice.Enter final Scope scope, - @Advice.Argument(0) final ServletRequest req, @Advice.Thrown final Throwable throwable) { // Set user.principal regardless of who created this span. final Span currentSpan = GlobalTracer.get().activeSpan(); if (currentSpan != null) { - if (req instanceof HttpServletRequest) { - final Principal principal = ((HttpServletRequest) req).getUserPrincipal(); + if (request instanceof HttpServletRequest) { + final Principal principal = ((HttpServletRequest) request).getUserPrincipal(); if (principal != null) { currentSpan.setTag(DDTags.USER_NAME, principal.getName()); } @@ -70,14 +64,10 @@ public class Servlet2Advice { } if (scope != null) { - final Span span = scope.span(); + Servlet2Decorator.INSTANCE.onResponse(scope.span(), response); + Servlet2Decorator.INSTANCE.onError(scope.span(), throwable); + Servlet2Decorator.INSTANCE.beforeFinish(scope.span()); - // HttpServletResponse doesn't have accessor for status code. - - if (throwable != null) { - Tags.ERROR.set(span, Boolean.TRUE); - span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); - } if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(false); } diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java new file mode 100644 index 0000000000..f68cb2a2c2 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java @@ -0,0 +1,55 @@ +package datadog.trace.instrumentation.servlet2; + +import datadog.trace.agent.decorator.HttpServerDecorator; +import io.opentracing.Span; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; + +public class Servlet2Decorator extends HttpServerDecorator { + public static final Servlet2Decorator INSTANCE = new Servlet2Decorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"servlet", "servlet-2"}; + } + + @Override + protected String component() { + return "java-web-servlet"; + } + + @Override + protected String method(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getMethod(); + } + + @Override + protected String url(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getRequestURL().toString(); + } + + @Override + protected String hostname(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getServerName(); + } + + @Override + protected Integer port(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getServerPort(); + } + + @Override + protected Integer status(final ServletResponse httpServletResponse) { + // HttpServletResponse doesn't have accessor for status code. + return null; + } + + @Override + public Span onRequest(final Span span, final HttpServletRequest request) { + assert span != null; + if (request != null) { + span.setTag("servlet.context", request.getContextPath()); + } + return super.onRequest(span, request); + } +} diff --git a/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy b/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy index 85719d75d8..7db9432c38 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy +++ b/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy @@ -91,6 +91,8 @@ class JettyServlet2Test extends AgentTestRunner { "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" + "peer.hostname" "localhost" + "peer.port" port "span.type" DDSpanTypes.HTTP_SERVER "span.origin.type" "TestServlet2\$Sync" "servlet.context" "/ctx" @@ -136,6 +138,8 @@ class JettyServlet2Test extends AgentTestRunner { "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" + "peer.hostname" "localhost" + "peer.port" port "span.type" DDSpanTypes.HTTP_SERVER "span.origin.type" "TestServlet2\$Sync" "servlet.context" "/ctx" @@ -177,6 +181,8 @@ class JettyServlet2Test extends AgentTestRunner { "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" + "peer.hostname" "localhost" + "peer.port" port "span.type" DDSpanTypes.HTTP_SERVER "span.origin.type" "TestServlet2\$Sync" "servlet.context" "/ctx" diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AsyncContextInstrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AsyncContextInstrumentation.java index 09339bcb14..9c819412df 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AsyncContextInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AsyncContextInstrumentation.java @@ -33,7 +33,7 @@ public final class AsyncContextInstrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { - return new String[] {"datadog.trace.instrumentation.servlet3.HttpServletRequestInjectAdapter"}; + return new String[] {packageName + ".HttpServletRequestInjectAdapter"}; } @Override diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java index 57bf33b061..a559c7b155 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/FilterChain3Instrumentation.java @@ -24,9 +24,13 @@ public final class FilterChain3Instrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { return new String[] { - "datadog.trace.instrumentation.servlet3.HttpServletRequestExtractAdapter", - "datadog.trace.instrumentation.servlet3.HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator", - "datadog.trace.instrumentation.servlet3.TagSettingAsyncListener" + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".HttpServletRequestExtractAdapter", + packageName + ".HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator", + packageName + ".Servlet3Decorator", + packageName + ".TagSettingAsyncListener" }; } diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java index 1ce61eb884..f3e7967ed5 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServlet3Instrumentation.java @@ -24,9 +24,13 @@ public final class HttpServlet3Instrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { return new String[] { - "datadog.trace.instrumentation.servlet3.HttpServletRequestExtractAdapter", - "datadog.trace.instrumentation.servlet3.HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator", - "datadog.trace.instrumentation.servlet3.TagSettingAsyncListener" + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".HttpServletRequestExtractAdapter", + packageName + ".HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator", + packageName + ".Servlet3Decorator", + packageName + ".TagSettingAsyncListener", }; } diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java index 2be69a8ee5..a69b959a6e 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java @@ -1,8 +1,5 @@ package datadog.trace.instrumentation.servlet3; -import static io.opentracing.log.Fields.ERROR_OBJECT; - -import datadog.trace.api.DDSpanTypes; import datadog.trace.api.DDTags; import datadog.trace.context.TraceScope; import io.opentracing.Scope; @@ -12,7 +9,6 @@ import io.opentracing.propagation.Format; import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import java.security.Principal; -import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; @@ -43,15 +39,12 @@ public class Servlet3Advice { GlobalTracer.get() .buildSpan("servlet.request") .asChildOf(extractedContext) - .withTag(Tags.COMPONENT.getKey(), "java-web-servlet") - .withTag(Tags.HTTP_METHOD.getKey(), httpServletRequest.getMethod()) - .withTag(Tags.HTTP_URL.getKey(), httpServletRequest.getRequestURL().toString()) - .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER) - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET) .withTag("span.origin.type", servlet.getClass().getName()) - .withTag("servlet.context", httpServletRequest.getContextPath()) .startActive(false); + Servlet3Decorator.INSTANCE.afterStart(scope.span()); + Servlet3Decorator.INSTANCE.onRequest(scope.span(), httpServletRequest); + if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(true); } @@ -82,12 +75,13 @@ public class Servlet3Advice { final Span span = scope.span(); if (throwable != null) { + Servlet3Decorator.INSTANCE.onResponse(span, resp); if (resp.getStatus() == HttpServletResponse.SC_OK) { // exception is thrown in filter chain, but status code is incorrect Tags.HTTP_STATUS.set(span, 500); } - Tags.ERROR.set(span, Boolean.TRUE); - span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + Servlet3Decorator.INSTANCE.onError(span, throwable); + Servlet3Decorator.INSTANCE.beforeFinish(span); req.removeAttribute(SERVLET_SPAN); span.finish(); // Finish the span manually since finishSpanOnClose was false } else { @@ -102,7 +96,8 @@ public class Servlet3Advice { } // Check again in case the request finished before adding the listener. if (!req.isAsyncStarted() && activated.compareAndSet(false, true)) { - Tags.HTTP_STATUS.set(span, resp.getStatus()); + Servlet3Decorator.INSTANCE.onResponse(span, resp); + Servlet3Decorator.INSTANCE.beforeFinish(span); req.removeAttribute(SERVLET_SPAN); span.finish(); // Finish the span manually since finishSpanOnClose was false } diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java new file mode 100644 index 0000000000..54f1714093 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java @@ -0,0 +1,55 @@ +package datadog.trace.instrumentation.servlet3; + +import datadog.trace.agent.decorator.HttpServerDecorator; +import io.opentracing.Span; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +public class Servlet3Decorator + extends HttpServerDecorator { + public static final Servlet3Decorator INSTANCE = new Servlet3Decorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"servlet", "servlet-3"}; + } + + @Override + protected String component() { + return "java-web-servlet"; + } + + @Override + protected String method(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getMethod(); + } + + @Override + protected String url(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getRequestURL().toString(); + } + + @Override + protected String hostname(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getServerName(); + } + + @Override + protected Integer port(final HttpServletRequest httpServletRequest) { + return httpServletRequest.getServerPort(); + } + + @Override + protected Integer status(final HttpServletResponse httpServletResponse) { + return httpServletResponse.getStatus(); + } + + @Override + public Span onRequest(final Span span, final HttpServletRequest request) { + assert span != null; + if (request != null) { + span.setTag("servlet.context", request.getContextPath()); + } + return super.onRequest(span, request); + } +} diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/TagSettingAsyncListener.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/TagSettingAsyncListener.java index f37de59adf..1e261e7033 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/TagSettingAsyncListener.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/TagSettingAsyncListener.java @@ -1,13 +1,8 @@ package datadog.trace.instrumentation.servlet3; -import static io.opentracing.log.Fields.ERROR_OBJECT; - -import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.tag.Tags; -import io.opentracing.util.GlobalTracer; import java.io.IOException; -import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.AsyncEvent; import javax.servlet.AsyncListener; @@ -25,34 +20,34 @@ public class TagSettingAsyncListener implements AsyncListener { @Override public void onComplete(final AsyncEvent event) throws IOException { if (activated.compareAndSet(false, true)) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { - Tags.HTTP_STATUS.set(span, ((HttpServletResponse) event.getSuppliedResponse()).getStatus()); - } + Servlet3Decorator.INSTANCE.onResponse( + span, (HttpServletResponse) event.getSuppliedResponse()); + Servlet3Decorator.INSTANCE.beforeFinish(span); + span.finish(); } } @Override public void onTimeout(final AsyncEvent event) throws IOException { if (activated.compareAndSet(false, true)) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { - Tags.ERROR.set(span, Boolean.TRUE); - span.setTag("timeout", event.getAsyncContext().getTimeout()); - } + Tags.ERROR.set(span, Boolean.TRUE); + span.setTag("timeout", event.getAsyncContext().getTimeout()); + Servlet3Decorator.INSTANCE.beforeFinish(span); + span.finish(); } } @Override public void onError(final AsyncEvent event) throws IOException { if (event.getThrowable() != null && activated.compareAndSet(false, true)) { - try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, true)) { - if (((HttpServletResponse) event.getSuppliedResponse()).getStatus() - == HttpServletResponse.SC_OK) { - // exception is thrown in filter chain, but status code is incorrect - Tags.HTTP_STATUS.set(span, 500); - } - Tags.ERROR.set(span, Boolean.TRUE); - span.log(Collections.singletonMap(ERROR_OBJECT, event.getThrowable())); + if (((HttpServletResponse) event.getSuppliedResponse()).getStatus() + == HttpServletResponse.SC_OK) { + // exception is thrown in filter chain, but status code is incorrect + Tags.HTTP_STATUS.set(span, 500); } + Servlet3Decorator.INSTANCE.onError(span, event.getThrowable()); + Servlet3Decorator.INSTANCE.beforeFinish(span); + span.finish(); } } diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/AbstractServlet3Test.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/AbstractServlet3Test.groovy index 03608f84ca..aa00058efc 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/AbstractServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/AbstractServlet3Test.groovy @@ -89,6 +89,8 @@ abstract class AbstractServlet3Test extends AgentTestRunner { "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" + "peer.hostname" "localhost" + "peer.port" port "span.type" DDSpanTypes.HTTP_SERVER "span.origin.type" { it == "TestServlet3\$$origin" || it == ApplicationFilterChain.name } "servlet.context" "/$context" @@ -155,6 +157,8 @@ abstract class AbstractServlet3Test extends AgentTestRunner { "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" + "peer.hostname" "localhost" + "peer.port" port "span.type" DDSpanTypes.HTTP_SERVER "span.origin.type" { it == "TestServlet3\$Dispatch$origin" || it == ApplicationFilterChain.name } "http.status_code" 200 @@ -179,6 +183,8 @@ abstract class AbstractServlet3Test extends AgentTestRunner { "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" + "peer.hostname" "localhost" + "peer.port" port "span.type" DDSpanTypes.HTTP_SERVER "span.origin.type" { it == "TestServlet3\$$origin" || it == "TestServlet3\$DispatchRecursive" || it == ApplicationFilterChain.name @@ -211,6 +217,8 @@ abstract class AbstractServlet3Test extends AgentTestRunner { "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" + "peer.hostname" "localhost" + "peer.port" port "span.type" DDSpanTypes.HTTP_SERVER "span.origin.type" { it == "TestServlet3\$Dispatch$origin" || it == ApplicationFilterChain.name } "http.status_code" 200 @@ -269,6 +277,8 @@ abstract class AbstractServlet3Test extends AgentTestRunner { "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" + "peer.hostname" "localhost" + "peer.port" port "span.type" DDSpanTypes.HTTP_SERVER "span.origin.type" { it == "TestServlet3\$Dispatch$origin" || it == ApplicationFilterChain.name } "http.status_code" 200 @@ -291,6 +301,8 @@ abstract class AbstractServlet3Test extends AgentTestRunner { "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" + "peer.hostname" "localhost" + "peer.port" port "span.type" DDSpanTypes.HTTP_SERVER "span.origin.type" { it == "TestServlet3\$$origin" || it == "TestServlet3\$DispatchRecursive" || it == ApplicationFilterChain.name @@ -379,6 +391,8 @@ abstract class AbstractServlet3Test extends AgentTestRunner { "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" + "peer.hostname" "localhost" + "peer.port" port "span.type" DDSpanTypes.HTTP_SERVER "span.origin.type" { it == "TestServlet3\$$origin" || it == ApplicationFilterChain.name } "servlet.context" "/$context" @@ -423,6 +437,8 @@ abstract class AbstractServlet3Test extends AgentTestRunner { "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" + "peer.hostname" "localhost" + "peer.port" port "span.type" DDSpanTypes.HTTP_SERVER "span.origin.type" { it == "TestServlet3\$$origin" || it == ApplicationFilterChain.name } "servlet.context" "/$context" diff --git a/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy index 671c698f12..55d219c392 100644 --- a/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring-web/src/test/groovy/test/SpringBootBasedTest.groovy @@ -44,6 +44,8 @@ class SpringBootBasedTest extends AgentTestRunner { tags { "http.url" "http://localhost:$port/" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" "web" "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" @@ -80,6 +82,8 @@ class SpringBootBasedTest extends AgentTestRunner { tags { "http.url" "http://localhost:$port/param/$param/" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" "web" "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" @@ -118,6 +122,8 @@ class SpringBootBasedTest extends AgentTestRunner { tags { "http.url" "http://localhost:$port/param/asdf1234/" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" "web" "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" @@ -137,6 +143,8 @@ class SpringBootBasedTest extends AgentTestRunner { tags { "http.url" "http://localhost:$port/error" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" "web" "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" @@ -170,6 +178,8 @@ class SpringBootBasedTest extends AgentTestRunner { tags { "http.url" "http://localhost:$port/invalid" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" "web" "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" @@ -191,6 +201,8 @@ class SpringBootBasedTest extends AgentTestRunner { tags { "http.url" "http://localhost:$port/error" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" "web" "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" @@ -226,6 +238,8 @@ class SpringBootBasedTest extends AgentTestRunner { tags { "http.url" "http://localhost:$port/error/qwerty/" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" "web" "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" @@ -248,6 +262,8 @@ class SpringBootBasedTest extends AgentTestRunner { tags { "http.url" "http://localhost:$port/error" "http.method" "GET" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" "web" "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" @@ -278,6 +294,8 @@ class SpringBootBasedTest extends AgentTestRunner { tags { "http.url" "http://localhost:$port/validated" "http.method" "POST" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" "web" "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" @@ -314,6 +332,8 @@ class SpringBootBasedTest extends AgentTestRunner { tags { "http.url" "http://localhost:$port/validated" "http.method" "POST" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" "web" "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" @@ -339,6 +359,8 @@ class SpringBootBasedTest extends AgentTestRunner { tags { "http.url" "http://localhost:$port/error" "http.method" "POST" + "peer.hostname" "localhost" + "peer.port" port "span.kind" "server" "span.type" "web" "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"