GH-615 - Make sure we always place the ModuleEntryInterceptor after the AsyncAnnotationAdvisor.

To properly create spans for the actual method invocation, not the async dispatch.

Author: odrotbohm

spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ModuleTracingBeanPostProcessor.java

@@ -18,11 +18,13 @@
 import io.micrometer.tracing.Tracer;
 
 import java.lang.reflect.Method;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.function.Supplier;
 
 import org.springframework.aop.Advisor;
+import org.springframework.aop.framework.Advised;
 import org.springframework.aop.support.ComposablePointcut;
 import org.springframework.aop.support.DefaultPointcutAdvisor;
 import org.springframework.aop.support.StaticMethodMatcher;
@@ -81,19 +83,21 @@ public Object postProcessAfterInitialization(Object bean, String beanName) throw
 			return bean;
 		}
 
+		if (alreadyAdvised(bean)) {
+			return bean;
+		}
+
 		var modules = runtime.get();
 
-		return modules.getModuleByType(type.getName())
-				.map(DefaultObservedModule::new)
-				.map(it -> {
+		return modules.getModuleByType(type.getName()).map(DefaultObservedModule::new).map(it -> {
 
-					var moduleType = it.getObservedModuleType(type, modules);
+			var moduleType = it.getObservedModuleType(type, modules);
 
-					return moduleType != null //
-							? addAdvisor(bean, getOrBuildAdvisor(it, moduleType)) //
-							: bean;
+			return moduleType != null //
+					? addAdvisor(bean, getOrBuildAdvisor(it, moduleType)) //
+					: bean;
 
-				}).orElse(bean);
+		}).orElse(bean);
 	}
 
 	private boolean isInfrastructureBean(String beanName) {
@@ -116,13 +120,15 @@ private boolean isInfrastructureBean(String beanName) {
 	private Advisor getOrBuildAdvisor(ObservedModule module, ObservedModuleType type) {
 
 		return advisors.computeIfAbsent(module.getName(), __ -> {
+			return new ApplicationModuleObservingAdvisor(type, ModuleEntryInterceptor.of(module, tracer.get()));
+		});
+	}
 
-			var interceptor = ModuleEntryInterceptor.of(module, tracer.get());
-			var matcher = new ObservableTypeMethodMatcher(type);
-			var pointcut = new ComposablePointcut(matcher);
+	private static boolean alreadyAdvised(Object bean) {
 
-			return new DefaultPointcutAdvisor(pointcut, interceptor);
-		});
+		return bean instanceof Advised advised
+				&& Arrays.stream(advised.getAdvisors())
+						.anyMatch(ApplicationModuleObservingAdvisor.class::isInstance);
 	}
 
 	private static class ObservableTypeMethodMatcher extends StaticMethodMatcher {
@@ -150,4 +156,13 @@ public boolean matches(Method method, Class<?> targetClass) {
 			return type.getMethodsToIntercept().test(method);
 		}
 	}
+
+	static class ApplicationModuleObservingAdvisor extends DefaultPointcutAdvisor {
+
+		private static final long serialVersionUID = -391548409986032658L;
+
+		public ApplicationModuleObservingAdvisor(ObservedModuleType type, ModuleEntryInterceptor interceptor) {
+			super(new ComposablePointcut(new ObservableTypeMethodMatcher(type)), interceptor);
+		}
+	}
 }

spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ModuleTracingSupport.java

@@ -21,6 +21,7 @@
 import org.springframework.aop.framework.Advised;
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.beans.factory.BeanClassLoaderAware;
+import org.springframework.scheduling.annotation.AsyncAnnotationAdvisor;
 
 /**
  * @author Oliver Drotbohm
@@ -44,9 +45,10 @@ protected final Object addAdvisor(Object bean, Advisor advisor) {
 
 	protected final Object addAdvisor(Object bean, Advisor advisor, Consumer<ProxyFactory> customizer) {
 
-		if (Advised.class.isInstance(bean)) {
+		if (bean instanceof Advised advised) {
+
+			advised.addAdvisor(asyncAdvisorIndex(advised) + 1, advisor);
 
-			((Advised) bean).addAdvisor(0, advisor);
 			return bean;
 
 		} else {
@@ -58,4 +60,18 @@ protected final Object addAdvisor(Object bean, Advisor advisor, Consumer<ProxyFa
 			return factory.getProxy(classLoader);
 		}
 	}
+
+	private static int asyncAdvisorIndex(Advised advised) {
+
+		Advisor[] advisors = advised.getAdvisors();
+
+		for (int i = 0; i < advised.getAdvisorCount(); i++) {
+
+			if (advisors[i] instanceof AsyncAnnotationAdvisor) {
+				return i;
+			}
+		}
+
+		return -1;
+	}
 }

spring-modulith-observability/src/test/java/example/ExampleApplication.java

@@ -17,10 +17,12 @@
 
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.scheduling.annotation.EnableAsync;
 
 /**
  * @author Oliver Drotbohm
  */
