From ed4d13a8bf6b697185ff42da65a372be5fa5b3db Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Wed, 21 Feb 2024 10:06:30 +0100 Subject: [PATCH] Move zipkin.Span types in the OpenTelemetry auto-configuration Brave can work without zipkin2 on the classpath, OpenTelemetry can't. To not force users to have zipkin2 on the classpath, move it into the OpenTelemetry auto-configuration. See gh-39049 --- .../zipkin/ZipkinAutoConfiguration.java | 9 ----- .../tracing/zipkin/ZipkinConfigurations.java | 19 +++++++---- .../zipkin/ZipkinAutoConfigurationTests.java | 28 ++++++++++------ ...ationsOpenTelemetryConfigurationTests.java | 33 +++++++++++++++---- 4 files changed, 57 insertions(+), 32 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinAutoConfiguration.java index ebb1e4c7818a..dca170f32f28 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinAutoConfiguration.java @@ -16,10 +16,7 @@ package org.springframework.boot.actuate.autoconfigure.tracing.zipkin; -import zipkin2.Span; -import zipkin2.reporter.BytesEncoder; import zipkin2.reporter.Encoding; -import zipkin2.reporter.SpanBytesEncoder; import org.springframework.boot.actuate.autoconfigure.tracing.zipkin.ZipkinConfigurations.BraveConfiguration; import org.springframework.boot.actuate.autoconfigure.tracing.zipkin.ZipkinConfigurations.OpenTelemetryConfiguration; @@ -63,10 +60,4 @@ Encoding encoding(ZipkinProperties properties) { }; } - @Bean - @ConditionalOnMissingBean(value = Span.class, parameterizedContainer = BytesEncoder.class) - BytesEncoder zipkinSpanEncoder(Encoding encoding) { - return SpanBytesEncoder.forEncoding(encoding); - } - } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java index 57db8042b2f1..7700a43343ba 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java @@ -29,6 +29,7 @@ import zipkin2.reporter.Encoding; import zipkin2.reporter.HttpEndpointSupplier; import zipkin2.reporter.HttpEndpointSuppliers; +import zipkin2.reporter.SpanBytesEncoder; import zipkin2.reporter.brave.AsyncZipkinSpanHandler; import zipkin2.reporter.brave.MutableSpanBytesEncoder; import zipkin2.reporter.urlconnection.URLConnectionSender; @@ -177,7 +178,7 @@ static class BraveConfiguration { @Bean @ConditionalOnMissingBean(value = MutableSpan.class, parameterizedContainer = BytesEncoder.class) - BytesEncoder braveSpanEncoder(Encoding encoding, + BytesEncoder mutableSpanBytesEncoder(Encoding encoding, ObjectProvider> throwableTagProvider) { Tag throwableTag = throwableTagProvider.getIfAvailable(() -> Tags.ERROR); return MutableSpanBytesEncoder.create(encoding, throwableTag); @@ -188,22 +189,28 @@ BytesEncoder braveSpanEncoder(Encoding encoding, @ConditionalOnBean(BytesMessageSender.class) @ConditionalOnEnabledTracing AsyncZipkinSpanHandler asyncZipkinSpanHandler(BytesMessageSender sender, - BytesEncoder braveSpanEncoder) { - return AsyncZipkinSpanHandler.newBuilder(sender).build(braveSpanEncoder); + BytesEncoder mutableSpanBytesEncoder) { + return AsyncZipkinSpanHandler.newBuilder(sender).build(mutableSpanBytesEncoder); } } @Configuration(proxyBeanMethods = false) - @ConditionalOnClass(ZipkinSpanExporter.class) + @ConditionalOnClass({ ZipkinSpanExporter.class, Span.class }) static class OpenTelemetryConfiguration { + @Bean + @ConditionalOnMissingBean(value = Span.class, parameterizedContainer = BytesEncoder.class) + BytesEncoder spanBytesEncoder(Encoding encoding) { + return SpanBytesEncoder.forEncoding(encoding); + } + @Bean @ConditionalOnMissingBean @ConditionalOnBean(BytesMessageSender.class) @ConditionalOnEnabledTracing - ZipkinSpanExporter zipkinSpanExporter(BytesMessageSender sender, BytesEncoder zipkinSpanEncoder) { - return ZipkinSpanExporter.builder().setSender(sender).setEncoder(zipkinSpanEncoder).build(); + ZipkinSpanExporter zipkinSpanExporter(BytesMessageSender sender, BytesEncoder spanBytesEncoder) { + return ZipkinSpanExporter.builder().setSender(sender).setEncoder(spanBytesEncoder).build(); } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinAutoConfigurationTests.java index e0fac436583c..3a5323cef723 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinAutoConfigurationTests.java @@ -40,8 +40,7 @@ class ZipkinAutoConfigurationTests { @Test void shouldSupplyBeans() { this.contextRunner.run((context) -> assertThat(context).hasSingleBean(Encoding.class) - .hasSingleBean(PropertiesZipkinConnectionDetails.class) - .hasBean("zipkinSpanEncoder")); + .hasSingleBean(PropertiesZipkinConnectionDetails.class)); } @Test @@ -65,14 +64,8 @@ void definesPropertiesBasedConnectionDetailsByDefault() { @Test void shouldUseCustomConnectionDetailsWhenDefined() { - this.contextRunner.withBean(ZipkinConnectionDetails.class, () -> new ZipkinConnectionDetails() { - - @Override - public String getSpanEndpoint() { - return "http://localhost"; - } - - }) + this.contextRunner + .withBean(ZipkinConnectionDetails.class, () -> new FixedZipkinConnectionDetails("http://localhost")) .run((context) -> assertThat(context).hasSingleBean(ZipkinConnectionDetails.class) .doesNotHaveBean(PropertiesZipkinConnectionDetails.class)); } @@ -85,6 +78,21 @@ void shouldWorkWithoutSenders() { .run((context) -> assertThat(context).hasNotFailed()); } + private static final class FixedZipkinConnectionDetails implements ZipkinConnectionDetails { + + private final String spanEndpoint; + + private FixedZipkinConnectionDetails(String spanEndpoint) { + this.spanEndpoint = spanEndpoint; + } + + @Override + public String getSpanEndpoint() { + return this.spanEndpoint; + } + + } + @Configuration(proxyBeanMethods = false) private static final class CustomConfiguration { diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsOpenTelemetryConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsOpenTelemetryConfigurationTests.java index 92f241c966f4..5e8f40eec9e6 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsOpenTelemetryConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsOpenTelemetryConfigurationTests.java @@ -31,7 +31,6 @@ import org.springframework.context.annotation.Configuration; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; /** * Tests for {@link OpenTelemetryConfiguration}. @@ -48,22 +47,40 @@ class ZipkinConfigurationsOpenTelemetryConfigurationTests { @Test void shouldSupplyBeans() { - this.contextRunner.withUserConfiguration(SenderConfiguration.class) - .withBean(BytesEncoder.class, () -> mock(BytesEncoder.class)) - .run((context) -> assertThat(context).hasSingleBean(ZipkinSpanExporter.class)); + this.contextRunner.withUserConfiguration(SenderConfiguration.class, CustomEncoderConfiguration.class) + .run((context) -> { + assertThat(context).hasSingleBean(ZipkinSpanExporter.class); + assertThat(context).hasBean("customSpanEncoder"); + }); } @Test void shouldNotSupplyZipkinSpanExporterIfSenderIsMissing() { - this.contextRunner.run((context) -> assertThat(context).doesNotHaveBean(ZipkinSpanExporter.class)); + this.contextRunner.run((context) -> { + assertThat(context).doesNotHaveBean(ZipkinSpanExporter.class); + assertThat(context).hasBean("spanBytesEncoder"); + }); } @Test void shouldNotSupplyZipkinSpanExporterIfNotOnClasspath() { this.contextRunner.withClassLoader(new FilteredClassLoader("io.opentelemetry.exporter.zipkin")) .withUserConfiguration(SenderConfiguration.class) - .run((context) -> assertThat(context).doesNotHaveBean(ZipkinSpanExporter.class)); + .run((context) -> { + assertThat(context).doesNotHaveBean(ZipkinSpanExporter.class); + assertThat(context).doesNotHaveBean("spanBytesEncoder"); + }); + + } + @Test + void shouldBackOffIfZipkinIsNotOnClasspath() { + this.contextRunner.withClassLoader(new FilteredClassLoader("zipkin2.Span")) + .withUserConfiguration(SenderConfiguration.class) + .run((context) -> { + assertThat(context).doesNotHaveBean(ZipkinSpanExporter.class); + assertThat(context).doesNotHaveBean("spanBytesEncoder"); + }); } @Test @@ -85,6 +102,7 @@ void shouldUseCustomEncoderBean() { this.contextRunner.withUserConfiguration(SenderConfiguration.class, CustomEncoderConfiguration.class) .run((context) -> { assertThat(context).hasSingleBean(ZipkinSpanExporter.class); + assertThat(context).hasBean("customSpanEncoder"); assertThat(context.getBean(ZipkinSpanExporter.class)).extracting("encoder") .isInstanceOf(CustomSpanEncoder.class) .extracting("encoding") @@ -99,6 +117,7 @@ void shouldUseCustomEncodingBean() { CustomEncoderConfiguration.class) .run((context) -> { assertThat(context).hasSingleBean(ZipkinSpanExporter.class); + assertThat(context).hasBean("customSpanEncoder"); assertThat(context.getBean(ZipkinSpanExporter.class)).extracting("encoder") .isInstanceOf(CustomSpanEncoder.class) .extracting("encoding") @@ -140,7 +159,7 @@ ZipkinSpanExporter customZipkinSpanExporter() { private static final class CustomEncoderConfiguration { @Bean - BytesEncoder encoder(Encoding encoding) { + BytesEncoder customSpanEncoder(Encoding encoding) { return new CustomSpanEncoder(encoding); }