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

ZipkinRestTemplateSender is not customizable #33399

Closed
natrem opened this issue Nov 28, 2022 · 2 comments
Closed

ZipkinRestTemplateSender is not customizable #33399

natrem opened this issue Nov 28, 2022 · 2 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@natrem
Copy link
Contributor

natrem commented Nov 28, 2022

In Spring Boot 3.0.0, ZipkinConfigurations.RestTemplateSenderConfiguration create a RestTemplate by relying on RestTemplateBuilder.
The contract indicates that providing a ZipkinRestTemplateBuilderCustomizer would allow for customization of the generated RestTemplate.

However, RestTemplateBuilder is immutable and each method returns a new instance (contrary to the WebClient version).
As a consequence, customization is impossible.

Example failing test:

	@Test
	void should_apply_interceptors() {
		ClientHttpRequestInterceptor interceptor1 = mock(ClientHttpRequestInterceptor.class);
		ZipkinRestTemplateBuilderCustomizer customizer = builder -> builder.additionalInterceptors(interceptor1);
		RestTemplate restTemplate = restTemplateSender(new ZipkinProperties(), new TestObjectProvider<>(customizer));

		// empty list
		assertThat(restTemplate.getInterceptors()).singleElement().isEqualTo(interceptor1);
	}

	// copied from org.springframework.boot.actuate.autoconfigure.tracing.zipkin.ZipkinConfigurations.RestTemplateSenderConfiguration#restTemplateSender
	RestTemplate restTemplateSender(ZipkinProperties properties,
		ObjectProvider<ZipkinRestTemplateBuilderCustomizer> customizers) {
		RestTemplateBuilder restTemplateBuilder = new RestTemplateBuilder()
			.setConnectTimeout(properties.getConnectTimeout()).setReadTimeout(properties.getReadTimeout());

		customizers.orderedStream().forEach((customizer) -> customizer.customize(restTemplateBuilder));

		return restTemplateBuilder.build();
	}

	@RequiredArgsConstructor
	static class TestObjectProvider<T> implements ObjectProvider<T> {
		private final T t;

		@Override
		public T getObject(Object... args) throws BeansException {
			return t;
		}

		@Override
		public T getIfAvailable() throws BeansException {
			return t;
		}

		@Override
		public T getIfUnique() throws BeansException {
			return t;
		}

		@Override
		public T getObject() throws BeansException {
			return t;
		}

		@Override
		public Stream<T> orderedStream() {
			return Stream.of(t);
		}
	}

I think that it is required to make ZipkinRestTemplateBuilderCustomizer return a RestTemplateBuilder to have a working solution, or mutate the RestTemplate instead of the RestTemplateBuilder.

Work-around:

  1. Define your own ZipkinRestTemplateSender (which requires duplicating a lot of classes since ZipkinRestTemplateSender is package private)
  2. Use the web client version, which should not have this problem.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 28, 2022
@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 28, 2022
@wilkinsona wilkinsona modified the milestones: 3.0.1, 3.0.x Nov 28, 2022
@wilkinsona
Copy link
Member

I think we should change the signature of the customize method from void customize(RestTemplateBuilder restTemplateBuilder) to RestTemplateBuilder customize(RestTemplateBuilder restTemplateBuilder). While it is technically a breaking change, I think it's OK as the method's useless in its current form.

@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Nov 28, 2022
@wilkinsona
Copy link
Member

We discussed this today and decided to change customize to return RestTemplateBuilder .

@wilkinsona wilkinsona removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Nov 28, 2022
@mhalbritter mhalbritter self-assigned this Nov 29, 2022
@mhalbritter mhalbritter modified the milestones: 3.0.x, 3.0.1 Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants