Skip to content

Commit

Permalink
Skip registering Caffeine meters when statistics are not enabled
Browse files Browse the repository at this point in the history
This commit also fixes and improves GuavaCacheMetricsTest and CaffeineCacheMetricsTest.

Closes micrometer-metricsgh-5408
  • Loading branch information
izeye committed Aug 29, 2024
1 parent fc6a491 commit 9b1be25
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ public abstract class CacheMeterBinder<C> implements MeterBinder {

private static final String DESCRIPTION_CACHE_GETS = "The number of times cache lookup methods have returned a cached (hit) or uncached (newly loaded or null) value (miss).";

static final long UNSUPPORTED = -1L;

private final WeakReference<C> cacheRef;

private final Iterable<Tag> tags;
Expand Down Expand Up @@ -75,16 +77,20 @@ public final void bindTo(MeterRegistry registry) {
}).tags(tags).tag("result", "miss").description(DESCRIPTION_CACHE_GETS).register(registry);
}

FunctionCounter.builder("cache.gets", cache, c -> hitCount())
.tags(tags)
.tag("result", "hit")
.description(DESCRIPTION_CACHE_GETS)
.register(registry);
if (hitCount() != UNSUPPORTED) {
FunctionCounter.builder("cache.gets", cache, c -> hitCount())
.tags(tags)
.tag("result", "hit")
.description(DESCRIPTION_CACHE_GETS)
.register(registry);
}

FunctionCounter.builder("cache.puts", cache, c -> putCount())
.tags(tags)
.description("The number of entries added to the cache")
.register(registry);
if (putCount() != UNSUPPORTED) {
FunctionCounter.builder("cache.puts", cache, c -> putCount())
.tags(tags)
.description("The number of entries added to the cache")
.register(registry);
}

if (evictionCount() != null) {
FunctionCounter.builder("cache.evictions", cache, c -> {
Expand All @@ -108,7 +114,8 @@ public final void bindTo(MeterRegistry registry) {

/**
* @return Get requests that resulted in a "hit" against an existing cache entry.
* Monotonically increasing hit count.
* Monotonically increasing hit count. Returns -1 if the cache implementation does not
* support this.
*/
protected abstract long hitCount();

Expand All @@ -134,6 +141,7 @@ public final void bindTo(MeterRegistry registry) {
* The put mechanism is unimportant - this count applies to entries added to the cache
* according to a pre-defined load function such as exists in Guava/Caffeine caches as
* well as manual puts. Note that Guava/Caffeine caches don't count manual puts.
* Returns -1 if the cache implementation does not support this.
* @return Total number of entries added to the cache. Monotonically increasing count.
*/
protected abstract long putCount();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public CaffeineCacheMetrics(C cache, String cacheName, Iterable<Tag> tags) {

if (!cache.policy().isRecordingStats()) {
log.warn(
"The cache '{}' is not recording statistics, so their values will be zero. Call 'Caffeine#recordStats()' prior to building the cache for metrics to be recorded.",
"The cache '{}' is not recording statistics. No meters except 'cache.size' will be registered. Call 'Caffeine#recordStats()' prior to building the cache for metrics to be recorded.",
cacheName);
}
}
Expand Down Expand Up @@ -176,6 +176,10 @@ protected long putCount() {
@Override
protected void bindImplementationSpecificMetrics(MeterRegistry registry) {
C cache = getCache();
if (cache == null || !cache.policy().isRecordingStats()) {
return;
}

FunctionCounter.builder("cache.eviction.weight", cache, c -> c.stats().evictionWeight())
.tags(getTagsWithCacheName())
.description("The sum of weights of evicted entries. This total does not include manual invalidations.")
Expand Down Expand Up @@ -206,6 +210,10 @@ protected void bindImplementationSpecificMetrics(MeterRegistry registry) {
private Long getOrDefault(Function<C, Long> function, @Nullable Long defaultValue) {
C cache = getCache();
if (cache != null) {
if (!cache.policy().isRecordingStats()) {
return null;
}

return function.apply(cache);
}

Expand All @@ -215,6 +223,10 @@ private Long getOrDefault(Function<C, Long> function, @Nullable Long defaultValu
private long getOrDefault(ToLongFunction<C> function, long defaultValue) {
C cache = getCache();
if (cache != null) {
if (!cache.policy().isRecordingStats()) {
return UNSUPPORTED;
}

return function.applyAsLong(cache);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Tags;
import io.micrometer.core.instrument.TimeGauge;
import io.micrometer.core.instrument.search.MeterNotFoundException;
import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
import org.junit.jupiter.api.Test;

import java.util.concurrent.TimeUnit;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.*;

/**
* Tests for {@link CaffeineCacheMetrics}.
Expand Down Expand Up @@ -103,21 +104,34 @@ void returnCacheSize() {

@Test
void returnHitCount() {
MeterRegistry meterRegistry = new SimpleMeterRegistry();
metrics.bindTo(meterRegistry);

cache.put("a", "1");
cache.get("a");
cache.get("a");

assertThat(metrics.hitCount()).isEqualTo(cache.stats().hitCount()).isEqualTo(2);
assertThat(meterRegistry.get("cache.gets").tag("result", "hit").functionCounter().count()).isEqualTo(2);
}

@Test
void returnHitCountWithoutRecordStats() {
LoadingCache<String, String> cache = Caffeine.newBuilder().build(key -> "");
CaffeineCacheMetrics<String, String, Cache<String, String>> metrics = new CaffeineCacheMetrics<>(cache,
"testCache", expectedTag);

MeterRegistry meterRegistry = new SimpleMeterRegistry();
metrics.bindTo(meterRegistry);

cache.put("a", "1");
cache.get("a");
cache.get("a");

assertThat(metrics.hitCount()).isEqualTo(cache.stats().hitCount()).isEqualTo(0);
assertThat(cache.stats().hitCount()).isEqualTo(0);
assertThat(metrics.hitCount()).isEqualTo(CaffeineCacheMetrics.UNSUPPORTED);
assertThatExceptionOfType(MeterNotFoundException.class)
.isThrownBy(() -> meterRegistry.get("cache.gets").tag("result", "hit").functionCounter());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,15 @@ void returnCacheSize() {

@Test
void returnHitCount() throws ExecutionException {
MeterRegistry meterRegistry = new SimpleMeterRegistry();
metrics.bindTo(meterRegistry);

cache.put("a", "1");
cache.get("a");
cache.get("a");

assertThat(metrics.hitCount()).isEqualTo(cache.stats().hitCount()).isEqualTo(2);
assertThat(meterRegistry.get("cache.gets").tag("result", "hit").functionCounter().count()).isEqualTo(2);
}

@Test
Expand All @@ -117,12 +121,18 @@ public String load(String key) {
return "";
}
});
GuavaCacheMetrics<String, String, Cache<String, String>> metrics = new GuavaCacheMetrics<>(cache, "testCache",
expectedTag);

MeterRegistry meterRegistry = new SimpleMeterRegistry();
metrics.bindTo(meterRegistry);

cache.put("a", "1");
cache.get("a");
cache.get("a");

assertThat(metrics.hitCount()).isEqualTo(cache.stats().hitCount()).isEqualTo(0);
assertThat(meterRegistry.get("cache.gets").tag("result", "hit").functionCounter().count()).isEqualTo(0);
}

@Test
Expand Down

0 comments on commit 9b1be25

Please sign in to comment.