spring-cloud/spring-cloud-commons

LoadBalancerAutoConfiguration does not add LoadBalancerInterceptor via org.springframework.cloud.client.loadbalancer.RestTemplateCustomizer when another org.springframework.cloud.client.loadbalancer.RestTemplateCustomizer bean exists

ZIRAKrezovic opened this issue · 6 comments

ZIRAKrezovic commented

Due to usage of @ConditionalOnMissingBean in LoadBalancerAutoConfiguration, bean

@Bean
@ConditionalOnMissingBean
public RestTemplateCustomizer restTemplateCustomizer(final LoadBalancerInterceptor loadBalancerInterceptor) {
	return restTemplate -> {
		List<ClientHttpRequestInterceptor> list = new ArrayList<>(restTemplate.getInterceptors());
		list.add(loadBalancerInterceptor);
		restTemplate.setInterceptors(list);
	};
}

will not get published if another @Bean implementing org.springframework.cloud.client.loadbalancer.RestTemplateCustomizer exists, resulting in RestTemplate that is annotated with @LoadBalanced not having the LoadBalancerInterceptor interceptor registered. This is a consequence when using @ConditionalOnMissingBean

According to the code above,

@Bean
public SmartInitializingSingleton loadBalancedRestTemplateInitializerDeprecated(
		final ObjectProvider<List<RestTemplateCustomizer>> restTemplateCustomizers) {
	return () -> restTemplateCustomizers.ifAvailable(customizers -> {
		for (RestTemplate restTemplate : LoadBalancerAutoConfiguration.this.restTemplates) {
			for (RestTemplateCustomizer customizer : customizers) {
				customizer.customize(restTemplate);
			}
		}
	});
}

more than one Bean implementing org.springframework.cloud.client.loadbalancer.RestTemplateCustomizer can be present.

OlgaMaciaszek commented

Hello @ZIRAKrezovic this interface is provided by the loadbalancer package, added there specifically for the purpose of setting up load-balancing; we do not really expect the users to create one unless they want to override the behaviour of the one we create. Other than that, I'm not sure why it would be used by the users, since the users create and can customise the RestTemplate bean itself. Could you provide more information on what your use-case is here?

ZIRAKrezovic commented

Hi @OlgaMaciaszek. It seems that I found out why I ran into the problem in the first place.

In short, my use case is "Add some default headers to all RestTemplates produced by AutoConfiguration". BUT: I have implemented the wrong customizer

There are two classes that provide exactly the same behavior

org.springframework.boot.web.client.RestTemplateCustomizer
org.springframework.cloud.client.loadbalancer.RestTemplateCustomizer

I was most likely going for the former one while following Spring Web doc, but due to spring cloud being in the class path, the IDE chose the latter one. And it has worked until the changes made in spring-cloud-commons 4.1.1.

Now, it is misleading that the LoadBalancerAutoConfiguration works with a list of initializers of second type mentioned above when the default implementation backs off. I thought this was a regression.

In any case, I will migrate my code to the Spring Web interface, rather than Spring Cloud Commons one, but you may want to mention this somewhere in the changelog, at least.

OlgaMaciaszek commented

Not sure what you mean. We still don't use org.springframework.boot.web.client.RestTemplateCustomizer there and we have used org.springframework.cloud.client.loadbalancer.RestTemplateCustomizer forever in that class. The only thing that's changed in 4.1.1 is a different customiser introduced that works on RestClient and not RestTemplate. The changelog for 4.1.1: https://github.com/spring-cloud/spring-cloud-commons/releases/tag/v4.1.1.

ZIRAKrezovic commented

I meant that I intended to use

org.springframework.boot.web.client.RestTemplateCustomizer

but ended up using

org.springframework.cloud.client.loadbalancer.RestTemplateCustomizer

in my code. Still, the problem remains that usage of the latter did not break LoadBalancerInterceptor before.

OlgaMaciaszek commented

That @ConditionalOnMissingBean has been there since 2015. Not sure why this would change now, but if you provide a sample that reproduces the issue, I can take a look.

ZIRAKrezovic commented

Hi @OlgaMaciaszek, you are right. I have tried to create a reproducer https://github.com/ZIRAKrezovic/spring-cloud-reproducer/tree/commons-1335

As soon as I uncomment @Configuration in https://github.com/ZIRAKrezovic/spring-cloud-reproducer/blob/commons-1335/src/main/java/com/github/krezovic/springcloudreproducer/ProjectRestTemplateCustomizer.java, load balancing breaks and the test fails.

It is possible that it never worked but nobody tested it or that somehow the old post-processor picked it up. But I can't remember defining any RestTemplate.Builder beans that the PostProcessor picked up.

You may close the issue.

  • 👍1