Skip to content
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

Inconsistent http.route tracing attribute value between RestTemplate & WebClient since 6.1.2 #32202

Closed
wleese opened this issue Feb 6, 2024 · 4 comments
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply theme: observability An issue related to observability and tracing

Comments

@wleese
Copy link

wleese commented Feb 6, 2024

In 2 tests, one using restTemplate and the other webClient, the tracing attribute http.route has differing values.

        var restTemplate = restTemplateBuilder.rootUri("http://localhost:" + port).build();
        var response = restTemplate.getForEntity("/foo/{foo-id}?query=true", String.class, "bar");

       // assert that org.springframework.http.client.observation.ClientRequestObservationContext.getUriTemplate() is http://localhost:1234/foo/{foo-id}

ClientRequestObservationContext

        var webClient = webClientBuilder.baseUrl("http://localhost:" + port).build();
        StepVerifier.create(webClient
                .get().uri("/foo/{foo-id}?query=true", "bar").attributes()
                .retrieve()
                .bodyToMono(String.class))
                .expectNext("bar?query=true")
                .verifyComplete();

       // assert that org.springframework.web.reactive.function.client.ClientRequestObservationContext.getUriTemplate() is /foo/{foo-id}

This not only requires multiple test paths (webmvc leads to a different value than webflux), but also leads to an inconsistent experience when using tracing data (raw data, or using a UI). When using tracing data, one now has to consider if webClient or restTemplate was used, which may be unreasonable if not familiar with the relevant application(s).

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 6, 2024
@bclozel
Copy link
Member

bclozel commented Feb 6, 2024

Spring framework 6.3.x does not exist. Can you elaborate in which version this behavior appeared and how it was behaving before?

@bclozel bclozel added status: waiting-for-feedback We need additional information before we can continue in: web Issues in web modules (web, webmvc, webflux, websocket) theme: observability An issue related to observability and tracing labels Feb 6, 2024
@wleese wleese changed the title Inconsistent http.route tracing attribute value between RestTemplate & WebClient since 6.3.x Inconsistent http.route tracing attribute value between RestTemplate & WebClient since 6.1.x Feb 6, 2024
@wleese
Copy link
Author

wleese commented Feb 6, 2024

@bclozel oh sorry, I've changed the title.
We originally reported a similar issue in #31882 and #32003.

All these issues relate to the same test on our side. In the 2 issues I linked, we failed to mention that this inconsistency between restTemplate and webClient makes things awkward, so that is what this issue is about.

Also updated the original post to reflect exactly what we're asserting.

@wleese wleese changed the title Inconsistent http.route tracing attribute value between RestTemplate & WebClient since 6.1.x Inconsistent http.route tracing attribute value between RestTemplate & WebClient since 6.1.2 Feb 6, 2024
@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 6, 2024
@bclozel
Copy link
Member

bclozel commented Feb 12, 2024

Hello @wleese

I had another look to this issue and unfortunately I don't think we can change this. You're right that this change of behavior is due to #31882 and #32003 and I think they're valid. On the other hand, the same feature in RestTemplate is not implemented in Spring Framework but in Spring Boot.

In Spring Framework, WebClient has its own Builder interface with a base URI support. #31882 and #32003 refined this behavior because of community reports.

In Spring Framework, RestTemplate has no builder and no notion of base URI. This is implemented in Spring Boot with RestTemplateBuilder and predates WebClient and the observability support. The base URI support is using a UriTemplateHandler contract to support that. This handler delegates to the actual uri template handler for expansion and prepends the base URI if necessary. See https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java#L642 and https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RootUriTemplateHandler.java#L69.

Unfortunately, this base URI information is lost during the expansion and it's too late to get it for the RestTemplate instrumentation to get it. The only proper solution here would be to introduce a builder directly in Spring Framework and align the behavior here. Feel free to create an issue for that in Spring Framework, but you should know that if we do so, deprecating Spring Boot's ResTemplateBuilder and retiring it would require a transition period that would be quite long.

If consistency is very important for you in the short term, you can use a custom ObservationFilter that processes the http.route keyvalue and removes the host part to revert this behavior.

I'm closing this issue now as I don't see any way to fix this without causing other, more important regressions.

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Feb 12, 2024
@bclozel bclozel added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Feb 12, 2024
@wleese
Copy link
Author

wleese commented Feb 12, 2024

Thanks for the elaborate explanation. We'll make it consistent on our end.

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) status: declined A suggestion or change that we don't feel we should currently apply theme: observability An issue related to observability and tracing
Projects
None yet
Development

No branches or pull requests

3 participants