From d14ba59837bd366cbbcaac00aae1e01fc0fd7e2a Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Mon, 16 Oct 2023 13:34:41 +0200 Subject: [PATCH] GH-331 - Avoid processing infrastructure beans in observability infrastructure. We now skip infrastructure role beans during the processing of beans that could be subject to inter-module interaction observability. Previously, we could accidentally trigger a dependency cycle if the ModuleTracingBeanPostProcessor triggered the initial creation of AutoConfigurationPackages as that would then trigger the PostProcessor in turn which would try to lookup the ACP bean again to determine whether it should post process that to apply observability. --- .../ModuleTracingBeanPostProcessor.java | 20 ++++++++++++++----- .../ModuleObservabilityAutoConfiguration.java | 5 +++-- .../SpringBootApplicationRuntime.java | 13 +++++++++++- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ModuleTracingBeanPostProcessor.java b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ModuleTracingBeanPostProcessor.java index af18d481..7bf3133c 100644 --- a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ModuleTracingBeanPostProcessor.java +++ b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/ModuleTracingBeanPostProcessor.java @@ -27,8 +27,9 @@ import org.springframework.aop.support.ComposablePointcut; import org.springframework.aop.support.DefaultPointcutAdvisor; import org.springframework.aop.support.StaticMethodMatcher; import org.springframework.beans.BeansException; +import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanPostProcessor; -import org.springframework.modulith.core.ApplicationModules; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.modulith.runtime.ApplicationModulesRuntime; import org.springframework.util.Assert; @@ -45,6 +46,7 @@ public class ModuleTracingBeanPostProcessor extends ModuleTracingSupport impleme private final ApplicationModulesRuntime runtime; private final Supplier tracer; private final Map advisors; + private final ConfigurableListableBeanFactory factory; /** * Creates a new {@link ModuleTracingBeanPostProcessor} for the given {@link ApplicationModulesRuntime} and @@ -53,7 +55,8 @@ public class ModuleTracingBeanPostProcessor extends ModuleTracingSupport impleme * @param runtime must not be {@literal null}. * @param tracer must not be {@literal null}. */ - public ModuleTracingBeanPostProcessor(ApplicationModulesRuntime runtime, Supplier tracer) { + public ModuleTracingBeanPostProcessor(ApplicationModulesRuntime runtime, Supplier tracer, + ConfigurableListableBeanFactory factory) { Assert.notNull(runtime, "ApplicationModulesRuntime must not be null!"); Assert.notNull(tracer, "Tracer must not be null!"); @@ -61,6 +64,7 @@ public class ModuleTracingBeanPostProcessor extends ModuleTracingSupport impleme this.runtime = runtime; this.tracer = tracer; this.advisors = new HashMap<>(); + this.factory = factory; } /* @@ -70,13 +74,13 @@ public class ModuleTracingBeanPostProcessor extends ModuleTracingSupport impleme @Override public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { - Class type = runtime.getUserClass(bean, beanName); + var type = runtime.getUserClass(bean, beanName); - if (!runtime.isApplicationClass(type) || !type.isInstance(bean)) { + if (!type.isInstance(bean) || isInfrastructureBean(beanName) || !runtime.isApplicationClass(type)) { return bean; } - ApplicationModules modules = runtime.get(); + var modules = runtime.get(); return modules.getModuleByType(type.getName()) .map(DefaultObservedModule::new) @@ -91,6 +95,12 @@ public class ModuleTracingBeanPostProcessor extends ModuleTracingSupport impleme }).orElse(bean); } + private boolean isInfrastructureBean(String beanName) { + + return factory.containsBean(beanName) && + factory.getBeanDefinition(beanName).getRole() == BeanDefinition.ROLE_INFRASTRUCTURE; + } + private Advisor getOrBuildAdvisor(ObservedModule module, ObservedModuleType type) { return advisors.computeIfAbsent(module.getName(), __ -> { diff --git a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/autoconfigure/ModuleObservabilityAutoConfiguration.java b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/autoconfigure/ModuleObservabilityAutoConfiguration.java index c7cde293..2f24bb8d 100644 --- a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/autoconfigure/ModuleObservabilityAutoConfiguration.java +++ b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/autoconfigure/ModuleObservabilityAutoConfiguration.java @@ -25,6 +25,7 @@ import brave.propagation.TraceContext; import io.micrometer.tracing.Tracer; import org.springframework.beans.factory.ObjectProvider; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Bean; @@ -42,8 +43,8 @@ class ModuleObservabilityAutoConfiguration { @Bean static ModuleTracingBeanPostProcessor moduleTracingBeanPostProcessor(ApplicationModulesRuntime runtime, - ObjectProvider tracer) { - return new ModuleTracingBeanPostProcessor(runtime, () -> tracer.getObject()); + ObjectProvider tracer, ConfigurableListableBeanFactory factory) { + return new ModuleTracingBeanPostProcessor(runtime, () -> tracer.getObject(), factory); } @Bean diff --git a/spring-modulith-runtime/src/main/java/org/springframework/modulith/runtime/autoconfigure/SpringBootApplicationRuntime.java b/spring-modulith-runtime/src/main/java/org/springframework/modulith/runtime/autoconfigure/SpringBootApplicationRuntime.java index e05741af..a4e98fc5 100644 --- a/spring-modulith-runtime/src/main/java/org/springframework/modulith/runtime/autoconfigure/SpringBootApplicationRuntime.java +++ b/spring-modulith-runtime/src/main/java/org/springframework/modulith/runtime/autoconfigure/SpringBootApplicationRuntime.java @@ -16,6 +16,7 @@ package org.springframework.modulith.runtime.autoconfigure; import java.util.Arrays; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -38,6 +39,7 @@ class SpringBootApplicationRuntime implements ApplicationRuntime { private final ApplicationContext context; private Class mainApplicationClass; + private List resolvedAutoConfigurationPackages; /** * Creates a new {@link SpringBootApplicationRuntime} for the given {@link ApplicationContext}. @@ -113,6 +115,15 @@ class SpringBootApplicationRuntime implements ApplicationRuntime { } return fqn.startsWith(applicationClass.getPackage().getName()) - || AutoConfigurationPackages.get(context).stream().anyMatch(pkg -> fqn.startsWith(pkg)); + || getAutoConfigurationPackages().stream().anyMatch(pkg -> fqn.startsWith(pkg)); + } + + private List getAutoConfigurationPackages() { + + if (resolvedAutoConfigurationPackages == null) { + this.resolvedAutoConfigurationPackages = AutoConfigurationPackages.get(context); + } + + return resolvedAutoConfigurationPackages; } }