From 5c9f8a7618bbaf52d604d42c2f25642714c7f127 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 17 Jun 2020 16:05:56 +0200 Subject: [PATCH] Fixes the case where the sender is overriden with the predefined name; fixes gh-1637 --- ...ckwardsCompatibilityAutoConfiguration.java | 16 ++++-- ...dsCompatibilityAutoConfigurationTests.java | 54 +++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinBackwardsCompatibilityAutoConfiguration.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinBackwardsCompatibilityAutoConfiguration.java index 38aaf8bc8..3fb0aefc6 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinBackwardsCompatibilityAutoConfiguration.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin2/ZipkinBackwardsCompatibilityAutoConfiguration.java @@ -98,7 +98,10 @@ public class ZipkinBackwardsCompatibilityAutoConfiguration { DefaultListableBeanFactory beanFactory) { List beanNames = new ArrayList<>( Arrays.asList(beanFactory.getBeanNamesForType(Sender.class))); - beanNames.remove(ZipkinAutoConfiguration.SENDER_BEAN_NAME); + if (beanNames.size() != 1 + || !beanNames.contains(ZipkinAutoConfiguration.SENDER_BEAN_NAME)) { + beanNames.remove(ZipkinAutoConfiguration.SENDER_BEAN_NAME); + } Sender sender = (Sender) beanFactory.getBean(beanNames.get(0)); // historical constraint. Note: AsyncReporter supports memory bounds return AsyncReporter.builder(sender).queuedMaxSpans(1000) @@ -145,18 +148,23 @@ public class ZipkinBackwardsCompatibilityAutoConfiguration { context.getBeanFactory()); DefaultListableBeanFactory listableBeanFactory = (DefaultListableBeanFactory) context .getBeanFactory(); - int foundSenders = listableBeanFactory - .getBeanNamesForType(Sender.class).length; + String[] foundSenders = listableBeanFactory.getBeanNamesForType(Sender.class); + int foundSendersSize = foundSenders.length; // Previously we supported 1 Sender bean at a time // which could be overridden by another auto-configuration. // Now we support both the overridden bean and our default zipkinSender bean. // Since this config is adapting the old config we're searching for exactly 1 // `Sender` bean before `ZipkinAutoConfiguration` kicks in. - if (foundSenders != 1) { + if (foundSendersSize != 1) { return ConditionOutcome.noMatch( "None or multiple Sender beans found - no reason to apply backwards compatibility"); } + else if (foundSenders[0].equals(ZipkinAutoConfiguration.SENDER_BEAN_NAME)) { + return ConditionOutcome.noMatch("A single, [" + + ZipkinAutoConfiguration.SENDER_BEAN_NAME + + "] named bean found - no reason to apply backwards compatibility"); + } int foundReporters = listableBeanFactory .getBeanNamesForType(Reporter.class).length; // Check if we need to provide a Reporter bean for the overridden Sender bean diff --git a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinBackwardsCompatibilityAutoConfigurationTests.java b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinBackwardsCompatibilityAutoConfigurationTests.java index d22d70d1a..33928e457 100644 --- a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinBackwardsCompatibilityAutoConfigurationTests.java +++ b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin2/ZipkinBackwardsCompatibilityAutoConfigurationTests.java @@ -16,15 +16,23 @@ package org.springframework.cloud.sleuth.zipkin2; +import java.util.List; + import org.junit.Test; +import zipkin2.Call; import zipkin2.codec.BytesEncoder; +import zipkin2.codec.Encoding; +import zipkin2.reporter.AsyncReporter; import zipkin2.reporter.InMemoryReporterMetrics; import zipkin2.reporter.Reporter; import zipkin2.reporter.ReporterMetrics; +import zipkin2.reporter.Sender; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; import static org.assertj.core.api.Assertions.assertThat; @@ -71,4 +79,50 @@ public class ZipkinBackwardsCompatibilityAutoConfigurationTests { }); } + @Test + public void shouldAllowOverridingSenderOnlyWithoutOverridingTheReporter() { + this.contextRunner.withUserConfiguration(MyConfig.class).run(context -> { + assertThat(context.getBean(ZipkinProperties.class)).isNotNull(); + assertThat(context.getBean(Reporter.class)).isInstanceOf(AsyncReporter.class); + assertThat(context.getBean(Sender.class)).isInstanceOf(MySender.class); + assertThat(context.getBean(BytesEncoder.class)).isNotNull(); + assertThat(context.getBean(ReporterMetrics.class)) + .isInstanceOf(InMemoryReporterMetrics.class); + }); + } + + @Configuration + protected static class MyConfig { + + @Bean(ZipkinAutoConfiguration.SENDER_BEAN_NAME) + Sender mySender() { + return new MySender(); + } + + } + + static class MySender extends Sender { + + @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) { + return Call.create(null); + } + + } + }