From 2590f0e9f249b715e04c782765bd04b77107eade Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 07:39:50 +0000 Subject: [PATCH 1/2] feat: add http.client.request.duration metric for HTTP requests --- .../StorageHttpMetricsInterceptor.java | 226 +++++++++++ .../google/cloud/storage/StorageOptions.java | 11 + .../cloud/storage/spi/v1/HttpStorageRpc.java | 4 +- .../StorageHttpMetricsInterceptorTest.java | 374 ++++++++++++++++++ 4 files changed, 614 insertions(+), 1 deletion(-) create mode 100644 java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java create mode 100644 java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java new file mode 100644 index 000000000000..9c50c5d7448c --- /dev/null +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java @@ -0,0 +1,226 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage; + +import com.google.api.client.http.GenericUrl; +import com.google.api.client.http.HttpExecuteInterceptor; +import com.google.api.client.http.HttpIOExceptionHandler; +import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpRequestInitializer; +import com.google.api.client.http.HttpResponseInterceptor; +import com.google.api.core.ApiClock; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.context.Context; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * HTTP request initializer that records HTTP client metrics such as http.client.request.duration. + */ +final class StorageHttpMetricsInterceptor implements HttpRequestInitializer { + + @Nullable private final HttpRequestInitializer delegate; + private final StorageClientMetrics metrics; + private final ApiClock clock; + @Nullable private final String defaultServerAddress; + + StorageHttpMetricsInterceptor( + @Nullable HttpRequestInitializer delegate, + StorageClientMetrics metrics, + ApiClock clock, + @Nullable String host) { + this.delegate = delegate; + this.metrics = metrics; + this.clock = clock; + this.defaultServerAddress = StorageOptions.extractServerAddress(host); + } + + @Override + public void initialize(HttpRequest request) throws java.io.IOException { + if (delegate != null) { + delegate.initialize(request); + } + if (metrics == null || metrics.getHttpClientRequestDuration() == null) { + return; + } + + HttpExecuteInterceptor existingExec = request.getInterceptor(); + HttpResponseInterceptor existingResp = request.getResponseInterceptor(); + HttpIOExceptionHandler existingIoHandler = request.getIOExceptionHandler(); + + AtomicLong startTimeNanos = new AtomicLong(clock.nanoTime()); + AtomicBoolean recorded = new AtomicBoolean(false); + + request.setInterceptor( + req -> { + if (existingExec != null) { + existingExec.intercept(req); + } + startTimeNanos.set(clock.nanoTime()); + recorded.set(false); + }); + + request.setResponseInterceptor( + response -> { + if (existingResp != null) { + existingResp.interceptResponse(response); + } + if (recorded.compareAndSet(false, true) + && metrics.getHttpClientRequestDuration() != null) { + double durationSeconds = + Math.max(0.0, (clock.nanoTime() - startTimeNanos.get()) / 1_000_000_000.0); + int statusCode = response.getStatusCode(); + String method = + request.getRequestMethod() != null ? request.getRequestMethod() : "UNKNOWN"; + String urlTemplate = extractUrlTemplate(request.getUrl()); + String serverAddress = resolveServerAddress(request.getUrl(), defaultServerAddress); + + AttributesBuilder attrBuilder = + metrics + .clientAttributesBuilder() + .put(StorageClientMetrics.KEY_HTTP_REQUEST_METHOD, method) + .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, (long) statusCode) + .put( + StorageClientMetrics.KEY_NETWORK_PATH, + StorageClientMetrics.NETWORK_PATH_CLOUDPATH) + .put(StorageClientMetrics.KEY_URL_TEMPLATE, urlTemplate) + .put( + StorageClientMetrics.KEY_SERVER_ADDRESS, + serverAddress != null ? serverAddress : "") + .put( + StorageClientMetrics.KEY_ERROR_TYPE, + statusCode >= 400 + ? StorageErrorTypeMapper.mapHttpResponseCode(statusCode) + : StorageErrorTypeMapper.STATUS_OK); + + metrics + .getHttpClientRequestDuration() + .record(durationSeconds, attrBuilder.build(), Context.current()); + } + }); + + request.setIOExceptionHandler( + (req, supportsRetry) -> { + if (recorded.compareAndSet(false, true) + && metrics.getHttpClientRequestDuration() != null) { + double durationSeconds = + Math.max(0.0, (clock.nanoTime() - startTimeNanos.get()) / 1_000_000_000.0); + String method = req.getRequestMethod() != null ? req.getRequestMethod() : "UNKNOWN"; + String urlTemplate = extractUrlTemplate(req.getUrl()); + String serverAddress = resolveServerAddress(req.getUrl(), defaultServerAddress); + + AttributesBuilder attrBuilder = + metrics + .clientAttributesBuilder() + .put(StorageClientMetrics.KEY_HTTP_REQUEST_METHOD, method) + .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 0L) + .put( + StorageClientMetrics.KEY_NETWORK_PATH, + StorageClientMetrics.NETWORK_PATH_CLOUDPATH) + .put(StorageClientMetrics.KEY_URL_TEMPLATE, urlTemplate) + .put( + StorageClientMetrics.KEY_SERVER_ADDRESS, + serverAddress != null ? serverAddress : "") + .put( + StorageClientMetrics.KEY_ERROR_TYPE, + StorageErrorTypeMapper.STATUS_CONNECTIVITY); + + metrics + .getHttpClientRequestDuration() + .record(durationSeconds, attrBuilder.build(), Context.current()); + } + return existingIoHandler != null + && existingIoHandler.handleIOException(req, supportsRetry); + }); + } + + static String resolveServerAddress( + @Nullable GenericUrl url, @Nullable String defaultServerAddress) { + String host = url != null ? url.getHost() : null; + return (host != null && !host.isEmpty()) ? host : defaultServerAddress; + } + + static String extractUrlTemplate(@Nullable GenericUrl url) { + if (url == null) { + return "/"; + } + if (url.getFirst("upload_id") != null) { + return "/upload/storage/v1/b/{bucket}/o"; + } + String path = url.getRawPath(); + if (path == null || path.isEmpty()) { + return "/"; + } + if (path.length() > 1 && path.endsWith("/")) { + path = path.substring(0, path.length() - 1); + } + if (path.startsWith("/upload/storage/v1/b/")) { + return "/upload/storage/v1/b/{bucket}/o"; + } + if (path.startsWith("/download/storage/v1/b/")) { + return "/download/storage/v1/b/{bucket}/o/{object}"; + } + if (path.startsWith("/batch/storage/v1")) { + return "/batch/storage/v1"; + } + if (path.startsWith("/storage/v1/")) { + String rest = path.substring("/storage/v1/".length()); + String[] segments = rest.split("/"); + if (segments.length >= 2 && "b".equals(segments[0])) { + segments[1] = "{bucket}"; + if (segments.length >= 4 && "o".equals(segments[2])) { + segments[3] = "{object}"; + if (segments.length >= 6) { + if ("acl".equals(segments[4])) { + segments[5] = "{entity}"; + } else if ("copyTo".equals(segments[4]) || "rewriteTo".equals(segments[4])) { + if (segments.length >= 7 && "b".equals(segments[5])) { + segments[6] = "{destinationBucket}"; + } + if (segments.length >= 9 && "o".equals(segments[7])) { + segments[8] = "{destinationObject}"; + } + } else if ("moveTo".equals(segments[4]) + && "o".equals(segments[5]) + && segments.length >= 7) { + segments[6] = "{destinationObject}"; + } + } + } else if (segments.length >= 4) { + if ("acl".equals(segments[2]) || "defaultObjectAcl".equals(segments[2])) { + segments[3] = "{entity}"; + } else if ("notifications".equals(segments[2])) { + segments[3] = "{notification}"; + } + } + } else if (segments.length >= 2 && "projects".equals(segments[0])) { + segments[1] = "{project}"; + if (segments.length >= 4 && "hmacKeys".equals(segments[2])) { + segments[3] = "{accessId}"; + } + } + return "/storage/v1/" + String.join("/", segments); + } + String trimmed = path.startsWith("/") ? path.substring(1) : path; + if (trimmed.isEmpty()) { + return "/"; + } + String[] parts = trimmed.split("/", 2); + return parts.length == 1 ? "/{bucket}" : "/{bucket}/{object}"; + } +} diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java index 330be593875f..9ad129f50f3e 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; +import com.google.api.client.http.HttpRequestInitializer; import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; import com.google.cloud.NoCredentials; @@ -518,6 +519,16 @@ private Attributes buildBaseMetricsAttributes(StorageClientMetrics metrics) { return builder.build(); } + @InternalApi + @Nullable + public HttpRequestInitializer decorateWithMetrics(@Nullable HttpRequestInitializer initializer) { + StorageClientMetrics metrics = getStorageClientMetrics(); + if (metrics == null) { + return initializer; + } + return new StorageHttpMetricsInterceptor(initializer, metrics, getClock(), getHost()); + } + @Nullable static String extractServerAddress(@Nullable String host) { if (host == null || host.trim().isEmpty()) { diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java index ec7d8e02e411..930388f78572 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java @@ -157,7 +157,9 @@ public HttpStorageRpc(StorageOptions options, JsonFactory jsonFactory) { CensusHttpModule censusHttpModule = new CensusHttpModule(tracer, IS_RECORD_EVENTS); initializer = censusHttpModule.getHttpRequestInitializer(initializer); initializer = new InvocationIdInitializer(initializer, applicationName, tm); - batchRequestInitializer = censusHttpModule.getHttpRequestInitializer(null); + initializer = options.decorateWithMetrics(initializer); + batchRequestInitializer = + options.decorateWithMetrics(censusHttpModule.getHttpRequestInitializer(null)); storage = new Storage.Builder(transport, jsonFactory, initializer) .setRootUrl(options.getHost()) diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java new file mode 100644 index 000000000000..0ce3d3376225 --- /dev/null +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java @@ -0,0 +1,374 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import com.google.api.client.http.GenericUrl; +import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpTransport; +import com.google.api.client.http.LowLevelHttpRequest; +import com.google.api.client.http.LowLevelHttpResponse; +import com.google.api.client.testing.http.MockHttpTransport; +import com.google.api.client.testing.http.MockLowLevelHttpRequest; +import com.google.api.client.testing.http.MockLowLevelHttpResponse; +import com.google.api.core.ApiClock; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.data.HistogramPointData; +import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; +import java.io.IOException; +import java.util.Collection; +import java.util.concurrent.atomic.AtomicLong; +import org.junit.Before; +import org.junit.Test; + +public final class StorageHttpMetricsInterceptorTest { + + private InMemoryMetricReader reader; + private SdkMeterProvider meterProvider; + private FakeClock clock; + + @Before + public void setUp() { + reader = InMemoryMetricReader.create(); + meterProvider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + clock = new FakeClock(); + } + + @Test + public void extractUrlTemplate_cases() { + assertThat(StorageHttpMetricsInterceptor.extractUrlTemplate(null)).isEqualTo("/"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/upload/storage/v1/b/my-b/o?upload_id=xyz"))) + .isEqualTo("/upload/storage/v1/b/{bucket}/o"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://storage.googleapis.com/upload/storage/v1/b/my-b/o"))) + .isEqualTo("/upload/storage/v1/b/{bucket}/o"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/download/storage/v1/b/my-b/o/my-obj"))) + .isEqualTo("/download/storage/v1/b/{bucket}/o/{object}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://storage.googleapis.com/batch/storage/v1"))) + .isEqualTo("/batch/storage/v1"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b"))) + .isEqualTo("/storage/v1/b/{bucket}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b/o/my-obj"))) + .isEqualTo("/storage/v1/b/{bucket}/o/{object}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/b/my-b/o/my-obj/acl/allUsers"))) + .isEqualTo("/storage/v1/b/{bucket}/o/{object}/acl/{entity}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/b/b1/o/o1/copyTo/b/b2/o/o2"))) + .isEqualTo( + "/storage/v1/b/{bucket}/o/{object}/copyTo/b/{destinationBucket}/o/{destinationObject}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/b/b1/o/o1/rewriteTo/b/b2/o/o2"))) + .isEqualTo( + "/storage/v1/b/{bucket}/o/{object}/rewriteTo/b/{destinationBucket}/o/{destinationObject}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/b1/o/o1/moveTo/o/o2"))) + .isEqualTo("/storage/v1/b/{bucket}/o/{object}/moveTo/o/{destinationObject}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/b1/acl/allUsers"))) + .isEqualTo("/storage/v1/b/{bucket}/acl/{entity}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/b/b1/defaultObjectAcl/allUsers"))) + .isEqualTo("/storage/v1/b/{bucket}/defaultObjectAcl/{entity}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/b/b1/notifications/notif1"))) + .isEqualTo("/storage/v1/b/{bucket}/notifications/{notification}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/projects/my-p/hmacKeys/key1"))) + .isEqualTo("/storage/v1/projects/{project}/hmacKeys/{accessId}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://storage.googleapis.com/my-b"))) + .isEqualTo("/{bucket}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://storage.googleapis.com/my-b/my-obj"))) + .isEqualTo("/{bucket}/{object}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://storage.googleapis.com/"))) + .isEqualTo("/"); + } + + @Test + public void initialize_200Ok_recordsDurationWithoutErrorType() throws IOException { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); + StorageHttpMetricsInterceptor interceptor = + new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); + + MockLowLevelHttpResponse lowLevelResponse = new MockLowLevelHttpResponse(); + lowLevelResponse.setStatusCode(200); + HttpTransport transport = + new MockHttpTransport.Builder().setLowLevelHttpResponse(lowLevelResponse).build(); + + HttpRequest request = + transport + .createRequestFactory(interceptor) + .buildGetRequest( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b/o/my-obj")); + request.execute(); + + Collection metricData = reader.collectAllMetrics(); + MetricData durationData = + metricData.stream() + .filter( + m -> m.getName().equals(StorageClientMetrics.METRIC_HTTP_CLIENT_REQUEST_DURATION)) + .findFirst() + .orElse(null); + + assertThat(durationData).isNotNull(); + assertThat(durationData.getHistogramData().getPoints()).hasSize(1); + HistogramPointData point = durationData.getHistogramData().getPoints().iterator().next(); + + Attributes attrs = point.getAttributes(); + assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_REQUEST_METHOD)).isEqualTo("GET"); + assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)).isEqualTo(200L); + assertThat(attrs.get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); + assertThat(attrs.get(StorageClientMetrics.KEY_URL_TEMPLATE)) + .isEqualTo("/storage/v1/b/{bucket}/o/{object}"); + assertThat(attrs.get(StorageClientMetrics.KEY_SERVER_ADDRESS)) + .isEqualTo("storage.googleapis.com"); + assertThat(attrs.get(StorageClientMetrics.KEY_ERROR_TYPE)).isEqualTo("OK"); + } + + @Test + public void initialize_308ResumeIncomplete_recordsWithoutErrorType() throws IOException { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); + StorageHttpMetricsInterceptor interceptor = + new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); + + MockLowLevelHttpResponse lowLevelResponse = new MockLowLevelHttpResponse(); + lowLevelResponse.setStatusCode(308); + HttpTransport transport = + new MockHttpTransport.Builder().setLowLevelHttpResponse(lowLevelResponse).build(); + + HttpRequest request = + transport + .createRequestFactory(interceptor) + .buildPutRequest( + new GenericUrl( + "https://storage.googleapis.com/upload/storage/v1/b/my-b/o?upload_id=123"), + null); + request.setThrowExceptionOnExecuteError(false); + request.execute(); + + Collection metricData = reader.collectAllMetrics(); + MetricData durationData = + metricData.stream() + .filter( + m -> m.getName().equals(StorageClientMetrics.METRIC_HTTP_CLIENT_REQUEST_DURATION)) + .findFirst() + .orElse(null); + + assertThat(durationData).isNotNull(); + HistogramPointData point = durationData.getHistogramData().getPoints().iterator().next(); + Attributes attrs = point.getAttributes(); + assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)).isEqualTo(308L); + assertThat(attrs.get(StorageClientMetrics.KEY_ERROR_TYPE)).isEqualTo("OK"); + assertThat(attrs.get(StorageClientMetrics.KEY_SERVER_ADDRESS)) + .isEqualTo("storage.googleapis.com"); + } + + @Test + public void initialize_404NotFound_recordsErrorType() throws IOException { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); + StorageHttpMetricsInterceptor interceptor = + new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); + + MockLowLevelHttpResponse lowLevelResponse = new MockLowLevelHttpResponse(); + lowLevelResponse.setStatusCode(404); + HttpTransport transport = + new MockHttpTransport.Builder().setLowLevelHttpResponse(lowLevelResponse).build(); + + HttpRequest request = + transport + .createRequestFactory(interceptor) + .buildGetRequest( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/missing-bucket")); + request.setThrowExceptionOnExecuteError(false); + request.execute(); + + Collection metricData = reader.collectAllMetrics(); + MetricData durationData = + metricData.stream() + .filter( + m -> m.getName().equals(StorageClientMetrics.METRIC_HTTP_CLIENT_REQUEST_DURATION)) + .findFirst() + .orElse(null); + + assertThat(durationData).isNotNull(); + HistogramPointData point = durationData.getHistogramData().getPoints().iterator().next(); + Attributes attrs = point.getAttributes(); + assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)).isEqualTo(404L); + assertThat(attrs.get(StorageClientMetrics.KEY_ERROR_TYPE)).isEqualTo("NOT_FOUND"); + assertThat(attrs.get(StorageClientMetrics.KEY_SERVER_ADDRESS)) + .isEqualTo("storage.googleapis.com"); + } + + @Test + public void initialize_503ServiceUnavailable_recordsErrorType() throws IOException { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); + StorageHttpMetricsInterceptor interceptor = + new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); + + MockLowLevelHttpResponse lowLevelResponse = new MockLowLevelHttpResponse(); + lowLevelResponse.setStatusCode(503); + HttpTransport transport = + new MockHttpTransport.Builder().setLowLevelHttpResponse(lowLevelResponse).build(); + + HttpRequest request = + transport + .createRequestFactory(interceptor) + .buildGetRequest( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b/o/my-obj")); + request.setThrowExceptionOnExecuteError(false); + request.execute(); + + Collection metricData = reader.collectAllMetrics(); + MetricData durationData = + metricData.stream() + .filter( + m -> m.getName().equals(StorageClientMetrics.METRIC_HTTP_CLIENT_REQUEST_DURATION)) + .findFirst() + .orElse(null); + + assertThat(durationData).isNotNull(); + HistogramPointData point = durationData.getHistogramData().getPoints().iterator().next(); + Attributes attrs = point.getAttributes(); + assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)).isEqualTo(503L); + assertThat(attrs.get(StorageClientMetrics.KEY_ERROR_TYPE)).isEqualTo("UNAVAILABLE"); + assertThat(attrs.get(StorageClientMetrics.KEY_SERVER_ADDRESS)) + .isEqualTo("storage.googleapis.com"); + } + + @Test + public void initialize_networkIOException_recordsErrorTypeWithoutStatusCode() { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); + StorageHttpMetricsInterceptor interceptor = + new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); + + HttpTransport transport = + new MockHttpTransport() { + @Override + public LowLevelHttpRequest buildRequest(String method, String url) { + return new MockLowLevelHttpRequest() { + @Override + public LowLevelHttpResponse execute() throws IOException { + throw new IOException("Connection reset by peer"); + } + }; + } + }; + + assertThrows( + IOException.class, + () -> { + HttpRequest req = + transport + .createRequestFactory(interceptor) + .buildGetRequest( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b")); + req.execute(); + }); + + Collection metricData = reader.collectAllMetrics(); + MetricData durationData = + metricData.stream() + .filter( + m -> m.getName().equals(StorageClientMetrics.METRIC_HTTP_CLIENT_REQUEST_DURATION)) + .findFirst() + .orElse(null); + + assertThat(durationData).isNotNull(); + HistogramPointData point = durationData.getHistogramData().getPoints().iterator().next(); + Attributes attrs = point.getAttributes(); + assertThat(attrs.get(StorageClientMetrics.KEY_ERROR_TYPE)).isEqualTo("CONNECTIVITY"); + assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)).isEqualTo(0L); + assertThat(attrs.get(StorageClientMetrics.KEY_SERVER_ADDRESS)) + .isEqualTo("storage.googleapis.com"); + } + + @Test + public void initialize_metricsDisabled_doesNotRecord() throws IOException { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, false, false); + StorageHttpMetricsInterceptor interceptor = + new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); + + MockLowLevelHttpResponse lowLevelResponse = new MockLowLevelHttpResponse(); + lowLevelResponse.setStatusCode(200); + HttpTransport transport = + new MockHttpTransport.Builder().setLowLevelHttpResponse(lowLevelResponse).build(); + + HttpRequest request = + transport + .createRequestFactory(interceptor) + .buildGetRequest(new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b")); + request.execute(); + + assertThat(reader.collectAllMetrics()).isEmpty(); + } + + private static final class FakeClock implements ApiClock { + private final AtomicLong nanos = new AtomicLong(1_000_000_000L); + + @Override + public long nanoTime() { + return nanos.get(); + } + + @Override + public long millisTime() { + return nanos.get() / 1_000_000L; + } + + void advanceNanos(long amount) { + nanos.addAndGet(amount); + } + } +} From 510824ae4dda6ccdfe5032ce821f01b7d8a2b7c2 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Tue, 3 Nov 2026 10:33:12 +0000 Subject: [PATCH 2/2] fix(storage): address PR #14744 review comments and align HTTP duration metrics [Generated-by: AI] --- .../StorageHttpMetricsInterceptor.java | 235 +++++++++++------- .../StorageHttpMetricsInterceptorTest.java | 193 ++++++++++++-- 2 files changed, 308 insertions(+), 120 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java index 9c50c5d7448c..09670057bcb4 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageHttpMetricsInterceptor.java @@ -23,25 +23,29 @@ import com.google.api.client.http.HttpRequestInitializer; import com.google.api.client.http.HttpResponseInterceptor; import com.google.api.core.ApiClock; -import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.context.Context; +import java.io.IOException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; -import org.checkerframework.checker.nullness.qual.Nullable; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; /** * HTTP request initializer that records HTTP client metrics such as http.client.request.duration. */ +@NullMarked final class StorageHttpMetricsInterceptor implements HttpRequestInitializer { @Nullable private final HttpRequestInitializer delegate; - private final StorageClientMetrics metrics; + @Nullable private final StorageClientMetrics metrics; private final ApiClock clock; @Nullable private final String defaultServerAddress; StorageHttpMetricsInterceptor( @Nullable HttpRequestInitializer delegate, - StorageClientMetrics metrics, + @Nullable StorageClientMetrics metrics, ApiClock clock, @Nullable String host) { this.delegate = delegate; @@ -51,11 +55,15 @@ final class StorageHttpMetricsInterceptor implements HttpRequestInitializer { } @Override - public void initialize(HttpRequest request) throws java.io.IOException { + public void initialize(HttpRequest request) throws IOException { if (delegate != null) { delegate.initialize(request); } - if (metrics == null || metrics.getHttpClientRequestDuration() == null) { + if (metrics == null) { + return; + } + DoubleHistogram durationHistogram = metrics.getHttpClientRequestDuration(); + if (durationHistogram == null) { return; } @@ -63,8 +71,8 @@ public void initialize(HttpRequest request) throws java.io.IOException { HttpResponseInterceptor existingResp = request.getResponseInterceptor(); HttpIOExceptionHandler existingIoHandler = request.getIOExceptionHandler(); - AtomicLong startTimeNanos = new AtomicLong(clock.nanoTime()); - AtomicBoolean recorded = new AtomicBoolean(false); + AtomicLong startTimeNanos = new AtomicLong(); + AtomicBoolean recorded = new AtomicBoolean(true); request.setInterceptor( req -> { @@ -80,79 +88,80 @@ public void initialize(HttpRequest request) throws java.io.IOException { if (existingResp != null) { existingResp.interceptResponse(response); } - if (recorded.compareAndSet(false, true) - && metrics.getHttpClientRequestDuration() != null) { - double durationSeconds = - Math.max(0.0, (clock.nanoTime() - startTimeNanos.get()) / 1_000_000_000.0); + if (recorded.compareAndSet(false, true)) { int statusCode = response.getStatusCode(); - String method = - request.getRequestMethod() != null ? request.getRequestMethod() : "UNKNOWN"; - String urlTemplate = extractUrlTemplate(request.getUrl()); - String serverAddress = resolveServerAddress(request.getUrl(), defaultServerAddress); - - AttributesBuilder attrBuilder = - metrics - .clientAttributesBuilder() - .put(StorageClientMetrics.KEY_HTTP_REQUEST_METHOD, method) - .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, (long) statusCode) - .put( - StorageClientMetrics.KEY_NETWORK_PATH, - StorageClientMetrics.NETWORK_PATH_CLOUDPATH) - .put(StorageClientMetrics.KEY_URL_TEMPLATE, urlTemplate) - .put( - StorageClientMetrics.KEY_SERVER_ADDRESS, - serverAddress != null ? serverAddress : "") - .put( - StorageClientMetrics.KEY_ERROR_TYPE, - statusCode >= 400 - ? StorageErrorTypeMapper.mapHttpResponseCode(statusCode) - : StorageErrorTypeMapper.STATUS_OK); - - metrics - .getHttpClientRequestDuration() - .record(durationSeconds, attrBuilder.build(), Context.current()); + String errorType = + statusCode >= 400 + ? StorageErrorTypeMapper.mapHttpResponseCode(statusCode) + : StorageErrorTypeMapper.STATUS_OK; + recordDuration(durationHistogram, request, startTimeNanos.get(), statusCode, errorType); } }); + // HttpIOExceptionHandler does not receive the thrown IOException instance from + // google-http-client, so transport-level IOExceptions default to STATUS_CONNECTIVITY here + // (specific subtypes like TIMEOUT, DNS_FAILURE, and TLS_FAILURE are mapped in Retrying). request.setIOExceptionHandler( (req, supportsRetry) -> { - if (recorded.compareAndSet(false, true) - && metrics.getHttpClientRequestDuration() != null) { - double durationSeconds = - Math.max(0.0, (clock.nanoTime() - startTimeNanos.get()) / 1_000_000_000.0); - String method = req.getRequestMethod() != null ? req.getRequestMethod() : "UNKNOWN"; - String urlTemplate = extractUrlTemplate(req.getUrl()); - String serverAddress = resolveServerAddress(req.getUrl(), defaultServerAddress); - - AttributesBuilder attrBuilder = - metrics - .clientAttributesBuilder() - .put(StorageClientMetrics.KEY_HTTP_REQUEST_METHOD, method) - .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 0L) - .put( - StorageClientMetrics.KEY_NETWORK_PATH, - StorageClientMetrics.NETWORK_PATH_CLOUDPATH) - .put(StorageClientMetrics.KEY_URL_TEMPLATE, urlTemplate) - .put( - StorageClientMetrics.KEY_SERVER_ADDRESS, - serverAddress != null ? serverAddress : "") - .put( - StorageClientMetrics.KEY_ERROR_TYPE, - StorageErrorTypeMapper.STATUS_CONNECTIVITY); - - metrics - .getHttpClientRequestDuration() - .record(durationSeconds, attrBuilder.build(), Context.current()); + if (recorded.compareAndSet(false, true)) { + recordDuration( + durationHistogram, + req, + startTimeNanos.get(), + 0L, + StorageErrorTypeMapper.STATUS_CONNECTIVITY); } return existingIoHandler != null && existingIoHandler.handleIOException(req, supportsRetry); }); } + private void recordDuration( + DoubleHistogram durationHistogram, + HttpRequest request, + long startNanos, + long statusCode, + String errorType) { + if (metrics == null) { + return; + } + double durationSeconds = Math.max(0.0, (clock.nanoTime() - startNanos) / 1_000_000_000.0); + String method = request.getRequestMethod() != null ? request.getRequestMethod() : "UNKNOWN"; + String urlTemplate = extractUrlTemplate(request.getUrl()); + String serverAddress = resolveServerAddress(request.getUrl(), defaultServerAddress); + + Attributes attributes = + metrics + .clientAttributesBuilder() + .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http") + .put(StorageClientMetrics.KEY_HTTP_REQUEST_METHOD, method) + .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, statusCode) + .put(StorageClientMetrics.KEY_NETWORK_PATH, StorageClientMetrics.NETWORK_PATH_CLOUDPATH) + .put(StorageClientMetrics.KEY_URL_TEMPLATE, urlTemplate) + .put( + StorageClientMetrics.KEY_SERVER_ADDRESS, serverAddress != null ? serverAddress : "") + .put(StorageClientMetrics.KEY_ERROR_TYPE, errorType) + .build(); + + durationHistogram.record(durationSeconds, attributes, Context.current()); + } + + @Nullable static String resolveServerAddress( @Nullable GenericUrl url, @Nullable String defaultServerAddress) { String host = url != null ? url.getHost() : null; - return (host != null && !host.isEmpty()) ? host : defaultServerAddress; + if (host == null || host.isEmpty()) { + return defaultServerAddress; + } + if (host.endsWith(".storage.googleapis.com") && !host.equals("storage.googleapis.com")) { + return "storage.googleapis.com"; + } + if (defaultServerAddress != null + && !defaultServerAddress.isEmpty() + && host.endsWith("." + defaultServerAddress)) { + return defaultServerAddress; + } + return host; } static String extractUrlTemplate(@Nullable GenericUrl url) { @@ -166,6 +175,12 @@ static String extractUrlTemplate(@Nullable GenericUrl url) { if (path == null || path.isEmpty()) { return "/"; } + String host = url.getHost(); + if (host != null + && host.endsWith(".storage.googleapis.com") + && !host.equals("storage.googleapis.com")) { + return "/".equals(path) ? "/" : "/{object}"; + } if (path.length() > 1 && path.endsWith("/")) { path = path.substring(0, path.length() - 1); } @@ -182,39 +197,73 @@ static String extractUrlTemplate(@Nullable GenericUrl url) { String rest = path.substring("/storage/v1/".length()); String[] segments = rest.split("/"); if (segments.length >= 2 && "b".equals(segments[0])) { - segments[1] = "{bucket}"; - if (segments.length >= 4 && "o".equals(segments[2])) { - segments[3] = "{object}"; - if (segments.length >= 6) { - if ("acl".equals(segments[4])) { - segments[5] = "{entity}"; - } else if ("copyTo".equals(segments[4]) || "rewriteTo".equals(segments[4])) { - if (segments.length >= 7 && "b".equals(segments[5])) { - segments[6] = "{destinationBucket}"; - } - if (segments.length >= 9 && "o".equals(segments[7])) { - segments[8] = "{destinationObject}"; - } - } else if ("moveTo".equals(segments[4]) - && "o".equals(segments[5]) - && segments.length >= 7) { - segments[6] = "{destinationObject}"; - } + if (segments.length == 2) { + return "/storage/v1/b/{bucket}"; + } + if ("o".equals(segments[2])) { + if (segments.length == 3) { + return "/storage/v1/b/{bucket}/o"; + } + if (segments.length == 4) { + return "/storage/v1/b/{bucket}/o/{object}"; + } + String action = segments[4]; + if (segments.length == 5 + && ("compose".equals(action) + || "restore".equals(action) + || "watchAll".equals(action))) { + return "/storage/v1/b/{bucket}/o/{object}/" + action; + } + if ("acl".equals(action)) { + return segments.length == 5 + ? "/storage/v1/b/{bucket}/o/{object}/acl" + : "/storage/v1/b/{bucket}/o/{object}/acl/{entity}"; + } + if (("copyTo".equals(action) || "rewriteTo".equals(action)) + && segments.length >= 8 + && "b".equals(segments[5]) + && "o".equals(segments[7])) { + return "/storage/v1/b/{bucket}/o/{object}/" + + action + + "/b/{destinationBucket}/o/{destinationObject}"; } - } else if (segments.length >= 4) { - if ("acl".equals(segments[2]) || "defaultObjectAcl".equals(segments[2])) { - segments[3] = "{entity}"; - } else if ("notifications".equals(segments[2])) { - segments[3] = "{notification}"; + if ("moveTo".equals(action) && segments.length >= 7 && "o".equals(segments[5])) { + return "/storage/v1/b/{bucket}/o/{object}/moveTo/o/{destinationObject}"; } + return "/storage/v1/b/{bucket}/o/{object}"; } + String subResource = segments[2]; + if ("acl".equals(subResource) || "defaultObjectAcl".equals(subResource)) { + return segments.length == 3 + ? "/storage/v1/b/{bucket}/" + subResource + : "/storage/v1/b/{bucket}/" + subResource + "/{entity}"; + } + if ("notifications".equals(subResource) || "notificationConfigs".equals(subResource)) { + return segments.length == 3 + ? "/storage/v1/b/{bucket}/" + subResource + : "/storage/v1/b/{bucket}/" + subResource + "/{notification}"; + } + if (segments.length == 3) { + return "/storage/v1/b/{bucket}/" + subResource; + } + if (segments.length == 4 + && "iam".equals(subResource) + && "testPermissions".equals(segments[3])) { + return "/storage/v1/b/{bucket}/iam/testPermissions"; + } + return "/storage/v1/b/{bucket}"; } else if (segments.length >= 2 && "projects".equals(segments[0])) { - segments[1] = "{project}"; - if (segments.length >= 4 && "hmacKeys".equals(segments[2])) { - segments[3] = "{accessId}"; + if (segments.length >= 3 && "hmacKeys".equals(segments[2])) { + return segments.length == 3 + ? "/storage/v1/projects/{project}/hmacKeys" + : "/storage/v1/projects/{project}/hmacKeys/{accessId}"; + } + if (segments.length >= 3 && "serviceAccount".equals(segments[2])) { + return "/storage/v1/projects/{project}/serviceAccount"; } + return "/storage/v1/projects/{project}"; } - return "/storage/v1/" + String.join("/", segments); + return "/storage/v1"; } String trimmed = path.startsWith("/") ? path.substring(1) : path; if (trimmed.isEmpty()) { diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java index 0ce3d3376225..d10200a71222 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageHttpMetricsInterceptorTest.java @@ -21,6 +21,7 @@ import com.google.api.client.http.GenericUrl; import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpRequestInitializer; import com.google.api.client.http.HttpTransport; import com.google.api.client.http.LowLevelHttpRequest; import com.google.api.client.http.LowLevelHttpResponse; @@ -35,6 +36,7 @@ import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import java.io.IOException; import java.util.Collection; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import org.junit.Before; import org.junit.Test; @@ -77,10 +79,34 @@ public void extractUrlTemplate_cases() { StorageHttpMetricsInterceptor.extractUrlTemplate( new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b"))) .isEqualTo("/storage/v1/b/{bucket}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b/o"))) + .isEqualTo("/storage/v1/b/{bucket}/o"); assertThat( StorageHttpMetricsInterceptor.extractUrlTemplate( new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b/o/my-obj"))) .isEqualTo("/storage/v1/b/{bucket}/o/{object}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/b/my-b/o/dir/sub/file.txt"))) + .isEqualTo("/storage/v1/b/{bucket}/o/{object}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/b/my-b/o/my-obj/compose"))) + .isEqualTo("/storage/v1/b/{bucket}/o/{object}/compose"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/b/my-b/o/my-obj/restore"))) + .isEqualTo("/storage/v1/b/{bucket}/o/{object}/restore"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/b/my-b/o/my-obj/watchAll"))) + .isEqualTo("/storage/v1/b/{bucket}/o/{object}/watchAll"); assertThat( StorageHttpMetricsInterceptor.extractUrlTemplate( new GenericUrl( @@ -116,11 +142,25 @@ public void extractUrlTemplate_cases() { new GenericUrl( "https://storage.googleapis.com/storage/v1/b/b1/notifications/notif1"))) .isEqualTo("/storage/v1/b/{bucket}/notifications/{notification}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/b1/iam"))) + .isEqualTo("/storage/v1/b/{bucket}/iam"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/b/b1/iam/testPermissions"))) + .isEqualTo("/storage/v1/b/{bucket}/iam/testPermissions"); assertThat( StorageHttpMetricsInterceptor.extractUrlTemplate( new GenericUrl( "https://storage.googleapis.com/storage/v1/projects/my-p/hmacKeys/key1"))) .isEqualTo("/storage/v1/projects/{project}/hmacKeys/{accessId}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl( + "https://storage.googleapis.com/storage/v1/projects/my-p/serviceAccount"))) + .isEqualTo("/storage/v1/projects/{project}/serviceAccount"); assertThat( StorageHttpMetricsInterceptor.extractUrlTemplate( new GenericUrl("https://storage.googleapis.com/my-b"))) @@ -133,6 +173,34 @@ public void extractUrlTemplate_cases() { StorageHttpMetricsInterceptor.extractUrlTemplate( new GenericUrl("https://storage.googleapis.com/"))) .isEqualTo("/"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://my-bucket.storage.googleapis.com/"))) + .isEqualTo("/"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://my-bucket.storage.googleapis.com/my-obj"))) + .isEqualTo("/{object}"); + assertThat( + StorageHttpMetricsInterceptor.extractUrlTemplate( + new GenericUrl("https://my-bucket.storage.googleapis.com/dir/my-obj"))) + .isEqualTo("/{object}"); + } + + @Test + public void resolveServerAddress_stripsVirtualHostedBucketPrefix() { + assertThat( + StorageHttpMetricsInterceptor.resolveServerAddress( + new GenericUrl("https://my-bucket.storage.googleapis.com/my-obj"), + "storage.googleapis.com")) + .isEqualTo("storage.googleapis.com"); + assertThat( + StorageHttpMetricsInterceptor.resolveServerAddress( + new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b"), + "storage.googleapis.com")) + .isEqualTo("storage.googleapis.com"); + assertThat(StorageHttpMetricsInterceptor.resolveServerAddress(null, "storage.googleapis.com")) + .isEqualTo("storage.googleapis.com"); } @Test @@ -141,10 +209,7 @@ public void initialize_200Ok_recordsDurationWithoutErrorType() throws IOExceptio StorageHttpMetricsInterceptor interceptor = new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); - MockLowLevelHttpResponse lowLevelResponse = new MockLowLevelHttpResponse(); - lowLevelResponse.setStatusCode(200); - HttpTransport transport = - new MockHttpTransport.Builder().setLowLevelHttpResponse(lowLevelResponse).build(); + HttpTransport transport = createAdvancingTransport(200, 250_000_000L); HttpRequest request = transport @@ -164,8 +229,10 @@ public void initialize_200Ok_recordsDurationWithoutErrorType() throws IOExceptio assertThat(durationData).isNotNull(); assertThat(durationData.getHistogramData().getPoints()).hasSize(1); HistogramPointData point = durationData.getHistogramData().getPoints().iterator().next(); + assertThat(point.getSum()).isWithin(1e-6).of(0.25); Attributes attrs = point.getAttributes(); + assertThat(attrs.get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME)).isEqualTo("http"); assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_REQUEST_METHOD)).isEqualTo("GET"); assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)).isEqualTo(200L); assertThat(attrs.get(StorageClientMetrics.KEY_NETWORK_PATH)) @@ -183,10 +250,7 @@ public void initialize_308ResumeIncomplete_recordsWithoutErrorType() throws IOEx StorageHttpMetricsInterceptor interceptor = new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); - MockLowLevelHttpResponse lowLevelResponse = new MockLowLevelHttpResponse(); - lowLevelResponse.setStatusCode(308); - HttpTransport transport = - new MockHttpTransport.Builder().setLowLevelHttpResponse(lowLevelResponse).build(); + HttpTransport transport = createAdvancingTransport(308, 120_000_000L); HttpRequest request = transport @@ -208,7 +272,9 @@ public void initialize_308ResumeIncomplete_recordsWithoutErrorType() throws IOEx assertThat(durationData).isNotNull(); HistogramPointData point = durationData.getHistogramData().getPoints().iterator().next(); + assertThat(point.getSum()).isWithin(1e-6).of(0.12); Attributes attrs = point.getAttributes(); + assertThat(attrs.get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME)).isEqualTo("http"); assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)).isEqualTo(308L); assertThat(attrs.get(StorageClientMetrics.KEY_ERROR_TYPE)).isEqualTo("OK"); assertThat(attrs.get(StorageClientMetrics.KEY_SERVER_ADDRESS)) @@ -221,10 +287,7 @@ public void initialize_404NotFound_recordsErrorType() throws IOException { StorageHttpMetricsInterceptor interceptor = new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); - MockLowLevelHttpResponse lowLevelResponse = new MockLowLevelHttpResponse(); - lowLevelResponse.setStatusCode(404); - HttpTransport transport = - new MockHttpTransport.Builder().setLowLevelHttpResponse(lowLevelResponse).build(); + HttpTransport transport = createAdvancingTransport(404, 80_000_000L); HttpRequest request = transport @@ -244,7 +307,9 @@ public void initialize_404NotFound_recordsErrorType() throws IOException { assertThat(durationData).isNotNull(); HistogramPointData point = durationData.getHistogramData().getPoints().iterator().next(); + assertThat(point.getSum()).isWithin(1e-6).of(0.08); Attributes attrs = point.getAttributes(); + assertThat(attrs.get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME)).isEqualTo("http"); assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)).isEqualTo(404L); assertThat(attrs.get(StorageClientMetrics.KEY_ERROR_TYPE)).isEqualTo("NOT_FOUND"); assertThat(attrs.get(StorageClientMetrics.KEY_SERVER_ADDRESS)) @@ -257,10 +322,7 @@ public void initialize_503ServiceUnavailable_recordsErrorType() throws IOExcepti StorageHttpMetricsInterceptor interceptor = new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); - MockLowLevelHttpResponse lowLevelResponse = new MockLowLevelHttpResponse(); - lowLevelResponse.setStatusCode(503); - HttpTransport transport = - new MockHttpTransport.Builder().setLowLevelHttpResponse(lowLevelResponse).build(); + HttpTransport transport = createAdvancingTransport(503, 400_000_000L); HttpRequest request = transport @@ -280,7 +342,9 @@ public void initialize_503ServiceUnavailable_recordsErrorType() throws IOExcepti assertThat(durationData).isNotNull(); HistogramPointData point = durationData.getHistogramData().getPoints().iterator().next(); + assertThat(point.getSum()).isWithin(1e-6).of(0.40); Attributes attrs = point.getAttributes(); + assertThat(attrs.get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME)).isEqualTo("http"); assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)).isEqualTo(503L); assertThat(attrs.get(StorageClientMetrics.KEY_ERROR_TYPE)).isEqualTo("UNAVAILABLE"); assertThat(attrs.get(StorageClientMetrics.KEY_SERVER_ADDRESS)) @@ -288,7 +352,7 @@ public void initialize_503ServiceUnavailable_recordsErrorType() throws IOExcepti } @Test - public void initialize_networkIOException_recordsErrorTypeWithoutStatusCode() { + public void initialize_networkIOException_recordsErrorTypeWithoutStatusCode() throws IOException { StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); StorageHttpMetricsInterceptor interceptor = new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); @@ -300,22 +364,19 @@ public LowLevelHttpRequest buildRequest(String method, String url) { return new MockLowLevelHttpRequest() { @Override public LowLevelHttpResponse execute() throws IOException { + clock.advanceNanos(150_000_000L); throw new IOException("Connection reset by peer"); } }; } }; - assertThrows( - IOException.class, - () -> { - HttpRequest req = - transport - .createRequestFactory(interceptor) - .buildGetRequest( - new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b")); - req.execute(); - }); + HttpRequest req = + transport + .createRequestFactory(interceptor) + .buildGetRequest(new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b")); + + assertThrows(IOException.class, req::execute); Collection metricData = reader.collectAllMetrics(); MetricData durationData = @@ -327,13 +388,74 @@ public LowLevelHttpResponse execute() throws IOException { assertThat(durationData).isNotNull(); HistogramPointData point = durationData.getHistogramData().getPoints().iterator().next(); + assertThat(point.getSum()).isWithin(1e-6).of(0.15); Attributes attrs = point.getAttributes(); + assertThat(attrs.get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME)).isEqualTo("http"); assertThat(attrs.get(StorageClientMetrics.KEY_ERROR_TYPE)).isEqualTo("CONNECTIVITY"); assertThat(attrs.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)).isEqualTo(0L); assertThat(attrs.get(StorageClientMetrics.KEY_SERVER_ADDRESS)) .isEqualTo("storage.googleapis.com"); } + @Test + public void initialize_ioExceptionBeforeExecuteInterceptorRuns_doesNotRecordMetric() + throws IOException { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); + StorageHttpMetricsInterceptor interceptor = + new StorageHttpMetricsInterceptor(null, metrics, clock, "https://storage.googleapis.com"); + + HttpTransport transport = new MockHttpTransport(); + HttpRequest req = + transport + .createRequestFactory(interceptor) + .buildGetRequest(new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b")); + + clock.advanceNanos(500_000_000L); + boolean handled = req.getIOExceptionHandler().handleIOException(req, false); + + assertThat(handled).isFalse(); + assertThat(reader.collectAllMetrics()).isEmpty(); + } + + @Test + public void initialize_chainsExistingDelegateAndInterceptors() throws IOException { + StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, true, false); + AtomicBoolean delegateCalled = new AtomicBoolean(false); + AtomicBoolean execCalled = new AtomicBoolean(false); + AtomicBoolean respCalled = new AtomicBoolean(false); + AtomicBoolean ioHandlerCalled = new AtomicBoolean(false); + + HttpRequestInitializer delegate = + request -> { + delegateCalled.set(true); + request.setInterceptor(req -> execCalled.set(true)); + request.setResponseInterceptor(resp -> respCalled.set(true)); + request.setIOExceptionHandler( + (req, supportsRetry) -> { + ioHandlerCalled.set(true); + return false; + }); + }; + + StorageHttpMetricsInterceptor interceptor = + new StorageHttpMetricsInterceptor( + delegate, metrics, clock, "https://storage.googleapis.com"); + + HttpTransport transport = createAdvancingTransport(200, 100_000_000L); + HttpRequest request = + transport + .createRequestFactory(interceptor) + .buildGetRequest(new GenericUrl("https://storage.googleapis.com/storage/v1/b/my-b")); + request.execute(); + + assertThat(delegateCalled.get()).isTrue(); + assertThat(execCalled.get()).isTrue(); + assertThat(respCalled.get()).isTrue(); + + request.getIOExceptionHandler().handleIOException(request, false); + assertThat(ioHandlerCalled.get()).isTrue(); + } + @Test public void initialize_metricsDisabled_doesNotRecord() throws IOException { StorageClientMetrics metrics = StorageClientMetrics.create(meterProvider, false, false); @@ -354,6 +476,23 @@ public void initialize_metricsDisabled_doesNotRecord() throws IOException { assertThat(reader.collectAllMetrics()).isEmpty(); } + private HttpTransport createAdvancingTransport(int statusCode, long advanceNanos) { + return new MockHttpTransport() { + @Override + public LowLevelHttpRequest buildRequest(String method, String url) { + return new MockLowLevelHttpRequest() { + @Override + public LowLevelHttpResponse execute() { + clock.advanceNanos(advanceNanos); + MockLowLevelHttpResponse response = new MockLowLevelHttpResponse(); + response.setStatusCode(statusCode); + return response; + } + }; + } + }; + } + private static final class FakeClock implements ApiClock { private final AtomicLong nanos = new AtomicLong(1_000_000_000L);