From 3a6ec241a395b15b76d530b75d81dbd5f738b21b Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Thu, 2 Jun 2022 15:54:45 +0200 Subject: [PATCH 1/3] Add a more type safe way of registering gauges --- .../metrics/InstrumentedExecutorService.java | 44 ++++++------ .../com/codahale/metrics/MetricRegistry.java | 13 +++- .../codahale/metrics/MetricRegistryTest.java | 18 ++++- .../metrics/ehcache/InstrumentedEhcache.java | 69 +++++++++---------- .../ehcache/InstrumentedEhcacheTest.java | 22 ++++++ .../InstrumentedNClientConnManager.java | 32 ++++----- ...strumentedHttpClientConnectionManager.java | 33 ++++----- ...mentedHttpClientConnectionManagerTest.java | 11 ++- ...trumentedAsyncClientConnectionManager.java | 33 ++++----- ...strumentedHttpClientConnectionManager.java | 32 ++++----- ...entedAsyncClientConnectionManagerTest.java | 11 ++- ...mentedHttpClientConnectionManagerTest.java | 11 ++- .../jetty10/InstrumentedQueuedThreadPool.java | 11 ++- .../jetty11/InstrumentedQueuedThreadPool.java | 11 ++- .../jetty9/InstrumentedQueuedThreadPool.java | 11 ++- .../codahale/metrics/jmx/JmxReporterTest.java | 5 +- 16 files changed, 199 insertions(+), 168 deletions(-) diff --git a/metrics-core/src/main/java/com/codahale/metrics/InstrumentedExecutorService.java b/metrics-core/src/main/java/com/codahale/metrics/InstrumentedExecutorService.java index 0984e9973f..96035150b2 100644 --- a/metrics-core/src/main/java/com/codahale/metrics/InstrumentedExecutorService.java +++ b/metrics-core/src/main/java/com/codahale/metrics/InstrumentedExecutorService.java @@ -58,31 +58,31 @@ public InstrumentedExecutorService(ExecutorService delegate, MetricRegistry regi if (delegate instanceof ThreadPoolExecutor) { ThreadPoolExecutor executor = (ThreadPoolExecutor) delegate; - registry.register(MetricRegistry.name(name, "pool.size"), - (Gauge) executor::getPoolSize); - registry.register(MetricRegistry.name(name, "pool.core"), - (Gauge) executor::getCorePoolSize); - registry.register(MetricRegistry.name(name, "pool.max"), - (Gauge) executor::getMaximumPoolSize); + registry.registerGauge(MetricRegistry.name(name, "pool.size"), + executor::getPoolSize); + registry.registerGauge(MetricRegistry.name(name, "pool.core"), + executor::getCorePoolSize); + registry.registerGauge(MetricRegistry.name(name, "pool.max"), + executor::getMaximumPoolSize); final BlockingQueue queue = executor.getQueue(); - registry.register(MetricRegistry.name(name, "tasks.active"), - (Gauge) executor::getActiveCount); - registry.register(MetricRegistry.name(name, "tasks.completed"), - (Gauge) executor::getCompletedTaskCount); - registry.register(MetricRegistry.name(name, "tasks.queued"), - (Gauge) queue::size); - registry.register(MetricRegistry.name(name, "tasks.capacity"), - (Gauge) queue::remainingCapacity); + registry.registerGauge(MetricRegistry.name(name, "tasks.active"), + executor::getActiveCount); + registry.registerGauge(MetricRegistry.name(name, "tasks.completed"), + executor::getCompletedTaskCount); + registry.registerGauge(MetricRegistry.name(name, "tasks.queued"), + queue::size); + registry.registerGauge(MetricRegistry.name(name, "tasks.capacity"), + queue::remainingCapacity); } else if (delegate instanceof ForkJoinPool) { ForkJoinPool forkJoinPool = (ForkJoinPool) delegate; - registry.register(MetricRegistry.name(name, "tasks.stolen"), - (Gauge) forkJoinPool::getStealCount); - registry.register(MetricRegistry.name(name, "tasks.queued"), - (Gauge) forkJoinPool::getQueuedTaskCount); - registry.register(MetricRegistry.name(name, "threads.active"), - (Gauge) forkJoinPool::getActiveThreadCount); - registry.register(MetricRegistry.name(name, "threads.running"), - (Gauge) forkJoinPool::getRunningThreadCount); + registry.registerGauge(MetricRegistry.name(name, "tasks.stolen"), + forkJoinPool::getStealCount); + registry.registerGauge(MetricRegistry.name(name, "tasks.queued"), + forkJoinPool::getQueuedTaskCount); + registry.registerGauge(MetricRegistry.name(name, "threads.active"), + forkJoinPool::getActiveThreadCount); + registry.registerGauge(MetricRegistry.name(name, "threads.running"), + forkJoinPool::getRunningThreadCount); } } diff --git a/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java b/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java index a5becbb81b..59b4f13b01 100644 --- a/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java +++ b/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java @@ -76,6 +76,17 @@ protected ConcurrentMap buildMap() { return new ConcurrentHashMap<>(); } + /** + * Given a {@link Gauge}, registers it under the given name and returns it + * + * @param name the name of the gauge + * @param the type of the gauge's value + * @return the registered {@link Gauge} + */ + public Gauge registerGauge(String name, Gauge metric) throws IllegalArgumentException { + return register(name, metric); + } + /** * Given a {@link Metric}, registers it under the given name. * @@ -93,7 +104,7 @@ public T register(String name, T metric) throws IllegalArgume } if (metric instanceof MetricRegistry) { - final MetricRegistry childRegistry = (MetricRegistry)metric; + final MetricRegistry childRegistry = (MetricRegistry) metric; final String childName = name; childRegistry.addListener(new MetricRegistryListener() { @Override diff --git a/metrics-core/src/test/java/com/codahale/metrics/MetricRegistryTest.java b/metrics-core/src/test/java/com/codahale/metrics/MetricRegistryTest.java index 726627c767..cbc86acc4b 100644 --- a/metrics-core/src/test/java/com/codahale/metrics/MetricRegistryTest.java +++ b/metrics-core/src/test/java/com/codahale/metrics/MetricRegistryTest.java @@ -615,10 +615,10 @@ public void removingDeepChildMetricsAfterRegister() { assertThat(deepChildMetrics.size()).isEqualTo(1); assertThat(childMetrics.size()).isEqualTo(3); } - + @Test public void registerNullMetric() { - MetricRegistry registry = new MetricRegistry(); + MetricRegistry registry = new MetricRegistry(); try { registry.register("any_name", null); Assert.fail("NullPointerException must be thrown !!!"); @@ -626,4 +626,18 @@ public void registerNullMetric() { Assert.assertEquals("metric == null", e.getMessage()); } } + + @Test + public void infersGaugeType() { + Gauge gauge = registry.registerGauge("gauge", () -> 10_000_000_000L); + + assertThat(gauge.getValue()).isEqualTo(10_000_000_000L); + } + + @Test + public void registersGaugeAsLambda() { + registry.registerGauge("gauge", () -> 3.14); + + assertThat(registry.gauge("gauge").getValue()).isEqualTo(3.14); + } } diff --git a/metrics-ehcache/src/main/java/com/codahale/metrics/ehcache/InstrumentedEhcache.java b/metrics-ehcache/src/main/java/com/codahale/metrics/ehcache/InstrumentedEhcache.java index a00524cc2a..4ffcc926f4 100644 --- a/metrics-ehcache/src/main/java/com/codahale/metrics/ehcache/InstrumentedEhcache.java +++ b/metrics-ehcache/src/main/java/com/codahale/metrics/ehcache/InstrumentedEhcache.java @@ -1,6 +1,5 @@ package com.codahale.metrics.ehcache; -import com.codahale.metrics.Gauge; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import net.sf.ehcache.CacheException; @@ -118,56 +117,56 @@ public class InstrumentedEhcache extends EhcacheDecoratorAdapter { public static Ehcache instrument(MetricRegistry registry, final Ehcache cache) { final String prefix = name(cache.getClass(), cache.getName()); - registry.register(name(prefix, "hits"), - (Gauge) () -> cache.getStatistics().cacheHitCount()); + registry.registerGauge(name(prefix, "hits"), + () -> cache.getStatistics().cacheHitCount()); - registry.register(name(prefix, "in-memory-hits"), - (Gauge) () -> cache.getStatistics().localHeapHitCount()); + registry.registerGauge(name(prefix, "in-memory-hits"), + () -> cache.getStatistics().localHeapHitCount()); - registry.register(name(prefix, "off-heap-hits"), - (Gauge) () -> cache.getStatistics().localOffHeapHitCount()); + registry.registerGauge(name(prefix, "off-heap-hits"), + () -> cache.getStatistics().localOffHeapHitCount()); - registry.register(name(prefix, "on-disk-hits"), - (Gauge) () -> cache.getStatistics().localDiskHitCount()); + registry.registerGauge(name(prefix, "on-disk-hits"), + () -> cache.getStatistics().localDiskHitCount()); - registry.register(name(prefix, "misses"), - (Gauge) () -> cache.getStatistics().cacheMissCount()); + registry.registerGauge(name(prefix, "misses"), + () -> cache.getStatistics().cacheMissCount()); - registry.register(name(prefix, "in-memory-misses"), - (Gauge) () -> cache.getStatistics().localHeapMissCount()); + registry.registerGauge(name(prefix, "in-memory-misses"), + () -> cache.getStatistics().localHeapMissCount()); - registry.register(name(prefix, "off-heap-misses"), - (Gauge) () -> cache.getStatistics().localOffHeapMissCount()); + registry.registerGauge(name(prefix, "off-heap-misses"), + () -> cache.getStatistics().localOffHeapMissCount()); - registry.register(name(prefix, "on-disk-misses"), - (Gauge) () -> cache.getStatistics().localDiskMissCount()); + registry.registerGauge(name(prefix, "on-disk-misses"), + () -> cache.getStatistics().localDiskMissCount()); - registry.register(name(prefix, "objects"), - (Gauge) () -> cache.getStatistics().getSize()); + registry.registerGauge(name(prefix, "objects"), + () -> cache.getStatistics().getSize()); - registry.register(name(prefix, "in-memory-objects"), - (Gauge) () -> cache.getStatistics().getLocalHeapSize()); + registry.registerGauge(name(prefix, "in-memory-objects"), + () -> cache.getStatistics().getLocalHeapSize()); - registry.register(name(prefix, "off-heap-objects"), - (Gauge) () -> cache.getStatistics().getLocalOffHeapSize()); + registry.registerGauge(name(prefix, "off-heap-objects"), + () -> cache.getStatistics().getLocalOffHeapSize()); - registry.register(name(prefix, "on-disk-objects"), - (Gauge) () -> cache.getStatistics().getLocalDiskSize()); + registry.registerGauge(name(prefix, "on-disk-objects"), + () -> cache.getStatistics().getLocalDiskSize()); - registry.register(name(prefix, "mean-get-time"), - (Gauge) () -> cache.getStatistics().cacheGetOperation().latency().average().value()); + registry.registerGauge(name(prefix, "mean-get-time"), + () -> cache.getStatistics().cacheGetOperation().latency().average().value()); - registry.register(name(prefix, "mean-search-time"), - (Gauge) () -> cache.getStatistics().cacheSearchOperation().latency().average().value()); + registry.registerGauge(name(prefix, "mean-search-time"), + () -> cache.getStatistics().cacheSearchOperation().latency().average().value()); - registry.register(name(prefix, "eviction-count"), - (Gauge) () -> cache.getStatistics().cacheEvictionOperation().count().value()); + registry.registerGauge(name(prefix, "eviction-count"), + () -> cache.getStatistics().cacheEvictionOperation().count().value()); - registry.register(name(prefix, "searches-per-second"), - (Gauge) () -> cache.getStatistics().cacheSearchOperation().rate().value()); + registry.registerGauge(name(prefix, "searches-per-second"), + () -> cache.getStatistics().cacheSearchOperation().rate().value()); - registry.register(name(prefix, "writer-queue-size"), - (Gauge) () -> cache.getStatistics().getWriterQueueLength()); + registry.registerGauge(name(prefix, "writer-queue-size"), + () -> cache.getStatistics().getWriterQueueLength()); return new InstrumentedEhcache(registry, cache); } diff --git a/metrics-ehcache/src/test/java/com/codahale/metrics/ehcache/InstrumentedEhcacheTest.java b/metrics-ehcache/src/test/java/com/codahale/metrics/ehcache/InstrumentedEhcacheTest.java index 43687044a0..a2f863664c 100644 --- a/metrics-ehcache/src/test/java/com/codahale/metrics/ehcache/InstrumentedEhcacheTest.java +++ b/metrics-ehcache/src/test/java/com/codahale/metrics/ehcache/InstrumentedEhcacheTest.java @@ -12,6 +12,7 @@ import static com.codahale.metrics.MetricRegistry.name; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; public class InstrumentedEhcacheTest { private static final CacheManager MANAGER = CacheManager.create(); @@ -24,6 +25,27 @@ public void setUp() { final Cache c = new Cache(new CacheConfiguration("test", 100)); MANAGER.addCache(c); this.cache = InstrumentedEhcache.instrument(registry, c); + assertThat(registry.getGauges().entrySet().stream() + .map(e -> entry(e.getKey(), e.getValue().getValue()))) + .containsOnly( + entry("net.sf.ehcache.Cache.test.eviction-count", 0L), + entry("net.sf.ehcache.Cache.test.hits", 0L), + entry("net.sf.ehcache.Cache.test.in-memory-hits", 0L), + entry("net.sf.ehcache.Cache.test.in-memory-misses", 0L), + entry("net.sf.ehcache.Cache.test.in-memory-objects", 0L), + entry("net.sf.ehcache.Cache.test.mean-get-time", Double.NaN), + entry("net.sf.ehcache.Cache.test.mean-search-time", Double.NaN), + entry("net.sf.ehcache.Cache.test.misses", 0L), + entry("net.sf.ehcache.Cache.test.objects", 0L), + entry("net.sf.ehcache.Cache.test.off-heap-hits", 0L), + entry("net.sf.ehcache.Cache.test.off-heap-misses", 0L), + entry("net.sf.ehcache.Cache.test.off-heap-objects", 0L), + entry("net.sf.ehcache.Cache.test.on-disk-hits", 0L), + entry("net.sf.ehcache.Cache.test.on-disk-misses", 0L), + entry("net.sf.ehcache.Cache.test.on-disk-objects", 0L), + entry("net.sf.ehcache.Cache.test.searches-per-second", 0.0), + entry("net.sf.ehcache.Cache.test.writer-queue-size", 0L) + ); } @Test diff --git a/metrics-httpasyncclient/src/main/java/com/codahale/metrics/httpasyncclient/InstrumentedNClientConnManager.java b/metrics-httpasyncclient/src/main/java/com/codahale/metrics/httpasyncclient/InstrumentedNClientConnManager.java index aee7c0516a..4e0c162d60 100644 --- a/metrics-httpasyncclient/src/main/java/com/codahale/metrics/httpasyncclient/InstrumentedNClientConnManager.java +++ b/metrics-httpasyncclient/src/main/java/com/codahale/metrics/httpasyncclient/InstrumentedNClientConnManager.java @@ -21,25 +21,25 @@ public class InstrumentedNClientConnManager extends PoolingNHttpClientConnection public InstrumentedNClientConnManager(final ConnectingIOReactor ioreactor, final NHttpConnectionFactory connFactory, final SchemePortResolver schemePortResolver, final MetricRegistry metricRegistry, final Registry iosessionFactoryRegistry, final long timeToLive, final TimeUnit tunit, final DnsResolver dnsResolver, final String name) { super(ioreactor, connFactory, iosessionFactoryRegistry, schemePortResolver, dnsResolver, timeToLive, tunit); metricRegistry.register(name(NHttpClientConnectionManager.class, name, "available-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getAvailable(); - }); + (Gauge) () -> { + // this acquires a lock on the connection pool; remove if contention sucks + return getTotalStats().getAvailable(); + }); metricRegistry.register(name(NHttpClientConnectionManager.class, name, "leased-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getLeased(); - }); + (Gauge) () -> { + // this acquires a lock on the connection pool; remove if contention sucks + return getTotalStats().getLeased(); + }); metricRegistry.register(name(NHttpClientConnectionManager.class, name, "max-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getMax(); - }); + (Gauge) () -> { + // this acquires a lock on the connection pool; remove if contention sucks + return getTotalStats().getMax(); + }); metricRegistry.register(name(NHttpClientConnectionManager.class, name, "pending-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getPending(); - }); + (Gauge) () -> { + // this acquires a lock on the connection pool; remove if contention sucks + return getTotalStats().getPending(); + }); } } diff --git a/metrics-httpclient/src/main/java/com/codahale/metrics/httpclient/InstrumentedHttpClientConnectionManager.java b/metrics-httpclient/src/main/java/com/codahale/metrics/httpclient/InstrumentedHttpClientConnectionManager.java index 073e3ce553..89d397858e 100644 --- a/metrics-httpclient/src/main/java/com/codahale/metrics/httpclient/InstrumentedHttpClientConnectionManager.java +++ b/metrics-httpclient/src/main/java/com/codahale/metrics/httpclient/InstrumentedHttpClientConnectionManager.java @@ -1,6 +1,5 @@ package com.codahale.metrics.httpclient; -import com.codahale.metrics.Gauge; import com.codahale.metrics.MetricRegistry; import org.apache.http.config.Registry; import org.apache.http.config.RegistryBuilder; @@ -104,26 +103,18 @@ public InstrumentedHttpClientConnectionManager(MetricRegistry metricsRegistry, this.metricsRegistry = metricsRegistry; this.name = name; - metricsRegistry.register(name(HttpClientConnectionManager.class, name, "available-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getAvailable(); - }); - metricsRegistry.register(name(HttpClientConnectionManager.class, name, "leased-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getLeased(); - }); - metricsRegistry.register(name(HttpClientConnectionManager.class, name, "max-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getMax(); - }); - metricsRegistry.register(name(HttpClientConnectionManager.class, name, "pending-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getPending(); - }); + // this acquires a lock on the connection pool; remove if contention sucks + metricsRegistry.registerGauge(name(HttpClientConnectionManager.class, name, "available-connections"), + () -> getTotalStats().getAvailable()); + // this acquires a lock on the connection pool; remove if contention sucks + metricsRegistry.registerGauge(name(HttpClientConnectionManager.class, name, "leased-connections"), + () -> getTotalStats().getLeased()); + // this acquires a lock on the connection pool; remove if contention sucks + metricsRegistry.registerGauge(name(HttpClientConnectionManager.class, name, "max-connections"), + () -> getTotalStats().getMax()); + // this acquires a lock on the connection pool; remove if contention sucks + metricsRegistry.registerGauge(name(HttpClientConnectionManager.class, name, "pending-connections"), + () -> getTotalStats().getPending()); } @Override diff --git a/metrics-httpclient/src/test/java/com/codahale/metrics/httpclient/InstrumentedHttpClientConnectionManagerTest.java b/metrics-httpclient/src/test/java/com/codahale/metrics/httpclient/InstrumentedHttpClientConnectionManagerTest.java index c3be3c933e..662fa1adbf 100644 --- a/metrics-httpclient/src/test/java/com/codahale/metrics/httpclient/InstrumentedHttpClientConnectionManagerTest.java +++ b/metrics-httpclient/src/test/java/com/codahale/metrics/httpclient/InstrumentedHttpClientConnectionManagerTest.java @@ -7,6 +7,8 @@ import org.mockito.Mockito; import static junit.framework.TestCase.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.mockito.ArgumentMatchers.any; @@ -16,7 +18,12 @@ public class InstrumentedHttpClientConnectionManagerTest { @Test public void shouldRemoveGauges() { final InstrumentedHttpClientConnectionManager instrumentedHttpClientConnectionManager = InstrumentedHttpClientConnectionManager.builder(metricRegistry).build(); - Assert.assertEquals(4, metricRegistry.getGauges().size()); + assertThat(metricRegistry.getGauges().entrySet().stream() + .map(e -> entry(e.getKey(), e.getValue().getValue()))) + .containsOnly(entry("org.apache.http.conn.HttpClientConnectionManager.available-connections", 0), + entry("org.apache.http.conn.HttpClientConnectionManager.leased-connections", 0), + entry("org.apache.http.conn.HttpClientConnectionManager.max-connections", 20), + entry("org.apache.http.conn.HttpClientConnectionManager.pending-connections", 0)); instrumentedHttpClientConnectionManager.close(); Assert.assertEquals(0, metricRegistry.getGauges().size()); @@ -36,7 +43,7 @@ public void configurableViaBuilder() { .close(); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(registry, Mockito.atLeast(1)).register(argumentCaptor.capture(), any()); + Mockito.verify(registry, Mockito.atLeast(1)).registerGauge(argumentCaptor.capture(), any()); assertTrue(argumentCaptor.getValue().contains("some-other-name")); } } diff --git a/metrics-httpclient5/src/main/java/com/codahale/metrics/httpclient5/InstrumentedAsyncClientConnectionManager.java b/metrics-httpclient5/src/main/java/com/codahale/metrics/httpclient5/InstrumentedAsyncClientConnectionManager.java index ed4ffbb1a8..b778c54e8c 100644 --- a/metrics-httpclient5/src/main/java/com/codahale/metrics/httpclient5/InstrumentedAsyncClientConnectionManager.java +++ b/metrics-httpclient5/src/main/java/com/codahale/metrics/httpclient5/InstrumentedAsyncClientConnectionManager.java @@ -1,6 +1,5 @@ package com.codahale.metrics.httpclient5; -import com.codahale.metrics.Gauge; import com.codahale.metrics.MetricRegistry; import org.apache.hc.client5.http.DnsResolver; import org.apache.hc.client5.http.SchemePortResolver; @@ -49,26 +48,18 @@ protected static Registry getDefaultTlsStrategy() { this.metricsRegistry = requireNonNull(metricRegistry, "metricRegistry"); this.name = name; - metricRegistry.register(name(METRICS_PREFIX, name, "available-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getAvailable(); - }); - metricRegistry.register(name(METRICS_PREFIX, name, "leased-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getLeased(); - }); - metricRegistry.register(name(METRICS_PREFIX, name, "max-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getMax(); - }); - metricRegistry.register(name(METRICS_PREFIX, name, "pending-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getPending(); - }); + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(METRICS_PREFIX, name, "available-connections"), + () -> getTotalStats().getAvailable()); + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(METRICS_PREFIX, name, "leased-connections"), + () -> getTotalStats().getLeased()); + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(METRICS_PREFIX, name, "max-connections"), + () -> getTotalStats().getMax()); + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(METRICS_PREFIX, name, "pending-connections"), + () -> getTotalStats().getPending()); } /** diff --git a/metrics-httpclient5/src/main/java/com/codahale/metrics/httpclient5/InstrumentedHttpClientConnectionManager.java b/metrics-httpclient5/src/main/java/com/codahale/metrics/httpclient5/InstrumentedHttpClientConnectionManager.java index ff3a421d43..c98b97f322 100644 --- a/metrics-httpclient5/src/main/java/com/codahale/metrics/httpclient5/InstrumentedHttpClientConnectionManager.java +++ b/metrics-httpclient5/src/main/java/com/codahale/metrics/httpclient5/InstrumentedHttpClientConnectionManager.java @@ -1,6 +1,5 @@ package com.codahale.metrics.httpclient5; -import com.codahale.metrics.Gauge; import com.codahale.metrics.MetricRegistry; import org.apache.hc.client5.http.DnsResolver; import org.apache.hc.client5.http.SchemePortResolver; @@ -52,26 +51,21 @@ protected static Registry getDefaultRegistry() { this.metricsRegistry = requireNonNull(metricRegistry, "metricRegistry"); this.name = name; - metricRegistry.register(name(METRICS_PREFIX, name, "available-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(METRICS_PREFIX, name, "available-connections"), + () -> { return getTotalStats().getAvailable(); }); - metricRegistry.register(name(METRICS_PREFIX, name, "leased-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getLeased(); - }); - metricRegistry.register(name(METRICS_PREFIX, name, "max-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getMax(); - }); - metricRegistry.register(name(METRICS_PREFIX, name, "pending-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getPending(); - }); + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(METRICS_PREFIX, name, "leased-connections"), + () -> getTotalStats().getLeased()); + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(METRICS_PREFIX, name, "max-connections"), + () -> getTotalStats().getMax() + ); + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(METRICS_PREFIX, name, "pending-connections"), + () -> getTotalStats().getPending()); } /** diff --git a/metrics-httpclient5/src/test/java/com/codahale/metrics/httpclient5/InstrumentedAsyncClientConnectionManagerTest.java b/metrics-httpclient5/src/test/java/com/codahale/metrics/httpclient5/InstrumentedAsyncClientConnectionManagerTest.java index 9a3cc14fd9..79316927fb 100644 --- a/metrics-httpclient5/src/test/java/com/codahale/metrics/httpclient5/InstrumentedAsyncClientConnectionManagerTest.java +++ b/metrics-httpclient5/src/test/java/com/codahale/metrics/httpclient5/InstrumentedAsyncClientConnectionManagerTest.java @@ -7,6 +7,8 @@ import org.mockito.Mockito; import static junit.framework.TestCase.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.mockito.ArgumentMatchers.any; public class InstrumentedAsyncClientConnectionManagerTest { @@ -15,7 +17,12 @@ public class InstrumentedAsyncClientConnectionManagerTest { @Test public void shouldRemoveGauges() { final InstrumentedAsyncClientConnectionManager instrumentedHttpClientConnectionManager = InstrumentedAsyncClientConnectionManager.builder(metricRegistry).build(); - Assert.assertEquals(4, metricRegistry.getGauges().size()); + assertThat(metricRegistry.getGauges().entrySet().stream() + .map(e -> entry(e.getKey(), e.getValue().getValue()))) + .containsOnly(entry("org.apache.hc.client5.http.nio.AsyncClientConnectionManager.available-connections", 0), + entry("org.apache.hc.client5.http.nio.AsyncClientConnectionManager.leased-connections", 0), + entry("org.apache.hc.client5.http.nio.AsyncClientConnectionManager.max-connections", 25), + entry("org.apache.hc.client5.http.nio.AsyncClientConnectionManager.pending-connections", 0)); instrumentedHttpClientConnectionManager.close(); Assert.assertEquals(0, metricRegistry.getGauges().size()); @@ -35,7 +42,7 @@ public void configurableViaBuilder() { .close(); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(registry, Mockito.atLeast(1)).register(argumentCaptor.capture(), any()); + Mockito.verify(registry, Mockito.atLeast(1)).registerGauge(argumentCaptor.capture(), any()); assertTrue(argumentCaptor.getValue().contains("some-other-name")); } } diff --git a/metrics-httpclient5/src/test/java/com/codahale/metrics/httpclient5/InstrumentedHttpClientConnectionManagerTest.java b/metrics-httpclient5/src/test/java/com/codahale/metrics/httpclient5/InstrumentedHttpClientConnectionManagerTest.java index 91fdce31da..c2d157177a 100644 --- a/metrics-httpclient5/src/test/java/com/codahale/metrics/httpclient5/InstrumentedHttpClientConnectionManagerTest.java +++ b/metrics-httpclient5/src/test/java/com/codahale/metrics/httpclient5/InstrumentedHttpClientConnectionManagerTest.java @@ -7,6 +7,8 @@ import org.mockito.Mockito; import static junit.framework.TestCase.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.mockito.ArgumentMatchers.any; public class InstrumentedHttpClientConnectionManagerTest { @@ -15,7 +17,12 @@ public class InstrumentedHttpClientConnectionManagerTest { @Test public void shouldRemoveGauges() { final InstrumentedHttpClientConnectionManager instrumentedHttpClientConnectionManager = InstrumentedHttpClientConnectionManager.builder(metricRegistry).build(); - Assert.assertEquals(4, metricRegistry.getGauges().size()); + assertThat(metricRegistry.getGauges().entrySet().stream() + .map(e -> entry(e.getKey(), e.getValue().getValue()))) + .containsOnly(entry("org.apache.hc.client5.http.io.HttpClientConnectionManager.available-connections", 0), + entry("org.apache.hc.client5.http.io.HttpClientConnectionManager.leased-connections", 0), + entry("org.apache.hc.client5.http.io.HttpClientConnectionManager.max-connections", 25), + entry("org.apache.hc.client5.http.io.HttpClientConnectionManager.pending-connections", 0)); instrumentedHttpClientConnectionManager.close(); Assert.assertEquals(0, metricRegistry.getGauges().size()); @@ -35,7 +42,7 @@ public void configurableViaBuilder() { .close(); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(registry, Mockito.atLeast(1)).register(argumentCaptor.capture(), any()); + Mockito.verify(registry, Mockito.atLeast(1)).registerGauge(argumentCaptor.capture(), any()); assertTrue(argumentCaptor.getValue().contains("some-other-name")); } } diff --git a/metrics-jetty10/src/main/java/io/dropwizard/metrics/jetty10/InstrumentedQueuedThreadPool.java b/metrics-jetty10/src/main/java/io/dropwizard/metrics/jetty10/InstrumentedQueuedThreadPool.java index 270c02ee7d..d0f6135b40 100644 --- a/metrics-jetty10/src/main/java/io/dropwizard/metrics/jetty10/InstrumentedQueuedThreadPool.java +++ b/metrics-jetty10/src/main/java/io/dropwizard/metrics/jetty10/InstrumentedQueuedThreadPool.java @@ -1,6 +1,5 @@ package io.dropwizard.metrics.jetty10; -import com.codahale.metrics.Gauge; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.RatioGauge; import org.eclipse.jetty.util.annotation.Name; @@ -87,12 +86,10 @@ protected Ratio getRatio() { return Ratio.of(getThreads() - getIdleThreads(), getMaxThreads()); } }); - metricRegistry.register(name(prefix, NAME_SIZE), (Gauge) this::getThreads); - metricRegistry.register(name(prefix, NAME_JOBS), (Gauge) () -> { - // This assumes the QueuedThreadPool is using a BlockingArrayQueue or - // ArrayBlockingQueue for its queue, and is therefore a constant-time operation. - return getQueue().size(); - }); + // This assumes the QueuedThreadPool is using a BlockingArrayQueue or + // ArrayBlockingQueue for its queue, and is therefore a constant-time operation. + metricRegistry.registerGauge(name(prefix, NAME_SIZE), this::getThreads); + metricRegistry.registerGauge(name(prefix, NAME_JOBS), () -> getQueue().size()); metricRegistry.register(name(prefix, NAME_JOBS_QUEUE_UTILIZATION), new RatioGauge() { @Override protected Ratio getRatio() { diff --git a/metrics-jetty11/src/main/java/io/dropwizard/metrics/jetty11/InstrumentedQueuedThreadPool.java b/metrics-jetty11/src/main/java/io/dropwizard/metrics/jetty11/InstrumentedQueuedThreadPool.java index 7a631b464d..6566703a4f 100644 --- a/metrics-jetty11/src/main/java/io/dropwizard/metrics/jetty11/InstrumentedQueuedThreadPool.java +++ b/metrics-jetty11/src/main/java/io/dropwizard/metrics/jetty11/InstrumentedQueuedThreadPool.java @@ -1,6 +1,5 @@ package io.dropwizard.metrics.jetty11; -import com.codahale.metrics.Gauge; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.RatioGauge; import org.eclipse.jetty.util.annotation.Name; @@ -87,12 +86,10 @@ protected Ratio getRatio() { return Ratio.of(getThreads() - getIdleThreads(), getMaxThreads()); } }); - metricRegistry.register(name(prefix, NAME_SIZE), (Gauge) this::getThreads); - metricRegistry.register(name(prefix, NAME_JOBS), (Gauge) () -> { - // This assumes the QueuedThreadPool is using a BlockingArrayQueue or - // ArrayBlockingQueue for its queue, and is therefore a constant-time operation. - return getQueue().size(); - }); + metricRegistry.registerGauge(name(prefix, NAME_SIZE), this::getThreads); + // This assumes the QueuedThreadPool is using a BlockingArrayQueue or + // ArrayBlockingQueue for its queue, and is therefore a constant-time operation. + metricRegistry.registerGauge(name(prefix, NAME_JOBS), () -> getQueue().size()); metricRegistry.register(name(prefix, NAME_JOBS_QUEUE_UTILIZATION), new RatioGauge() { @Override protected Ratio getRatio() { diff --git a/metrics-jetty9/src/main/java/com/codahale/metrics/jetty9/InstrumentedQueuedThreadPool.java b/metrics-jetty9/src/main/java/com/codahale/metrics/jetty9/InstrumentedQueuedThreadPool.java index 141750b8af..d3889ca2c9 100644 --- a/metrics-jetty9/src/main/java/com/codahale/metrics/jetty9/InstrumentedQueuedThreadPool.java +++ b/metrics-jetty9/src/main/java/com/codahale/metrics/jetty9/InstrumentedQueuedThreadPool.java @@ -1,6 +1,5 @@ package com.codahale.metrics.jetty9; -import com.codahale.metrics.Gauge; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.RatioGauge; import org.eclipse.jetty.util.annotation.Name; @@ -87,12 +86,10 @@ protected Ratio getRatio() { return Ratio.of(getThreads() - getIdleThreads(), getMaxThreads()); } }); - metricRegistry.register(name(prefix, NAME_SIZE), (Gauge) this::getThreads); - metricRegistry.register(name(prefix, NAME_JOBS), (Gauge) () -> { - // This assumes the QueuedThreadPool is using a BlockingArrayQueue or - // ArrayBlockingQueue for its queue, and is therefore a constant-time operation. - return getQueue().size(); - }); + metricRegistry.registerGauge(name(prefix, NAME_SIZE), this::getThreads); + // This assumes the QueuedThreadPool is using a BlockingArrayQueue or + // ArrayBlockingQueue for its queue, and is therefore a constant-time operation. + metricRegistry.registerGauge(name(prefix, NAME_JOBS), () -> getQueue().size()); metricRegistry.register(name(prefix, NAME_JOBS_QUEUE_UTILIZATION), new RatioGauge() { @Override protected Ratio getRatio() { diff --git a/metrics-jmx/src/test/java/com/codahale/metrics/jmx/JmxReporterTest.java b/metrics-jmx/src/test/java/com/codahale/metrics/jmx/JmxReporterTest.java index a1a5907fe3..7c1dfa2bbd 100644 --- a/metrics-jmx/src/test/java/com/codahale/metrics/jmx/JmxReporterTest.java +++ b/metrics-jmx/src/test/java/com/codahale/metrics/jmx/JmxReporterTest.java @@ -126,15 +126,12 @@ public void registersMBeansForMetricObjectsUsingProvidedObjectNameFactory() thro try { String widgetName = "something"; when(mockObjectNameFactory.createName(any(String.class), any(String.class), any(String.class))).thenReturn(n); - Gauge aGauge = mock(Gauge.class); - when(aGauge.getValue()).thenReturn(1); - JmxReporter reporter = JmxReporter.forRegistry(registry) .registerWith(mBeanServer) .inDomain(name) .createsObjectNamesWith(mockObjectNameFactory) .build(); - registry.register(widgetName, aGauge); + registry.registerGauge(widgetName, () -> 1); reporter.start(); verify(mockObjectNameFactory).createName(eq("gauges"), any(String.class), eq("something")); //verifyNoMoreInteractions(mockObjectNameFactory); From ceb7d4dcd4d537dfefcec49de9c5f21445810e87 Mon Sep 17 00:00:00 2001 From: Artem Prigoda Date: Thu, 16 Jun 2022 17:30:49 +0200 Subject: [PATCH 2/3] Migrate InstrumentedNClientConnManager --- .../InstrumentedNClientConnManager.java | 33 +++++++------------ 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/metrics-httpasyncclient/src/main/java/com/codahale/metrics/httpasyncclient/InstrumentedNClientConnManager.java b/metrics-httpasyncclient/src/main/java/com/codahale/metrics/httpasyncclient/InstrumentedNClientConnManager.java index 4e0c162d60..e541f5dc51 100644 --- a/metrics-httpasyncclient/src/main/java/com/codahale/metrics/httpasyncclient/InstrumentedNClientConnManager.java +++ b/metrics-httpasyncclient/src/main/java/com/codahale/metrics/httpasyncclient/InstrumentedNClientConnManager.java @@ -1,6 +1,5 @@ package com.codahale.metrics.httpasyncclient; -import com.codahale.metrics.Gauge; import com.codahale.metrics.MetricRegistry; import org.apache.http.config.Registry; import org.apache.http.conn.DnsResolver; @@ -20,26 +19,18 @@ public class InstrumentedNClientConnManager extends PoolingNHttpClientConnection public InstrumentedNClientConnManager(final ConnectingIOReactor ioreactor, final NHttpConnectionFactory connFactory, final SchemePortResolver schemePortResolver, final MetricRegistry metricRegistry, final Registry iosessionFactoryRegistry, final long timeToLive, final TimeUnit tunit, final DnsResolver dnsResolver, final String name) { super(ioreactor, connFactory, iosessionFactoryRegistry, schemePortResolver, dnsResolver, timeToLive, tunit); - metricRegistry.register(name(NHttpClientConnectionManager.class, name, "available-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getAvailable(); - }); - metricRegistry.register(name(NHttpClientConnectionManager.class, name, "leased-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getLeased(); - }); - metricRegistry.register(name(NHttpClientConnectionManager.class, name, "max-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getMax(); - }); - metricRegistry.register(name(NHttpClientConnectionManager.class, name, "pending-connections"), - (Gauge) () -> { - // this acquires a lock on the connection pool; remove if contention sucks - return getTotalStats().getPending(); - }); + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(NHttpClientConnectionManager.class, name, "available-connections"), + () -> getTotalStats().getAvailable()); + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(NHttpClientConnectionManager.class, name, "leased-connections"), + () -> getTotalStats().getLeased()); + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(NHttpClientConnectionManager.class, name, "max-connections"), + () -> getTotalStats().getMax()); + // this acquires a lock on the connection pool; remove if contention sucks + metricRegistry.registerGauge(name(NHttpClientConnectionManager.class, name, "pending-connections"), + () -> getTotalStats().getPending()); } } From 54c3b70d1839bdaf0e4866baf656679573e31e0e Mon Sep 17 00:00:00 2001 From: Jochen Schalanda Date: Sat, 18 Jun 2022 17:39:16 +0000 Subject: [PATCH 3/3] Add since version to Javadoc of MetricRegistry#registerGauge() --- .../src/main/java/com/codahale/metrics/MetricRegistry.java | 1 + 1 file changed, 1 insertion(+) diff --git a/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java b/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java index 59b4f13b01..528ed2b224 100644 --- a/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java +++ b/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java @@ -82,6 +82,7 @@ protected ConcurrentMap buildMap() { * @param name the name of the gauge * @param the type of the gauge's value * @return the registered {@link Gauge} + * @since 4.2.10 */ public Gauge registerGauge(String name, Gauge metric) throws IllegalArgumentException { return register(name, metric);