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.
MODIFY
dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java
dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Decorator.java
@@ -2,8 +2,11 @@ package datadog.trace.instrumentation.servlet3;
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 @@ public class Servlet3Decorator
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) {

MODIFY
dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java
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.activeSpan
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 final class DispatcherServletInstrumentation extends Instrumenter.Default
return named("org.springframework.web.servlet.DispatcherServlet");
}
+ @Override
+ public Map<String, String> 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<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
final Map<ElementMatcher<? super MethodDescription>, 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 final class DispatcherServletInstrumentation extends Instrumenter.Default
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<HandlerMapping> handlerMappings,
+ @Advice.Thrown final Throwable throwable) {
+ final ServletContext servletContext = springCtx.getBean(ServletContext.class);
+ if (handlerMappings != null && servletContext != null) {
+ final ContextStore<DispatcherServlet, HandlerMappingResourceNameFilter> 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 final class DispatcherServletInstrumentation extends Instrumenter.Default
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 {

ADD
dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/HandlerMappingResourceNameFilter.java
+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<HandlerMapping> 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<HandlerMapping> handlerMappings) {
+ this.handlerMappings = handlerMappings;
+ }
+}

RENAME
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/AppConfig.groovy
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/AppConfig.groovy
-package test
+package test.boot
import org.apache.catalina.connector.Connector
import org.springframework.boot.autoconfigure.SpringBootApplication

RENAME
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/SpringBootBasedTest.groovy
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy
-package test
+package test.boot
import datadog.opentracing.DDSpan
import datadog.trace.agent.test.asserts.ListWriterAssert
@@ -51,6 +51,11 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
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<ConfigurableApplicationContext>
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) {

RENAME
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/TestController.groovy
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/TestController.groovy
-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() {

ADD
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/FilteredAppConfig.groovy
+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<MediaType> 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() {
+ }
+ }
+ }
+}

ADD
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy
+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<ConfigurableApplicationContext> {
+
+ @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)
+ }
+ }
+ }
+}

ADD
dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/TestController.groovy
+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)
+ }
+}

MODIFY
dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy
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<SERVER> 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<SERVER> 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<SERVER> 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<String, ServerEndpoint> PATH_MAP = values().collectEntries { [it.path, it] }
static ServerEndpoint forPath(String path) {
@@ -288,6 +298,37 @@ abstract class HttpServerTest<SERVER> 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<SERVER> 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) {

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.