From cde02e1ae508ee958294ec175805b48c9613058c Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 14 Nov 2018 14:38:38 -0800 Subject: [PATCH 1/4] Fix servlet async dispatch Finish existing trace and propagate to the next request. Improve dispatch testing. --- .../AbstractServlet3Instrumentation.java | 19 --- .../servlet3/AsyncContextInstrumentation.java | 97 ++++++++++++ .../servlet3/FilterChain3Instrumentation.java | 14 +- .../servlet3/HttpServlet3Instrumentation.java | 14 +- .../HttpServletRequestExtractAdapter.java | 15 ++ .../HttpServletRequestInjectAdapter.java | 27 ++++ .../servlet3/Servlet3Advice.java | 40 +++-- .../servlet3/TagSettingAsyncListener.java | 9 +- .../src/test/groovy/JettyServlet3Test.groovy | 128 +++++++++++++-- .../src/test/groovy/TestServlet3.groovy | 48 ++++++ .../src/test/groovy/TomcatServlet3Test.groovy | 148 +++++++++++++++++- .../opentracing/propagation/HTTPCodec.java | 4 + 12 files changed, 502 insertions(+), 61 deletions(-) delete mode 100644 dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java create mode 100644 dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AsyncContextInstrumentation.java create mode 100644 dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestInjectAdapter.java diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java deleted file mode 100644 index 70843b55f3..0000000000 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AbstractServlet3Instrumentation.java +++ /dev/null @@ -1,19 +0,0 @@ -package datadog.trace.instrumentation.servlet3; - -import datadog.trace.agent.tooling.Instrumenter; - -public abstract class AbstractServlet3Instrumentation extends Instrumenter.Default { - - public AbstractServlet3Instrumentation() { - super("servlet", "servlet-3"); - } - - @Override - public String[] helperClassNames() { - return new String[] { - "datadog.trace.instrumentation.servlet3.HttpServletRequestExtractAdapter", - "datadog.trace.instrumentation.servlet3.HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator", - "datadog.trace.instrumentation.servlet3.TagSettingAsyncListener" - }; - } -} 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 new file mode 100644 index 0000000000..13200e532c --- /dev/null +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/AsyncContextInstrumentation.java @@ -0,0 +1,97 @@ +package datadog.trace.instrumentation.servlet3; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.instrumentation.servlet3.Servlet3Advice.SERVLET_SPAN; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import io.opentracing.Span; +import io.opentracing.propagation.Format; +import io.opentracing.util.GlobalTracer; +import java.util.Collections; +import java.util.Map; +import javax.servlet.AsyncContext; +import javax.servlet.ServletRequest; +import javax.servlet.http.HttpServletRequest; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class AsyncContextInstrumentation extends Instrumenter.Default { + + public AsyncContextInstrumentation() { + super("servlet", "servlet-3"); + } + + @Override + public String[] helperClassNames() { + return new String[] {"datadog.trace.instrumentation.servlet3.HttpServletRequestInjectAdapter"}; + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()).and(safeHasSuperType(named("javax.servlet.AsyncContext"))); + } + + @Override + public Map transformers() { + return Collections.singletonMap( + isMethod().and(isPublic()).and(named("dispatch")), DispatchAdvice.class.getName()); + } + + /** + * When a request is dispatched, we want to close out the existing request and replace the + * propagation info in the headers with the closed span. + */ + public static class DispatchAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static boolean enter( + @Advice.This final AsyncContext context, @Advice.AllArguments final Object[] args) { + final int depth = CallDepthThreadLocalMap.incrementCallDepth(AsyncContext.class); + if (depth > 0) { + return false; + } + + final ServletRequest request = context.getRequest(); + final Object spanAttr = request.getAttribute(SERVLET_SPAN); + if (spanAttr instanceof Span) { + request.removeAttribute(SERVLET_SPAN); + final Span span = (Span) spanAttr; + // Override propagation headers by injecting attributes with new values. + if (request instanceof HttpServletRequest) { + GlobalTracer.get() + .inject( + span.context(), + Format.Builtin.TEXT_MAP, + new HttpServletRequestInjectAdapter((HttpServletRequest) request)); + } + final String path; + if (args.length == 1 && args[0] instanceof String) { + path = (String) args[0]; + } else if (args.length == 2 && args[1] instanceof String) { + path = (String) args[1]; + } else { + path = "true"; + } + span.setTag("servlet.dispatch", path); + span.finish(); + } + return true; + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + public static void exit(@Advice.Enter final boolean topLevel) { + if (topLevel) { + CallDepthThreadLocalMap.reset(AsyncContext.class); + } + } + } +} 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 cf22f3f398..85009e75a3 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 @@ -15,7 +15,19 @@ import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) -public final class FilterChain3Instrumentation extends AbstractServlet3Instrumentation { +public final class FilterChain3Instrumentation extends Instrumenter.Default { + public FilterChain3Instrumentation() { + super("servlet", "servlet-3"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.instrumentation.servlet3.HttpServletRequestExtractAdapter", + "datadog.trace.instrumentation.servlet3.HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator", + "datadog.trace.instrumentation.servlet3.TagSettingAsyncListener" + }; + } @Override public ElementMatcher typeMatcher() { 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 fc4afda416..e653a88bde 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 @@ -15,7 +15,19 @@ import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) -public final class HttpServlet3Instrumentation extends AbstractServlet3Instrumentation { +public final class HttpServlet3Instrumentation extends Instrumenter.Default { + public HttpServlet3Instrumentation() { + super("servlet", "servlet-3"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.instrumentation.servlet3.HttpServletRequestExtractAdapter", + "datadog.trace.instrumentation.servlet3.HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator", + "datadog.trace.instrumentation.servlet3.TagSettingAsyncListener" + }; + } @Override public ElementMatcher typeMatcher() { diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestExtractAdapter.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestExtractAdapter.java index 0ecb00f9c0..e50ad6f693 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestExtractAdapter.java +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestExtractAdapter.java @@ -3,6 +3,7 @@ package datadog.trace.instrumentation.servlet3; import io.opentracing.propagation.TextMap; import java.util.AbstractMap; import java.util.ArrayList; +import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.Iterator; @@ -52,6 +53,20 @@ public class HttpServletRequestExtractAdapter implements TextMap { headersResult.put(headerName, valuesList); } + /* + * Read from the attributes and override the headers. + * This is used by HttpServletRequestInjectAdapter when a request is async-dispatched. + */ + final Enumeration attributeNamesIt = httpServletRequest.getAttributeNames(); + while (attributeNamesIt.hasMoreElements()) { + final String attributeName = attributeNamesIt.nextElement(); + + final Object valuesIt = httpServletRequest.getAttribute(attributeName); + if (valuesIt instanceof String) { + headersResult.put(attributeName, Collections.singletonList((String) valuesIt)); + } + } + return headersResult; } diff --git a/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestInjectAdapter.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestInjectAdapter.java new file mode 100644 index 0000000000..967243bbf2 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestInjectAdapter.java @@ -0,0 +1,27 @@ +package datadog.trace.instrumentation.servlet3; + +import io.opentracing.propagation.TextMap; +import java.util.Iterator; +import java.util.Map; +import javax.servlet.http.HttpServletRequest; + +/** Inject into request attributes since the request headers can't be modified. */ +public class HttpServletRequestInjectAdapter implements TextMap { + + private final HttpServletRequest httpServletRequest; + + public HttpServletRequestInjectAdapter(final HttpServletRequest httpServletRequest) { + this.httpServletRequest = httpServletRequest; + } + + @Override + public Iterator> iterator() { + throw new UnsupportedOperationException( + "This class should be used only with Tracer.extract()!"); + } + + @Override + public void put(final String key, final String value) { + httpServletRequest.setAttribute(key, value); + } +} 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 0617619454..16dd74c6c9 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 @@ -21,11 +21,13 @@ import javax.servlet.http.HttpServletResponse; import net.bytebuddy.asm.Advice; public class Servlet3Advice { + public static final String SERVLET_SPAN = "datadog.servlet.span"; @Advice.OnMethodEnter(suppress = Throwable.class) public static Scope startSpan( @Advice.This final Object servlet, @Advice.Argument(0) final ServletRequest req) { - if (GlobalTracer.get().activeSpan() != null || !(req instanceof HttpServletRequest)) { + final Object spanAttr = req.getAttribute(SERVLET_SPAN); + if (!(req instanceof HttpServletRequest) || spanAttr != null) { // Tracing might already be applied by the FilterChain. If so ignore this. return null; } @@ -53,6 +55,8 @@ public class Servlet3Advice { if (scope instanceof TraceScope) { ((TraceScope) scope).setAsyncPropagation(true); } + + req.setAttribute(SERVLET_SPAN, scope.span()); return scope; } @@ -63,13 +67,11 @@ public class Servlet3Advice { @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) { // Set user.principal regardless of who created this span. - final Span currentSpan = GlobalTracer.get().activeSpan(); - if (currentSpan != null) { - if (request instanceof HttpServletRequest) { - final Principal principal = ((HttpServletRequest) request).getUserPrincipal(); - if (principal != null) { - currentSpan.setTag(DDTags.USER_NAME, principal.getName()); - } + final Object spanAttr = request.getAttribute(SERVLET_SPAN); + if (spanAttr instanceof Span && request instanceof HttpServletRequest) { + final Principal principal = ((HttpServletRequest) request).getUserPrincipal(); + if (principal != null) { + ((Span) spanAttr).setTag(DDTags.USER_NAME, principal.getName()); } } @@ -90,19 +92,23 @@ public class Servlet3Advice { ((TraceScope) scope).setAsyncPropagation(false); } scope.close(); + req.removeAttribute(SERVLET_SPAN); span.finish(); // Finish the span manually since finishSpanOnClose was false - } else if (req.isAsyncStarted()) { - final AtomicBoolean activated = new AtomicBoolean(false); - // what if async is already finished? This would not be called - req.getAsyncContext().addListener(new TagSettingAsyncListener(activated, span)); - scope.close(); } else { - Tags.HTTP_STATUS.set(span, resp.getStatus()); - if (scope instanceof TraceScope) { - ((TraceScope) scope).setAsyncPropagation(false); + final AtomicBoolean activated = new AtomicBoolean(false); + if (req.isAsyncStarted()) { + req.getAsyncContext().addListener(new TagSettingAsyncListener(activated, span)); + } + // 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()); + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(false); + } + req.removeAttribute(SERVLET_SPAN); + span.finish(); // Finish the span manually since finishSpanOnClose was false } scope.close(); - 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/TagSettingAsyncListener.java b/dd-java-agent/instrumentation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/TagSettingAsyncListener.java index e99645e387..08b5a39d75 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 @@ -10,6 +10,7 @@ import io.opentracing.util.GlobalTracer; import java.io.IOException; import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; +import javax.servlet.AsyncContext; import javax.servlet.AsyncEvent; import javax.servlet.AsyncListener; import javax.servlet.http.HttpServletResponse; @@ -72,6 +73,12 @@ public class TagSettingAsyncListener implements AsyncListener { } } + /** Re-attach listener for dispatch. */ @Override - public void onStartAsync(final AsyncEvent event) throws IOException {} + public void onStartAsync(final AsyncEvent event) { + final AsyncContext eventAsyncContext = event.getAsyncContext(); + if (eventAsyncContext != null) { + eventAsyncContext.addListener(this, event.getSuppliedRequest(), event.getSuppliedResponse()); + } + } } diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy index 052c96c451..85baaf2f41 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy @@ -49,6 +49,10 @@ class JettyServlet3Test extends AgentTestRunner { servletContext.addServlet(TestServlet3.Sync, "/auth/sync") servletContext.addServlet(TestServlet3.Async, "/async") servletContext.addServlet(TestServlet3.Async, "/auth/async") + servletContext.addServlet(TestServlet3.BlockingAsync, "/blocking") + servletContext.addServlet(TestServlet3.DispatchSync, "/dispatch/sync") + servletContext.addServlet(TestServlet3.DispatchAsync, "/dispatch/async") + servletContext.addServlet(TestServlet3.FakeAsync, "/fake") jettyServer.setHandler(servletContext) jettyServer.start() @@ -111,40 +115,134 @@ class JettyServlet3Test extends AgentTestRunner { } where: - path | expectedResponse | auth | origin | distributedTracing - "async" | "Hello Async" | false | "Async" | false - "sync" | "Hello Sync" | false | "Sync" | false - "auth/async" | "Hello Async" | true | "Async" | false - "auth/sync" | "Hello Sync" | true | "Sync" | false - "async" | "Hello Async" | false | "Async" | true - "sync" | "Hello Sync" | false | "Sync" | true - "auth/async" | "Hello Async" | true | "Async" | true - "auth/sync" | "Hello Sync" | true | "Sync" | true + path | expectedResponse | auth | origin | distributedTracing + "async" | "Hello Async" | false | "Async" | false + "sync" | "Hello Sync" | false | "Sync" | false + "auth/async" | "Hello Async" | true | "Async" | false + "auth/sync" | "Hello Sync" | true | "Sync" | false + "blocking" | "Hello BlockingAsync" | false | "BlockingAsync" | false + "fake" | "Hello FakeAsync" | false | "FakeAsync" | false + "async" | "Hello Async" | false | "Async" | true + "sync" | "Hello Sync" | false | "Sync" | true + "auth/async" | "Hello Async" | true | "Async" | true + "auth/sync" | "Hello Sync" | true | "Sync" | true + "blocking" | "Hello BlockingAsync" | false | "BlockingAsync" | true + "fake" | "Hello FakeAsync" | false | "FakeAsync" | true + } + + def "test dispatch #path"() { + setup: + def requestBuilder = new Request.Builder() + .url("http://localhost:$port/dispatch/$path") + .get() + if (distributedTracing) { + requestBuilder.header("x-datadog-trace-id", "123") + requestBuilder.header("x-datadog-parent-id", "456") + } + def response = client.newCall(requestBuilder.build()).execute() + + expect: + response.body().string().trim() == "Hello $type" + + assertTraces(2) { + trace(0, 1) { + span(0) { + if (distributedTracing) { + traceId "123" + parentId "456" + } else { + parent() + } + serviceName "unnamed-java-app" + operationName "servlet.request" + resourceName "GET /dispatch/$path" + spanType DDSpanTypes.WEB_SERVLET + errored false + tags { + "http.url" "http://localhost:$port/dispatch/$path" + "http.method" "GET" + "span.kind" "server" + "component" "java-web-servlet" + "span.origin.type" "TestServlet3\$Dispatch$type" + "span.type" DDSpanTypes.WEB_SERVLET + "http.status_code" 200 + "servlet.dispatch" "/$path" + defaultTags(distributedTracing) + } + } + } + trace(1, 1) { + span(0) { + serviceName "unnamed-java-app" + operationName "servlet.request" + resourceName "GET /$path" + spanType DDSpanTypes.WEB_SERVLET + errored false + tags { + "http.url" "http://localhost:$port/$path" + "http.method" "GET" + "span.kind" "server" + "component" "java-web-servlet" + "span.origin.type" "TestServlet3\$$type" + "span.type" DDSpanTypes.WEB_SERVLET + "http.status_code" 200 + defaultTags(true) + } + } + } + } + + where: + path | distributedTracing + "sync" | true + "sync" | false + "async" | true + "async" | false + + type = path.capitalize() } def "servlet instrumentation clears state after async request"() { setup: def request = new Request.Builder() - .url("http://localhost:$port/async") + .url("http://localhost:$port/$path") .get() .build() - def numTraces = 5 + def numTraces = 10 for (int i = 0; i < numTraces; ++i) { client.newCall(request).execute() } expect: - assertTraces(numTraces) { - for (int i = 0; i < numTraces; ++i) { - trace(i, 1) { + assertTraces(dispatched ? numTraces * 2 : numTraces) { + for (int i = 0; (dispatched ? i + 1 : i) < TEST_WRITER.size(); i += (dispatched ? 2 : 1)) { + if (dispatched) { + trace(i, 1) { + span(0) { + operationName "servlet.request" + resourceName "GET /dispatch/async" + parent() + } + } + } + trace(dispatched ? i + 1 : i, 1) { span(0) { - serviceName "unnamed-java-app" operationName "servlet.request" resourceName "GET /async" + if (dispatched) { + childOf TEST_WRITER[i][0] + } else { + parent() + } } } } } + + where: + path | dispatched + "async" | false + "dispatch/async" | true } def "test #path error servlet call"() { diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet3.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet3.groovy index 27b5ebb2ed..673e36479e 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet3.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet3.groovy @@ -3,6 +3,7 @@ import groovy.servlet.AbstractHttpServlet import javax.servlet.annotation.WebServlet import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse +import java.util.concurrent.CountDownLatch class TestServlet3 { @@ -25,11 +26,58 @@ class TestServlet3 { static class Async extends AbstractHttpServlet { @Override void doGet(HttpServletRequest req, HttpServletResponse resp) { + def latch = new CountDownLatch(1) def context = req.startAsync() context.start { + latch.await() resp.writer.print("Hello Async") context.complete() } + latch.countDown() + } + } + + @WebServlet(asyncSupported = true) + static class BlockingAsync extends AbstractHttpServlet { + @Override + void doGet(HttpServletRequest req, HttpServletResponse resp) { + def latch = new CountDownLatch(1) + def context = req.startAsync() + context.start { + resp.writer.print("Hello BlockingAsync") + context.complete() + latch.countDown() + } + latch.await() + } + } + + @WebServlet(asyncSupported = true) + static class DispatchSync extends AbstractHttpServlet { + @Override + void doGet(HttpServletRequest req, HttpServletResponse resp) { + req.startAsync().dispatch("/sync") + } + } + + @WebServlet(asyncSupported = true) + static class DispatchAsync extends AbstractHttpServlet { + @Override + void doGet(HttpServletRequest req, HttpServletResponse resp) { + def context = req.startAsync() + context.start { + context.dispatch("/async") + } + } + } + + @WebServlet(asyncSupported = true) + static class FakeAsync extends AbstractHttpServlet { + @Override + void doGet(HttpServletRequest req, HttpServletResponse resp) { + def context = req.startAsync() + resp.writer.print("Hello FakeAsync") + context.complete() } } } diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy index d823067a11..d23defacd1 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy @@ -52,6 +52,18 @@ class TomcatServlet3Test extends AgentTestRunner { Tomcat.addServlet(appContext, "asyncServlet", new TestServlet3.Async()) appContext.addServletMappingDecoded("/async", "asyncServlet") + Tomcat.addServlet(appContext, "blockingServlet", new TestServlet3.BlockingAsync()) + appContext.addServletMappingDecoded("/blocking", "blockingServlet") + + Tomcat.addServlet(appContext, "dispatchServlet", new TestServlet3.DispatchSync()) + appContext.addServletMappingDecoded("/dispatch/sync", "dispatchServlet") + + Tomcat.addServlet(appContext, "dispatchAsyncServlet", new TestServlet3.DispatchAsync()) + appContext.addServletMappingDecoded("/dispatch/async", "dispatchAsyncServlet") + + Tomcat.addServlet(appContext, "fakeServlet", new TestServlet3.FakeAsync()) + appContext.addServletMappingDecoded("/fake", "fakeServlet") + tomcatServer.start() System.out.println( "Tomcat server: http://" + tomcatServer.getHost().getName() + ":" + port + "/") @@ -62,7 +74,7 @@ class TomcatServlet3Test extends AgentTestRunner { tomcatServer.destroy() } - def "test #path servlet call (distributed tracing: #distributedTracing)"() { + def "test #path servlet call (auth: #auth, distributed tracing: #distributedTracing)"() { setup: def requestBuilder = new Request.Builder() .url("http://localhost:$port/my-context/$path") @@ -106,11 +118,133 @@ class TomcatServlet3Test extends AgentTestRunner { } where: - path | expectedResponse | distributedTracing - "async" | "Hello Async" | false - "sync" | "Hello Sync" | false - "async" | "Hello Async" | true - "sync" | "Hello Sync" | true + path | expectedResponse | origin | distributedTracing + "async" | "Hello Async" | "Async" | false + "sync" | "Hello Sync" | "Sync" | false + "blocking" | "Hello BlockingAsync" | "BlockingAsync" | false + "fake" | "Hello FakeAsync" | "FakeAsync" | false + "async" | "Hello Async" | "Async" | true + "sync" | "Hello Sync" | "Sync" | true + "blocking" | "Hello BlockingAsync" | "BlockingAsync" | true + "fake" | "Hello FakeAsync" | "FakeAsync" | true + } + + def "test dispatch #path"() { + setup: + def requestBuilder = new Request.Builder() + .url("http://localhost:$port/my-context/dispatch/$path") + .get() + if (distributedTracing) { + requestBuilder.header("x-datadog-trace-id", "123") + requestBuilder.header("x-datadog-parent-id", "456") + } + def response = client.newCall(requestBuilder.build()).execute() + + expect: + response.body().string().trim() == "Hello $type" + + assertTraces(2) { + trace(0, 1) { + span(0) { + if (distributedTracing) { + traceId "123" + parentId "456" + } else { + parent() + } + serviceName "my-context" + operationName "servlet.request" + resourceName "GET /my-context/dispatch/$path" + spanType DDSpanTypes.WEB_SERVLET + errored false + tags { + "http.url" "http://localhost:$port/my-context/dispatch/$path" + "http.method" "GET" + "span.kind" "server" + "component" "java-web-servlet" + "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" + "span.type" DDSpanTypes.WEB_SERVLET + "http.status_code" 200 + "servlet.context" "/my-context" + "servlet.dispatch" "/$path" + defaultTags(distributedTracing) + } + } + } + trace(1, 1) { + span(0) { + serviceName "my-context" + operationName "servlet.request" + resourceName "GET /my-context/$path" + spanType DDSpanTypes.WEB_SERVLET + errored false + childOf TEST_WRITER[0][0] + tags { + "http.url" "http://localhost:$port/my-context/$path" + "http.method" "GET" + "span.kind" "server" + "component" "java-web-servlet" + "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" + "span.type" DDSpanTypes.WEB_SERVLET + "http.status_code" 200 + "servlet.context" "/my-context" + defaultTags(true) + } + } + } + } + + where: + path | distributedTracing + "sync" | true + "sync" | false + "async" | true + "async" | false + + type = path.capitalize() + } + + def "servlet instrumentation clears state after async request"() { + setup: + def request = new Request.Builder() + .url("http://localhost:$port/my-context/$path") + .get() + .build() + def numTraces = 10 + for (int i = 0; i < numTraces; ++i) { + client.newCall(request).execute() + } + + expect: + assertTraces(dispatched ? numTraces * 2 : numTraces) { + for (int i = 0; (dispatched ? i + 1 : i) < TEST_WRITER.size(); i += (dispatched ? 2 : 1)) { + if (dispatched) { + trace(i, 1) { + span(0) { + operationName "servlet.request" + resourceName "GET /my-context/dispatch/async" + parent() + } + } + } + trace(dispatched ? i + 1 : i, 1) { + span(0) { + operationName "servlet.request" + resourceName "GET /my-context/async" + if (dispatched) { + childOf TEST_WRITER[i][0] + } else { + parent() + } + } + } + } + } + + where: + path | dispatched + "async" | false + "dispatch/async" | true } def "test #path error servlet call"() { @@ -155,7 +289,7 @@ class TomcatServlet3Test extends AgentTestRunner { "sync" | "Hello Sync" } - def "test #path error servlet call for non-throwing error"() { + def "test #path non-throwing-error servlet call"() { setup: def request = new Request.Builder() .url("http://localhost:$port/my-context/$path?non-throwing-error=true") diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java index 3d5158a85e..7dd0676241 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/propagation/HTTPCodec.java @@ -62,6 +62,10 @@ public class HTTPCodec implements Codec { final String key = entry.getKey().toLowerCase(); final String val = entry.getValue(); + if (val == null) { + continue; + } + if (TRACE_ID_KEY.equalsIgnoreCase(key)) { traceId = validateUInt64BitsID(val); } else if (SPAN_ID_KEY.equalsIgnoreCase(key)) { From 817895b559f9a8ca4ba06f26e34f3e7be8acbcb4 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 15 Nov 2018 09:51:22 -0800 Subject: [PATCH 2/4] Extract shared tests into abstract parent --- .../test/groovy/AbstractServlet3Test.groovy | 323 +++++++++++++++++ .../src/test/groovy/JettyServlet3Test.groovy | 326 +----------------- .../src/test/groovy/TomcatServlet3Test.groovy | 320 +++-------------- .../src/test/resources/tomcat-users.xml | 4 + 4 files changed, 387 insertions(+), 586 deletions(-) create mode 100644 dd-java-agent/instrumentation/servlet-3/src/test/groovy/AbstractServlet3Test.groovy create mode 100644 dd-java-agent/instrumentation/servlet-3/src/test/resources/tomcat-users.xml 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 new file mode 100644 index 0000000000..870f197160 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/AbstractServlet3Test.groovy @@ -0,0 +1,323 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.TestUtils +import datadog.trace.agent.test.utils.OkHttpUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.api.DDTags +import okhttp3.Credentials +import okhttp3.Interceptor +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.Response +import org.apache.catalina.core.ApplicationFilterChain +import spock.lang.Shared +import spock.lang.Unroll + +import javax.servlet.Servlet + +// Need to be explicit to unroll inherited tests: +@Unroll +abstract class AbstractServlet3Test extends AgentTestRunner { + + @Shared + OkHttpClient client = OkHttpUtils.clientBuilder().addNetworkInterceptor(new Interceptor() { + @Override + Response intercept(Interceptor.Chain chain) throws IOException { + def response = chain.proceed(chain.request()) + TEST_WRITER.waitForTraces(1) + return response + } + }) + .build() + + @Shared + int port = TestUtils.randomOpenPort() + @Shared + String user = "user" + @Shared + String pass = "password" + + abstract String getContext() + + abstract void addServlet(CONTEXT context, String url, Class servlet) + + protected void setupServlets(CONTEXT context) { + addServlet(context, "/sync", TestServlet3.Sync) + addServlet(context, "/auth/sync", TestServlet3.Sync) + addServlet(context, "/async", TestServlet3.Async) + addServlet(context, "/auth/async", TestServlet3.Async) + addServlet(context, "/blocking", TestServlet3.BlockingAsync) + addServlet(context, "/dispatch/sync", TestServlet3.DispatchSync) + addServlet(context, "/dispatch/async", TestServlet3.DispatchAsync) + addServlet(context, "/fake", TestServlet3.FakeAsync) + } + + def "test #path servlet call (auth: #auth, distributed tracing: #distributedTracing)"() { + setup: + def requestBuilder = new Request.Builder() + .url("http://localhost:$port/$context/$path") + .get() + if (distributedTracing) { + requestBuilder.header("x-datadog-trace-id", "123") + requestBuilder.header("x-datadog-parent-id", "456") + } + if (auth) { + requestBuilder.header("Authorization", Credentials.basic(user, pass)) + } + def response = client.newCall(requestBuilder.build()).execute() + + expect: + response.body().string().trim() == expectedResponse + + assertTraces(1) { + trace(0, 1) { + span(0) { + if (distributedTracing) { + traceId "123" + parentId "456" + } else { + parent() + } + serviceName context + operationName "servlet.request" + resourceName "GET /$context/$path" + spanType DDSpanTypes.WEB_SERVLET + errored false + tags { + "http.url" "http://localhost:$port/$context/$path" + "http.method" "GET" + "span.kind" "server" + "component" "java-web-servlet" + "span.origin.type" { it == "TestServlet3\$$origin" || it == ApplicationFilterChain.name } + "span.type" DDSpanTypes.WEB_SERVLET + "servlet.context" "/$context" + "http.status_code" 200 + if (auth) { + "$DDTags.USER_NAME" user + } + defaultTags(distributedTracing) + } + } + } + } + + where: + path | expectedResponse | auth | origin | distributedTracing + "async" | "Hello Async" | false | "Async" | false + "sync" | "Hello Sync" | false | "Sync" | false + "auth/async" | "Hello Async" | true | "Async" | false + "auth/sync" | "Hello Sync" | true | "Sync" | false + "blocking" | "Hello BlockingAsync" | false | "BlockingAsync" | false + "fake" | "Hello FakeAsync" | false | "FakeAsync" | false + "async" | "Hello Async" | false | "Async" | true + "sync" | "Hello Sync" | false | "Sync" | true + "auth/async" | "Hello Async" | true | "Async" | true + "auth/sync" | "Hello Sync" | true | "Sync" | true + "blocking" | "Hello BlockingAsync" | false | "BlockingAsync" | true + "fake" | "Hello FakeAsync" | false | "FakeAsync" | true + } + + def "test dispatch #path"() { + setup: + def requestBuilder = new Request.Builder() + .url("http://localhost:$port/$context/dispatch/$path") + .get() + if (distributedTracing) { + requestBuilder.header("x-datadog-trace-id", "123") + requestBuilder.header("x-datadog-parent-id", "456") + } + def response = client.newCall(requestBuilder.build()).execute() + + expect: + response.body().string().trim() == "Hello $origin" + + assertTraces(2) { + trace(0, 1) { + span(0) { + if (distributedTracing) { + traceId "123" + parentId "456" + } else { + parent() + } + serviceName context + operationName "servlet.request" + resourceName "GET /$context/dispatch/$path" + spanType DDSpanTypes.WEB_SERVLET + errored false + tags { + "http.url" "http://localhost:$port/$context/dispatch/$path" + "http.method" "GET" + "span.kind" "server" + "component" "java-web-servlet" + "span.origin.type" { it == "TestServlet3\$Dispatch$origin" || it == ApplicationFilterChain.name } + "span.type" DDSpanTypes.WEB_SERVLET + "http.status_code" 200 + "servlet.context" "/$context" + "servlet.dispatch" "/$path" + defaultTags(distributedTracing) + } + } + } + trace(1, 1) { + span(0) { + serviceName context + operationName "servlet.request" + resourceName "GET /$context/$path" + spanType DDSpanTypes.WEB_SERVLET + errored false + childOf TEST_WRITER[0][0] + tags { + "http.url" "http://localhost:$port/$context/$path" + "http.method" "GET" + "span.kind" "server" + "component" "java-web-servlet" + "span.origin.type" { it == "TestServlet3\$$origin" || it == ApplicationFilterChain.name } + "span.type" DDSpanTypes.WEB_SERVLET + "http.status_code" 200 + "servlet.context" "/$context" + defaultTags(true) + } + } + } + } + + where: + path | distributedTracing + "sync" | true + "sync" | false + "async" | true + "async" | false + + origin = path.capitalize() + } + + def "servlet instrumentation clears state after async request"() { + setup: + def request = new Request.Builder() + .url("http://localhost:$port/$context/$path") + .get() + .build() + def numTraces = 1 + for (int i = 0; i < numTraces; ++i) { + client.newCall(request).execute() + } + + expect: + assertTraces(dispatched ? numTraces * 2 : numTraces) { + for (int i = 0; (dispatched ? i + 1 : i) < TEST_WRITER.size(); i += (dispatched ? 2 : 1)) { + if (dispatched) { + trace(i, 1) { + span(0) { + operationName "servlet.request" + resourceName "GET /$context/dispatch/async" + parent() + } + } + } + trace(dispatched ? i + 1 : i, 1) { + span(0) { + operationName "servlet.request" + resourceName "GET /$context/async" + if (dispatched) { + childOf TEST_WRITER[i][0] + } else { + parent() + } + } + } + } + } + + where: + path | dispatched + "async" | false + "dispatch/async" | true + } + + def "test #path error servlet call"() { + setup: + def request = new Request.Builder() + .url("http://localhost:$port/$context/$path?error=true") + .get() + .build() + def response = client.newCall(request).execute() + + expect: + response.body().string().trim() != expectedResponse + + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName context + operationName "servlet.request" + resourceName "GET /$context/$path" + spanType DDSpanTypes.WEB_SERVLET + errored true + parent() + tags { + "http.url" "http://localhost:$port/$context/$path" + "http.method" "GET" + "span.kind" "server" + "component" "java-web-servlet" + "span.type" DDSpanTypes.WEB_SERVLET + "span.origin.type" { it == "TestServlet3\$$origin" || it == ApplicationFilterChain.name } + "servlet.context" "/$context" + "http.status_code" 500 + errorTags(RuntimeException, "some $path error") + defaultTags() + } + } + } + } + + where: + path | expectedResponse + //"async" | "Hello Async" // FIXME: I can't seem get the async error handler to trigger + "sync" | "Hello Sync" + + origin = path.capitalize() + } + + def "test #path non-throwing-error servlet call"() { + setup: + def request = new Request.Builder() + .url("http://localhost:$port/$context/$path?non-throwing-error=true") + .get() + .build() + def response = client.newCall(request).execute() + + expect: + response.body().string().trim() != expectedResponse + + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName context + operationName "servlet.request" + resourceName "GET /$context/$path" + spanType DDSpanTypes.WEB_SERVLET + errored true + parent() + tags { + "http.url" "http://localhost:$port/$context/$path" + "http.method" "GET" + "span.kind" "server" + "component" "java-web-servlet" + "span.type" DDSpanTypes.WEB_SERVLET + "span.origin.type" { it == "TestServlet3\$$origin" || it == ApplicationFilterChain.name } + "servlet.context" "/$context" + "http.status_code" 500 + "error" true + defaultTags() + } + } + } + } + + where: + path | expectedResponse + "sync" | "Hello Sync" + + origin = path.capitalize() + } +} diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy index 85baaf2f41..edaae29fd2 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy @@ -1,13 +1,4 @@ -import datadog.trace.agent.test.AgentTestRunner import datadog.trace.agent.test.TestUtils -import datadog.trace.agent.test.utils.OkHttpUtils -import datadog.trace.api.DDSpanTypes -import datadog.trace.api.DDTags -import okhttp3.Credentials -import okhttp3.Interceptor -import okhttp3.OkHttpClient -import okhttp3.Request -import okhttp3.Response import org.eclipse.jetty.security.ConstraintMapping import org.eclipse.jetty.security.ConstraintSecurityHandler import org.eclipse.jetty.security.HashLoginService @@ -18,43 +9,22 @@ import org.eclipse.jetty.servlet.ServletContextHandler import org.eclipse.jetty.util.security.Constraint import spock.lang.Shared -class JettyServlet3Test extends AgentTestRunner { +import javax.servlet.Servlet - OkHttpClient client = OkHttpUtils.clientBuilder().addNetworkInterceptor(new Interceptor() { - @Override - Response intercept(Interceptor.Chain chain) throws IOException { - def response = chain.proceed(chain.request()) - TEST_WRITER.waitForTraces(1) - return response - } - }) - .build() +class JettyServlet3Test extends AbstractServlet3Test { - @Shared - int port @Shared private Server jettyServer - @Shared - private ServletContextHandler servletContext def setupSpec() { port = TestUtils.randomOpenPort() jettyServer = new Server(port) - servletContext = new ServletContextHandler() - - ConstraintSecurityHandler security = setupAuthentication(jettyServer) - - servletContext.setSecurityHandler(security) - servletContext.addServlet(TestServlet3.Sync, "/sync") - servletContext.addServlet(TestServlet3.Sync, "/auth/sync") - servletContext.addServlet(TestServlet3.Async, "/async") - servletContext.addServlet(TestServlet3.Async, "/auth/async") - servletContext.addServlet(TestServlet3.BlockingAsync, "/blocking") - servletContext.addServlet(TestServlet3.DispatchSync, "/dispatch/sync") - servletContext.addServlet(TestServlet3.DispatchAsync, "/dispatch/async") - servletContext.addServlet(TestServlet3.FakeAsync, "/fake") + ServletContextHandler servletContext = new ServletContextHandler(null, "/$context") + setupAuthentication(jettyServer, servletContext) + setupServlets(servletContext) jettyServer.setHandler(servletContext) + jettyServer.start() System.out.println( @@ -66,278 +36,18 @@ class JettyServlet3Test extends AgentTestRunner { jettyServer.destroy() } - def "test #path servlet call (auth: #auth, distributed tracing: #distributedTracing)"() { - setup: - def requestBuilder = new Request.Builder() - .url("http://localhost:$port/$path") - .get() - if (distributedTracing) { - requestBuilder.header("x-datadog-trace-id", "123") - requestBuilder.header("x-datadog-parent-id", "456") - } - if (auth) { - requestBuilder.header("Authorization", Credentials.basic("user", "password")) - } - def response = client.newCall(requestBuilder.build()).execute() - - expect: - response.body().string().trim() == expectedResponse - - assertTraces(1) { - trace(0, 1) { - span(0) { - if (distributedTracing) { - traceId "123" - parentId "456" - } else { - parent() - } - serviceName "unnamed-java-app" - operationName "servlet.request" - resourceName "GET /$path" - spanType DDSpanTypes.WEB_SERVLET - errored false - tags { - "http.url" "http://localhost:$port/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "span.origin.type" "TestServlet3\$$origin" - "span.type" DDSpanTypes.WEB_SERVLET - "http.status_code" 200 - if (auth) { - "$DDTags.USER_NAME" "user" - } - defaultTags(distributedTracing) - } - } - } - } - - where: - path | expectedResponse | auth | origin | distributedTracing - "async" | "Hello Async" | false | "Async" | false - "sync" | "Hello Sync" | false | "Sync" | false - "auth/async" | "Hello Async" | true | "Async" | false - "auth/sync" | "Hello Sync" | true | "Sync" | false - "blocking" | "Hello BlockingAsync" | false | "BlockingAsync" | false - "fake" | "Hello FakeAsync" | false | "FakeAsync" | false - "async" | "Hello Async" | false | "Async" | true - "sync" | "Hello Sync" | false | "Sync" | true - "auth/async" | "Hello Async" | true | "Async" | true - "auth/sync" | "Hello Sync" | true | "Sync" | true - "blocking" | "Hello BlockingAsync" | false | "BlockingAsync" | true - "fake" | "Hello FakeAsync" | false | "FakeAsync" | true + @Override + String getContext() { + return "jetty-context" } - def "test dispatch #path"() { - setup: - def requestBuilder = new Request.Builder() - .url("http://localhost:$port/dispatch/$path") - .get() - if (distributedTracing) { - requestBuilder.header("x-datadog-trace-id", "123") - requestBuilder.header("x-datadog-parent-id", "456") - } - def response = client.newCall(requestBuilder.build()).execute() - - expect: - response.body().string().trim() == "Hello $type" - - assertTraces(2) { - trace(0, 1) { - span(0) { - if (distributedTracing) { - traceId "123" - parentId "456" - } else { - parent() - } - serviceName "unnamed-java-app" - operationName "servlet.request" - resourceName "GET /dispatch/$path" - spanType DDSpanTypes.WEB_SERVLET - errored false - tags { - "http.url" "http://localhost:$port/dispatch/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "span.origin.type" "TestServlet3\$Dispatch$type" - "span.type" DDSpanTypes.WEB_SERVLET - "http.status_code" 200 - "servlet.dispatch" "/$path" - defaultTags(distributedTracing) - } - } - } - trace(1, 1) { - span(0) { - serviceName "unnamed-java-app" - operationName "servlet.request" - resourceName "GET /$path" - spanType DDSpanTypes.WEB_SERVLET - errored false - tags { - "http.url" "http://localhost:$port/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "span.origin.type" "TestServlet3\$$type" - "span.type" DDSpanTypes.WEB_SERVLET - "http.status_code" 200 - defaultTags(true) - } - } - } - } - - where: - path | distributedTracing - "sync" | true - "sync" | false - "async" | true - "async" | false - - type = path.capitalize() + @Override + void addServlet(ServletContextHandler servletContext, String url, Class servlet) { + servletContext.addServlet(servlet, url) } - def "servlet instrumentation clears state after async request"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/$path") - .get() - .build() - def numTraces = 10 - for (int i = 0; i < numTraces; ++i) { - client.newCall(request).execute() - } - - expect: - assertTraces(dispatched ? numTraces * 2 : numTraces) { - for (int i = 0; (dispatched ? i + 1 : i) < TEST_WRITER.size(); i += (dispatched ? 2 : 1)) { - if (dispatched) { - trace(i, 1) { - span(0) { - operationName "servlet.request" - resourceName "GET /dispatch/async" - parent() - } - } - } - trace(dispatched ? i + 1 : i, 1) { - span(0) { - operationName "servlet.request" - resourceName "GET /async" - if (dispatched) { - childOf TEST_WRITER[i][0] - } else { - parent() - } - } - } - } - } - - where: - path | dispatched - "async" | false - "dispatch/async" | true - } - - def "test #path error servlet call"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/$path?error=true") - .get() - .build() - def response = client.newCall(request).execute() - - expect: - response.body().string().trim() != expectedResponse - - assertTraces(1) { - trace(0, 1) { - span(0) { - serviceName "unnamed-java-app" - operationName "servlet.request" - resourceName "GET /$path" - spanType DDSpanTypes.WEB_SERVLET - errored true - parent() - tags { - "http.url" "http://localhost:$port/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "span.type" DDSpanTypes.WEB_SERVLET - "span.origin.type" "TestServlet3\$Sync" - "http.status_code" 500 - errorTags(RuntimeException, "some $path error") - defaultTags() - } - } - } - } - - where: - path | expectedResponse - //"async" | "Hello Async" // FIXME: I can't seem get the async error handler to trigger - "sync" | "Hello Sync" - } - - def "test #path non-throwing-error servlet call"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/$path?non-throwing-error=true") - .get() - .build() - def response = client.newCall(request).execute() - - expect: - response.body().string().trim() != expectedResponse - - assertTraces(1) { - trace(0, 1) { - span(0) { - serviceName "unnamed-java-app" - operationName "servlet.request" - resourceName "GET /$path" - spanType DDSpanTypes.WEB_SERVLET - errored true - parent() - tags { - "http.url" "http://localhost:$port/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "span.type" DDSpanTypes.WEB_SERVLET - "span.origin.type" "TestServlet3\$Sync" - "http.status_code" 500 - "error" true - defaultTags() - } - } - } - } - - where: - path | expectedResponse - "sync" | "Hello Sync" - } - - /** - * Setup simple authentication for tests - *

- * requests to {@code /auth/*} need login 'user' and password 'password' - *

- * For details @see http://www.eclipse.org/jetty/documentation/9.3.x/embedded-examples.html - * - * @param jettyServer server to attach login service - * @return SecurityHandler that can be assigned to servlet - */ - private ConstraintSecurityHandler setupAuthentication(Server jettyServer) { - ConstraintSecurityHandler security = new ConstraintSecurityHandler() + static setupAuthentication(Server jettyServer, ServletContextHandler servletContext) { + ConstraintSecurityHandler authConfig = new ConstraintSecurityHandler() Constraint constraint = new Constraint() constraint.setName("auth") @@ -348,14 +58,14 @@ class JettyServlet3Test extends AgentTestRunner { mapping.setPathSpec("/auth/*") mapping.setConstraint(constraint) - security.setConstraintMappings(mapping) - security.setAuthenticator(new BasicAuthenticator()) + authConfig.setConstraintMappings(mapping) + authConfig.setAuthenticator(new BasicAuthenticator()) LoginService loginService = new HashLoginService("TestRealm", "src/test/resources/realm.properties") - security.setLoginService(loginService) + authConfig.setLoginService(loginService) jettyServer.addBean(loginService) - security + servletContext.setSecurityHandler(authConfig) } } diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy index d23defacd1..5371a6395d 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy @@ -1,68 +1,47 @@ import com.google.common.io.Files -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.agent.test.TestUtils -import datadog.trace.agent.test.utils.OkHttpUtils -import datadog.trace.api.DDSpanTypes -import okhttp3.OkHttpClient -import okhttp3.Request import org.apache.catalina.Context -import org.apache.catalina.core.ApplicationFilterChain +import org.apache.catalina.realm.MemoryRealm import org.apache.catalina.startup.Tomcat import org.apache.tomcat.JarScanFilter import org.apache.tomcat.JarScanType +import org.apache.tomcat.util.descriptor.web.LoginConfig +import org.apache.tomcat.util.descriptor.web.SecurityCollection +import org.apache.tomcat.util.descriptor.web.SecurityConstraint import spock.lang.Shared -class TomcatServlet3Test extends AgentTestRunner { +import javax.servlet.Servlet - OkHttpClient client = OkHttpUtils.client() +class TomcatServlet3Test extends AbstractServlet3Test { - @Shared - int port @Shared Tomcat tomcatServer @Shared - Context appContext + def baseDir = Files.createTempDir() def setupSpec() { - port = TestUtils.randomOpenPort() tomcatServer = new Tomcat() tomcatServer.setPort(port) tomcatServer.getConnector() - def baseDir = Files.createTempDir() baseDir.deleteOnExit() tomcatServer.setBaseDir(baseDir.getAbsolutePath()) final File applicationDir = new File(baseDir, "/webapps/ROOT") if (!applicationDir.exists()) { applicationDir.mkdirs() + applicationDir.deleteOnExit() } - appContext = tomcatServer.addWebapp("/my-context", applicationDir.getAbsolutePath()) + Context servletContext = tomcatServer.addWebapp("/$context", applicationDir.getAbsolutePath()) // Speed up startup by disabling jar scanning: - appContext.getJarScanner().setJarScanFilter(new JarScanFilter() { + servletContext.getJarScanner().setJarScanFilter(new JarScanFilter() { @Override boolean check(JarScanType jarScanType, String jarName) { return false } }) - Tomcat.addServlet(appContext, "syncServlet", new TestServlet3.Sync()) - appContext.addServletMappingDecoded("/sync", "syncServlet") - - Tomcat.addServlet(appContext, "asyncServlet", new TestServlet3.Async()) - appContext.addServletMappingDecoded("/async", "asyncServlet") - - Tomcat.addServlet(appContext, "blockingServlet", new TestServlet3.BlockingAsync()) - appContext.addServletMappingDecoded("/blocking", "blockingServlet") - - Tomcat.addServlet(appContext, "dispatchServlet", new TestServlet3.DispatchSync()) - appContext.addServletMappingDecoded("/dispatch/sync", "dispatchServlet") - - Tomcat.addServlet(appContext, "dispatchAsyncServlet", new TestServlet3.DispatchAsync()) - appContext.addServletMappingDecoded("/dispatch/async", "dispatchAsyncServlet") - - Tomcat.addServlet(appContext, "fakeServlet", new TestServlet3.FakeAsync()) - appContext.addServletMappingDecoded("/fake", "fakeServlet") + setupAuthentication(tomcatServer, servletContext) + setupServlets(servletContext) tomcatServer.start() System.out.println( @@ -74,259 +53,44 @@ class TomcatServlet3Test extends AgentTestRunner { tomcatServer.destroy() } - def "test #path servlet call (auth: #auth, distributed tracing: #distributedTracing)"() { - setup: - def requestBuilder = new Request.Builder() - .url("http://localhost:$port/my-context/$path") - .get() - if (distributedTracing) { - requestBuilder.header("x-datadog-trace-id", "123") - requestBuilder.header("x-datadog-parent-id", "456") - } - def response = client.newCall(requestBuilder.build()).execute() - - expect: - response.body().string().trim() == expectedResponse - - assertTraces(1) { - trace(0, 1) { - span(0) { - if (distributedTracing) { - traceId "123" - parentId "456" - } else { - parent() - } - serviceName "my-context" - operationName "servlet.request" - resourceName "GET /my-context/$path" - spanType DDSpanTypes.WEB_SERVLET - errored false - tags { - "http.url" "http://localhost:$port/my-context/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "span.type" DDSpanTypes.WEB_SERVLET - "span.origin.type" ApplicationFilterChain.name - "servlet.context" "/my-context" - "http.status_code" 200 - defaultTags(distributedTracing) - } - } - } - } - - where: - path | expectedResponse | origin | distributedTracing - "async" | "Hello Async" | "Async" | false - "sync" | "Hello Sync" | "Sync" | false - "blocking" | "Hello BlockingAsync" | "BlockingAsync" | false - "fake" | "Hello FakeAsync" | "FakeAsync" | false - "async" | "Hello Async" | "Async" | true - "sync" | "Hello Sync" | "Sync" | true - "blocking" | "Hello BlockingAsync" | "BlockingAsync" | true - "fake" | "Hello FakeAsync" | "FakeAsync" | true + @Override + String getContext() { + return "tomcat-context" } - def "test dispatch #path"() { - setup: - def requestBuilder = new Request.Builder() - .url("http://localhost:$port/my-context/dispatch/$path") - .get() - if (distributedTracing) { - requestBuilder.header("x-datadog-trace-id", "123") - requestBuilder.header("x-datadog-parent-id", "456") - } - def response = client.newCall(requestBuilder.build()).execute() - - expect: - response.body().string().trim() == "Hello $type" - - assertTraces(2) { - trace(0, 1) { - span(0) { - if (distributedTracing) { - traceId "123" - parentId "456" - } else { - parent() - } - serviceName "my-context" - operationName "servlet.request" - resourceName "GET /my-context/dispatch/$path" - spanType DDSpanTypes.WEB_SERVLET - errored false - tags { - "http.url" "http://localhost:$port/my-context/dispatch/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" - "span.type" DDSpanTypes.WEB_SERVLET - "http.status_code" 200 - "servlet.context" "/my-context" - "servlet.dispatch" "/$path" - defaultTags(distributedTracing) - } - } - } - trace(1, 1) { - span(0) { - serviceName "my-context" - operationName "servlet.request" - resourceName "GET /my-context/$path" - spanType DDSpanTypes.WEB_SERVLET - errored false - childOf TEST_WRITER[0][0] - tags { - "http.url" "http://localhost:$port/my-context/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" - "span.type" DDSpanTypes.WEB_SERVLET - "http.status_code" 200 - "servlet.context" "/my-context" - defaultTags(true) - } - } - } - } - - where: - path | distributedTracing - "sync" | true - "sync" | false - "async" | true - "async" | false - - type = path.capitalize() + @Override + void addServlet(Context servletContext, String url, Class servlet) { + String name = UUID.randomUUID() + Tomcat.addServlet(servletContext, name, servlet.newInstance()) + servletContext.addServletMappingDecoded(url, name) } - def "servlet instrumentation clears state after async request"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/my-context/$path") - .get() - .build() - def numTraces = 10 - for (int i = 0; i < numTraces; ++i) { - client.newCall(request).execute() - } + static setupAuthentication(Tomcat server, Context servletContext) { + // Login Config + LoginConfig authConfig = new LoginConfig(); + authConfig.setAuthMethod("BASIC"); - expect: - assertTraces(dispatched ? numTraces * 2 : numTraces) { - for (int i = 0; (dispatched ? i + 1 : i) < TEST_WRITER.size(); i += (dispatched ? 2 : 1)) { - if (dispatched) { - trace(i, 1) { - span(0) { - operationName "servlet.request" - resourceName "GET /my-context/dispatch/async" - parent() - } - } - } - trace(dispatched ? i + 1 : i, 1) { - span(0) { - operationName "servlet.request" - resourceName "GET /my-context/async" - if (dispatched) { - childOf TEST_WRITER[i][0] - } else { - parent() - } - } - } - } - } + // adding constraint with role "test" + SecurityConstraint constraint = new SecurityConstraint(); + constraint.addAuthRole("role"); - where: - path | dispatched - "async" | false - "dispatch/async" | true - } + // add constraint to a collection with pattern /second + SecurityCollection collection = new SecurityCollection(); + collection.addPattern("/auth/*"); + constraint.addCollection(collection); - def "test #path error servlet call"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/my-context/$path?error=true") - .get() - .build() - def response = client.newCall(request).execute() + servletContext.setLoginConfig(authConfig); + // does the context need a auth role too? + servletContext.addSecurityRole("role"); + servletContext.addConstraint(constraint); - expect: - response.body().string().trim() != expectedResponse + // add tomcat users to realm + URL uri = getClass().getResource("/tomcat-users.xml"); + assert uri + MemoryRealm realm = new MemoryRealm(); + realm.setPathname(uri.toString()); + server.getEngine().setRealm(realm); - assertTraces(1) { - trace(0, 1) { - span(0) { - serviceName "my-context" - operationName "servlet.request" - resourceName "GET /my-context/$path" - spanType DDSpanTypes.WEB_SERVLET - errored true - parent() - tags { - "http.url" "http://localhost:$port/my-context/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "span.type" DDSpanTypes.WEB_SERVLET - "span.origin.type" ApplicationFilterChain.name - "servlet.context" "/my-context" - "http.status_code" 500 - errorTags(RuntimeException, "some $path error") - defaultTags() - } - } - } - } - - where: - path | expectedResponse - //"async" | "Hello Async" // FIXME: I can't seem get the async error handler to trigger - "sync" | "Hello Sync" - } - - def "test #path non-throwing-error servlet call"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/my-context/$path?non-throwing-error=true") - .get() - .build() - def response = client.newCall(request).execute() - - expect: - response.body().string().trim() != expectedResponse - - assertTraces(1) { - trace(0, 1) { - span(0) { - serviceName "my-context" - operationName "servlet.request" - resourceName "GET /my-context/$path" - spanType DDSpanTypes.WEB_SERVLET - errored true - parent() - tags { - "http.url" "http://localhost:$port/my-context/$path" - "http.method" "GET" - "span.kind" "server" - "component" "java-web-servlet" - "span.type" DDSpanTypes.WEB_SERVLET - "span.origin.type" ApplicationFilterChain.name - "servlet.context" "/my-context" - "http.status_code" 500 - "error" true - defaultTags() - } - } - } - } - - where: - path | expectedResponse - "sync" | "Hello Sync" + servletContext.setLoginConfig(authConfig) } } diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/resources/tomcat-users.xml b/dd-java-agent/instrumentation/servlet-3/src/test/resources/tomcat-users.xml new file mode 100644 index 0000000000..c8abe128e8 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet-3/src/test/resources/tomcat-users.xml @@ -0,0 +1,4 @@ + + + + From e96752bbade164f8646c992edd9838f2fa59da58 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 16 Nov 2018 09:56:42 -0800 Subject: [PATCH 3/4] Remove need for tomcat user file config --- .../test/groovy/AbstractServlet3Test.groovy | 4 +- .../src/test/groovy/TomcatServlet3Test.groovy | 37 +++++++++++-------- .../src/test/resources/tomcat-users.xml | 4 -- 3 files changed, 23 insertions(+), 22 deletions(-) delete mode 100644 dd-java-agent/instrumentation/servlet-3/src/test/resources/tomcat-users.xml 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 870f197160..cc34696d13 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 @@ -32,9 +32,9 @@ abstract class AbstractServlet3Test extends AgentTestRunner { @Shared int port = TestUtils.randomOpenPort() @Shared - String user = "user" + protected String user = "user" @Shared - String pass = "password" + protected String pass = "password" abstract String getContext() diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy index 5371a6395d..5ba55cb224 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy @@ -1,6 +1,8 @@ import com.google.common.io.Files import org.apache.catalina.Context +import org.apache.catalina.LifecycleState import org.apache.catalina.realm.MemoryRealm +import org.apache.catalina.realm.MessageDigestCredentialHandler import org.apache.catalina.startup.Tomcat import org.apache.tomcat.JarScanFilter import org.apache.tomcat.JarScanType @@ -65,31 +67,34 @@ class TomcatServlet3Test extends AbstractServlet3Test { servletContext.addServletMappingDecoded(url, name) } - static setupAuthentication(Tomcat server, Context servletContext) { + private setupAuthentication(Tomcat server, Context servletContext) { // Login Config - LoginConfig authConfig = new LoginConfig(); - authConfig.setAuthMethod("BASIC"); + LoginConfig authConfig = new LoginConfig() + authConfig.setAuthMethod("BASIC") // adding constraint with role "test" - SecurityConstraint constraint = new SecurityConstraint(); - constraint.addAuthRole("role"); + SecurityConstraint constraint = new SecurityConstraint() + constraint.addAuthRole("role") // add constraint to a collection with pattern /second - SecurityCollection collection = new SecurityCollection(); - collection.addPattern("/auth/*"); - constraint.addCollection(collection); + SecurityCollection collection = new SecurityCollection() + collection.addPattern("/auth/*") + constraint.addCollection(collection) - servletContext.setLoginConfig(authConfig); + servletContext.setLoginConfig(authConfig) // does the context need a auth role too? - servletContext.addSecurityRole("role"); - servletContext.addConstraint(constraint); + servletContext.addSecurityRole("role") + servletContext.addConstraint(constraint) // add tomcat users to realm - URL uri = getClass().getResource("/tomcat-users.xml"); - assert uri - MemoryRealm realm = new MemoryRealm(); - realm.setPathname(uri.toString()); - server.getEngine().setRealm(realm); + MemoryRealm realm = new MemoryRealm() { + protected void startInternal() { + credentialHandler = new MessageDigestCredentialHandler() + setState(LifecycleState.STARTING) + } + } + realm.addUser(user, pass, "role") + server.getEngine().setRealm(realm) servletContext.setLoginConfig(authConfig) } diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/resources/tomcat-users.xml b/dd-java-agent/instrumentation/servlet-3/src/test/resources/tomcat-users.xml deleted file mode 100644 index c8abe128e8..0000000000 --- a/dd-java-agent/instrumentation/servlet-3/src/test/resources/tomcat-users.xml +++ /dev/null @@ -1,4 +0,0 @@ - - - - From e4a12409c805f80890fb9e82eed1b7669e32321d Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 16 Nov 2018 14:40:35 -0800 Subject: [PATCH 4/4] Add recursive servlet dispatch tests. --- .../test/groovy/AbstractServlet3Test.groovy | 81 +++++++++++++++---- .../src/test/groovy/TestServlet3.groovy | 18 +++++ 2 files changed, 82 insertions(+), 17 deletions(-) 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 cc34696d13..b9ba46e14f 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 @@ -48,6 +48,8 @@ abstract class AbstractServlet3Test extends AgentTestRunner { addServlet(context, "/blocking", TestServlet3.BlockingAsync) addServlet(context, "/dispatch/sync", TestServlet3.DispatchSync) addServlet(context, "/dispatch/async", TestServlet3.DispatchAsync) + addServlet(context, "/dispatch/recursive", TestServlet3.DispatchRecursive) + addServlet(context, "/recursive", TestServlet3.DispatchRecursive) addServlet(context, "/fake", TestServlet3.FakeAsync) } @@ -116,10 +118,10 @@ abstract class AbstractServlet3Test extends AgentTestRunner { "fake" | "Hello FakeAsync" | false | "FakeAsync" | true } - def "test dispatch #path"() { + def "test dispatch #path with depth #depth"() { setup: def requestBuilder = new Request.Builder() - .url("http://localhost:$port/$context/dispatch/$path") + .url("http://localhost:$port/$context/dispatch/$path?depth=$depth") .get() if (distributedTracing) { requestBuilder.header("x-datadog-trace-id", "123") @@ -130,14 +132,51 @@ abstract class AbstractServlet3Test extends AgentTestRunner { expect: response.body().string().trim() == "Hello $origin" - assertTraces(2) { - trace(0, 1) { + assertTraces(2 + depth) { + for (int i = 0; i < depth; i++) { + trace(i, 1) { + span(0) { + if (i == 0) { + if (distributedTracing) { + traceId "123" + parentId "456" + } else { + parent() + } + } else { + childOf TEST_WRITER[i - 1][0] + } + serviceName context + operationName "servlet.request" + resourceName "GET /$context/dispatch/$path" + spanType DDSpanTypes.WEB_SERVLET + errored false + tags { + "http.url" "http://localhost:$port/$context/dispatch/$path" + "http.method" "GET" + "span.kind" "server" + "component" "java-web-servlet" + "span.origin.type" { it == "TestServlet3\$Dispatch$origin" || it == ApplicationFilterChain.name } + "span.type" DDSpanTypes.WEB_SERVLET + "http.status_code" 200 + "servlet.context" "/$context" + "servlet.dispatch" "/dispatch/recursive?depth=${depth - i - 1}" + defaultTags(i > 0 ? true : distributedTracing) + } + } + } + } + trace(depth, 1) { span(0) { - if (distributedTracing) { - traceId "123" - parentId "456" + if (depth > 0) { + childOf TEST_WRITER[depth - 1][0] } else { - parent() + if (distributedTracing) { + traceId "123" + parentId "456" + } else { + parent() + } } serviceName context operationName "servlet.request" @@ -154,24 +193,26 @@ abstract class AbstractServlet3Test extends AgentTestRunner { "http.status_code" 200 "servlet.context" "/$context" "servlet.dispatch" "/$path" - defaultTags(distributedTracing) + defaultTags(depth > 0 ? true : distributedTracing) } } } - trace(1, 1) { + trace(depth + 1, 1) { span(0) { serviceName context operationName "servlet.request" resourceName "GET /$context/$path" spanType DDSpanTypes.WEB_SERVLET errored false - childOf TEST_WRITER[0][0] + childOf TEST_WRITER[depth][0] tags { "http.url" "http://localhost:$port/$context/$path" "http.method" "GET" "span.kind" "server" "component" "java-web-servlet" - "span.origin.type" { it == "TestServlet3\$$origin" || it == ApplicationFilterChain.name } + "span.origin.type" { + it == "TestServlet3\$$origin" || it == "TestServlet3\$DispatchRecursive" || it == ApplicationFilterChain.name + } "span.type" DDSpanTypes.WEB_SERVLET "http.status_code" 200 "servlet.context" "/$context" @@ -182,11 +223,17 @@ abstract class AbstractServlet3Test extends AgentTestRunner { } where: - path | distributedTracing - "sync" | true - "sync" | false - "async" | true - "async" | false + path | distributedTracing | depth + "sync" | true | 0 + "sync" | false | 0 + "async" | true | 0 + "async" | false | 0 + "recursive" | true | 0 + "recursive" | false | 0 + "recursive" | true | 1 + "recursive" | false | 1 + "recursive" | true | 20 + "recursive" | false | 20 origin = path.capitalize() } diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet3.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet3.groovy index 673e36479e..d79060eb5f 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet3.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TestServlet3.groovy @@ -71,6 +71,23 @@ class TestServlet3 { } } + @WebServlet(asyncSupported = true) + static class DispatchRecursive extends AbstractHttpServlet { + @Override + void doGet(HttpServletRequest req, HttpServletResponse resp) { + if (req.servletPath.equals("/recursive")) { + resp.writer.print("Hello Recursive") + return + } + def depth = Integer.parseInt(req.getParameter("depth")) + if (depth > 0) { + req.startAsync().dispatch("/dispatch/recursive?depth=" + (depth - 1)) + } else { + req.startAsync().dispatch("/recursive") + } + } + } + @WebServlet(asyncSupported = true) static class FakeAsync extends AbstractHttpServlet { @Override @@ -80,4 +97,5 @@ class TestServlet3 { context.complete() } } + }