Update RestClientBeanPostProcessor/WebClientBeanPostProcessor to create new bean only when adding OTEL Interceptor #15546
Conversation
|
|
github-actions
bot
added
the
test native
lenin-jaganathan
force-pushed
the
gh-15545-restclient-postprocessing
branch
2 times, most recently
from
6850089 to
c528516
Compare
...mentation/spring/autoconfigure/internal/instrumentation/web/RestClientBeanPostProcessor.java
Outdated
Show resolved
Hide resolved
laurit
added
the
needs author feedback
lenin-jaganathan
force-pushed
the
gh-15545-restclient-postprocessing
branch
2 times, most recently
from
b418b63 to
c7b9736
Compare
lenin-jaganathan
changed the title
|
|
||
| @Bean | ||
| @Order(Ordered.HIGHEST_PRECEDENCE + 10) | ||
| WebClientCustomizer otelWebClientCustomizer( |
There was a problem hiding this comment.
This is the cleaner approach to add instrumentation and is also consistent with what is being done in RestClient.
But doing so would mean we have to add different modules for spring4 where the customizer is moved to the spring-boot-webclient module. What are the thoughts here?
lenin-jaganathan
force-pushed
the
gh-15545-restclient-postprocessing
branch
4 times, most recently
from
9aa1769 to
92119a4
Compare
|
This PR has been labeled as stale due to lack of activity and needing author feedback. It will be automatically closed if there is no further activity over the next 7 days. |
|
This PR has been labeled as stale due to lack of activity and needing author feedback. It will be automatically closed if there is no further activity over the next 7 days. |
|
@lenin-jaganathan in case you might not have noticed, there are some merge conflicts that need to be resolved |
…ceptor already present The RestClientBeanPostProcessor was always creating a new RestClient bean via mutate().build(), even when the OpenTelemetry interceptor was already present. This resulted in unnecessary bean creation. Fixes open-telemetry#15545 Signed-off-by: Lenin Jaganathan<[email protected]>
Signed-off-by: Lenin Jaganathan <[email protected]>
Signed-off-by: Lenin Jaganathan <[email protected]> Signed-off-by: Lenin Jaganathan <[email protected]>
lenin-jaganathan
force-pushed
the
gh-15545-restclient-postprocessing
branch
from
92119a4 to
933565b
Compare
Signed-off-by: Lenin Jaganathan <[email protected]>
lenin-jaganathan
force-pushed
the
gh-15545-restclient-postprocessing
branch
from
933565b to
69a6c45
Compare
Just now coming off a break and saw it. I have fixed it and pushed changes. |
github-actions
bot
removed
the
needs author feedback
...mentation/spring/autoconfigure/internal/instrumentation/web/RestClientBeanPostProcessor.java
Outdated
Show resolved
Hide resolved
| * Abstract base test for WebClient customizer auto-configurations. Subclasses must provide the | ||
| * auto-configuration class and WebClientCustomizer class for their Spring Boot version. | ||
| */ | ||
| public abstract class AbstractWebClientCustomizerAutoConfigurationTest { |
There was a problem hiding this comment.
could parameterize this class
AbstractWebClientCustomizerAutoConfigurationTest<T>
protected abstract Class<T> webClientCustomizerClass();
protected abstract void customizeWebClient(T customizer, WebClient.Builder builder);
subclasses could then use
extends AbstractWebClientCustomizerAutoConfigurationTest<WebClientCustomizer>
protected Class<WebClientCustomizer> webClientCustomizerClass()
protected void customizeWebClient(WebClientCustomizer customizer, WebClient.Builder builder)
Signed-off-by: Lenin Jaganathan <[email protected]>
| functions -> | ||
| assertThat( | ||
| functions.stream() | ||
| private static void assertFilterCount(WebClient webClient, long expectedCount) { |
There was a problem hiding this comment.
nitpick: expectedCount isn't necessary since it's always 1
3 participants
The RestClientBeanPostProcessor was always creating a new RestClient bean via mutate().build(), even when the OpenTelemetry interceptor was already present. This resulted in unnecessary bean creation.
Fixes #15545