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 7ecb647dc..a82cc25fd 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 @@ -27,7 +27,11 @@ 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; 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 bf24fd56f..8879b7493 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 @@ -20,10 +20,10 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; -import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; 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,18 +42,9 @@ public class TraceTxAutoConfiguration { @Bean @ConditionalOnClass(name = "org.springframework.transaction.PlatformTransactionManager") - @ConditionalOnMissingClass("org.springframework.kafka.transaction.KafkaAwareTransactionManager") - static TracePlatformTransactionManagerBeanPostProcessor tracePlatformTransactionManagerBeanPostProcessor( + TracePlatformTransactionManagerAspect tracePlatformTransactionManagerAspect(Tracer tracer, BeanFactory beanFactory) { - return new TracePlatformTransactionManagerBeanPostProcessor(beanFactory); - } - - @Bean - @ConditionalOnClass(name = { "org.springframework.transaction.PlatformTransactionManager", - "org.springframework.kafka.transaction.KafkaAwareTransactionManager" }) - static TraceKafkaPlatformTransactionManagerBeanPostProcessor traceKafkaPlatformTransactionManagerBeanPostProcessor( - BeanFactory beanFactory) { - return new TraceKafkaPlatformTransactionManagerBeanPostProcessor(beanFactory); + return new TracePlatformTransactionManagerAspect(tracer, 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 new file mode 100644 index 000000000..220380555 --- /dev/null +++ b/spring-cloud-sleuth-autoconfigure/src/test/java/org/springframework/cloud/sleuth/autoconfig/instrument/tx/TraceTxAutoConfigurationAspectsTests.java @@ -0,0 +1,129 @@ +/* + * 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.aspectj.lang.ProceedingJoinPoint; +import org.junit.jupiter.api.Test; + +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.TransactionDefinition; +import org.springframework.transaction.TransactionException; +import org.springframework.transaction.TransactionStatus; + +import static org.assertj.core.api.Assertions.assertThat; + +@SpringBootTest(properties = "spring.sleuth.noop.enabled=true", + classes = TraceTxAutoConfigurationAspectsTests.Config.class) +class TraceTxAutoConfigurationAspectsTests { + + @Autowired + MyPlatformTransactionManager myPlatformTransactionManager; + + @Autowired + TestPlatformAspect testPlatformAspect; + + @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(); + } + + @Configuration(proxyBeanMethods = false) + @ImportAutoConfiguration({ TraceNoOpAutoConfiguration.class, TraceTxAutoConfiguration.class, + AopAutoConfiguration.class }) + static class Config { + + @Bean + MyPlatformTransactionManager myPlatformTransactionManager() { + return new MyPlatformTransactionManager(); + } + + @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; + } + + } + + static class MyPlatformTransactionManager implements PlatformTransactionManager { + + @Override + public TransactionStatus getTransaction(TransactionDefinition definition) throws TransactionException { + return null; + } + + @Override + public void commit(TransactionStatus status) throws TransactionException { + + } + + @Override + public void rollback(TransactionStatus status) throws TransactionException { + + } + + } + +} 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 ceddc8905..e5a0bc62d 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,7 +16,6 @@ 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; @@ -24,9 +23,12 @@ 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() @@ -34,42 +36,27 @@ class TraceTxAutoConfigurationTests { .withConfiguration(AutoConfigurations.of(TraceNoOpAutoConfiguration.class, TraceTxAutoConfiguration.class)); @Test - void should_register_bean_post_processors() { - this.contextRunner.run(context -> Assertions.assertThat(context) - .hasSingleBean(TraceKafkaPlatformTransactionManagerBeanPostProcessor.class) - .doesNotHaveBean(TracePlatformTransactionManagerBeanPostProcessor.class) + void should_register_infrastructure_beans() { + this.contextRunner.run(context -> assertThat(context).hasSingleBean(TracePlatformTransactionManagerAspect.class) .hasSingleBean(TraceReactiveTransactionManagerBeanPostProcessor.class)); } @Test - void should_register_non_kafka_bean_post_processors_when_kafka_not_on_classpath() { - this.contextRunner - .withClassLoader( - new FilteredClassLoader("org.springframework.kafka.transaction.KafkaAwareTransactionManager")) - .run(context -> Assertions.assertThat(context) - .doesNotHaveBean(TraceKafkaPlatformTransactionManagerBeanPostProcessor.class) - .hasSingleBean(TracePlatformTransactionManagerBeanPostProcessor.class) - .hasSingleBean(TraceReactiveTransactionManagerBeanPostProcessor.class)); - } - - @Test - void should_not_register_bean_post_processor_when_tx_not_on_classpath() { + void should_not_register_aspect_when_tx_not_on_classpath() { this.contextRunner.withClassLoader(new FilteredClassLoader(PlatformTransactionManager.class)) - .run(context -> Assertions.assertThat(context) - .doesNotHaveBean(TracePlatformTransactionManagerBeanPostProcessor.class)); + .run(context -> assertThat(context).doesNotHaveBean(TracePlatformTransactionManagerAspect.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 -> Assertions.assertThat(context) - .doesNotHaveBean(TraceReactiveTransactionManagerBeanPostProcessor.class)); + this.contextRunner.withClassLoader(new FilteredClassLoader(ReactiveTransactionManager.class)).run( + context -> assertThat(context).doesNotHaveBean(TraceReactiveTransactionManagerBeanPostProcessor.class)); } @Test - 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)); + 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)); } } 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 c20099b61..65e6297be 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,9 @@ import org.springframework.kafka.transaction.KafkaAwareTransactionManager; * * @author Marcin Grzejszczak * @since 3.1.0 + * @deprecated will use {@link TracePlatformTransactionManagerAspect} instead */ +@Deprecated public class TraceKafkaAwareTransactionManager extends TracePlatformTransactionManager implements KafkaAwareTransactionManager { 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 new file mode 100644 index 000000000..5efc43d88 --- /dev/null +++ b/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/tx/TracePlatformTransactionManagerAspect.java @@ -0,0 +1,84 @@ +/* + * 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)); + } + +}