From 35ea3b234dc430d8957c2d376bb52a6b61dcf593 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 12 Aug 2021 10:05:31 +0100 Subject: [PATCH] Prevent repository metrics from causing a circular reference Fixes gh-27591 --- .../RepositoryMetricsAutoConfiguration.java | 8 +++--- ...ricsAutoConfigurationIntegrationTests.java | 19 +++++++++++++ ...positoryMetricsAutoConfigurationTests.java | 13 +++++---- ...icsRepositoryMethodInvocationListener.java | 27 ++++++++++++++++--- ...positoryMethodInvocationListenerTests.java | 4 +-- 5 files changed, 56 insertions(+), 15 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfiguration.java index 18347e09008..b89546df22a 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfiguration.java @@ -64,11 +64,11 @@ public class RepositoryMetricsAutoConfiguration { @Bean @ConditionalOnMissingBean - public MetricsRepositoryMethodInvocationListener metricsRepositoryMethodInvocationListener(MeterRegistry registry, - RepositoryTagsProvider tagsProvider) { + public MetricsRepositoryMethodInvocationListener metricsRepositoryMethodInvocationListener( + ObjectProvider registry, RepositoryTagsProvider tagsProvider) { Repository properties = this.properties.getData().getRepository(); - return new MetricsRepositoryMethodInvocationListener(registry, tagsProvider, properties.getMetricName(), - properties.getAutotime()); + return new MetricsRepositoryMethodInvocationListener(registry::getObject, tagsProvider, + properties.getMetricName(), properties.getAutotime()); } @Bean diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfigurationIntegrationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfigurationIntegrationTests.java index 85605838716..53c110fdeec 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfigurationIntegrationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfigurationIntegrationTests.java @@ -16,7 +16,9 @@ package org.springframework.boot.actuate.autoconfigure.metrics.data; +import io.micrometer.core.instrument.Gauge; import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.binder.MeterBinder; import org.junit.jupiter.api.Test; import org.springframework.boot.actuate.autoconfigure.metrics.data.city.CityRepository; @@ -28,6 +30,7 @@ import org.springframework.boot.autoconfigure.data.jpa.JpaRepositoriesAutoConfig import org.springframework.boot.autoconfigure.jdbc.EmbeddedDataSourceConfiguration; import org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaAutoConfiguration; import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import static org.assertj.core.api.Assertions.assertThat; @@ -55,10 +58,26 @@ class RepositoryMetricsAutoConfigurationIntegrationTests { }); } + @Test + void doesNotPreventMeterBindersFromDependingUponSpringDataRepositories() { + this.contextRunner.withUserConfiguration(SpringDataRepositoryMeterBinderConfiguration.class) + .run((context) -> assertThat(context).hasNotFailed()); + } + @Configuration(proxyBeanMethods = false) @AutoConfigurationPackage static class TestConfig { } + @Configuration(proxyBeanMethods = false) + static class SpringDataRepositoryMeterBinderConfiguration { + + @Bean + MeterBinder meterBinder(CityRepository repository) { + return (registry) -> Gauge.builder("city.count", repository::count); + } + + } + } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfigurationTests.java index bb5c75a9495..bf461b030f1 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfigurationTests.java @@ -18,6 +18,7 @@ package org.springframework.boot.actuate.autoconfigure.metrics.data; import java.util.Collection; import java.util.Collections; +import java.util.function.Supplier; import io.micrometer.core.annotation.Timed; import io.micrometer.core.instrument.Meter; @@ -28,6 +29,7 @@ import io.micrometer.core.instrument.binder.MeterBinder; import io.micrometer.core.instrument.distribution.HistogramSnapshot; import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.ObjectFactory; import org.springframework.boot.actuate.autoconfigure.metrics.test.MetricsRun; import org.springframework.boot.actuate.metrics.AutoTimer; import org.springframework.boot.actuate.metrics.data.DefaultRepositoryTagsProvider; @@ -180,17 +182,18 @@ class RepositoryMetricsAutoConfigurationTests { static class MetricsRepositoryMethodInvocationListenerConfiguration { @Bean - MetricsRepositoryMethodInvocationListener metricsRepositoryMethodInvocationListener(MeterRegistry registry, - RepositoryTagsProvider tagsProvider) { - return new TestMetricsRepositoryMethodInvocationListener(registry, tagsProvider); + MetricsRepositoryMethodInvocationListener metricsRepositoryMethodInvocationListener( + ObjectFactory registry, RepositoryTagsProvider tagsProvider) { + return new TestMetricsRepositoryMethodInvocationListener(registry::getObject, tagsProvider); } } static class TestMetricsRepositoryMethodInvocationListener extends MetricsRepositoryMethodInvocationListener { - TestMetricsRepositoryMethodInvocationListener(MeterRegistry registry, RepositoryTagsProvider tagsProvider) { - super(registry, tagsProvider, "test", AutoTimer.DISABLED); + TestMetricsRepositoryMethodInvocationListener(Supplier registrySupplier, + RepositoryTagsProvider tagsProvider) { + super(registrySupplier, tagsProvider, "test", AutoTimer.DISABLED); } } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/data/MetricsRepositoryMethodInvocationListener.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/data/MetricsRepositoryMethodInvocationListener.java index 328c2c9f6f8..add03dc6917 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/data/MetricsRepositoryMethodInvocationListener.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/data/MetricsRepositoryMethodInvocationListener.java @@ -18,6 +18,7 @@ package org.springframework.boot.actuate.metrics.data; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; import io.micrometer.core.annotation.Timed; import io.micrometer.core.instrument.MeterRegistry; @@ -26,6 +27,7 @@ import io.micrometer.core.instrument.Tag; import org.springframework.boot.actuate.metrics.AutoTimer; import org.springframework.boot.actuate.metrics.annotation.TimedAnnotations; import org.springframework.data.repository.core.support.RepositoryMethodInvocationListener; +import org.springframework.util.function.SingletonSupplier; /** * Intercepts Spring Data {@code Repository} invocations and records metrics about @@ -36,7 +38,7 @@ import org.springframework.data.repository.core.support.RepositoryMethodInvocati */ public class MetricsRepositoryMethodInvocationListener implements RepositoryMethodInvocationListener { - private final MeterRegistry registry; + private final SingletonSupplier registrySupplier; private final RepositoryTagsProvider tagsProvider; @@ -50,10 +52,27 @@ public class MetricsRepositoryMethodInvocationListener implements RepositoryMeth * @param tagsProvider provider for metrics tags * @param metricName name of the metric to record * @param autoTimer the auto-timers to apply or {@code null} to disable auto-timing + * @deprecated since 2.5.4 for removal in 2.7.0 in favor of + * {@link #MetricsRepositoryMethodInvocationListener(Supplier, RepositoryTagsProvider, String, AutoTimer)} */ + @Deprecated public MetricsRepositoryMethodInvocationListener(MeterRegistry registry, RepositoryTagsProvider tagsProvider, String metricName, AutoTimer autoTimer) { - this.registry = registry; + this(SingletonSupplier.of(registry), tagsProvider, metricName, autoTimer); + } + + /** + * Create a new {@code MetricsRepositoryMethodInvocationListener}. + * @param registrySupplier a supplier for the registry to which metrics are recorded + * @param tagsProvider provider for metrics tags + * @param metricName name of the metric to record + * @param autoTimer the auto-timers to apply or {@code null} to disable auto-timing + * @since 2.5.4 + */ + public MetricsRepositoryMethodInvocationListener(Supplier registrySupplier, + RepositoryTagsProvider tagsProvider, String metricName, AutoTimer autoTimer) { + this.registrySupplier = (registrySupplier instanceof SingletonSupplier) + ? (SingletonSupplier) registrySupplier : SingletonSupplier.of(registrySupplier); this.tagsProvider = tagsProvider; this.metricName = metricName; this.autoTimer = (autoTimer != null) ? autoTimer : AutoTimer.DISABLED; @@ -64,8 +83,8 @@ public class MetricsRepositoryMethodInvocationListener implements RepositoryMeth Set annotations = TimedAnnotations.get(invocation.getMethod(), invocation.getRepositoryInterface()); Iterable tags = this.tagsProvider.repositoryTags(invocation); long duration = invocation.getDuration(TimeUnit.NANOSECONDS); - AutoTimer.apply(this.autoTimer, this.metricName, annotations, - (builder) -> builder.tags(tags).register(this.registry).record(duration, TimeUnit.NANOSECONDS)); + AutoTimer.apply(this.autoTimer, this.metricName, annotations, (builder) -> builder.tags(tags) + .register(this.registrySupplier.get()).record(duration, TimeUnit.NANOSECONDS)); } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/data/MetricsRepositoryMethodInvocationListenerTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/data/MetricsRepositoryMethodInvocationListenerTests.java index 15b766db7c0..375b1aaabf1 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/data/MetricsRepositoryMethodInvocationListenerTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/data/MetricsRepositoryMethodInvocationListenerTests.java @@ -53,13 +53,13 @@ class MetricsRepositoryMethodInvocationListenerTests { void setup() { MockClock clock = new MockClock(); this.registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, clock); - this.listener = new MetricsRepositoryMethodInvocationListener(this.registry, + this.listener = new MetricsRepositoryMethodInvocationListener(() -> this.registry, new DefaultRepositoryTagsProvider(), REQUEST_METRICS_NAME, AutoTimer.ENABLED); } @Test void afterInvocationWhenNoTimerAnnotationsAndNoAutoTimerDoesNothing() { - this.listener = new MetricsRepositoryMethodInvocationListener(this.registry, + this.listener = new MetricsRepositoryMethodInvocationListener(() -> this.registry, new DefaultRepositoryTagsProvider(), REQUEST_METRICS_NAME, null); this.listener.afterInvocation(createInvocation(NoAnnotationsRepository.class)); assertThat(this.registry.find(REQUEST_METRICS_NAME).timers()).isEmpty();