From ed1f3afc8edca234d81fecd79f896dc4382a8b49 Mon Sep 17 00:00:00 2001 From: jalogar Date: Wed, 25 Jul 2018 10:56:47 +0200 Subject: [PATCH] ability to disable instrumentation in ThreadPoolTaskExecutor (#1035) * ability to disable instrumentation in ThreadPoolTaskExecutor * Fixing documentation and adding unit tests --- .../async/AsyncAutoConfiguration.java | 34 +++++++++++++++ .../instrument/async/AsyncProperties.java | 42 +++++++++++++++++++ .../async/ExecutorBeanPostProcessor.java | 26 ++++++++++-- .../main/resources/META-INF/spring.factories | 1 + .../async/ExecutorBeanPostProcessorTests.java | 39 +++++++++++++++++ 5 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/AsyncAutoConfiguration.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/AsyncProperties.java diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/AsyncAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/AsyncAutoConfiguration.java new file mode 100644 index 000000000..32c6ceadd --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/AsyncAutoConfiguration.java @@ -0,0 +1,34 @@ +/* + * Copyright 2013-2018 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 + * + * http://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 org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.context.annotation.Configuration; +import org.springframework.scheduling.annotation.AsyncConfigurer; + +/** + * {@link org.springframework.boot.autoconfigure.EnableAutoConfiguration Auto-configuration} + * that wraps an existing custom {@link AsyncConfigurer} in a {@link LazyTraceAsyncCustomizer} + * + * @author Jesus Alonso + * @since 2.1.0 + */ + +@Configuration +@EnableConfigurationProperties(AsyncProperties.class) +public class AsyncAutoConfiguration { +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/AsyncProperties.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/AsyncProperties.java new file mode 100644 index 000000000..4c10e72c2 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/AsyncProperties.java @@ -0,0 +1,42 @@ +/* + * Copyright 2013-2018 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 + * + * http://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.Collections; +import java.util.List; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * Settings for disable instrumentation of ThreadPoolTaskExecutors + * + * @author Jesus Alonso + * @since 2.1.0 + */ + +@ConfigurationProperties(prefix = "spring.sleuth.async") +public class AsyncProperties { + + private List ignoredBeans = Collections.emptyList(); + + public List getIgnoredBeans() { + return this.ignoredBeans; + } + public void setIgnoredBeans(List ignoredBeans) { + this.ignoredBeans = ignoredBeans; + } +} 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 9ce03995e..8bc2ea255 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 @@ -38,6 +38,7 @@ import org.springframework.util.ReflectionUtils; * method or is final. * * @author Marcin Grzejszczak + * @author Jesus Alonso * @since 1.1.4 */ class ExecutorBeanPostProcessor implements BeanPostProcessor { @@ -46,6 +47,7 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { ExecutorBeanPostProcessor.class); private final BeanFactory beanFactory; + private AsyncProperties asyncProperties; ExecutorBeanPostProcessor(BeanFactory beanFactory) { this.beanFactory = beanFactory; @@ -78,14 +80,23 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { throw e; } } else if (bean instanceof ThreadPoolTaskExecutor) { - boolean classFinal = Modifier.isFinal(bean.getClass().getModifiers()); - boolean cglibProxy = !classFinal; - ThreadPoolTaskExecutor executor = (ThreadPoolTaskExecutor) bean; - return createThreadPoolTaskExecutorProxy(bean, cglibProxy, executor); + if (isProxyNeeded(beanName)) { + boolean classFinal = Modifier.isFinal(bean.getClass().getModifiers()); + boolean cglibProxy = !classFinal; + ThreadPoolTaskExecutor executor = (ThreadPoolTaskExecutor) bean; + return createThreadPoolTaskExecutorProxy(bean, cglibProxy, executor); + } else { + log.info("Not instrumenting bean " + beanName); + } } return bean; } + boolean isProxyNeeded(String beanName) { + AsyncProperties asyncProperties = asyncConfigurationProperties(); + return !asyncProperties.getIgnoredBeans().contains(beanName); + } + Object createThreadPoolTaskExecutorProxy(Object bean, boolean cglibProxy, ThreadPoolTaskExecutor executor) { ProxyFactoryBean factory = new ProxyFactoryBean(); @@ -107,6 +118,13 @@ class ExecutorBeanPostProcessor implements BeanPostProcessor { factory.setTarget(bean); return factory.getObject(); } + + private AsyncProperties asyncConfigurationProperties() { + if (this.asyncProperties == null) { + this.asyncProperties = this.beanFactory.getBean(AsyncProperties.class); + } + return this.asyncProperties; + } } class ExecutorMethodInterceptor implements MethodInterceptor { 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 122fb754a..09a0a60af 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 @@ -8,6 +8,7 @@ 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,\ +org.springframework.cloud.sleuth.instrument.async.AsyncAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.async.AsyncCustomAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.async.AsyncDefaultAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.scheduling.TraceSchedulingAutoConfiguration,\ 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 66d6fd06d..25991617a 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 @@ -20,15 +20,20 @@ import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.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 com.google.common.collect.ImmutableList; + +import static org.junit.Assert.*; import static org.assertj.core.api.BDDAssertions.then; import static org.assertj.core.api.BDDAssertions.thenThrownBy; @@ -39,6 +44,13 @@ import static org.assertj.core.api.BDDAssertions.thenThrownBy; public class ExecutorBeanPostProcessorTests { @Mock BeanFactory beanFactory; + private AsyncProperties asyncProperties; + + @Before + public void setup() { + this.asyncProperties = new AsyncProperties(); + Mockito.when(beanFactory.getBean(AsyncProperties.class)).thenReturn(this.asyncProperties); + } @Test public void should_create_a_cglib_proxy_by_default() throws Exception { @@ -109,5 +121,32 @@ public class ExecutorBeanPostProcessorTests { .isInstanceOf(AopConfigException.class) .hasMessage("foo"); } + + @Test + public void proxy_is_not_needed() throws Exception { + this.asyncProperties.setIgnoredBeans(ImmutableList.of("fooExecutor")); + + boolean isProxyNeeded = new ExecutorBeanPostProcessor(this.beanFactory).isProxyNeeded("fooExecutor"); + + assertFalse(isProxyNeeded); + } + + @Test + public void proxy_is_needed() throws Exception { + boolean isProxyNeeded = new ExecutorBeanPostProcessor(this.beanFactory).isProxyNeeded("fooExecutor"); + + assertTrue(isProxyNeeded); + } + + @Test + public void should_not_create_proxy() throws Exception { + this.asyncProperties.setIgnoredBeans(ImmutableList.of("fooExecutor")); + + Object o = new ExecutorBeanPostProcessor(this.beanFactory) + .postProcessAfterInitialization(new ThreadPoolTaskExecutor(), "fooExecutor"); + + then(o).isInstanceOf(ThreadPoolTaskExecutor.class); + then(ClassUtils.isCglibProxy(o)).isFalse(); + } } \ No newline at end of file