From 64161d93c9c9ea1ecd95ee77ef72baccd6bd996f Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 9 Apr 2020 14:26:33 -0400 Subject: [PATCH 1/2] Add instrumentation to detect the route at the beginning of the spring request Instead of waiting till the handler is called, otherwise if a response is returned by a filter then the proper name wouldn't be set and would fall back to the URL. --- .../servlet3/Servlet3Decorator.java | 25 ++- .../DispatcherServletInstrumentation.java | 64 ++++++- .../HandlerMappingResourceNameFilter.java | 65 +++++++ .../groovy/test/{ => boot}/AppConfig.groovy | 2 +- .../{ => boot}/SpringBootBasedTest.groovy | 9 +- .../test/{ => boot}/TestController.groovy | 12 +- .../test/filter/FilteredAppConfig.groovy | 112 ++++++++++++ .../test/filter/ServletFilterTest.groovy | 160 ++++++++++++++++++ .../groovy/test/filter/TestController.groovy | 74 ++++++++ .../agent/test/base/HttpServerTest.groovy | 45 ++++- 10 files changed, 553 insertions(+), 15 deletions(-) create mode 100644 dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HandlerMappingResourceNameFilter.java rename dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/{ => boot}/AppConfig.groovy (98%) rename dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/{ => boot}/SpringBootBasedTest.groovy (96%) rename dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/{ => boot}/TestController.groovy (85%) create mode 100644 dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/FilteredAppConfig.groovy create mode 100644 dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy create mode 100644 dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/TestController.groovy diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java index 955bdbe3ff2..0c9822d6c30 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java @@ -2,8 +2,11 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator; +import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import javax.servlet.Filter; +import javax.servlet.ServletContext; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -58,12 +61,32 @@ protected Integer status(final HttpServletResponse httpServletResponse) { public AgentSpan onRequest(final AgentSpan span, final HttpServletRequest request) { assert span != null; if (request != null) { - span.setTag("servlet.context", request.getContextPath()); span.setTag("servlet.path", request.getServletPath()); + span.setTag("servlet.context", request.getContextPath()); + onContext(span, request, request.getServletContext()); } return super.onRequest(span, request); } + /** + * This method executes the filter created by + * datadog.trace.instrumentation.springweb.DispatcherServletInstrumentation$HandlerMappingAdvice. + * This was easier and less "hacky" than other ways to add the filter to the front of the filter + * chain. + */ + private void onContext( + final AgentSpan span, final HttpServletRequest request, final ServletContext context) { + final Object attribute = context.getAttribute("dd.dispatcher-filter"); + if (attribute instanceof Filter) { + final Filter filter = (Filter) attribute; + try { + request.setAttribute(DD_SPAN_ATTRIBUTE, span); + filter.doFilter(request, null, null); + } catch (final IOException | ServletException e) { + } + } + } + @Override public AgentSpan onError(final AgentSpan span, final Throwable throwable) { if (throwable instanceof ServletException && throwable.getCause() != null) { diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java index 5450bb64106..be2bf9dd5f8 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java @@ -5,23 +5,31 @@ import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; import static datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator.DECORATE; import static datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator.DECORATE_RENDER; +import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isProtected; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.ContextStore; +import datadog.trace.bootstrap.InstrumentationContext; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import java.util.HashMap; +import java.util.List; import java.util.Map; +import javax.servlet.ServletContext; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; -import org.springframework.web.method.HandlerMethod; +import org.springframework.context.ApplicationContext; +import org.springframework.web.servlet.DispatcherServlet; +import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.ModelAndView; @AutoService(Instrumenter.class) @@ -36,23 +44,38 @@ public ElementMatcher typeMatcher() { return named("org.springframework.web.servlet.DispatcherServlet"); } + @Override + public Map contextStore() { + return singletonMap( + "org.springframework.web.servlet.DispatcherServlet", + packageName + ".HandlerMappingResourceNameFilter"); + } + @Override public String[] helperClassNames() { return new String[] { packageName + ".SpringWebHttpServerDecorator", packageName + ".SpringWebHttpServerDecorator$1", + packageName + ".HandlerMappingResourceNameFilter", }; } @Override public Map, String> transformers() { final Map, String> transformers = new HashMap<>(); + transformers.put( + isMethod() + .and(isProtected()) + .and(named("onRefresh")) + .and(takesArgument(0, named("org.springframework.context.ApplicationContext"))) + .and(takesArguments(1)), + DispatcherServletInstrumentation.class.getName() + "$HandlerMappingAdvice"); transformers.put( isMethod() .and(isProtected()) .and(named("render")) .and(takesArgument(0, named("org.springframework.web.servlet.ModelAndView"))), - DispatcherServletInstrumentation.class.getName() + "$DispatcherAdvice"); + DispatcherServletInstrumentation.class.getName() + "$RenderAdvice"); transformers.put( isMethod() .and(isProtected()) @@ -62,7 +85,37 @@ public Map, String> transfor return transformers; } - public static class DispatcherAdvice { + /** + * This advice creates a filter that has reference to the handlerMappings from DispatcherServlet + * which allows the mappings to be evaluated at the beginning of the filter chain. This evaluation + * is done inside the Servlet3Decorator.onContext method. + */ + public static class HandlerMappingAdvice { + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void afterRefresh( + @Advice.This final DispatcherServlet dispatcher, + @Advice.Argument(0) final ApplicationContext springCtx, + @Advice.FieldValue("handlerMappings") final List handlerMappings, + @Advice.Thrown final Throwable throwable) { + final ServletContext servletContext = springCtx.getBean(ServletContext.class); + if (handlerMappings != null && servletContext != null) { + final ContextStore contextStore = + InstrumentationContext.get( + DispatcherServlet.class, HandlerMappingResourceNameFilter.class); + HandlerMappingResourceNameFilter filter = contextStore.get(dispatcher); + if (filter == null) { + filter = new HandlerMappingResourceNameFilter(); + contextStore.put(dispatcher, filter); + } + filter.setHandlerMappings(handlerMappings); + servletContext.setAttribute( + "dd.dispatcher-filter", filter); // used by Servlet3Decorator.onContext + } + } + } + + public static class RenderAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static AgentScope onEnter(@Advice.Argument(0) final ModelAndView mv) { @@ -79,11 +132,6 @@ public static void stopSpan( DECORATE_RENDER.beforeFinish(scope); scope.close(); } - - // Make this advice match consistently with HandlerAdapterInstrumentation - private void muzzleCheck(final HandlerMethod method) { - method.getMethod(); - } } public static class ErrorHandlerAdvice { diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HandlerMappingResourceNameFilter.java b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HandlerMappingResourceNameFilter.java new file mode 100644 index 00000000000..268a02ed41b --- /dev/null +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HandlerMappingResourceNameFilter.java @@ -0,0 +1,65 @@ +package datadog.trace.instrumentation.springweb; + +import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE; +import static datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator.DECORATE; + +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import java.util.List; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import org.springframework.web.servlet.HandlerExecutionChain; +import org.springframework.web.servlet.HandlerMapping; + +public class HandlerMappingResourceNameFilter implements Filter { + private volatile List handlerMappings; + + @Override + public void init(final FilterConfig filterConfig) {} + + @Override + public void doFilter( + final ServletRequest servletRequest, + final ServletResponse servletResponse, + final FilterChain filterChain) { + if (servletRequest instanceof HttpServletRequest && handlerMappings != null) { + final HttpServletRequest request = (HttpServletRequest) servletRequest; + try { + if (findMapping(request)) { + // Name the parent span based on the matching pattern + final Object parentSpan = request.getAttribute(DD_SPAN_ATTRIBUTE); + if (parentSpan instanceof AgentSpan) { + // Let the parent span resource name be set with the attribute set in findMapping. + DECORATE.onRequest((AgentSpan) parentSpan, request); + } + } + } catch (final Exception e) { + } + } + } + + @Override + public void destroy() {} + + /** + * When a HandlerMapping matches a request, it sets HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE + * as an attribute on the request. This attribute is read by + * SpringWebHttpServerDecorator.onRequest and set as the resource name. + */ + private boolean findMapping(final HttpServletRequest request) throws Exception { + for (final HandlerMapping mapping : handlerMappings) { + final HandlerExecutionChain handler = mapping.getHandler(request); + if (handler != null) { + return true; + } + } + return false; + } + + public void setHandlerMappings(final List handlerMappings) { + this.handlerMappings = handlerMappings; + } +} diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/AppConfig.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/AppConfig.groovy similarity index 98% rename from dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/AppConfig.groovy rename to dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/AppConfig.groovy index 1d7a41b5a83..fce7d0d2ab9 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/AppConfig.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/AppConfig.groovy @@ -1,4 +1,4 @@ -package test +package test.boot import org.apache.catalina.connector.Connector import org.springframework.boot.autoconfigure.SpringBootApplication diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy similarity index 96% rename from dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/SpringBootBasedTest.groovy rename to dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy index 860f6438453..c58ca3078c0 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy @@ -1,4 +1,4 @@ -package test +package test.boot import datadog.opentracing.DDSpan import datadog.trace.agent.test.asserts.ListWriterAssert @@ -51,6 +51,11 @@ class SpringBootBasedTest extends HttpServerTest true } + @Override + String testPathParam() { + "/path/{id}/param" + } + @Override boolean testNotFound() { // FIXME: the instrumentation adds an extra controller span which is not consistent. @@ -116,7 +121,7 @@ class SpringBootBasedTest extends HttpServerTest trace.span(index) { serviceName expectedServiceName() operationName expectedOperationName() - resourceName endpoint.status == 404 ? "404" : "$method ${endpoint.resolve(address).path}" + resourceName endpoint.resource(method, address, testPathParam()) spanType DDSpanTypes.HTTP_SERVER errored endpoint.errored if (parentID != null) { diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/TestController.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/TestController.groovy similarity index 85% rename from dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/TestController.groovy rename to dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/TestController.groovy index 3380af95223..b7685749950 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/TestController.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/TestController.groovy @@ -1,10 +1,11 @@ -package test +package test.boot import datadog.trace.agent.test.base.HttpServerTest import org.springframework.http.HttpStatus import org.springframework.http.ResponseEntity import org.springframework.stereotype.Controller import org.springframework.web.bind.annotation.ExceptionHandler +import org.springframework.web.bind.annotation.PathVariable import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RequestParam import org.springframework.web.bind.annotation.ResponseBody @@ -12,6 +13,7 @@ import org.springframework.web.servlet.view.RedirectView import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -35,6 +37,14 @@ class TestController { } } + @RequestMapping("/path/{id}/param") + @ResponseBody + String path_param(@PathVariable Integer id) { + HttpServerTest.controller(PATH_PARAM) { + "$id" + } + } + @RequestMapping("/redirect") @ResponseBody RedirectView redirect() { diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/FilteredAppConfig.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/FilteredAppConfig.groovy new file mode 100644 index 00000000000..46367b50aea --- /dev/null +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/FilteredAppConfig.groovy @@ -0,0 +1,112 @@ +package test.filter + +import datadog.trace.agent.test.base.HttpServerTest +import org.apache.catalina.connector.Connector +import org.springframework.boot.autoconfigure.SpringBootApplication +import org.springframework.boot.context.embedded.EmbeddedServletContainerFactory +import org.springframework.boot.context.embedded.tomcat.TomcatConnectorCustomizer +import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory +import org.springframework.context.annotation.Bean +import org.springframework.http.MediaType +import org.springframework.web.HttpMediaTypeNotAcceptableException +import org.springframework.web.accept.ContentNegotiationStrategy +import org.springframework.web.context.request.NativeWebRequest +import org.springframework.web.servlet.config.annotation.ContentNegotiationConfigurer +import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter + +import javax.servlet.Filter +import javax.servlet.FilterChain +import javax.servlet.FilterConfig +import javax.servlet.ServletException +import javax.servlet.ServletRequest +import javax.servlet.ServletResponse +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +@SpringBootApplication +class FilteredAppConfig extends WebMvcConfigurerAdapter { + + @Override + void configureContentNegotiation(ContentNegotiationConfigurer configurer) { + configurer.favorPathExtension(false) + .favorParameter(true) + .ignoreAcceptHeader(true) + .useJaf(false) + .defaultContentTypeStrategy(new ContentNegotiationStrategy() { + @Override + List resolveMediaTypes(NativeWebRequest webRequest) throws HttpMediaTypeNotAcceptableException { + return [MediaType.TEXT_PLAIN, MediaType.APPLICATION_JSON] + } + }) + } + + @Bean + EmbeddedServletContainerFactory servletContainerFactory() { + def factory = new TomcatEmbeddedServletContainerFactory() + + factory.addConnectorCustomizers( + new TomcatConnectorCustomizer() { + @Override + void customize(final Connector connector) { + connector.setEnableLookups(true) + } + }) + + return factory + } + + @Bean + Filter servletFilter() { + return new Filter() { + + @Override + void init(FilterConfig filterConfig) throws ServletException { + } + + @Override + void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { + HttpServletRequest req = (HttpServletRequest) request + HttpServletResponse resp = (HttpServletResponse) response + HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath) + HttpServerTest.controller(endpoint) { + resp.contentType = "text/plain" + switch (endpoint) { + case SUCCESS: + resp.status = endpoint.status + resp.writer.print(endpoint.body) + break + case QUERY_PARAM: + resp.status = endpoint.status + resp.writer.print(req.queryString) + break + case PATH_PARAM: + resp.status = endpoint.status + resp.writer.print(endpoint.body) + break + case REDIRECT: + resp.sendRedirect(endpoint.body) + break + case ERROR: + resp.sendError(endpoint.status, endpoint.body) + break + case EXCEPTION: + throw new Exception(endpoint.body) + default: + chain.doFilter(request, response) + } + } + } + + @Override + void destroy() { + } + } + } +} diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy new file mode 100644 index 00000000000..13eb4589181 --- /dev/null +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy @@ -0,0 +1,160 @@ +package test.filter + +import datadog.opentracing.DDSpan +import datadog.trace.agent.test.asserts.ListWriterAssert +import datadog.trace.agent.test.asserts.SpanAssert +import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.api.DDSpanTypes +import datadog.trace.api.DDTags +import datadog.trace.bootstrap.instrumentation.api.Tags +import datadog.trace.instrumentation.servlet3.Servlet3Decorator +import datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator +import groovy.transform.stc.ClosureParams +import groovy.transform.stc.SimpleType +import org.apache.catalina.core.ApplicationFilterChain +import org.springframework.boot.SpringApplication +import org.springframework.context.ConfigurableApplicationContext +import org.springframework.web.servlet.view.RedirectView + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS +import static java.util.Collections.singletonMap + +class ServletFilterTest extends HttpServerTest { + + @Override + ConfigurableApplicationContext startServer(int port) { + def app = new SpringApplication(FilteredAppConfig) + app.setDefaultProperties(singletonMap("server.port", port)) + def context = app.run() + return context + } + + @Override + void stopServer(ConfigurableApplicationContext ctx) { + ctx.close() + } + + @Override + String component() { + return Servlet3Decorator.DECORATE.component() + } + + @Override + String expectedOperationName() { + return "servlet.request" + } + + @Override + boolean hasHandlerSpan() { + false + } + + @Override + String testPathParam() { + "/path/{id}/param" + } + + @Override + boolean testExceptionBody() { + false + } + + @Override + boolean testNotFound() { + // FIXME: the instrumentation adds an extra controller span which is not consistent. + // Fix tests or remove extra span. + false + } + + void cleanAndAssertTraces( + final int size, + @ClosureParams(value = SimpleType, options = "datadog.trace.agent.test.asserts.ListWriterAssert") + @DelegatesTo(value = ListWriterAssert, strategy = Closure.DELEGATE_FIRST) + final Closure spec) { + + // If this is failing, make sure HttpServerTestAdvice is applied correctly. + TEST_WRITER.waitForTraces(size * 2) + + TEST_WRITER.each { + def renderSpan = it.find { + it.operationName == "response.render" + } + if (renderSpan) { + SpanAssert.assertSpan(renderSpan) { + operationName "response.render" + resourceName "response.render" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "spring-webmvc" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + "view.type" RedirectView.name + defaultTags() + } + } + it.remove(renderSpan) + } + } + + super.cleanAndAssertTraces(size, spec) + } + + @Override + void handlerSpan(TraceAssert trace, int index, Object parent, ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + serviceName expectedServiceName() + operationName "spring.handler" + resourceName "TestController.${endpoint.name().toLowerCase()}" + spanType DDSpanTypes.HTTP_SERVER + errored endpoint == EXCEPTION + childOf(parent as DDSpan) + tags { + "$Tags.COMPONENT" SpringWebHttpServerDecorator.DECORATE.component() + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + if (endpoint == EXCEPTION) { + errorTags(Exception, EXCEPTION.body) + } + defaultTags() + } + } + } + + @Override + void serverSpan(TraceAssert trace, int index, BigInteger traceID = null, BigInteger parentID = null, String method = "GET", ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + serviceName expectedServiceName() + operationName expectedOperationName() + resourceName endpoint.resource(method, address, testPathParam()) + spanType DDSpanTypes.HTTP_SERVER + errored endpoint.errored + if (parentID != null) { + traceId traceID + parentId parentID + } else { + parent() + } + tags { + "$Tags.COMPONENT" component + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + "$Tags.PEER_HOST_IPV4" { it == null || it == "127.0.0.1" } // Optional + "$Tags.PEER_PORT" Integer + "$Tags.HTTP_URL" "${endpoint.resolve(address)}" + "$Tags.HTTP_METHOD" method + "$Tags.HTTP_STATUS" endpoint.status + "span.origin.type" ApplicationFilterChain.name + "servlet.path" endpoint.path + if (endpoint.errored) { + "error.msg" { it == null || it == EXCEPTION.body } + "error.type" { it == null || it == Exception.name } + "error.stack" { it == null || it instanceof String } + } + if (endpoint.query) { + "$DDTags.HTTP_QUERY" endpoint.query + } + defaultTags(true) + } + } + } +} diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/TestController.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/TestController.groovy new file mode 100644 index 00000000000..3a33cd10e14 --- /dev/null +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/TestController.groovy @@ -0,0 +1,74 @@ +package test.filter + +import datadog.trace.agent.test.base.HttpServerTest +import org.springframework.http.HttpStatus +import org.springframework.http.ResponseEntity +import org.springframework.stereotype.Controller +import org.springframework.web.bind.annotation.ExceptionHandler +import org.springframework.web.bind.annotation.PathVariable +import org.springframework.web.bind.annotation.RequestMapping +import org.springframework.web.bind.annotation.RequestParam +import org.springframework.web.bind.annotation.ResponseBody +import org.springframework.web.servlet.view.RedirectView + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +@Controller +class TestController { + + @RequestMapping("/success") + @ResponseBody + String success() { + HttpServerTest.controller(SUCCESS) { + SUCCESS.body + } + } + + @RequestMapping("/query") + @ResponseBody + String query_param(@RequestParam("some") String param) { + HttpServerTest.controller(QUERY_PARAM) { + "some=$param" + } + } + + @RequestMapping("/path/{id}/param") + @ResponseBody + String path_param(@PathVariable Integer id) { + HttpServerTest.controller(PATH_PARAM) { + "$id" + } + } + + @RequestMapping("/redirect") + @ResponseBody + RedirectView redirect() { + HttpServerTest.controller(REDIRECT) { + new RedirectView(REDIRECT.body) + } + } + + @RequestMapping("/error-status") + ResponseEntity error() { + HttpServerTest.controller(ERROR) { + new ResponseEntity(ERROR.body, HttpStatus.valueOf(ERROR.status)) + } + } + + @RequestMapping("/exception") + ResponseEntity exception() { + HttpServerTest.controller(EXCEPTION) { + throw new Exception(EXCEPTION.body) + } + } + + @ExceptionHandler + ResponseEntity handleException(Throwable throwable) { + new ResponseEntity(throwable.message, HttpStatus.INTERNAL_SERVER_ERROR) + } +} diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index 67d3fcd5b1e..52462ca71e0 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -27,6 +27,7 @@ import static datadog.trace.agent.test.asserts.TraceAssert.assertTrace import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS @@ -92,7 +93,7 @@ abstract class HttpServerTest extends AgentTestRunner { false } - // Return the handler span's name + /** Return the handler span's name */ String reorderHandlerSpan() { null } @@ -113,6 +114,11 @@ abstract class HttpServerTest extends AgentTestRunner { true } + /** Return the expected resource name */ + String testPathParam() { + null + } + enum ServerEndpoint { SUCCESS("success", 200, "success"), REDIRECT("redirect", 302, "/redirected"), @@ -157,6 +163,10 @@ abstract class HttpServerTest extends AgentTestRunner { return address.resolve(path) } + String resource(String method, URI address, String pathParam) { + return status == 404 ? "404" : "$method ${pathParam ? pathParam : resolve(address).path}" + } + private static final Map PATH_MAP = values().collectEntries { [it.path, it] } static ServerEndpoint forPath(String path) { @@ -288,6 +298,37 @@ abstract class HttpServerTest extends AgentTestRunner { endpoint << [SUCCESS, QUERY_PARAM] } + def "test path param"() { + setup: + assumeTrue(testPathParam() != null) + def request = request(PATH_PARAM, method, body).build() + def response = client.newCall(request).execute() + + expect: + response.code() == PATH_PARAM.status + response.body().string() == PATH_PARAM.body + + and: + cleanAndAssertTraces(1) { + if (hasHandlerSpan()) { + trace(0, 3) { + serverSpan(it, 0, null, null, method, PATH_PARAM) + handlerSpan(it, 1, span(0), PATH_PARAM) + controllerSpan(it, 2, span(1)) + } + } else { + trace(0, 2) { + serverSpan(it, 0, null, null, method, PATH_PARAM) + controllerSpan(it, 1, span(0)) + } + } + } + + where: + method = "GET" + body = null + } + def "test success with multiple header attached parent"() { setup: def traceId = 123G @@ -521,7 +562,7 @@ abstract class HttpServerTest extends AgentTestRunner { trace.span(index) { serviceName expectedServiceName() operationName expectedOperationName() - resourceName endpoint.status == 404 ? "404" : "$method ${endpoint.resolve(address).path}" + resourceName endpoint.resource(method, address, testPathParam()) spanType DDSpanTypes.HTTP_SERVER errored endpoint.errored if (parentID != null) { From ce006e14056443ee856e888fe313844e7563d4c1 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 14 Apr 2020 14:31:46 -0400 Subject: [PATCH 2/2] Ensure RequestDispatcher span is part of expected trace --- .../instrumentation/api/AgentSpan.java | 2 ++ .../instrumentation/api/AgentTracer.java | 6 ++++ .../trace/agent/tooling/OpenTracing32.java | 14 +++++++++ .../servlet3/Servlet3Decorator.java | 8 +++-- .../RequestDispatcherInstrumentation.java | 19 ++++++++++-- .../test/groovy/RequestDispatcherTest.groovy | 3 ++ .../spring-webmvc-3.1.gradle | 1 + .../test/groovy/test/boot/AppConfig.groovy | 2 +- .../test/boot/SpringBootBasedTest.groovy | 15 +++++++++ .../test/filter/FilteredAppConfig.groovy | 31 ++++++++++++++++++- .../test/filter/ServletFilterTest.groovy | 15 +++++++++ .../agent/test/base/HttpServerTest.groovy | 4 ++- 12 files changed, 112 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java index c049b31d0ba..3d5b82cd407 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java @@ -19,6 +19,8 @@ public interface AgentSpan { AgentSpan getLocalRootSpan(); + boolean isSameTrace(AgentSpan otherSpan); + Context context(); void finish(); diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java index 0d208577be5..fb992597d5f 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java @@ -182,6 +182,12 @@ public AgentSpan getLocalRootSpan() { return this; } + @Override + public boolean isSameTrace(final AgentSpan otherSpan) { + // Not sure if this is the best idea... + return otherSpan instanceof NoopAgentSpan; + } + @Override public Context context() { return NoopContext.INSTANCE; diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/OpenTracing32.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/OpenTracing32.java index 3464072270b..77fa060595f 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/OpenTracing32.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/OpenTracing32.java @@ -198,6 +198,20 @@ public AgentSpan getLocalRootSpan() { return this; } + @Override + public boolean isSameTrace(final AgentSpan otherAgentSpan) { + if (otherAgentSpan instanceof OT32Span) { + final Span otherSpan = ((OT32Span) otherAgentSpan).span; + if (span instanceof DDSpan && otherSpan instanceof DDSpan) { + // minor optimization to avoid BigInteger.toString() + return ((DDSpan) span).getTraceId().equals(((DDSpan) otherSpan).getTraceId()); + } else { + return span.context().toTraceId().equals(otherSpan.context().toTraceId()); + } + } + return false; + } + @Override public OT32Context context() { final SpanContext context = span.context(); diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java index 0c9822d6c30..d939ae27642 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java @@ -10,7 +10,9 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import lombok.extern.slf4j.Slf4j; +@Slf4j public class Servlet3Decorator extends HttpServerDecorator { public static final Servlet3Decorator DECORATE = new Servlet3Decorator(); @@ -78,11 +80,11 @@ private void onContext( final AgentSpan span, final HttpServletRequest request, final ServletContext context) { final Object attribute = context.getAttribute("dd.dispatcher-filter"); if (attribute instanceof Filter) { - final Filter filter = (Filter) attribute; + request.setAttribute(DD_SPAN_ATTRIBUTE, span); try { - request.setAttribute(DD_SPAN_ATTRIBUTE, span); - filter.doFilter(request, null, null); + ((Filter) attribute).doFilter(request, null, null); } catch (final IOException | ServletException e) { + log.debug("Exception unexpectedly thrown by filter", e); } } } diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java index 6ff07cb0af5..55be3188f98 100644 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java @@ -79,12 +79,27 @@ public static AgentScope start( @Advice.This final RequestDispatcher dispatcher, @Advice.Local("_requestSpan") Object requestSpan, @Advice.Argument(0) final ServletRequest request) { - if (activeSpan() == null) { + final AgentSpan parentSpan = activeSpan(); + + final Object servletSpanObject = request.getAttribute(DD_SPAN_ATTRIBUTE); + final AgentSpan servletSpan = + servletSpanObject instanceof AgentSpan ? (AgentSpan) servletSpanObject : null; + + if (parentSpan == null && servletSpan == null) { // Don't want to generate a new top-level span return null; } + final AgentSpan.Context parent; + if (servletSpan == null || (parentSpan != null && servletSpan.isSameTrace(parentSpan))) { + // Use the parentSpan if the servletSpan is null or part of the same trace. + parent = parentSpan.context(); + } else { + // parentSpan is part of a different trace, so lets ignore it. + // This can happen with the way Tomcat does error handling. + parent = servletSpan.context(); + } - final AgentSpan span = startSpan("servlet." + method); + final AgentSpan span = startSpan("servlet." + method, parent); DECORATE.afterStart(span); final String target = diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy index 04c1389f978..9804eab5c51 100644 --- a/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy @@ -25,6 +25,7 @@ class RequestDispatcherTest extends AgentTestRunner { dispatcher.include("") then: + 2 * request.getAttribute(DD_SPAN_ATTRIBUTE) assertTraces(2) { trace(0, 1) { basicSpan(it, 0, "forward-child") @@ -45,6 +46,7 @@ class RequestDispatcherTest extends AgentTestRunner { } then: + 1 * request.getAttribute(DD_SPAN_ATTRIBUTE) assertTraces(1) { trace(0, 3) { basicSpan(it, 0, "parent") @@ -97,6 +99,7 @@ class RequestDispatcherTest extends AgentTestRunner { def th = thrown(ServletException) th == ex + 1 * request.getAttribute(DD_SPAN_ATTRIBUTE) assertTraces(1) { trace(0, 3) { basicSpan(it, 0, "parent", null, ex) diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/spring-webmvc-3.1.gradle b/dd-java-agent/instrumentation/spring-webmvc-3.1/spring-webmvc-3.1.gradle index cf71c6fea7a..b2c76e86683 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/spring-webmvc-3.1.gradle +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/spring-webmvc-3.1.gradle @@ -48,6 +48,7 @@ dependencies { } // Include servlet instrumentation for verifying the tomcat requests + testCompile project(':dd-java-agent:instrumentation:servlet') testCompile project(':dd-java-agent:instrumentation:servlet:request-3') testCompile group: 'javax.validation', name: 'validation-api', version: '1.1.0.Final' diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/AppConfig.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/AppConfig.groovy index fce7d0d2ab9..ddc7251604f 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/AppConfig.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/AppConfig.groovy @@ -25,7 +25,7 @@ class AppConfig extends WebMvcConfigurerAdapter { .defaultContentTypeStrategy(new ContentNegotiationStrategy() { @Override List resolveMediaTypes(NativeWebRequest webRequest) throws HttpMediaTypeNotAcceptableException { - return [MediaType.TEXT_PLAIN, MediaType.APPLICATION_JSON] + return [MediaType.TEXT_PLAIN] } }) } diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy index c58ca3078c0..a3de8560980 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy @@ -91,6 +91,21 @@ class SpringBootBasedTest extends HttpServerTest } it.remove(renderSpan) } + def responseSpan = it.find { + it.operationName == "servlet.response" + } + if (responseSpan) { + SpanAssert.assertSpan(responseSpan) { + operationName "servlet.response" + resourceName { it == "HttpServletResponse.sendRedirect" || it == "HttpServletResponse.sendError" } + errored false + tags { + "$Tags.COMPONENT" "java-web-servlet-response" + defaultTags() + } + } + it.remove(responseSpan) + } } super.cleanAndAssertTraces(size, spec) diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/FilteredAppConfig.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/FilteredAppConfig.groovy index 46367b50aea..66bd9676bfd 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/FilteredAppConfig.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/FilteredAppConfig.groovy @@ -1,5 +1,6 @@ package test.filter +import com.google.common.base.Charsets import datadog.trace.agent.test.base.HttpServerTest import org.apache.catalina.connector.Connector import org.springframework.boot.autoconfigure.SpringBootApplication @@ -7,7 +8,14 @@ import org.springframework.boot.context.embedded.EmbeddedServletContainerFactory import org.springframework.boot.context.embedded.tomcat.TomcatConnectorCustomizer import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory import org.springframework.context.annotation.Bean +import org.springframework.http.HttpInputMessage +import org.springframework.http.HttpOutputMessage import org.springframework.http.MediaType +import org.springframework.http.converter.AbstractHttpMessageConverter +import org.springframework.http.converter.HttpMessageConverter +import org.springframework.http.converter.HttpMessageNotReadableException +import org.springframework.http.converter.HttpMessageNotWritableException +import org.springframework.util.StreamUtils import org.springframework.web.HttpMediaTypeNotAcceptableException import org.springframework.web.accept.ContentNegotiationStrategy import org.springframework.web.context.request.NativeWebRequest @@ -42,7 +50,7 @@ class FilteredAppConfig extends WebMvcConfigurerAdapter { .defaultContentTypeStrategy(new ContentNegotiationStrategy() { @Override List resolveMediaTypes(NativeWebRequest webRequest) throws HttpMediaTypeNotAcceptableException { - return [MediaType.TEXT_PLAIN, MediaType.APPLICATION_JSON] + return [MediaType.TEXT_PLAIN] } }) } @@ -62,6 +70,27 @@ class FilteredAppConfig extends WebMvcConfigurerAdapter { return factory } + @Bean + HttpMessageConverter> createPlainMapMessageConverter() { + return new AbstractHttpMessageConverter>(MediaType.TEXT_PLAIN) { + + @Override + protected boolean supports(Class clazz) { + return Map.isAssignableFrom(clazz) + } + + @Override + protected Map readInternal(Class> clazz, HttpInputMessage inputMessage) throws IOException, HttpMessageNotReadableException { + return null + } + + @Override + protected void writeInternal(Map stringObjectMap, HttpOutputMessage outputMessage) throws IOException, HttpMessageNotWritableException { + StreamUtils.copy(stringObjectMap.get("message"), Charsets.UTF_8, outputMessage.getBody()) + } + } + } + @Bean Filter servletFilter() { return new Filter() { diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy index 13eb4589181..0644ad26724 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy @@ -96,6 +96,21 @@ class ServletFilterTest extends HttpServerTest { } it.remove(renderSpan) } + def responseSpan = it.find { + it.operationName == "servlet.response" + } + if (responseSpan) { + SpanAssert.assertSpan(responseSpan) { + operationName "servlet.response" + resourceName { it == "HttpServletResponse.sendRedirect" || it == "HttpServletResponse.sendError" } + errored false + tags { + "$Tags.COMPONENT" "java-web-servlet-response" + defaultTags() + } + } + it.remove(responseSpan) + } } super.cleanAndAssertTraces(size, spec) diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index 52462ca71e0..3b14193ce81 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -140,6 +140,7 @@ abstract class HttpServerTest extends AgentTestRunner { final int status final String body final Boolean errored + final boolean hasPathParam ServerEndpoint(String uri, int status, String body) { def uriObj = URI.create(uri) @@ -149,6 +150,7 @@ abstract class HttpServerTest extends AgentTestRunner { this.status = status this.body = body this.errored = status >= 500 + this.hasPathParam = body == "123" } String getPath() { @@ -164,7 +166,7 @@ abstract class HttpServerTest extends AgentTestRunner { } String resource(String method, URI address, String pathParam) { - return status == 404 ? "404" : "$method ${pathParam ? pathParam : resolve(address).path}" + return status == 404 ? "404" : "$method ${hasPathParam ? pathParam : resolve(address).path}" } private static final Map PATH_MAP = values().collectEntries { [it.path, it] }