Exceptions caught by @ExceptionHandler are not recorded by MetricsWebFilter

[s-volkov-1]

Dependencies

• spring-boot 2.3.4
• webflux
• actuator
• micrometer 1.5.4
• kotlin coroutines-reactor 1.3.9

Task

Observe exceptions which led to 4xx/5xx server responses, with a help of http.server.requests metric.

Setup

• Annotated @RestController with @RequestMapping suspend functions
• Annotated @RestControllerAdvice with multiple @ExceptionHandler(Exception::class) regular functions

Expected behavior

On each exception caught and processed by @ExceptionHandler(Exception::class), e.g. UserNotFoundException, it should appear in /actuator/prometheus output, e.g.
http_server_requests_seconds_count{exception="UserNotFoundException",method="POST",outcome="CLIENT_ERROR",status="400",uri="/v1/users",} 1.0
This is the way it works in spring-boot-starter-web.

Actual behavior, problem

However, this refuses to work as expected with webflux. I used same annotated components.
http_server_requests_seconds_count{exception="None",method="POST",outcome="CLIENT_ERROR",status="400",uri="/v1/users",} 1.0
Notice: exception this time is "None".
In this case there is a call to MetricsWebFilter.onSuccess(...), which does not have exception parameter.
Desired exception tag is filled only on exceptions uncaught by @RestControllerAdvice. This may seem like a reasonable thing to do, but it is unexpected compared to web-mvc.

[bclozel]

Right now we're recording an "exception" tag for handled errors with Spring MVC, but only in some cases. I've tracked this behavior back to 25815ca (and probably earlier in some other form).

We're fetching the handled exception from a particular request attribute.
There are several issues with this approach in general:

• if the @ExceptionHandler returns a view name, the exception tag is not set since the request attribute is not present
• if the application is using MVC functional endpoints with error handling as a filter in the RouterFunction or in the handler function directly, the exception tag is not set since the request attribute is not present
• if the exception handler uses a 2xx response status, we'll tag responses with exceptions for non-error cases.

I don't know if we can fix those inconsistencies at the Spring Boot / Spring Framework level.

Given the current WebFlux architecture, it might be challenging to apply the same approach, even with the same limitations.

With that in mind, I'm wondering if we should only record the exception tag information for errors that acually bubble up to the Spring Boot error handling infrastructure.

[bclozel]

After discussing that point with the team, the team has decided to remove this behavior in a future version of Spring Boot (2.6.x at the earliest, targeted for now for 3.x).

In general, we feel that recording that tag is in many cases not useful (we're recording exceptions that are considered as "normal" behavior for the app, for example a missing entry in the database leading to a 404) or inconsistent (see my previous comment). We still believe this behavior is useful, but on an opt-in basis by the application. This will be taken care of in #24028, well before we change this behavior for all developers.

[bclozel]

First, we need to take care of #24028 and wait until we're in a position to make important changes in the metrics + error handling space.

At that point, we can amend the WebMvcMetricsFilter to not look at the org.springframework.web.servlet.DispatcherServlet#EXCEPTION_ATTRIBUTE for tagging purposes. As described by the request attribute in Spring Framework, its goal is to save the information for exceptions handled by HandlerExceptionResolver (only when no views are not rendered). This should align the filter behavior with its WebFlux counterpart.

We should then thoroughly review the various exception/error handling mechanisms in Spring Framework and Spring Boot to make sure that we have a consistent support. #19525 and a broader revision of error handling might be a prerequisite to this change if we want to empower developers.

[s-volkov-1]

Thanks for detailed explanation.

I'd like to share personal experience about following part

In general, we feel that recording that tag is in many cases not useful (we're recording exceptions that are considered as "normal" behavior for the app, for example a missing entry in the database leading to a 404) or ...

• in case of any error (expected or not) our applications must respond with specifically formatted json. This may be a common pattern for backends, I believe. That's why we have @ExceptionHandler for everything, including Throwable, as unexpected case.
• even if we face expected errors, it's very useful to have them reported in metrics for monitoring/alerting purposes (helps to quickly narrow down cause, to some extent, without using kibana/sentry/etc.)
• truly unexpected exceptions, potentially, may (but must never) occur in @ExceptionHandler methods themselves

[bclozel]

I'm closing this issue as it's now superseded by the observability efforts in Spring Framework directly, especially with spring-projects/spring-framework#28880.

The filter implementation in Framework will not use DispatcherServlet#EXCEPTION_ATTRIBUTE; instead, applications should rely on the new Problem details support in Spring Framework. For non-REST calls, it will be possible to fetch the observability context as a request attribute and set the error on it directly.