From e9d12703c1fa04a0b36bc1d3cd05f30fbc4d905d Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sun, 17 May 2020 08:38:08 +0800 Subject: [PATCH 1/2] Pares back some deprecation after analyzing each file (#1640) This puts specific comments in as to why certain files that seem like they shouldn't be public are. Notably, this includes entrypoint autoconfiguration, which are sometimes order sensitive. In other cases there are types that were documented (notably the async package). This also untangles a few configuration. --- .../autoconfig/TraceAutoConfiguration.java | 48 +++++++++---------- .../async/LazyTraceAsyncCustomizer.java | 4 +- .../async/LazyTraceAsyncTaskExecutor.java | 4 +- .../instrument/async/LazyTraceExecutor.java | 4 +- .../LazyTraceThreadPoolTaskExecutor.java | 4 +- .../instrument/async/TraceCallable.java | 4 +- .../instrument/async/TraceRunnable.java | 4 +- .../async/TraceableExecutorService.java | 4 +- .../TraceableScheduledExecutorService.java | 4 +- .../TraceMessagingAutoConfiguration.java | 19 ++++---- .../rpc/TraceRpcAutoConfiguration.java | 19 ++++---- .../web/TraceHttpAutoConfiguration.java | 40 +++++++++------- .../TraceWebAsyncClientAutoConfiguration.java | 4 +- .../TraceWebClientAutoConfiguration.java | 4 +- .../sleuth/log/SleuthSlf4jProperties.java | 1 - .../sampler/SamplerAutoConfiguration.java | 3 -- .../TracingJmsListenerEndpointRegistry.java | 3 -- .../main/resources/META-INF/spring.factories | 1 - 18 files changed, 75 insertions(+), 99 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 bf8f74f51..7b94d441c 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 @@ -16,7 +16,6 @@ package org.springframework.cloud.sleuth.autoconfig; -import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -70,14 +69,13 @@ import org.springframework.util.StringUtils; * @author Marcin Grzejszczak * @author Tim Ysewyn * @since 2.0.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated @Configuration(proxyBeanMethods = false) @ConditionalOnProperty(value = "spring.sleuth.enabled", matchIfMissing = true) @EnableConfigurationProperties(SleuthProperties.class) @Import({ SleuthLogAutoConfiguration.class, SamplerAutoConfiguration.class }) +// public allows @AutoConfigureAfter(TraceAutoConfiguration) +// for components needing Tracing public class TraceAutoConfiguration { /** @@ -90,42 +88,40 @@ public class TraceAutoConfiguration { */ public static final String DEFAULT_SERVICE_NAME = "default"; - @Autowired(required = false) - List spanAdjusters = new ArrayList<>(); - - @Autowired(required = false) - List spanHandlers = new ArrayList<>(); - @Autowired(required = false) ExtraFieldPropagation.FactoryBuilder extraFieldPropagationFactoryBuilder; - @Autowired(required = false) - List tracingCustomizers = new ArrayList<>(); - - @Autowired(required = false) - List extraFieldCustomizers = new ArrayList<>(); - @Bean @ConditionalOnMissingBean // NOTE: stable bean name as might be used outside sleuth Tracing tracing(@LocalServiceName String serviceName, Propagation.Factory factory, CurrentTraceContext currentTraceContext, Sampler sampler, ErrorParser errorParser, SleuthProperties sleuthProperties, - @Nullable List> spanReporters) { + @Nullable List> spanReporters, + @Nullable List spanAdjusters, + @Nullable List spanHandlers, + @Nullable List tracingCustomizers) { + if (spanAdjusters == null) { + spanAdjusters = Collections.emptyList(); + } Tracing.Builder builder = Tracing.newBuilder().sampler(sampler) .errorParser(errorParser) .localServiceName(StringUtils.isEmpty(serviceName) ? DEFAULT_SERVICE_NAME : serviceName) .propagationFactory(factory).currentTraceContext(currentTraceContext) - .spanReporter(new CompositeReporter(this.spanAdjusters, + .spanReporter(new CompositeReporter(spanAdjusters, spanReporters != null ? spanReporters : Collections.emptyList())) .traceId128Bit(sleuthProperties.isTraceId128()) .supportsJoin(sleuthProperties.isSupportsJoin()); - for (SpanHandler spanHandlerFactory : this.spanHandlers) { - builder.addSpanHandler(spanHandlerFactory); + if (spanHandlers != null) { + for (SpanHandler spanHandlerFactory : spanHandlers) { + builder.addSpanHandler(spanHandlerFactory); + } } - for (TracingCustomizer customizer : this.tracingCustomizers) { - customizer.customize(builder); + if (tracingCustomizers != null) { + for (TracingCustomizer customizer : tracingCustomizers) { + customizer.customize(builder); + } } return builder.build(); } @@ -144,7 +140,11 @@ public class TraceAutoConfiguration { @Bean @ConditionalOnMissingBean - Propagation.Factory sleuthPropagation(SleuthProperties sleuthProperties) { + Propagation.Factory sleuthPropagation(SleuthProperties sleuthProperties, + List extraFieldCustomizers) { + if (extraFieldCustomizers == null) { + extraFieldCustomizers = Collections.emptyList(); + } if (sleuthProperties.getBaggageKeys().isEmpty() && sleuthProperties.getPropagationKeys().isEmpty() && extraFieldCustomizers.isEmpty() @@ -177,7 +177,7 @@ public class TraceAutoConfiguration { factoryBuilder = factoryBuilder.addRedactedField(key); } } - for (ExtraFieldCustomizer customizer : this.extraFieldCustomizers) { + for (ExtraFieldCustomizer customizer : extraFieldCustomizers) { customizer.customize(factoryBuilder); } return factoryBuilder.build(); diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceAsyncCustomizer.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceAsyncCustomizer.java index 76d35e539..2e39af032 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceAsyncCustomizer.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceAsyncCustomizer.java @@ -29,10 +29,8 @@ import org.springframework.scheduling.annotation.AsyncConfigurerSupport; * * @author Dave Syer * @since 1.0.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated +// public as most types in this package were documented for use public class LazyTraceAsyncCustomizer extends AsyncConfigurerSupport { private final BeanFactory beanFactory; diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceAsyncTaskExecutor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceAsyncTaskExecutor.java index acd608f4e..de5e32bc0 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceAsyncTaskExecutor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceAsyncTaskExecutor.java @@ -35,10 +35,8 @@ import org.springframework.core.task.AsyncTaskExecutor; * * @author Marcin Grzejszczak * @since 2.1.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated +// public as most types in this package were documented for use public class LazyTraceAsyncTaskExecutor implements AsyncTaskExecutor { private static final Log log = LogFactory.getLog(LazyTraceAsyncTaskExecutor.class); diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceExecutor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceExecutor.java index 3c86536a2..c68c67e6e 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceExecutor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceExecutor.java @@ -32,10 +32,8 @@ import org.springframework.cloud.sleuth.SpanNamer; * * @author Dave Syer * @since 1.0.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated +// public as most types in this package were documented for use public class LazyTraceExecutor implements Executor { private static final Log log = LogFactory.getLog(LazyTraceExecutor.class); diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceThreadPoolTaskExecutor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceThreadPoolTaskExecutor.java index e29b62531..34ca5ea15 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceThreadPoolTaskExecutor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/LazyTraceThreadPoolTaskExecutor.java @@ -39,11 +39,9 @@ import org.springframework.util.concurrent.ListenableFuture; * * @author Marcin Grzejszczak * @since 1.0.10 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated @SuppressWarnings("serial") +// public as most types in this package were documented for use public class LazyTraceThreadPoolTaskExecutor extends ThreadPoolTaskExecutor { private static final Log log = LogFactory diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceCallable.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceCallable.java index d7f5ba688..054d643cb 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceCallable.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceCallable.java @@ -33,10 +33,8 @@ import org.springframework.cloud.sleuth.SpanNamer; * @author Spencer Gibb * @author Marcin Grzejszczak * @since 1.0.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated +// public as most types in this package were documented for use public class TraceCallable implements Callable { /** diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceRunnable.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceRunnable.java index 5fa349143..33c943eff 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceRunnable.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceRunnable.java @@ -30,10 +30,8 @@ import org.springframework.cloud.sleuth.SpanNamer; * @author Spencer Gibb * @author Marcin Grzejszczak * @since 1.0.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated +// public as most types in this package were documented for use public class TraceRunnable implements Runnable { /** diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceableExecutorService.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceableExecutorService.java index f5a4edea4..6ca48073f 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceableExecutorService.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceableExecutorService.java @@ -36,10 +36,8 @@ import org.springframework.cloud.sleuth.SpanNamer; * * @author Gaurav Rai Mazra * @since 1.0.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated +// public as most types in this package were documented for use public class TraceableExecutorService implements ExecutorService { final ExecutorService delegate; diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceableScheduledExecutorService.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceableScheduledExecutorService.java index 936d1aa45..0de1b4d60 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceableScheduledExecutorService.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceableScheduledExecutorService.java @@ -29,10 +29,8 @@ import org.springframework.beans.factory.BeanFactory; * * @author Gaurav Rai Mazra * @since 1.0.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated +// public as most types in this package were documented for use public class TraceableScheduledExecutorService extends TraceableExecutorService implements ScheduledExecutorService { 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 363ed298b..6886f7534 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 @@ -17,7 +17,6 @@ package org.springframework.cloud.sleuth.instrument.messaging; import java.lang.reflect.Field; -import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -49,7 +48,6 @@ import org.springframework.amqp.rabbit.core.RabbitTemplate; import org.springframework.aop.framework.ProxyFactoryBean; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.boot.autoconfigure.AutoConfigureAfter; @@ -86,10 +84,7 @@ import org.springframework.util.ReflectionUtils; * * @author Marcin Grzejszczak * @since 2.0.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated @Configuration(proxyBeanMethods = false) @ConditionalOnBean(Tracing.class) @ConditionalOnClass(MessagingTracing.class) @@ -97,17 +92,17 @@ import org.springframework.util.ReflectionUtils; TraceSpringMessagingAutoConfiguration.class }) @OnMessagingEnabled @EnableConfigurationProperties(SleuthMessagingProperties.class) +// public allows @AutoConfigureAfter(TraceMessagingAutoConfiguration) +// for components needing MessagingTracing public class TraceMessagingAutoConfiguration { - @Autowired(required = false) - List messagingTracingCustomizers = new ArrayList<>(); - @Bean @ConditionalOnMissingBean // NOTE: stable bean name as might be used outside sleuth MessagingTracing messagingTracing(Tracing tracing, @Nullable @ProducerSampler SamplerFunction producerSampler, - @Nullable @ConsumerSampler SamplerFunction consumerSampler) { + @Nullable @ConsumerSampler SamplerFunction consumerSampler, + @Nullable List messagingTracingCustomizers) { MessagingTracing.Builder builder = MessagingTracing.newBuilder(tracing); if (producerSampler != null) { @@ -116,8 +111,10 @@ public class TraceMessagingAutoConfiguration { if (consumerSampler != null) { builder.consumerSampler(consumerSampler); } - for (MessagingTracingCustomizer customizer : this.messagingTracingCustomizers) { - customizer.customize(builder); + if (messagingTracingCustomizers != null) { + for (MessagingTracingCustomizer customizer : messagingTracingCustomizers) { + customizer.customize(builder); + } } return builder.build(); } 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 f92f748af..ab0c623bc 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 @@ -16,7 +16,6 @@ package org.springframework.cloud.sleuth.instrument.rpc; -import java.util.ArrayList; import java.util.List; import brave.Tracing; @@ -25,7 +24,6 @@ import brave.rpc.RpcTracing; import brave.rpc.RpcTracingCustomizer; 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; @@ -41,27 +39,24 @@ import org.springframework.lang.Nullable; * Auto-configuration} related to RPC based communication. * * @since 2.2.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated @Configuration(proxyBeanMethods = false) @ConditionalOnProperty(name = "spring.sleuth.rpc.enabled", havingValue = "true", matchIfMissing = true) @ConditionalOnBean(Tracing.class) @ConditionalOnClass(RpcTracing.class) @AutoConfigureAfter(TraceAutoConfiguration.class) +// public allows @AutoConfigureAfter(TraceRpcAutoConfiguration) +// for components needing RpcTracing public class TraceRpcAutoConfiguration { - @Autowired(required = false) - List rpcTracingCustomizers = new ArrayList<>(); - @Bean @ConditionalOnMissingBean // NOTE: stable bean name as might be used outside sleuth RpcTracing rpcTracing(Tracing tracing, @Nullable @RpcClientSampler SamplerFunction clientSampler, - @Nullable @RpcServerSampler SamplerFunction serverSampler) { + @Nullable @RpcServerSampler SamplerFunction serverSampler, + @Nullable List rpcTracingCustomizers) { RpcTracing.Builder builder = RpcTracing.newBuilder(tracing); if (clientSampler != null) { @@ -70,8 +65,10 @@ public class TraceRpcAutoConfiguration { if (serverSampler != null) { builder.serverSampler(serverSampler); } - for (RpcTracingCustomizer customizer : this.rpcTracingCustomizers) { - customizer.customize(builder); + if (rpcTracingCustomizers != null) { + for (RpcTracingCustomizer customizer : rpcTracingCustomizers) { + customizer.customize(builder); + } } return builder.build(); } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java index a4f44d3a0..f780254e3 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java @@ -16,7 +16,6 @@ package org.springframework.cloud.sleuth.instrument.web; -import java.util.ArrayList; import java.util.List; import java.util.regex.Pattern; @@ -30,14 +29,16 @@ import brave.http.HttpTracingCustomizer; import brave.sampler.SamplerFunction; import brave.sampler.SamplerFunctions; -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.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.Import; import org.springframework.core.Ordered; import org.springframework.lang.Nullable; @@ -47,23 +48,26 @@ import org.springframework.lang.Nullable; * * @author Marcin Grzejszczak * @since 2.0.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated @Configuration(proxyBeanMethods = false) -@ConditionalOnBean(TraceWebAutoConfiguration.class) -@ConditionalOnProperty(name = "spring.sleuth.http.enabled", havingValue = "true", - matchIfMissing = true) -@AutoConfigureAfter(TraceWebAutoConfiguration.class) -@EnableConfigurationProperties({ TraceKeys.class, SleuthHttpLegacyProperties.class }) +// This was formerly conditional on TraceWebAutoConfiguration, which was +// conditional on "spring.sleuth.web.enabled". As this is conditional on +// "spring.sleuth.http.enabled", to be compatible with old behavior we have +// to be conditional on two properties. +@ConditionalOnProperty( + name = { "spring.sleuth.http.enabled", "spring.sleuth.web.enabled" }, + havingValue = "true", matchIfMissing = true) +@ConditionalOnBean(Tracing.class) +@ConditionalOnClass(HttpTracing.class) +@AutoConfigureAfter(TraceAutoConfiguration.class) +@Import(TraceWebAutoConfiguration.class) +@EnableConfigurationProperties(TraceKeys.class) +// public allows @AutoConfigureAfter(TraceHttpAutoConfiguration) +// for components needing HttpTracing public class TraceHttpAutoConfiguration { static final int TRACING_FILTER_ORDER = Ordered.HIGHEST_PRECEDENCE + 5; - @Autowired(required = false) - List httpTracingCustomizers = new ArrayList<>(); - @Bean @ConditionalOnMissingBean // NOTE: stable bean name as might be used outside sleuth @@ -76,7 +80,8 @@ public class TraceHttpAutoConfiguration { @Nullable brave.http.HttpServerParser serverParser, @HttpClientSampler SamplerFunction httpClientSampler, @Nullable @ServerSampler HttpSampler serverSampler, - @Nullable @HttpServerSampler SamplerFunction httpServerSampler) { + @Nullable @HttpServerSampler SamplerFunction httpServerSampler, + @Nullable List httpTracingCustomizers) { if (httpServerSampler == null) { httpServerSampler = serverSampler; } @@ -109,10 +114,11 @@ public class TraceHttpAutoConfiguration { builder.serverParser(serverParser); } - for (HttpTracingCustomizer customizer : this.httpTracingCustomizers) { - customizer.customize(builder); + if (httpTracingCustomizers != null) { + for (HttpTracingCustomizer customizer : httpTracingCustomizers) { + customizer.customize(builder); + } } - return builder.build(); } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebAsyncClientAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebAsyncClientAutoConfiguration.java index 3b21c5c3a..8d16fc7b7 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebAsyncClientAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebAsyncClientAutoConfiguration.java @@ -30,7 +30,7 @@ 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.ConditionalOnProperty; -import org.springframework.cloud.sleuth.instrument.web.TraceWebServletAutoConfiguration; +import org.springframework.cloud.sleuth.instrument.web.TraceHttpAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.client.AsyncClientHttpRequestFactory; @@ -54,7 +54,7 @@ import org.springframework.web.client.AsyncRestTemplate; matchIfMissing = true) @ConditionalOnClass(AsyncRestTemplate.class) @ConditionalOnBean(HttpTracing.class) -@AutoConfigureAfter(TraceWebServletAutoConfiguration.class) +@AutoConfigureAfter(TraceHttpAutoConfiguration.class) public class TraceWebAsyncClientAutoConfiguration { @Configuration(proxyBeanMethods = false) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java index 725388453..f0f34a8cc 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java @@ -42,7 +42,7 @@ import org.springframework.boot.autoconfigure.security.oauth2.resource.UserInfoR import org.springframework.boot.web.client.RestTemplateCustomizer; import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.gateway.filter.headers.HttpHeadersFilter; -import org.springframework.cloud.sleuth.instrument.web.TraceWebServletAutoConfiguration; +import org.springframework.cloud.sleuth.instrument.web.TraceHttpAutoConfiguration; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -69,7 +69,7 @@ import org.springframework.web.reactive.function.client.WebClient; @Configuration(proxyBeanMethods = false) @SleuthWebClientEnabled @ConditionalOnBean(HttpTracing.class) -@AutoConfigureAfter(TraceWebServletAutoConfiguration.class) +@AutoConfigureAfter(TraceHttpAutoConfiguration.class) @AutoConfigureBefore(HttpClientConfiguration.class) public class TraceWebClientAutoConfiguration { diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthSlf4jProperties.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthSlf4jProperties.java index 1b84071eb..ffcdddd96 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthSlf4jProperties.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthSlf4jProperties.java @@ -26,7 +26,6 @@ import org.springframework.boot.context.properties.ConfigurationProperties; * * @author Arthur Gavlyukovskiy * @since 1.0.12 - * 3.0 */ @ConfigurationProperties("spring.sleuth.log.slf4j") // TODO: Hide in 3.x, if it isn't already deleted diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/SamplerAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/SamplerAutoConfiguration.java index 9a91461df..473fbdd5e 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/SamplerAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/SamplerAutoConfiguration.java @@ -33,10 +33,7 @@ import org.springframework.context.annotation.Configuration; * @author Marcin Grzejszczak * @see SamplerCondition * @since 2.1.0 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated @Configuration(proxyBeanMethods = false) @EnableConfigurationProperties(SamplerProperties.class) // This is not auto-configuration, but it was in the past. Leaving the name as diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/jms/config/TracingJmsListenerEndpointRegistry.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/jms/config/TracingJmsListenerEndpointRegistry.java index 3a93bacbc..c0088eaf0 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/jms/config/TracingJmsListenerEndpointRegistry.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/jms/config/TracingJmsListenerEndpointRegistry.java @@ -46,10 +46,7 @@ import org.springframework.lang.Nullable; * * @author Marcin Grzejszczak * @since 2.1.1 - * @deprecated This type should have never been public and will be hidden or removed in - * 3.0 */ -@Deprecated public final class TracingJmsListenerEndpointRegistry extends JmsListenerEndpointRegistry { diff --git a/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories b/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories index fcad7b053..a522ae58c 100644 --- a/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories +++ b/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories @@ -4,7 +4,6 @@ org.springframework.cloud.sleuth.annotation.SleuthAnnotationAutoConfiguration,\ org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration,\ org.springframework.cloud.sleuth.propagation.SleuthTagPropagationAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.TraceHttpAutoConfiguration,\ -org.springframework.cloud.sleuth.instrument.web.TraceWebAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.TraceWebServletAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.client.TraceWebClientAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.client.TraceWebAsyncClientAutoConfiguration,\ From 60bf5ce49772c9e450042b5e8aa088de7b376e11 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sun, 17 May 2020 14:21:30 +0800 Subject: [PATCH 2/2] Rewrites async test to be less circuitous and more valid (#1641) I had flakes on TraceAsyncIntegrationTests and it was hard to tell why as there were so many problems with it. It used a normal list in async setting, it unnecessarily created extra spans, it added a hook inside the async method which if passed only told you it was in the middle of an op, not the end of it. The method danced around making failure in console almost impossible to decipher. This took me a few hours to make just a normal test. Hopefully, the others will keep passing as I can't afford time to redo all of them. --- .../pom.xml | 5 + .../async/TraceAsyncIntegrationTests.java | 214 +++++------------- 2 files changed, 64 insertions(+), 155 deletions(-) diff --git a/tests/spring-cloud-sleuth-instrumentation-async-tests/pom.xml b/tests/spring-cloud-sleuth-instrumentation-async-tests/pom.xml index 947798f7f..b49903bec 100644 --- a/tests/spring-cloud-sleuth-instrumentation-async-tests/pom.xml +++ b/tests/spring-cloud-sleuth-instrumentation-async-tests/pom.xml @@ -59,6 +59,11 @@ org.springframework.cloud spring-cloud-starter-sleuth + + io.zipkin.brave + brave-tests + test + org.springframework.boot spring-boot-starter-test diff --git a/tests/spring-cloud-sleuth-instrumentation-async-tests/src/test/java/org/springframework/cloud/sleuth/instrument/async/TraceAsyncIntegrationTests.java b/tests/spring-cloud-sleuth-instrumentation-async-tests/src/test/java/org/springframework/cloud/sleuth/instrument/async/TraceAsyncIntegrationTests.java index 3ca91cd8f..a71d3df1f 100644 --- a/tests/spring-cloud-sleuth-instrumentation-async-tests/src/test/java/org/springframework/cloud/sleuth/instrument/async/TraceAsyncIntegrationTests.java +++ b/tests/spring-cloud-sleuth-instrumentation-async-tests/src/test/java/org/springframework/cloud/sleuth/instrument/async/TraceAsyncIntegrationTests.java @@ -16,183 +16,102 @@ package org.springframework.cloud.sleuth.instrument.async; -import java.util.AbstractMap; -import java.util.List; -import java.util.concurrent.atomic.AtomicReference; - -import brave.Span; -import brave.Tracer; -import brave.Tracing; -import brave.sampler.Sampler; -import org.awaitility.Awaitility; -import org.junit.Before; +import brave.SpanCustomizer; +import brave.handler.MutableSpan; +import brave.handler.SpanHandler; +import brave.propagation.CurrentTraceContext; +import brave.propagation.CurrentTraceContext.Scope; +import brave.propagation.TraceContext; +import brave.test.IntegrationTestSpanHandler; +import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.cloud.sleuth.SpanName; -import org.springframework.cloud.sleuth.util.ArrayListSpanReporter; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.scheduling.annotation.Async; import org.springframework.scheduling.annotation.EnableAsync; import org.springframework.test.context.junit4.SpringRunner; -import static java.util.concurrent.TimeUnit.SECONDS; -import static org.assertj.core.api.BDDAssertions.then; +import static org.assertj.core.api.Assertions.assertThat; @RunWith(SpringRunner.class) @SpringBootTest( + webEnvironment = SpringBootTest.WebEnvironment.NONE, classes = { TraceAsyncIntegrationTests.TraceAsyncITestConfiguration.class }) public class TraceAsyncIntegrationTests { - @Autowired - ClassPerformingAsyncLogic classPerformingAsyncLogic; + @ClassRule + public static IntegrationTestSpanHandler spans = new IntegrationTestSpanHandler(); + + TraceContext context = TraceContext.newBuilder().traceId(1).spanId(2).sampled(true) + .build(); @Autowired - Tracing tracer; + AsyncLogic asyncLogic; @Autowired - ArrayListSpanReporter reporter; - - @Before - public void cleanup() { - this.classPerformingAsyncLogic.clear(); - this.reporter.clear(); - } + CurrentTraceContext currentTraceContext; @Test public void should_set_span_on_an_async_annotated_method() { - whenAsyncProcessingTakesPlace(); + asyncLogic.invokeAsync(); - thenANewAsyncSpanGetsCreated(); + assertSpan_invokeAsync(takeDesirableSpan()); } @Test public void should_set_span_with_custom_method_on_an_async_annotated_method() { - whenAsyncProcessingTakesPlaceWithCustomSpanName(); + asyncLogic.invokeAsync_customName(); - thenAsyncSpanHasCustomName(); + assertSpan_invokeAsync_customName(takeDesirableSpan()); } @Test public void should_continue_a_span_on_an_async_annotated_method() { - Span span = givenASpanInCurrentThread(); + try (Scope ws = currentTraceContext.maybeScope(context)) { + asyncLogic.invokeAsync(); - try (Tracer.SpanInScope ws = this.tracer.tracer().withSpanInScope(span.start())) { - whenAsyncProcessingTakesPlace(); - - thenTraceIdIsPassedFromTheCurrentThreadToTheAsyncOne(span); - } - finally { - span.finish(); + MutableSpan span = assertSpan_invokeAsync(takeDesirableSpan()); + assertThat(span.traceId()).isEqualTo(context.traceIdString()); } } @Test public void should_continue_a_span_with_custom_method_on_an_async_annotated_method() { - Span span = givenASpanInCurrentThread(); + try (Scope ws = currentTraceContext.maybeScope(context)) { + asyncLogic.invokeAsync_customName(); - try (Tracer.SpanInScope ws = this.tracer.tracer().withSpanInScope(span.start())) { - whenAsyncProcessingTakesPlaceWithCustomSpanName(); - - thenTraceIdIsPassedFromTheCurrentThreadToTheAsyncOneAndSpanHasCustomName( - span); - } - finally { - span.finish(); + MutableSpan span = assertSpan_invokeAsync_customName(takeDesirableSpan()); + assertThat(span.traceId()).isEqualTo(context.traceIdString()); } } - private Span givenASpanInCurrentThread() { - return this.tracer.tracer().nextSpan().name("http:existing"); + static MutableSpan assertSpan_invokeAsync_customName(MutableSpan span) { + assertThat(span.name()).isEqualTo("foo"); + assertThat(span.containsAnnotation("@Async")).isTrue(); + assertThat(span.tags()).containsEntry("class", "AsyncLogic") + .containsEntry("method", "invokeAsync_customName"); + return span; } - private void whenAsyncProcessingTakesPlace() { - this.classPerformingAsyncLogic.invokeAsynchronousLogic(); + static MutableSpan assertSpan_invokeAsync(MutableSpan span) { + assertThat(span.name()).isEqualTo("invoke-async"); + assertThat(span.containsAnnotation("@Async")).isTrue(); + assertThat(span.tags()).containsEntry("class", "AsyncLogic") + .containsEntry("method", "invokeAsync"); + return span; } - private void whenAsyncProcessingTakesPlaceWithCustomSpanName() { - this.classPerformingAsyncLogic.customNameInvokeAsynchronousLogic(); - } - - private void thenTraceIdIsPassedFromTheCurrentThreadToTheAsyncOne(final Span span) { - Awaitility.await().atMost(5, SECONDS).untilAsserted(() -> { - Span asyncSpan = TraceAsyncIntegrationTests.this.classPerformingAsyncLogic - .getSpan(); - then(asyncSpan.context().traceId()).isEqualTo(span.context().traceId()); - List spans = TraceAsyncIntegrationTests.this.reporter - .getSpans(); - zipkin2.Span reportedAsyncSpan = spans.stream() - .filter(span2 -> span2.name().equals("invoke-asynchronous-logic")) - .findFirst().orElseThrow(() -> new AssertionError( - "Should have a span with custom name")); - then(reportedAsyncSpan.traceId()).isEqualTo(span.context().traceIdString()); - then(reportedAsyncSpan.name()).isEqualTo("invoke-asynchronous-logic"); - then(reportedAsyncSpan.tags()) - .contains(new AbstractMap.SimpleEntry<>("class", - "ClassPerformingAsyncLogic")) - .contains(new AbstractMap.SimpleEntry<>("method", - "invokeAsynchronousLogic")); - }); - } - - private void thenANewAsyncSpanGetsCreated() { - Awaitility.await().atMost(5, SECONDS).untilAsserted(() -> { - List spans = TraceAsyncIntegrationTests.this.reporter - .getSpans(); - then(spans).hasSize(2); - zipkin2.Span reportedAsyncSpan = spans.stream() - .filter(span -> span.name().equals("invoke-asynchronous-logic")) - .findFirst().orElseThrow(() -> new AssertionError( - "Should have a span with custom name")); - then(reportedAsyncSpan.tags()) - .contains(new AbstractMap.SimpleEntry<>("class", - "ClassPerformingAsyncLogic")) - .contains(new AbstractMap.SimpleEntry<>("method", - "invokeAsynchronousLogic")); - }); - } - - private void thenTraceIdIsPassedFromTheCurrentThreadToTheAsyncOneAndSpanHasCustomName( - final Span span) { - Awaitility.await().atMost(5, SECONDS).untilAsserted(() -> { - Span asyncSpan = TraceAsyncIntegrationTests.this.classPerformingAsyncLogic - .getSpan(); - then(asyncSpan.context().traceId()).isEqualTo(span.context().traceId()); - List spans = TraceAsyncIntegrationTests.this.reporter - .getSpans(); - then(spans).hasSize(2); - zipkin2.Span reportedAsyncSpan = spans.stream() - .filter(span2 -> span2.name().equals("foo")).findFirst() - .orElseThrow(() -> new AssertionError( - "Should have a span with custom name")); - then(reportedAsyncSpan.traceId()).isEqualTo(span.context().traceIdString()); - then(reportedAsyncSpan.name()).isEqualTo("foo"); - then(reportedAsyncSpan.tags()) - .contains(new AbstractMap.SimpleEntry<>("class", - "ClassPerformingAsyncLogic")) - .contains(new AbstractMap.SimpleEntry<>("method", - "customNameInvokeAsynchronousLogic")); - }); - } - - private void thenAsyncSpanHasCustomName() { - Awaitility.await().atMost(5, SECONDS).untilAsserted(() -> { - List spans = TraceAsyncIntegrationTests.this.reporter - .getSpans(); - zipkin2.Span reportedAsyncSpan = spans.stream() - .filter(span2 -> span2.name().equals("foo")).findFirst() - .orElseThrow(() -> new AssertionError( - "Should have a span with custom name")); - then(reportedAsyncSpan.name()).isEqualTo("foo"); - then(reportedAsyncSpan.tags()) - .contains(new AbstractMap.SimpleEntry<>("class", - "ClassPerformingAsyncLogic")) - .contains(new AbstractMap.SimpleEntry<>("method", - "customNameInvokeAsynchronousLogic")); - }); + // Sleuth adds spans named "async" with no tags when an executor is used. + // We don't want that one. + MutableSpan takeDesirableSpan() { + MutableSpan span1 = spans.takeLocalSpan(); + MutableSpan span2 = spans.takeLocalSpan(); + return span1.name().equals("async") ? span2 : span1; } @DefaultTestAutoConfiguration @@ -201,49 +120,34 @@ public class TraceAsyncIntegrationTests { static class TraceAsyncITestConfiguration { @Bean - ClassPerformingAsyncLogic asyncClass(Tracer tracer) { - return new ClassPerformingAsyncLogic(tracer); + AsyncLogic asyncLogic(SpanCustomizer customizer) { + return new AsyncLogic(customizer); } @Bean - Sampler defaultSampler() { - return Sampler.ALWAYS_SAMPLE; - } - - @Bean - ArrayListSpanReporter reporter() { - return new ArrayListSpanReporter(); + SpanHandler spanHandler() { + return spans; } } - static class ClassPerformingAsyncLogic { + static class AsyncLogic { - private final Tracer tracer; + final SpanCustomizer customizer; - AtomicReference span = new AtomicReference<>(); - - ClassPerformingAsyncLogic(Tracer tracer) { - this.tracer = tracer; + AsyncLogic(SpanCustomizer customizer) { + this.customizer = customizer; } @Async - public void invokeAsynchronousLogic() { - this.span.set(this.tracer.currentSpan()); + public void invokeAsync() { + customizer.annotate("@Async"); // proves the handler is in scope } @Async @SpanName("foo") - public void customNameInvokeAsynchronousLogic() { - this.span.set(this.tracer.currentSpan()); - } - - public Span getSpan() { - return this.span.get(); - } - - public void clear() { - this.span.set(null); + public void invokeAsync_customName() { + customizer.annotate("@Async"); // proves the handler is in scope } }