From 163532fd8ade48a4e2d4e56dda5148992cb47b5e Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Wed, 29 Jan 2020 14:05:46 +0800 Subject: [PATCH 1/3] latest brave --- benchmarks/pom.xml | 2 +- pom.xml | 2 +- spring-cloud-sleuth-dependencies/pom.xml | 4 ++-- spring-cloud-sleuth-samples/pom.xml | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/benchmarks/pom.xml b/benchmarks/pom.xml index c5f8035a3..535a21d6f 100644 --- a/benchmarks/pom.xml +++ b/benchmarks/pom.xml @@ -33,7 +33,7 @@ 1.8 1.8 2.3.0.BUILD-SNAPSHOT - 5.9.1 + 5.9.2 3.14.6 diff --git a/pom.xml b/pom.xml index 032413095..6ba2f17b2 100644 --- a/pom.xml +++ b/pom.xml @@ -257,7 +257,7 @@ Horsham.SR1 2.2.2.BUILD-SNAPSHOT 2.2.2.BUILD-SNAPSHOT - 5.9.1 + 5.9.2 2.1.7.RELEASE 2.2.2.BUILD-SNAPSHOT false diff --git a/spring-cloud-sleuth-dependencies/pom.xml b/spring-cloud-sleuth-dependencies/pom.xml index a019df6fc..56225537b 100644 --- a/spring-cloud-sleuth-dependencies/pom.xml +++ b/spring-cloud-sleuth-dependencies/pom.xml @@ -31,8 +31,8 @@ spring-cloud-sleuth-dependencies Spring Cloud Sleuth Dependencies - 5.9.1 - 0.35.0 + 5.9.2 + 0.35.1 3.4.1 diff --git a/spring-cloud-sleuth-samples/pom.xml b/spring-cloud-sleuth-samples/pom.xml index 12c863448..0390daa62 100644 --- a/spring-cloud-sleuth-samples/pom.xml +++ b/spring-cloud-sleuth-samples/pom.xml @@ -73,7 +73,7 @@ io.zipkin.zipkin2 zipkin - 2.19.2 + 2.19.3 From cba886720d33404d28d43ddbe7c1c762bdf6234e Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Wed, 29 Jan 2020 14:05:46 +0800 Subject: [PATCH 2/3] latest brave --- benchmarks/pom.xml | 3 +- pom.xml | 2 +- .../JmsTracingConfigurationTest.java | 28 ++----------------- spring-cloud-sleuth-samples/pom.xml | 2 +- 4 files changed, 6 insertions(+), 29 deletions(-) diff --git a/benchmarks/pom.xml b/benchmarks/pom.xml index b89d325b3..b0576968e 100644 --- a/benchmarks/pom.xml +++ b/benchmarks/pom.xml @@ -34,7 +34,8 @@ 1.8 1.8 2.1.10.RELEASE - 5.8.0 + 2.3.0.BUILD-SNAPSHOT + 5.9.2 3.11.0 diff --git a/pom.xml b/pom.xml index 4d826eb1d..f4c4ea03b 100644 --- a/pom.xml +++ b/pom.xml @@ -264,7 +264,7 @@ Fishtown.SR4 2.1.5.BUILD-SNAPSHOT 2.1.5.BUILD-SNAPSHOT - 5.8.0 + 5.9.2 2.1.2.RELEASE diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/JmsTracingConfigurationTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/JmsTracingConfigurationTest.java index 13fe6e1e8..4f5e3ce41 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/JmsTracingConfigurationTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/JmsTracingConfigurationTest.java @@ -32,13 +32,11 @@ import javax.jms.XAConnectionFactory; import javax.resource.spi.ResourceAdapter; import brave.Tracing; -import brave.internal.HexCodec; import brave.propagation.CurrentTraceContext; import brave.propagation.TraceContext; import org.apache.activemq.ra.ActiveMQActivationSpec; import org.apache.activemq.ra.ActiveMQResourceAdapter; import org.junit.Test; -import zipkin2.Annotation; import zipkin2.Span; import org.springframework.beans.factory.annotation.Autowired; @@ -281,8 +279,6 @@ public class JmsTracingConfigurationTest { EurekaClientAutoConfiguration.class }) class JmsTestTracingConfiguration { - static final String CONTEXT_LEAK = "context.leak"; - /** * When testing servers or asynchronous clients, spans are reported on a worker * thread. In order to read them on the main thread, we use a concurrent queue. As @@ -304,34 +300,14 @@ class JmsTestTracingConfiguration { return () -> { Span result = this.spans.poll(3, TimeUnit.SECONDS); assertThat(result).withFailMessage("Span was not reported").isNotNull(); - assertThat(result.annotations()).extracting(Annotation::value) - .doesNotContain(CONTEXT_LEAK); return result; }; } @Bean Tracing tracing(CurrentTraceContext currentTraceContext) { - return Tracing.newBuilder().spanReporter(s -> { - // make sure the context was cleared prior to finish.. no leaks! - TraceContext current = currentTraceContext.get(); - boolean contextLeak = false; - if (current != null) { - // add annotation in addition to throwing, in case we are off the main - // thread - if (HexCodec.toLowerHex(current.spanId()).equals(s.id())) { - s = s.toBuilder().addAnnotation(s.timestampAsLong(), CONTEXT_LEAK) - .build(); - contextLeak = true; - } - } - this.spans.add(s); - // throw so that we can see the path to the code that leaked the context - if (contextLeak) { - throw new AssertionError( - CONTEXT_LEAK + " on " + Thread.currentThread().getName()); - } - }).currentTraceContext(currentTraceContext).build(); + return Tracing.newBuilder().spanReporter(spans::add) + .currentTraceContext(currentTraceContext).build(); } } diff --git a/spring-cloud-sleuth-samples/pom.xml b/spring-cloud-sleuth-samples/pom.xml index 3657f9ea2..18c17a060 100644 --- a/spring-cloud-sleuth-samples/pom.xml +++ b/spring-cloud-sleuth-samples/pom.xml @@ -73,7 +73,7 @@ io.zipkin.zipkin2 zipkin - 2.17.0 + 2.19.3 From b830137ac5456e20c3215f197efb505f3259f9e9 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 30 Jan 2020 12:58:37 +0100 Subject: [PATCH 3/3] Fixing wrong ScheduledExecutorService wrapping; fixes gh-1536 --- .../async/ExecutorBeanPostProcessor.java | 24 ++++++++ .../TraceWebClientAutoConfiguration.java | 6 +- .../AsyncDefaultAutoConfigurationTests.java | 56 +++++++++++++++++++ 3 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/AsyncDefaultAutoConfigurationTests.java diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessor.java index 3e7db1624..391374201 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessor.java @@ -21,6 +21,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; @@ -80,6 +81,14 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { log.info("Not instrumenting bean " + beanName); } } + else if (bean instanceof ScheduledExecutorService && !alreadyTraced) { + if (isProxyNeeded(beanName)) { + return wrapScheduledExecutorService(bean); + } + else { + log.info("Not instrumenting bean " + beanName); + } + } else if (bean instanceof ExecutorService && !alreadyTraced) { if (isProxyNeeded(beanName)) { return wrapExecutorService(bean); @@ -104,6 +113,7 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { private boolean alreadyTraced(Object bean) { return bean instanceof LazyTraceThreadPoolTaskExecutor + || bean instanceof TraceableScheduledExecutorService || bean instanceof TraceableExecutorService || bean instanceof LazyTraceAsyncTaskExecutor || bean instanceof LazyTraceExecutor; @@ -148,6 +158,14 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { return createExecutorServiceProxy(bean, cglibProxy, executor); } + private Object wrapScheduledExecutorService(Object bean) { + ScheduledExecutorService executor = (ScheduledExecutorService) bean; + boolean classFinal = Modifier.isFinal(bean.getClass().getModifiers()); + boolean methodFinal = anyFinalMethods(executor, ExecutorService.class); + boolean cglibProxy = !classFinal && !methodFinal; + return createScheduledExecutorServiceProxy(bean, cglibProxy, executor); + } + private Object wrapAsyncTaskExecutor(Object bean) { AsyncTaskExecutor executor = (AsyncTaskExecutor) bean; boolean classFinal = Modifier.isFinal(bean.getClass().getModifiers()); @@ -188,6 +206,12 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { () -> new TraceableExecutorService(this.beanFactory, executor)); } + Object createScheduledExecutorServiceProxy(Object bean, boolean cglibProxy, + ScheduledExecutorService executor) { + return getProxiedObject(bean, cglibProxy, executor, + () -> new TraceableScheduledExecutorService(this.beanFactory, executor)); + } + Object createAsyncTaskExecutorProxy(Object bean, boolean cglibProxy, AsyncTaskExecutor executor) { return getProxiedObject(bean, cglibProxy, executor, () -> { 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 3028c6f90..8a004930c 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 @@ -160,7 +160,7 @@ public class TraceWebClientAutoConfiguration { static class NettyConfiguration { @Bean - public HttpClientBeanPostProcessor httpClientBeanPostProcessor( + static HttpClientBeanPostProcessor httpClientBeanPostProcessor( BeanFactory beanFactory) { return new HttpClientBeanPostProcessor(beanFactory); } @@ -173,14 +173,14 @@ public class TraceWebClientAutoConfiguration { protected static class TraceOAuthConfiguration { @Bean - UserInfoRestTemplateCustomizerBPP userInfoRestTemplateCustomizerBeanPostProcessor( + static UserInfoRestTemplateCustomizerBPP userInfoRestTemplateCustomizerBeanPostProcessor( BeanFactory beanFactory) { return new UserInfoRestTemplateCustomizerBPP(beanFactory); } @Bean @ConditionalOnMissingBean - UserInfoRestTemplateCustomizer traceUserInfoRestTemplateCustomizer( + static UserInfoRestTemplateCustomizer traceUserInfoRestTemplateCustomizer( BeanFactory beanFactory) { return new TraceUserInfoRestTemplateCustomizer(beanFactory); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/AsyncDefaultAutoConfigurationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/AsyncDefaultAutoConfigurationTests.java new file mode 100644 index 000000000..0177d66a4 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/AsyncDefaultAutoConfigurationTests.java @@ -0,0 +1,56 @@ +/* + * Copyright 2013-2020 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.async; + +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; + +import org.assertj.core.api.BDDAssertions; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.junit4.SpringRunner; + +@RunWith(SpringRunner.class) +@SpringBootTest(classes = AsyncDefaultAutoConfigurationTests.Config.class) +public class AsyncDefaultAutoConfigurationTests { + + @Autowired + ScheduledExecutorService executor; + + @Test + public void should_work_with_proxies() { + BDDAssertions.then(this.executor).isNotNull(); + } + + @Configuration + @EnableAutoConfiguration + static class Config { + + @Bean + public ScheduledExecutorService createExecutorService() { + return Executors.newSingleThreadScheduledExecutor(); + } + + } + +}