Spring-boot: WebMvgTags.uri uses NOT_FOUND for a matched request mapping that produces a 404 response

Created on 21 Mar 2018  路  10Comments  路  Source: spring-projects/spring-boot

Greetings,

I'm migrating my project from Boot 1.5.x to 2.0.0.RELEASE and to micrometer in the process.
I'm a bit annoyed with WebMvcTags' default behavior regarding exceptions and URI tracking for a WebMVC REST API application :

    public static Tag uri(HttpServletRequest request, HttpServletResponse response) {
        if (response != null) {
            HttpStatus status = extractStatus(response);
            if (status != null && status.is3xxRedirection()) {
                return Tag.of("uri", "REDIRECTION");
            }
            if (status != null && status.equals(HttpStatus.NOT_FOUND)) {
                return Tag.of("uri", "NOT_FOUND");
            }
        }
        if (request == null) {
            return Tag.of("uri", "UNKNOWN");
        }
        String uri = getUri(request);
        uri = uri.replaceAll("//+", "/").replaceAll("/$", "");
        return Tag.of("uri", uri.isEmpty() ? "root" : uri);
    }

This implies that All 404 errors won't have any specific URI information, being grouped in that NOT_FOUND uri.
This does effectly prevents anything relevant from being retrieved of those metrics.

It seems to me that using relevant http codes for functional returns is a good practice, but this default tag provider harms this.

Was the reason backing this implementation to avoid creating meaningless metrics entries for each and every false / crawled URI ? It could be worth it checking wether the initial URI did match a correct route, and in that case, use the route's mapping as uri.

Thanks for any hint / answer

bug

Most helpful comment

We can't recommend keeping the user-supplied URI for 404 conditions

@jkschneider what about using the HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE when the request matched a request mapping, but the user's controller produced a response entity with a 404 status?

All 10 comments

Ok I've just stumbled upon the relevant documentation and the RestTemplateExchangeTagsProvider / RestTemplateExchangeTags ...

My bad !

Sorry for the open/close... it's somehow still an issue :
As is, both the RestTemplateMetricsAutoConfiguration and WebMvcMetricsAutoConfiguration do instantiate their TagsProvider

    @Bean
    @ConditionalOnMissingBean(WebMvcTagsProvider.class)
    public DefaultWebMvcTagsProvider webMvcTagsProvider() {
        return new DefaultWebMvcTagsProvider();
    }

vs

    @Bean
    @ConditionalOnMissingBean(RestTemplateExchangeTagsProvider.class)
    public DefaultRestTemplateExchangeTagsProvider restTemplateTagConfigurer() {
        return new DefaultRestTemplateExchangeTagsProvider();
    }

And eventualy in my case (a full rest template app) the WebMvcTagsProvider ends up being use instead of the DefaultRestTemplateExchangeTagsProvider .

A configuration somehow does not inject the correct customizer. As far as I can see metricsRestTemplateCustomizeris being injected the correct RestTemplateExchangeTagsProvider

@jceloi Your latest comment has left me confused. The WebMvcTagsProvider is used to provide tags for metrics that are recorded on the server side during Spring MVC request handling. The RestTemplateExchangeTagsProvider is used to provide tags for metrics that are recorded on the client side when sending a request using RestTemplate.

I think that I was even more confused than my comment, my bad. That comment and analytsis is totally irrelevant.

Yet the very first of the issue is still relevant : the WebMvcTags.uri's behavior override the endpoint URI for a 404 error with a "not_found" value, preventing each 404's metric for each endopoint to be separated.

the WebMvcTags.uri's behavior override the endpoint URI for a 404 error with a "not_found" value, preventing each 404's metric for each endopoint to be separated

I believe this is intentional to prevent cardinality from exploding, which would create a vector for a DoS-type attack on your metrics system by requesting random routes.

I suppose for routes that are mapped but may return 404, it would be nice to have metrics on their returning 404, assuming they conditionally return 404 or successful status codes.

/cc @jkschneider

@jceloi The tags are sourced by the WebMvcTagsProvider interface. WebMvcTags just provides static builders for common tag values. You don't have to use WebMvcTags#uri in your construction of the uri tag.

Like @shakuzen suggested, mapping the user-supplied URI that resulted in a 404 can lead to an unbounded number of unique tag values -- therefore time series in your monitoring system.

We can't recommend keeping the user-supplied URI for 404 conditions, but you are free to do so if you must with your own implementation of WebMvcTagsProvider.

Thank you @shakuzen and @jkschneider for your feedback. I understand that explanation (and I was supposing that it was the reason why it was the default behaviour).

I do agree with you @shakuzen , I think that by default for known, mapped routes keeping the uri would be interesting. I'll do something like that using the best matched route as an URI (similar to what the RestTemplateExchange tags use and which seems very relevant).

We can't recommend keeping the user-supplied URI for 404 conditions

@jkschneider what about using the HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE when the request matched a request mapping, but the user's controller produced a response entity with a 404 status?

@wilkinsona That is certainly an improvement.

Thanks for the fix @bclozel !

Was this page helpful?
0 / 5 - 0 ratings