diff --git a/src/main/java/com/zaxxer/hikari/HikariConfig.java b/src/main/java/com/zaxxer/hikari/HikariConfig.java index 305ad6d5..f2bea8d1 100644 --- a/src/main/java/com/zaxxer/hikari/HikariConfig.java +++ b/src/main/java/com/zaxxer/hikari/HikariConfig.java @@ -40,10 +40,9 @@ import org.slf4j.LoggerFactory; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.health.HealthCheckRegistry; +import com.zaxxer.hikari.metrics.MetricsTrackerFactory; import com.zaxxer.hikari.pool.PoolElf; import com.zaxxer.hikari.util.PropertyElf; -import com.zaxxer.hikari.metrics.CodahaleMetricsTrackerFactory; -import com.zaxxer.hikari.metrics.MetricsTrackerFactory; public class HikariConfig implements HikariConfigMXBean { @@ -91,6 +90,7 @@ public class HikariConfig implements HikariConfigMXBean private ThreadFactory threadFactory; private ScheduledThreadPoolExecutor scheduledExecutor; private MetricsTrackerFactory metricsTrackerFactory; + private Object metricRegistry; private Object healthCheckRegistry; private Properties healthCheckProperties; @@ -451,6 +451,10 @@ public class HikariConfig implements HikariConfigMXBean public void setMetricsTrackerFactory(MetricsTrackerFactory metricsTrackerFactory) { + if (metricRegistry != null) { + throw new IllegalStateException("setMetricsTrackerFactory() cannot used in combination with setMetricRegistry()"); + } + this.metricsTrackerFactory = metricsTrackerFactory; } @@ -461,12 +465,7 @@ public class HikariConfig implements HikariConfigMXBean */ public Object getMetricRegistry() { - if (metricsTrackerFactory instanceof CodahaleMetricsTrackerFactory) { - return ((CodahaleMetricsTrackerFactory) metricsTrackerFactory).getRegistry(); - } - else { - return null; - } + return metricRegistry; } /** @@ -476,7 +475,10 @@ public class HikariConfig implements HikariConfigMXBean */ public void setMetricRegistry(Object metricRegistry) { - MetricsTrackerFactory metricsTrackerFactory; + if (metricsTrackerFactory != null) { + throw new IllegalStateException("setMetricRegistry() cannot used in combination with setMetricsTrackerFactory()"); + } + if (metricRegistry != null) { if (metricRegistry instanceof String) { try { @@ -491,13 +493,9 @@ public class HikariConfig implements HikariConfigMXBean if (!(metricRegistry instanceof MetricRegistry)) { throw new IllegalArgumentException("Class must be an instance of com.codahale.metrics.MetricRegistry"); } - metricsTrackerFactory = new CodahaleMetricsTrackerFactory((MetricRegistry) metricRegistry); } - else { - metricsTrackerFactory = null; - } - // use setter to allow HikariDataSource to alter the behavior of both setters in a single override - setMetricsTrackerFactory(metricsTrackerFactory); + + this.metricRegistry = metricRegistry; } /** diff --git a/src/main/java/com/zaxxer/hikari/HikariDataSource.java b/src/main/java/com/zaxxer/hikari/HikariDataSource.java index b5f056a7..0c5c112e 100644 --- a/src/main/java/com/zaxxer/hikari/HikariDataSource.java +++ b/src/main/java/com/zaxxer/hikari/HikariDataSource.java @@ -191,6 +191,23 @@ public class HikariDataSource extends HikariConfig implements DataSource, Closea return false; } + /** {@inheritDoc} */ + @Override + public void setMetricRegistry(Object metricRegistry) + { + boolean isAlreadySet = getMetricRegistry() != null; + super.setMetricRegistry(metricRegistry); + + if (pool != null) { + if (isAlreadySet) { + throw new IllegalStateException("MetricRegistry can only be set one time"); + } + else { + pool.setMetricRegistry(super.getMetricRegistry()); + } + } + } + /** {@inheritDoc} */ @Override public void setMetricsTrackerFactory(MetricsTrackerFactory metricsTrackerFactory) diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index c35ad2c1..d3ba46ef 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -44,12 +44,11 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.health.HealthCheckRegistry; import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.HikariPoolMXBean; -import com.zaxxer.hikari.metrics.CodaHaleMetricsTracker; import com.zaxxer.hikari.metrics.CodahaleHealthChecker; import com.zaxxer.hikari.metrics.CodahaleMetricsTrackerFactory; import com.zaxxer.hikari.metrics.MetricsTracker; -import com.zaxxer.hikari.metrics.MetricsTrackerFactory; import com.zaxxer.hikari.metrics.MetricsTracker.MetricsContext; +import com.zaxxer.hikari.metrics.MetricsTrackerFactory; import com.zaxxer.hikari.metrics.PoolStats; import com.zaxxer.hikari.proxy.ConnectionProxy; import com.zaxxer.hikari.proxy.IHikariConnectionProxy; @@ -135,7 +134,13 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener this.leakTask = new LeakTask(config.getLeakDetectionThreshold(), houseKeepingExecutorService); - setMetricRegistry(config.getMetricRegistry()); + if (config.getMetricsTrackerFactory() != null) { + setMetricsTrackerFactory(config.getMetricsTrackerFactory()); + } + else { + setMetricRegistry(config.getMetricRegistry()); + } + setHealthCheckRegistry(config.getHealthCheckRegistry()); poolElf.registerMBeans(this); @@ -287,7 +292,7 @@ public class HikariPool implements HikariPoolMXBean, IBagStateListener public void setMetricRegistry(Object metricRegistry) { - if (metricRegistry != null) { + if (isRecordMetrics) { setMetricsTrackerFactory(new CodahaleMetricsTrackerFactory((MetricRegistry) metricRegistry)); } else { diff --git a/src/test/java/com/zaxxer/hikari/TestMetrics.java b/src/test/java/com/zaxxer/hikari/TestMetrics.java index 58380d06..a004b0f8 100644 --- a/src/test/java/com/zaxxer/hikari/TestMetrics.java +++ b/src/test/java/com/zaxxer/hikari/TestMetrics.java @@ -266,11 +266,11 @@ public class TestMetrics ds.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource"); MetricRegistry metricRegistry = new MetricRegistry(); - MetricsTrackerFactory metricsTrackerFactory = new CodahaleMetricsTrackerFactory(metricRegistry); // before the pool is started, we can set it any number of times using either setter ds.setMetricRegistry(metricRegistry); - ds.setMetricsTrackerFactory(metricsTrackerFactory); + ds.setMetricRegistry(metricRegistry); + ds.setMetricRegistry(metricRegistry); try { Connection connection = ds.getConnection(); @@ -282,14 +282,37 @@ public class TestMetrics } catch (IllegalStateException ise) { // pass - try { - // after the pool is started, we cannot set it any more - ds.setMetricsTrackerFactory(metricsTrackerFactory); - Assert.fail("Should not have been allowed to set metricsTrackerFactory after pool started"); - } - catch (IllegalStateException ise2) { - // pass - } + } + finally { + ds.close(); + } + } + + @Test + public void testSetters5() throws Exception + { + HikariDataSource ds = new HikariDataSource(); + ds.setMaximumPoolSize(1); + ds.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource"); + + MetricRegistry metricRegistry = new MetricRegistry(); + MetricsTrackerFactory metricsTrackerFactory = new CodahaleMetricsTrackerFactory(metricRegistry); + + // before the pool is started, we can set it any number of times using either setter + ds.setMetricsTrackerFactory(metricsTrackerFactory); + ds.setMetricsTrackerFactory(metricsTrackerFactory); + ds.setMetricsTrackerFactory(metricsTrackerFactory); + + try { + Connection connection = ds.getConnection(); + connection.close(); + + // after the pool is started, we cannot set it any more + ds.setMetricsTrackerFactory(metricsTrackerFactory); + Assert.fail("Should not have been allowed to set registry factory after pool started"); + } + catch (IllegalStateException ise) { + // pass } finally { ds.close();