Skip to content

WebClient- and RestTemplate-based Zipkin senders can cause a bean dependency cycle #32528

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

Conversation

marcingrzejszczak
Copy link
Contributor

without this change we're using a RestTemplate and WebClient beans for sending out spans. The same beans require Zipkin senders to be pre-configured through the tracing handlers. That's a cycle.

with this change we're breaking the cycle by not creating RT and WC as beans but we're creating them via new and we're providing customizers to allow RT and WC customization

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 28, 2022
@marcingrzejszczak marcingrzejszczak force-pushed the httpClientCycleInZipkinConfig branch 3 times, most recently from f21878e to 0fbb1cc Compare September 28, 2022 14:31
without this change we're using a RestTemplate and WebClient beans for sending out spans. The same beans require Zipkin senders to be pre-configured through the tracing handlers. That's a cycle.
with this change we're breaking the cycle by not creating RT and WC as beans but we're creating them via new and we're providing customizers to allow RT and WC customization
@wilkinsona
Copy link
Member

Cycle:

The dependencies of some of the beans in the application context form a cycle:

   steepController defined in file [/home/marcin/repo/observability/projects/teahouse/tea-service/build/classes/java/main/org/example/teahouse/tea/controller/SteepController.class]
      ↓
   teaService defined in file [/home/marcin/repo/observability/projects/teahouse/tea-service/build/classes/java/main/org/example/teahouse/tea/service/TeaService.class]
      ↓
   defaultClientInterceptorProvider defined in org.springframework.cloud.openfeign.FeignClientsConfiguration$MetricsConfiguration
┌─────┐
|  observationRegistry defined in class path resource [org/springframework/boot/actuate/autoconfigure/observation/ObservationAutoConfiguration.class]
↑     ↓
|  defaultTracingObservationHandler defined in class path resource [org/springframework/boot/actuate/autoconfigure/tracing/MicrometerTracingAutoConfiguration.class]
↑     ↓
|  braveTracerBridge defined in class path resource [org/springframework/boot/actuate/autoconfigure/tracing/BraveAutoConfiguration.class]
↑     ↓
|  braveTracer defined in class path resource [org/springframework/boot/actuate/autoconfigure/tracing/BraveAutoConfiguration.class]
↑     ↓
|  braveTracing defined in class path resource [org/springframework/boot/actuate/autoconfigure/tracing/BraveAutoConfiguration.class]
↑     ↓
|  zipkinSpanHandler defined in class path resource [org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations$BraveConfiguration.class]
↑     ↓
|  spanReporter defined in class path resource [org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations$ReporterConfiguration.class]
↑     ↓
|  restTemplateSender defined in class path resource [org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations$RestTemplateSenderConfiguration.class]
↑     ↓
|  restTemplateBuilder defined in class path resource [org/springframework/boot/autoconfigure/web/client/RestTemplateAutoConfiguration.class]
↑     ↓
|  restTemplateBuilderConfigurer defined in class path resource [org/springframework/boot/autoconfigure/web/client/RestTemplateAutoConfiguration.class]
↑     ↓
|  metricsRestTemplateCustomizer defined in class path resource [org/springframework/boot/actuate/autoconfigure/metrics/web/client/RestTemplateObservationConfiguration.class]
└─────┘

@wilkinsona wilkinsona changed the title Breaks the RestTemplate & WebClient cycles through Zipkin WebClient- and RestTemplate-based Zipkin senders can cause a bean dependency cycle Sep 28, 2022
@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 28, 2022
@wilkinsona wilkinsona added this to the 3.0.x milestone Sep 28, 2022
@wilkinsona wilkinsona self-assigned this Sep 28, 2022
@wilkinsona
Copy link
Member

wilkinsona commented Sep 28, 2022

As far as I can tell the cycle does not occur at the moment when using WebClient. It looks like this is due to WebClientMetricsConfiguration depending on MeterRegistry whereas RestTemplateObservationConfiguration depends on ObservationRegistry. This situation will change with #32518. Regardless of this, I think this is a good change to make as I think it makes sense to isolate the RestTemplate or WebClient that's used for Zipkin sending from the rest of the application's customization of them.

@wilkinsona wilkinsona modified the milestones: 3.0.x, 3.0.0-RC1 Sep 28, 2022
wilkinsona pushed a commit that referenced this pull request Sep 28, 2022
Previously, RestTemplateBuilder and WebClient.Builder beans were used
to create the HTTP client for sending out spans. Those same beans are
also instrumented for observability which results in a cycle.

This commit breaks the cycle by not using the application-web
builders to create the RestTemplate and WebClient's used by the Zipkin
senders. Instead, builders are created inline, with new callbacks
being introduced to allow the user to customize these Zipkin-specific
builders.

See gh-32528
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

Successfully merging this pull request may close these issues.

3 participants