Skip to content

Commit

Permalink
Check that stats recording is enabled when registering a cache (#1922)
Browse files Browse the repository at this point in the history
  • Loading branch information
pkoenig10 authored Mar 28, 2024
1 parent ef613d6 commit b1ae2bb
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 3 deletions.
1 change: 1 addition & 0 deletions tritium-caffeine/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ dependencies {
implementation 'io.dropwizard.metrics:metrics-core'
implementation 'org.checkerframework:checker-qual'

testImplementation 'com.palantir.safe-logging:preconditions-assertj'
testImplementation 'org.assertj:assertj-core'
testImplementation 'org.awaitility:awaitility'
testImplementation 'org.junit.jupiter:junit-jupiter'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.palantir.tritium.metrics.caffeine;

import static com.palantir.logsafe.Preconditions.checkState;

import com.codahale.metrics.Counting;
import com.codahale.metrics.Meter;
import com.codahale.metrics.Timer;
Expand Down Expand Up @@ -86,6 +88,11 @@ public static CacheStats of(TaggedMetricRegistry taggedMetricRegistry, @Safe Str
public <K, V, C extends Cache<K, V>> C register(Function<CacheStats, C> cacheFactory) {
C cache = cacheFactory.apply(this);

checkState(
cache.policy().isRecordingStats(),
"Registered cache is not recording stats. Registered caches must enabled stats recording with "
+ ".recordStats(stats).");

metrics.estimatedSize().cache(name).build(cache::estimatedSize);
metrics.weightedSize().cache(name).build(() -> cache.policy()
.eviction()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.palantir.tritium.metrics.caffeine;

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;
Expand All @@ -33,6 +34,7 @@
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.registry.DefaultTaggedMetricRegistry;
import com.palantir.tritium.metrics.registry.MetricName;
import com.palantir.tritium.metrics.registry.TaggedMetricRegistry;
Expand Down Expand Up @@ -336,6 +338,18 @@ void registerLoadingTaggedMetrics() {
.isNull();
}

@Test
void registerWithoutStatsRecording() {
CacheStats cacheStats = CacheStats.of(taggedMetricRegistry, "test");

assertThatLoggableExceptionThrownBy(() ->
cacheStats.register(_stats -> Caffeine.newBuilder().build()))
.isInstanceOf(SafeIllegalStateException.class)
.hasLogMessage("Registered cache is not recording stats. Registered caches must enabled stats "
+ "recording with .recordStats(stats).")
.hasNoArgs();
}

static AbstractObjectAssert<?, Object> assertGauge(TaggedMetricRegistry taggedMetricRegistry, String name) {
return assertMetric(taggedMetricRegistry, Gauge.class, name).extracting(Gauge::getValue);
}
Expand Down
7 changes: 4 additions & 3 deletions versions.lock
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ com.palantir.nylon:nylon-threads:0.4.0 (1 constraints: 0605f735)
com.palantir.safe-logging:logger:3.7.0 (3 constraints: f31f6c22)
com.palantir.safe-logging:logger-slf4j:3.7.0 (1 constraints: 050e6842)
com.palantir.safe-logging:logger-spi:3.7.0 (2 constraints: 191ea27b)
com.palantir.safe-logging:preconditions:3.7.0 (3 constraints: f31f6c22)
com.palantir.safe-logging:safe-logging:3.7.0 (6 constraints: 794ed480)
com.palantir.safe-logging:preconditions:3.7.0 (4 constraints: 2134beac)
com.palantir.safe-logging:safe-logging:3.7.0 (7 constraints: a7621b39)
com.palantir.tracing:tracing:6.18.0 (1 constraints: 41055f3b)
com.palantir.tracing:tracing-api:6.18.0 (2 constraints: 17121d19)
com.squareup:javapoet:1.13.0 (3 constraints: 9e27444d)
Expand All @@ -38,6 +38,7 @@ com.google.auto.value:auto-value:1.10 (1 constraints: e711f8e8)
com.google.auto.value:auto-value-annotations:1.8.1 (1 constraints: 620a29b9)
com.google.testing.compile:compile-testing:0.21.0 (1 constraints: 35052a3b)
com.google.truth:truth:1.1.3 (1 constraints: 18120efb)
com.palantir.safe-logging:preconditions-assertj:3.7.0 (1 constraints: 0c050f36)
com.squareup.okhttp3:okhttp:4.12.0 (1 constraints: 3905413b)
com.squareup.okio:okio:3.6.0 (1 constraints: 530c38fd)
com.squareup.okio:okio-jvm:3.6.0 (1 constraints: 500ad3b9)
Expand All @@ -52,7 +53,7 @@ net.jqwik:jqwik-web:1.8.4 (1 constraints: a007fe6c)
net.sf.jopt-simple:jopt-simple:5.0.4 (1 constraints: be0ad6cc)
org.apache.commons:commons-math3:3.6.1 (1 constraints: bf0adbcc)
org.apiguardian:apiguardian-api:1.1.2 (10 constraints: 4f819858)
org.assertj:assertj-core:3.25.3 (1 constraints: 3f054b3b)
org.assertj:assertj-core:3.25.3 (2 constraints: 9e19a0df)
org.awaitility:awaitility:4.2.1 (1 constraints: 09050636)
org.hamcrest:hamcrest:2.1 (1 constraints: 6f0b2cce)
org.hamcrest:hamcrest-core:1.3 (1 constraints: cc05fe3f)
Expand Down

0 comments on commit b1ae2bb

Please sign in to comment.