Fixed wrong proxying of executors

without this change tracing worked fine but the custom types were not registered as beans. Thus autowiring of them was not possible
with this change the bean post processor is removed and an aspect is used - that way tracing is still working fine but we don't interfere in bean registration

fixes #445
This commit is contained in:
Marcin Grzejszczak
2016-11-21 16:41:20 +01:00
parent 24e52c690d
commit a3c0f81671
5 changed files with 60 additions and 82 deletions

View File

@@ -51,6 +51,8 @@ import org.springframework.scheduling.annotation.EnableAsync;
@AutoConfigureAfter(AsyncCustomAutoConfiguration.class)
public class AsyncDefaultAutoConfiguration {
@Autowired private BeanFactory beanFactory;
@Configuration
@ConditionalOnMissingBean(AsyncConfigurer.class)
@ConditionalOnProperty(value = "spring.sleuth.async.configurer.enabled", matchIfMissing = true)
@@ -66,12 +68,7 @@ public class AsyncDefaultAutoConfiguration {
@Bean
public TraceAsyncAspect traceAsyncAspect(Tracer tracer, TraceKeys traceKeys) {
return new TraceAsyncAspect(tracer, traceKeys);
}
@Bean
public TraceExecutorBeanPostProcessor traceExecutorBeanPostProcessor(BeanFactory beanFactory) {
return new TraceExecutorBeanPostProcessor(beanFactory);
return new TraceAsyncAspect(tracer, traceKeys, this.beanFactory);
}
}

View File

@@ -19,6 +19,7 @@ package org.springframework.cloud.sleuth.instrument.async;
import java.lang.invoke.MethodHandles;
import java.util.concurrent.Callable;
import java.util.concurrent.Future;
import java.util.concurrent.ThreadPoolExecutor;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -84,6 +85,11 @@ public class LazyTraceThreadPoolTaskExecutor extends ThreadPoolTaskExecutor {
return this.delegate.submitListenable(new SpanContinuingTraceCallable<>(tracer(), traceKeys(), spanNamer(), task));
}
@Override
public ThreadPoolExecutor getThreadPoolExecutor() throws IllegalStateException {
return this.delegate.getThreadPoolExecutor();
}
public void destroy() {
this.delegate.destroy();
super.destroy();

View File

@@ -16,13 +16,20 @@
package org.springframework.cloud.sleuth.instrument.async;
import java.lang.reflect.Method;
import java.util.concurrent.Executor;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.reflect.MethodSignature;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.Tracer;
import org.springframework.cloud.sleuth.TraceKeys;
import org.springframework.cloud.sleuth.Tracer;
import org.springframework.cloud.sleuth.util.SpanNameUtil;
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;
import org.springframework.util.ReflectionUtils;
/**
* Aspect that creates a new Span for running threads executing methods annotated with
@@ -40,10 +47,12 @@ public class TraceAsyncAspect {
private final Tracer tracer;
private final TraceKeys traceKeys;
private final BeanFactory beanFactory;
public TraceAsyncAspect(Tracer tracer, TraceKeys traceKeys) {
public TraceAsyncAspect(Tracer tracer, TraceKeys traceKeys, BeanFactory beanFactory) {
this.tracer = tracer;
this.traceKeys = traceKeys;
this.beanFactory = beanFactory;
}
@Around("execution (@org.springframework.scheduling.annotation.Async * *.*(..))")
@@ -62,4 +71,33 @@ public class TraceAsyncAspect {
}
}
@Around("execution (* org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor.*(..))")
public Object traceThreadPoolTaskExecutor(final ProceedingJoinPoint pjp) throws Throwable {
LazyTraceThreadPoolTaskExecutor executor = new LazyTraceThreadPoolTaskExecutor(this.beanFactory,
(ThreadPoolTaskExecutor) pjp.getTarget());
Method methodOnTracedBean = getMethod(pjp, executor);
if (methodOnTracedBean != null) {
return methodOnTracedBean.invoke(executor, pjp.getArgs());
}
return pjp.proceed();
}
@Around("execution (* java.util.concurrent.Executor.*(..))")
public Object traceExecutor(final ProceedingJoinPoint pjp) throws Throwable {
LazyTraceExecutor executor = new LazyTraceExecutor(this.beanFactory,
(Executor) pjp.getTarget());
Method methodOnTracedBean = getMethod(pjp, executor);
if (methodOnTracedBean != null) {
return methodOnTracedBean.invoke(executor, pjp.getArgs());
}
return pjp.proceed();
}
private Method getMethod(ProceedingJoinPoint pjp, Object object) {
MethodSignature signature = (MethodSignature) pjp.getSignature();
Method method = signature.getMethod();
return ReflectionUtils
.findMethod(object.getClass(), method.getName(), method.getParameterTypes());
}
}

View File

@@ -1,60 +0,0 @@
/*
* Copyright 2013-2016 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.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.scheduling.TaskScheduler;
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;
/**
* Bean post processor that wraps {@link Executor} in its Trace representation
*
* @author Marcin Grzejszczak
* @since 1.0.10
*/
class TraceExecutorBeanPostProcessor implements BeanPostProcessor {
private final BeanFactory beanFactory;
public TraceExecutorBeanPostProcessor(BeanFactory beanFactory) {
this.beanFactory = beanFactory;
}
@Override public Object postProcessBeforeInitialization(Object bean, String beanName)
throws BeansException {
if (bean instanceof ThreadPoolTaskExecutor && !(bean instanceof TaskScheduler) &&
!(bean instanceof LazyTraceThreadPoolTaskExecutor)) {
return new LazyTraceThreadPoolTaskExecutor(this.beanFactory, (ThreadPoolTaskExecutor) bean);
} else if (bean instanceof Executor && !(bean instanceof ExecutorService) &&
!(bean instanceof TaskScheduler) && !(bean instanceof LazyTraceExecutor)) {
return new LazyTraceExecutor(this.beanFactory, (Executor) bean);
} else if (bean instanceof ExecutorService) {
return new TraceableExecutorService(this.beanFactory, (ExecutorService) bean);
}
return bean;
}
@Override public Object postProcessAfterInitialization(Object bean, String beanName)
throws BeansException {
return bean;
}
}

View File

@@ -19,8 +19,6 @@ package org.springframework.cloud.sleuth.instrument.async.issues.issue410;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.commons.logging.Log;
@@ -38,7 +36,6 @@ import org.springframework.cloud.sleuth.Sampler;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.Tracer;
import org.springframework.cloud.sleuth.instrument.async.LazyTraceExecutor;
import org.springframework.cloud.sleuth.instrument.async.TraceableExecutorService;
import org.springframework.cloud.sleuth.sampler.AlwaysSampler;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
@@ -73,7 +70,7 @@ public class Issue410Tests {
/**
* Related to issue #445
*/
@Autowired ExecutorService executorService;
@Autowired Application.MyService executorService;
@Test
public void should_pass_tracing_info_for_tasks_running_without_a_pool() {
@@ -145,14 +142,6 @@ public class Issue410Tests {
}
}
/**
* Related to issue #445
*/
@Test
public void should_wrap_executor_service_in_trace_representation() {
then(this.executorService).isInstanceOf(TraceableExecutorService.class);
}
private int port() {
return this.environment.getProperty("local.server.port", Integer.class);
}
@@ -299,8 +288,16 @@ class Application {
/**
* Related to issue #445
*/
@Bean public ExecutorService executorService() {
return Executors.newSingleThreadExecutor();
@Bean public MyService executorService() {
return new MyService() {
@Override public void execute(Runnable command) {
}
};
}
interface MyService extends Executor {
}
}