From bf27382ca8b633e5b651ce48b01a34f5b33863cf Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 1 Jun 2017 14:11:26 +0200 Subject: [PATCH] Fixes the way span adjusters are injected without this you can't start the context when seuth is disabled with this change we can autowire an empty list fixes #600 --- .../stream/SleuthStreamAutoConfiguration.java | 9 +- ...lientEndpointLocatorConfigurationTest.java | 135 +++++++++--------- .../stream/StreamWithDisabledSleuthTests.java | 37 +++++ .../zipkin/ZipkinAutoConfiguration.java | 7 +- ...lientEndpointLocatorConfigurationTest.java | 123 ++++++++-------- .../zipkin/ZipkinWithDisabledSleuthTests.java | 37 +++++ 6 files changed, 214 insertions(+), 134 deletions(-) create mode 100644 spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/StreamWithDisabledSleuthTests.java create mode 100644 spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinWithDisabledSleuthTests.java diff --git a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfiguration.java b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfiguration.java index 77f58ed02..bf8225ca1 100644 --- a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfiguration.java +++ b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfiguration.java @@ -16,6 +16,7 @@ package org.springframework.cloud.sleuth.stream; +import java.util.ArrayList; import java.util.List; import org.springframework.beans.factory.annotation.Autowired; @@ -67,6 +68,8 @@ import org.springframework.scheduling.support.PeriodicTrigger; @ConditionalOnProperty(value = "spring.sleuth.stream.enabled", matchIfMissing = true) public class SleuthStreamAutoConfiguration { + @Autowired(required = false) List spanAdjusters = new ArrayList<>(); + @Bean @ConditionalOnMissingBean public Sampler defaultTraceSampler(SamplerProperties config) { @@ -82,9 +85,9 @@ public class SleuthStreamAutoConfiguration { @Bean @ConditionalOnMissingBean public StreamSpanReporter sleuthStreamSpanReporter(HostLocator endpointLocator, - SpanMetricReporter spanMetricReporter, Environment environment, - List spanAdjusters) { - return new StreamSpanReporter(endpointLocator, spanMetricReporter, environment, spanAdjusters); + SpanMetricReporter spanMetricReporter, Environment environment) { + return new StreamSpanReporter(endpointLocator, spanMetricReporter, environment, + this.spanAdjusters); } @Bean(name = StreamSpanReporter.POLLER) diff --git a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientEndpointLocatorConfigurationTest.java b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientEndpointLocatorConfigurationTest.java index c01dd1e16..2aeb0b5d1 100644 --- a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientEndpointLocatorConfigurationTest.java +++ b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientEndpointLocatorConfigurationTest.java @@ -15,80 +15,79 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Matcin Wielgus */ public class DiscoveryClientEndpointLocatorConfigurationTest { - @Test - public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocator() { - try (ConfigurableApplicationContext ctxt = new SpringApplication( - EmptyConfiguration.class).run("--spring.main.web_environment=false")) { - assertThat(ctxt.getBean(HostLocator.class)) - .isInstanceOf(ServerPropertiesHostLocator.class); - } - } + @Test + public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocator() { + try (ConfigurableApplicationContext ctxt = new SpringApplication( + EmptyConfiguration.class).run("--spring.jmx.enabled=false", + "--spring.main.web_environment=false")) { + assertThat(ctxt.getBean(HostLocator.class)) + .isInstanceOf(ServerPropertiesHostLocator.class); + } + } - @Test - public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocatorEvenWhenDiscoveryClientPresent() { - try (ConfigurableApplicationContext ctxt = new SpringApplication( - ConfigurationWithDiscoveryClient.class) - .run("--spring.main.web_environment=false")) { - assertThat(ctxt.getBean(HostLocator.class)) - .isInstanceOf(ServerPropertiesHostLocator.class); - } - } + @Test + public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocatorEvenWhenDiscoveryClientPresent() { + try (ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class).run("--spring.jmx.enabled=false", + "--spring.main.web_environment=false")) { + assertThat(ctxt.getBean(HostLocator.class)) + .isInstanceOf(ServerPropertiesHostLocator.class); + } + } - @Test - public void endpointLocatorShouldRespectExistingEndpointLocator() { - try (ConfigurableApplicationContext ctxt = new SpringApplication( - ConfigurationWithCustomLocator.class) - .run("--spring.main.web_environment=false")) { - assertThat(ctxt.getBean(HostLocator.class)) - .isSameAs(ConfigurationWithCustomLocator.locator); - } - } + @Test + public void endpointLocatorShouldRespectExistingEndpointLocator() { + try (ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithCustomLocator.class).run("--spring.jmx.enabled=false", + "--spring.main.web_environment=false")) { + assertThat(ctxt.getBean(HostLocator.class)) + .isSameAs(ConfigurationWithCustomLocator.locator); + } + } - @Test - public void endpointLocatorShouldBeFallbackHavingEndpointLocatorWhenAskedTo() { - try (ConfigurableApplicationContext ctxt = new SpringApplication( - ConfigurationWithDiscoveryClient.class).run( - "--spring.zipkin.locator.discovery.enabled=true", - "--spring.main.web_environment=false")) { - assertThat(ctxt.getBean(HostLocator.class)) - .isInstanceOf(DiscoveryClientHostLocator.class); - } - } + @Test + public void endpointLocatorShouldBeFallbackHavingEndpointLocatorWhenAskedTo() { + try (ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class).run("--spring.jmx.enabled=false", + "--spring.zipkin.locator.discovery.enabled=true", + "--spring.main.web_environment=false")) { + assertThat(ctxt.getBean(HostLocator.class)) + .isInstanceOf(DiscoveryClientHostLocator.class); + } + } - @Test - public void endpointLocatorShouldRespectExistingEndpointLocatorEvenWhenAskedToBeDiscovery() { - try (ConfigurableApplicationContext ctxt = new SpringApplication( - ConfigurationWithDiscoveryClient.class, - ConfigurationWithCustomLocator.class).run( - "--spring.zipkin.locator.discovery.enabled=true", - "--spring.main.web_environment=false")) { - assertThat(ctxt.getBean(HostLocator.class)) - .isSameAs(ConfigurationWithCustomLocator.locator); - } - } + @Test + public void endpointLocatorShouldRespectExistingEndpointLocatorEvenWhenAskedToBeDiscovery() { + try (ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class, + ConfigurationWithCustomLocator.class).run("--spring.jmx.enabled=false", + "--spring.zipkin.locator.discovery.enabled=true", + "--spring.main.web_environment=false")) { + assertThat(ctxt.getBean(HostLocator.class)) + .isSameAs(ConfigurationWithCustomLocator.locator); + } + } - @Configuration - @EnableAutoConfiguration - public static class EmptyConfiguration { - } + @Configuration + @EnableAutoConfiguration + public static class EmptyConfiguration { + } - @Configuration - @EnableAutoConfiguration - public static class ConfigurationWithDiscoveryClient { - @Bean - public DiscoveryClient getDiscoveryClient() { - return Mockito.mock(DiscoveryClient.class); - } - } + @Configuration + @EnableAutoConfiguration + public static class ConfigurationWithDiscoveryClient { + @Bean public DiscoveryClient getDiscoveryClient() { + return Mockito.mock(DiscoveryClient.class); + } + } - @Configuration - @EnableAutoConfiguration - public static class ConfigurationWithCustomLocator { - static HostLocator locator = Mockito.mock(HostLocator.class); + @Configuration + @EnableAutoConfiguration + public static class ConfigurationWithCustomLocator { + static HostLocator locator = Mockito.mock(HostLocator.class); - @Bean - public HostLocator getEndpointLocator() { - return locator; - } - } + @Bean public HostLocator getEndpointLocator() { + return locator; + } + } } \ No newline at end of file diff --git a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/StreamWithDisabledSleuthTests.java b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/StreamWithDisabledSleuthTests.java new file mode 100644 index 000000000..af0542655 --- /dev/null +++ b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/StreamWithDisabledSleuthTests.java @@ -0,0 +1,37 @@ +/* + * Copyright 2013-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.cloud.sleuth.stream; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration(classes = StreamWithDisabledSleuthTests.Config.class) +@TestPropertySource(properties = "spring.sleuth.enabled=false") +public class StreamWithDisabledSleuthTests { + + @Test public void shouldStartContext() { + + } + + @EnableAutoConfiguration + static class Config { + } +} diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java index b575d3d8c..da83b58e6 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java @@ -16,6 +16,7 @@ package org.springframework.cloud.sleuth.zipkin; +import java.util.ArrayList; import java.util.List; import org.springframework.beans.factory.annotation.Autowired; @@ -62,6 +63,8 @@ import org.springframework.web.client.RestTemplate; @AutoConfigureBefore(TraceAutoConfiguration.class) public class ZipkinAutoConfiguration { + @Autowired(required = false) List spanAdjusters = new ArrayList<>(); + @Bean @ConditionalOnMissingBean public ZipkinSpanReporter reporter(SpanMetricReporter spanMetricReporter, ZipkinProperties zipkin, @@ -86,8 +89,8 @@ public class ZipkinAutoConfiguration { @Bean public SpanReporter zipkinSpanListener(ZipkinSpanReporter reporter, EndpointLocator endpointLocator, - Environment environment, List spanAdjusters) { - return new ZipkinSpanListener(reporter, endpointLocator, environment, spanAdjusters); + Environment environment) { + return new ZipkinSpanListener(reporter, endpointLocator, environment, this.spanAdjusters); } @Configuration diff --git a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/DiscoveryClientEndpointLocatorConfigurationTest.java b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/DiscoveryClientEndpointLocatorConfigurationTest.java index 510d18899..fd5dd32a3 100644 --- a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/DiscoveryClientEndpointLocatorConfigurationTest.java +++ b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/DiscoveryClientEndpointLocatorConfigurationTest.java @@ -16,74 +16,75 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class DiscoveryClientEndpointLocatorConfigurationTest { - @Test - public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocator() { - ConfigurableApplicationContext ctxt = new SpringApplication( - EmptyConfiguration.class).run(); - assertThat(ctxt.getBean(EndpointLocator.class)) - .isInstanceOf(ServerPropertiesEndpointLocator.class); - ctxt.close(); - } + @Test + public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocator() { + ConfigurableApplicationContext ctxt = new SpringApplication( + EmptyConfiguration.class).run("--spring.jmx.enabled=false"); + assertThat(ctxt.getBean(EndpointLocator.class)) + .isInstanceOf(ServerPropertiesEndpointLocator.class); + ctxt.close(); + } - @Test - public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocatorEvenWhenDiscoveryClientPresent() { - ConfigurableApplicationContext ctxt = new SpringApplication( - ConfigurationWithDiscoveryClient.class).run(); - assertThat(ctxt.getBean(EndpointLocator.class)) - .isInstanceOf(ServerPropertiesEndpointLocator.class); - ctxt.close(); - } + @Test + public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocatorEvenWhenDiscoveryClientPresent() { + ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class).run("--spring.jmx.enabled=false"); + assertThat(ctxt.getBean(EndpointLocator.class)) + .isInstanceOf(ServerPropertiesEndpointLocator.class); + ctxt.close(); + } - @Test - public void endpointLocatorShouldRespectExistingEndpointLocator() { - ConfigurableApplicationContext ctxt = new SpringApplication( - ConfigurationWithCustomLocator.class).run(); - assertThat(ctxt.getBean(EndpointLocator.class)) - .isSameAs(ConfigurationWithCustomLocator.locator); - ctxt.close(); - } + @Test + public void endpointLocatorShouldRespectExistingEndpointLocator() { + ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithCustomLocator.class).run("--spring.jmx.enabled=false"); + assertThat(ctxt.getBean(EndpointLocator.class)) + .isSameAs(ConfigurationWithCustomLocator.locator); + ctxt.close(); + } - @Test - public void endpointLocatorShouldBeFallbackHavingEndpointLocatorWhenAskedTo() { - ConfigurableApplicationContext ctxt = new SpringApplication( - ConfigurationWithDiscoveryClient.class).run("--spring.zipkin.locator.discovery.enabled=true"); - assertThat(ctxt.getBean(EndpointLocator.class)) - .isInstanceOf(FallbackHavingEndpointLocator.class); - ctxt.close(); - } + @Test + public void endpointLocatorShouldBeFallbackHavingEndpointLocatorWhenAskedTo() { + ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class).run("--spring.jmx.enabled=false", + "--spring.zipkin.locator.discovery.enabled=true"); + assertThat(ctxt.getBean(EndpointLocator.class)) + .isInstanceOf(FallbackHavingEndpointLocator.class); + ctxt.close(); + } - @Test - public void endpointLocatorShouldRespectExistingEndpointLocatorEvenWhenAskedToBeDiscovery() { - ConfigurableApplicationContext ctxt = new SpringApplication( - ConfigurationWithDiscoveryClient.class,ConfigurationWithCustomLocator.class).run("--spring.zipkin.locator.discovery.enabled=true"); - assertThat(ctxt.getBean(EndpointLocator.class)) - .isSameAs(ConfigurationWithCustomLocator.locator); - ctxt.close(); - } + @Test + public void endpointLocatorShouldRespectExistingEndpointLocatorEvenWhenAskedToBeDiscovery() { + ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class, + ConfigurationWithCustomLocator.class).run("--spring.jmx.enabled=false", + "--spring.zipkin.locator.discovery.enabled=true"); + assertThat(ctxt.getBean(EndpointLocator.class)) + .isSameAs(ConfigurationWithCustomLocator.locator); + ctxt.close(); + } - @Configuration - @EnableAutoConfiguration - public static class EmptyConfiguration { - } + @Configuration + @EnableAutoConfiguration + public static class EmptyConfiguration { + } - @Configuration - @EnableAutoConfiguration - public static class ConfigurationWithDiscoveryClient { - @Bean - public DiscoveryClient getDiscoveryClient() { - return Mockito.mock(DiscoveryClient.class); - } - } + @Configuration + @EnableAutoConfiguration + public static class ConfigurationWithDiscoveryClient { + @Bean public DiscoveryClient getDiscoveryClient() { + return Mockito.mock(DiscoveryClient.class); + } + } - @Configuration - @EnableAutoConfiguration - public static class ConfigurationWithCustomLocator { - static EndpointLocator locator = Mockito.mock(EndpointLocator.class); + @Configuration + @EnableAutoConfiguration + public static class ConfigurationWithCustomLocator { + static EndpointLocator locator = Mockito.mock(EndpointLocator.class); - @Bean - public EndpointLocator getEndpointLocator() { - return locator; - } - } + @Bean public EndpointLocator getEndpointLocator() { + return locator; + } + } } \ No newline at end of file diff --git a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinWithDisabledSleuthTests.java b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinWithDisabledSleuthTests.java new file mode 100644 index 000000000..5d09755fc --- /dev/null +++ b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinWithDisabledSleuthTests.java @@ -0,0 +1,37 @@ +/* + * Copyright 2013-2017 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.cloud.sleuth.zipkin; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration(classes = ZipkinWithDisabledSleuthTests.Config.class) +@TestPropertySource(properties = "spring.sleuth.enabled=false") +public class ZipkinWithDisabledSleuthTests { + + @Test public void shouldStartContext() { + + } + + @EnableAutoConfiguration + static class Config { + } +}