From 343d84346fa2e21cd2ed2dd9a127f0fa0a6f91a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E5=93=88=E5=B8=8C?= Date: Thu, 30 Apr 2020 01:58:59 +0000 Subject: [PATCH 1/6] replace method for deprecation and keep reference of requestTemplate --- .../sleuth/instrument/web/client/feign/TracingFeignClient.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java index ac6d69442..601d487ca 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java @@ -173,11 +173,10 @@ final class TracingFeignClient implements Client { if (headers == null) { return delegate; } - String method = delegate.method(); String url = delegate.url(); byte[] body = delegate.body(); Charset charset = delegate.charset(); - return Request.create(method, url, headers, body, charset); + return Request.create(delegate.httpMethod(), url, headers, body, charset, delegate.requestTemplate()); } } From b3cfa4244848815e2e09e4de6d385aa8d29d10f5 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 6 May 2020 08:46:46 +0200 Subject: [PATCH 2/6] Get KafkaStreamsTracing via BeanFactory to prevent eager initialization (#1623) --- .../SleuthKafkaStreamsConfiguration.java | 36 +++-- ...aStreamsConfigurationIntegrationTests.java | 131 ++++++++++++++++++ 2 files changed, 152 insertions(+), 15 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfigurationIntegrationTests.java diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfiguration.java index 7fb7f6e8e..32e0e6dc5 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfiguration.java @@ -23,6 +23,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.kafka.streams.KafkaStreams; import org.springframework.beans.BeansException; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; @@ -44,8 +45,7 @@ import org.springframework.kafka.config.StreamsBuilderFactoryBean; @ConditionalOnBean(Tracing.class) @AutoConfigureAfter({ TraceAutoConfiguration.class }) @OnMessagingEnabled -@ConditionalOnProperty(value = "spring.sleuth.messaging.kafka.streams.enabled", - matchIfMissing = true) +@ConditionalOnProperty(value = "spring.sleuth.messaging.kafka.streams.enabled", matchIfMissing = true) @ConditionalOnClass(KafkaStreams.class) public class SleuthKafkaStreamsConfiguration { @@ -55,6 +55,7 @@ public class SleuthKafkaStreamsConfiguration { /** * Expose {@link KafkaStreamsTracing} as bean to allow for filter/map/peek/transform * operations. + * * @param tracing Brave Tracing instance from TraceAutoConfiguration * @return instance for use in further manual instrumentation */ @@ -64,10 +65,17 @@ public class SleuthKafkaStreamsConfiguration { return KafkaStreamsTracing.create(tracing); } + /** + * Call {@link StreamsBuilderFactoryBean#setClientSupplier(org.apache.kafka.streams.KafkaClientSupplier)} with + * Brave's TracingKafkaClientSupplier. + * + * @param objectProvider provides KafkaStreamsTracing; prevents eager initialization + * @return + */ @Bean static KafkaStreamsBuilderFactoryBeanPostProcessor kafkaStreamsBuilderFactoryBeanPostProcessor( - KafkaStreamsTracing kafkaStreamsTracing) { - return new KafkaStreamsBuilderFactoryBeanPostProcessor(kafkaStreamsTracing); + ObjectProvider objectProvider) { + return new KafkaStreamsBuilderFactoryBeanPostProcessor(objectProvider); } } @@ -83,25 +91,23 @@ public class SleuthKafkaStreamsConfiguration { */ class KafkaStreamsBuilderFactoryBeanPostProcessor implements BeanPostProcessor { - private static final Log log = LogFactory - .getLog(KafkaStreamsBuilderFactoryBeanPostProcessor.class); + private static final Log log = LogFactory.getLog(KafkaStreamsBuilderFactoryBeanPostProcessor.class); - private final KafkaStreamsTracing kafkaStreamsTracing; + private final ObjectProvider objectProvider; - KafkaStreamsBuilderFactoryBeanPostProcessor(KafkaStreamsTracing kafkaStreamsTracing) { - this.kafkaStreamsTracing = kafkaStreamsTracing; + KafkaStreamsBuilderFactoryBeanPostProcessor(ObjectProvider objectProvider) { + this.objectProvider = objectProvider; } @Override - public Object postProcessAfterInitialization(Object bean, String beanName) - throws BeansException { + public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { if (bean instanceof StreamsBuilderFactoryBean) { - StreamsBuilderFactoryBean sbfb = (StreamsBuilderFactoryBean) bean; + // KafkaStreamsTracing is created in SleuthKafkaStreamsConfiguration above, so should not be null here + KafkaStreamsTracing kafkaStreamsTracing = this.objectProvider.getIfAvailable(); + ((StreamsBuilderFactoryBean) bean).setClientSupplier(kafkaStreamsTracing.kafkaClientSupplier()); if (log.isDebugEnabled()) { - log.debug( - "StreamsBuilderFactoryBean bean is auto-configured to enable tracing."); + log.debug("StreamsBuilderFactoryBean bean is auto-configured to enable tracing."); } - sbfb.setClientSupplier(kafkaStreamsTracing.kafkaClientSupplier()); } return bean; } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfigurationIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfigurationIntegrationTests.java new file mode 100644 index 000000000..77cc0a07a --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfigurationIntegrationTests.java @@ -0,0 +1,131 @@ +/* + * Copyright 2013-2019 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 + * + * https://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.instrument.messaging; + +import javax.annotation.PostConstruct; + +import brave.Tracing; +import brave.kafka.streams.KafkaStreamsTracing; +import org.apache.kafka.streams.KafkaClientSupplier; +import org.apache.kafka.streams.KafkaStreams; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.test.context.FilteredClassLoader; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.boot.test.system.CapturedOutput; +import org.springframework.boot.test.system.OutputCaptureExtension; +import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.kafka.config.StreamsBuilderFactoryBean; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +@ExtendWith(OutputCaptureExtension.class) +class SleuthKafkaStreamsConfigurationIntegrationTests { + + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of( + TraceAutoConfiguration.class, + SleuthKafkaStreamsConfiguration.class)) + .withUserConfiguration(UserConfig.class); + + @Test + void should_create_KafkaStreamsTracing() { + this.contextRunner + .run(context -> assertThat(context).hasSingleBean(KafkaStreamsTracing.class)); + } + + @Test + void should_not_create_KafkaStreamsTracing_when_KafkaStreams_not_present() { + this.contextRunner + .withClassLoader(new FilteredClassLoader(KafkaStreams.class)) + .run(context -> assertThat(context).doesNotHaveBean(KafkaStreamsTracing.class)); + } + + @Test + void should_not_create_KafkaStreamsTracing_when_kafkastreams_disabled() { + this.contextRunner + .withPropertyValues("spring.sleuth.messaging.kafka.streams.enabled=false") + .run(context -> assertThat(context).doesNotHaveBean(KafkaStreamsTracing.class)); + } + + @Test + void should_not_create_KafkaStreamsTracing_when_messaging_disabled() { + this.contextRunner + .withPropertyValues("spring.sleuth.messaging.enabled=false") + .run(context -> assertThat(context).doesNotHaveBean(KafkaStreamsTracing.class)); + } + + @Test + void should_set_KafkaClientSupplier_on_StreamsBuilderFactoryBean() { + this.contextRunner + .run(context -> verify(UserConfig.streamsBuilderFactoryBean) + .setClientSupplier(any(KafkaClientSupplier.class))); + } + + @Test + void should_not_complain_about_eager_initialization() { + this.contextRunner + .withUserConfiguration(EagerInitializationConfig.class) + .run(context -> verify(UserConfig.streamsBuilderFactoryBean) + .setClientSupplier(any(KafkaClientSupplier.class))); + } + + @AfterEach + void afterEach(CapturedOutput output) { + assertThat(output).doesNotContain("is not eligible for getting processed by all BeanPostProcessors"); + } + + @Configuration + static class UserConfig { + static StreamsBuilderFactoryBean streamsBuilderFactoryBean; + + @Bean + StreamsBuilderFactoryBean streamsBuilderFactoryBean() { + streamsBuilderFactoryBean = mock(StreamsBuilderFactoryBean.class); + return UserConfig.streamsBuilderFactoryBean; + } + } + + @Configuration + static class EagerInitializationConfig { + @Bean + EagerInitializationComponent eagerInitializationComponent() { + return new EagerInitializationComponent(); + } + } + + static class EagerInitializationComponent { + + @Autowired + private Tracing tracing; + private KafkaStreamsTracing kafkaStreamsTracing; + + @PostConstruct + void init() { + kafkaStreamsTracing = KafkaStreamsTracing.create(tracing); + } + } +} From e92a735faa8dbd7fedcc9aee0434125df7da25a7 Mon Sep 17 00:00:00 2001 From: zhanghaoxin-at-826767166263 Date: Thu, 7 May 2020 08:19:50 +0000 Subject: [PATCH 3/6] add unit test for saved template --- .../SleuthKafkaStreamsConfiguration.java | 31 ++++++++------ .../web/client/feign/TracingFeignClient.java | 3 +- ...aStreamsConfigurationIntegrationTests.java | 40 +++++++++++-------- .../client/feign/TracingFeignClientTests.java | 24 ++++++++++- 4 files changed, 67 insertions(+), 31 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfiguration.java index 32e0e6dc5..8570db8d9 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfiguration.java @@ -45,7 +45,8 @@ import org.springframework.kafka.config.StreamsBuilderFactoryBean; @ConditionalOnBean(Tracing.class) @AutoConfigureAfter({ TraceAutoConfiguration.class }) @OnMessagingEnabled -@ConditionalOnProperty(value = "spring.sleuth.messaging.kafka.streams.enabled", matchIfMissing = true) +@ConditionalOnProperty(value = "spring.sleuth.messaging.kafka.streams.enabled", + matchIfMissing = true) @ConditionalOnClass(KafkaStreams.class) public class SleuthKafkaStreamsConfiguration { @@ -55,7 +56,6 @@ public class SleuthKafkaStreamsConfiguration { /** * Expose {@link KafkaStreamsTracing} as bean to allow for filter/map/peek/transform * operations. - * * @param tracing Brave Tracing instance from TraceAutoConfiguration * @return instance for use in further manual instrumentation */ @@ -66,9 +66,9 @@ public class SleuthKafkaStreamsConfiguration { } /** - * Call {@link StreamsBuilderFactoryBean#setClientSupplier(org.apache.kafka.streams.KafkaClientSupplier)} with - * Brave's TracingKafkaClientSupplier. - * + * Call + * {@link StreamsBuilderFactoryBean#setClientSupplier(org.apache.kafka.streams.KafkaClientSupplier)} + * with Brave's TracingKafkaClientSupplier. * @param objectProvider provides KafkaStreamsTracing; prevents eager initialization * @return */ @@ -91,22 +91,29 @@ public class SleuthKafkaStreamsConfiguration { */ class KafkaStreamsBuilderFactoryBeanPostProcessor implements BeanPostProcessor { - private static final Log log = LogFactory.getLog(KafkaStreamsBuilderFactoryBeanPostProcessor.class); + private static final Log log = LogFactory + .getLog(KafkaStreamsBuilderFactoryBeanPostProcessor.class); private final ObjectProvider objectProvider; - KafkaStreamsBuilderFactoryBeanPostProcessor(ObjectProvider objectProvider) { + KafkaStreamsBuilderFactoryBeanPostProcessor( + ObjectProvider objectProvider) { this.objectProvider = objectProvider; } @Override - public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { + public Object postProcessAfterInitialization(Object bean, String beanName) + throws BeansException { if (bean instanceof StreamsBuilderFactoryBean) { - // KafkaStreamsTracing is created in SleuthKafkaStreamsConfiguration above, so should not be null here - KafkaStreamsTracing kafkaStreamsTracing = this.objectProvider.getIfAvailable(); - ((StreamsBuilderFactoryBean) bean).setClientSupplier(kafkaStreamsTracing.kafkaClientSupplier()); + // KafkaStreamsTracing is created in SleuthKafkaStreamsConfiguration above, so + // should not be null here + KafkaStreamsTracing kafkaStreamsTracing = this.objectProvider + .getIfAvailable(); + ((StreamsBuilderFactoryBean) bean) + .setClientSupplier(kafkaStreamsTracing.kafkaClientSupplier()); if (log.isDebugEnabled()) { - log.debug("StreamsBuilderFactoryBean bean is auto-configured to enable tracing."); + log.debug( + "StreamsBuilderFactoryBean bean is auto-configured to enable tracing."); } } return bean; diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java index 601d487ca..3039ecb54 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClient.java @@ -176,7 +176,8 @@ final class TracingFeignClient implements Client { String url = delegate.url(); byte[] body = delegate.body(); Charset charset = delegate.charset(); - return Request.create(delegate.httpMethod(), url, headers, body, charset, delegate.requestTemplate()); + return Request.create(delegate.httpMethod(), url, headers, body, charset, + delegate.requestTemplate()); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfigurationIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfigurationIntegrationTests.java index 77cc0a07a..5aba918e5 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfigurationIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/SleuthKafkaStreamsConfigurationIntegrationTests.java @@ -46,60 +46,60 @@ import static org.mockito.Mockito.verify; class SleuthKafkaStreamsConfigurationIntegrationTests { private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() - .withConfiguration(AutoConfigurations.of( - TraceAutoConfiguration.class, + .withConfiguration(AutoConfigurations.of(TraceAutoConfiguration.class, SleuthKafkaStreamsConfiguration.class)) .withUserConfiguration(UserConfig.class); @Test void should_create_KafkaStreamsTracing() { - this.contextRunner - .run(context -> assertThat(context).hasSingleBean(KafkaStreamsTracing.class)); + this.contextRunner.run( + context -> assertThat(context).hasSingleBean(KafkaStreamsTracing.class)); } @Test void should_not_create_KafkaStreamsTracing_when_KafkaStreams_not_present() { - this.contextRunner - .withClassLoader(new FilteredClassLoader(KafkaStreams.class)) - .run(context -> assertThat(context).doesNotHaveBean(KafkaStreamsTracing.class)); + this.contextRunner.withClassLoader(new FilteredClassLoader(KafkaStreams.class)) + .run(context -> assertThat(context) + .doesNotHaveBean(KafkaStreamsTracing.class)); } @Test void should_not_create_KafkaStreamsTracing_when_kafkastreams_disabled() { this.contextRunner .withPropertyValues("spring.sleuth.messaging.kafka.streams.enabled=false") - .run(context -> assertThat(context).doesNotHaveBean(KafkaStreamsTracing.class)); + .run(context -> assertThat(context) + .doesNotHaveBean(KafkaStreamsTracing.class)); } @Test void should_not_create_KafkaStreamsTracing_when_messaging_disabled() { - this.contextRunner - .withPropertyValues("spring.sleuth.messaging.enabled=false") - .run(context -> assertThat(context).doesNotHaveBean(KafkaStreamsTracing.class)); + this.contextRunner.withPropertyValues("spring.sleuth.messaging.enabled=false") + .run(context -> assertThat(context) + .doesNotHaveBean(KafkaStreamsTracing.class)); } @Test void should_set_KafkaClientSupplier_on_StreamsBuilderFactoryBean() { - this.contextRunner - .run(context -> verify(UserConfig.streamsBuilderFactoryBean) - .setClientSupplier(any(KafkaClientSupplier.class))); + this.contextRunner.run(context -> verify(UserConfig.streamsBuilderFactoryBean) + .setClientSupplier(any(KafkaClientSupplier.class))); } @Test void should_not_complain_about_eager_initialization() { - this.contextRunner - .withUserConfiguration(EagerInitializationConfig.class) + this.contextRunner.withUserConfiguration(EagerInitializationConfig.class) .run(context -> verify(UserConfig.streamsBuilderFactoryBean) .setClientSupplier(any(KafkaClientSupplier.class))); } @AfterEach void afterEach(CapturedOutput output) { - assertThat(output).doesNotContain("is not eligible for getting processed by all BeanPostProcessors"); + assertThat(output).doesNotContain( + "is not eligible for getting processed by all BeanPostProcessors"); } @Configuration static class UserConfig { + static StreamsBuilderFactoryBean streamsBuilderFactoryBean; @Bean @@ -107,25 +107,31 @@ class SleuthKafkaStreamsConfigurationIntegrationTests { streamsBuilderFactoryBean = mock(StreamsBuilderFactoryBean.class); return UserConfig.streamsBuilderFactoryBean; } + } @Configuration static class EagerInitializationConfig { + @Bean EagerInitializationComponent eagerInitializationComponent() { return new EagerInitializationComponent(); } + } static class EagerInitializationComponent { @Autowired private Tracing tracing; + private KafkaStreamsTracing kafkaStreamsTracing; @PostConstruct void init() { kafkaStreamsTracing = KafkaStreamsTracing.create(tracing); } + } + } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClientTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClientTests.java index 55f21872e..14ab048fc 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClientTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignClientTests.java @@ -28,24 +28,32 @@ import brave.http.HttpTracing; import brave.propagation.StrictCurrentTraceContext; import feign.Client; import feign.Request; +import feign.RequestTemplate; import org.assertj.core.api.BDDAssertions; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.BDDMockito; import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; import static org.assertj.core.api.BDDAssertions.then; /** * @author Marcin Grzejszczak + * @author Hash.Jang */ @RunWith(MockitoJUnitRunner.class) public class TracingFeignClientTests { - Request request = Request.create("GET", "https://foo", new HashMap<>(), null, null); + RequestTemplate requestTemplate = new RequestTemplate(); + + Request request = Request.create(Request.HttpMethod.GET, "https://foo", + new HashMap<>(), null, null, requestTemplate); Request.Options options = new Request.Options(); @@ -112,4 +120,18 @@ public class TracingFeignClientTests { then(this.spans.get(0).tags()).containsEntry("error", "exception has occurred"); } + @Test + public void keep_requestTemplate() throws IOException { + BDDMockito.given(this.client.execute(BDDMockito.any(), BDDMockito.any())) + .willAnswer(new Answer() { + public Object answer(InvocationOnMock invocation) { + Object[] args = invocation.getArguments(); + Assert.assertEquals(((Request) args[0]).requestTemplate(), + requestTemplate); + return null; + } + }); + this.traceFeignClient.execute(this.request, this.options); + } + } From c3f67f31d467ca77e7b1ec8c753422c31f2220a3 Mon Sep 17 00:00:00 2001 From: Toshiaki Maki Date: Fri, 8 May 2020 19:12:11 +0900 Subject: [PATCH 4/6] This commit makes TraceRpcAutoConfiguration happens only when (#1628) RpcTracing so that excluding brave-instrumentation-rpc dependency does't make an exception. Also add an missing property to disable RPC tracing in additional-spring-configuration-metadata.json --- .../sleuth/instrument/rpc/TraceRpcAutoConfiguration.java | 2 ++ .../META-INF/additional-spring-configuration-metadata.json | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/rpc/TraceRpcAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/rpc/TraceRpcAutoConfiguration.java index d9a9feb63..7ce6b59cc 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/rpc/TraceRpcAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/rpc/TraceRpcAutoConfiguration.java @@ -28,6 +28,7 @@ import brave.sampler.SamplerFunction; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; @@ -45,6 +46,7 @@ import org.springframework.lang.Nullable; @ConditionalOnProperty(name = "spring.sleuth.rpc.enabled", havingValue = "true", matchIfMissing = true) @ConditionalOnBean(Tracing.class) +@ConditionalOnClass(RpcTracing.class) @AutoConfigureAfter(TraceAutoConfiguration.class) public class TraceRpcAutoConfiguration { diff --git a/spring-cloud-sleuth-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-cloud-sleuth-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json index 68e857166..c4b959e08 100644 --- a/spring-cloud-sleuth-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/spring-cloud-sleuth-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -71,6 +71,12 @@ "type": "java.lang.Boolean", "description": "Enable DefaultKafkaHeaderMapper tracing for Kafka.", "defaultValue": true + }, + { + "name": "spring.sleuth.rpc.enabled", + "type": "java.lang.Boolean", + "description": "Enable tracing of RPC.", + "defaultValue": true } ] } From 79f258c2ebd83fa080caa651212d1a29e504a34d Mon Sep 17 00:00:00 2001 From: Toshiaki Maki Date: Fri, 8 May 2020 19:13:16 +0900 Subject: [PATCH 5/6] This commit makes TraceMessagingAutoConfiguration happens only when (#1629) MessagingTracing so that excluding brave-instrumentation-messaging dependency does't make an exception. --- .../instrument/messaging/TraceMessagingAutoConfiguration.java | 1 + 1 file changed, 1 insertion(+) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceMessagingAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceMessagingAutoConfiguration.java index e685b44c5..3ad111f85 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceMessagingAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceMessagingAutoConfiguration.java @@ -89,6 +89,7 @@ import org.springframework.util.ReflectionUtils; */ @Configuration(proxyBeanMethods = false) @ConditionalOnBean(Tracing.class) +@ConditionalOnClass(MessagingTracing.class) @AutoConfigureAfter({ TraceAutoConfiguration.class, TraceSpringMessagingAutoConfiguration.class }) @OnMessagingEnabled From 75756fd0926fd842c592f3a4e02299433c1c711d Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 6 May 2020 17:38:18 +0200 Subject: [PATCH 6/6] Move TraceSchedulingAutoConfiguration @Conditional for optional AspectJ Co-authored-by: Tim te Beek --- .../TraceSchedulingAutoConfiguration.java | 4 +- .../TraceSchedulingAutoConfigurationTest.java | 49 +++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAutoConfigurationTest.java diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAutoConfiguration.java index 45765ce35..065d89ced 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAutoConfiguration.java @@ -29,7 +29,6 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.EnableAspectJAutoProxy; /** * Registers beans related to task scheduling. @@ -40,7 +39,7 @@ import org.springframework.context.annotation.EnableAspectJAutoProxy; * @see TraceSchedulingAspect */ @Configuration(proxyBeanMethods = false) -@EnableAspectJAutoProxy +@ConditionalOnClass(name = "org.aspectj.lang.ProceedingJoinPoint") @ConditionalOnProperty(value = "spring.sleuth.scheduled.enabled", matchIfMissing = true) @ConditionalOnBean(Tracing.class) @AutoConfigureAfter(TraceAutoConfiguration.class) @@ -48,7 +47,6 @@ import org.springframework.context.annotation.EnableAspectJAutoProxy; public class TraceSchedulingAutoConfiguration { @Bean - @ConditionalOnClass(name = "org.aspectj.lang.ProceedingJoinPoint") public TraceSchedulingAspect traceSchedulingAspect(Tracer tracer, SleuthSchedulingProperties sleuthSchedulingProperties) { return new TraceSchedulingAspect(tracer, diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAutoConfigurationTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAutoConfigurationTest.java new file mode 100644 index 000000000..f187ad4e6 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAutoConfigurationTest.java @@ -0,0 +1,49 @@ +/* + * Copyright 2013-2019 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 + * + * https://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.instrument.scheduling; + +import org.aspectj.lang.ProceedingJoinPoint; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.test.context.FilteredClassLoader; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; + +import static org.assertj.core.api.Assertions.assertThat; + +class TraceSchedulingAutoConfigurationTest { + + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(TraceAutoConfiguration.class, + TraceSchedulingAutoConfiguration.class)); + + @Test + void shoud_create_TraceSchedulingAspect() { + this.contextRunner.run(context -> assertThat(context) + .hasSingleBean(TraceSchedulingAspect.class)); + } + + @Test + void shoud_not_create_TraceSchedulingAspect_without_aspectJ() { + this.contextRunner + .withClassLoader(new FilteredClassLoader(ProceedingJoinPoint.class)) + .run(context -> assertThat(context) + .doesNotHaveBean(TraceSchedulingAspect.class)); + } + +}