The cache.stats.disabled metric is listed in metric-schema (#2005)

The `cache.stats.disabled` metric is listed in metric-schema
palantir · Aug 29, 2024 · d27bf86 · d27bf86
1 parent 64de19d
commit d27bf86
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 27 deletions.
diff --git a/changelog/@unreleased/pr-2005.v2.yml 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
@@ -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
@@ -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/...-caffeine/src/test/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStatsTest.java b/...-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,30 +219,27 @@ void registerCacheWithoutRecordingStats() {
         Cache<Integer, String> 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);
     }
 
     @Test
     void registerCacheWithoutRecordingStatsTagged() {
         Cache<Integer, String> 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();
     }