Skip to content

S
spring-boot
Commit cc285d92 authored by Johnny Lim's avatar Johnny Lim Committed by Andy Wilkinson
Browse files
Options

Prevent double update of metrics when CompositeMeterRegistry exists 

See gh-16221
parent ba0279be
Hide whitespace changes
Inline Side-by-side
Showing with 83 additions and 13 deletions
spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryConfigurer.java
View file @ cc285d92
......@@ -22,6 +22,7 @@ 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.composite.CompositeMeterRegistry;
import io.micrometer.core.instrument.config.MeterFilter;
import org.springframework.beans.factory.ObjectProvider;
......@@ -45,13 +46,16 @@ class MeterRegistryConfigurer {
private final boolean addToGlobalRegistry;
private final boolean hasCompositeMeterRegistry;
MeterRegistryConfigurer(ObjectProvider<MeterRegistryCustomizer<?>> customizers,
ObjectProvider<MeterFilter> filters, ObjectProvider<MeterBinder> binders,
boolean addToGlobalRegistry) {
boolean addToGlobalRegistry, boolean hasCompositeMeterRegistry) {
this.customizers = customizers;
this.filters = filters;
this.binders = binders;
this.addToGlobalRegistry = addToGlobalRegistry;
this.hasCompositeMeterRegistry = hasCompositeMeterRegistry;
}
void configure(MeterRegistry registry) {
......@@ -59,7 +63,14 @@ class MeterRegistryConfigurer {
// tags or alter timer or summary configuration.
customize(registry);
addFilters(registry);
addBinders(registry);
if (this.hasCompositeMeterRegistry) {
if (registry instanceof CompositeMeterRegistry) {
addBinders(registry);
}
}
else {
addBinders(registry);
}
if (this.addToGlobalRegistry && registry != Metrics.globalRegistry) {
Metrics.addRegistry(registry);
}
......
spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryPostProcessor.java
View file @ cc285d92
......@@ -18,11 +18,13 @@ 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 io.micrometer.core.instrument.config.MeterFilter;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.context.ApplicationContext;
/**
* {@link BeanPostProcessor} that delegates to a lazily created
......@@ -44,14 +46,18 @@ class MeterRegistryPostProcessor implements BeanPostProcessor {
private volatile MeterRegistryConfigurer configurer;
private final ApplicationContext applicationContext;
MeterRegistryPostProcessor(ObjectProvider<MeterBinder> meterBinders,
ObjectProvider<MeterFilter> meterFilters,
ObjectProvider<MeterRegistryCustomizer<?>> meterRegistryCustomizers,
ObjectProvider<MetricsProperties> metricsProperties) {
ObjectProvider<MetricsProperties> metricsProperties,
ApplicationContext applicationContext) {
this.meterBinders = meterBinders;
this.meterFilters = meterFilters;
this.meterRegistryCustomizers = meterRegistryCustomizers;
this.metricsProperties = metricsProperties;
this.applicationContext = applicationContext;
}
@Override
......@@ -65,9 +71,13 @@ class MeterRegistryPostProcessor implements BeanPostProcessor {
private MeterRegistryConfigurer getConfigurer() {
if (this.configurer == null) {
boolean hasCompositeMeterRegistry = this.applicationContext
.getBeanNamesForType(CompositeMeterRegistry.class, false,
false).length != 0;
this.configurer = new MeterRegistryConfigurer(this.meterRegistryCustomizers,
this.meterFilters, this.meterBinders,
this.metricsProperties.getObject().isUseGlobalRegistry());
this.metricsProperties.getObject().isUseGlobalRegistry(),
hasCompositeMeterRegistry);
}
return this.configurer;
}
......
spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsAutoConfiguration.java
View file @ cc285d92
......@@ -27,6 +27,7 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.annotation.Order;
......@@ -55,9 +56,10 @@ public class MetricsAutoConfiguration {
ObjectProvider<MeterBinder> meterBinders,
ObjectProvider<MeterFilter> meterFilters,
ObjectProvider<MeterRegistryCustomizer<?>> meterRegistryCustomizers,
ObjectProvider<MetricsProperties> metricsProperties) {
ObjectProvider<MetricsProperties> metricsProperties,
ApplicationContext applicationContext) {
return new MeterRegistryPostProcessor(meterBinders, meterFilters,
meterRegistryCustomizers, metricsProperties);
meterRegistryCustomizers, metricsProperties, applicationContext);
}
@Bean
......
spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryConfigurerIntegrationTests.java
View file @ cc285d92
......@@ -16,14 +16,19 @@
package org.springframework.boot.actuate.autoconfigure.metrics;
import java.util.Map;
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.slf4j.Logger;
import org.slf4j.LoggerFactory;
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.jmx.JmxMetricsExportAutoConfiguration;
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;
......@@ -33,6 +38,8 @@ import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import static org.assertj.core.api.Assertions.assertThat;
/**
* Integration tests for {@link MeterRegistryConfigurer}.
*
......@@ -66,6 +73,46 @@ public class MeterRegistryConfigurerIntegrationTests {
});
}
@Test
public void counterWhenCompositeMeterRegistryDoesNotExistShouldWork() {
new ApplicationContextRunner()
.with(MetricsRun.limitedTo(JmxMetricsExportAutoConfiguration.class))
.withConfiguration(
AutoConfigurations.of(LogbackMetricsAutoConfiguration.class))
.run((context) -> {
Logger logger = LoggerFactory.getLogger("test-logger");
logger.error("Error.");
Map<String, MeterRegistry> registriesByName = context
.getBeansOfType(MeterRegistry.class);
assertThat(registriesByName).hasSize(1);
MeterRegistry registry = registriesByName.values().iterator().next();
assertThat(registry.get("logback.events").tag("level", "error")
.counter().count()).isEqualTo(1);
});
}
@Test
public void counterWhenCompositeMeterRegistryExistsShouldWork() {
new ApplicationContextRunner()
.with(MetricsRun.limitedTo(JmxMetricsExportAutoConfiguration.class,
PrometheusMetricsExportAutoConfiguration.class))
.withConfiguration(
AutoConfigurations.of(LogbackMetricsAutoConfiguration.class))
.run((context) -> {
Logger logger = LoggerFactory.getLogger("test-logger");
logger.error("Error.");
Map<String, MeterRegistry> registriesByName = context
.getBeansOfType(MeterRegistry.class);
assertThat(registriesByName).hasSize(3);
registriesByName.forEach((name,
registry) -> assertThat(registry.get("logback.events")
.tag("level", "error").counter().count())
.isEqualTo(1));
});
}
@Configuration
static class TestConfiguration {
......
spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryConfigurerTests.java
View file @ cc285d92
......@@ -80,7 +80,7 @@ public class MeterRegistryConfigurerTests {
MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
createObjectProvider(this.customizers),
createObjectProvider(this.filters), createObjectProvider(this.binders),
false);
false, true);
CompositeMeterRegistry composite = new CompositeMeterRegistry();
configurer.configure(composite);
verify(this.mockCustomizer).customize(composite);
......@@ -92,7 +92,7 @@ public class MeterRegistryConfigurerTests {
MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
createObjectProvider(this.customizers),
createObjectProvider(this.filters), createObjectProvider(this.binders),
false);
false, false);
configurer.configure(this.mockRegistry);
verify(this.mockCustomizer).customize(this.mockRegistry);
}
......@@ -103,7 +103,7 @@ public class MeterRegistryConfigurerTests {
MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
createObjectProvider(this.customizers),
createObjectProvider(this.filters), createObjectProvider(this.binders),
false);
false, false);
configurer.configure(this.mockRegistry);
verify(this.mockConfig).meterFilter(this.mockFilter);
}
......@@ -114,7 +114,7 @@ public class MeterRegistryConfigurerTests {
MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
createObjectProvider(this.customizers),
createObjectProvider(this.filters), createObjectProvider(this.binders),
false);
false, false);
configurer.configure(this.mockRegistry);
verify(this.mockBinder).bindTo(this.mockRegistry);
}
......@@ -127,7 +127,7 @@ public class MeterRegistryConfigurerTests {
MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
createObjectProvider(this.customizers),
createObjectProvider(this.filters), createObjectProvider(this.binders),
false);
false, false);
configurer.configure(this.mockRegistry);
InOrder ordered = inOrder(this.mockBinder, this.mockConfig, this.mockCustomizer);
ordered.verify(this.mockCustomizer).customize(this.mockRegistry);
......@@ -140,7 +140,7 @@ public class MeterRegistryConfigurerTests {
MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
createObjectProvider(this.customizers),
createObjectProvider(this.filters), createObjectProvider(this.binders),
true);
true, false);
try {
configurer.configure(this.mockRegistry);
assertThat(Metrics.globalRegistry.getRegistries())
......@@ -156,7 +156,7 @@ public class MeterRegistryConfigurerTests {
MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
createObjectProvider(this.customizers),
createObjectProvider(this.filters), createObjectProvider(this.binders),
false);
false, false);
configurer.configure(this.mockRegistry);
assertThat(Metrics.globalRegistry.getRegistries())
.doesNotContain(this.mockRegistry);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment