From 66a48887de1b4769a3c55c753b25158277b58b9e Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 13:41:11 -0500 Subject: [PATCH 01/16] Initial Caffeine CacheStats recorder --- tritium-caffeine/build.gradle | 1 + .../tritium/metrics/caffeine/CacheStats.java | 166 ++++++++++++++++++ .../caffeine/CaffeineCacheStatsTest.java | 152 +++++++++++++++- .../src/main/metrics/cache-metrics.yml | 66 +++++++ 4 files changed, 377 insertions(+), 8 deletions(-) create mode 100644 tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java create mode 100644 tritium-metrics/src/main/metrics/cache-metrics.yml diff --git a/tritium-caffeine/build.gradle b/tritium-caffeine/build.gradle index 66dcb746d..5aecddc25 100644 --- a/tritium-caffeine/build.gradle +++ b/tritium-caffeine/build.gradle @@ -11,6 +11,7 @@ dependencies { implementation 'com.palantir.safe-logging:preconditions' implementation 'com.palantir.safe-logging:safe-logging' implementation 'io.dropwizard.metrics:metrics-core' + implementation 'org.checkerframework:checker-qual' testImplementation 'org.assertj:assertj-core' testImplementation 'org.awaitility:awaitility' diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java new file mode 100644 index 000000000..3db6413fb --- /dev/null +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java @@ -0,0 +1,166 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.tritium.metrics.caffeine; + +import com.codahale.metrics.Counter; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.LoadingCache; +import com.github.benmanes.caffeine.cache.Policy; +import com.github.benmanes.caffeine.cache.RemovalCause; +import com.github.benmanes.caffeine.cache.stats.StatsCounter; +import com.google.common.collect.ImmutableMap; +import com.palantir.tritium.metrics.cache.CacheMetrics; +import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; +import java.util.Arrays; +import java.util.concurrent.atomic.LongAdder; +import java.util.function.Function; +import java.util.function.Supplier; +import org.checkerframework.checker.index.qual.NonNegative; + +public final class CacheStats implements StatsCounter { + private final CacheMetrics metrics; + private final String name; + private final Counter hitCounter; + private final Counter missCounter; + + private final LongAdder totalLoadNanos = new LongAdder(); + private final Counter loadSuccessCounter; + private final Counter loadFailureCounter; + private final Counter evictionsTotalCounter; + private final ImmutableMap evictionCounters; + + /** + * Creates a {@link CacheStats} instance that registers metrics for Caffeine cache statistics. + *

+ * Example method + * 

+     *         CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "your-cache-name");
+     *         LoadingCache<Integer, String> cache = cacheStats.register(Caffeine.newBuilder()
+     *                 .recordStats(cacheStats.record())
+     *                 .build(key -> computeSomethingExpensive(key));
+     *
+ * @param taggedMetricRegistry tagged metric registry to add cache metrics + * @param name cache name + * @return Caffeine stats instance to register via + * {@link com.github.benmanes.caffeine.cache.Caffeine#recordStats(Supplier)}. + */ + public static CacheStats of(TaggedMetricRegistry taggedMetricRegistry, String name) { + return new CacheStats(CacheMetrics.of(taggedMetricRegistry), name); + } + + public static CacheStats of(CacheMetrics metrics, String name) { + return new CacheStats(metrics, name); + } + + private CacheStats(CacheMetrics metrics, String name) { + this.metrics = metrics; + this.name = name; + this.hitCounter = metrics.hitCount(name); + this.missCounter = metrics.missCount(name); + this.loadSuccessCounter = metrics.loadSuccessCount(name); + this.loadFailureCounter = metrics.loadFailureCount(name); + this.evictionsTotalCounter = metrics.evictionCount(name); + this.evictionCounters = Arrays.stream(RemovalCause.values()) + .collect(ImmutableMap.toImmutableMap(Function.identity(), cause -> metrics.evictions() + .cache(name) + .cause(cause.toString()) + .build())); + metrics.requestCount().cache(name).build(() -> hitCounter.getCount() + missCounter.getCount()); + } + + public > C register(C cache) { + metrics.estimatedSize().cache(name).build(cache::estimatedSize); + metrics.maximumSize().cache(name).build(() -> cache.policy() + .eviction() + .map(Policy.Eviction::getMaximum) + .orElse(-1L)); + metrics.weightedSize().cache(name).build(() -> cache.policy() + .eviction() + .flatMap(e -> e.weightedSize().stream().boxed().findFirst()) + .orElse(0L)); + metrics.hitRatio().cache(name).build(() -> { + double hitCount = hitCounter.getCount(); + return hitCount / (hitCount + missCounter.getCount()); + }); + metrics.missRatio().cache(name).build(() -> { + double missCount = missCounter.getCount(); + return missCount / (hitCounter.getCount() + missCount); + }); + metrics.loadAverageMillis() + .cache(name) + .build(() -> + // convert nanoseconds to milliseconds + totalLoadNanos.sum() / 1_000_000.0d); + return cache; + } + + public LoadingCache register(LoadingCache cache) { + return (LoadingCache) register((Cache) cache); + } + + public Supplier recorder() { + return () -> this; + } + + @Override + public void recordHits(@NonNegative int count) { + hitCounter.inc(count); + } + + @Override + public void recordMisses(@NonNegative int count) { + missCounter.inc(count); + } + + @Override + public void recordLoadSuccess(@NonNegative long loadTime) { + loadSuccessCounter.inc(); + totalLoadNanos.add(loadTime); + } + + @Override + public void recordLoadFailure(@NonNegative long loadTime) { + loadFailureCounter.inc(); + totalLoadNanos.add(loadTime); + } + + @Override + public void recordEviction(@NonNegative int weight, RemovalCause cause) { + Counter counter = evictionCounters.get(cause); + if (counter != null) { + counter.inc(weight); + } + evictionsTotalCounter.inc(weight); + } + + @Override + public com.github.benmanes.caffeine.cache.stats.CacheStats snapshot() { + return com.github.benmanes.caffeine.cache.stats.CacheStats.of( + hitCounter.getCount(), + missCounter.getCount(), + loadSuccessCounter.getCount(), + loadFailureCounter.getCount(), + totalLoadNanos.sum(), + evictionsTotalCounter.getCount(), + evictionCounters.values().stream().mapToLong(Counter::getCount).sum()); + } + + @Override + public String toString() { + return name + ": " + snapshot(); + } +} 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 54178c48b..c10fbb6b2 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 @@ -17,6 +17,7 @@ package com.palantir.tritium.metrics.caffeine; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.InstanceOfAssertFactories.collection; import static org.assertj.core.api.InstanceOfAssertFactories.type; import static org.awaitility.Awaitility.await; @@ -27,13 +28,21 @@ import com.codahale.metrics.MetricRegistry; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; +import com.google.common.collect.ImmutableSetMultimap; +import com.google.common.collect.Multimaps; import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry; import com.palantir.tritium.metrics.registry.MetricName; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.time.Duration; import java.util.Map; +import java.util.Map.Entry; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; import java.util.function.Function; import org.assertj.core.api.AbstractObjectAssert; +import org.assertj.core.api.ObjectAssert; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -202,21 +211,148 @@ void registerCacheWithoutRecordingStatsTagged() { .isEqualTo(1L); } - static AbstractObjectAssert assertGauge(TaggedMetricRegistry taggedMetricRegistry, String name) { - Gauge metric = getMetric(taggedMetricRegistry, Gauge.class, name); + @Test + void registerTaggedMetrics() { + CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "test"); + Cache cache = cacheStats.register(Caffeine.newBuilder() + .recordStats(cacheStats.recorder()) + .maximumSize(2) + .build()); + assertThat(taggedMetricRegistry.getMetrics().keySet()) + .extracting(MetricName::safeName) + .contains( + "cache.estimated.size", + "cache.maximum.size", + "cache.weighted.size", + "cache.request.count", + "cache.hit.count", + "cache.hit.ratio", + "cache.miss.count", + "cache.miss.ratio", + "cache.eviction.count", + "cache.load.success.count", + "cache.load.failure.count", + "cache.load.average.millis"); + + await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> { + assertGauge(taggedMetricRegistry, "cache.request.count").isEqualTo(0L); + assertCounter(taggedMetricRegistry, "cache.hit.count").isEqualTo(0L); + assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(0L); + }); + + assertThat(cache.get(0, mapping)).isEqualTo("0"); + assertThat(cache.get(1, mapping)).isEqualTo("1"); + assertThat(cache.get(2, mapping)).isEqualTo("2"); + assertThat(cache.get(1, mapping)).isEqualTo("1"); + + await().atMost(Duration.ofSeconds(10)).untilAsserted(() -> { + // await to avoid flakes as gauges may be memoized + assertGauge(taggedMetricRegistry, "cache.request.count").isEqualTo(4L); + assertCounter(taggedMetricRegistry, "cache.hit.count").isEqualTo(1L); + assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3L); + assertGauge(taggedMetricRegistry, "cache.hit.ratio").isEqualTo(0.25); + + assertThat(taggedMetricRegistry.getMetrics()) + .extractingByKey(MetricName.builder() + .safeName("cache.stats.disabled") + .putSafeTags("cache", "test") + .build()) + .isNull(); + }); + } + + @Test + void registerLoadingTaggedMetrics() { + CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "test"); + LoadingCache cache = cacheStats.register(Caffeine.newBuilder() + .recordStats(cacheStats.recorder()) + .maximumSize(2) + .build(mapping::apply)); + assertThat(taggedMetricRegistry.getMetrics().keySet()) + .extracting(MetricName::safeName) + .contains( + "cache.estimated.size", + "cache.maximum.size", + "cache.weighted.size", + "cache.request.count", + "cache.hit.count", + "cache.hit.ratio", + "cache.miss.count", + "cache.miss.ratio", + "cache.eviction.count", + "cache.load.success.count", + "cache.load.failure.count", + "cache.load.average.millis"); + + await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> { + assertGauge(taggedMetricRegistry, "cache.request.count").isEqualTo(0L); + assertCounter(taggedMetricRegistry, "cache.hit.count").isEqualTo(0L); + assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(0L); + }); + + assertThat(cache.get(0)).isEqualTo("0"); + assertThat(cache.get(1)).isEqualTo("1"); + assertThat(cache.get(2)).isEqualTo("2"); + assertThat(cache.get(1)).isEqualTo("1"); + + await().atMost(Duration.ofSeconds(10)).untilAsserted(() -> { + // await to avoid flakes as gauges may be memoized + assertGauge(taggedMetricRegistry, "cache.request.count").isEqualTo(4L); + assertCounter(taggedMetricRegistry, "cache.hit.count").isEqualTo(1L); + assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3L); + assertGauge(taggedMetricRegistry, "cache.hit.ratio").isEqualTo(0.25); + assertThat(taggedMetricRegistry.getMetrics()) + .extractingByKey(MetricName.builder() + .safeName("cache.stats.disabled") + .putSafeTags("cache", "test") + .build()) + .isNull(); + }); + } + + static AbstractObjectAssert assertGauge(TaggedMetricRegistry taggedMetricRegistry, String name) { + return assertMetric(taggedMetricRegistry, Gauge.class, name).extracting(Gauge::getValue); + } + + static AbstractObjectAssert assertCounter(TaggedMetricRegistry taggedMetricRegistry, String name) { + return assertMetric(taggedMetricRegistry, Counter.class, name).extracting(Counter::getCount); + } + + static ObjectAssert assertMetric( + TaggedMetricRegistry taggedMetricRegistry, Class clazz, String name) { + T metric = getMetric(taggedMetricRegistry, clazz, name); return assertThat(metric) .as("metric '%s': '%s'", name, metric) .isNotNull() - .extracting(Gauge::getValue); + .asInstanceOf(type(clazz)); } private static T getMetric(TaggedMetricRegistry metrics, Class clazz, String name) { - return clazz.cast(metrics.getMetrics().entrySet().stream() + Optional> metric = metrics.getMetrics().entrySet().stream() .filter(e -> name.equals(e.getKey().safeName())) .filter(e -> clazz.isInstance(e.getValue())) - .findFirst() - .orElseThrow(() -> new IllegalArgumentException( - "No such metric '" + name + "' of type " + clazz.getCanonicalName())) - .getValue()); + .findFirst(); + if (metric.isEmpty()) { + Map> metricNameToType = Multimaps.asMap(metrics.getMetrics().entrySet().stream() + .filter(e -> Objects.nonNull(e.getKey())) + .filter(e -> Objects.nonNull(e.getValue())) + .collect(ImmutableSetMultimap.toImmutableSetMultimap( + e -> e.getKey().safeName(), e -> Optional.ofNullable(e.getValue()) + .map(x -> x.getClass().getCanonicalName()) + .orElse("")))); + + assertThat(metricNameToType) + .containsKey(name) + .extractingByKey(name) + .asInstanceOf(collection(String.class)) + .contains(clazz.getCanonicalName()); + + assertThat(metric) + .as( + "Metric named '%s' of type '%s' should exist but was not found in [%s]", + name, clazz.getCanonicalName(), metricNameToType.keySet()) + .isPresent(); + } + return clazz.cast(metric.orElseThrow().getValue()); } } diff --git a/tritium-metrics/src/main/metrics/cache-metrics.yml b/tritium-metrics/src/main/metrics/cache-metrics.yml new file mode 100644 index 000000000..8c6812cf7 --- /dev/null +++ b/tritium-metrics/src/main/metrics/cache-metrics.yml @@ -0,0 +1,66 @@ +options: + javaPackage: com.palantir.tritium.metrics.cache + javaVisibility: public +namespaces: + cache: + docs: Cache statistic metrics + metrics: + estimated.size: + type: gauge + tags: [cache] + docs: Approximate number of entries in this cache + weighted.size: + type: gauge + tags: [cache] + docs: Approximate accumulated weight of entries in this cache + maximum.size: + type: gauge + tags: [cache] + docs: Maximum number of cache entries cache can hold if limited, otherwise -1 + request.count: + type: gauge + tags: [cache] + docs: Count of cache requests + hit.count: + type: counter + tags: [cache] + docs: Count of cache hits + hit.ratio: + type: gauge + tags: [cache] + docs: Percentage of cache hits + miss.count: + type: counter + tags: [cache] + docs: Count of cache misses + miss.ratio: + type: gauge + tags: [cache] + docs: Percentage of cache misses + load.success.count: + type: counter + tags: [cache] + docs: Count of successful cache loads + load.failure.count: + type: counter + tags: [cache] + docs: Count of failed cache loads + load.average.millis: + type: gauge + tags: [cache] + docs: Average duration of cache load in milliseconds + evictions: + type: counter + tags: [cache, cause] + docs: Count of evicted entries by cause + eviction.count: + type: counter + tags: [cache] + docs: Total count of evicted entries + stats.disabled: + type: counter + tags: [cache] + docs: | + 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: + Caffeine.newBuilder().recordStats() From 0af16d2aa9062d11fbf7aa4dde3b7f803f84b32a Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 13:49:19 -0500 Subject: [PATCH 02/16] readme --- README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/README.md b/README.md index 3f15abfe3..195de6270 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,23 @@ Service instrumentedService = Tritium.instrument(Service.class, interestingService, environment.metrics()); ``` +## Instrumenting a [Caffeine cache](https://github.com/ben-manes/caffeine/) + +```java +import com.palantir.tritium.metrics.caffeine.CacheStats; + +TaggedMetricRegistry taggedMetricRegistry = ... +CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "unique-cache-name"); +Cache cache = cacheStats.register(Caffeine.newBuilder() + .recordStats(cacheStats.recorder()) + .build()); + +CacheStats loadingCacheStats = CacheStats.of(taggedMetricRegistry, "unique-loading-cache-name"); +LoadingCache loadingCache = loadingCacheStats.register(Caffeine.newBuilder() + .recordStats(loadingCacheStats.recorder()) + .build(key::length)); +``` + ## Creating a metric registry with reservoirs backed by [HDR Histograms](https://hdrhistogram.github.io/HdrHistogram/). HDR histograms are more useful if the service is long running, so the stats represents the lifetime of the server rather than using default exponential decay which can lead to some mis-interpretations of timings (especially higher percentiles and things like max dropping over time) if the consumer isn't aware of these assumptions. From 683c1669e06e9e2e886f1d5938c76a0b99d23b3f Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 13:51:50 -0500 Subject: [PATCH 03/16] self supply --- README.md | 4 ++-- .../tritium/metrics/caffeine/CacheStats.java | 7 ++++--- .../metrics/caffeine/CaffeineCacheStatsTest.java | 12 ++++-------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 195de6270..21a15c967 100644 --- a/README.md +++ b/README.md @@ -88,12 +88,12 @@ import com.palantir.tritium.metrics.caffeine.CacheStats; TaggedMetricRegistry taggedMetricRegistry = ... CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "unique-cache-name"); Cache cache = cacheStats.register(Caffeine.newBuilder() - .recordStats(cacheStats.recorder()) + .recordStats(cacheStats) .build()); CacheStats loadingCacheStats = CacheStats.of(taggedMetricRegistry, "unique-loading-cache-name"); LoadingCache loadingCache = loadingCacheStats.register(Caffeine.newBuilder() - .recordStats(loadingCacheStats.recorder()) + .recordStats(loadingCacheStats) .build(key::length)); ``` diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java index 3db6413fb..36f102d00 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java @@ -31,7 +31,7 @@ import java.util.function.Supplier; import org.checkerframework.checker.index.qual.NonNegative; -public final class CacheStats implements StatsCounter { +public final class CacheStats implements StatsCounter, Supplier { private final CacheMetrics metrics; private final String name; private final Counter hitCounter; @@ -112,8 +112,9 @@ public LoadingCache register(LoadingCache cache) { return (LoadingCache) register((Cache) cache); } - public Supplier recorder() { - return () -> this; + @Override + public StatsCounter get() { + return this; } @Override 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 c10fbb6b2..fd280df9b 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 @@ -214,10 +214,8 @@ void registerCacheWithoutRecordingStatsTagged() { @Test void registerTaggedMetrics() { CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "test"); - Cache cache = cacheStats.register(Caffeine.newBuilder() - .recordStats(cacheStats.recorder()) - .maximumSize(2) - .build()); + Cache cache = cacheStats.register( + Caffeine.newBuilder().recordStats(cacheStats).maximumSize(2).build()); assertThat(taggedMetricRegistry.getMetrics().keySet()) .extracting(MetricName::safeName) .contains( @@ -264,10 +262,8 @@ void registerTaggedMetrics() { @Test void registerLoadingTaggedMetrics() { CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "test"); - LoadingCache cache = cacheStats.register(Caffeine.newBuilder() - .recordStats(cacheStats.recorder()) - .maximumSize(2) - .build(mapping::apply)); + LoadingCache cache = cacheStats.register( + Caffeine.newBuilder().recordStats(cacheStats).maximumSize(2).build(mapping::apply)); assertThat(taggedMetricRegistry.getMetrics().keySet()) .extracting(MetricName::safeName) .contains( From 61e72e59546b698cfe5eac815d24110bf64fd3c0 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 14:07:33 -0500 Subject: [PATCH 04/16] Remove gauges & simplify --- README.md | 6 +- .../tritium/metrics/caffeine/CacheStats.java | 36 ----- .../caffeine/CaffeineCacheStatsTest.java | 128 +++++++++--------- .../src/main/metrics/cache-metrics.yml | 28 ---- 4 files changed, 68 insertions(+), 130 deletions(-) diff --git a/README.md b/README.md index 21a15c967..b5f32eb0d 100644 --- a/README.md +++ b/README.md @@ -86,14 +86,12 @@ Service instrumentedService = Tritium.instrument(Service.class, import com.palantir.tritium.metrics.caffeine.CacheStats; TaggedMetricRegistry taggedMetricRegistry = ... -CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "unique-cache-name"); Cache cache = cacheStats.register(Caffeine.newBuilder() - .recordStats(cacheStats) + .recordStats(CacheStats.of(taggedMetricRegistry, "unique-cache-name")) .build()); -CacheStats loadingCacheStats = CacheStats.of(taggedMetricRegistry, "unique-loading-cache-name"); LoadingCache loadingCache = loadingCacheStats.register(Caffeine.newBuilder() - .recordStats(loadingCacheStats) + .recordStats(CacheStats.of(taggedMetricRegistry, "unique-loading-cache-name")) .build(key::length)); ``` diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java index 36f102d00..f872b12aa 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java @@ -17,9 +17,6 @@ package com.palantir.tritium.metrics.caffeine; import com.codahale.metrics.Counter; -import com.github.benmanes.caffeine.cache.Cache; -import com.github.benmanes.caffeine.cache.LoadingCache; -import com.github.benmanes.caffeine.cache.Policy; import com.github.benmanes.caffeine.cache.RemovalCause; import com.github.benmanes.caffeine.cache.stats.StatsCounter; import com.google.common.collect.ImmutableMap; @@ -32,7 +29,6 @@ import org.checkerframework.checker.index.qual.NonNegative; public final class CacheStats implements StatsCounter, Supplier { - private final CacheMetrics metrics; private final String name; private final Counter hitCounter; private final Counter missCounter; @@ -67,7 +63,6 @@ public static CacheStats of(CacheMetrics metrics, String name) { } private CacheStats(CacheMetrics metrics, String name) { - this.metrics = metrics; this.name = name; this.hitCounter = metrics.hitCount(name); this.missCounter = metrics.missCount(name); @@ -79,37 +74,6 @@ private CacheStats(CacheMetrics metrics, String name) { .cache(name) .cause(cause.toString()) .build())); - metrics.requestCount().cache(name).build(() -> hitCounter.getCount() + missCounter.getCount()); - } - - public > C register(C cache) { - metrics.estimatedSize().cache(name).build(cache::estimatedSize); - metrics.maximumSize().cache(name).build(() -> cache.policy() - .eviction() - .map(Policy.Eviction::getMaximum) - .orElse(-1L)); - metrics.weightedSize().cache(name).build(() -> cache.policy() - .eviction() - .flatMap(e -> e.weightedSize().stream().boxed().findFirst()) - .orElse(0L)); - metrics.hitRatio().cache(name).build(() -> { - double hitCount = hitCounter.getCount(); - return hitCount / (hitCount + missCounter.getCount()); - }); - metrics.missRatio().cache(name).build(() -> { - double missCount = missCounter.getCount(); - return missCount / (hitCounter.getCount() + missCount); - }); - metrics.loadAverageMillis() - .cache(name) - .build(() -> - // convert nanoseconds to milliseconds - totalLoadNanos.sum() / 1_000_000.0d); - return cache; - } - - public LoadingCache register(LoadingCache cache) { - return (LoadingCache) register((Cache) cache); } @Override 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 fd280df9b..581f5dfd3 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 @@ -31,6 +31,7 @@ import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Multimaps; +import com.palantir.tritium.metrics.cache.CacheMetrics; import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry; import com.palantir.tritium.metrics.registry.MetricName; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; @@ -41,7 +42,9 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; +import org.assertj.core.api.AbstractLongAssert; import org.assertj.core.api.AbstractObjectAssert; +import org.assertj.core.api.InstanceOfAssertFactories; import org.assertj.core.api.ObjectAssert; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -213,105 +216,106 @@ void registerCacheWithoutRecordingStatsTagged() { @Test void registerTaggedMetrics() { - CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "test"); - Cache cache = cacheStats.register( - Caffeine.newBuilder().recordStats(cacheStats).maximumSize(2).build()); + CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry); + Cache cache = Caffeine.newBuilder() + .recordStats(CacheStats.of(cacheMetrics, "test")) + .maximumSize(2) + .build(); assertThat(taggedMetricRegistry.getMetrics().keySet()) .extracting(MetricName::safeName) - .contains( - "cache.estimated.size", - "cache.maximum.size", - "cache.weighted.size", - "cache.request.count", + .containsExactlyInAnyOrder( "cache.hit.count", - "cache.hit.ratio", "cache.miss.count", - "cache.miss.ratio", "cache.eviction.count", + "cache.evictions", // RemovalCause.EXPLICIT + "cache.evictions", // RemovalCause.REPLACED + "cache.evictions", // RemovalCause.COLLECTED + "cache.evictions", // RemovalCause.EXPIRED + "cache.evictions", // RemovalCause.SIZE "cache.load.success.count", - "cache.load.failure.count", - "cache.load.average.millis"); + "cache.load.failure.count"); - await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> { - assertGauge(taggedMetricRegistry, "cache.request.count").isEqualTo(0L); - assertCounter(taggedMetricRegistry, "cache.hit.count").isEqualTo(0L); - assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(0L); - }); + assertThat(cacheMetrics.hitCount("test").getCount()).isZero(); + assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) + .asInstanceOf(InstanceOfAssertFactories.LONG) + .isZero(); + assertCounter(taggedMetricRegistry, "cache.hit.count").isZero(); + assertCounter(taggedMetricRegistry, "cache.miss.count").isZero(); assertThat(cache.get(0, mapping)).isEqualTo("0"); assertThat(cache.get(1, mapping)).isEqualTo("1"); assertThat(cache.get(2, mapping)).isEqualTo("2"); assertThat(cache.get(1, mapping)).isEqualTo("1"); - await().atMost(Duration.ofSeconds(10)).untilAsserted(() -> { - // await to avoid flakes as gauges may be memoized - assertGauge(taggedMetricRegistry, "cache.request.count").isEqualTo(4L); - assertCounter(taggedMetricRegistry, "cache.hit.count").isEqualTo(1L); - assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3L); - assertGauge(taggedMetricRegistry, "cache.hit.ratio").isEqualTo(0.25); + assertCounter(taggedMetricRegistry, "cache.hit.count").isOne(); + assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3); + assertThat(cacheMetrics.hitCount("test").getCount()).isOne(); + assertThat(cacheMetrics.missCount("test").getCount()).isEqualTo(3); + assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) + .asInstanceOf(InstanceOfAssertFactories.LONG) + .isOne(); - assertThat(taggedMetricRegistry.getMetrics()) - .extractingByKey(MetricName.builder() - .safeName("cache.stats.disabled") - .putSafeTags("cache", "test") - .build()) - .isNull(); - }); + assertThat(taggedMetricRegistry.getMetrics()) + .extractingByKey(MetricName.builder() + .safeName("cache.stats.disabled") + .putSafeTags("cache", "test") + .build()) + .isNull(); } @Test void registerLoadingTaggedMetrics() { - CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "test"); - LoadingCache cache = cacheStats.register( - Caffeine.newBuilder().recordStats(cacheStats).maximumSize(2).build(mapping::apply)); + CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry); + LoadingCache cache = Caffeine.newBuilder() + .recordStats(CacheStats.of(cacheMetrics, "test")) + .maximumSize(2) + .build(mapping::apply); assertThat(taggedMetricRegistry.getMetrics().keySet()) .extracting(MetricName::safeName) - .contains( - "cache.estimated.size", - "cache.maximum.size", - "cache.weighted.size", - "cache.request.count", + .containsExactlyInAnyOrder( "cache.hit.count", - "cache.hit.ratio", "cache.miss.count", - "cache.miss.ratio", "cache.eviction.count", + "cache.evictions", // RemovalCause.EXPLICIT + "cache.evictions", // RemovalCause.REPLACED + "cache.evictions", // RemovalCause.COLLECTED + "cache.evictions", // RemovalCause.EXPIRED + "cache.evictions", // RemovalCause.SIZE "cache.load.success.count", - "cache.load.failure.count", - "cache.load.average.millis"); + "cache.load.failure.count"); - await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> { - assertGauge(taggedMetricRegistry, "cache.request.count").isEqualTo(0L); - assertCounter(taggedMetricRegistry, "cache.hit.count").isEqualTo(0L); - assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(0L); - }); + assertCounter(taggedMetricRegistry, "cache.hit.count").isZero(); + assertCounter(taggedMetricRegistry, "cache.miss.count").isZero(); assertThat(cache.get(0)).isEqualTo("0"); assertThat(cache.get(1)).isEqualTo("1"); assertThat(cache.get(2)).isEqualTo("2"); assertThat(cache.get(1)).isEqualTo("1"); - await().atMost(Duration.ofSeconds(10)).untilAsserted(() -> { - // await to avoid flakes as gauges may be memoized - assertGauge(taggedMetricRegistry, "cache.request.count").isEqualTo(4L); - assertCounter(taggedMetricRegistry, "cache.hit.count").isEqualTo(1L); - assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3L); - assertGauge(taggedMetricRegistry, "cache.hit.ratio").isEqualTo(0.25); - assertThat(taggedMetricRegistry.getMetrics()) - .extractingByKey(MetricName.builder() - .safeName("cache.stats.disabled") - .putSafeTags("cache", "test") - .build()) - .isNull(); - }); + assertCounter(taggedMetricRegistry, "cache.hit.count").isOne(); + assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3); + assertThat(cacheMetrics.hitCount("test").getCount()).isOne(); + assertThat(cacheMetrics.missCount("test").getCount()).isEqualTo(3); + assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) + .asInstanceOf(InstanceOfAssertFactories.LONG) + .isOne(); + + assertThat(taggedMetricRegistry.getMetrics()) + .extractingByKey(MetricName.builder() + .safeName("cache.stats.disabled") + .putSafeTags("cache", "test") + .build()) + .isNull(); } static AbstractObjectAssert assertGauge(TaggedMetricRegistry taggedMetricRegistry, String name) { return assertMetric(taggedMetricRegistry, Gauge.class, name).extracting(Gauge::getValue); } - static AbstractObjectAssert assertCounter(TaggedMetricRegistry taggedMetricRegistry, String name) { - return assertMetric(taggedMetricRegistry, Counter.class, name).extracting(Counter::getCount); + static AbstractLongAssert assertCounter(TaggedMetricRegistry taggedMetricRegistry, String name) { + return assertMetric(taggedMetricRegistry, Counter.class, name) + .extracting(Counter::getCount) + .asInstanceOf(InstanceOfAssertFactories.LONG); } static ObjectAssert assertMetric( diff --git a/tritium-metrics/src/main/metrics/cache-metrics.yml b/tritium-metrics/src/main/metrics/cache-metrics.yml index 8c6812cf7..efac1959e 100644 --- a/tritium-metrics/src/main/metrics/cache-metrics.yml +++ b/tritium-metrics/src/main/metrics/cache-metrics.yml @@ -5,38 +5,14 @@ namespaces: cache: docs: Cache statistic metrics metrics: - estimated.size: - type: gauge - tags: [cache] - docs: Approximate number of entries in this cache - weighted.size: - type: gauge - tags: [cache] - docs: Approximate accumulated weight of entries in this cache - maximum.size: - type: gauge - tags: [cache] - docs: Maximum number of cache entries cache can hold if limited, otherwise -1 - request.count: - type: gauge - tags: [cache] - docs: Count of cache requests hit.count: type: counter tags: [cache] docs: Count of cache hits - hit.ratio: - type: gauge - tags: [cache] - docs: Percentage of cache hits miss.count: type: counter tags: [cache] docs: Count of cache misses - miss.ratio: - type: gauge - tags: [cache] - docs: Percentage of cache misses load.success.count: type: counter tags: [cache] @@ -45,10 +21,6 @@ namespaces: type: counter tags: [cache] docs: Count of failed cache loads - load.average.millis: - type: gauge - tags: [cache] - docs: Average duration of cache load in milliseconds evictions: type: counter tags: [cache, cause] From 7c83c7a9fb94d51c94816425013265794f8a79d5 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 15:00:31 -0500 Subject: [PATCH 05/16] javadoc --- .../tritium/metrics/caffeine/CacheStats.java | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java index f872b12aa..caabbb4d9 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java @@ -42,12 +42,12 @@ public final class CacheStats implements StatsCounter, Supplier { /** * Creates a {@link CacheStats} instance that registers metrics for Caffeine cache statistics. *

- * Example method + * Example usage for a {@link com.github.benmanes.caffeine.cache.Cache} or + * {@link com.github.benmanes.caffeine.cache.LoadingCache}: * 

-     *         CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "your-cache-name");
-     *         LoadingCache<Integer, String> cache = cacheStats.register(Caffeine.newBuilder()
-     *                 .recordStats(cacheStats.record())
-     *                 .build(key -> computeSomethingExpensive(key));
+     *     LoadingCache<Integer, String> cache = Caffeine.newBuilder()
+     *             .recordStats(CacheStats.of(taggedMetricRegistry, "your-cache-name"))
+     *             .build(key -> computeSomethingExpensive(key));
      *
* @param taggedMetricRegistry tagged metric registry to add cache metrics * @param name cache name @@ -58,6 +58,22 @@ public static CacheStats of(TaggedMetricRegistry taggedMetricRegistry, String na return new CacheStats(CacheMetrics.of(taggedMetricRegistry), name); } + /** + * Creates a {@link CacheStats} instance that registers metrics for Caffeine cache statistics. + *

+ * Example usage for a {@link com.github.benmanes.caffeine.cache.Cache} or + * {@link com.github.benmanes.caffeine.cache.LoadingCache}: + * 

+     *     CacheMetrics metrics = CacheMetrics.of(taggedMetricRegistry);
+     *     LoadingCache<Integer, String> cache = Caffeine.newBuilder()
+     *             .recordStats(CacheStats.of(metrics, "your-cache-name"))
+     *             .build(key -> computeSomethingExpensive(key));
+     *
+ * @param metrics metric instance to use for cache metrics + * @param name cache name + * @return Caffeine stats instance to register via + * {@link com.github.benmanes.caffeine.cache.Caffeine#recordStats(Supplier)}. + */ public static CacheStats of(CacheMetrics metrics, String name) { return new CacheStats(metrics, name); } From 0f65783539d4110ec13478e7829c63acc0c0a631 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 15:05:05 -0500 Subject: [PATCH 06/16] enum map --- .../com/palantir/tritium/metrics/caffeine/CacheStats.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java index caabbb4d9..6e67a9e59 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java @@ -20,11 +20,11 @@ import com.github.benmanes.caffeine.cache.RemovalCause; import com.github.benmanes.caffeine.cache.stats.StatsCounter; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import com.palantir.tritium.metrics.cache.CacheMetrics; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.util.Arrays; import java.util.concurrent.atomic.LongAdder; -import java.util.function.Function; import java.util.function.Supplier; import org.checkerframework.checker.index.qual.NonNegative; @@ -86,7 +86,7 @@ private CacheStats(CacheMetrics metrics, String name) { this.loadFailureCounter = metrics.loadFailureCount(name); this.evictionsTotalCounter = metrics.evictionCount(name); this.evictionCounters = Arrays.stream(RemovalCause.values()) - .collect(ImmutableMap.toImmutableMap(Function.identity(), cause -> metrics.evictions() + .collect(Maps.toImmutableEnumMap(cause -> cause, cause -> metrics.evictions() .cache(name) .cause(cause.toString()) .build())); From 9f1a69887a3058a77b6184bfcf7ca8eac5f50615 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Thu, 7 Mar 2024 20:12:03 +0000 Subject: [PATCH 07/16] Add generated changelog entries --- changelog/@unreleased/pr-1897.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-1897.v2.yml diff --git a/changelog/@unreleased/pr-1897.v2.yml b/changelog/@unreleased/pr-1897.v2.yml new file mode 100644 index 000000000..ece909e18 --- /dev/null +++ b/changelog/@unreleased/pr-1897.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Initial Caffeine CacheStats recorder + links: + - https://github.com/palantir/tritium/pull/1897 From 7ad96c1c30847db8e8ab4a40a28279ddd646c657 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 15:24:42 -0500 Subject: [PATCH 08/16] private metric schema --- tritium-caffeine/build.gradle | 1 + .../tritium/metrics/caffeine/CacheStats.java | 24 +------------------ .../src/main/metrics/cache-metrics.yml | 4 ++-- .../caffeine/CaffeineCacheStatsTest.java | 9 ++++--- 4 files changed, 8 insertions(+), 30 deletions(-) rename {tritium-metrics => tritium-caffeine}/src/main/metrics/cache-metrics.yml (92%) diff --git a/tritium-caffeine/build.gradle b/tritium-caffeine/build.gradle index 5aecddc25..36ab16622 100644 --- a/tritium-caffeine/build.gradle +++ b/tritium-caffeine/build.gradle @@ -1,4 +1,5 @@ apply plugin: 'com.palantir.external-publish-jar' +apply plugin: 'com.palantir.metric-schema' dependencies { api 'com.github.ben-manes.caffeine:caffeine' diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java index 6e67a9e59..8153e1bac 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java @@ -21,7 +21,6 @@ import com.github.benmanes.caffeine.cache.stats.StatsCounter; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; -import com.palantir.tritium.metrics.cache.CacheMetrics; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.util.Arrays; import java.util.concurrent.atomic.LongAdder; @@ -32,12 +31,11 @@ public final class CacheStats implements StatsCounter, Supplier { private final String name; private final Counter hitCounter; private final Counter missCounter; - - private final LongAdder totalLoadNanos = new LongAdder(); private final Counter loadSuccessCounter; private final Counter loadFailureCounter; private final Counter evictionsTotalCounter; private final ImmutableMap evictionCounters; + private final LongAdder totalLoadNanos = new LongAdder(); /** * Creates a {@link CacheStats} instance that registers metrics for Caffeine cache statistics. @@ -58,26 +56,6 @@ public static CacheStats of(TaggedMetricRegistry taggedMetricRegistry, String na return new CacheStats(CacheMetrics.of(taggedMetricRegistry), name); } - /** - * Creates a {@link CacheStats} instance that registers metrics for Caffeine cache statistics. - *

- * Example usage for a {@link com.github.benmanes.caffeine.cache.Cache} or - * {@link com.github.benmanes.caffeine.cache.LoadingCache}: - * 

-     *     CacheMetrics metrics = CacheMetrics.of(taggedMetricRegistry);
-     *     LoadingCache<Integer, String> cache = Caffeine.newBuilder()
-     *             .recordStats(CacheStats.of(metrics, "your-cache-name"))
-     *             .build(key -> computeSomethingExpensive(key));
-     *
- * @param metrics metric instance to use for cache metrics - * @param name cache name - * @return Caffeine stats instance to register via - * {@link com.github.benmanes.caffeine.cache.Caffeine#recordStats(Supplier)}. - */ - public static CacheStats of(CacheMetrics metrics, String name) { - return new CacheStats(metrics, name); - } - private CacheStats(CacheMetrics metrics, String name) { this.name = name; this.hitCounter = metrics.hitCount(name); diff --git a/tritium-metrics/src/main/metrics/cache-metrics.yml b/tritium-caffeine/src/main/metrics/cache-metrics.yml similarity index 92% rename from tritium-metrics/src/main/metrics/cache-metrics.yml rename to tritium-caffeine/src/main/metrics/cache-metrics.yml index efac1959e..3d146df4d 100644 --- a/tritium-metrics/src/main/metrics/cache-metrics.yml +++ b/tritium-caffeine/src/main/metrics/cache-metrics.yml @@ -1,6 +1,6 @@ options: - javaPackage: com.palantir.tritium.metrics.cache - javaVisibility: public + javaPackage: com.palantir.tritium.metrics.caffeine + javaVisibility: packagePrivate namespaces: cache: docs: Cache statistic metrics 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 581f5dfd3..26d079fff 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 @@ -31,7 +31,6 @@ import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Multimaps; -import com.palantir.tritium.metrics.cache.CacheMetrics; import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry; import com.palantir.tritium.metrics.registry.MetricName; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; @@ -216,9 +215,8 @@ void registerCacheWithoutRecordingStatsTagged() { @Test void registerTaggedMetrics() { - CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry); Cache cache = Caffeine.newBuilder() - .recordStats(CacheStats.of(cacheMetrics, "test")) + .recordStats(CacheStats.of(taggedMetricRegistry, "test")) .maximumSize(2) .build(); assertThat(taggedMetricRegistry.getMetrics().keySet()) @@ -235,6 +233,7 @@ void registerTaggedMetrics() { "cache.load.success.count", "cache.load.failure.count"); + CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry); assertThat(cacheMetrics.hitCount("test").getCount()).isZero(); assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) .asInstanceOf(InstanceOfAssertFactories.LONG) @@ -265,9 +264,8 @@ void registerTaggedMetrics() { @Test void registerLoadingTaggedMetrics() { - CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry); LoadingCache cache = Caffeine.newBuilder() - .recordStats(CacheStats.of(cacheMetrics, "test")) + .recordStats(CacheStats.of(taggedMetricRegistry, "test")) .maximumSize(2) .build(mapping::apply); assertThat(taggedMetricRegistry.getMetrics().keySet()) @@ -294,6 +292,7 @@ void registerLoadingTaggedMetrics() { assertCounter(taggedMetricRegistry, "cache.hit.count").isOne(); assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3); + CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry); assertThat(cacheMetrics.hitCount("test").getCount()).isOne(); assertThat(cacheMetrics.missCount("test").getCount()).isEqualTo(3); assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) From ff78d58a48a1be500c78ff2ddf7be396b5187750 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 15:25:50 -0500 Subject: [PATCH 09/16] safe --- .../com/palantir/tritium/metrics/caffeine/CacheStats.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java index 8153e1bac..64fcee95a 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java @@ -21,6 +21,7 @@ import com.github.benmanes.caffeine.cache.stats.StatsCounter; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; +import com.palantir.logsafe.Safe; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.util.Arrays; import java.util.concurrent.atomic.LongAdder; @@ -52,11 +53,11 @@ public final class CacheStats implements StatsCounter, Supplier { * @return Caffeine stats instance to register via * {@link com.github.benmanes.caffeine.cache.Caffeine#recordStats(Supplier)}. */ - public static CacheStats of(TaggedMetricRegistry taggedMetricRegistry, String name) { + public static CacheStats of(TaggedMetricRegistry taggedMetricRegistry, @Safe String name) { return new CacheStats(CacheMetrics.of(taggedMetricRegistry), name); } - private CacheStats(CacheMetrics metrics, String name) { + private CacheStats(CacheMetrics metrics, @Safe String name) { this.name = name; this.hitCounter = metrics.hitCount(name); this.missCounter = metrics.missCount(name); From 031b770037d61300c0eb8ec27ff82b56fc6cb24d Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 15:31:56 -0500 Subject: [PATCH 10/16] deprecate older cache registration mechanisms --- .../com/palantir/tritium/metrics/caffeine/CacheStats.java | 4 ++-- .../tritium/metrics/caffeine/CaffeineCacheStats.java | 5 ++++- .../java/com/palantir/tritium/metrics/MetricRegistries.java | 3 +++ .../com/palantir/tritium/metrics/MetricRegistriesTest.java | 1 + 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java index 64fcee95a..c84f2fa0a 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java @@ -44,9 +44,9 @@ public final class CacheStats implements StatsCounter, Supplier { * Example usage for a {@link com.github.benmanes.caffeine.cache.Cache} or * {@link com.github.benmanes.caffeine.cache.LoadingCache}: * 
-     *     LoadingCache<Integer, String> cache = Caffeine.newBuilder()
+     *     LoadingCache<Integer, String> cache = Caffeine.newBuilder()
      *             .recordStats(CacheStats.of(taggedMetricRegistry, "your-cache-name"))
-     *             .build(key -> computeSomethingExpensive(key));
+     *             .build(key -> computeSomethingExpensive(key));
      *
* @param taggedMetricRegistry tagged metric registry to add cache metrics * @param name cache name 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 937898dea..7ffde67c5 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 @@ -33,6 +33,7 @@ import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.util.concurrent.TimeUnit; import java.util.function.Function; +import java.util.function.Supplier; public final class CaffeineCacheStats { @@ -43,7 +44,7 @@ private CaffeineCacheStats() {} /** * Register specified cache with the given metric registry. - * + *

* Callers should ensure that they have {@link Caffeine#recordStats() enabled stats recording} * {@code Caffeine.newBuilder().recordStats()} otherwise there are no cache metrics to register. * @@ -76,7 +77,9 @@ public static void registerCache(MetricRegistry registry, Cache cache, Str * @param registry metric registry * @param cache cache to instrument * @param name cache name + * @deprecated prefer {@link Caffeine#recordStats(Supplier)} and {@link CacheStats#of(TaggedMetricRegistry, String)} */ + @Deprecated public static void registerCache(TaggedMetricRegistry registry, Cache cache, @Safe String name) { checkNotNull(registry, "registry"); checkNotNull(cache, "cache"); diff --git a/tritium-metrics/src/main/java/com/palantir/tritium/metrics/MetricRegistries.java b/tritium-metrics/src/main/java/com/palantir/tritium/metrics/MetricRegistries.java index b6c2f8d28..53d309730 100644 --- a/tritium-metrics/src/main/java/com/palantir/tritium/metrics/MetricRegistries.java +++ b/tritium-metrics/src/main/java/com/palantir/tritium/metrics/MetricRegistries.java @@ -270,7 +270,10 @@ static void registerCache(MetricRegistry registry, Cache cache, @Safe Stri * @param cache cache to instrument * @param name cache name * @throws IllegalArgumentException if name is blank + * @deprecated Do not use Guava caches, they are outperformed by and harder to use than Caffeine caches. + * Prefer {@link Caffeine#recordStats(Supplier)} and {@link CacheStats#of(TaggedMetricRegistry, String)}. */ + @Deprecated // BanGuavaCaches @SuppressWarnings("BanGuavaCaches") // this implementation is explicitly for Guava caches public static void registerCache(TaggedMetricRegistry registry, Cache cache, @Safe String name) { checkNotNull(registry, "metric registry"); diff --git a/tritium-metrics/src/test/java/com/palantir/tritium/metrics/MetricRegistriesTest.java b/tritium-metrics/src/test/java/com/palantir/tritium/metrics/MetricRegistriesTest.java index fce417a10..eb840265b 100644 --- a/tritium-metrics/src/test/java/com/palantir/tritium/metrics/MetricRegistriesTest.java +++ b/tritium-metrics/src/test/java/com/palantir/tritium/metrics/MetricRegistriesTest.java @@ -289,6 +289,7 @@ void testNoStats() { } @Test + @SuppressWarnings("deprecation") // testing deprecated cache registration void registerCacheTaggedMetrics() throws ExecutionException { MetricRegistries.registerCache(taggedMetricRegistry, cache, "test"); assertThat(taggedMetricRegistry.getMetrics().keySet()) From c89fdc2fc4155725f8b7c0431aff5e84f7ced0d7 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 15:33:34 -0500 Subject: [PATCH 11/16] trigger cache cleanup & evictions in tests --- .../tritium/metrics/caffeine/CaffeineCacheStatsTest.java | 2 ++ 1 file changed, 2 insertions(+) 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 26d079fff..d7289aff6 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 @@ -250,6 +250,7 @@ void registerTaggedMetrics() { assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3); assertThat(cacheMetrics.hitCount("test").getCount()).isOne(); assertThat(cacheMetrics.missCount("test").getCount()).isEqualTo(3); + cache.cleanUp(); assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) .asInstanceOf(InstanceOfAssertFactories.LONG) .isOne(); @@ -295,6 +296,7 @@ void registerLoadingTaggedMetrics() { CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry); assertThat(cacheMetrics.hitCount("test").getCount()).isOne(); assertThat(cacheMetrics.missCount("test").getCount()).isEqualTo(3); + cache.cleanUp(); assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) .asInstanceOf(InstanceOfAssertFactories.LONG) .isOne(); From cd7e68b72a10f9600814d3ad87ddb9f14eb41ed0 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 15:36:18 -0500 Subject: [PATCH 12/16] update readme --- README.md | 8 ++++---- changelog/@unreleased/pr-1897.v2.yml | 15 ++++++++++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b5f32eb0d..d16152fdf 100644 --- a/README.md +++ b/README.md @@ -86,13 +86,13 @@ Service instrumentedService = Tritium.instrument(Service.class, import com.palantir.tritium.metrics.caffeine.CacheStats; TaggedMetricRegistry taggedMetricRegistry = ... -Cache cache = cacheStats.register(Caffeine.newBuilder() +Cache cache = Caffeine.newBuilder() .recordStats(CacheStats.of(taggedMetricRegistry, "unique-cache-name")) - .build()); + .build(); -LoadingCache loadingCache = loadingCacheStats.register(Caffeine.newBuilder() +LoadingCache loadingCache = Caffeine.newBuilder() .recordStats(CacheStats.of(taggedMetricRegistry, "unique-loading-cache-name")) - .build(key::length)); + .build(key::length); ``` ## Creating a metric registry with reservoirs backed by [HDR Histograms](https://hdrhistogram.github.io/HdrHistogram/). diff --git a/changelog/@unreleased/pr-1897.v2.yml b/changelog/@unreleased/pr-1897.v2.yml index ece909e18..52f601a0b 100644 --- a/changelog/@unreleased/pr-1897.v2.yml +++ b/changelog/@unreleased/pr-1897.v2.yml @@ -1,5 +1,18 @@ type: improvement improvement: - description: Initial Caffeine CacheStats recorder + description: | + Initial Caffeine CacheStats recorder + + Example usage: + ``` + TaggedMetricRegistry taggedMetricRegistry = ... + Cache cache = Caffeine.newBuilder() + .recordStats(CacheStats.of(taggedMetricRegistry, "unique-cache-name")) + .build(); + + LoadingCache loadingCache = Caffeine.newBuilder() + .recordStats(CacheStats.of(taggedMetricRegistry, "unique-loading-cache-name")) + .build(key::length); + ``` links: - https://github.com/palantir/tritium/pull/1897 From 4530a288073d4364b9ae4adb605b1cae49fbae54 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 15:41:37 -0500 Subject: [PATCH 13/16] meters --- .../tritium/metrics/caffeine/CacheStats.java | 53 ++++++++++--------- .../src/main/metrics/cache-metrics.yml | 14 ++--- .../caffeine/CaffeineCacheStatsTest.java | 24 +++++---- 3 files changed, 47 insertions(+), 44 deletions(-) diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java index c84f2fa0a..c22156a67 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java @@ -16,7 +16,8 @@ package com.palantir.tritium.metrics.caffeine; -import com.codahale.metrics.Counter; +import com.codahale.metrics.Counting; +import com.codahale.metrics.Meter; import com.github.benmanes.caffeine.cache.RemovalCause; import com.github.benmanes.caffeine.cache.stats.StatsCounter; import com.google.common.collect.ImmutableMap; @@ -30,12 +31,12 @@ public final class CacheStats implements StatsCounter, Supplier { private final String name; - private final Counter hitCounter; - private final Counter missCounter; - private final Counter loadSuccessCounter; - private final Counter loadFailureCounter; - private final Counter evictionsTotalCounter; - private final ImmutableMap evictionCounters; + private final Meter hitMeter; + private final Meter missMeter; + private final Meter loadSuccessMeter; + private final Meter loadFailureMeter; + private final Meter evictionsTotalMeter; + private final ImmutableMap evictionMeters; private final LongAdder totalLoadNanos = new LongAdder(); /** @@ -59,12 +60,12 @@ public static CacheStats of(TaggedMetricRegistry taggedMetricRegistry, @Safe Str private CacheStats(CacheMetrics metrics, @Safe String name) { this.name = name; - this.hitCounter = metrics.hitCount(name); - this.missCounter = metrics.missCount(name); - this.loadSuccessCounter = metrics.loadSuccessCount(name); - this.loadFailureCounter = metrics.loadFailureCount(name); - this.evictionsTotalCounter = metrics.evictionCount(name); - this.evictionCounters = Arrays.stream(RemovalCause.values()) + this.hitMeter = metrics.hitCount(name); + this.missMeter = metrics.missCount(name); + this.loadSuccessMeter = metrics.loadSuccessCount(name); + this.loadFailureMeter = metrics.loadFailureCount(name); + this.evictionsTotalMeter = metrics.evictionCount(name); + this.evictionMeters = Arrays.stream(RemovalCause.values()) .collect(Maps.toImmutableEnumMap(cause -> cause, cause -> metrics.evictions() .cache(name) .cause(cause.toString()) @@ -78,45 +79,45 @@ public StatsCounter get() { @Override public void recordHits(@NonNegative int count) { - hitCounter.inc(count); + hitMeter.mark(count); } @Override public void recordMisses(@NonNegative int count) { - missCounter.inc(count); + missMeter.mark(count); } @Override public void recordLoadSuccess(@NonNegative long loadTime) { - loadSuccessCounter.inc(); + loadSuccessMeter.mark(); totalLoadNanos.add(loadTime); } @Override public void recordLoadFailure(@NonNegative long loadTime) { - loadFailureCounter.inc(); + loadFailureMeter.mark(); totalLoadNanos.add(loadTime); } @Override public void recordEviction(@NonNegative int weight, RemovalCause cause) { - Counter counter = evictionCounters.get(cause); + Meter counter = evictionMeters.get(cause); if (counter != null) { - counter.inc(weight); + counter.mark(weight); } - evictionsTotalCounter.inc(weight); + evictionsTotalMeter.mark(weight); } @Override public com.github.benmanes.caffeine.cache.stats.CacheStats snapshot() { return com.github.benmanes.caffeine.cache.stats.CacheStats.of( - hitCounter.getCount(), - missCounter.getCount(), - loadSuccessCounter.getCount(), - loadFailureCounter.getCount(), + hitMeter.getCount(), + missMeter.getCount(), + loadSuccessMeter.getCount(), + loadFailureMeter.getCount(), totalLoadNanos.sum(), - evictionsTotalCounter.getCount(), - evictionCounters.values().stream().mapToLong(Counter::getCount).sum()); + evictionsTotalMeter.getCount(), + evictionMeters.values().stream().mapToLong(Counting::getCount).sum()); } @Override diff --git a/tritium-caffeine/src/main/metrics/cache-metrics.yml b/tritium-caffeine/src/main/metrics/cache-metrics.yml index 3d146df4d..8de75c0fd 100644 --- a/tritium-caffeine/src/main/metrics/cache-metrics.yml +++ b/tritium-caffeine/src/main/metrics/cache-metrics.yml @@ -6,31 +6,31 @@ namespaces: docs: Cache statistic metrics metrics: hit.count: - type: counter + type: meter tags: [cache] docs: Count of cache hits miss.count: - type: counter + type: meter tags: [cache] docs: Count of cache misses load.success.count: - type: counter + type: meter tags: [cache] docs: Count of successful cache loads load.failure.count: - type: counter + type: meter tags: [cache] docs: Count of failed cache loads evictions: - type: counter + type: meter tags: [cache, cause] docs: Count of evicted entries by cause eviction.count: - type: counter + type: meter tags: [cache] docs: Total count of evicted entries stats.disabled: - type: counter + type: meter tags: [cache] docs: | Registered cache does not have stats recording enabled, stats will always be zero. 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 d7289aff6..8cc20ce1d 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 @@ -23,7 +23,9 @@ import com.codahale.metrics.ConsoleReporter; import com.codahale.metrics.Counter; +import com.codahale.metrics.Counting; 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; @@ -238,16 +240,16 @@ void registerTaggedMetrics() { assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) .asInstanceOf(InstanceOfAssertFactories.LONG) .isZero(); - assertCounter(taggedMetricRegistry, "cache.hit.count").isZero(); - assertCounter(taggedMetricRegistry, "cache.miss.count").isZero(); + assertMeter(taggedMetricRegistry, "cache.hit.count").isZero(); + assertMeter(taggedMetricRegistry, "cache.miss.count").isZero(); assertThat(cache.get(0, mapping)).isEqualTo("0"); assertThat(cache.get(1, mapping)).isEqualTo("1"); assertThat(cache.get(2, mapping)).isEqualTo("2"); assertThat(cache.get(1, mapping)).isEqualTo("1"); - assertCounter(taggedMetricRegistry, "cache.hit.count").isOne(); - assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3); + assertMeter(taggedMetricRegistry, "cache.hit.count").isOne(); + assertMeter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3); assertThat(cacheMetrics.hitCount("test").getCount()).isOne(); assertThat(cacheMetrics.missCount("test").getCount()).isEqualTo(3); cache.cleanUp(); @@ -283,16 +285,16 @@ void registerLoadingTaggedMetrics() { "cache.load.success.count", "cache.load.failure.count"); - assertCounter(taggedMetricRegistry, "cache.hit.count").isZero(); - assertCounter(taggedMetricRegistry, "cache.miss.count").isZero(); + assertMeter(taggedMetricRegistry, "cache.hit.count").isZero(); + assertMeter(taggedMetricRegistry, "cache.miss.count").isZero(); assertThat(cache.get(0)).isEqualTo("0"); assertThat(cache.get(1)).isEqualTo("1"); assertThat(cache.get(2)).isEqualTo("2"); assertThat(cache.get(1)).isEqualTo("1"); - assertCounter(taggedMetricRegistry, "cache.hit.count").isOne(); - assertCounter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3); + assertMeter(taggedMetricRegistry, "cache.hit.count").isOne(); + assertMeter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3); CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry); assertThat(cacheMetrics.hitCount("test").getCount()).isOne(); assertThat(cacheMetrics.missCount("test").getCount()).isEqualTo(3); @@ -313,9 +315,9 @@ static AbstractObjectAssert assertGauge(TaggedMetricRegistry taggedMe return assertMetric(taggedMetricRegistry, Gauge.class, name).extracting(Gauge::getValue); } - static AbstractLongAssert assertCounter(TaggedMetricRegistry taggedMetricRegistry, String name) { - return assertMetric(taggedMetricRegistry, Counter.class, name) - .extracting(Counter::getCount) + static AbstractLongAssert assertMeter(TaggedMetricRegistry taggedMetricRegistry, String name) { + return assertMetric(taggedMetricRegistry, Meter.class, name) + .extracting(Counting::getCount) .asInstanceOf(InstanceOfAssertFactories.LONG); } From af50aaf7de09b251ee77136890b83fc925c2c405 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 15:52:24 -0500 Subject: [PATCH 14/16] Cleanup metric names --- .../tritium/metrics/caffeine/CacheStats.java | 13 ++-- .../src/main/metrics/cache-metrics.yml | 17 +++-- .../caffeine/CaffeineCacheStatsTest.java | 70 ++++++++++++------- 3 files changed, 60 insertions(+), 40 deletions(-) diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java index c22156a67..3df13ff6a 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.palantir.logsafe.Safe; +import com.palantir.tritium.metrics.caffeine.CacheMetrics.Load_Result; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.util.Arrays; import java.util.concurrent.atomic.LongAdder; @@ -60,11 +61,13 @@ public static CacheStats of(TaggedMetricRegistry taggedMetricRegistry, @Safe Str private CacheStats(CacheMetrics metrics, @Safe String name) { this.name = name; - this.hitMeter = metrics.hitCount(name); - this.missMeter = metrics.missCount(name); - this.loadSuccessMeter = metrics.loadSuccessCount(name); - this.loadFailureMeter = metrics.loadFailureCount(name); - this.evictionsTotalMeter = metrics.evictionCount(name); + this.hitMeter = metrics.hit(name); + this.missMeter = metrics.miss(name); + this.loadSuccessMeter = + metrics.load().cache(name).result(Load_Result.SUCCESS).build(); + this.loadFailureMeter = + metrics.load().cache(name).result(Load_Result.FAILURE).build(); + this.evictionsTotalMeter = metrics.eviction(name); this.evictionMeters = Arrays.stream(RemovalCause.values()) .collect(Maps.toImmutableEnumMap(cause -> cause, cause -> metrics.evictions() .cache(name) diff --git a/tritium-caffeine/src/main/metrics/cache-metrics.yml b/tritium-caffeine/src/main/metrics/cache-metrics.yml index 8de75c0fd..4ff989c69 100644 --- a/tritium-caffeine/src/main/metrics/cache-metrics.yml +++ b/tritium-caffeine/src/main/metrics/cache-metrics.yml @@ -5,27 +5,26 @@ namespaces: cache: docs: Cache statistic metrics metrics: - hit.count: + hit: type: meter tags: [cache] docs: Count of cache hits - miss.count: + miss: type: meter tags: [cache] docs: Count of cache misses - load.success.count: + load: type: meter - tags: [cache] + tags: + - name: cache + - name: result + values: [success, failure] docs: Count of successful cache loads - load.failure.count: - type: meter - tags: [cache] - docs: Count of failed cache loads evictions: type: meter tags: [cache, cause] docs: Count of evicted entries by cause - eviction.count: + eviction: type: meter tags: [cache] docs: Total count of evicted entries 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 8cc20ce1d..482b0275c 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 @@ -224,35 +224,42 @@ void registerTaggedMetrics() { assertThat(taggedMetricRegistry.getMetrics().keySet()) .extracting(MetricName::safeName) .containsExactlyInAnyOrder( - "cache.hit.count", - "cache.miss.count", - "cache.eviction.count", + "cache.hit", + "cache.miss", + "cache.eviction", "cache.evictions", // RemovalCause.EXPLICIT "cache.evictions", // RemovalCause.REPLACED "cache.evictions", // RemovalCause.COLLECTED "cache.evictions", // RemovalCause.EXPIRED "cache.evictions", // RemovalCause.SIZE - "cache.load.success.count", - "cache.load.failure.count"); + "cache.load", // success + "cache.load" // failure + ); CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry); - assertThat(cacheMetrics.hitCount("test").getCount()).isZero(); assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) .asInstanceOf(InstanceOfAssertFactories.LONG) .isZero(); - assertMeter(taggedMetricRegistry, "cache.hit.count").isZero(); - assertMeter(taggedMetricRegistry, "cache.miss.count").isZero(); + assertMeter(taggedMetricRegistry, "cache.hit") + .isEqualTo(cacheMetrics.hit("test").getCount()) + .isZero(); + assertMeter(taggedMetricRegistry, "cache.miss") + .isEqualTo(cacheMetrics.miss("test").getCount()) + .isZero(); assertThat(cache.get(0, mapping)).isEqualTo("0"); assertThat(cache.get(1, mapping)).isEqualTo("1"); assertThat(cache.get(2, mapping)).isEqualTo("2"); assertThat(cache.get(1, mapping)).isEqualTo("1"); - assertMeter(taggedMetricRegistry, "cache.hit.count").isOne(); - assertMeter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3); - assertThat(cacheMetrics.hitCount("test").getCount()).isOne(); - assertThat(cacheMetrics.missCount("test").getCount()).isEqualTo(3); - cache.cleanUp(); + assertMeter(taggedMetricRegistry, "cache.hit") + .isEqualTo(cacheMetrics.hit("test").getCount()) + .isOne(); + assertMeter(taggedMetricRegistry, "cache.miss") + .isEqualTo(cacheMetrics.miss("test").getCount()) + .isEqualTo(3); + + cache.cleanUp(); // force eviction processing assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) .asInstanceOf(InstanceOfAssertFactories.LONG) .isOne(); @@ -274,31 +281,42 @@ void registerLoadingTaggedMetrics() { assertThat(taggedMetricRegistry.getMetrics().keySet()) .extracting(MetricName::safeName) .containsExactlyInAnyOrder( - "cache.hit.count", - "cache.miss.count", - "cache.eviction.count", + "cache.hit", + "cache.miss", + "cache.eviction", "cache.evictions", // RemovalCause.EXPLICIT "cache.evictions", // RemovalCause.REPLACED "cache.evictions", // RemovalCause.COLLECTED "cache.evictions", // RemovalCause.EXPIRED "cache.evictions", // RemovalCause.SIZE - "cache.load.success.count", - "cache.load.failure.count"); + "cache.load", // success + "cache.load" // failure + ); - assertMeter(taggedMetricRegistry, "cache.hit.count").isZero(); - assertMeter(taggedMetricRegistry, "cache.miss.count").isZero(); + CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry); + assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) + .asInstanceOf(InstanceOfAssertFactories.LONG) + .isZero(); + assertMeter(taggedMetricRegistry, "cache.hit") + .isEqualTo(cacheMetrics.hit("test").getCount()) + .isZero(); + assertMeter(taggedMetricRegistry, "cache.miss") + .isEqualTo(cacheMetrics.miss("test").getCount()) + .isZero(); assertThat(cache.get(0)).isEqualTo("0"); assertThat(cache.get(1)).isEqualTo("1"); assertThat(cache.get(2)).isEqualTo("2"); assertThat(cache.get(1)).isEqualTo("1"); - assertMeter(taggedMetricRegistry, "cache.hit.count").isOne(); - assertMeter(taggedMetricRegistry, "cache.miss.count").isEqualTo(3); - CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry); - assertThat(cacheMetrics.hitCount("test").getCount()).isOne(); - assertThat(cacheMetrics.missCount("test").getCount()).isEqualTo(3); - cache.cleanUp(); + assertMeter(taggedMetricRegistry, "cache.hit") + .isEqualTo(cacheMetrics.hit("test").getCount()) + .isOne(); + assertMeter(taggedMetricRegistry, "cache.miss") + .isEqualTo(cacheMetrics.miss("test").getCount()) + .isEqualTo(3); + + cache.cleanUp(); // force eviction processing assertThat(cacheMetrics.evictions().cache("test").cause("SIZE").build().getCount()) .asInstanceOf(InstanceOfAssertFactories.LONG) .isOne(); From 1fd78b9dcf52ca0b91e6f8406912a3c22f6afd12 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 15:57:34 -0500 Subject: [PATCH 15/16] relax deprecation for now --- .../tritium/metrics/caffeine/CaffeineCacheStats.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 7ffde67c5..6cdf205fb 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 @@ -70,16 +70,17 @@ public static void registerCache(MetricRegistry registry, Cache cache, Str /** * Register specified cache with the given metric registry. - * + *

* Callers should ensure that they have {@link Caffeine#recordStats() enabled stats recording} * {@code Caffeine.newBuilder().recordStats()} otherwise there are no cache metrics to register. * * @param registry metric registry * @param cache cache to instrument * @param name cache name - * @deprecated prefer {@link Caffeine#recordStats(Supplier)} and {@link CacheStats#of(TaggedMetricRegistry, String)} + *

+ * Soon to be deprecated, prefer {@link Caffeine#recordStats(Supplier)} and {@link CacheStats#of(TaggedMetricRegistry, String)} */ - @Deprecated + // Soon to be @Deprecated public static void registerCache(TaggedMetricRegistry registry, Cache cache, @Safe String name) { checkNotNull(registry, "registry"); checkNotNull(cache, "cache"); From 1bcf5272ffab2823d660fead9c6aceacbceff258 Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Thu, 7 Mar 2024 16:13:28 -0500 Subject: [PATCH 16/16] load timer --- .../tritium/metrics/caffeine/CacheStats.java | 18 ++++++++++-------- .../src/main/metrics/cache-metrics.yml | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java index 3df13ff6a..f7273aaba 100644 --- a/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java +++ b/tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java @@ -18,6 +18,7 @@ import com.codahale.metrics.Counting; import com.codahale.metrics.Meter; +import com.codahale.metrics.Timer; import com.github.benmanes.caffeine.cache.RemovalCause; import com.github.benmanes.caffeine.cache.stats.StatsCounter; import com.google.common.collect.ImmutableMap; @@ -26,6 +27,7 @@ import com.palantir.tritium.metrics.caffeine.CacheMetrics.Load_Result; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.util.Arrays; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.LongAdder; import java.util.function.Supplier; import org.checkerframework.checker.index.qual.NonNegative; @@ -34,8 +36,8 @@ public final class CacheStats implements StatsCounter, Supplier { private final String name; private final Meter hitMeter; private final Meter missMeter; - private final Meter loadSuccessMeter; - private final Meter loadFailureMeter; + private final Timer loadSuccessTimer; + private final Timer loadFailureTimer; private final Meter evictionsTotalMeter; private final ImmutableMap evictionMeters; private final LongAdder totalLoadNanos = new LongAdder(); @@ -63,9 +65,9 @@ private CacheStats(CacheMetrics metrics, @Safe String name) { this.name = name; this.hitMeter = metrics.hit(name); this.missMeter = metrics.miss(name); - this.loadSuccessMeter = + this.loadSuccessTimer = metrics.load().cache(name).result(Load_Result.SUCCESS).build(); - this.loadFailureMeter = + this.loadFailureTimer = metrics.load().cache(name).result(Load_Result.FAILURE).build(); this.evictionsTotalMeter = metrics.eviction(name); this.evictionMeters = Arrays.stream(RemovalCause.values()) @@ -92,13 +94,13 @@ public void recordMisses(@NonNegative int count) { @Override public void recordLoadSuccess(@NonNegative long loadTime) { - loadSuccessMeter.mark(); + loadSuccessTimer.update(loadTime, TimeUnit.NANOSECONDS); totalLoadNanos.add(loadTime); } @Override public void recordLoadFailure(@NonNegative long loadTime) { - loadFailureMeter.mark(); + loadFailureTimer.update(loadTime, TimeUnit.NANOSECONDS); totalLoadNanos.add(loadTime); } @@ -116,8 +118,8 @@ public com.github.benmanes.caffeine.cache.stats.CacheStats snapshot() { return com.github.benmanes.caffeine.cache.stats.CacheStats.of( hitMeter.getCount(), missMeter.getCount(), - loadSuccessMeter.getCount(), - loadFailureMeter.getCount(), + loadSuccessTimer.getCount(), + loadFailureTimer.getCount(), totalLoadNanos.sum(), evictionsTotalMeter.getCount(), evictionMeters.values().stream().mapToLong(Counting::getCount).sum()); diff --git a/tritium-caffeine/src/main/metrics/cache-metrics.yml b/tritium-caffeine/src/main/metrics/cache-metrics.yml index 4ff989c69..0dde1da6a 100644 --- a/tritium-caffeine/src/main/metrics/cache-metrics.yml +++ b/tritium-caffeine/src/main/metrics/cache-metrics.yml @@ -14,7 +14,7 @@ namespaces: tags: [cache] docs: Count of cache misses load: - type: meter + type: timer tags: - name: cache - name: result