Skip to content

Commit

Permalink
Combine hit and miss cache metrics into single tagged metric (#1921)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pkoenig10
pkoenig10 authored Apr 1, 2024
1 parent 8a13350 commit 8687863
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 108 deletions.
6 changes: 6 additions & 0 deletions changelog/@unreleased/pr-1921.v2.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type: improvement
improvement:
description: The `cache.hit` and `cache.miss` metrics have been combined into a
single `cache.request` metric tagged by `result`.
links:
- https://github.com/palantir/tritium/pull/1921
6 changes: 4 additions & 2 deletions tritium-caffeine/src/main/java/com/palantir/tritium/metrics/caffeine/CacheStats.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
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.caffeine.CacheMetrics.Request_Result;
import com.palantir.tritium.metrics.registry.TaggedMetricRegistry;
import java.util.Arrays;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -109,8 +110,9 @@ public <K, V, C extends Cache<K, V>> C register(Function<CacheStats, C> cacheFac
private CacheStats(CacheMetrics metrics, @Safe String name) {
this.metrics = metrics;
this.name = name;
this.hitMeter = metrics.hit(name);
this.missMeter = metrics.miss(name);
this.hitMeter = metrics.request().cache(name).result(Request_Result.HIT).build();
this.missMeter =
metrics.request().cache(name).result(Request_Result.MISS).build();
this.loadSuccessTimer =
metrics.load().cache(name).result(Load_Result.SUCCESS).build();
this.loadFailureTimer =
Expand Down
20 changes: 6 additions & 14 deletions ...ffeine/src/main/metrics/cache-metrics.yml → tritium-caffeine/src/main/metrics/cache.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ namespaces:
cache:
docs: Cache statistic metrics
metrics:
hit:
request:
type: meter
tags: [cache]
docs: Count of cache hits
miss:
type: meter
tags: [cache]
docs: Count of cache misses
tags:
- name: cache
- name: result
values: [hit, miss]
docs: Count of cache requests
load:
type: timer
tags:
Expand All @@ -28,13 +27,6 @@ namespaces:
type: meter
tags: [cache, cause]
docs: Count of evicted weights
stats.disabled:
type: meter
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()
estimated.size:
type: gauge
tags: [cache]
Expand Down
197 changes: 105 additions & 92 deletions ...-caffeine/src/test/java/com/palantir/tritium/metrics/caffeine/CaffeineCacheStatsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,37 +18,26 @@

import static com.palantir.logsafe.testing.Assertions.assertThatLoggableExceptionThrownBy;
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;

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;
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.logsafe.exceptions.SafeIllegalStateException;
import com.palantir.tritium.metrics.caffeine.CacheMetrics.Request_Result;
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.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;

Expand Down Expand Up @@ -159,9 +148,27 @@ void registerCacheTaggedMetrics() {
"cache.load.average.millis");

await().atMost(Duration.ofSeconds(5)).untilAsserted(() -> {
assertGauge(taggedMetricRegistry, "cache.request.count").isEqualTo(0L);
assertGauge(taggedMetricRegistry, "cache.hit.count").isEqualTo(0L);
assertGauge(taggedMetricRegistry, "cache.miss.count").isEqualTo(0L);
assertGauge(
taggedMetricRegistry,
MetricName.builder()
.safeName("cache.request.count")
.putSafeTags("cache", "test")
.build())
.isEqualTo(0L);
assertGauge(
taggedMetricRegistry,
MetricName.builder()
.safeName("cache.hit.count")
.putSafeTags("cache", "test")
.build())
.isEqualTo(0L);
assertGauge(
taggedMetricRegistry,
MetricName.builder()
.safeName("cache.miss.count")
.putSafeTags("cache", "test")
.build())
.isEqualTo(0L);
});

assertThat(cache.get(0, mapping)).isEqualTo("0");
Expand All @@ -171,10 +178,34 @@ void registerCacheTaggedMetrics() {

await().atMost(Duration.ofSeconds(10)).untilAsserted(() -> {
// await to avoid flakes as gauges may be memoized
assertGauge(taggedMetricRegistry, "cache.request.count").isEqualTo(4L);
assertGauge(taggedMetricRegistry, "cache.hit.count").isEqualTo(1L);
assertGauge(taggedMetricRegistry, "cache.miss.count").isEqualTo(3L);
assertGauge(taggedMetricRegistry, "cache.hit.ratio").isEqualTo(0.25);
assertGauge(
taggedMetricRegistry,
MetricName.builder()
.safeName("cache.request.count")
.putSafeTags("cache", "test")
.build())
.isEqualTo(4L);
assertGauge(
taggedMetricRegistry,
MetricName.builder()
.safeName("cache.hit.count")
.putSafeTags("cache", "test")
.build())
.isEqualTo(1L);
assertGauge(
taggedMetricRegistry,
MetricName.builder()
.safeName("cache.miss.count")
.putSafeTags("cache", "test")
.build())
.isEqualTo(3L);
assertGauge(
taggedMetricRegistry,
MetricName.builder()
.safeName("cache.hit.ratio")
.putSafeTags("cache", "test")
.build())
.isEqualTo(0.25);

assertThat(taggedMetricRegistry.getMetrics())
.extractingByKey(MetricName.builder()
Expand Down Expand Up @@ -226,8 +257,8 @@ void registerTaggedMetrics() {
assertThat(taggedMetricRegistry.getMetrics().keySet())
.extracting(MetricName::safeName)
.containsExactlyInAnyOrder(
"cache.hit",
"cache.miss",
"cache.request", // hit
"cache.request", // miss
"cache.eviction", // RemovalCause.EXPLICIT
"cache.eviction", // RemovalCause.REPLACED
"cache.eviction", // RemovalCause.COLLECTED
Expand All @@ -247,23 +278,39 @@ void registerTaggedMetrics() {
CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry);
assertThat(cacheMetrics.eviction().cache("test").cause("SIZE").build().getCount())
.isZero();
assertMeter(taggedMetricRegistry, "cache.hit")
.isEqualTo(cacheMetrics.hit("test").getCount())
assertThat(cacheMetrics
.request()
.cache("test")
.result(Request_Result.HIT)
.build()
.getCount())
.isZero();
assertMeter(taggedMetricRegistry, "cache.miss")
.isEqualTo(cacheMetrics.miss("test").getCount())
assertThat(cacheMetrics
.request()
.cache("test")
.result(Request_Result.MISS)
.build()
.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")
.isEqualTo(cacheMetrics.hit("test").getCount())
assertThat(cacheMetrics
.request()
.cache("test")
.result(Request_Result.HIT)
.build()
.getCount())
.isOne();
assertMeter(taggedMetricRegistry, "cache.miss")
.isEqualTo(cacheMetrics.miss("test").getCount())
assertThat(cacheMetrics
.request()
.cache("test")
.result(Request_Result.MISS)
.build()
.getCount())
.isEqualTo(3);

cache.cleanUp(); // force eviction processing
Expand All @@ -286,8 +333,8 @@ void registerLoadingTaggedMetrics() {
assertThat(taggedMetricRegistry.getMetrics().keySet())
.extracting(MetricName::safeName)
.containsExactlyInAnyOrder(
"cache.hit",
"cache.miss",
"cache.request", // hit
"cache.request", // miss
"cache.eviction", // RemovalCause.EXPLICIT
"cache.eviction", // RemovalCause.REPLACED
"cache.eviction", // RemovalCause.COLLECTED
Expand All @@ -307,35 +354,44 @@ void registerLoadingTaggedMetrics() {
CacheMetrics cacheMetrics = CacheMetrics.of(taggedMetricRegistry);
assertThat(cacheMetrics.eviction().cache("test").cause("SIZE").build().getCount())
.isZero();
assertMeter(taggedMetricRegistry, "cache.hit")
.isEqualTo(cacheMetrics.hit("test").getCount())
assertThat(cacheMetrics
.request()
.cache("test")
.result(Request_Result.HIT)
.build()
.getCount())
.isZero();
assertMeter(taggedMetricRegistry, "cache.miss")
.isEqualTo(cacheMetrics.miss("test").getCount())
assertThat(cacheMetrics
.request()
.cache("test")
.result(Request_Result.MISS)
.build()
.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")
.isEqualTo(cacheMetrics.hit("test").getCount())
assertThat(cacheMetrics
.request()
.cache("test")
.result(Request_Result.HIT)
.build()
.getCount())
.isOne();
assertMeter(taggedMetricRegistry, "cache.miss")
.isEqualTo(cacheMetrics.miss("test").getCount())
assertThat(cacheMetrics
.request()
.cache("test")
.result(Request_Result.MISS)
.build()
.getCount())
.isEqualTo(3);

cache.cleanUp(); // force eviction processing
assertThat(cacheMetrics.eviction().cache("test").cause("SIZE").build().getCount())
.isOne();

assertThat(taggedMetricRegistry.getMetrics())
.extractingByKey(MetricName.builder()
.safeName("cache.stats.disabled")
.putSafeTags("cache", "test")
.build())
.isNull();
}

@Test
Expand All @@ -350,51 +406,8 @@ void registerWithoutStatsRecording() {
.hasNoArgs();
}

static AbstractObjectAssert<?, Object> assertGauge(TaggedMetricRegistry taggedMetricRegistry, String name) {
return assertMetric(taggedMetricRegistry, Gauge.class, name).extracting(Gauge::getValue);
}

static AbstractLongAssert<?> assertMeter(TaggedMetricRegistry taggedMetricRegistry, String name) {
return assertMetric(taggedMetricRegistry, Meter.class, name)
.extracting(Counting::getCount)
.asInstanceOf(InstanceOfAssertFactories.LONG);
}

static <T extends Metric> ObjectAssert<T> assertMetric(
TaggedMetricRegistry taggedMetricRegistry, Class<T> clazz, String name) {
T metric = getMetric(taggedMetricRegistry, clazz, name);
return assertThat(metric)
.as("metric '%s': '%s'", name, metric)
.isNotNull()
.asInstanceOf(type(clazz));
}

private static <T extends Metric> T getMetric(TaggedMetricRegistry metrics, Class<T> clazz, String name) {
Optional<Entry<MetricName, Metric>> metric = metrics.getMetrics().entrySet().stream()
.filter(e -> name.equals(e.getKey().safeName()))
.filter(e -> clazz.isInstance(e.getValue()))
.findFirst();
if (metric.isEmpty()) {
Map<String, Set<String>> 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());
static AbstractObjectAssert<?, Object> assertGauge(
TaggedMetricRegistry taggedMetricRegistry, MetricName metricName) {
return assertThat(taggedMetricRegistry.gauge(metricName)).get().extracting(Gauge::getValue);
}
}

0 comments on commit 8687863

Please sign in to comment.