From 267a642e0f77204a1f2df1f8583d4bf17866d06a Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 20 Sep 2024 10:23:04 +0100 Subject: [PATCH] Reduce duplicate binding of meters to user-defined composites Fixes gh-42396 --- .../metrics/MeterRegistryPostProcessor.java | 51 ++++++++--- .../MeterRegistryPostProcessorTests.java | 88 +++++++++++++++---- 2 files changed, 106 insertions(+), 33 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryPostProcessor.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryPostProcessor.java index 06a0789ec6d..801a65f7e60 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryPostProcessor.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-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. @@ -42,7 +42,7 @@ import org.springframework.context.ApplicationContext; */ class MeterRegistryPostProcessor implements BeanPostProcessor, SmartInitializingSingleton { - private final boolean hasNoCompositeMeterRegistryBeans; + private final CompositeMeterRegistries compositeMeterRegistries; private final ObjectProvider properties; @@ -59,17 +59,13 @@ class MeterRegistryPostProcessor implements BeanPostProcessor, SmartInitializing MeterRegistryPostProcessor(ApplicationContext applicationContext, ObjectProvider metricsProperties, ObjectProvider> customizers, ObjectProvider filters, ObjectProvider binders) { - this(hasNoCompositeMeterRegistryBeans(applicationContext), metricsProperties, customizers, filters, binders); + this(CompositeMeterRegistries.of(applicationContext), metricsProperties, customizers, filters, binders); } - private static boolean hasNoCompositeMeterRegistryBeans(ApplicationContext applicationContext) { - return applicationContext.getBeanNamesForType(CompositeMeterRegistry.class, false, false).length == 0; - } - - MeterRegistryPostProcessor(boolean hasNoCompositeMeterRegistryBeans, ObjectProvider properties, - ObjectProvider> customizers, ObjectProvider filters, - ObjectProvider binders) { - this.hasNoCompositeMeterRegistryBeans = hasNoCompositeMeterRegistryBeans; + MeterRegistryPostProcessor(CompositeMeterRegistries compositeMeterRegistries, + ObjectProvider properties, ObjectProvider> customizers, + ObjectProvider filters, ObjectProvider binders) { + this.compositeMeterRegistries = compositeMeterRegistries; this.properties = properties; this.customizers = customizers; this.filters = filters; @@ -130,11 +126,21 @@ class MeterRegistryPostProcessor implements BeanPostProcessor, SmartInitializing } private boolean isBindable(MeterRegistry meterRegistry) { - return this.hasNoCompositeMeterRegistryBeans || isCompositeMeterRegistry(meterRegistry); + return isAutoConfiguredComposite(meterRegistry) || isCompositeWithOnlyUserDefinedComposites(meterRegistry) + || noCompositeMeterRegistries(); } - private boolean isCompositeMeterRegistry(MeterRegistry meterRegistry) { - return meterRegistry instanceof CompositeMeterRegistry; + private boolean isAutoConfiguredComposite(MeterRegistry meterRegistry) { + return meterRegistry instanceof AutoConfiguredCompositeMeterRegistry; + } + + private boolean isCompositeWithOnlyUserDefinedComposites(MeterRegistry meterRegistry) { + return this.compositeMeterRegistries == CompositeMeterRegistries.ONLY_USER_DEFINED + && meterRegistry instanceof CompositeMeterRegistry; + } + + private boolean noCompositeMeterRegistries() { + return this.compositeMeterRegistries == CompositeMeterRegistries.NONE; } void applyBinders(MeterRegistry meterRegistry) { @@ -149,4 +155,21 @@ class MeterRegistryPostProcessor implements BeanPostProcessor, SmartInitializing this.binders.orderedStream().forEach((binder) -> binder.bindTo(meterRegistry)); } + enum CompositeMeterRegistries { + + NONE, AUTO_CONFIGURED, ONLY_USER_DEFINED; + + private static CompositeMeterRegistries of(ApplicationContext context) { + if (hasBeansOfType(AutoConfiguredCompositeMeterRegistry.class, context)) { + return AUTO_CONFIGURED; + } + return hasBeansOfType(CompositeMeterRegistry.class, context) ? ONLY_USER_DEFINED : NONE; + } + + private static boolean hasBeansOfType(Class type, ApplicationContext context) { + return context.getBeanNamesForType(type, false, false).length > 0; + } + + } + } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryPostProcessorTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryPostProcessorTests.java index 54d3bbcc416..4621577027a 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryPostProcessorTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryPostProcessorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-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. @@ -17,8 +17,10 @@ package org.springframework.boot.actuate.autoconfigure.metrics; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import io.micrometer.core.instrument.Clock; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.MeterRegistry.Config; import io.micrometer.core.instrument.Metrics; @@ -32,6 +34,7 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.beans.factory.ObjectProvider; +import org.springframework.boot.actuate.autoconfigure.metrics.MeterRegistryPostProcessor.CompositeMeterRegistries; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; @@ -76,21 +79,34 @@ class MeterRegistryPostProcessorTests { } @Test - void postProcessAndInitializeWhenCompositeAppliesCustomizer() { + void postProcessAndInitializeWhenUserDefinedCompositeAppliesCustomizer() { this.customizers.add(this.mockCustomizer); - MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(false, - createObjectProvider(this.properties), createObjectProvider(this.customizers), - createObjectProvider(this.filters), createObjectProvider(this.binders)); + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor( + CompositeMeterRegistries.ONLY_USER_DEFINED, createObjectProvider(this.properties), + createObjectProvider(this.customizers), createObjectProvider(this.filters), + createObjectProvider(this.binders)); CompositeMeterRegistry composite = new CompositeMeterRegistry(); postProcessAndInitialize(processor, composite); then(this.mockCustomizer).should().customize(composite); } + @Test + void postProcessAndInitializeWhenAutoConfiguredCompositeAppliesCustomizer() { + this.customizers.add(this.mockCustomizer); + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(CompositeMeterRegistries.AUTO_CONFIGURED, + createObjectProvider(this.properties), createObjectProvider(this.customizers), null, + createObjectProvider(this.binders)); + AutoConfiguredCompositeMeterRegistry composite = new AutoConfiguredCompositeMeterRegistry(Clock.SYSTEM, + Collections.emptyList()); + postProcessAndInitialize(processor, composite); + then(this.mockCustomizer).should().customize(composite); + } + @Test void postProcessAndInitializeAppliesCustomizer() { given(this.mockRegistry.config()).willReturn(this.mockConfig); this.customizers.add(this.mockCustomizer); - MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(true, + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(CompositeMeterRegistries.NONE, createObjectProvider(this.properties), createObjectProvider(this.customizers), createObjectProvider(this.filters), createObjectProvider(this.binders)); postProcessAndInitialize(processor, this.mockRegistry); @@ -101,7 +117,7 @@ class MeterRegistryPostProcessorTests { void postProcessAndInitializeAppliesFilter() { given(this.mockRegistry.config()).willReturn(this.mockConfig); this.filters.add(this.mockFilter); - MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(true, + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(CompositeMeterRegistries.NONE, createObjectProvider(this.properties), createObjectProvider(this.customizers), createObjectProvider(this.filters), createObjectProvider(this.binders)); postProcessAndInitialize(processor, this.mockRegistry); @@ -112,7 +128,7 @@ class MeterRegistryPostProcessorTests { void postProcessAndInitializeBindsTo() { given(this.mockRegistry.config()).willReturn(this.mockConfig); this.binders.add(this.mockBinder); - MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(true, + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(CompositeMeterRegistries.NONE, createObjectProvider(this.properties), createObjectProvider(this.customizers), createObjectProvider(this.filters), createObjectProvider(this.binders)); postProcessAndInitialize(processor, this.mockRegistry); @@ -120,20 +136,54 @@ class MeterRegistryPostProcessorTests { } @Test - void postProcessAndInitializeWhenCompositeBindsTo() { + void whenUserDefinedCompositeThenPostProcessAndInitializeCompositeBindsTo() { this.binders.add(this.mockBinder); - MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(false, - createObjectProvider(this.properties), createObjectProvider(this.customizers), - createObjectProvider(this.filters), createObjectProvider(this.binders)); + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor( + CompositeMeterRegistries.ONLY_USER_DEFINED, createObjectProvider(this.properties), + createObjectProvider(this.customizers), createObjectProvider(this.filters), + createObjectProvider(this.binders)); CompositeMeterRegistry composite = new CompositeMeterRegistry(); postProcessAndInitialize(processor, composite); then(this.mockBinder).should().bindTo(composite); } @Test - void postProcessAndInitializeWhenCompositeExistsDoesNotBindTo() { + void whenUserDefinedCompositeThenPostProcessAndInitializeStandardRegistryDoesNotBindTo() { given(this.mockRegistry.config()).willReturn(this.mockConfig); - MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(false, + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor( + CompositeMeterRegistries.ONLY_USER_DEFINED, createObjectProvider(this.properties), + createObjectProvider(this.customizers), createObjectProvider(this.filters), null); + postProcessAndInitialize(processor, this.mockRegistry); + then(this.mockBinder).shouldHaveNoInteractions(); + } + + @Test + void whenAutoConfiguredCompositeThenPostProcessAndInitializeAutoConfiguredCompositeBindsTo() { + this.binders.add(this.mockBinder); + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(CompositeMeterRegistries.AUTO_CONFIGURED, + createObjectProvider(this.properties), createObjectProvider(this.customizers), null, + createObjectProvider(this.binders)); + AutoConfiguredCompositeMeterRegistry composite = new AutoConfiguredCompositeMeterRegistry(Clock.SYSTEM, + Collections.emptyList()); + postProcessAndInitialize(processor, composite); + then(this.mockBinder).should().bindTo(composite); + } + + @Test + void whenAutoConfiguredCompositeThenPostProcessAndInitializeCompositeDoesNotBindTo() { + this.binders.add(this.mockBinder); + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(CompositeMeterRegistries.AUTO_CONFIGURED, + createObjectProvider(this.properties), createObjectProvider(this.customizers), + createObjectProvider(this.filters), null); + CompositeMeterRegistry composite = new CompositeMeterRegistry(); + postProcessAndInitialize(processor, composite); + then(this.mockBinder).shouldHaveNoInteractions(); + } + + @Test + void whenAutoConfiguredCompositeThenPostProcessAndInitializeStandardRegistryDoesNotBindTo() { + given(this.mockRegistry.config()).willReturn(this.mockConfig); + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(CompositeMeterRegistries.AUTO_CONFIGURED, createObjectProvider(this.properties), createObjectProvider(this.customizers), createObjectProvider(this.filters), null); postProcessAndInitialize(processor, this.mockRegistry); @@ -141,12 +191,12 @@ class MeterRegistryPostProcessorTests { } @Test - void postProcessAndInitializeBeOrderedCustomizerThenFilterThenBindTo() { + void postProcessAndInitializeIsOrderedCustomizerThenFilterThenBindTo() { given(this.mockRegistry.config()).willReturn(this.mockConfig); this.customizers.add(this.mockCustomizer); this.filters.add(this.mockFilter); this.binders.add(this.mockBinder); - MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(true, + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(CompositeMeterRegistries.NONE, createObjectProvider(this.properties), createObjectProvider(this.customizers), createObjectProvider(this.filters), createObjectProvider(this.binders)); postProcessAndInitialize(processor, this.mockRegistry); @@ -160,7 +210,7 @@ class MeterRegistryPostProcessorTests { void postProcessAndInitializeWhenUseGlobalRegistryTrueAddsToGlobalRegistry() { given(this.mockRegistry.config()).willReturn(this.mockConfig); this.properties.setUseGlobalRegistry(true); - MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(true, + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(CompositeMeterRegistries.NONE, createObjectProvider(this.properties), createObjectProvider(this.customizers), createObjectProvider(this.filters), createObjectProvider(this.binders)); try { @@ -175,7 +225,7 @@ class MeterRegistryPostProcessorTests { @Test void postProcessAndInitializeWhenUseGlobalRegistryFalseDoesNotAddToGlobalRegistry() { given(this.mockRegistry.config()).willReturn(this.mockConfig); - MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(true, + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(CompositeMeterRegistries.NONE, createObjectProvider(this.properties), createObjectProvider(this.customizers), createObjectProvider(this.filters), createObjectProvider(this.binders)); postProcessAndInitialize(processor, this.mockRegistry); @@ -186,7 +236,7 @@ class MeterRegistryPostProcessorTests { void postProcessDoesNotBindToUntilSingletonsInitialized() { given(this.mockRegistry.config()).willReturn(this.mockConfig); this.binders.add(this.mockBinder); - MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(true, + MeterRegistryPostProcessor processor = new MeterRegistryPostProcessor(CompositeMeterRegistries.NONE, createObjectProvider(this.properties), createObjectProvider(this.customizers), createObjectProvider(this.filters), createObjectProvider(this.binders)); processor.postProcessAfterInitialization(this.mockRegistry, "meterRegistry");