Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Wrap request to avoid modifying attributes of the original request #10389

Merged
merged 2 commits into from
Feb 6, 2024
Merged

Wrap request to avoid modifying attributes of the original request #10389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ead7de
Wrap request to avoid modifying attributes of the original request
laurit Feb 2, 2024
5824886
remove comment
laurit Feb 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 35 additions & 35 deletions ...src/main/java/org/springframework/web/servlet/v3_1/OpenTelemetryHandlerMappingFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -26,41 +28,29 @@
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;
import org.springframework.web.servlet.HandlerMapping;
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<HttpServletRequest> 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;
};

Expand All @@ -84,14 +74,39 @@ 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));
}
}
}

@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<String, Object> 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
Expand Down Expand Up @@ -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);
}
}
}
50 changes: 34 additions & 16 deletions ...src/main/java/org/springframework/web/servlet/v6_0/OpenTelemetryHandlerMappingFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,25 +37,16 @@ public class OpenTelemetryHandlerMappingFilter implements Filter, Ordered {

private final HttpServerRouteGetter<HttpServletRequest> 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;
};

Expand All @@ -77,14 +70,39 @@ 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));
}
}
}

@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<String, Object> 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
Expand Down