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
spring-projects · Feb 21, 2024 · ed4d13a · ed4d13a
1 parent 5e844db
commit ed4d13a
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 32 deletions.
diff --git a/...rg/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinAutoConfiguration.java b/...rg/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<Span> zipkinSpanEncoder(Encoding encoding) {
-		return SpanBytesEncoder.forEncoding(encoding);
-	}
-
 }
diff --git a/...a/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurations.java b/...a/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<MutableSpan> braveSpanEncoder(Encoding encoding,
+		BytesEncoder<MutableSpan> mutableSpanBytesEncoder(Encoding encoding,
 				ObjectProvider<Tag<Throwable>> throwableTagProvider) {
 			Tag<Throwable> throwableTag = throwableTagProvider.getIfAvailable(() -> Tags.ERROR);
 			return MutableSpanBytesEncoder.create(encoding, throwableTag);
@@ -188,22 +189,28 @@ BytesEncoder<MutableSpan> braveSpanEncoder(Encoding encoding,
 		@ConditionalOnBean(BytesMessageSender.class)
 		@ConditionalOnEnabledTracing
 		AsyncZipkinSpanHandler asyncZipkinSpanHandler(BytesMessageSender sender,
-				BytesEncoder<MutableSpan> braveSpanEncoder) {
-			return AsyncZipkinSpanHandler.newBuilder(sender).build(braveSpanEncoder);
+				BytesEncoder<MutableSpan> 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<Span> spanBytesEncoder(Encoding encoding) {
+			return SpanBytesEncoder.forEncoding(encoding);
+		}
+
 		@Bean
 		@ConditionalOnMissingBean
 		@ConditionalOnBean(BytesMessageSender.class)
 		@ConditionalOnEnabledTracing
-		ZipkinSpanExporter zipkinSpanExporter(BytesMessageSender sender, BytesEncoder<Span> zipkinSpanEncoder) {
-			return ZipkinSpanExporter.builder().setSender(sender).setEncoder(zipkinSpanEncoder).build();
+		ZipkinSpanExporter zipkinSpanExporter(BytesMessageSender sender, BytesEncoder<Span> spanBytesEncoder) {
+			return ZipkinSpanExporter.builder().setSender(sender).setEncoder(spanBytesEncoder).build();
 		}
 
 	}

diff --git a/...ringframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinAutoConfigurationTests.java b/...ringframework/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/...ate/autoconfigure/tracing/zipkin/ZipkinConfigurationsOpenTelemetryConfigurationTests.java b/...ate/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<Span> encoder(Encoding encoding) {
+		BytesEncoder<Span> customSpanEncoder(Encoding encoding) {
 			return new CustomSpanEncoder(encoding);
 		}