Align WebClient uri metric tag with RestTemplate's

[nhmarujo]

I think that http.client.requests metric has a wrong behaviour with query parameters. The query parameters should not be considered as part of the uri tag.

If you have 2 calls to http://test123.io/api and http://test123.io/api?q=1 I wouldn't expect them to be counted as different uris. Not only I think it is conceptually wrong, but also there is no way to group them later.

On top of that I can observe some inconsistencies on the result you get by using different approaches with either RestTemplate and WebClient.

Some examples:

1. Will produce the tag uri="/api/{pathParam}?queryParam={queryParam}"

webClient.get()
    .uri("http://test123.io/api/{pathParam}?queryParam={queryParam}", "foobar", "xpto")
    .retrieve()
    .toEntity(Void.class)
    .block();

2. Will produce the tag uri="/test123.io/api/foobar"

webClient.get()
    .uri(uriBuilder -> uriBuilder.path("http://test123.io/api/{pathParam}")
        .queryParam("queryParam", "xpto")
        .build("foobar"))
    .retrieve()
    .toEntity(Void.class)
    .block();

3. Will produce the tag uri="/api/foobar"

webClient.get()
    .uri(uriBuilder -> uriBuilder.host("http://test123.io")
        .path("/api/{pathParam}")
        .queryParam("queryParam", "xpto")
        .build("foobar"))
    .retrieve()
    .toEntity(Void.class)
    .block();

4. Will produce the tag uri="/api/{pathParam}?queryParam={queryParam}"

restTemplate.getForObject("http://test123.io/api/{pathParam}?queryParam={queryParam}", Void.class, "foobar", "xpto");

5. Will produce the tag uri="/api/foobar?queryParam=xpto"

URI uri=UriComponentsBuilder.fromHttpUrl("http://test123.io/api/{pathParam}")
    .queryParam("queryParam", "xpto")
    .build("foobar");

restTemplate.getForObject(uri, Void.class);

In any of these cases I believe the correct value, for the tag, would be uri="/api/{pathParam}"

This was all tested using SB2.3.2 using the spring-boot-starter-webflux and spring-boot-starter-web.

I believe this is related to what is reported on this issue spring-projects/spring-framework#22371 , which was closed and marked as superseded, but references itself.

[wilkinsona]

5 is working as expected. From RestTemplate's perspective, there's no variable substitution being performed as you've passed in a URI where the substitution has already taken place. 2 and 3 are quite similar, although I wonder if we may be able to do something there as there isn't quite the same degree of separation between the client and the URI creation.

For 2 and 3, the URI template can be captured and the URIBuilder still used if you call uri(String, Function<UriBuilder, URI>) rather than the uri(Function<UriBuilder, URI>) which you're currently calling.

[nhmarujo]

Well I see your point regarding 5.. I was not sure about how URI handles the substitution and thought that those could be extracted from it separately. That said, I still would expect query parameters to be trimmed off.

[wilkinsona]

I still would expect query parameters to be trimmed off

You said above that "it is conceptually wrong", but did not explain why. Could you please do so? Query parameters are part of the URI so it feels correct to me that they should be included in a URI tag when available.

[nhmarujo]

We are just talking about metrics here and in that scope I think it makes no sense. I did said "there is no way to group them later", but let me explain better 😄

Imagine you are monitoring an app and want to know, for instance, which endpoints have most calls. Now imagine you have an endpoint that can take like 5 optional query parameters. This means you will have 1 different uri value for each possible combination of those optional query parameters (C_(5,1) + C_(5,2) + C_(5,3) + C_(5,4) + C_(5,5) = 31 combinations). You will not be able to sum (or any other type of aggregation) those metrics because the uri tag is different, but in fact is a call to the same endpoint.

[nhmarujo]

You may say that you may want to know about the query parameters for the metrics in some cases. And though I will agree with you, I think you could put the query parameters part in another tag and that would cover those cases too.

[wilkinsona]

That doesn't sound conceptually wrong to me, rather it sounds like a situation where it isn't possible to provide default behaviour that meets everyone's needs. Some will want the query parameters in the URI tag, others like you won't. If the default tagging behaviour doesn't meet your needs, you can provide a WebClientExchangeTagsProvider or a RestTemplateExchangeTagsProvider bean to take control.

[nhmarujo]

Well, if it is provided all in one tag, it will serve one group of people. But if it is provided separately, it will serve both groups :)

So I think the default behaviour should be the one that satisfies the majority (and in this case, the second approach actually serves the purpose of the first too, so there is no downside to it).

I still think it is conceptually wrong in the scope of metrics tagging.