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

Missing observability support in RestTemplateTransportClientFactory #4255

Closed
ZIRAKrezovic opened this issue Feb 19, 2024 · 2 comments · Fixed by #4262 or #4272
Closed

Missing observability support in RestTemplateTransportClientFactory #4255

ZIRAKrezovic opened this issue Feb 19, 2024 · 2 comments · Fixed by #4262 or #4272
Assignees
Milestone

Comments

@ZIRAKrezovic
Copy link
Contributor

ZIRAKrezovic commented Feb 19, 2024

Describe the bug

I have just spent some hours debugging why RestTemplate is not propagating traces to Eureka Server. Turns out RestTemplateTransportClientFactory creates a RestTemplate instance by calling new RestTemplate().

To fix this, RestTemplate.Builder should be used to create a RestTemplate in RestTemplateTransportClientFactory and we get observability support for free.

Now consider the WebFlux WebClient equivalent

public WebClientTransportClientFactories webClientTransportClientFactories(
		ObjectProvider<WebClient.Builder> builder) {
	return new WebClientTransportClientFactories(builder::getIfAvailable);
}

Here, as expected, the WebClient.Builder is used, and produced TransportClientFactory has observability support ...

Version: Spring Cloud Starter 2023.0.0

@ZIRAKrezovic ZIRAKrezovic changed the title Misleading usage of RestTemplate in DiscoveryClientOptionalArgsConfiguration Missing observability support in RestTemplateTransportClientFactory Feb 19, 2024
ZIRAKrezovic added a commit to ZIRAKrezovic/spring-cloud-netflix that referenced this issue Feb 19, 2024
ZIRAKrezovic added a commit to ZIRAKrezovic/spring-cloud-netflix that referenced this issue Feb 19, 2024
@OlgaMaciaszek OlgaMaciaszek self-assigned this Mar 21, 2024
@OlgaMaciaszek
Copy link
Collaborator

Thanks for reporting the issue, @ZIRAKrezovic. Makes sense.

@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2023.0.1 Mar 26, 2024
@OlgaMaciaszek OlgaMaciaszek added this to the 4.1.1 milestone Mar 26, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in 2023.0.1 Mar 26, 2024
OlgaMaciaszek added a commit that referenced this issue Mar 26, 2024
@OlgaMaciaszek OlgaMaciaszek removed this from 2023.0.1 Mar 26, 2024
@OlgaMaciaszek OlgaMaciaszek removed this from the 4.1.1 milestone Mar 26, 2024
@OlgaMaciaszek OlgaMaciaszek reopened this Mar 26, 2024
@OlgaMaciaszek
Copy link
Collaborator

Reopening as changes reverted due to broken tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment