Skip to content

Commit e5a0b16

Browse files
philwebbwilkinsona
philwebb
authored and
wilkinsona
committed
Configure MeterBinders after beans have been created
Update `MeterRegistryPostProcessor` to configure `MeterRegistry` beans in two distinct sweeps. The first sweep applies customizers and filters as the `MeterRegistry` bean is initialized, the second sweep applies `MeterBinder` beans once all singletons have been instantiated. Prior to this commit, it was not possible for a `MeterBinder` bean to directly or indirectly use a `MeterRegistry`. It was also possible for bound meters to cause a deadlock during refresh processing if those meters could be updated on a thread other than main, such as GC notifications. Fixes gh-30636 Fixes gh-33070
1 parent e600841 commit e5a0b16

File tree

7 files changed

+242
-164
lines changed

7 files changed

+242
-164
lines changed

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

Lines changed: 0 additions & 86 deletions
This file was deleted.

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

Lines changed: 99 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,65 +16,138 @@
1616

1717
package org.springframework.boot.actuate.autoconfigure.metrics;
1818

19+
import java.util.LinkedHashSet;
20+
import java.util.List;
21+
import java.util.Set;
22+
1923
import io.micrometer.core.instrument.MeterRegistry;
24+
import io.micrometer.core.instrument.Metrics;
2025
import io.micrometer.core.instrument.binder.MeterBinder;
2126
import io.micrometer.core.instrument.composite.CompositeMeterRegistry;
2227
import io.micrometer.core.instrument.config.MeterFilter;
2328

2429
import org.springframework.beans.BeansException;
2530
import org.springframework.beans.factory.ObjectProvider;
31+
import org.springframework.beans.factory.SmartInitializingSingleton;
2632
import org.springframework.beans.factory.config.BeanPostProcessor;
33+
import org.springframework.boot.util.LambdaSafe;
2734
import org.springframework.context.ApplicationContext;
2835

