Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions src/main/java/land/oras/auth/HttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -533,7 +535,7 @@ private <T> ResponseWrapper<T> executeRequest(

HttpRequest request = builder.build();
logRequest(request, body);
HttpResponse<T> response = client.send(request, handler);
HttpResponse<T> response = executeAndRecordRequest(request, handler);

// Follow redirect
if (shouldRedirect(response)) {
Expand Down Expand Up @@ -565,6 +567,23 @@ private <T> ResponseWrapper<T> executeRequest(
}
}

private <T> HttpResponse<T> executeAndRecordRequest(HttpRequest request, HttpResponse.BodyHandler<T> handler)
throws Exception {
long start = System.nanoTime();
HttpResponse<T> 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 <T> String getLocationHeader(HttpResponse<T> response) {
return response.headers()
.firstValue("Location")
Expand Down Expand Up @@ -593,7 +612,7 @@ private <T> ResponseWrapper<T> redoRequest(
String service = token.service();
try {
builder = builder.setHeader(Const.AUTHORIZATION_HEADER, "Bearer " + bearerToken);
HttpResponse<T> newResponse = client.send(builder.build(), handler);
HttpResponse<T> newResponse = executeAndRecordRequest(builder.build(), handler);

// Follow redirect
if (shouldRedirect(newResponse)) {
Expand All @@ -608,7 +627,9 @@ private <T> ResponseWrapper<T> 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);

Expand Down Expand Up @@ -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
*/
Expand Down
67 changes: 45 additions & 22 deletions src/main/java/land/oras/auth/TokenCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,6 +38,11 @@
@NullMarked
public final class TokenCache {

/**
* Use global registry by default
*/
private static MeterRegistry meterRegistry = Metrics.globalRegistry;

/**
* Hard cache limit
*/
Expand All @@ -52,36 +60,51 @@ private TokenCache() {
// Private constructor to prevent instantiation
}

/**
* The cache
*/
private static final Cache<Scopes, HttpClient.TokenResponse> CACHE;

static {
CACHE = Caffeine.newBuilder()
.maximumSize(MAX_CACHE_SIZE)
.recordStats()
.expireAfter(new Expiry<Scopes, HttpClient.TokenResponse>() {
@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
*/
private static final Cache<String, String> SERVICE_CACHE =
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<Scopes, HttpClient.TokenResponse> CACHE = Caffeine.newBuilder()
.maximumSize(MAX_CACHE_SIZE)
.expireAfter(new Expiry<Scopes, HttpClient.TokenResponse>() {
@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.
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/land/oras/utils/Const.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 6 additions & 9 deletions src/test/java/land/oras/DockerIoITCase.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions src/test/java/land/oras/RegistryWireMockTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand All @@ -616,6 +618,7 @@ void shouldRefreshExpiredToken(WireMockRuntimeInfo wmRuntimeInfo) {
.mapToDouble(Counter::count)
.sum());
TestUtils.dumpMetrics(meterRegistry);
TestUtils.dumpMetrics(Metrics.globalRegistry);
}

@Test
Expand Down
30 changes: 29 additions & 1 deletion src/test/java/land/oras/auth/TokenCacheTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
Expand All @@ -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");
Expand All @@ -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
Expand Down