From a271f0d93caf6f4ebf81d2100b2ed9ca0ac96525 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 2 Jun 2026 15:04:02 +0200 Subject: [PATCH 1/2] Drop the duplicate handler resolution in Spring Web 6 instrumentation --- .../DispatcherServletInstrumentation.java | 13 +-- ...rvletPathRequestFilterInstrumentation.java | 64 ----------- .../WebApplicationContextInstrumentation.java | 60 ---------- .../springweb6/ControllerAdvice.java | 6 +- .../springweb6/FilterInjectingAdvice.java | 19 ---- .../springweb6/HandleMatchAdvice.java | 7 -- .../springweb6/HandlerMappingAdvice.java | 45 +++++--- .../HandlerMappingResourceNameFilter.java | 105 ------------------ .../OrderedServletPathRequestFilter.java | 29 ----- ...PathMatchingHttpServletRequestWrapper.java | 33 ------ .../ServletPathFilterInjectingAdvice.java | 19 ---- .../boot/SpringBootBasedTest.groovy | 21 +--- .../springweb6/boot/FailOnHeaderConfig.java | 13 +++ .../boot/FailOnHeaderInterceptor.java | 21 ++++ 14 files changed, 81 insertions(+), 374 deletions(-) delete mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ServletPathRequestFilterInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/WebApplicationContextInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/FilterInjectingAdvice.java delete mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingResourceNameFilter.java delete mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/OrderedServletPathRequestFilter.java delete mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/PathMatchingHttpServletRequestWrapper.java delete mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ServletPathFilterInjectingAdvice.java create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/java/datadog/trace/instrumentation/springweb6/boot/FailOnHeaderConfig.java create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/java/datadog/trace/instrumentation/springweb6/boot/FailOnHeaderInterceptor.java diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/DispatcherServletInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/DispatcherServletInstrumentation.java index 2c2874bee35..bfe0f775159 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/DispatcherServletInstrumentation.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/DispatcherServletInstrumentation.java @@ -4,6 +4,7 @@ import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isProtected; +import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; @@ -27,10 +28,7 @@ public String instrumentedType() { @Override public String[] helperClassNames() { return new String[] { - packageName + ".SpringWebHttpServerDecorator", - packageName + ".ServletRequestURIAdapter", - packageName + ".HandlerMappingResourceNameFilter", - packageName + ".PathMatchingHttpServletRequestWrapper", + packageName + ".SpringWebHttpServerDecorator", packageName + ".ServletRequestURIAdapter", }; } @@ -39,9 +37,10 @@ public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( isMethod() .and(isProtected()) - .and(named("onRefresh")) - .and(takesArgument(0, named("org.springframework.context.ApplicationContext"))) - .and(takesArguments(1)), + .and(named("getHandler")) + .and(takesArgument(0, named("jakarta.servlet.http.HttpServletRequest"))) + .and(takesArguments(1)) + .and(returns(named("org.springframework.web.servlet.HandlerExecutionChain"))), packageName + ".HandlerMappingAdvice"); transformer.applyAdvice( isMethod() diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ServletPathRequestFilterInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ServletPathRequestFilterInstrumentation.java deleted file mode 100644 index 5c2db70ba85..00000000000 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ServletPathRequestFilterInstrumentation.java +++ /dev/null @@ -1,64 +0,0 @@ -package datadog.trace.instrumentation.springweb6; - -import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed; -import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass; -import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface; -import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.agent.tooling.InstrumenterModule; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -/** - * This instrumentation adds the ServletPathRequestFilter definition to the spring context When the - * context is created, the filter will be added to the beginning of the filter chain - */ -@AutoService(InstrumenterModule.class) -public class ServletPathRequestFilterInstrumentation extends InstrumenterModule.Tracing - implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { - public ServletPathRequestFilterInstrumentation() { - super("spring-web", "spring-path-filter"); - } - - @Override - public ElementMatcher.Junction classLoaderMatcher() { - return hasClassNamed("org.springframework.web.filter.ServletRequestPathFilter") - .and(hasClassNamed("jakarta.servlet.Filter")); - } - - @Override - public String hierarchyMarkerType() { - return "org.springframework.web.context.WebApplicationContext"; - } - - @Override - public ElementMatcher hierarchyMatcher() { - return extendsClass(named("org.springframework.context.support.AbstractApplicationContext")) - .and(implementsInterface(named(hierarchyMarkerType()))); - } - - @Override - public String[] helperClassNames() { - return new String[] { - packageName + ".OrderedServletPathRequestFilter", - packageName + ".OrderedServletPathRequestFilter$BeanDefinition", - }; - } - - @Override - public void methodAdvice(MethodTransformer transformer) { - transformer.applyAdvice( - isMethod() - .and(named("postProcessBeanFactory")) - .and( - takesArgument( - 0, - named( - "org.springframework.beans.factory.config.ConfigurableListableBeanFactory"))), - packageName + ".ServletPathFilterInjectingAdvice"); - } -} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/WebApplicationContextInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/WebApplicationContextInstrumentation.java deleted file mode 100644 index 672570f94bf..00000000000 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/WebApplicationContextInstrumentation.java +++ /dev/null @@ -1,60 +0,0 @@ -package datadog.trace.instrumentation.springweb6; - -import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass; -import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface; -import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.agent.tooling.InstrumenterModule; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -/** - * This instrumentation adds the HandlerMappingResourceNameFilter definition to the spring context - * When the context is created, the filter will be added to the beginning of the filter chain - */ -@AutoService(InstrumenterModule.class) -public class WebApplicationContextInstrumentation extends InstrumenterModule.Tracing - implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { - public WebApplicationContextInstrumentation() { - super("spring-web", "spring-path-filter"); - } - - @Override - public String hierarchyMarkerType() { - return "org.springframework.web.context.WebApplicationContext"; - } - - @Override - public ElementMatcher hierarchyMatcher() { - return extendsClass(named("org.springframework.context.support.AbstractApplicationContext")) - .and(implementsInterface(named(hierarchyMarkerType()))); - } - - @Override - public String[] helperClassNames() { - return new String[] { - packageName + ".SpringWebHttpServerDecorator", - packageName + ".ServletRequestURIAdapter", - packageName + ".HandlerMappingResourceNameFilter", - packageName + ".HandlerMappingResourceNameFilter$BeanDefinition", - packageName + ".PathMatchingHttpServletRequestWrapper", - }; - } - - @Override - public void methodAdvice(MethodTransformer transformer) { - transformer.applyAdvice( - isMethod() - .and(named("postProcessBeanFactory")) - .and( - takesArgument( - 0, - named( - "org.springframework.beans.factory.config.ConfigurableListableBeanFactory"))), - packageName + ".FilterInjectingAdvice"); - } -} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ControllerAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ControllerAdvice.java index 5a675262cad..b5eaeb2b9bd 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ControllerAdvice.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ControllerAdvice.java @@ -25,7 +25,11 @@ public static ContextScope nameResourceAndStartSpan( @Advice.Argument(2) final Object handler, @Advice.Local("handlerSpanKey") String handlerSpanKey) { handlerSpanKey = ""; - // Name the parent span based on the matching pattern + + /* + By the time HandlerAdapter.handle runs, every handler mapping kind (annotated and SimpleUrlHandlerMapping via its + PathExposingHandlerInterceptor) has populated BEST_MATCHING_PATTERN_ATTRIBUTE. + */ Object contextObj = request.getAttribute(DD_CONTEXT_ATTRIBUTE); if (contextObj instanceof Context) { Context context = (Context) contextObj; diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/FilterInjectingAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/FilterInjectingAdvice.java deleted file mode 100644 index 0442b2ba567..00000000000 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/FilterInjectingAdvice.java +++ /dev/null @@ -1,19 +0,0 @@ -package datadog.trace.instrumentation.springweb6; - -import net.bytebuddy.asm.Advice; -import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; -import org.springframework.beans.factory.support.BeanDefinitionRegistry; - -public class FilterInjectingAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter( - @Advice.Argument(0) final ConfigurableListableBeanFactory beanFactory) { - if (beanFactory instanceof BeanDefinitionRegistry - && !beanFactory.containsBean("ddDispatcherFilter")) { - - ((BeanDefinitionRegistry) beanFactory) - .registerBeanDefinition( - "ddDispatcherFilter", new HandlerMappingResourceNameFilter.BeanDefinition()); - } - } -} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java index e299ac0f91c..1c26c564f8b 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java @@ -37,13 +37,6 @@ public static void after( return; } - // hacky, but APM instrumentation causes the instrumented method to be called twice - if (req.getClass() - .getName() - .equals("datadog.trace.instrumentation.springweb6.PathMatchingHttpServletRequestWrapper")) { - return; - } - AgentSpan agentSpan = AgentTracer.activeSpan(); if (agentSpan == null) { return; diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingAdvice.java index ffabc4115be..fe818edbf3b 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingAdvice.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingAdvice.java @@ -1,26 +1,41 @@ package datadog.trace.instrumentation.springweb6; -import java.util.List; +import static datadog.context.Context.root; +import static datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge.spanFromContext; +import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_CONTEXT_ATTRIBUTE; +import static datadog.trace.instrumentation.springweb6.SpringWebHttpServerDecorator.DECORATE; + +import datadog.context.Context; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import jakarta.servlet.http.HttpServletRequest; import net.bytebuddy.asm.Advice; -import org.springframework.context.ApplicationContext; -import org.springframework.web.servlet.HandlerMapping; +import org.springframework.web.servlet.HandlerExecutionChain; /** - * 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. + * Names the server span route as soon as {@code DispatcherServlet} has resolved the handler. + * + *

For {@code RequestMappingInfoHandlerMapping}, {@code handleMatch} sets {@link + * org.springframework.web.servlet.HandlerMapping#BEST_MATCHING_PATTERN_ATTRIBUTE} synchronously + * inside {@code getHandler}, before interceptors run. Naming the route here means it survives a + * {@code HandlerInterceptor.preHandle} that aborts the request before the controller executes. */ public class HandlerMappingAdvice { - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void afterRefresh( - @Advice.Argument(0) final ApplicationContext springCtx, - @Advice.FieldValue("handlerMappings") final List handlerMappings) { - if (springCtx.containsBean("ddDispatcherFilter")) { - final datadog.trace.instrumentation.springweb6.HandlerMappingResourceNameFilter filter = - (HandlerMappingResourceNameFilter) springCtx.getBean("ddDispatcherFilter"); - if (handlerMappings != null && filter != null) { - filter.setHandlerMappings(handlerMappings); + @Advice.OnMethodExit(suppress = Throwable.class) + public static void onExit( + @Advice.Argument(0) final HttpServletRequest request, + @Advice.Return final HandlerExecutionChain chain) { + if (chain == null) { + // No handler matched (e.g. 404): leave the resource name untouched. + return; + } + final Object contextObj = request.getAttribute(DD_CONTEXT_ATTRIBUTE); + if (contextObj instanceof Context) { + final AgentSpan parentSpan = spanFromContext((Context) contextObj); + if (parentSpan != null) { + // onRequest reads BEST_MATCHING_PATTERN_ATTRIBUTE, skips "/**", and guards against + // re-application (also by ControllerAdvice) on forwards/redirects. + DECORATE.onRequest(parentSpan, request, request, root()); } } } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingResourceNameFilter.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingResourceNameFilter.java deleted file mode 100644 index eac11f69ca2..00000000000 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingResourceNameFilter.java +++ /dev/null @@ -1,105 +0,0 @@ -package datadog.trace.instrumentation.springweb6; - -import static datadog.context.Context.root; -import static datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge.spanFromContext; -import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_CONTEXT_ATTRIBUTE; -import static datadog.trace.instrumentation.springweb6.SpringWebHttpServerDecorator.DECORATE; - -import datadog.context.Context; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition; -import org.springframework.core.Ordered; -import org.springframework.web.filter.OncePerRequestFilter; -import org.springframework.web.servlet.HandlerExecutionChain; -import org.springframework.web.servlet.HandlerMapping; -import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping; - -public class HandlerMappingResourceNameFilter extends OncePerRequestFilter implements Ordered { - - private static final Logger log = LoggerFactory.getLogger(HandlerMappingResourceNameFilter.class); - private final List handlerMappings = new CopyOnWriteArrayList<>(); - - @Override - protected void doFilterInternal( - final HttpServletRequest request, - final HttpServletResponse response, - final FilterChain filterChain) - throws ServletException, IOException { - - final Object contextObj = request.getAttribute(DD_CONTEXT_ATTRIBUTE); - if (contextObj instanceof Context) { - Context context = (Context) contextObj; - AgentSpan parentSpan = spanFromContext(context); - if (parentSpan != null) { - PathMatchingHttpServletRequestWrapper wrappedRequest = - new PathMatchingHttpServletRequestWrapper(request); - try { - if (findMapping(wrappedRequest)) { - // Name the parent span based on the matching pattern - // Let the parent span resource name be set with the attribute set in findMapping. - DECORATE.onRequest(parentSpan, wrappedRequest, wrappedRequest, root()); - } - } catch (final Exception ignored) { - // mapping.getHandler() threw exception. Ignore - } - } - } - - filterChain.doFilter(request, response); - } - - /** - * 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) { - for (HandlerMapping handlerMapping : handlerMappings) { - if (handlerMapping instanceof RequestMappingInfoHandlerMapping) { - if (!this.handlerMappings.contains(handlerMapping)) { - this.handlerMappings.add(handlerMapping); - } - } else { - log.debug( - "discarding handler mapping {} which won't set BEST_MATCHING_PATTERN_ATTRIBUTE", - handlerMapping); - } - } - } - - @Override - public int getOrder() { - // Run after all HIGHEST_PRECEDENCE items - return Ordered.HIGHEST_PRECEDENCE + 1; - } - - public static class BeanDefinition extends AnnotatedGenericBeanDefinition { - private static final long serialVersionUID = 5623859691503032280L; - - public BeanDefinition() { - super(HandlerMappingResourceNameFilter.class); - // don't call setBeanClassName as it overwrites 'beanClass' - setScope(SCOPE_SINGLETON); - setLazyInit(true); - } - } -} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/OrderedServletPathRequestFilter.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/OrderedServletPathRequestFilter.java deleted file mode 100644 index d0db80e1d7c..00000000000 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/OrderedServletPathRequestFilter.java +++ /dev/null @@ -1,29 +0,0 @@ -package datadog.trace.instrumentation.springweb6; - -import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition; -import org.springframework.core.Ordered; -import org.springframework.web.filter.DelegatingFilterProxy; -import org.springframework.web.filter.ServletRequestPathFilter; - -public class OrderedServletPathRequestFilter extends DelegatingFilterProxy implements Ordered { - - @Override - public int getOrder() { - return Ordered.HIGHEST_PRECEDENCE; - } - - public OrderedServletPathRequestFilter() { - super(new ServletRequestPathFilter()); - } - - public static class BeanDefinition extends AnnotatedGenericBeanDefinition { - private static final long serialVersionUID = -5117836999188187797L; - - public BeanDefinition() { - super(OrderedServletPathRequestFilter.class); - // don't call setBeanClassName as it overwrites 'beanClass' - setScope(SCOPE_SINGLETON); - setLazyInit(true); - } - } -} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/PathMatchingHttpServletRequestWrapper.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/PathMatchingHttpServletRequestWrapper.java deleted file mode 100644 index 7c0e05f23b9..00000000000 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/PathMatchingHttpServletRequestWrapper.java +++ /dev/null @@ -1,33 +0,0 @@ -package datadog.trace.instrumentation.springweb6; - -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletRequestWrapper; -import java.util.HashMap; -import java.util.Map; - -class PathMatchingHttpServletRequestWrapper extends HttpServletRequestWrapper { - private final Map localAttributes = new HashMap<>(); - - public PathMatchingHttpServletRequestWrapper(HttpServletRequest request) { - super(request); - } - - @Override - public Object getAttribute(String name) { - final Object ret = localAttributes.get(name); - if (ret == null) { - return super.getAttribute(name); - } - return ret; - } - - @Override - public void setAttribute(String name, Object o) { - localAttributes.put(name, o); - } - - @Override - public void removeAttribute(String name) { - localAttributes.remove(name); - } -} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ServletPathFilterInjectingAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ServletPathFilterInjectingAdvice.java deleted file mode 100644 index 83d1afed091..00000000000 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ServletPathFilterInjectingAdvice.java +++ /dev/null @@ -1,19 +0,0 @@ -package datadog.trace.instrumentation.springweb6; - -import net.bytebuddy.asm.Advice; -import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; -import org.springframework.beans.factory.support.BeanDefinitionRegistry; - -public class ServletPathFilterInjectingAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter( - @Advice.Argument(0) final ConfigurableListableBeanFactory beanFactory) { - if (beanFactory instanceof BeanDefinitionRegistry - && !beanFactory.containsBean("servletPathRequestFilter")) { - - ((BeanDefinitionRegistry) beanFactory) - .registerBeanDefinition( - "servletPathRequestFilter", new OrderedServletPathRequestFilter.BeanDefinition()); - } - } -} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy index 3cd4ff3efd9..abb1b322214 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy @@ -16,8 +16,6 @@ import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.core.DDSpan import datadog.trace.instrumentation.springweb6.SetupSpecHelper import datadog.trace.instrumentation.springweb6.SpringWebHttpServerDecorator -import jakarta.servlet.http.HttpServletRequest -import jakarta.servlet.http.HttpServletResponse import okhttp3.Credentials import okhttp3.FormBody import okhttp3.Request @@ -28,7 +26,6 @@ import org.springframework.boot.SpringApplication import org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext import org.springframework.context.ConfigurableApplicationContext import org.springframework.http.MediaType -import org.springframework.web.servlet.HandlerInterceptor import org.springframework.web.socket.BinaryMessage import org.springframework.web.socket.TextMessage import org.springframework.web.socket.WebSocketSession @@ -59,7 +56,7 @@ class SpringBootBasedTest extends HttpServerTest Map extraServerTags = [:] SpringApplication application() { - return new SpringApplication(SecurityConfig, TestController, AppConfig, WebsocketConfig) + return new SpringApplication(SecurityConfig, TestController, AppConfig, WebsocketConfig, FailOnHeaderConfig) } class SpringBootServer implements WebsocketServer { @@ -451,26 +448,20 @@ class SpringBootBasedTest extends HttpServerTest InstrumentationBridge.clearIastModules() } - def 'path is extract when preHandle fails'() { + def 'path is extracted when preHandle fails'() { setup: def request = request(PATH_PARAM, 'GET', null).header("fail", "true").build() - context.getBeanFactory().registerSingleton("testHandler", new HandlerInterceptor() { - @Override - boolean preHandle(HttpServletRequest req, HttpServletResponse response, Object handler) throws Exception { - if ("true".equalsIgnoreCase(req.getHeader("fail"))) { - throw new RuntimeException("Stop here") - } - return true - } - }) when: - client.newCall(request).execute() + def response = client.newCall(request).execute() + response.close() TEST_WRITER.waitForTraces(1) DDSpan span = TEST_WRITER.flatten().find { "servlet.request".contentEquals(it.operationName) } then: + response.code() == 500 span.getResourceName().toString() == "GET " + testPathParam() + span.isError() } boolean hasResponseSpan(ServerEndpoint endpoint) { diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/java/datadog/trace/instrumentation/springweb6/boot/FailOnHeaderConfig.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/java/datadog/trace/instrumentation/springweb6/boot/FailOnHeaderConfig.java new file mode 100644 index 00000000000..654893ffd40 --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/java/datadog/trace/instrumentation/springweb6/boot/FailOnHeaderConfig.java @@ -0,0 +1,13 @@ +package datadog.trace.instrumentation.springweb6.boot; + +import org.springframework.context.annotation.Configuration; +import org.springframework.web.servlet.config.annotation.InterceptorRegistry; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; + +@Configuration +public class FailOnHeaderConfig implements WebMvcConfigurer { + @Override + public void addInterceptors(final InterceptorRegistry registry) { + registry.addInterceptor(new FailOnHeaderInterceptor()); + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/java/datadog/trace/instrumentation/springweb6/boot/FailOnHeaderInterceptor.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/java/datadog/trace/instrumentation/springweb6/boot/FailOnHeaderInterceptor.java new file mode 100644 index 00000000000..03728a89e4e --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/java/datadog/trace/instrumentation/springweb6/boot/FailOnHeaderInterceptor.java @@ -0,0 +1,21 @@ +package datadog.trace.instrumentation.springweb6.boot; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.springframework.web.servlet.HandlerInterceptor; + +/** + * Aborts the request from {@code preHandle} when the {@code fail} header is set, so the controller + * is never invoked. Test to verify the route is still named on the server span in that case (see + * {@code HandlerMappingAdvice}). + */ +public class FailOnHeaderInterceptor implements HandlerInterceptor { + @Override + public boolean preHandle( + final HttpServletRequest request, final HttpServletResponse response, final Object handler) { + if ("true".equalsIgnoreCase(request.getHeader("fail"))) { + throw new RuntimeException("Stop here"); + } + return true; + } +} From 1bb144416a57b746af790102008ace3ec7fd51de Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 3 Jun 2026 09:15:02 +0200 Subject: [PATCH 2/2] reintroduce the old instrumentation and make it opt-out --- ...ourceNameFilterMappingInstrumentation.java | 58 ++++++++++ ...rvletPathRequestFilterInstrumentation.java | 71 ++++++++++++ .../WebApplicationContextInstrumentation.java | 67 +++++++++++ .../springweb6/FilterInjectingAdvice.java | 19 ++++ .../springweb6/HandleMatchAdvice.java | 8 ++ .../springweb6/HandlerMappingAdvice.java | 2 - .../HandlerMappingResourceNameFilter.java | 105 ++++++++++++++++++ .../OrderedServletPathRequestFilter.java | 29 +++++ ...PathMatchingHttpServletRequestWrapper.java | 33 ++++++ .../ResourceNameFilterMappingAdvice.java | 26 +++++ .../ServletPathFilterInjectingAdvice.java | 19 ++++ .../boot/SpringBootBasedTest.groovy | 13 +++ 12 files changed, 448 insertions(+), 2 deletions(-) create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ResourceNameFilterMappingInstrumentation.java create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ServletPathRequestFilterInstrumentation.java create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/WebApplicationContextInstrumentation.java create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/FilterInjectingAdvice.java create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingResourceNameFilter.java create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/OrderedServletPathRequestFilter.java create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/PathMatchingHttpServletRequestWrapper.java create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ResourceNameFilterMappingAdvice.java create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ServletPathFilterInjectingAdvice.java diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ResourceNameFilterMappingInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ResourceNameFilterMappingInstrumentation.java new file mode 100644 index 00000000000..9b1b73a4f62 --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ResourceNameFilterMappingInstrumentation.java @@ -0,0 +1,58 @@ +package datadog.trace.instrumentation.springweb6; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isProtected; +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.agent.tooling.InstrumenterModule; + +/** + * When the opt-in {@code spring-path-filter} integration is enabled, wires the {@code + * DispatcherServlet} handler mappings into {@link HandlerMappingResourceNameFilter} on context + * refresh. Disabled by default; the default route naming is handled by {@link HandlerMappingAdvice} + * ({@code getHandler}) and {@link ControllerAdvice}. + */ +@AutoService(InstrumenterModule.class) +public final class ResourceNameFilterMappingInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + public ResourceNameFilterMappingInstrumentation() { + super("spring-web", "spring-path-filter"); + } + + @Override + protected boolean defaultEnabled() { + return false; + } + + @Override + public String instrumentedType() { + return "org.springframework.web.servlet.DispatcherServlet"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".SpringWebHttpServerDecorator", + packageName + ".ServletRequestURIAdapter", + packageName + ".HandlerMappingResourceNameFilter", + packageName + ".HandlerMappingResourceNameFilter$BeanDefinition", + packageName + ".PathMatchingHttpServletRequestWrapper", + }; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isMethod() + .and(isProtected()) + .and(named("onRefresh")) + .and(takesArgument(0, named("org.springframework.context.ApplicationContext"))) + .and(takesArguments(1)), + packageName + ".ResourceNameFilterMappingAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ServletPathRequestFilterInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ServletPathRequestFilterInstrumentation.java new file mode 100644 index 00000000000..784da331cf0 --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/ServletPathRequestFilterInstrumentation.java @@ -0,0 +1,71 @@ +package datadog.trace.instrumentation.springweb6; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed; +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass; +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +/** + * Companion to {@link WebApplicationContextInstrumentation}: when the opt-in {@code + * spring-path-filter} integration is enabled, registers Spring's {@code ServletRequestPathFilter} + * first in the chain so the request path is parsed before {@link HandlerMappingResourceNameFilter} + * resolves the handler. Disabled by default. + */ +@AutoService(InstrumenterModule.class) +public class ServletPathRequestFilterInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { + public ServletPathRequestFilterInstrumentation() { + super("spring-web", "spring-path-filter"); + } + + @Override + protected boolean defaultEnabled() { + return false; + } + + @Override + public ElementMatcher.Junction classLoaderMatcher() { + return hasClassNamed("org.springframework.web.filter.ServletRequestPathFilter") + .and(hasClassNamed("jakarta.servlet.Filter")); + } + + @Override + public String hierarchyMarkerType() { + return "org.springframework.web.context.WebApplicationContext"; + } + + @Override + public ElementMatcher hierarchyMatcher() { + return extendsClass(named("org.springframework.context.support.AbstractApplicationContext")) + .and(implementsInterface(named(hierarchyMarkerType()))); + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".OrderedServletPathRequestFilter", + packageName + ".OrderedServletPathRequestFilter$BeanDefinition", + }; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isMethod() + .and(named("postProcessBeanFactory")) + .and( + takesArgument( + 0, + named( + "org.springframework.beans.factory.config.ConfigurableListableBeanFactory"))), + packageName + ".ServletPathFilterInjectingAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/WebApplicationContextInstrumentation.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/WebApplicationContextInstrumentation.java new file mode 100644 index 00000000000..a7c54f26e93 --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java/datadog/trace/instrumentation/springweb6/WebApplicationContextInstrumentation.java @@ -0,0 +1,67 @@ +package datadog.trace.instrumentation.springweb6; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass; +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +/** + * Opt-in alternative to the default {@code getHandler}/{@code ControllerAdvice} route naming. Adds + * the {@link HandlerMappingResourceNameFilter} bean to the Spring context, which names the route at + * the start of the filter chain by resolving the handler itself. This is the legacy approach and is + * disabled by default; enable it with {@code dd.integration.spring-path-filter.enabled=true}. + */ +@AutoService(InstrumenterModule.class) +public class WebApplicationContextInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { + public WebApplicationContextInstrumentation() { + super("spring-web", "spring-path-filter"); + } + + @Override + protected boolean defaultEnabled() { + return false; + } + + @Override + public String hierarchyMarkerType() { + return "org.springframework.web.context.WebApplicationContext"; + } + + @Override + public ElementMatcher hierarchyMatcher() { + return extendsClass(named("org.springframework.context.support.AbstractApplicationContext")) + .and(implementsInterface(named(hierarchyMarkerType()))); + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".SpringWebHttpServerDecorator", + packageName + ".ServletRequestURIAdapter", + packageName + ".HandlerMappingResourceNameFilter", + packageName + ".HandlerMappingResourceNameFilter$BeanDefinition", + packageName + ".PathMatchingHttpServletRequestWrapper", + }; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isMethod() + .and(named("postProcessBeanFactory")) + .and( + takesArgument( + 0, + named( + "org.springframework.beans.factory.config.ConfigurableListableBeanFactory"))), + packageName + ".FilterInjectingAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/FilterInjectingAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/FilterInjectingAdvice.java new file mode 100644 index 00000000000..0442b2ba567 --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/FilterInjectingAdvice.java @@ -0,0 +1,19 @@ +package datadog.trace.instrumentation.springweb6; + +import net.bytebuddy.asm.Advice; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; + +public class FilterInjectingAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.Argument(0) final ConfigurableListableBeanFactory beanFactory) { + if (beanFactory instanceof BeanDefinitionRegistry + && !beanFactory.containsBean("ddDispatcherFilter")) { + + ((BeanDefinitionRegistry) beanFactory) + .registerBeanDefinition( + "ddDispatcherFilter", new HandlerMappingResourceNameFilter.BeanDefinition()); + } + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java index 1c26c564f8b..426aff5ffcb 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandleMatchAdvice.java @@ -37,6 +37,14 @@ public static void after( return; } + // When the opt-in spring-path-filter integration is enabled, the filter resolves the handler + // against a PathMatchingHttpServletRequestWrapper, which triggers handleMatch a second time. + if (req.getClass() + .getName() + .equals("datadog.trace.instrumentation.springweb6.PathMatchingHttpServletRequestWrapper")) { + return; + } + AgentSpan agentSpan = AgentTracer.activeSpan(); if (agentSpan == null) { return; diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingAdvice.java index fe818edbf3b..08783e50a96 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingAdvice.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingAdvice.java @@ -33,8 +33,6 @@ public static void onExit( if (contextObj instanceof Context) { final AgentSpan parentSpan = spanFromContext((Context) contextObj); if (parentSpan != null) { - // onRequest reads BEST_MATCHING_PATTERN_ATTRIBUTE, skips "/**", and guards against - // re-application (also by ControllerAdvice) on forwards/redirects. DECORATE.onRequest(parentSpan, request, request, root()); } } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingResourceNameFilter.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingResourceNameFilter.java new file mode 100644 index 00000000000..eac11f69ca2 --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/HandlerMappingResourceNameFilter.java @@ -0,0 +1,105 @@ +package datadog.trace.instrumentation.springweb6; + +import static datadog.context.Context.root; +import static datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge.spanFromContext; +import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.DD_CONTEXT_ATTRIBUTE; +import static datadog.trace.instrumentation.springweb6.SpringWebHttpServerDecorator.DECORATE; + +import datadog.context.Context; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition; +import org.springframework.core.Ordered; +import org.springframework.web.filter.OncePerRequestFilter; +import org.springframework.web.servlet.HandlerExecutionChain; +import org.springframework.web.servlet.HandlerMapping; +import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping; + +public class HandlerMappingResourceNameFilter extends OncePerRequestFilter implements Ordered { + + private static final Logger log = LoggerFactory.getLogger(HandlerMappingResourceNameFilter.class); + private final List handlerMappings = new CopyOnWriteArrayList<>(); + + @Override + protected void doFilterInternal( + final HttpServletRequest request, + final HttpServletResponse response, + final FilterChain filterChain) + throws ServletException, IOException { + + final Object contextObj = request.getAttribute(DD_CONTEXT_ATTRIBUTE); + if (contextObj instanceof Context) { + Context context = (Context) contextObj; + AgentSpan parentSpan = spanFromContext(context); + if (parentSpan != null) { + PathMatchingHttpServletRequestWrapper wrappedRequest = + new PathMatchingHttpServletRequestWrapper(request); + try { + if (findMapping(wrappedRequest)) { + // Name the parent span based on the matching pattern + // Let the parent span resource name be set with the attribute set in findMapping. + DECORATE.onRequest(parentSpan, wrappedRequest, wrappedRequest, root()); + } + } catch (final Exception ignored) { + // mapping.getHandler() threw exception. Ignore + } + } + } + + filterChain.doFilter(request, response); + } + + /** + * 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) { + for (HandlerMapping handlerMapping : handlerMappings) { + if (handlerMapping instanceof RequestMappingInfoHandlerMapping) { + if (!this.handlerMappings.contains(handlerMapping)) { + this.handlerMappings.add(handlerMapping); + } + } else { + log.debug( + "discarding handler mapping {} which won't set BEST_MATCHING_PATTERN_ATTRIBUTE", + handlerMapping); + } + } + } + + @Override + public int getOrder() { + // Run after all HIGHEST_PRECEDENCE items + return Ordered.HIGHEST_PRECEDENCE + 1; + } + + public static class BeanDefinition extends AnnotatedGenericBeanDefinition { + private static final long serialVersionUID = 5623859691503032280L; + + public BeanDefinition() { + super(HandlerMappingResourceNameFilter.class); + // don't call setBeanClassName as it overwrites 'beanClass' + setScope(SCOPE_SINGLETON); + setLazyInit(true); + } + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/OrderedServletPathRequestFilter.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/OrderedServletPathRequestFilter.java new file mode 100644 index 00000000000..d0db80e1d7c --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/OrderedServletPathRequestFilter.java @@ -0,0 +1,29 @@ +package datadog.trace.instrumentation.springweb6; + +import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition; +import org.springframework.core.Ordered; +import org.springframework.web.filter.DelegatingFilterProxy; +import org.springframework.web.filter.ServletRequestPathFilter; + +public class OrderedServletPathRequestFilter extends DelegatingFilterProxy implements Ordered { + + @Override + public int getOrder() { + return Ordered.HIGHEST_PRECEDENCE; + } + + public OrderedServletPathRequestFilter() { + super(new ServletRequestPathFilter()); + } + + public static class BeanDefinition extends AnnotatedGenericBeanDefinition { + private static final long serialVersionUID = -5117836999188187797L; + + public BeanDefinition() { + super(OrderedServletPathRequestFilter.class); + // don't call setBeanClassName as it overwrites 'beanClass' + setScope(SCOPE_SINGLETON); + setLazyInit(true); + } + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/PathMatchingHttpServletRequestWrapper.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/PathMatchingHttpServletRequestWrapper.java new file mode 100644 index 00000000000..7c0e05f23b9 --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/PathMatchingHttpServletRequestWrapper.java @@ -0,0 +1,33 @@ +package datadog.trace.instrumentation.springweb6; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import java.util.HashMap; +import java.util.Map; + +class PathMatchingHttpServletRequestWrapper extends HttpServletRequestWrapper { + private final Map localAttributes = new HashMap<>(); + + public PathMatchingHttpServletRequestWrapper(HttpServletRequest request) { + super(request); + } + + @Override + public Object getAttribute(String name) { + final Object ret = localAttributes.get(name); + if (ret == null) { + return super.getAttribute(name); + } + return ret; + } + + @Override + public void setAttribute(String name, Object o) { + localAttributes.put(name, o); + } + + @Override + public void removeAttribute(String name) { + localAttributes.remove(name); + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ResourceNameFilterMappingAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ResourceNameFilterMappingAdvice.java new file mode 100644 index 00000000000..7794aa0ac15 --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ResourceNameFilterMappingAdvice.java @@ -0,0 +1,26 @@ +package datadog.trace.instrumentation.springweb6; + +import java.util.List; +import net.bytebuddy.asm.Advice; +import org.springframework.context.ApplicationContext; +import org.springframework.web.servlet.HandlerMapping; + +/** + * Wires the {@code DispatcherServlet} handler mappings into the (opt-in) {@link + * HandlerMappingResourceNameFilter} after the context refreshes, so the filter can evaluate them at + * the start of the filter chain. Only active when the {@code spring-path-filter} integration is + * enabled (see {@code ResourceNameFilterMappingInstrumentation}). + */ +public class ResourceNameFilterMappingAdvice { + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void afterRefresh( + @Advice.Argument(0) final ApplicationContext springCtx, + @Advice.FieldValue("handlerMappings") final List handlerMappings) { + if (handlerMappings != null && springCtx.containsBean("ddDispatcherFilter")) { + final HandlerMappingResourceNameFilter filter = + (HandlerMappingResourceNameFilter) springCtx.getBean("ddDispatcherFilter"); + filter.setHandlerMappings(handlerMappings); + } + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ServletPathFilterInjectingAdvice.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ServletPathFilterInjectingAdvice.java new file mode 100644 index 00000000000..83d1afed091 --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ServletPathFilterInjectingAdvice.java @@ -0,0 +1,19 @@ +package datadog.trace.instrumentation.springweb6; + +import net.bytebuddy.asm.Advice; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; + +public class ServletPathFilterInjectingAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.Argument(0) final ConfigurableListableBeanFactory beanFactory) { + if (beanFactory instanceof BeanDefinitionRegistry + && !beanFactory.containsBean("servletPathRequestFilter")) { + + ((BeanDefinitionRegistry) beanFactory) + .registerBeanDefinition( + "servletPathRequestFilter", new OrderedServletPathRequestFilter.BeanDefinition()); + } + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy index abb1b322214..7bf01c8905f 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy @@ -517,3 +517,16 @@ class SpringBootRumInjectionForkedTest extends SpringBootBasedTest { true } } + +/** + * Runs the full suite with the opt-in legacy route-naming filter enabled, to verify that path still + * produces the same traces (including APMS-8174). The default path (getHandler + ControllerAdvice) + * stays active; the DD_FILTERED_SPRING_ROUTE_ALREADY_APPLIED guard prevents double naming. + */ +class SpringBootResourceNameFilterForkedTest extends SpringBootBasedTest { + @Override + protected void configurePreAgent() { + super.configurePreAgent() + injectSysConfig("dd.integration.spring-path-filter.enabled", "true") + } +}