2936
/**
30-
* {@link BeanPostProcessor} that delegates to a lazily created
31-
* {@link MeterRegistryConfigurer} to post-process {@link MeterRegistry} beans.
37+
* {@link BeanPostProcessor} for {@link MeterRegistry} beans.
3238
*
3339
* @author Jon Schneider
3440
* @author Phillip Webb
3541
* @author Andy Wilkinson
3642
*/
37-
class MeterRegistryPostProcessor implements BeanPostProcessor {
43+
class MeterRegistryPostProcessor implements BeanPostProcessor, SmartInitializingSingleton {
44+
45+
private final boolean hasNoCompositeMeterRegistryBeans;
46+
47+
private final boolean useGlobalRegistry;
48+
49+
private final ObjectProvider<MeterRegistryCustomizer<?>> customizers;
3850

39-
private final ObjectProvider<MeterBinder> meterBinders;
51+
private final ObjectProvider<MeterFilter> filters;
4052

41-
private final ObjectProvider<MeterFilter> meterFilters;
53+
private final ObjectProvider<MeterBinder> binders;
4254

43-
private final ObjectProvider<MeterRegistryCustomizer<?>> meterRegistryCustomizers;
55+
private volatile boolean deferBinding = true;
4456

45-
private final ObjectProvider<MetricsProperties> metricsProperties;
57+
private final Set<MeterRegistry> deferredBindings = new LinkedHashSet<>();
4658

47-
private volatile MeterRegistryConfigurer configurer;
59+
MeterRegistryPostProcessor(ApplicationContext applicationContext, MetricsProperties metricsProperties,
60+
ObjectProvider<MeterRegistryCustomizer<?>> customizers, ObjectProvider<MeterFilter> filters,
61+
ObjectProvider<MeterBinder> binders) {
62+
this(hasNoCompositeMeterRegistryBeans(applicationContext), metricsProperties.isUseGlobalRegistry(), customizers,
63+
filters, binders);
64+
}
65+
66+
private static boolean hasNoCompositeMeterRegistryBeans(ApplicationContext applicationContext) {
67+
return applicationContext.getBeanNamesForType(CompositeMeterRegistry.class, false, false).length == 0;
68+
}
4869

49-
private final ApplicationContext applicationContext;
70+
MeterRegistryPostProcessor(boolean hasNoCompositeMeterRegistryBeans, boolean useGlobalRegistry,
71+
ObjectProvider<MeterRegistryCustomizer<?>> customizers, ObjectProvider<MeterFilter> filters,
72+
ObjectProvider<MeterBinder> binders) {
73+
this.hasNoCompositeMeterRegistryBeans = hasNoCompositeMeterRegistryBeans;
74+
this.useGlobalRegistry = useGlobalRegistry;
75+
this.customizers = customizers;
76+
this.filters = filters;
77+
this.binders = binders;
5078

51-
MeterRegistryPostProcessor(ObjectProvider<MeterBinder> meterBinders, ObjectProvider<MeterFilter> meterFilters,
52-
ObjectProvider<MeterRegistryCustomizer<?>> meterRegistryCustomizers,
53-
ObjectProvider<MetricsProperties> metricsProperties, ApplicationContext applicationContext) {
54-
this.meterBinders = meterBinders;
55-
this.meterFilters = meterFilters;
56-
this.meterRegistryCustomizers = meterRegistryCustomizers;
57-
this.metricsProperties = metricsProperties;
58-
this.applicationContext = applicationContext;
5979
}
6080

6181
@Override
6282
public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException {
6383
if (bean instanceof MeterRegistry meterRegistry) {
64-
getConfigurer().configure(meterRegistry);
84+
postProcessMeterRegistry(meterRegistry);
6585
}
6686
return bean;
6787
}
6888

69-
private MeterRegistryConfigurer getConfigurer() {
70-
if (this.configurer == null) {
71-
boolean hasCompositeMeterRegistry = this.applicationContext
72-
.getBeanNamesForType(CompositeMeterRegistry.class, false, false).length != 0;
73-
this.configurer = new MeterRegistryConfigurer(this.meterRegistryCustomizers, this.meterFilters,
74-
this.meterBinders, this.metricsProperties.getObject().isUseGlobalRegistry(),
75-
hasCompositeMeterRegistry);
89+
@Override
90+
public void afterSingletonsInstantiated() {
91+
synchronized (this.deferredBindings) {
92+
this.deferBinding = false;
93+
this.deferredBindings.forEach(this::applyBinders);
94+
}
95+
}
96+
97+
private void postProcessMeterRegistry(MeterRegistry meterRegistry) {
98+
// Customizers must be applied before binders, as they may add custom tags or
99+
// alter timer or summary configuration.
100+
applyCustomizers(meterRegistry);
101+
applyFilters(meterRegistry);
102+
addToGlobalRegistryIfNecessary(meterRegistry);
103+
if (isBindable(meterRegistry)) {
104+
applyBinders(meterRegistry);
105+
}
106+
}
107+
108+
@SuppressWarnings("unchecked")
109+
private void applyCustomizers(MeterRegistry meterRegistry) {
110+
List<MeterRegistryCustomizer<?>> customizers = this.customizers.orderedStream().toList();
111+
LambdaSafe.callbacks(MeterRegistryCustomizer.class, customizers, meterRegistry)
112+
.withLogger(MeterRegistryPostProcessor.class)
113+
.invoke((customizer) -> customizer.customize(meterRegistry));
114+
}
115+
116+
private void applyFilters(MeterRegistry meterRegistry) {
117+
if (meterRegistry instanceof AutoConfiguredCompositeMeterRegistry) {
118+
return;
119+
}
120+
this.filters.orderedStream().forEach(meterRegistry.config()::meterFilter);
121+
}
122+
123+
private void addToGlobalRegistryIfNecessary(MeterRegistry meterRegistry) {
124+
if (this.useGlobalRegistry && !isGlobalRegistry(meterRegistry)) {
125+
Metrics.addRegistry(meterRegistry);
126+
}
127+
}
128+
129+
private boolean isGlobalRegistry(MeterRegistry meterRegistry) {
130+
return meterRegistry == Metrics.globalRegistry;
131+
}
132+
133+
private boolean isBindable(MeterRegistry meterRegistry) {
134+
return this.hasNoCompositeMeterRegistryBeans || isCompositeMeterRegistry(meterRegistry);
135+
}
136+
137+
private boolean isCompositeMeterRegistry(MeterRegistry meterRegistry) {
138+
return meterRegistry instanceof CompositeMeterRegistry;
139+
}
140+
141+
void applyBinders(MeterRegistry meterRegistry) {
142+
if (this.deferBinding) {
143+
synchronized (this.deferredBindings) {
144+
if (this.deferBinding) {
145+
this.deferredBindings.add(meterRegistry);
146+
return;
147+
}
148+
}
76149
}
77-
return this.configurer;
150+
this.binders.orderedStream().forEach((binder) -> binder.bindTo(meterRegistry));
78151
}
79152

80153
}

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,11 @@ public Clock micrometerClock() {
5050
}
5151

5252
@Bean
53-
public static MeterRegistryPostProcessor meterRegistryPostProcessor(ObjectProvider<MeterBinder> meterBinders,
54-
ObjectProvider<MeterFilter> meterFilters,
55-
ObjectProvider<MeterRegistryCustomizer<?>> meterRegistryCustomizers,
56-
ObjectProvider<MetricsProperties> metricsProperties, ApplicationContext applicationContext) {
57-
return new MeterRegistryPostProcessor(meterBinders, meterFilters, meterRegistryCustomizers, metricsProperties,
58-
applicationContext);
53+
public static MeterRegistryPostProcessor meterRegistryPostProcessor(ApplicationContext applicationContext,
54+
MetricsProperties metricsProperties, ObjectProvider<MeterRegistryCustomizer<?>> meterRegistryCustomizers,
55+
ObjectProvider<MeterFilter> meterFilters, ObjectProvider<MeterBinder> meterBinders) {
56+
return new MeterRegistryPostProcessor(applicationContext, metricsProperties, meterRegistryCustomizers,
57+
meterFilters, meterBinders);
5958
}
6059

6160
@Bean

‎spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/amqp/RabbitMetricsAutoConfigurationMeterBinderCycleIntegrationTests.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/*
2+
* Copyright 2012-2022 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.actuate.autoconfigure.amqp;
18+
19+
import io.micrometer.core.instrument.MeterRegistry;
20+
import io.micrometer.core.instrument.binder.MeterBinder;
21+
import org.junit.jupiter.api.Test;
22+
23+
import org.springframework.amqp.rabbit.core.RabbitTemplate;
24+
import org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration;
25+
import org.springframework.boot.actuate.autoconfigure.metrics.amqp.RabbitMetricsAutoConfiguration;
26+
import org.springframework.boot.actuate.autoconfigure.metrics.export.simple.SimpleMetricsExportAutoConfiguration;
27+
import org.springframework.boot.autoconfigure.amqp.RabbitAutoConfiguration;
28+
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
29+
import org.springframework.context.annotation.Configuration;
30+
import org.springframework.context.annotation.Import;
31+
32+
/**
33+
* Integration test to check that {@link RabbitMetricsAutoConfiguration} does not cause a
34+
* dependency cycle when used with {@link MeterBinder}.
35+
*
36+
* @author Phillip Webb
37+
* @see <a href="https://github.com/spring-projects/spring-boot/issues/30636">gh-30636</a>
38+
*/
39+
class RabbitMetricsAutoConfigurationMeterBinderCycleIntegrationTests {
40+
41+
@Test
42+
void doesNotFormCycle() {
43+
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfig.class);
44+
context.getBean(TestService.class);
45+
context.close();
46+
}
47+
48+
@Configuration
49+
@Import({ TestService.class, RabbitAutoConfiguration.class, MetricsAutoConfiguration.class,
50+
SimpleMetricsExportAutoConfiguration.class, RabbitMetricsAutoConfiguration.class })
51+
static class TestConfig {
52+
53+
}
54+
55+
static class TestService implements MeterBinder {
56+
57+
TestService(RabbitTemplate rabbitTemplate) {
58+
}
59+
60+
@Override
61+
public void bindTo(MeterRegistry registry) {
62+
}
63+
64+
}
65+
66+
}

0 commit comments

Comments
 (0)