diff --git a/src/main/java/land/oras/auth/HttpClient.java b/src/main/java/land/oras/auth/HttpClient.java index a18952ba..fa27176f 100644 --- a/src/main/java/land/oras/auth/HttpClient.java +++ b/src/main/java/land/oras/auth/HttpClient.java @@ -21,7 +21,8 @@ package land.oras.auth; import io.micrometer.core.instrument.MeterRegistry; -import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import io.micrometer.core.instrument.Metrics; +import io.micrometer.core.instrument.Timer; import java.io.FileNotFoundException; import java.io.InputStream; import java.net.*; @@ -37,6 +38,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -108,7 +110,7 @@ private HttpClient() { this.skipTlsVerify = false; this.builder.cookieHandler(new CookieManager(null, CookiePolicy.ACCEPT_NONE)); this.setTimeout(60); - this.meterRegistry = new SimpleMeterRegistry(); + this.meterRegistry = Metrics.globalRegistry; } /** @@ -533,7 +535,7 @@ private ResponseWrapper executeRequest( HttpRequest request = builder.build(); logRequest(request, body); - HttpResponse response = client.send(request, handler); + HttpResponse response = executeAndRecordRequest(request, handler); // Follow redirect if (shouldRedirect(response)) { @@ -565,6 +567,23 @@ private ResponseWrapper executeRequest( } } + private HttpResponse executeAndRecordRequest(HttpRequest request, HttpResponse.BodyHandler handler) + throws Exception { + long start = System.nanoTime(); + HttpResponse response = client.send(request, handler); + long duration = System.nanoTime() - start; + Timer.builder(Const.METRIC_HTTP_REQUESTS) + .tag("method", request.method()) + .tag("host", request.uri().getHost()) + .tag("status", response != null ? String.valueOf(response.statusCode()) : "IO_ERROR") + .register(meterRegistry) + .record(duration, TimeUnit.NANOSECONDS); + if (response == null) { + throw new OrasException("No response received"); + } + return response; + } + private String getLocationHeader(HttpResponse response) { return response.headers() .firstValue("Location") @@ -593,7 +612,7 @@ private ResponseWrapper redoRequest( String service = token.service(); try { builder = builder.setHeader(Const.AUTHORIZATION_HEADER, "Bearer " + bearerToken); - HttpResponse newResponse = client.send(builder.build(), handler); + HttpResponse newResponse = executeAndRecordRequest(builder.build(), handler); // Follow redirect if (shouldRedirect(newResponse)) { @@ -608,7 +627,9 @@ private ResponseWrapper redoRequest( } return toResponseWrapper( - client.send(builder.uri(URI.create(location)).build(), handler), service); + executeAndRecordRequest( + builder.uri(URI.create(location)).build(), handler), + service); } return toResponseWrapper(newResponse, service); @@ -763,7 +784,6 @@ public Builder withSkipTlsVerify(boolean skipTlsVerify) { /** * Set the meter registry for metrics. Following Micrometer best practices for libraries, - * a {@link SimpleMeterRegistry} is used by default when no registry is provided. * @param meterRegistry The meter registry * @return The builder */ diff --git a/src/main/java/land/oras/auth/TokenCache.java b/src/main/java/land/oras/auth/TokenCache.java index 1a2f7302..38229cce 100644 --- a/src/main/java/land/oras/auth/TokenCache.java +++ b/src/main/java/land/oras/auth/TokenCache.java @@ -23,6 +23,9 @@ import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.Expiry; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Metrics; +import io.micrometer.core.instrument.binder.cache.CaffeineCacheMetrics; import java.util.concurrent.TimeUnit; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -35,6 +38,11 @@ @NullMarked public final class TokenCache { + /** + * Use global registry by default + */ + private static MeterRegistry meterRegistry = Metrics.globalRegistry; + /** * Hard cache limit */ @@ -52,6 +60,37 @@ private TokenCache() { // Private constructor to prevent instantiation } + /** + * The cache + */ + private static final Cache CACHE; + + static { + CACHE = Caffeine.newBuilder() + .maximumSize(MAX_CACHE_SIZE) + .recordStats() + .expireAfter(new Expiry() { + @Override + public long expireAfterCreate(Scopes key, HttpClient.TokenResponse token, long currentTime) { + return getExpiration(token); + } + + @Override + public long expireAfterUpdate( + Scopes key, HttpClient.TokenResponse token, long currentTime, long currentDuration) { + return currentDuration; + } + + @Override + public long expireAfterRead( + Scopes key, HttpClient.TokenResponse token, long currentTime, long currentDuration) { + return currentDuration; + } + }) + .build(); + CaffeineCacheMetrics.monitor(TokenCache.meterRegistry, CACHE, "land.oras.token.cache"); + } + /** * Cache for storing service information based on the service URL. This is used to avoid redundant */ @@ -59,29 +98,13 @@ private TokenCache() { Caffeine.newBuilder().maximumSize(MAX_CACHE_SIZE).build(); /** - * The cache + * Set the meter registry for monitoring the cache metrics + * @param meterRegistry the meter registry to use for monitoring the cache metrics */ - private static final Cache CACHE = Caffeine.newBuilder() - .maximumSize(MAX_CACHE_SIZE) - .expireAfter(new Expiry() { - @Override - public long expireAfterCreate(Scopes key, HttpClient.TokenResponse token, long currentTime) { - return getExpiration(token); - } - - @Override - public long expireAfterUpdate( - Scopes key, HttpClient.TokenResponse token, long currentTime, long currentDuration) { - return currentDuration; - } - - @Override - public long expireAfterRead( - Scopes key, HttpClient.TokenResponse token, long currentTime, long currentDuration) { - return currentDuration; - } - }) - .build(); + public static void setMeterRegistry(MeterRegistry meterRegistry) { + TokenCache.meterRegistry = meterRegistry; + CaffeineCacheMetrics.monitor(TokenCache.meterRegistry, CACHE, "land.oras.token.cache"); + } /** * Put a token response in the cache with the associated scopes. diff --git a/src/main/java/land/oras/utils/Const.java b/src/main/java/land/oras/utils/Const.java index b03708c4..b072b55f 100644 --- a/src/main/java/land/oras/utils/Const.java +++ b/src/main/java/land/oras/utils/Const.java @@ -444,7 +444,12 @@ public static String currentTimestamp() { /** * Metric name for token refresh counter */ - public static final String METRIC_TOKEN_REFRESH = "land_oras_auth_token_refresh_total"; + public static final String METRIC_TOKEN_REFRESH = "land.oras.auth.token.refresh"; + + /** + * Metric name for HTTP request + */ + public static final String METRIC_HTTP_REQUESTS = "http.client.requests"; /** * Metric name for token refresh duration diff --git a/src/test/java/land/oras/DockerIoITCase.java b/src/test/java/land/oras/DockerIoITCase.java index 76507b99..1a419b74 100644 --- a/src/test/java/land/oras/DockerIoITCase.java +++ b/src/test/java/land/oras/DockerIoITCase.java @@ -23,8 +23,7 @@ import static org.junit.jupiter.api.Assertions.*; import io.micrometer.core.instrument.Counter; -import io.micrometer.core.instrument.MeterRegistry; -import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import io.micrometer.core.instrument.Metrics; import java.nio.file.Path; import land.oras.utils.Const; import land.oras.utils.ZotUnsecureContainer; @@ -110,12 +109,8 @@ void shouldPullOneBlob() { void shouldCopyTagToInternalRegistry() { // Source registry - MeterRegistry meterRegistry = new SimpleMeterRegistry(); - Registry sourceRegistry = Registry.Builder.builder() - .withMeterRegistry(meterRegistry) - .withParallelism(3) - .defaults() - .build(); + Registry sourceRegistry = + Registry.Builder.builder().withParallelism(3).defaults().build(); // Copy to this internal registry Registry targetRegistry = Registry.Builder.builder() @@ -133,9 +128,11 @@ void shouldCopyTagToInternalRegistry() { assertEquals( 1.0, - meterRegistry.find(Const.METRIC_TOKEN_REFRESH).counters().stream() + Metrics.globalRegistry.find(Const.METRIC_TOKEN_REFRESH).counters().stream() .mapToDouble(Counter::count) .sum()); + + TestUtils.dumpMetrics(Metrics.globalRegistry); } @Test diff --git a/src/test/java/land/oras/RegistryWireMockTest.java b/src/test/java/land/oras/RegistryWireMockTest.java index c2572103..ff16e451 100644 --- a/src/test/java/land/oras/RegistryWireMockTest.java +++ b/src/test/java/land/oras/RegistryWireMockTest.java @@ -32,6 +32,7 @@ import com.github.tomakehurst.wiremock.junit5.WireMockTest; import com.github.tomakehurst.wiremock.stubbing.Scenario; import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.Metrics; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import java.io.IOException; import java.io.InputStream; @@ -558,6 +559,9 @@ void shouldGetAuthToken(WireMockRuntimeInfo wmRuntimeInfo) { @Test void shouldRefreshExpiredToken(WireMockRuntimeInfo wmRuntimeInfo) { + SimpleMeterRegistry meterRegistry = new SimpleMeterRegistry(); + Metrics.addRegistry(meterRegistry); + String digest = SupportedAlgorithm.SHA256.digest("blob-data".getBytes()); // Return data from wiremock @@ -586,11 +590,9 @@ void shouldRefreshExpiredToken(WireMockRuntimeInfo wmRuntimeInfo) { WireMock.ok().withBody("blob-data").withHeader(Const.DOCKER_CONTENT_DIGEST_HEADER, digest))); // Insecure registry with a custom meter registry to track metrics - SimpleMeterRegistry meterRegistry = new SimpleMeterRegistry(); Registry registry = Registry.Builder.builder() .withAuthProvider(new BearerTokenProvider()) // Already bearer token .withInsecure(true) - .withMeterRegistry(meterRegistry) .build(); ContainerRef containerRef = @@ -616,6 +618,7 @@ void shouldRefreshExpiredToken(WireMockRuntimeInfo wmRuntimeInfo) { .mapToDouble(Counter::count) .sum()); TestUtils.dumpMetrics(meterRegistry); + TestUtils.dumpMetrics(Metrics.globalRegistry); } @Test diff --git a/src/test/java/land/oras/auth/TokenCacheTest.java b/src/test/java/land/oras/auth/TokenCacheTest.java index caed3f11..397f89f4 100644 --- a/src/test/java/land/oras/auth/TokenCacheTest.java +++ b/src/test/java/land/oras/auth/TokenCacheTest.java @@ -22,8 +22,13 @@ 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 io.micrometer.core.instrument.FunctionCounter; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import land.oras.ContainerRef; +import land.oras.TestUtils; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.parallel.Execution; @@ -40,7 +45,10 @@ static void beforeAll() { } @Test - void shouldLooupWithGlobalScope() { + @Execution(ExecutionMode.SAME_THREAD) + void shouldLookupWithGlobalScope() { + MeterRegistry meterRegistry = new SimpleMeterRegistry(); + TokenCache.setMeterRegistry(meterRegistry); HttpClient.TokenResponse tokenResponse = new HttpClient.TokenResponse("other-token", null, "dockerhub", 1, null); ContainerRef containerRef = ContainerRef.parse("docker.io/library/alpine:latest"); @@ -50,10 +58,22 @@ void shouldLooupWithGlobalScope() { tokenResponse, TokenCache.get(Scopes.empty(containerRef, "dockerhub").withAddedGlobalScopes("aws")), "Should retrieve the token before expiration"); + TestUtils.dumpMetrics(meterRegistry); + + // At least one hit + assertTrue( + meterRegistry.find("cache.gets").tags("result", "hit").functionCounters().stream() + .mapToDouble(FunctionCounter::count) + .sum() + >= 1, + "Should have at least one cache hit"); } @Test + @Execution(ExecutionMode.SAME_THREAD) void shouldAddAndRetrieveTokenThenExpiredIt() throws InterruptedException { + MeterRegistry meterRegistry = new SimpleMeterRegistry(); + TokenCache.setMeterRegistry(meterRegistry); HttpClient.TokenResponse tokenResponse = new HttpClient.TokenResponse("other-token", null, "dockerhub", 1, null); ContainerRef containerRef = ContainerRef.parse("docker.io/library/alpine0:latest"); @@ -62,6 +82,14 @@ void shouldAddAndRetrieveTokenThenExpiredIt() throws InterruptedException { assertEquals(tokenResponse, TokenCache.get(scopes), "Should retrieve the token before expiration"); Thread.sleep(1500); // Wait for the token to expire assertNull(TokenCache.get(scopes), "Should return null after token expiration"); + TestUtils.dumpMetrics(meterRegistry); + // At least one eviction + assertTrue( + meterRegistry.find("cache.evictions").functionCounters().stream() + .mapToDouble(FunctionCounter::count) + .sum() + >= 1, + "Should have at least one eviction"); } @Test