diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/v3_1/OpenTelemetryHandlerMappingFilter.java b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/v3_1/OpenTelemetryHandlerMappingFilter.java index 214f1a568e24..fae2f33b9d9c 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/v3_1/OpenTelemetryHandlerMappingFilter.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/v3_1/OpenTelemetryHandlerMappingFilter.java @@ -16,7 +16,9 @@ import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import javax.annotation.Nullable; import javax.servlet.Filter; @@ -26,6 +28,7 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; import org.springframework.core.Ordered; import org.springframework.web.servlet.HandlerExecutionChain; @@ -33,34 +36,21 @@ import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; public class OpenTelemetryHandlerMappingFilter implements Filter, Ordered { - private static final String PATH_ATTRIBUTE = getRequestPathAttribute(); private static final MethodHandle usesPathPatternsMh = getUsesPathPatternsMh(); private static final MethodHandle parseAndCacheMh = parseAndCacheMh(); private final HttpServerRouteGetter serverSpanName = (context, request) -> { - Object previousValue = null; - if (this.parseRequestPath && PATH_ATTRIBUTE != null) { - previousValue = request.getAttribute(PATH_ATTRIBUTE); + if (this.parseRequestPath) { // sets new value for PATH_ATTRIBUTE of request parseAndCache(request); } - try { - if (findMapping(request)) { - // Name the parent span based on the matching pattern - // Let the parent span resource name be set with the attribute set in findMapping. - return SpringWebMvcServerSpanNaming.SERVER_SPAN_NAME.get(context, request); - } - } finally { - // mimic spring DispatcherServlet and restore the previous value of PATH_ATTRIBUTE - if (this.parseRequestPath && PATH_ATTRIBUTE != null) { - if (previousValue == null) { - request.removeAttribute(PATH_ATTRIBUTE); - } else { - request.setAttribute(PATH_ATTRIBUTE, previousValue); - } - } + if (findMapping(request)) { + // Name the parent span based on the matching pattern + // Let the parent span resource name be set with the attribute set in findMapping. + return SpringWebMvcServerSpanNaming.SERVER_SPAN_NAME.get(context, request); } + return null; }; @@ -84,7 +74,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha } finally { if (handlerMappings != null) { Context context = Context.current(); - HttpServerRoute.update(context, CONTROLLER, serverSpanName, (HttpServletRequest) request); + HttpServerRoute.update(context, CONTROLLER, serverSpanName, prepareRequest(request)); } } } @@ -92,6 +82,31 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha @Override public void destroy() {} + private static HttpServletRequest prepareRequest(ServletRequest request) { + // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/10379 + // Finding the request handler modifies request attributes. We are wrapping the request to avoid + // this. + return new HttpServletRequestWrapper((HttpServletRequest) request) { + private final Map attributes = new HashMap<>(); + + @Override + public void setAttribute(String name, Object o) { + attributes.put(name, o); + } + + @Override + public Object getAttribute(String name) { + Object value = attributes.get(name); + return value != null ? value : super.getAttribute(name); + } + + @Override + public void removeAttribute(String name) { + attributes.remove(name); + } + }; + } + /** * When a HandlerMapping matches a request, it sets HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE * as an attribute on the request. This attribute is read by SpringWebMvcDecorator.onRequest and @@ -186,19 +201,4 @@ private static void parseAndCache(HttpServletRequest request) { throw new IllegalStateException(throwable); } } - - private static String getRequestPathAttribute() { - try { - Class pathUtilsClass = - Class.forName("org.springframework.web.util.ServletRequestPathUtils"); - return (String) - MethodHandles.lookup() - .findStaticGetter(pathUtilsClass, "PATH_ATTRIBUTE", String.class) - .invoke(); - } catch (ClassNotFoundException | NoSuchFieldException | IllegalAccessException exception) { - return null; - } catch (Throwable throwable) { - throw new IllegalStateException(throwable); - } - } } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/org/springframework/web/servlet/v6_0/OpenTelemetryHandlerMappingFilter.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/org/springframework/web/servlet/v6_0/OpenTelemetryHandlerMappingFilter.java index 4df6f3d8ff3c..32a8782be2da 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/org/springframework/web/servlet/v6_0/OpenTelemetryHandlerMappingFilter.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/org/springframework/web/servlet/v6_0/OpenTelemetryHandlerMappingFilter.java @@ -18,14 +18,16 @@ import jakarta.servlet.ServletRequest; import jakarta.servlet.ServletResponse; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import javax.annotation.Nullable; import org.springframework.core.Ordered; -import org.springframework.http.server.RequestPath; import org.springframework.web.servlet.HandlerExecutionChain; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; @@ -35,25 +37,16 @@ public class OpenTelemetryHandlerMappingFilter implements Filter, Ordered { private final HttpServerRouteGetter serverSpanName = (context, request) -> { - RequestPath previousValue = null; if (this.parseRequestPath) { - previousValue = - (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE); // sets new value for PATH_ATTRIBUTE of request ServletRequestPathUtils.parseAndCache(request); } - try { - if (findMapping(request)) { - // Name the parent span based on the matching pattern - // Let the parent span resource name be set with the attribute set in findMapping. - return SpringWebMvcServerSpanNaming.SERVER_SPAN_NAME.get(context, request); - } - } finally { - // mimic spring DispatcherServlet and restore the previous value of PATH_ATTRIBUTE - if (this.parseRequestPath) { - ServletRequestPathUtils.setParsedRequestPath(previousValue, request); - } + if (findMapping(request)) { + // Name the parent span based on the matching pattern + // Let the parent span resource name be set with the attribute set in findMapping. + return SpringWebMvcServerSpanNaming.SERVER_SPAN_NAME.get(context, request); } + return null; }; @@ -77,7 +70,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha } finally { if (handlerMappings != null) { Context context = Context.current(); - HttpServerRoute.update(context, CONTROLLER, serverSpanName, (HttpServletRequest) request); + HttpServerRoute.update(context, CONTROLLER, serverSpanName, prepareRequest(request)); } } } @@ -85,6 +78,31 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha @Override public void destroy() {} + private static HttpServletRequest prepareRequest(ServletRequest request) { + // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/10379 + // Finding the request handler modifies request attributes. We are wrapping the request to avoid + // this. + return new HttpServletRequestWrapper((HttpServletRequest) request) { + private final Map attributes = new HashMap<>(); + + @Override + public void setAttribute(String name, Object o) { + attributes.put(name, o); + } + + @Override + public Object getAttribute(String name) { + Object value = attributes.get(name); + return value != null ? value : super.getAttribute(name); + } + + @Override + public void removeAttribute(String name) { + attributes.remove(name); + } + }; + } + /** * When a HandlerMapping matches a request, it sets HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE * as an attribute on the request. This attribute is read by SpringWebMvcDecorator.onRequest and