From 5c05186cf1017a95dcee2d8b94252f2eea158106 Mon Sep 17 00:00:00 2001 From: Tim Ysewyn Date: Mon, 14 Jan 2019 19:15:26 +0100 Subject: [PATCH] Incorporated PR feedback --- .../autoconfig/TraceAutoConfiguration.java | 1 + .../zipkin2/ZipkinAutoConfiguration.java | 17 +++- .../ZipkinKafkaSenderConfiguration.java | 3 +- .../ZipkinRabbitSenderConfiguration.java | 3 +- ...ZipkinRestTemplateSenderConfiguration.java | 3 +- .../zipkin2/ZipkinAutoConfigurationTests.java | 79 ++++++++++++------- 6 files changed, 74 insertions(+), 32 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java index b7d26bb4e..837e45cce 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java @@ -50,6 +50,7 @@ import org.springframework.context.annotation.Configuration; * * @author Spencer Gibb * @author Marcin Grzejszczak + * @author Tim Ysewyn * @since 2.0.0 */ @Configuration diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfiguration.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfiguration.java index c18dcbd86..265848817 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfiguration.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfiguration.java @@ -56,6 +56,7 @@ import org.springframework.web.client.RestTemplate; * {@link DefaultZipkinRestTemplateCustomizer} adds the GZip compression. * * @author Spencer Gibb + * @author Tim Ysewyn * @since 1.0.0 * @see SamplerAutoConfiguration * @see ZipkinRestTemplateCustomizer @@ -69,14 +70,26 @@ import org.springframework.web.client.RestTemplate; @Import({ ZipkinSenderConfigurationImportSelector.class, SamplerAutoConfiguration.class }) public class ZipkinAutoConfiguration { + /** + * Zipkin reporter bean name. Name of the bean matters for supporting multiple tracing + * systems. + */ + public static final String REPORTER_BEAN_NAME = "zipkinReporter"; + + /** + * Zipkin sender bean name. Name of the bean matters for supporting multiple tracing + * systems. + */ + public static final String SENDER_BEAN_NAME = "zipkinSender"; + /** * Accepts a sender so you can plug-in any standard one. Returns a Reporter so you can * also replace with a standard one. */ - @Bean("zipkinReporter") + @Bean(REPORTER_BEAN_NAME) @ConditionalOnMissingBean public Reporter reporter(ReporterMetrics reporterMetrics, - ZipkinProperties zipkin, @Qualifier("zipkinSender") Sender sender, + ZipkinProperties zipkin, @Qualifier(SENDER_BEAN_NAME) Sender sender, BytesEncoder spanBytesEncoder) { return AsyncReporter.builder(sender).queuedMaxSpans(1000) // historical // constraint. Note: diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinKafkaSenderConfiguration.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinKafkaSenderConfiguration.java index 204f28040..6bb89d241 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinKafkaSenderConfiguration.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinKafkaSenderConfiguration.java @@ -25,6 +25,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.kafka.KafkaProperties; +import org.springframework.cloud.sleuth.zipkin2.ZipkinAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; @@ -42,7 +43,7 @@ class ZipkinKafkaSenderConfiguration { @Value("${spring.zipkin.kafka.topic:zipkin}") private String topic; - @Bean("zipkinSender") + @Bean(ZipkinAutoConfiguration.SENDER_BEAN_NAME) Sender kafkaSender(KafkaProperties config) { Map properties = config.buildProducerProperties(); properties.put("key.serializer", ByteArraySerializer.class.getName()); diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinRabbitSenderConfiguration.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinRabbitSenderConfiguration.java index c705bf453..080a22d9d 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinRabbitSenderConfiguration.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinRabbitSenderConfiguration.java @@ -21,6 +21,7 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.amqp.RabbitProperties; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.cloud.sleuth.zipkin2.ZipkinAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; @@ -36,7 +37,7 @@ class ZipkinRabbitSenderConfiguration { @Value("${spring.zipkin.rabbitmq.queue:zipkin}") private String queue; - @Bean("zipkinSender") + @Bean(ZipkinAutoConfiguration.SENDER_BEAN_NAME) Sender rabbitSender(CachingConnectionFactory connectionFactory, RabbitProperties config) { return RabbitMQSender.newBuilder() diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinRestTemplateSenderConfiguration.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinRestTemplateSenderConfiguration.java index 6b0430715..8d9352e14 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinRestTemplateSenderConfiguration.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/sender/ZipkinRestTemplateSenderConfiguration.java @@ -28,6 +28,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClas import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; +import org.springframework.cloud.sleuth.zipkin2.ZipkinAutoConfiguration; import org.springframework.cloud.sleuth.zipkin2.ZipkinLoadBalancer; import org.springframework.cloud.sleuth.zipkin2.ZipkinProperties; import org.springframework.cloud.sleuth.zipkin2.ZipkinRestTemplateCustomizer; @@ -50,7 +51,7 @@ class ZipkinRestTemplateSenderConfiguration { @Autowired ZipkinUrlExtractor extractor; - @Bean("zipkinSender") + @Bean(ZipkinAutoConfiguration.SENDER_BEAN_NAME) @ConditionalOnMissingBean public Sender restTemplateSender(ZipkinProperties zipkin, ZipkinRestTemplateCustomizer zipkinRestTemplateCustomizer) { diff --git a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfigurationTests.java b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfigurationTests.java index 0da2fd8b3..73e3c52d7 100644 --- a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfigurationTests.java +++ b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinAutoConfigurationTests.java @@ -42,6 +42,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.mock.env.MockEnvironment; import zipkin2.Call; import zipkin2.codec.Encoding; +import zipkin2.reporter.AsyncReporter; import zipkin2.reporter.Reporter; import zipkin2.reporter.Sender; import zipkin2.reporter.amqp.RabbitMQSender; @@ -190,22 +191,37 @@ public class ZipkinAutoConfigurationTests { } @Test - public void supportsMultipleReporters() { + public void supportsMultipleReporters() throws Exception { this.context = new AnnotationConfigApplicationContext(); environment().setProperty("spring.zipkin.base-url", this.server.url("/").toString()); this.context.register(ZipkinAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, TraceAutoConfiguration.class, - MultipleReportersConfig.class); + Config.class, MultipleReportersConfig.class); this.context.refresh(); then(this.context.getBeansOfType(Sender.class)).hasSize(2); - then(this.context.getBeansOfType(Sender.class)).containsKeys("zipkinSender", "otherSender"); + then(this.context.getBeansOfType(Sender.class)).containsKeys("zipkinSender", + "otherSender"); then(this.context.getBeansOfType(Reporter.class)).hasSize(2); - then(this.context.getBeansOfType(Reporter.class)).containsKeys("zipkinReporter", "otherReporter"); + then(this.context.getBeansOfType(Reporter.class)).containsKeys("zipkinReporter", + "otherReporter"); - this.context.close(); + Span span = this.context.getBean(Tracing.class).tracer().nextSpan().name("foo") + .tag("foo", "bar").start(); + + span.finish(); + + Awaitility.await().untilAsserted( + () -> then(this.server.getRequestCount()).isGreaterThan(0)); + RecordedRequest request = this.server.takeRequest(); + then(request.getPath()).isEqualTo("/api/v2/spans"); + then(request.getBody().readUtf8()).contains("localEndpoint"); + + MultipleReportersConfig.OtherSender sender = this.context + .getBean(MultipleReportersConfig.OtherSender.class); + Awaitility.await().untilAsserted(() -> then(sender.isSpanSent()).isTrue()); } @Configuration @@ -250,34 +266,43 @@ public class ZipkinAutoConfigurationTests { @Bean Reporter otherReporter() { - return span -> { - - }; + return AsyncReporter.create(otherSender()); } @Bean - Sender otherSender() { - return new Sender() { - @Override - public Encoding encoding() { - return null; - } + OtherSender otherSender() { + return new OtherSender(); + } - @Override - public int messageMaxBytes() { - return 0; - } + static class OtherSender extends Sender { - @Override - public int messageSizeInBytes(List encodedSpans) { - return 0; - } + private boolean spanSent = false; + + boolean isSpanSent() { + return this.spanSent; + } + + @Override + public Encoding encoding() { + return Encoding.JSON; + } + + @Override + public int messageMaxBytes() { + return Integer.MAX_VALUE; + } + + @Override + public int messageSizeInBytes(List encodedSpans) { + return encoding().listSizeInBytes(encodedSpans); + } + + @Override + public Call sendSpans(List encodedSpans) { + this.spanSent = true; + return Call.create(null); + } - @Override - public Call sendSpans(List encodedSpans) { - return null; - } - }; } }