diff --git a/changelog/@unreleased/pr-2005.v2.yml b/changelog/@unreleased/pr-2005.v2.yml new file mode 100644 index 000000000..587d6f87e --- /dev/null +++ b/changelog/@unreleased/pr-2005.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: The `cache.stats.disabled` metric is listed in metric-schema + links: + - https://github.com/palantir/tritium/pull/2005 diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStats.java index 6cdf205fb..0cd1a8f3b 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStats.java @@ -18,8 +18,8 @@ import static com.palantir.logsafe.Preconditions.checkNotNull; -import com.codahale.metrics.Counter; import com.codahale.metrics.Gauge; +import com.codahale.metrics.Meter; import com.codahale.metrics.MetricRegistry; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; @@ -38,7 +38,6 @@ public final class CaffeineCacheStats { private static final SafeLogger log = SafeLoggerFactory.get(CaffeineCacheStats.class); - private static final String STATS_DISABLED = "cache.stats.disabled"; private CaffeineCacheStats() {} @@ -64,7 +63,10 @@ public static void registerCache(MetricRegistry registry, Cache cache, Str .getMetrics() .forEach((key, value) -> MetricRegistries.registerWithReplacement(registry, key, value)); } else { - warnNotRecordingStats(name, registry.counter(MetricRegistry.name(name, STATS_DISABLED))); + warnNotRecordingStats( + name, + registry.meter(MetricRegistry.name( + name, CacheMetrics.statsDisabledMetricName(name).safeName()))); } } @@ -88,14 +90,12 @@ public static void registerCache(TaggedMetricRegistry registry, Cache cach if (cache.policy().isRecordingStats()) { CaffeineCacheTaggedMetrics.create(cache, name).getMetrics().forEach(registry::registerWithReplacement); } else { - warnNotRecordingStats( - name, - registry.counter(InternalCacheMetrics.taggedMetricName(name).apply(STATS_DISABLED))); + warnNotRecordingStats(name, CacheMetrics.of(registry).statsDisabled(name)); } } - private static void warnNotRecordingStats(@Safe String name, Counter counter) { - counter.inc(); + private static void warnNotRecordingStats(@Safe String name, Meter counter) { + counter.mark(); log.warn( "Registered cache does not have stats recording enabled, stats will always be zero. " + "To enable cache metrics, stats recording must be enabled when constructing the cache: " diff --git a/tritium-caffeine/src/main/metrics/cache.yml b/tritium-caffeine/src/main/metrics/cache.yml index 3dc152f3b..efa46463d 100644 --- a/tritium-caffeine/src/main/metrics/cache.yml +++ b/tritium-caffeine/src/main/metrics/cache.yml @@ -39,3 +39,7 @@ namespaces: type: gauge tags: [cache] docs: Maximum number of cache entries cache can hold if limited + stats.disabled: + type: meter + tags: [cache] + docs: Meter marked when `CaffeineCacheStats.registerCache` is called on a cache that does not record stats using `caffeineBuilder.recordStats()`. diff --git a/tritium-caffeine/src/test/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStatsTest.java b/tritium-caffeine/src/test/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStatsTest.java index 29e76783e..bbe9aadd2 100644 --- a/tritium-caffeine/src/test/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStatsTest.java +++ b/tritium-caffeine/src/test/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStatsTest.java @@ -24,6 +24,7 @@ import com.codahale.metrics.ConsoleReporter; import com.codahale.metrics.Counter; import com.codahale.metrics.Gauge; +import com.codahale.metrics.Meter; import com.codahale.metrics.Metric; import com.codahale.metrics.MetricRegistry; import com.github.benmanes.caffeine.cache.Cache; @@ -121,7 +122,7 @@ void registerCacheMetrics() { .extracting(Gauge::getValue) .isEqualTo(3L); - assertThat(metricRegistry.getCounters()) + assertThat(metricRegistry.getMeters()) .extractingByKey("test.cache.stats.disabled") .isNull(); } @@ -208,10 +209,7 @@ void registerCacheTaggedMetrics() { .isEqualTo(0.25); assertThat(taggedMetricRegistry.getMetrics()) - .extractingByKey(MetricName.builder() - .safeName("cache.stats.disabled") - .putSafeTags("cache", "test") - .build()) + .extractingByKey(CacheMetrics.statsDisabledMetricName("test")) .isNull(); }); } @@ -221,12 +219,12 @@ void registerCacheWithoutRecordingStats() { Cache cache = Caffeine.newBuilder().build(); CaffeineCacheStats.registerCache(metricRegistry, cache, "test"); String disabledMetricName = "test.cache.stats.disabled"; - assertThat(metricRegistry.getCounters()) + assertThat(metricRegistry.getMeters()) .hasSize(1) .containsOnlyKeys(disabledMetricName) .extractingByKey(disabledMetricName) - .isInstanceOf(Counter.class) - .extracting(Counter::getCount) + .isInstanceOf(Meter.class) + .extracting(Meter::getCount) .isEqualTo(1L); } @@ -234,17 +232,14 @@ void registerCacheWithoutRecordingStats() { void registerCacheWithoutRecordingStatsTagged() { Cache cache = Caffeine.newBuilder().build(); CaffeineCacheStats.registerCache(taggedMetricRegistry, cache, "test"); - MetricName disabledMetricName = MetricName.builder() - .safeName("cache.stats.disabled") - .putSafeTags("cache", "test") - .build(); + MetricName disabledMetricName = CacheMetrics.statsDisabledMetricName("test"); assertThat(taggedMetricRegistry.getMetrics()) .hasSize(1) .containsOnlyKeys(disabledMetricName) .extractingByKey(disabledMetricName) - .isInstanceOf(Counter.class) - .asInstanceOf(type(Counter.class)) - .extracting(Counter::getCount) + .isInstanceOf(Meter.class) + .asInstanceOf(type(Meter.class)) + .extracting(Meter::getCount) .isEqualTo(1L); } @@ -318,10 +313,7 @@ void registerTaggedMetrics() { .isOne(); assertThat(taggedMetricRegistry.getMetrics()) - .extractingByKey(MetricName.builder() - .safeName("cache.stats.disabled") - .putSafeTags("cache", "test") - .build()) + .extractingByKey(CacheMetrics.statsDisabledMetricName("test")) .isNull(); }