From e056db7cf3dc4e995acc70fadacf3c7ff374b7c4 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Mon, 6 Dec 2021 17:03:05 +0100 Subject: [PATCH] Going with manual instrumentation of transaction managers; fixes gh-2067 --- ...bstractTransactionManagerInstrumenter.java | 174 ++++++++++++++++++ ...latformTransactionManagerInstrumenter.java | 40 ++++ ...eactiveTransactionManagerInstrumenter.java | 40 ++++ ...rmTransactionManagerBeanPostProcessor.java | 14 +- ...veTransactionManagerBeanPostProcessor.java | 10 +- .../tx/TraceTxAutoConfiguration.java | 5 +- .../TraceTxAutoConfigurationAspectsTests.java | 85 ++++----- .../tx/TraceTxAutoConfigurationTests.java | 25 +-- .../tx/TraceKafkaAwareTransactionManager.java | 2 +- ...TracePlatformTransactionManagerAspect.java | 84 --------- 10 files changed, 309 insertions(+), 170 deletions(-) create mode 100644 spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/AbstractTransactionManagerInstrumenter.java create mode 100644 spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/PlatformTransactionManagerInstrumenter.java create mode 100644 spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/ReactiveTransactionManagerInstrumenter.java delete mode 100644 spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/tx/TracePlatformTransactionManagerAspect.java diff --git a/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/AbstractTransactionManagerInstrumenter.java b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/AbstractTransactionManagerInstrumenter.java new file mode 100644 index 000000000..f07c5910c --- /dev/null +++ b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/AbstractTransactionManagerInstrumenter.java @@ -0,0 +1,174 @@ +/* + * Copyright 2013-2021 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.autoconfig.instrument.tx; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executor; + +import org.aopalliance.aop.Advice; +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.factory.BeanFactory; +import org.springframework.transaction.TransactionManager; +import org.springframework.util.ReflectionUtils; + +abstract class AbstractTransactionManagerInstrumenter { + + private static final Log log = LogFactory.getLog(AbstractTransactionManagerInstrumenter.class); + + protected final BeanFactory beanFactory; + + private final Class classToInstrument; + + AbstractTransactionManagerInstrumenter(BeanFactory beanFactory, Class classToInstrument) { + this.beanFactory = beanFactory; + this.classToInstrument = classToInstrument; + } + + private static boolean anyFinalMethods(T object, Class classToCheckAgainst) { + try { + for (Method method : ReflectionUtils.getAllDeclaredMethods(classToCheckAgainst)) { + if (method.getDeclaringClass().equals(Object.class)) { + continue; + } + Method m = ReflectionUtils.findMethod(object.getClass(), method.getName(), method.getParameterTypes()); + if (m != null && Modifier.isPublic(m.getModifiers()) && Modifier.isFinal(m.getModifiers())) { + return true; + } + } + } + catch (IllegalAccessError er) { + if (log.isDebugEnabled()) { + log.debug("Error occurred while trying to access methods", er); + } + return false; + } + return false; + } + + boolean isApplicableForInstrumentation(Object bean) { + return isNotYetTraced(bean) && !(tracedClass().isAssignableFrom(bean.getClass())); + } + + private boolean isNotYetTraced(Object bean) { + return this.classToInstrument.isAssignableFrom(bean.getClass()); + } + + @SuppressWarnings("rawtypes") + abstract Class tracedClass(); + + abstract T wrap(T transactionManager); + + /** + * Wraps an {@link Executor} bean in its trace representation. + * @param bean a bean (might be of {@link Executor} type + * @param beanName name of the bean + * @return wrapped bean or just bean if not {@link Executor} or already instrumented + */ + Object instrument(Object bean, String beanName) { + if (!isApplicableForInstrumentation(bean)) { + if (log.isTraceEnabled()) { + log.trace("Bean is already instrumented or is not applicable for instrumentation " + beanName); + } + return bean; + } + return wrapManager(bean); + } + + private Object wrapManager(Object bean) { + T manager = (T) bean; + boolean methodFinal = anyFinalMethods(manager, this.classToInstrument); + boolean classFinal = Modifier.isFinal(bean.getClass().getModifiers()); + boolean cglibProxy = !methodFinal && !classFinal; + try { + return createProxy(bean, cglibProxy, new TransactionManagerMethodInterceptor<>(this, manager)); + } + catch (AopConfigException ex) { + if (cglibProxy) { + if (log.isDebugEnabled()) { + log.debug("Exception occurred while trying to create a proxy, falling back to JDK proxy", ex); + } + return createProxy(bean, false, new TransactionManagerMethodInterceptor<>(this, manager)); + } + throw ex; + } + } + + private Object getObject(ProxyFactoryBean factory) { + return factory.getObject(); + } + + @SuppressWarnings("unchecked") + private Object createProxy(Object bean, boolean cglibProxy, Advice advice) { + ProxyFactoryBean factory = new ProxyFactoryBean(); + factory.setProxyTargetClass(cglibProxy); + factory.addAdvice(advice); + factory.setTarget(bean); + return getObject(factory); + } + + static class TransactionManagerMethodInterceptor implements MethodInterceptor { + + private static final Map CACHE = new ConcurrentHashMap<>(); + + private final AbstractTransactionManagerInstrumenter parent; + + private final T delegate; + + TransactionManagerMethodInterceptor(AbstractTransactionManagerInstrumenter parent, T delegate) { + this.parent = parent; + this.delegate = delegate; + } + + @Override + public Object invoke(MethodInvocation invocation) throws Throwable { + T tracedDelegate = traceDelegate(); + Method methodOnTracedBean = getMethod(invocation, tracedDelegate); + if (methodOnTracedBean != null) { + try { + return methodOnTracedBean.invoke(tracedDelegate, invocation.getArguments()); + } + catch (InvocationTargetException ex) { + Throwable cause = ex.getCause(); + throw (cause != null) ? cause : ex; + } + } + return invocation.proceed(); + } + + private Method getMethod(MethodInvocation invocation, Object object) { + Method method = invocation.getMethod(); + return ReflectionUtils.findMethod(object.getClass(), method.getName(), method.getParameterTypes()); + } + + @SuppressWarnings("unchecked") + private T traceDelegate() { + return (T) CACHE.computeIfAbsent(this.delegate, o -> parent.wrap((T) o)); + } + + } + +} diff --git a/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/PlatformTransactionManagerInstrumenter.java b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/PlatformTransactionManagerInstrumenter.java new file mode 100644 index 000000000..99501217e --- /dev/null +++ b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/PlatformTransactionManagerInstrumenter.java @@ -0,0 +1,40 @@ +/* + * Copyright 2013-2021 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.autoconfig.instrument.tx; + +import org.springframework.beans.factory.BeanFactory; +import org.springframework.cloud.sleuth.instrument.tx.TracePlatformTransactionManager; +import org.springframework.transaction.PlatformTransactionManager; + +class PlatformTransactionManagerInstrumenter + extends AbstractTransactionManagerInstrumenter { + + PlatformTransactionManagerInstrumenter(BeanFactory beanFactory) { + super(beanFactory, PlatformTransactionManager.class); + } + + @Override + Class tracedClass() { + return TracePlatformTransactionManager.class; + } + + @Override + PlatformTransactionManager wrap(PlatformTransactionManager transactionManager) { + return new TracePlatformTransactionManager(transactionManager, this.beanFactory); + } + +} diff --git a/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/ReactiveTransactionManagerInstrumenter.java b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/ReactiveTransactionManagerInstrumenter.java new file mode 100644 index 000000000..795c700ec --- /dev/null +++ b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/ReactiveTransactionManagerInstrumenter.java @@ -0,0 +1,40 @@ +/* + * Copyright 2013-2021 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.autoconfig.instrument.tx; + +import org.springframework.beans.factory.BeanFactory; +import org.springframework.cloud.sleuth.instrument.tx.TraceReactiveTransactionManager; +import org.springframework.transaction.ReactiveTransactionManager; + +class ReactiveTransactionManagerInstrumenter + extends AbstractTransactionManagerInstrumenter { + + ReactiveTransactionManagerInstrumenter(BeanFactory beanFactory) { + super(beanFactory, ReactiveTransactionManager.class); + } + + @Override + Class tracedClass() { + return TraceReactiveTransactionManager.class; + } + + @Override + ReactiveTransactionManager wrap(ReactiveTransactionManager transactionManager) { + return new TraceReactiveTransactionManager(transactionManager, this.beanFactory); + } + +} diff --git a/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TracePlatformTransactionManagerBeanPostProcessor.java b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TracePlatformTransactionManagerBeanPostProcessor.java index a82cc25fd..89bf8c653 100644 --- a/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TracePlatformTransactionManagerBeanPostProcessor.java +++ b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TracePlatformTransactionManagerBeanPostProcessor.java @@ -19,7 +19,6 @@ package org.springframework.cloud.sleuth.autoconfig.instrument.tx; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.config.BeanPostProcessor; -import org.springframework.cloud.sleuth.instrument.tx.TracePlatformTransactionManager; import org.springframework.transaction.PlatformTransactionManager; /** @@ -27,25 +26,18 @@ import org.springframework.transaction.PlatformTransactionManager; * * @author Marcin Grzejszczak * @since 3.1.0 - * @deprecated will use - * {@link org.springframework.cloud.sleuth.instrument.tx.TracePlatformTransactionManagerAspect} - * instead */ -@Deprecated public class TracePlatformTransactionManagerBeanPostProcessor implements BeanPostProcessor { - private final BeanFactory beanFactory; + private final PlatformTransactionManagerInstrumenter platformTransactionManagerInstrumenter; public TracePlatformTransactionManagerBeanPostProcessor(BeanFactory beanFactory) { - this.beanFactory = beanFactory; + this.platformTransactionManagerInstrumenter = new PlatformTransactionManagerInstrumenter(beanFactory); } @Override public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { - if (bean instanceof PlatformTransactionManager && !(bean instanceof TracePlatformTransactionManager)) { - return new TracePlatformTransactionManager((PlatformTransactionManager) bean, this.beanFactory); - } - return bean; + return this.platformTransactionManagerInstrumenter.instrument(bean, beanName); } } diff --git a/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceReactiveTransactionManagerBeanPostProcessor.java b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceReactiveTransactionManagerBeanPostProcessor.java index 8a7ac5510..d8bb51fd0 100644 --- a/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceReactiveTransactionManagerBeanPostProcessor.java +++ b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceReactiveTransactionManagerBeanPostProcessor.java @@ -19,7 +19,6 @@ package org.springframework.cloud.sleuth.autoconfig.instrument.tx; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.config.BeanPostProcessor; -import org.springframework.cloud.sleuth.instrument.tx.TraceReactiveTransactionManager; import org.springframework.transaction.ReactiveTransactionManager; /** @@ -30,18 +29,15 @@ import org.springframework.transaction.ReactiveTransactionManager; */ public class TraceReactiveTransactionManagerBeanPostProcessor implements BeanPostProcessor { - private final BeanFactory beanFactory; + private final ReactiveTransactionManagerInstrumenter reactiveTransactionManagerInstrumenter; public TraceReactiveTransactionManagerBeanPostProcessor(BeanFactory beanFactory) { - this.beanFactory = beanFactory; + this.reactiveTransactionManagerInstrumenter = new ReactiveTransactionManagerInstrumenter(beanFactory); } @Override public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { - if (bean instanceof ReactiveTransactionManager && !(bean instanceof TraceReactiveTransactionManager)) { - return new TraceReactiveTransactionManager((ReactiveTransactionManager) bean, this.beanFactory); - } - return bean; + return this.reactiveTransactionManagerInstrumenter.instrument(bean, beanName); } } diff --git a/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfiguration.java b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfiguration.java index 8879b7493..f1fd9b189 100644 --- a/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfiguration.java +++ b/spring-cloud-sleuth-autoconfigure/src/main/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfiguration.java @@ -23,7 +23,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.autoconfig.brave.BraveAutoConfiguration; -import org.springframework.cloud.sleuth.instrument.tx.TracePlatformTransactionManagerAspect; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -42,9 +41,9 @@ public class TraceTxAutoConfiguration { @Bean @ConditionalOnClass(name = "org.springframework.transaction.PlatformTransactionManager") - TracePlatformTransactionManagerAspect tracePlatformTransactionManagerAspect(Tracer tracer, + static TracePlatformTransactionManagerBeanPostProcessor tracePlatformTransactionManagerBeanPostProcessor( BeanFactory beanFactory) { - return new TracePlatformTransactionManagerAspect(tracer, beanFactory); + return new TracePlatformTransactionManagerBeanPostProcessor(beanFactory); } @Bean diff --git a/spring-cloud-sleuth-autoconfigure/src/test/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfigurationAspectsTests.java b/spring-cloud-sleuth-autoconfigure/src/test/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfigurationAspectsTests.java index 220380555..07a471c9b 100644 --- a/spring-cloud-sleuth-autoconfigure/src/test/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfigurationAspectsTests.java +++ b/spring-cloud-sleuth-autoconfigure/src/test/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfigurationAspectsTests.java @@ -16,46 +16,39 @@ package org.springframework.cloud.sleuth.autoconfig.instrument.tx; -import org.aspectj.lang.ProceedingJoinPoint; import org.junit.jupiter.api.Test; +import reactor.core.publisher.Mono; -import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.ImportAutoConfiguration; import org.springframework.boot.autoconfigure.aop.AopAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.autoconfig.TraceNoOpAutoConfiguration; -import org.springframework.cloud.sleuth.instrument.tx.TracePlatformTransactionManagerAspect; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.ReactiveTransaction; +import org.springframework.transaction.ReactiveTransactionManager; import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.TransactionException; import org.springframework.transaction.TransactionStatus; -import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.BDDAssertions.then; @SpringBootTest(properties = "spring.sleuth.noop.enabled=true", classes = TraceTxAutoConfigurationAspectsTests.Config.class) class TraceTxAutoConfigurationAspectsTests { @Autowired - MyPlatformTransactionManager myPlatformTransactionManager; + MyReactiveTransactionManager myReactiveTransactionManager; @Autowired - TestPlatformAspect testPlatformAspect; + MyPlatformTransactionManager myPlatformTransactionManager; @Test - void should_make_aspects_work_for_platform() { - myPlatformTransactionManager.getTransaction(null); - assertThat(testPlatformAspect.getTransactionCalled).isTrue(); - - myPlatformTransactionManager.commit(null); - assertThat(testPlatformAspect.commitCalled).isTrue(); - - myPlatformTransactionManager.rollback(null); - assertThat(testPlatformAspect.rollbackCalled).isTrue(); + void should_make_proxies_work_for_platform() { + then(this.myReactiveTransactionManager).isNotNull().isInstanceOf(MyReactiveTransactionManager.class); + then(this.myPlatformTransactionManager).isNotNull().isInstanceOf(MyPlatformTransactionManager.class); } @Configuration(proxyBeanMethods = false) @@ -64,45 +57,13 @@ class TraceTxAutoConfigurationAspectsTests { static class Config { @Bean - MyPlatformTransactionManager myPlatformTransactionManager() { - return new MyPlatformTransactionManager(); + MyReactiveTransactionManager myReactiveTransactionManager() { + return new MyReactiveTransactionManager(); } @Bean - TestPlatformAspect testPlatformAspect(Tracer tracer, BeanFactory beanFactory) { - return new TestPlatformAspect(tracer, beanFactory); - } - - } - - static class TestPlatformAspect extends TracePlatformTransactionManagerAspect { - - boolean commitCalled; - - boolean rollbackCalled; - - boolean getTransactionCalled; - - TestPlatformAspect(Tracer tracer, BeanFactory beanFactory) { - super(tracer, beanFactory); - } - - @Override - public Object traceCommit(ProceedingJoinPoint pjp, PlatformTransactionManager manager) { - this.commitCalled = true; - return null; - } - - @Override - public Object traceRollback(ProceedingJoinPoint pjp, PlatformTransactionManager manager) { - this.rollbackCalled = true; - return null; - } - - @Override - public Object traceGetTransaction(ProceedingJoinPoint pjp, PlatformTransactionManager manager) { - this.getTransactionCalled = true; - return null; + MyPlatformTransactionManager myPlatformTransactionManager() { + return new MyPlatformTransactionManager(); } } @@ -126,4 +87,24 @@ class TraceTxAutoConfigurationAspectsTests { } + static class MyReactiveTransactionManager implements ReactiveTransactionManager { + + @Override + public Mono getReactiveTransaction(TransactionDefinition definition) + throws TransactionException { + return null; + } + + @Override + public Mono commit(ReactiveTransaction transaction) throws TransactionException { + return null; + } + + @Override + public Mono rollback(ReactiveTransaction transaction) throws TransactionException { + return null; + } + + } + } diff --git a/spring-cloud-sleuth-autoconfigure/src/test/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfigurationTests.java b/spring-cloud-sleuth-autoconfigure/src/test/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfigurationTests.java index e5a0bc62d..04f3e5acd 100644 --- a/spring-cloud-sleuth-autoconfigure/src/test/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfigurationTests.java +++ b/spring-cloud-sleuth-autoconfigure/src/test/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfigurationTests.java @@ -16,6 +16,7 @@ package org.springframework.cloud.sleuth.autoconfig.instrument.tx; +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; import reactor.core.publisher.Mono; @@ -23,12 +24,9 @@ import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.cloud.sleuth.autoconfig.TraceNoOpAutoConfiguration; -import org.springframework.cloud.sleuth.instrument.tx.TracePlatformTransactionManagerAspect; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.ReactiveTransactionManager; -import static org.assertj.core.api.Assertions.assertThat; - class TraceTxAutoConfigurationTests { private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() @@ -36,27 +34,30 @@ class TraceTxAutoConfigurationTests { .withConfiguration(AutoConfigurations.of(TraceNoOpAutoConfiguration.class, TraceTxAutoConfiguration.class)); @Test - void should_register_infrastructure_beans() { - this.contextRunner.run(context -> assertThat(context).hasSingleBean(TracePlatformTransactionManagerAspect.class) + void should_register_bean_post_processors() { + this.contextRunner.run(context -> Assertions.assertThat(context) + .hasSingleBean(TracePlatformTransactionManagerBeanPostProcessor.class) .hasSingleBean(TraceReactiveTransactionManagerBeanPostProcessor.class)); } @Test - void should_not_register_aspect_when_tx_not_on_classpath() { + void should_not_register_bean_post_processor_when_tx_not_on_classpath() { this.contextRunner.withClassLoader(new FilteredClassLoader(PlatformTransactionManager.class)) - .run(context -> assertThat(context).doesNotHaveBean(TracePlatformTransactionManagerAspect.class)); + .run(context -> Assertions.assertThat(context) + .doesNotHaveBean(TracePlatformTransactionManagerBeanPostProcessor.class)); } @Test void should_not_register_reactive_bean_post_processor_when_reactive_tx_not_on_classpath() { - this.contextRunner.withClassLoader(new FilteredClassLoader(ReactiveTransactionManager.class)).run( - context -> assertThat(context).doesNotHaveBean(TraceReactiveTransactionManagerBeanPostProcessor.class)); + this.contextRunner.withClassLoader(new FilteredClassLoader(ReactiveTransactionManager.class)) + .run(context -> Assertions.assertThat(context) + .doesNotHaveBean(TraceReactiveTransactionManagerBeanPostProcessor.class)); } @Test - void should_not_register_reactive_aspect_when_reactor_not_on_classpath() { - this.contextRunner.withClassLoader(new FilteredClassLoader(Mono.class)).run( - context -> assertThat(context).doesNotHaveBean(TraceReactiveTransactionManagerBeanPostProcessor.class)); + void should_not_register_reactive_bean_post_processor_when_reactor_not_on_classpath() { + this.contextRunner.withClassLoader(new FilteredClassLoader(Mono.class)).run(context -> Assertions + .assertThat(context).doesNotHaveBean(TraceReactiveTransactionManagerBeanPostProcessor.class)); } } diff --git a/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/tx/TraceKafkaAwareTransactionManager.java b/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/tx/TraceKafkaAwareTransactionManager.java index 65e6297be..87ab76955 100644 --- a/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/tx/TraceKafkaAwareTransactionManager.java +++ b/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/tx/TraceKafkaAwareTransactionManager.java @@ -25,7 +25,7 @@ import org.springframework.kafka.transaction.KafkaAwareTransactionManager; * * @author Marcin Grzejszczak * @since 3.1.0 - * @deprecated will use {@link TracePlatformTransactionManagerAspect} instead + * @deprecated scheduled for removal */ @Deprecated public class TraceKafkaAwareTransactionManager extends TracePlatformTransactionManager diff --git a/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/tx/TracePlatformTransactionManagerAspect.java b/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/tx/TracePlatformTransactionManagerAspect.java deleted file mode 100644 index 5efc43d88..000000000 --- a/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/tx/TracePlatformTransactionManagerAspect.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Copyright 2013-2021 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.tx; - -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - -import org.aspectj.lang.ProceedingJoinPoint; -import org.aspectj.lang.annotation.Around; -import org.aspectj.lang.annotation.Aspect; -import org.jetbrains.annotations.NotNull; - -import org.springframework.beans.factory.BeanFactory; -import org.springframework.cloud.sleuth.ThreadLocalSpan; -import org.springframework.cloud.sleuth.Tracer; -import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.TransactionDefinition; -import org.springframework.transaction.TransactionStatus; - -/** - * An aspect around {@link PlatformTransactionManager}. - * - * @author Marcin Grzejszczak - * @since 3.1.1 - */ -@Aspect -public class TracePlatformTransactionManagerAspect { - - private final BeanFactory beanFactory; - - volatile ThreadLocalSpan threadLocalSpan; - - private static final Map CACHE = new ConcurrentHashMap<>(); - - public TracePlatformTransactionManagerAspect(Tracer tracer, BeanFactory beanFactory) { - this.threadLocalSpan = new ThreadLocalSpan(tracer); - this.beanFactory = beanFactory; - } - - @Around(value = "execution (* org.springframework.transaction.PlatformTransactionManager.commit(..)) && this(manager)", - argNames = "pjp,manager") - public Object traceCommit(final ProceedingJoinPoint pjp, PlatformTransactionManager manager) { - TransactionStatus transactionStatus = (TransactionStatus) pjp.getArgs()[0]; - tracedManager(manager).commit(transactionStatus); - return null; - } - - @Around(value = "execution (* org.springframework.transaction.PlatformTransactionManager.rollback(..)) && this(manager)", - argNames = "pjp,manager") - public Object traceRollback(final ProceedingJoinPoint pjp, PlatformTransactionManager manager) { - TransactionStatus transactionStatus = (TransactionStatus) pjp.getArgs()[0]; - tracedManager(manager).rollback(transactionStatus); - return null; - } - - @Around(value = "execution (* org.springframework.transaction.PlatformTransactionManager.getTransaction(..)) && this(manager)", - argNames = "pjp,manager") - public Object traceGetTransaction(final ProceedingJoinPoint pjp, PlatformTransactionManager manager) { - TransactionDefinition transactionDefinition = (TransactionDefinition) pjp.getArgs()[0]; - return tracedManager(manager).getTransaction(transactionDefinition); - } - - @NotNull - private TracePlatformTransactionManager tracedManager(PlatformTransactionManager manager) { - return CACHE.computeIfAbsent(manager, - platformTransactionManager -> new TracePlatformTransactionManager(platformTransactionManager, - this.beanFactory)); - } - -}