Avoid creating meter binders before registry has been customized

Previously, MeterRegistryPostProcessor would trigger the creation of all
meter binders and meter registry customizers before applying the
customizers and calling the binders. In some situations with complex
dependency graphs where the creation of a binder and the injection of
its dependencies inadvertently triggered some meter binding, this
could result in meters being bound before the registry had been
customized.

This commit reworks MeterRegistryPostProcessor and MeterRegistryConfigurer
to defer the retrieval of registry customizers and meter binders until
just before they are needed. As a result, customizers are now retrieved
and applied before the binders are retrieved.

Closes gh-15483

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryConfigurer.java

@@ -16,14 +16,15 @@
 
 package org.springframework.boot.actuate.autoconfigure.metrics;
 
-import java.util.Collection;
+import java.util.List;
-import java.util.Collections;
+import java.util.stream.Collectors;
 
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Metrics;
 import io.micrometer.core.instrument.binder.MeterBinder;
 import io.micrometer.core.instrument.config.MeterFilter;
 
+import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.boot.util.LambdaSafe;
 
 /**
@@ -36,21 +37,20 @@ import org.springframework.boot.util.LambdaSafe;
  */
 class MeterRegistryConfigurer {
 
-	private final Collection<MeterRegistryCustomizer<?>> customizers;
+	private final ObjectProvider<MeterRegistryCustomizer<?>> customizers;
 
-	private final Collection<MeterFilter> filters;
+	private final ObjectProvider<MeterFilter> filters;
 
-	private final Collection<MeterBinder> binders;
+	private final ObjectProvider<MeterBinder> binders;
 
 	private final boolean addToGlobalRegistry;
 
-	MeterRegistryConfigurer(Collection<MeterBinder> binders,
+	MeterRegistryConfigurer(ObjectProvider<MeterRegistryCustomizer<?>> customizers,
-			Collection<MeterFilter> filters,
+			ObjectProvider<MeterFilter> filters, ObjectProvider<MeterBinder> binders,
-			Collection<MeterRegistryCustomizer<?>> customizers,
 			boolean addToGlobalRegistry) {
-		this.binders = (binders != null) ? binders : Collections.emptyList();
+		this.customizers = customizers;
-		this.filters = (filters != null) ? filters : Collections.emptyList();
+		this.filters = filters;
-		this.customizers = (customizers != null) ? customizers : Collections.emptyList();
+		this.binders = binders;
 		this.addToGlobalRegistry = addToGlobalRegistry;
 	}
 
@@ -67,17 +67,23 @@ class MeterRegistryConfigurer {
 
 	@SuppressWarnings("unchecked")
 	private void customize(MeterRegistry registry) {
-		LambdaSafe.callbacks(MeterRegistryCustomizer.class, this.customizers, registry)
+		LambdaSafe
+				.callbacks(MeterRegistryCustomizer.class, asOrderedList(this.customizers),
+						registry)
 				.withLogger(MeterRegistryConfigurer.class)
 				.invoke((customizer) -> customizer.customize(registry));
 	}
 
 	private void addFilters(MeterRegistry registry) {
-		this.filters.forEach(registry.config()::meterFilter);
+		this.filters.orderedStream().forEach(registry.config()::meterFilter);
 	}
 
 	private void addBinders(MeterRegistry registry) {
-		this.binders.forEach((binder) -> binder.bindTo(registry));
+		this.binders.orderedStream().forEach((binder) -> binder.bindTo(registry));
+	}
+
+	private <T> List<T> asOrderedList(ObjectProvider<T> provider) {
+		return provider.orderedStream().collect(Collectors.toList());
 	}
 
 }

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryPostProcessor.java

@@ -16,9 +16,6 @@
 
 package org.springframework.boot.actuate.autoconfigure.metrics;
 
-import java.util.List;
-import java.util.stream.Collectors;
-
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.binder.MeterBinder;
 import io.micrometer.core.instrument.config.MeterFilter;
@@ -68,16 +65,11 @@ class MeterRegistryPostProcessor implements BeanPostProcessor {
 
 	private MeterRegistryConfigurer getConfigurer() {
 		if (this.configurer == null) {
-			this.configurer = new MeterRegistryConfigurer(
+			this.configurer = new MeterRegistryConfigurer(this.meterRegistryCustomizers,
-					asOrderedList(this.meterBinders), asOrderedList(this.meterFilters),
+					this.meterFilters, this.meterBinders,
-					asOrderedList(this.meterRegistryCustomizers),
 					this.metricsProperties.getObject().isUseGlobalRegistry());
 		}
 		return this.configurer;
 	}
 
-	private <T> List<T> asOrderedList(ObjectProvider<T> provider) {
-		return provider.orderedStream().collect(Collectors.toList());
-	}
-
 }

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryConfigurerIntegrationTests.java

@@ -17,14 +17,21 @@
 package org.springframework.boot.actuate.autoconfigure.metrics;
 
 import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.binder.MeterBinder;
 import io.micrometer.core.instrument.composite.CompositeMeterRegistry;
 import org.junit.Test;
 
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.config.BeanPostProcessor;
 import org.springframework.boot.actuate.autoconfigure.metrics.export.atlas.AtlasMetricsExportAutoConfiguration;
 import org.springframework.boot.actuate.autoconfigure.metrics.export.prometheus.PrometheusMetricsExportAutoConfiguration;
+import org.springframework.boot.actuate.autoconfigure.metrics.export.simple.SimpleMetricsExportAutoConfiguration;
 import org.springframework.boot.actuate.autoconfigure.metrics.test.MetricsRun;
 import org.springframework.boot.autoconfigure.AutoConfigurations;
 import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
 
 /**
  * Integration tests for {@link MeterRegistryConfigurer}.
@@ -49,4 +56,74 @@ public class MeterRegistryConfigurerIntegrationTests {
 		});
 	}
 
+	@Test
+	public void customizersAreAppliedBeforeBindersAreCreated() {
+		new ApplicationContextRunner()
+				.withConfiguration(AutoConfigurations.of(MetricsAutoConfiguration.class,
+						SimpleMetricsExportAutoConfiguration.class))
+				.withUserConfiguration(TestConfiguration.class).run((context) -> {
+
+				});
+	}
+
+	@Configuration
+	static class TestConfiguration {
+
+		@Bean
+		MeterBinder testBinder(Alpha thing) {
+			return (registry) -> {
+			};
+		}
+
+		@Bean
+		MeterRegistryCustomizer<?> testCustomizer() {
+			return (registry) -> {
+				registry.config().commonTags("testTag", "testValue");
+			};
+		}
+
+		@Bean
+		Alpha alpha() {
+			return new Alpha();
+		}
+
+		@Bean
+		Bravo bravo(Alpha alpha) {
+			return new Bravo(alpha);
+		}
+
+		@Bean
+		static BeanPostProcessor testPostProcessor(ApplicationContext context) {
+			return new BeanPostProcessor() {
+
+				@Override
+				public Object postProcessAfterInitialization(Object bean, String beanName)
+						throws BeansException {
+					if (bean instanceof Bravo) {
+						MeterRegistry meterRegistry = context
+								.getBean(MeterRegistry.class);
+						meterRegistry.gauge("test", 1);
+						System.out.println(
+								meterRegistry.find("test").gauge().getId().getTags());
+					}
+					return bean;
+				}
+
+			};
+		}
+
+	}
+
+	static class Alpha {
+
+	}
+
+	static class Bravo {
+
+		Bravo(Alpha alpha) {
+
+		}
+
+	}
+
 }

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryConfigurerTests.java

@@ -31,9 +31,12 @@ import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
+import org.springframework.beans.factory.ObjectProvider;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 
 /**
@@ -74,8 +77,10 @@ public class MeterRegistryConfigurerTests {
 	@Test
 	public void configureWhenCompositeShouldApplyCustomizer() {
 		this.customizers.add(this.mockCustomizer);
-		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(this.binders,
+		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
-				this.filters, this.customizers, false);
+				createObjectProvider(this.customizers),
+				createObjectProvider(this.filters), createObjectProvider(this.binders),
+				false);
 		CompositeMeterRegistry composite = new CompositeMeterRegistry();
 		configurer.configure(composite);
 		verify(this.mockCustomizer).customize(composite);
@@ -84,8 +89,10 @@ public class MeterRegistryConfigurerTests {
 	@Test
 	public void configureShouldApplyCustomizer() {
 		this.customizers.add(this.mockCustomizer);
-		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(this.binders,
+		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
-				this.filters, this.customizers, false);
+				createObjectProvider(this.customizers),
+				createObjectProvider(this.filters), createObjectProvider(this.binders),
+				false);
 		configurer.configure(this.mockRegistry);
 		verify(this.mockCustomizer).customize(this.mockRegistry);
 	}
@@ -93,8 +100,10 @@ public class MeterRegistryConfigurerTests {
 	@Test
 	public void configureShouldApplyFilter() {
 		this.filters.add(this.mockFilter);
-		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(this.binders,
+		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
-				this.filters, this.customizers, false);
+				createObjectProvider(this.customizers),
+				createObjectProvider(this.filters), createObjectProvider(this.binders),
+				false);
 		configurer.configure(this.mockRegistry);
 		verify(this.mockConfig).meterFilter(this.mockFilter);
 	}
@@ -102,8 +111,10 @@ public class MeterRegistryConfigurerTests {
 	@Test
 	public void configureShouldApplyBinder() {
 		this.binders.add(this.mockBinder);
-		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(this.binders,
+		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
-				this.filters, this.customizers, false);
+				createObjectProvider(this.customizers),
+				createObjectProvider(this.filters), createObjectProvider(this.binders),
+				false);
 		configurer.configure(this.mockRegistry);
 		verify(this.mockBinder).bindTo(this.mockRegistry);
 	}
@@ -113,8 +124,10 @@ public class MeterRegistryConfigurerTests {
 		this.customizers.add(this.mockCustomizer);
 		this.filters.add(this.mockFilter);
 		this.binders.add(this.mockBinder);
-		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(this.binders,
+		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
-				this.filters, this.customizers, false);
+				createObjectProvider(this.customizers),
+				createObjectProvider(this.filters), createObjectProvider(this.binders),
+				false);
 		configurer.configure(this.mockRegistry);
 		InOrder ordered = inOrder(this.mockBinder, this.mockConfig, this.mockCustomizer);
 		ordered.verify(this.mockCustomizer).customize(this.mockRegistry);
@@ -124,8 +137,10 @@ public class MeterRegistryConfigurerTests {
 
 	@Test
 	public void configureWhenAddToGlobalRegistryShouldAddToGlobalRegistry() {
-		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(this.binders,
+		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
-				this.filters, this.customizers, true);
+				createObjectProvider(this.customizers),
+				createObjectProvider(this.filters), createObjectProvider(this.binders),
+				true);
 		try {
 			configurer.configure(this.mockRegistry);
 			assertThat(Metrics.globalRegistry.getRegistries())
@@ -138,11 +153,20 @@ public class MeterRegistryConfigurerTests {
 
 	@Test
 	public void configureWhenNotAddToGlobalRegistryShouldAddToGlobalRegistry() {
-		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(this.binders,
+		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
-				this.filters, this.customizers, false);
+				createObjectProvider(this.customizers),
+				createObjectProvider(this.filters), createObjectProvider(this.binders),
+				false);
 		configurer.configure(this.mockRegistry);
 		assertThat(Metrics.globalRegistry.getRegistries())
 				.doesNotContain(this.mockRegistry);
 	}
 
+	@SuppressWarnings("unchecked")
+	private <T> ObjectProvider<T> createObjectProvider(List<T> objects) {
+		ObjectProvider<T> objectProvider = mock(ObjectProvider.class);
+		given(objectProvider.orderedStream()).willReturn(objects.stream());
+		return objectProvider;
+	}
+
 }