Skip to content

Client request observation contributes full URI template to uri meter tag values #29885

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

Closed
edwardsre opened this issue Jan 25, 2023 · 5 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) theme: observability An issue related to observability and tracing type: bug A general bug
Milestone

Comments

@edwardsre
Copy link

After upgrading to Spring Boot 3, we are seeing http.client.requests metrics with uri tags containing the scheme, host and port. This is a deviation from Spring Boot 2 and is causing problems with metric publication.

In Spring Boot 2.7, http.client.requests uri tag values were stripped of the scheme, host and port. See the discussion in this issue and the associated fix to align WebClient and RestTemplate uri tags. This test class shows a stripped URI tag value when using a full URI as the template.

Calling RestTemplate like this:

restTemplate.exchange("http://cis-public-api.cis.ident.service/v4/init?key={devKey}", HttpMethod.GET, ...);

yields uri tags in the Spring Boot 3.0.2 actuator/metrics/http.client.requests endpoint looking like this:

{
  "tag": "uri",
  "values": [
    "http://cis-public-api.cis.ident.service/v4/init?key={devKey}",
  ]
}

Results in Spring Boot 2.7 making the same call:

{
  "tag": "uri",
  "values": [
    "/v4/init?key={devKey}",
  ]
}

Observation: In Spring Boot 3, when setting a baseUrl in RestTemplate and using /v4/init?key={devKey} as the uriTemplate in the exchange call, the uri tag value looks the same as it did in Spring Boot 2.7.

As a side note, but related to this change, we use the Datadog StatsD flavor to publish metrics to Splunk. These full URI tag values no longer show up in Splunk metrics, but uri tag values without the scheme, host and port are showing up as expected.

I have verified the uri tag values are included in statsd line data for both scenarios, so somehow, the extra characters are causing problems (perhaps the extra : between the scheme and host, since the StatsD line format uses colons as delimiters).

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 25, 2023
@bclozel bclozel self-assigned this Jan 25, 2023
@bclozel bclozel added the theme: observability An issue related to observability and tracing label Jan 25, 2023
@bclozel bclozel transferred this issue from spring-projects/spring-boot Jan 25, 2023
@bclozel bclozel added this to the 6.0.5 milestone Jan 27, 2023
@bclozel bclozel added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 27, 2023
@bclozel
Copy link
Member

bclozel commented Jan 30, 2023

Thanks a lot for this report @edwardsre ! This will be fixed in the next maintenance version.

wilkinsona added a commit to spring-projects/spring-boot that referenced this issue Feb 15, 2023
@HaloFour
Copy link
Contributor

I hate to comment on a 1.5 year old closed issue, but was this problem identified/fixed with WebClient as well? Currently trying to port a service to Spring Boot 3.0.13 with Spring Framework 6.0.14 (stepping stone to later versions) and I noticed that the uri tag reported on http.client.requests metrics was including the full URI and not the URI template.

I stepped through and found that ClientObservationConventionAdapter#getLowCardinalityKeyValues is trying to set the request attribute to ClientRequestObservationContext#getUriTemplate before ClientRequestObservationContext#setUriTemplate is called, which is resetting the attribute back to null.

@bclozel
Copy link
Member

bclozel commented Apr 12, 2024

@HaloFour I'm sorry but this comment isn't really actionable on my side.

I've just tried the following and the test is green:

WebClient webClient = WebClient.builder().observationRegistry(observationRegistry).build();
webClient.get().uri("https://httpbin.org/{method}", "get").retrieve()
		.bodyToMono(Void.class).block(Duration.ofSeconds(10));
assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS")
		.hasLowCardinalityKeyValue("uri", "/{method}");

The commit listed right above applied the change to the webclient convention as well, see 5dfa61e. Can you create a new issue with a minimal sample application that shows the problem?

@HaloFour
Copy link
Contributor

@bclozel

It happens specifically if you add org.springframework.boot.actuate.autoconfigure.observation.web.client.ClientObservationConventionAdapter as a convention to the WebClient. I can try to throw together a repo later today.

This class has already been removed from I believe Spring Boot 3.2, and I've worked around the problem by adding my own (temporary) convention to adapt WebClientExchangeTagsProvider which doesn't attempt to overwrite the URI template attribute on the request.

@HaloFour
Copy link
Contributor

@bclozel

I have opened a new issue with a repro here: spring-projects/spring-boot#40330

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) theme: observability An issue related to observability and tracing type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants