Change CacheState.register to a fluent API (#1920)

changelog/@unreleased/pr-1920.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: '`CacheState.register` has been changed to be a more fluent API.'
+  links:
+  - https://github.com/palantir/tritium/pull/1920

tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java

@@ -20,6 +20,7 @@
 import com.codahale.metrics.Meter;
 import com.codahale.metrics.Timer;
 import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
 import com.github.benmanes.caffeine.cache.Policy;
 import com.github.benmanes.caffeine.cache.RemovalCause;
 import com.github.benmanes.caffeine.cache.stats.StatsCounter;
@@ -31,6 +32,7 @@
 import java.util.Arrays;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.LongAdder;
+import java.util.function.Function;
 import java.util.function.Supplier;
 import org.checkerframework.checker.index.qual.NonNegative;
 
@@ -47,16 +49,14 @@ public final class CacheStats implements StatsCounter, Supplier<StatsCounter> {
     private final LongAdder totalLoadTime = new LongAdder();
 
     /**
-     * Creates a {@link CacheStats} instance that registers metrics for Caffeine cache statistics.
+     * Creates a {@link CacheStats} instance that can be used to record metrics for Caffeine cache statistics.
      * <p>
-     * Example usage for a {@link com.github.benmanes.caffeine.cache.Cache} or
-     * {@link com.github.benmanes.caffeine.cache.LoadingCache}:
+     * Example usage:
      * <pre>
-     *     CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "your-cache-name")
-     *     LoadingCache&lt;Integer, String&gt; cache = Caffeine.newBuilder()
-     *             .recordStats(cacheStats)
-     *             .build(key -&gt; computeSomethingExpensive(key));
-     *     cacheStats.register(cache);
+     *     Cache&lt;Integer, String&gt; cache = CacheStats.of(taggedMetricRegistry, "your-cache-name")
+     *             .register(stats -> Caffeine.newBuilder()
+     *                     .recordStats(stats)
+     *                     .build());
      * </pre>
      * @param taggedMetricRegistry tagged metric registry to add cache metrics
      * @param name cache name
@@ -68,12 +68,24 @@ public static CacheStats of(TaggedMetricRegistry taggedMetricRegistry, @Safe Str
     }
 
     /**
-     * Registers additional metrics for a Caffeine cache.
-     *
-     * @param cache cache for which to register metrics
-     * @return the given cache instance
+     * Constructs and registers metrics for Caffeine cache statistics.
+     * <p>
+     * In order to record metrics, the {@code cacheFactory} must use the provided {@link CacheStats} with
+     * {@link Caffeine#recordStats(Supplier)})}.
+     * <p>
+     * Example usage:
+     * <pre>
+     *     Cache&lt;Integer, String&gt; cache = CacheStats.of(taggedMetricRegistry, "your-cache-name")
+     *             .register(stats -> Caffeine.newBuilder()
+     *                     .recordStats(stats)
+     *                     .build());
+     * </pre>
+     * @param cacheFactory method which will be invoked to construct the cache
+     * @return the constructed cache instance
      */
-    public <K, V, C extends Cache<K, V>> C register(C cache) {
+    public <K, V, C extends Cache<K, V>> C register(Function<CacheStats, C> cacheFactory) {
+        C cache = cacheFactory.apply(this);
+
         metrics.estimatedSize().cache(name).build(cache::estimatedSize);
         metrics.weightedSize().cache(name).build(() -> cache.policy()
                 .eviction()
@@ -83,6 +95,7 @@ public <K, V, C extends Cache<K, V>> C register(C cache) {
                 .eviction()
                 .map(Policy.Eviction::getMaximum)
                 .orElse(null));
+
         return cache;
     }
 

tritium-caffeine/src/main/metrics/cache-metrics.yml

@@ -46,4 +46,4 @@ namespaces:
       maximum.size:
         type: gauge
         tags: [cache]
-        docs: Maximum number of cache entries cache can hold if limited, otherwise -1
+        docs: Maximum number of cache entries cache can hold if limited

tritium-caffeine/src/test/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStatsTest.java

@@ -217,10 +217,9 @@ void registerCacheWithoutRecordingStatsTagged() {
 
     @Test
     void registerTaggedMetrics() {
-        CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "test");
-        Cache<Integer, String> cache =
-                Caffeine.newBuilder().recordStats(cacheStats).maximumSize(2).build();
-        cacheStats.register(cache);
+        Cache<Integer, String> cache = CacheStats.of(taggedMetricRegistry, "test")
+                .register(stats ->
+                        Caffeine.newBuilder().recordStats(stats).maximumSize(2).build());
 
         assertThat(taggedMetricRegistry.getMetrics().keySet())
                 .extracting(MetricName::safeName)
@@ -279,10 +278,9 @@ void registerTaggedMetrics() {
 
     @Test
     void registerLoadingTaggedMetrics() {
-        CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "test");
-        LoadingCache<Integer, String> cache =
-                Caffeine.newBuilder().recordStats(cacheStats).maximumSize(2).build(mapping::apply);
-        cacheStats.register(cache);
+        LoadingCache<Integer, String> cache = CacheStats.of(taggedMetricRegistry, "test")
+                .register(stats ->
+                        Caffeine.newBuilder().recordStats(stats).maximumSize(2).build(mapping::apply));
         assertThat(taggedMetricRegistry.getMetrics().keySet())
                 .extracting(MetricName::safeName)
                 .containsExactlyInAnyOrder(