The upgrade to Micrometer 1.0 RC 6 included this change that's derived from this change for https://github.com/micrometer-metrics/micrometer/pull/264. The intent was to avoid recording a 200 response for a request that's resulted in an exception being thrown and a 500 response being sent. However, the effect is that a 500 response is now recorded irrespective of how the exception is handled and the response status that is actually sent.
A concrete example of the problem is when Spring Security's method security throws an AccessDeniedException. WebMvcMetricsFilter catches the resulting NestedServletException and records a 500 response. Spring Security's ExceptionTranslationFilter then catches the exception, finds the AccessDeniedException, and, via org.springframework.security.web.access.AccessDeniedHandlerImpl, sets the response status to 403. This results in the client receiving a 403 but Micrometer recording at 500.
/cc @jkschneider
Comment From: jkschneider
cc / @adriancole
We had talked once before about moving instrumentation deeper into server-specific bindings to more accurately record the status.
Comment From: wilkinsona
Thanks, @jkschneider. I recalled you mentioning that in the past when I was opening this issue. An alternative might be to change the filter's order so that it goes first on the way in and last on the way back out. Another would be to split the filter into two, with one that has highest precedence and one that has lowest and both talking to a shared component that does the actual metric recording. Can you refresh my memory on the reasoning for the current ordering? One side-effect that it has beyond this issue is that no metrics are recorded at all for requests that are rejected by Spring Security's filter.
Comment From: jkschneider
@wilkinsona You answered your own question here at the end:
Can you refresh my memory on the reasoning for the current ordering? One side-effect that it has beyond this issue is that no metrics are recorded at all for requests that are rejected by Spring Security's filter.
It was precisely to be able to see requests rejected by Spring security that we made metrics a higher priority filter.
An alternative might be to change the filter's order so that it goes first on the way in and last on the way back out.
If this is possible, let's do this. It's always possible that something server-specific outside of the servlet framework is changing a response code, but probably very rarely. This would get us to near perfection.
Comment From: somayaj
figures the WebMvcMetricsFilter is setting the 500 error on ln: 104
catch (NestedServletException ex) {
response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR.value());
record(timingContext, request, response, ex.getCause());
throw ex;
PS: Can I work this PR?
Comment From: cantaylancapraz
Hi,
Any update about this issue?
Comment From: bclozel
WebMvcMetricsFilter has been removed in 3.0 and the 2.7.x generation is not supported anymore. I'm closing this issue as a result.