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
| } | ||
|
|
||
| private static boolean isInterceptorNotPresent( | ||
| java.util.List<ClientHttpRequestInterceptor> interceptors, |
There was a problem hiding this comment.
could you import java.util.List
There was a problem hiding this comment.
Yes, will also do some cleanups improvements once the approach is agreed upon. Created this for discussion, as the issue notes Webclient also suffers from same issue.
There was a problem hiding this comment.
I applied this pattern to my PR adding the RestClient for spring boot 4 #15684 , it seems like a fine approach to me.
There was a problem hiding this comment.
@lenin-jaganathan i think we can consider this approach "agreed upon", in case you were waiting for that to continue.
There was a problem hiding this comment.
I see this is added as part of another PR. What do you want me to do with this PR, I am make changes to polish this a bit further.
One more thing is We have to consider WebClient as well.
There was a problem hiding this comment.
my PR didn't touch the class you are updating here (for spring boot 3), so your PR is still valid. One thing you can do however, is move the test to an abstract class (do the opposite of this commit). and yea i'd say go ahead and do the same for webclient
There was a problem hiding this comment.
Okay then. I will update this PR and let you guys know.
There was a problem hiding this comment.
I have committed the changes.
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
2 times, most recently
from
b6e0180 to
0b88f60
Compare
…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<lenin.jaganathan@gmail.com>
Signed-off-by: Lenin Jaganathan <lenin.jaganathan@gmail.com>
Signed-off-by: Lenin Jaganathan <lenin.jaganathan@gmail.com> Signed-off-by: Lenin Jaganathan <lenin.jaganathan@gmail.com>
lenin-jaganathan
force-pushed
the
gh-15545-restclient-postprocessing
branch
from
0b88f60 to
9aa1769
Compare
Signed-off-by: Lenin Jaganathan <lenin.jaganathan@gmail.com>
lenin-jaganathan
force-pushed
the
gh-15545-restclient-postprocessing
branch
from
9aa1769 to
92119a4
Compare
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