Skip to content

Commit

Permalink
Add a more type safe way of registering gauges (#2642)
Browse files Browse the repository at this point in the history
Add the `registerGauge` method to `MetricRegistry` that accepts `Gauge<T>` instead of `Metric` which makes it possible to register a gauge as a lambda expression without declaring its type explicitly.
  • Loading branch information
arteam committed Jun 18, 2022
1 parent d785a56 commit c1d0fb9
Show file tree
Hide file tree
Showing 16 changed files with 196 additions and 173 deletions.
Expand Up @@ -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<Integer>) executor::getPoolSize);
registry.register(MetricRegistry.name(name, "pool.core"),
(Gauge<Integer>) executor::getCorePoolSize);
registry.register(MetricRegistry.name(name, "pool.max"),
(Gauge<Integer>) 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<Runnable> queue = executor.getQueue();
registry.register(MetricRegistry.name(name, "tasks.active"),
(Gauge<Integer>) executor::getActiveCount);
registry.register(MetricRegistry.name(name, "tasks.completed"),
(Gauge<Long>) executor::getCompletedTaskCount);
registry.register(MetricRegistry.name(name, "tasks.queued"),
(Gauge<Integer>) queue::size);
registry.register(MetricRegistry.name(name, "tasks.capacity"),
(Gauge<Integer>) 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<Long>) forkJoinPool::getStealCount);
registry.register(MetricRegistry.name(name, "tasks.queued"),
(Gauge<Long>) forkJoinPool::getQueuedTaskCount);
registry.register(MetricRegistry.name(name, "threads.active"),
(Gauge<Integer>) forkJoinPool::getActiveThreadCount);
registry.register(MetricRegistry.name(name, "threads.running"),
(Gauge<Integer>) 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);
}
}

Expand Down
Expand Up @@ -76,6 +76,18 @@ protected ConcurrentMap<String, Metric> 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 <T> the type of the gauge's value
* @return the registered {@link Gauge}
* @since 4.2.10
*/
public <T> Gauge<T> registerGauge(String name, Gauge<T> metric) throws IllegalArgumentException {
return register(name, metric);
}

