From e17769fc2fb0376121043522438470eac18119c1 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 1 Sep 2014 10:40:16 -0700 Subject: [PATCH] Polish DataSourceMetrics code --- .../DataSourceMetricsAutoConfiguration.java | 4 +- .../endpoint/DataSourcePublicMetrics.java | 84 +++++++------------ .../jdbc/AbstractDataSourceMetadata.java | 23 +++-- .../jdbc/CommonsDbcpDataSourceMetadata.java | 4 +- .../CompositeDataSourceMetadataProvider.java | 29 +++---- .../jdbc/DataSourceMetadata.java | 7 +- .../jdbc/HikariDataSourceMetadata.java | 68 +++------------ .../jdbc/TomcatDataSourceMetadata.java | 3 +- .../jdbc/AbstractDataSourceMetadataTests.java | 4 +- .../CommonsDbcpDataSourceMetadataTests.java | 2 + ...positeDataSourceMetadataProviderTests.java | 2 + .../jdbc/HikariDataSourceMetadataTests.java | 2 + .../jdbc/TomcatDataSourceMetadataTests.java | 2 + 13 files changed, 89 insertions(+), 145 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/DataSourceMetricsAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/DataSourceMetricsAutoConfiguration.java index 3489150b56..52b129ff1f 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/DataSourceMetricsAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/DataSourceMetricsAutoConfiguration.java @@ -43,8 +43,8 @@ public class DataSourceMetricsAutoConfiguration { @Bean @ConditionalOnBean(DataSourceMetadataProvider.class) - @ConditionalOnMissingBean(DataSourcePublicMetrics.class) - DataSourcePublicMetrics dataSourcePublicMetrics() { + @ConditionalOnMissingBean + public DataSourcePublicMetrics dataSourcePublicMetrics() { return new DataSourcePublicMetrics(); } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/DataSourcePublicMetrics.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/DataSourcePublicMetrics.java index f386836427..d7de1b25fc 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/DataSourcePublicMetrics.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/DataSourcePublicMetrics.java @@ -20,6 +20,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.LinkedHashSet; import java.util.Map; +import java.util.Set; import javax.annotation.PostConstruct; import javax.sql.DataSource; @@ -47,88 +48,63 @@ public class DataSourcePublicMetrics implements PublicMetrics { private ApplicationContext applicationContext; @Autowired - private Collection dataSourceMetadataProviders; + private Collection providers; - private final Map dataSourceMetadataByPrefix = new HashMap(); + private final Map metadataByPrefix = new HashMap(); @PostConstruct public void initialize() { - Map dataSources = this.applicationContext - .getBeansOfType(DataSource.class); DataSource primaryDataSource = getPrimaryDataSource(); - DataSourceMetadataProvider provider = new CompositeDataSourceMetadataProvider( - this.dataSourceMetadataProviders); - for (Map.Entry entry : dataSources.entrySet()) { - String prefix = createPrefix(entry.getKey(), entry.getValue(), entry - .getValue().equals(primaryDataSource)); - DataSourceMetadata dataSourceMetadata = provider.getDataSourceMetadata(entry - .getValue()); + this.providers); + for (Map.Entry entry : this.applicationContext + .getBeansOfType(DataSource.class).entrySet()) { + String beanName = entry.getKey(); + DataSource bean = entry.getValue(); + String prefix = createPrefix(beanName, bean, bean.equals(primaryDataSource)); + DataSourceMetadata dataSourceMetadata = provider.getDataSourceMetadata(bean); if (dataSourceMetadata != null) { - this.dataSourceMetadataByPrefix.put(prefix, dataSourceMetadata); + this.metadataByPrefix.put(prefix, dataSourceMetadata); } } } @Override public Collection> metrics() { - Collection> result = new LinkedHashSet>(); - for (Map.Entry entry : this.dataSourceMetadataByPrefix + Set> metrics = new LinkedHashSet>(); + for (Map.Entry entry : this.metadataByPrefix .entrySet()) { String prefix = entry.getKey(); - // Make sure the prefix ends with a dot - if (!prefix.endsWith(".")) { - prefix = prefix + "."; - } + prefix = (prefix.endsWith(".") ? prefix : prefix + "."); DataSourceMetadata dataSourceMetadata = entry.getValue(); - Integer poolSize = dataSourceMetadata.getPoolSize(); - if (poolSize != null) { - result.add(new Metric(prefix + "active", poolSize)); - } - Float poolUsage = dataSourceMetadata.getPoolUsage(); - if (poolUsage != null) { - result.add(new Metric(prefix + "usage", poolUsage)); - } + addMetric(metrics, prefix + "active", dataSourceMetadata.getPoolSize()); + addMetric(metrics, prefix + "usage", dataSourceMetadata.getPoolUsage()); + } + return metrics; + } + + private void addMetric(Set> metrics, String name, T value) { + if (value != null) { + metrics.add(new Metric(name, value)); } - return result; } /** * Create the prefix to use for the metrics to associate with the given * {@link DataSource}. - * @param dataSourceName the name of the data source bean + * @param name the name of the data source bean * @param dataSource the data source to configure * @param primary if this data source is the primary data source * @return a prefix for the given data source */ - protected String createPrefix(String dataSourceName, DataSource dataSource, - boolean primary) { - StringBuilder sb = new StringBuilder("datasource."); + protected String createPrefix(String name, DataSource dataSource, boolean primary) { if (primary) { - sb.append("primary"); + return "datasource.primary"; } - else if (endWithDataSource(dataSourceName)) { // Strip the data source part out of - // the name - sb.append(dataSourceName.substring(0, dataSourceName.length() - - DATASOURCE_SUFFIX.length())); + if (name.toLowerCase().endsWith(DATASOURCE_SUFFIX.toLowerCase())) { + name = name.substring(0, name.length() - DATASOURCE_SUFFIX.length()); } - else { - sb.append(dataSourceName); - } - return sb.toString(); - } - - /** - * Specify if the given value ends with {@code dataSource}. - */ - protected boolean endWithDataSource(String value) { - int suffixLength = DATASOURCE_SUFFIX.length(); - int valueLength = value.length(); - if (valueLength > suffixLength) { - String suffix = value.substring(valueLength - suffixLength, valueLength); - return suffix.equalsIgnoreCase(DATASOURCE_SUFFIX); - } - return false; + return "datasource." + name; } /** @@ -140,7 +116,7 @@ public class DataSourcePublicMetrics implements PublicMetrics { try { return this.applicationContext.getBean(DataSource.class); } - catch (NoSuchBeanDefinitionException e) { + catch (NoSuchBeanDefinitionException ex) { return null; } } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/AbstractDataSourceMetadata.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/AbstractDataSourceMetadata.java index 99308f0a21..9986507b58 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/AbstractDataSourceMetadata.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/AbstractDataSourceMetadata.java @@ -24,38 +24,35 @@ import javax.sql.DataSource; * @author Stephane Nicoll * @since 1.2.0 */ -public abstract class AbstractDataSourceMetadata implements +public abstract class AbstractDataSourceMetadata implements DataSourceMetadata { - private final D dataSource; + private final T dataSource; /** * Create an instance with the data source to use. */ - protected AbstractDataSourceMetadata(D dataSource) { + protected AbstractDataSourceMetadata(T dataSource) { this.dataSource = dataSource; } @Override public Float getPoolUsage() { - Integer max = getMaxPoolSize(); - if (max == null) { + Integer maxSize = getMaxPoolSize(); + Integer currentSize = getPoolSize(); + if (maxSize == null || currentSize == null) { return null; } - if (max < 0) { + if (maxSize < 0) { return -1F; } - Integer current = getPoolSize(); - if (current == null) { - return null; - } - if (current == 0) { + if (currentSize == 0) { return 0F; } - return (float) current / max; // something like that + return (float) currentSize / (float) maxSize; } - protected final D getDataSource() { + protected final T getDataSource() { return this.dataSource; } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/CommonsDbcpDataSourceMetadata.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/CommonsDbcpDataSourceMetadata.java index 5042debea5..4089c9bcaa 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/CommonsDbcpDataSourceMetadata.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/CommonsDbcpDataSourceMetadata.java @@ -16,10 +16,12 @@ package org.springframework.boot.autoconfigure.jdbc; +import javax.sql.DataSource; + import org.apache.commons.dbcp.BasicDataSource; /** - * A {@link DataSourceMetadata} implementation for the commons dbcp data source. + * {@link DataSourceMetadata} for a Apache Commons DBCP {@link DataSource}. * * @author Stephane Nicoll * @since 1.2.0 diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/CompositeDataSourceMetadataProvider.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/CompositeDataSourceMetadataProvider.java index 187e6357c9..1a794f793b 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/CompositeDataSourceMetadataProvider.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/CompositeDataSourceMetadataProvider.java @@ -18,6 +18,7 @@ package org.springframework.boot.autoconfigure.jdbc; import java.util.ArrayList; import java.util.Collection; +import java.util.List; import javax.sql.DataSource; @@ -30,30 +31,30 @@ import javax.sql.DataSource; */ public class CompositeDataSourceMetadataProvider implements DataSourceMetadataProvider { - private final Collection providers; + private final List providers; /** - * Create an instance with an initial collection of delegates to use. - */ - public CompositeDataSourceMetadataProvider( - Collection providers) { - this.providers = providers; - } - - /** - * Create an instance with no delegate. + * Create a {@link CompositeDataSourceMetadataProvider} instance with no delegate. */ public CompositeDataSourceMetadataProvider() { this(new ArrayList()); } + /** + * Create a {@link CompositeDataSourceMetadataProvider} instance with an initial + * collection of delegates to use. + */ + public CompositeDataSourceMetadataProvider( + Collection providers) { + this.providers = new ArrayList(providers); + } + @Override public DataSourceMetadata getDataSourceMetadata(DataSource dataSource) { for (DataSourceMetadataProvider provider : this.providers) { - DataSourceMetadata dataSourceMetadata = provider - .getDataSourceMetadata(dataSource); - if (dataSourceMetadata != null) { - return dataSourceMetadata; + DataSourceMetadata metadata = provider.getDataSourceMetadata(dataSource); + if (metadata != null) { + return metadata; } } return null; diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceMetadata.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceMetadata.java index b42e4b3767..0ed3ca5914 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceMetadata.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceMetadata.java @@ -19,8 +19,8 @@ package org.springframework.boot.autoconfigure.jdbc; import javax.sql.DataSource; /** - * Provide various metadata regarding a {@link DataSource} that are shared by most data - * source types but not accessible in a standard manner. + * Provides access meta-data that is commonly available from most {@link DataSource} + * implementations. * * @author Stephane Nicoll * @since 1.2.0 @@ -28,7 +28,8 @@ import javax.sql.DataSource; public interface DataSourceMetadata { /** - * Return the usage of the pool as a double value between 0 and 1. + * Return the usage of the pool as value between 0 and 1 (or -1 if the pool is not + * limited). *
    *
  • 1 means that the maximum number of connections have been allocated
    • *
  • 0 means that no connection is currently active
    • diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceMetadata.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceMetadata.java index 02e69d56a6..140a45cbab 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceMetadata.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceMetadata.java @@ -16,14 +16,15 @@ package org.springframework.boot.autoconfigure.jdbc; -import org.springframework.beans.BeansException; +import javax.sql.DataSource; + import org.springframework.beans.DirectFieldAccessor; import com.zaxxer.hikari.HikariDataSource; import com.zaxxer.hikari.pool.HikariPool; /** - * A {@link DataSourceMetadata} implementation for the hikari data source. + * {@link DataSourceMetadata} for a Hikari {@link DataSource}. * * @author Stephane Nicoll * @since 1.2.0 @@ -31,20 +32,23 @@ import com.zaxxer.hikari.pool.HikariPool; public class HikariDataSourceMetadata extends AbstractDataSourceMetadata { - private final HikariPoolProvider hikariPoolProvider; - public HikariDataSourceMetadata(HikariDataSource dataSource) { super(dataSource); - this.hikariPoolProvider = new HikariPoolProvider(dataSource); } @Override public Integer getPoolSize() { - HikariPool hikariPool = this.hikariPoolProvider.getHikariPool(); - if (hikariPool != null) { - return hikariPool.getActiveConnections(); + try { + return getHikariPool().getActiveConnections(); } - return null; + catch (Exception ex) { + return null; + } + } + + private HikariPool getHikariPool() { + return (HikariPool) new DirectFieldAccessor(getDataSource()) + .getPropertyValue("pool"); } @Override @@ -62,50 +66,4 @@ public class HikariDataSourceMetadata extends return getDataSource().getConnectionTestQuery(); } - /** - * Provide the {@link HikariPool} instance managed internally by the - * {@link HikariDataSource} as there is no other way to retrieve that information - * except JMX access. - */ - private static class HikariPoolProvider { - private final HikariDataSource dataSource; - - private boolean poolAvailable; - - private HikariPoolProvider(HikariDataSource dataSource) { - this.dataSource = dataSource; - this.poolAvailable = isHikariPoolAvailable(); - } - - public HikariPool getHikariPool() { - if (!this.poolAvailable) { - return null; - } - - Object value = doGetValue(); - if (value instanceof HikariPool) { - return (HikariPool) value; - } - return null; - } - - private boolean isHikariPoolAvailable() { - try { - doGetValue(); - return true; - } - catch (BeansException e) { // No such field - return false; - } - catch (SecurityException e) { // Security manager prevents to read the value - return false; - } - } - - private Object doGetValue() { - DirectFieldAccessor accessor = new DirectFieldAccessor(this.dataSource); - return accessor.getPropertyValue("pool"); - } - } - } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/TomcatDataSourceMetadata.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/TomcatDataSourceMetadata.java index fd15d1cb4a..20a178cece 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/TomcatDataSourceMetadata.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/TomcatDataSourceMetadata.java @@ -20,8 +20,7 @@ import org.apache.tomcat.jdbc.pool.ConnectionPool; import org.apache.tomcat.jdbc.pool.DataSource; /** - * - * A {@link DataSourceMetadata} implementation for the tomcat data source. + * {@link DataSourceMetadata} for a Tomcat {@link DataSource}. * * @author Stephane Nicoll */ diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/AbstractDataSourceMetadataTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/AbstractDataSourceMetadataTests.java index 71d45bdbe6..41ba22d879 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/AbstractDataSourceMetadataTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/AbstractDataSourceMetadataTests.java @@ -27,9 +27,11 @@ import org.springframework.jdbc.core.JdbcTemplate; import static org.junit.Assert.assertEquals; /** + * Abstract base class for {@link DataSourceMetadata} tests. + * * @author Stephane Nicoll */ -public abstract class AbstractDataSourceMetadataTests { +public abstract class AbstractDataSourceMetadataTests> { /** * Return a data source metadata instance with a min size of 0 and max size of 2. diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/CommonsDbcpDataSourceMetadataTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/CommonsDbcpDataSourceMetadataTests.java index 6c0232a832..ee8bad75ba 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/CommonsDbcpDataSourceMetadataTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/CommonsDbcpDataSourceMetadataTests.java @@ -24,6 +24,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; /** + * Tests for {@link CommonsDbcpDataSourceMetadata}. + * * @author Stephane Nicoll */ public class CommonsDbcpDataSourceMetadataTests extends diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/CompositeDataSourceMetadataProviderTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/CompositeDataSourceMetadataProviderTests.java index 6f064b4b83..4b02a0f390 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/CompositeDataSourceMetadataProviderTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/CompositeDataSourceMetadataProviderTests.java @@ -30,6 +30,8 @@ import static org.junit.Assert.assertSame; import static org.mockito.BDDMockito.given; /** + * Tests for {@link CompositeDataSourceMetadataProvider}. + * * @author Stephane Nicoll */ public class CompositeDataSourceMetadataProviderTests { diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceMetadataTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceMetadataTests.java index 3d6c79a4af..bac06bd32b 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceMetadataTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/HikariDataSourceMetadataTests.java @@ -23,6 +23,8 @@ import com.zaxxer.hikari.HikariDataSource; import static org.junit.Assert.assertEquals; /** + * Tests for {@link HikariDataSourceMetadata}. + * * @author Stephane Nicoll */ public class HikariDataSourceMetadataTests extends diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/TomcatDataSourceMetadataTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/TomcatDataSourceMetadataTests.java index 2b3388ec3e..6b0f8e2b0c 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/TomcatDataSourceMetadataTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/TomcatDataSourceMetadataTests.java @@ -22,6 +22,8 @@ import org.junit.Before; import static org.junit.Assert.assertEquals; /** + * Tests for {@link TomcatDataSourceMetadata}. + * * @author Stephane Nicoll */ public class TomcatDataSourceMetadataTests extends