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.

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

Spring WebMVC instrumentation alters HTTP Response code in certain scenarios #10379

Closed
kenfinnigan opened this issue Feb 1, 2024 · 0 comments · Fixed by #10389
Closed

Spring WebMVC instrumentation alters HTTP Response code in certain scenarios #10379

kenfinnigan opened this issue Feb 1, 2024 · 0 comments · Fixed by #10389
Assignees
Labels
bug Something isn't working needs triage New issue that requires triage

Comments

@kenfinnigan
Copy link
Member

Describe the bug

When a Spring Boot application has an endpoint defined that produces a response with an application/pdf content type, the service returning an error results in the response code being modified due to a failure in Spring.

My example is an application that returns a 403 response if the user is forbidden from accessing the endpoint, but when calling the API with the agent present the response becomes 406.

Steps to reproduce

I have a simple project which reproduces the failure: https://github.com/kenfinnigan/otel-spring-reproducer

Package the project with mvn clean package and call the endpoint with curl http://localhost:8080/v4/check/87238923/report -I

Expected behavior

HTTP response code of 403 is returned, as the user is forbidden from accessing the resource

Actual behavior

HTTP response code of 406 is returned, and org.springframework.web.HttpMediaTypeNotAcceptableException: Could not find acceptable representation is logged in the server

Javaagent or library instrumentation version

v1.3.1

Environment

JDK: 8
OS: MacOS

Additional context

I investigated the problem and have identified the root cause.

In OpenTelemetryHandlerMappingFilter.findMapping(), here, getHandler is called. Deep inside the getHandler call, RequestMappingInfoHandlerMapping.handleMatch(), here is called.

The RequestMappingInfoHandlerMapping.handleMatch() method sets several attributes onto the request, including org.springframework.web.servlet.HandlerMapping.producibleMediaTypes. Setting this attribute on the request, causes the error handling to fail as producibleMediaTypes is set to application/pdf from the request mapping. This content type does not match the JSON error response, resulting in an HTTP Response code of 406 being returned and an exception being logged.

Without the agent present, org.springframework.web.servlet.HandlerMapping.producibleMediaTypes is empty when processing the error response, allowing for the default application/json types to be used.

One of the attributes set on the request as part of RequestMappingInfoHandlerMapping.handleMatch() is used by SpringWebMvcServerSpanNaming here.

I'm happy to help with a solution

@kenfinnigan kenfinnigan added bug Something isn't working needs triage New issue that requires triage labels Feb 1, 2024
@laurit laurit self-assigned this Feb 2, 2024
kenfinnigan added a commit to lumigo-io/opentelemetry-java-distro that referenced this issue Feb 2, 2024
Incorporate the upstream fix to open-telemetry/opentelemetry-java-instrumentation#10379
by customizing the instrumentation as a temporary measure until the fix is merged and released upstream,
and this distro updates to that version

Signed-off-by: Ken Finnigan <ken@kenfinnigan.me>
kenfinnigan added a commit to lumigo-io/opentelemetry-java-distro that referenced this issue Feb 2, 2024
Incorporate the upstream fix to open-telemetry/opentelemetry-java-instrumentation#10379
by customizing the instrumentation as a temporary measure until the fix is merged and released upstream,
and this distro updates to that version

Signed-off-by: Ken Finnigan <ken@kenfinnigan.me>
kenfinnigan added a commit to lumigo-io/opentelemetry-java-distro that referenced this issue Feb 2, 2024
…bug (#279)

Incorporate the upstream fix to open-telemetry/opentelemetry-java-instrumentation#10379
by customizing the instrumentation as a temporary measure until the fix is merged and released upstream,
and this distro updates to that version

Signed-off-by: Ken Finnigan <ken@kenfinnigan.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs triage New issue that requires triage
Projects
None yet
2 participants