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 authored Jun 18, 2022
1 parent d785a56 commit c1d0fb9
Show file tree
Hide file tree
Showing 16 changed files with 196 additions and 173 deletions.
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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);
}
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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());
}

}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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"));
}
}
Loading

0 comments on commit c1d0fb9

Please sign in to comment.