From 4cbf409ee71db0d038ee5890710383d0d104355d Mon Sep 17 00:00:00 2001 From: fvlad Date: Tue, 18 Jun 2019 18:15:53 +0300 Subject: [PATCH] Decorate with Traceable Scheduled Executor service (#1375) * Decorate with Traceable Scheduled Executor service * Check for decorating bean that is already tracing --- .../async/ExecutorBeanPostProcessor.java | 29 ++++--- .../async/ExecutorBeanPostProcessorTests.java | 78 ++++++++++++++++++- 2 files changed, 95 insertions(+), 12 deletions(-) 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 443a82041..6de8313ff 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.function.Supplier; @@ -47,6 +48,7 @@ import org.springframework.util.ReflectionUtils; * @author Marcin Grzejszczak * @author Jesus Alonso * @author Denys Ivano + * @author Vladislav Fefelov * @since 1.1.4 */ class ExecutorBeanPostProcessor implements BeanPostProcessor { @@ -70,8 +72,14 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { @Override public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { - if (bean instanceof ThreadPoolTaskExecutor - && !(bean instanceof LazyTraceThreadPoolTaskExecutor)) { + if (bean instanceof LazyTraceThreadPoolTaskExecutor + || bean instanceof TraceableExecutorService + || bean instanceof LazyTraceAsyncTaskExecutor + || bean instanceof LazyTraceExecutor) { + log.info("Bean is already instrumented " + beanName); + return bean; + } + if (bean instanceof ThreadPoolTaskExecutor) { if (isProxyNeeded(beanName)) { return wrapThreadPoolTaskExecutor(bean); } @@ -79,8 +87,7 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { log.info("Not instrumenting bean " + beanName); } } - else if (bean instanceof ExecutorService - && !(bean instanceof TraceableExecutorService)) { + else if (bean instanceof ExecutorService) { if (isProxyNeeded(beanName)) { return wrapExecutorService(bean); } @@ -88,8 +95,7 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { log.info("Not instrumenting bean " + beanName); } } - else if (bean instanceof AsyncTaskExecutor - && !(bean instanceof LazyTraceAsyncTaskExecutor)) { + else if (bean instanceof AsyncTaskExecutor) { if (isProxyNeeded(beanName)) { return wrapAsyncTaskExecutor(bean); } @@ -97,7 +103,7 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { log.info("Not instrumenting bean " + beanName); } } - else if (bean instanceof Executor && !(bean instanceof LazyTraceExecutor)) { + else if (bean instanceof Executor) { return wrapExecutor(bean); } return bean; @@ -189,8 +195,13 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { Object createExecutorServiceProxy(Object bean, boolean cglibProxy, ExecutorService executor) { - return getProxiedObject(bean, cglibProxy, executor, - () -> new TraceableExecutorService(this.beanFactory, executor)); + return getProxiedObject(bean, cglibProxy, executor, () -> { + if (executor instanceof ScheduledExecutorService) { + return new TraceableScheduledExecutorService(this.beanFactory, executor); + } + + return new TraceableExecutorService(this.beanFactory, executor); + }); } Object createAsyncTaskExecutorProxy(Object bean, boolean cglibProxy, diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessorTests.java index cc6a4c5ff..c67c7b440 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessorTests.java @@ -53,6 +53,7 @@ import static org.assertj.core.api.BDDAssertions.thenThrownBy; /** * @author Marcin Grzejszczak * @author Denys Ivano + * @author Vladislav Fefelov */ @RunWith(MockitoJUnitRunner.class) public class ExecutorBeanPostProcessorTests { @@ -86,9 +87,9 @@ public class ExecutorBeanPostProcessorTests { } @Test - public void should_fallback_to_sleuth_implementation_when_cglib_cannot_be_created() + public void should_fallback_to_sleuth_implementation_when_cglib_cannot_be_created_for_executor() throws Exception { - ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor(); + ExecutorService service = Executors.newSingleThreadExecutor(); Object o = new ExecutorBeanPostProcessor(this.beanFactory) .postProcessAfterInitialization(service, "foo"); @@ -97,6 +98,77 @@ public class ExecutorBeanPostProcessorTests { service.shutdown(); } + @Test + public void should_fallback_to_sleuth_implementation_when_cglib_cannot_be_created_for_scheduled_executor() + throws Exception { + ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor(); + + Object o = new ExecutorBeanPostProcessor(this.beanFactory) + .postProcessAfterInitialization(service, "foo"); + + then(o).isInstanceOf(TraceableScheduledExecutorService.class); + service.shutdown(); + } + + @Test + public void should_do_nothing_when_bean_is_already_lazy_trace_async_task_executor() + throws Exception { + LazyTraceAsyncTaskExecutor service = BDDMockito + .mock(LazyTraceAsyncTaskExecutor.class); + + Object o = new ExecutorBeanPostProcessor(this.beanFactory) + .postProcessAfterInitialization(service, "foo"); + + then(o).isSameAs(service); + } + + @Test + public void should_do_nothing_when_bean_is_already_lazy_trace_executor() + throws Exception { + LazyTraceExecutor service = BDDMockito.mock(LazyTraceExecutor.class); + + Object o = new ExecutorBeanPostProcessor(this.beanFactory) + .postProcessAfterInitialization(service, "foo"); + + then(o).isSameAs(service); + } + + @Test + public void should_do_nothing_when_bean_is_already_lazy_thread_pool_task_executor() + throws Exception { + LazyTraceThreadPoolTaskExecutor service = BDDMockito + .mock(LazyTraceThreadPoolTaskExecutor.class); + + Object o = new ExecutorBeanPostProcessor(this.beanFactory) + .postProcessAfterInitialization(service, "foo"); + + then(o).isSameAs(service); + } + + @Test + public void should_do_nothing_when_bean_is_already_traceable_executor() + throws Exception { + TraceableExecutorService service = BDDMockito + .mock(TraceableExecutorService.class); + + Object o = new ExecutorBeanPostProcessor(this.beanFactory) + .postProcessAfterInitialization(service, "foo"); + + then(o).isSameAs(service); + } + + @Test + public void should_do_nothing_when_bean_is_already_traceable_scheduled_executor() + throws Exception { + TraceableScheduledExecutorService service = BDDMockito + .mock(TraceableScheduledExecutorService.class); + + Object o = new ExecutorBeanPostProcessor(this.beanFactory) + .postProcessAfterInitialization(service, "foo"); + + then(o).isSameAs(service); + } + @Test public void should_fallback_to_default_implementation_when_exception_thrown() throws Exception { @@ -112,7 +184,7 @@ public class ExecutorBeanPostProcessorTests { Object wrappedService = bpp.postProcessAfterInitialization(service, "foo"); - then(wrappedService).isInstanceOf(TraceableExecutorService.class); + then(wrappedService).isInstanceOf(TraceableScheduledExecutorService.class); service.shutdown(); }