Skip to content

Commit

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

Closes gh-5408
  • Loading branch information
izeye authored Aug 30, 2024
1 parent fc6a491 commit d14eac7
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 d14eac7

Please sign in to comment.