/**
* Given a {@link Metric}, registers it under the given name.
*
Expand All @@ -93,7 +105,7 @@ public <T extends Metric> 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
Expand Down
Expand Up @@ -615,15 +615,29 @@ 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 !!!");
} catch (NullPointerException e) {
Assert.assertEquals("metric == null", e.getMessage());
}
}

@Test
public void infersGaugeType() {
Gauge<Long> 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);
}
}
@@ -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;
Expand Down Expand Up @@ -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<Long>) () -> cache.getStatistics().cacheHitCount());
registry.registerGauge(name(prefix, "hits"),
() -> cache.getStatistics().cacheHitCount());

registry.register(name(prefix, "in-memory-hits"),
(Gauge<Long>) () -> cache.getStatistics().localHeapHitCount());
registry.registerGauge(name(prefix, "in-memory-hits"),
() -> cache.getStatistics().localHeapHitCount());

registry.register(name(prefix, "off-heap-hits"),
(Gauge<Long>) () -> cache.getStatistics().localOffHeapHitCount());
registry.registerGauge(name(prefix, "off-heap-hits"),
() -> cache.getStatistics().localOffHeapHitCount());

registry.register(name(prefix, "on-disk-hits"),
(Gauge<Long>) () -> cache.getStatistics().localDiskHitCount());
registry.registerGauge(name(prefix, "on-disk-hits"),
() -> cache.getStatistics().localDiskHitCount());

registry.register(name(prefix, "misses"),
(Gauge<Long>) () -> cache.getStatistics().cacheMissCount());
registry.registerGauge(name(prefix, "misses"),
() -> cache.getStatistics().cacheMissCount());

registry.register(name(prefix, "in-memory-misses"),
(Gauge<Long>) () -> cache.getStatistics().localHeapMissCount());
registry.registerGauge(name(prefix, "in-memory-misses"),
() -> cache.getStatistics().localHeapMissCount());

registry.register(name(prefix, "off-heap-misses"),
(Gauge<Long>) () -> cache.getStatistics().localOffHeapMissCount());
registry.registerGauge(name(prefix, "off-heap-misses"),
() -> cache.getStatistics().localOffHeapMissCount());

registry.register(name(prefix, "on-disk-misses"),
(Gauge<Long>) () -> cache.getStatistics().localDiskMissCount());
registry.registerGauge(name(prefix, "on-disk-misses"),
() -> cache.getStatistics().localDiskMissCount());

registry.register(name(prefix, "objects"),
(Gauge<Long>) () -> cache.getStatistics().getSize());
registry.registerGauge(name(prefix, "objects"),
() -> cache.getStatistics().getSize());

registry.register(name(prefix, "in-memory-objects"),
(Gauge<Long>) () -> cache.getStatistics().getLocalHeapSize());
registry.registerGauge(name(prefix, "in-memory-objects"),
() -> cache.getStatistics().getLocalHeapSize());

registry.register(name(prefix, "off-heap-objects"),
(Gauge<Long>) () -> cache.getStatistics().getLocalOffHeapSize());
registry.registerGauge(name(prefix, "off-heap-objects"),
() -> cache.getStatistics().getLocalOffHeapSize());

registry.register(name(prefix, "on-disk-objects"),
(Gauge<Long>) () -> cache.getStatistics().getLocalDiskSize());
registry.registerGauge(name(prefix, "on-disk-objects"),
() -> cache.getStatistics().getLocalDiskSize());

registry.register(name(prefix, "mean-get-time"),
(Gauge<Double>) () -> 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<Double>) () -> 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<Long>) () -> cache.getStatistics().cacheEvictionOperation().count().value());
registry.registerGauge(name(prefix, "eviction-count"),
() -> cache.getStatistics().cacheEvictionOperation().count().value());

registry.register(name(prefix, "searches-per-second"),
(Gauge<Double>) () -> 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<Long>) () -> cache.getStatistics().getWriterQueueLength());
registry.registerGauge(name(prefix, "writer-queue-size"),
() -> cache.getStatistics().getWriterQueueLength());

return new InstrumentedEhcache(registry, cache);
}
Expand Down
Expand Up @@ -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();
Expand All @@ -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
Expand Down
@@ -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;
Expand All @@ -20,26 +19,18 @@ public class InstrumentedNClientConnManager extends PoolingNHttpClientConnection

public InstrumentedNClientConnManager(final ConnectingIOReactor ioreactor, final NHttpConnectionFactory<ManagedNHttpClientConnection> connFactory, final SchemePortResolver schemePortResolver, final MetricRegistry metricRegistry, final Registry<SchemeIOSessionStrategy> 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<Integer>) () -> {
// this acquires a lock on the connection pool; remove if contention sucks
return getTotalStats().getAvailable();
});
metricRegistry.register(name(NHttpClientConnectionManager.class, name, "leased-connections"),
(Gauge<Integer>) () -> {
// this acquires a lock on the connection pool; remove if contention sucks
return getTotalStats().getLeased();
});
metricRegistry.register(name(NHttpClientConnectionManager.class, name, "max-connections"),
(Gauge<Integer>) () -> {
// this acquires a lock on the connection pool; remove if contention sucks
return getTotalStats().getMax();
});
metricRegistry.register(name(NHttpClientConnectionManager.class, name, "pending-connections"),
(Gauge<Integer>) () -> {
// 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());
}

}
@@ -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;
Expand Down Expand Up @@ -104,26 +103,18 @@ public InstrumentedHttpClientConnectionManager(MetricRegistry metricsRegistry,
this.metricsRegistry = metricsRegistry;
this.name = name;

metricsRegistry.register(name(HttpClientConnectionManager.class, name, "available-connections"),
(Gauge<Integer>) () -> {
// this acquires a lock on the connection pool; remove if contention sucks
return getTotalStats().getAvailable();
});
metricsRegistry.register(name(HttpClientConnectionManager.class, name, "leased-connections"),
(Gauge<Integer>) () -> {
// this acquires a lock on the connection pool; remove if contention sucks
return getTotalStats().getLeased();
});
metricsRegistry.register(name(HttpClientConnectionManager.class, name, "max-connections"),
(Gauge<Integer>) () -> {
// this acquires a lock on the connection pool; remove if contention sucks
return getTotalStats().getMax();
});
metricsRegistry.register(name(HttpClientConnectionManager.class, name, "pending-connections"),
(Gauge<Integer>) () -> {
// 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
Expand Down
Expand Up @@ -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;


Expand All @@ -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());
Expand All @@ -36,7 +43,7 @@ public void configurableViaBuilder() {
.close();

ArgumentCaptor<String> 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"));
}
}

0 comments on commit c1d0fb9

Please sign in to comment.