+@EnableAsync
 @SpringBootApplication
 public class ExampleApplication {
 

spring-modulith-observability/src/test/java/example/sample/SampleComponent.java

@@ -15,6 +15,7 @@
  */
 package example.sample;
 
+import org.springframework.scheduling.annotation.Async;
 import org.springframework.stereotype.Component;
 
 /**
@@ -23,5 +24,6 @@
 @Component
 public class SampleComponent {
 
-	void someMethod() {}
+	@Async
+	public void someMethod() {}
 }

spring-modulith-observability/src/test/java/org/springframework/modulith/observability/ModuleTracingBeanPostProcessorIntegrationTests.java

@@ -0,0 +1,87 @@
+/*
+ * Copyright 2024 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.modulith.observability;
+
+import static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+import example.ExampleApplication;
+import example.sample.SampleComponent;
+import io.micrometer.tracing.Tracer;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.aop.Advisor;
+import org.springframework.aop.framework.Advised;
+import org.springframework.boot.SpringApplication;
+import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.modulith.observability.ModuleTracingBeanPostProcessor.ApplicationModuleObservingAdvisor;
+import org.springframework.modulith.runtime.ApplicationModulesRuntime;
+import org.springframework.modulith.runtime.ApplicationRuntime;
+import org.springframework.modulith.test.TestApplicationModules;
+import org.springframework.scheduling.annotation.AsyncAnnotationAdvisor;
+
+/**
+ * Integration tests for {@link ModuleTracingBeanPostProcessor}.
+ *
+ * @author Oliver Drotbohm
+ */
+class ModuleTracingBeanPostProcessorIntegrationTests {
+
+	@Test
+	void decoratesExposedComponentsWithTracingInterceptor() {
+
+		SampleComponent bean = SpringApplication
+				.run(new Class<?>[] { ExampleApplication.class, ModuleTracingConfiguration.class }, new String[] {})
+				.getBean(SampleComponent.class);
+
+		assertThat(bean).isInstanceOfSatisfying(Advised.class, it -> {
+
+			var advisors = it.getAdvisors();
+
+			var asyncIndex = advisorIndex(AsyncAnnotationAdvisor.class, advisors);
+			var tracingIndex = advisorIndex(ApplicationModuleObservingAdvisor.class, advisors);
+
+			assertThat(tracingIndex).isGreaterThan(asyncIndex);
+		});
+	}
+
+	@Configuration
+	static class ModuleTracingConfiguration {
+
+		@Bean
+		ModuleTracingBeanPostProcessor foo(ConfigurableApplicationContext context) {
+
+			var runtime = ApplicationRuntime.of(context);
+			var modulesRuntime = new ApplicationModulesRuntime(() -> TestApplicationModules.of(ExampleApplication.class),
+					runtime);
+
+			return new ModuleTracingBeanPostProcessor(modulesRuntime, () -> mock(Tracer.class), context.getBeanFactory());
+		}
+	}
+
+	private static int advisorIndex(Class<? extends Advisor> type, Advisor[] advisors) {
+
+		for (int i = 0; i < advisors.length; i++) {
+			if (type.isInstance(advisors[i])) {
+				return i;
+			}
+		}
+
+		throw new AssertionError("No advisor of type %s found!".formatted(type.getName()));
+	}
+}

spring-modulith-observability/src/test/java/org/springframework/modulith/observability/ModuleTracingBeanPostProcessorUnitTests.java

@@ -31,7 +31,7 @@
  *
  * @author Oliver Drotbohm
  */
-public class ModuleTracingBeanPostProcessorUnitTests {
+class ModuleTracingBeanPostProcessorUnitTests {
 
 	@Test // GH-498
 	void doesNotProxyConfiguationProperties() {

spring-modulith-observability/src/test/java/org/springframework/modulith/observability/ObservedModuleTypeUnitTests.java

@@ -33,7 +33,7 @@
  *
  * @author Oliver Drotbohm
  */
-public class ObservedModuleTypeUnitTests {
+class ObservedModuleTypeUnitTests {
 
 	static final ApplicationModules modules = TestApplicationModules.of("example");