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

No transactionId anymore in implementation classes of ErrorWebExceptionHandler #42600

Closed
sigriswil opened this issue Oct 11, 2024 · 2 comments
Closed
Labels
for: external-project For an external project and not something we can fix

Comments

@sigriswil
Copy link

sigriswil commented Oct 11, 2024

Spring boot version: 3.3.2
Spring cloud dependencies version: 2023.0.3
sample issue spring cloud gateway repository: https://github.com/sigriswil/spring-gateway-sample

It contains a SpringBoot application that can be started locally. When reaching the endpoint /, it raises an exception which triggers the class GlobalErrorWebExceptionHandler which implements ErrorWebExceptionHandler.

In the main method, Hooks.enableAutomaticContextPropagation() and ContextRegistry.getInstance().registerThreadLocalAccessor(...) are called in FirstGatewayFilter.

Add the MDC's trasactionId to the FirstGatewayFilter, An exception occurs in the SecondGatewayFilterFactory.
transactionId is output from SecondGatewayFilterFactory and FirstGatewayFilter, but transactionId is not output from GlobalErrorWebExceptionHandler.

INFO 48248 --- c.e.s.FirstGatewayFilter: transactionId created. MDC: {transactionId=2e58da57-2f01-4d29-b63d-61b2e9682301}
INFO 48248 --- c.e.s.SecondGatewayFilterFactory: MDC: {transactionId=2e58da57-2f01-4d29-b63d-61b2e9682301}
ERROR 48248 --- c.e.s.GlobalErrorWebExceptionHandler: MDC: null
...
INFO 48248 --- c.e.s.FirstGatewayFilter: doFinally, MDC: {transactionId=2e58da57-2f01-4d29-b63d-61b2e9682301}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 11, 2024
@philwebb
Copy link
Member

philwebb commented Oct 15, 2024

I think someone from the Micrometer, Reactor or Spring Cloud team will need to help with this issue. Spring Boot isn't really involved at this level.

It looks to me like io.micrometer.context.DefaultContextSnapshotFactory.setAllThreadLocalsFrom is clearing the MDC thread local.

@spencergibb @chemicL are you able to help identify what's causing the issue and which project can help?

@philwebb philwebb closed this as not planned Won't fix, can't repro, duplicate, stale Oct 15, 2024
@philwebb philwebb added for: external-project For an external project and not something we can fix and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 15, 2024
@chemicL
Copy link
Member

chemicL commented Oct 16, 2024

I have no knowledge about Spring Cloud Gateway's lifecycle with the AbstractGatewayFilterFactory but this situation where there is a global instance of ErrorWebExceptionHandler sounds similar to the case encountered with Spring Framework where observability was added as a WebFilter which was at a more internal layer than the catch-all ExceptionHandlingWebHandler which is outside (check related discussion and fix). So MDC keys set in one AbstractGatewayFilterFactory (FirstGatewayFilterFactory) won't be visible to any filter that is ordered prior to it. I am certain @spencergibb is able to provide guidance :)

Regarding the comment of clearing thread locals: If these MDC entries were not cleared by the context-propagation then you'd face leaks from one request to another. The solution to the problem is proper ordering of call scopes and making sure things are properly cleared when exiting a scope and handling something unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix
Projects
None yet
Development

No branches or pull requests

4 participants