From 77560e49ce8806992a7c30b393082a00f30d896a Mon Sep 17 00:00:00 2001 From: Gwenneg Lepage Date: Thu, 18 Apr 2024 19:49:19 +0200 Subject: [PATCH] Do not increment metrics on CaffeineCache#getIfPresent call --- .../runtime/caffeine/CaffeineCacheImpl.java | 3 - .../it/cache/CaffeineCacheWrapper.java | 26 +++++++++ .../src/main/resources/application.properties | 1 + .../io/quarkus/it/cache/CacheTestCase.java | 56 +++++++++++++++++-- 4 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 integration-tests/cache/src/main/java/io/quarkus/it/cache/CaffeineCacheWrapper.java diff --git a/extensions/cache/runtime/src/main/java/io/quarkus/cache/runtime/caffeine/CaffeineCacheImpl.java b/extensions/cache/runtime/src/main/java/io/quarkus/cache/runtime/caffeine/CaffeineCacheImpl.java index c8baff5992b7a..dff31eb8dab1e 100644 --- a/extensions/cache/runtime/src/main/java/io/quarkus/cache/runtime/caffeine/CaffeineCacheImpl.java +++ b/extensions/cache/runtime/src/main/java/io/quarkus/cache/runtime/caffeine/CaffeineCacheImpl.java @@ -127,13 +127,10 @@ public CompletableFuture getIfPresent(Object key) { Objects.requireNonNull(key, NULL_KEYS_NOT_SUPPORTED_MSG); CompletableFuture existingCacheValue = cache.getIfPresent(key); - // record metrics, if not null apply casting if (existingCacheValue == null) { - statsCounter.recordMisses(1); return null; } else { LOGGER.tracef("Key [%s] found in cache [%s]", key, cacheInfo.name); - statsCounter.recordHits(1); // cast, but still throw the CacheException in case it fails return unwrapCacheValueOrThrowable(existingCacheValue) diff --git a/integration-tests/cache/src/main/java/io/quarkus/it/cache/CaffeineCacheWrapper.java b/integration-tests/cache/src/main/java/io/quarkus/it/cache/CaffeineCacheWrapper.java new file mode 100644 index 0000000000000..780a6e447cb7d --- /dev/null +++ b/integration-tests/cache/src/main/java/io/quarkus/it/cache/CaffeineCacheWrapper.java @@ -0,0 +1,26 @@ +package io.quarkus.it.cache; + +import java.util.concurrent.CompletableFuture; + +import jakarta.enterprise.context.ApplicationScoped; + +import io.quarkus.cache.Cache; +import io.quarkus.cache.CacheName; +import io.quarkus.cache.CaffeineCache; + +@ApplicationScoped +public class CaffeineCacheWrapper { + + public static final String GET_IF_PRESENT_CACHE_NAME = "getIfPresentCache"; + + @CacheName(GET_IF_PRESENT_CACHE_NAME) + Cache cache; + + public CompletableFuture getIfPresent(Object key) { + return cache.as(CaffeineCache.class).getIfPresent(key); + } + + public void put(Object key, CompletableFuture value) { + cache.as(CaffeineCache.class).put(key, value); + } +} diff --git a/integration-tests/cache/src/main/resources/application.properties b/integration-tests/cache/src/main/resources/application.properties index b94edbb983678..9f87c19434340 100644 --- a/integration-tests/cache/src/main/resources/application.properties +++ b/integration-tests/cache/src/main/resources/application.properties @@ -8,5 +8,6 @@ quarkus.cache.caffeine."forest".expire-after-write=10M quarkus.cache.caffeine."expensiveResourceCache".expire-after-write=10M quarkus.cache.caffeine."expensiveResourceCache".metrics-enabled=true +quarkus.cache.caffeine."getIfPresentCache".metrics-enabled=true io.quarkus.it.cache.SunriseRestClient/mp-rest/url=${test.url} diff --git a/integration-tests/cache/src/test/java/io/quarkus/it/cache/CacheTestCase.java b/integration-tests/cache/src/test/java/io/quarkus/it/cache/CacheTestCase.java index 48fc07d224685..4d8b87a4f3d32 100644 --- a/integration-tests/cache/src/test/java/io/quarkus/it/cache/CacheTestCase.java +++ b/integration-tests/cache/src/test/java/io/quarkus/it/cache/CacheTestCase.java @@ -1,9 +1,17 @@ package io.quarkus.it.cache; +import static io.quarkus.it.cache.CaffeineCacheWrapper.GET_IF_PRESENT_CACHE_NAME; import static io.restassured.RestAssured.when; +import static java.util.concurrent.CompletableFuture.completedFuture; import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.concurrent.ExecutionException; + +import jakarta.inject.Inject; + import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -13,21 +21,59 @@ @DisplayName("Tests the cache extension") public class CacheTestCase { + private static final String EXPENSIVE_RESOURCE_CACHE_NAME = "expensiveResourceCache"; + private static final String CACHE_KEY = "foo"; + + @Inject + CaffeineCacheWrapper caffeineCacheWrapper; + @Test - public void testCache() { + void testCache() { + assertMetrics(EXPENSIVE_RESOURCE_CACHE_NAME, 0, 0, 0); + runExpensiveRequest(); + assertMetrics(EXPENSIVE_RESOURCE_CACHE_NAME, 1, 1, 0); + runExpensiveRequest(); + assertMetrics(EXPENSIVE_RESOURCE_CACHE_NAME, 1, 1, 1); + runExpensiveRequest(); + assertMetrics(EXPENSIVE_RESOURCE_CACHE_NAME, 1, 1, 2); + when().get("/expensive-resource/invocations").then().statusCode(200).body(is("1")); + } - String metricsResponse = when().get("/q/metrics").then().extract().asString(); - assertTrue(metricsResponse.contains("cache_puts_total{cache=\"expensiveResourceCache\"} 1.0")); - assertTrue(metricsResponse.contains("cache_gets_total{cache=\"expensiveResourceCache\",result=\"miss\"} 1.0")); - assertTrue(metricsResponse.contains("cache_gets_total{cache=\"expensiveResourceCache\",result=\"hit\"} 2.0")); + @Test + void testGetIfPresentMetrics() throws ExecutionException, InterruptedException { + assertMetrics(GET_IF_PRESENT_CACHE_NAME, 0, 0, 0); + + assertNull(caffeineCacheWrapper.getIfPresent(CACHE_KEY)); + assertMetrics(GET_IF_PRESENT_CACHE_NAME, 0, 1, 0); + + assertNull(caffeineCacheWrapper.getIfPresent(CACHE_KEY)); + assertMetrics(GET_IF_PRESENT_CACHE_NAME, 0, 2, 0); + + caffeineCacheWrapper.put(CACHE_KEY, completedFuture("bar")); + assertMetrics(GET_IF_PRESENT_CACHE_NAME, 1, 2, 0); + + assertEquals("bar", caffeineCacheWrapper.getIfPresent(CACHE_KEY).get()); + assertMetrics(GET_IF_PRESENT_CACHE_NAME, 1, 2, 1); + + assertEquals("bar", caffeineCacheWrapper.getIfPresent(CACHE_KEY).get()); + assertMetrics(GET_IF_PRESENT_CACHE_NAME, 1, 2, 2); } private void runExpensiveRequest() { when().get("/expensive-resource/I/love/Quarkus?foo=bar").then().statusCode(200).body("result", is("I love Quarkus too!")); } + + private void assertMetrics(String cacheName, double expectedPuts, double expectedMisses, double expectedHits) { + String metricsResponse = when().get("/q/metrics").then().extract().asString(); + assertTrue(metricsResponse.contains(String.format("cache_puts_total{cache=\"%s\"} %.1f", cacheName, expectedPuts))); + assertTrue(metricsResponse + .contains(String.format("cache_gets_total{cache=\"%s\",result=\"miss\"} %.1f", cacheName, expectedMisses))); + assertTrue(metricsResponse + .contains(String.format("cache_gets_total{cache=\"%s\",result=\"hit\"} %.1f", cacheName, expectedHits))); + } }