From 68cd27c47b9bd866651605a73406e36cfdfd45ab Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 1 Jun 2018 17:55:43 +0100 Subject: [PATCH] Do not require DataSourcePoolMetrics to auto-configure Hikari meters Closes gh-13330 --- ...ataSourcePoolMetricsAutoConfiguration.java | 64 ++++++++------- ...urcePoolMetricsAutoConfigurationTests.java | 80 +++++++++++++++---- 2 files changed, 101 insertions(+), 43 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/jdbc/DataSourcePoolMetricsAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/jdbc/DataSourcePoolMetricsAutoConfiguration.java index e7cb8e955a..fb743cace8 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/jdbc/DataSourcePoolMetricsAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/jdbc/DataSourcePoolMetricsAutoConfiguration.java @@ -50,44 +50,50 @@ import org.springframework.util.StringUtils; @Configuration @AutoConfigureAfter({ MetricsAutoConfiguration.class, DataSourceAutoConfiguration.class, SimpleMetricsExportAutoConfiguration.class }) -@ConditionalOnBean({ DataSource.class, DataSourcePoolMetadataProvider.class, - MeterRegistry.class }) +@ConditionalOnBean({ DataSource.class, MeterRegistry.class }) public class DataSourcePoolMetricsAutoConfiguration { - private static final String DATASOURCE_SUFFIX = "dataSource"; + @Configuration + @ConditionalOnBean(DataSourcePoolMetadataProvider.class) + static class DataSourcePoolMetadataMetricsConfiguration { - private final MeterRegistry registry; + private static final String DATASOURCE_SUFFIX = "dataSource"; - private final Collection metadataProviders; + private final MeterRegistry registry; - public DataSourcePoolMetricsAutoConfiguration(MeterRegistry registry, - Collection metadataProviders) { - this.registry = registry; - this.metadataProviders = metadataProviders; - } + private final Collection metadataProviders; - @Autowired - public void bindDataSourcesToRegistry(Map dataSources) { - dataSources.forEach(this::bindDataSourceToRegistry); - } + DataSourcePoolMetadataMetricsConfiguration(MeterRegistry registry, + Collection metadataProviders) { + this.registry = registry; + this.metadataProviders = metadataProviders; + } - private void bindDataSourceToRegistry(String beanName, DataSource dataSource) { - String dataSourceName = getDataSourceName(beanName); - new DataSourcePoolMetrics(dataSource, this.metadataProviders, dataSourceName, - Collections.emptyList()).bindTo(this.registry); - } + @Autowired + public void bindDataSourcesToRegistry(Map dataSources) { + dataSources.forEach(this::bindDataSourceToRegistry); + } + + private void bindDataSourceToRegistry(String beanName, DataSource dataSource) { + String dataSourceName = getDataSourceName(beanName); + new DataSourcePoolMetrics(dataSource, this.metadataProviders, dataSourceName, + Collections.emptyList()).bindTo(this.registry); + } - /** - * Get the name of a DataSource based on its {@code beanName}. - * @param beanName the name of the data source bean - * @return a name for the given data source - */ - private String getDataSourceName(String beanName) { - if (beanName.length() > DATASOURCE_SUFFIX.length() - && StringUtils.endsWithIgnoreCase(beanName, DATASOURCE_SUFFIX)) { - return beanName.substring(0, beanName.length() - DATASOURCE_SUFFIX.length()); + /** + * Get the name of a DataSource based on its {@code beanName}. + * @param beanName the name of the data source bean + * @return a name for the given data source + */ + private String getDataSourceName(String beanName) { + if (beanName.length() > DATASOURCE_SUFFIX.length() + && StringUtils.endsWithIgnoreCase(beanName, DATASOURCE_SUFFIX)) { + return beanName.substring(0, + beanName.length() - DATASOURCE_SUFFIX.length()); + } + return beanName; } - return beanName; + } @Configuration diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/jdbc/DataSourcePoolMetricsAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/jdbc/DataSourcePoolMetricsAutoConfigurationTests.java index dc4797f9f7..3644314fc1 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/jdbc/DataSourcePoolMetricsAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/jdbc/DataSourcePoolMetricsAutoConfigurationTests.java @@ -33,6 +33,7 @@ import org.springframework.boot.actuate.autoconfigure.metrics.test.MetricsRun; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; import org.springframework.boot.jdbc.DataSourceBuilder; +import org.springframework.boot.jdbc.metadata.DataSourcePoolMetadataProvider; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -53,22 +54,29 @@ public class DataSourcePoolMetricsAutoConfigurationTests { private ApplicationContextRunner contextRunner = new ApplicationContextRunner() .withPropertyValues("spring.datasource.generate-unique-name=true") .with(MetricsRun.simple()) - .withConfiguration(AutoConfigurations.of(DataSourceAutoConfiguration.class, - DataSourcePoolMetricsAutoConfiguration.class)) + .withConfiguration( + AutoConfigurations.of(DataSourcePoolMetricsAutoConfiguration.class)) .withUserConfiguration(BaseConfiguration.class); @Test public void autoConfiguredDataSourceIsInstrumented() { - this.contextRunner.run((context) -> { - context.getBean(DataSource.class).getConnection().getMetaData(); - MeterRegistry registry = context.getBean(MeterRegistry.class); - registry.get("jdbc.connections.max").tags("name", "dataSource").meter(); - }); + this.contextRunner + .withConfiguration( + AutoConfigurations.of(DataSourceAutoConfiguration.class)) + .run((context) -> { + context.getBean(DataSource.class).getConnection().getMetaData(); + MeterRegistry registry = context.getBean(MeterRegistry.class); + registry.get("jdbc.connections.max").tags("name", "dataSource") + .meter(); + }); } @Test public void dataSourceInstrumentationCanBeDisabled() { - this.contextRunner.withPropertyValues("management.metrics.enable.jdbc=false") + this.contextRunner + .withConfiguration( + AutoConfigurations.of(DataSourceAutoConfiguration.class)) + .withPropertyValues("management.metrics.enable.jdbc=false") .run((context) -> { context.getBean(DataSource.class).getConnection().getMetaData(); MeterRegistry registry = context.getBean(MeterRegistry.class); @@ -79,7 +87,10 @@ public class DataSourcePoolMetricsAutoConfigurationTests { @Test public void allDataSourcesCanBeInstrumented() { - this.contextRunner.withUserConfiguration(TwoDataSourcesConfiguration.class) + this.contextRunner + .withConfiguration( + AutoConfigurations.of(DataSourceAutoConfiguration.class)) + .withUserConfiguration(TwoDataSourcesConfiguration.class) .run((context) -> { context.getBean("firstDataSource", DataSource.class).getConnection() .getMetaData(); @@ -94,11 +105,14 @@ public class DataSourcePoolMetricsAutoConfigurationTests { @Test public void autoConfiguredHikariDataSourceIsInstrumented() { - this.contextRunner.run((context) -> { - context.getBean(DataSource.class).getConnection(); - MeterRegistry registry = context.getBean(MeterRegistry.class); - registry.get("hikaricp.connections").meter(); - }); + this.contextRunner + .withConfiguration( + AutoConfigurations.of(DataSourceAutoConfiguration.class)) + .run((context) -> { + context.getBean(DataSource.class).getConnection(); + MeterRegistry registry = context.getBean(MeterRegistry.class); + registry.get("hikaricp.connections").meter(); + }); } @Test @@ -106,6 +120,8 @@ public class DataSourcePoolMetricsAutoConfigurationTests { this.contextRunner .withPropertyValues( "spring.datasource.schema:db/create-custom-schema.sql") + .withConfiguration( + AutoConfigurations.of(DataSourceAutoConfiguration.class)) .run((context) -> { context.getBean(DataSource.class).getConnection(); MeterRegistry registry = context.getBean(MeterRegistry.class); @@ -116,6 +132,8 @@ public class DataSourcePoolMetricsAutoConfigurationTests { @Test public void hikariCanBeInstrumentedAfterThePoolHasBeenSealed() { this.contextRunner.withUserConfiguration(HikariSealingConfiguration.class) + .withConfiguration( + AutoConfigurations.of(DataSourceAutoConfiguration.class)) .run((context) -> { assertThat(context).hasNotFailed(); context.getBean(DataSource.class).getConnection(); @@ -127,6 +145,8 @@ public class DataSourcePoolMetricsAutoConfigurationTests { @Test public void hikariDataSourceInstrumentationCanBeDisabled() { this.contextRunner.withPropertyValues("management.metrics.enable.hikaricp=false") + .withConfiguration( + AutoConfigurations.of(DataSourceAutoConfiguration.class)) .run((context) -> { context.getBean(DataSource.class).getConnection(); MeterRegistry registry = context.getBean(MeterRegistry.class); @@ -137,6 +157,8 @@ public class DataSourcePoolMetricsAutoConfigurationTests { @Test public void allHikariDataSourcesCanBeInstrumented() { this.contextRunner.withUserConfiguration(TwoHikariDataSourcesConfiguration.class) + .withConfiguration( + AutoConfigurations.of(DataSourceAutoConfiguration.class)) .run((context) -> { context.getBean("firstDataSource", DataSource.class).getConnection(); context.getBean("secondOne", DataSource.class).getConnection(); @@ -151,6 +173,8 @@ public class DataSourcePoolMetricsAutoConfigurationTests { @Test public void someHikariDataSourcesCanBeInstrumented() { this.contextRunner.withUserConfiguration(MixedDataSourcesConfiguration.class) + .withConfiguration( + AutoConfigurations.of(DataSourceAutoConfiguration.class)) .run((context) -> { context.getBean("firstDataSource", DataSource.class).getConnection(); context.getBean("secondOne", DataSource.class).getConnection(); @@ -161,6 +185,20 @@ public class DataSourcePoolMetricsAutoConfigurationTests { }); } + @Test + public void hikariDataSourceIsInstrumentedWithoutMetadataProvider() { + this.contextRunner.withUserConfiguration(OneHikariDataSourceConfiguration.class) + .run((context) -> { + assertThat(context) + .doesNotHaveBean(DataSourcePoolMetadataProvider.class); + context.getBean("hikariDataSource", DataSource.class).getConnection(); + MeterRegistry registry = context.getBean(MeterRegistry.class); + assertThat(registry.get("hikaricp.connections").meter().getId() + .getTags()) + .containsExactly(Tag.of("pool", "hikariDataSource")); + }); + } + @Configuration static class BaseConfiguration { @@ -214,6 +252,20 @@ public class DataSourcePoolMetricsAutoConfigurationTests { } + @Configuration + static class OneHikariDataSourceConfiguration { + + @Bean + public DataSource hikariDataSource() { + String url = "jdbc:hsqldb:mem:test-" + UUID.randomUUID(); + HikariDataSource hikariDataSource = DataSourceBuilder.create().url(url) + .type(HikariDataSource.class).build(); + hikariDataSource.setPoolName("hikariDataSource"); + return hikariDataSource; + } + + } + @Configuration static class MixedDataSourcesConfiguration {