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 1f2c91400..69f95f8da 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 @@ -16,8 +16,15 @@ package org.springframework.cloud.sleuth.instrument.async; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.concurrent.Executor; + import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.aop.framework.AopConfigException; import org.springframework.aop.framework.ProxyFactoryBean; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; @@ -25,10 +32,6 @@ import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; import org.springframework.util.ReflectionUtils; -import java.lang.reflect.Method; -import java.lang.reflect.Modifier; -import java.util.concurrent.Executor; - /** * Bean post processor that wraps a call to an {@link Executor} either in a * JDK or CGLIB proxy. Depending on whether the implementation has a final @@ -39,6 +42,8 @@ import java.util.concurrent.Executor; */ class ExecutorBeanPostProcessor implements BeanPostProcessor { + private static final Log log = LogFactory.getLog(ExecutorBeanPostProcessor.class); + private final BeanFactory beanFactory; ExecutorBeanPostProcessor(BeanFactory beanFactory) { @@ -60,27 +65,46 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { boolean classFinal = Modifier.isFinal(bean.getClass().getModifiers()); boolean cglibProxy = !methodFinal && !classFinal; Executor executor = (Executor) bean; - ProxyFactoryBean factory = new ProxyFactoryBean(); - factory.setProxyTargetClass(cglibProxy); - factory.addAdvice(new ExecutorMethodInterceptor<>(executor, this.beanFactory)); - factory.setTarget(bean); - return factory.getObject(); + try { + return createProxy(bean, cglibProxy, executor); + } catch (AopConfigException e) { + if (cglibProxy) { + if (log.isDebugEnabled()) { + log.debug("Exception occurred while trying to create a proxy, falling back to JDK proxy", e); + } + return createProxy(bean, false, executor); + } + throw e; + } } else if (bean instanceof ThreadPoolTaskExecutor) { boolean classFinal = Modifier.isFinal(bean.getClass().getModifiers()); boolean cglibProxy = !classFinal; ThreadPoolTaskExecutor executor = (ThreadPoolTaskExecutor) bean; - ProxyFactoryBean factory = new ProxyFactoryBean(); - factory.setProxyTargetClass(cglibProxy); - factory.addAdvice(new ExecutorMethodInterceptor(executor, this.beanFactory) { - @Override Executor executor(BeanFactory beanFactory, ThreadPoolTaskExecutor executor) { - return new LazyTraceThreadPoolTaskExecutor(beanFactory, executor); - } - }); - factory.setTarget(bean); - return factory.getObject(); + return createThreadPoolTaskExecutorProxy(bean, cglibProxy, executor); } return bean; } + + Object createThreadPoolTaskExecutorProxy(Object bean, boolean cglibProxy, + ThreadPoolTaskExecutor executor) { + ProxyFactoryBean factory = new ProxyFactoryBean(); + factory.setProxyTargetClass(cglibProxy); + factory.addAdvice(new ExecutorMethodInterceptor(executor, this.beanFactory) { + @Override Executor executor(BeanFactory beanFactory, ThreadPoolTaskExecutor executor) { + return new LazyTraceThreadPoolTaskExecutor(beanFactory, executor); + } + }); + factory.setTarget(bean); + return factory.getObject(); + } + + Object createProxy(Object bean, boolean cglibProxy, Executor executor) { + ProxyFactoryBean factory = new ProxyFactoryBean(); + factory.setProxyTargetClass(cglibProxy); + factory.addAdvice(new ExecutorMethodInterceptor(executor, this.beanFactory)); + factory.setTarget(bean); + return factory.getObject(); + } } class ExecutorMethodInterceptor implements MethodInterceptor { 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 new file mode 100644 index 000000000..90179ce64 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/ExecutorBeanPostProcessorTests.java @@ -0,0 +1,95 @@ +package org.springframework.cloud.sleuth.instrument.async; + +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.aop.framework.AopConfigException; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; +import org.springframework.util.ClassUtils; + +import static org.assertj.core.api.BDDAssertions.then; +import static org.assertj.core.api.BDDAssertions.thenThrownBy; + +/** + * @author Marcin Grzejszczak + */ +@RunWith(MockitoJUnitRunner.class) +public class ExecutorBeanPostProcessorTests { + + @Mock BeanFactory beanFactory; + + @Test + public void should_create_a_cglib_proxy_by_default() throws Exception { + Object o = new ExecutorBeanPostProcessor(this.beanFactory) + .postProcessAfterInitialization(new Foo(), "foo"); + + then(o).isInstanceOf(Foo.class); + then(ClassUtils.isCglibProxy(o)).isTrue(); + } + + class Foo implements Executor { + @Override public void execute(Runnable command) { + + } + } + + @Test + public void should_create_jdk_proxy_when_cglib_fails_to_be_done() throws Exception { + ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor(); + + Object o = new ExecutorBeanPostProcessor(this.beanFactory) + .postProcessAfterInitialization(service, "foo"); + + then(o).isInstanceOf(ScheduledExecutorService.class); + then(ClassUtils.isCglibProxy(o)).isFalse(); + } + + @Test + public void should_throw_exception_when_it_is_not_possible_to_create_any_proxy() throws Exception { + ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor(); + ExecutorBeanPostProcessor bpp = new ExecutorBeanPostProcessor(this.beanFactory) { + @Override Object createProxy(Object bean, boolean cglibProxy, + Executor executor) { + throw new AopConfigException("foo"); + } + }; + + thenThrownBy(() -> bpp.postProcessAfterInitialization(service, "foo")) + .isInstanceOf(AopConfigException.class) + .hasMessage("foo"); + } + + @Test + public void should_create_a_cglib_proxy_by_default_for_ThreadPoolTaskExecutor() throws Exception { + Object o = new ExecutorBeanPostProcessor(this.beanFactory) + .postProcessAfterInitialization(new FooThreadPoolTaskExecutor(), "foo"); + + then(o).isInstanceOf(FooThreadPoolTaskExecutor.class); + then(ClassUtils.isCglibProxy(o)).isTrue(); + } + + class FooThreadPoolTaskExecutor extends ThreadPoolTaskExecutor { + } + + @Test + public void should_throw_exception_when_it_is_not_possible_to_create_any_proxyfor_ThreadPoolTaskExecutor() throws Exception { + ThreadPoolTaskExecutor taskExecutor = new ThreadPoolTaskExecutor(); + ExecutorBeanPostProcessor bpp = new ExecutorBeanPostProcessor(this.beanFactory) { + @Override Object createThreadPoolTaskExecutorProxy(Object bean, boolean cglibProxy, + ThreadPoolTaskExecutor executor) { + throw new AopConfigException("foo"); + } + }; + + thenThrownBy(() -> bpp.postProcessAfterInitialization(taskExecutor, "foo")) + .isInstanceOf(AopConfigException.class) + .hasMessage("foo"); + } + +} \ No newline at end of file