From edb3def7d216bab555919a11776117ded2623d88 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 05:37:44 +0000 Subject: [PATCH 1/6] feat(storage): add StorageMetricsContext for operation metrics state [Generated-by: AI] --- .../cloud/storage/StorageMetricsContext.java | 326 ++++++++++++++++++ .../storage/StorageMetricsContextTest.java | 234 +++++++++++++ 2 files changed, 560 insertions(+) create mode 100644 java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java create mode 100644 java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java new file mode 100644 index 000000000000..bae073e1e19f --- /dev/null +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java @@ -0,0 +1,326 @@ +/* + * 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 java.util.Objects.requireNonNull; + +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.rpc.ApiCallContext; +import io.grpc.CallOptions; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * Package-private holder for operation metric state across asynchronous and thread-hopping + * boundaries. + */ +final class StorageMetricsContext { + + static final CallOptions.Key GRPC_CALL_OPTIONS_KEY = + CallOptions.Key.create("storage-metrics-context"); + static final ApiCallContext.Key API_CALL_CONTEXT_KEY = + ApiCallContext.Key.create("storage-metrics-context"); + static final String HTTP_REQUEST_PROPERTY_KEY = "com.google.cloud.storage.StorageMetricsContext"; + + private final String operationName; + private final long startTimeNanos; + private final Context otelContext; + private final AtomicInteger attemptCount; + private final AtomicLong requestBytes; + private final AtomicLong responseBytes; + private final Attributes baseAttributes; + private final boolean recordOperationOnRetrierExit; + private final AtomicBoolean operationRecorded; + + StorageMetricsContext(String operationName, Attributes baseAttributes) { + this(operationName, baseAttributes, System.nanoTime(), Context.current()); + } + + StorageMetricsContext(String operationName, Attributes baseAttributes, long startTimeNanos) { + this(operationName, baseAttributes, startTimeNanos, Context.current()); + } + + StorageMetricsContext( + String operationName, Attributes baseAttributes, long startTimeNanos, Context otelContext) { + this( + operationName, + baseAttributes, + startTimeNanos, + otelContext, + true, + new AtomicBoolean(false), + new AtomicInteger(0), + new AtomicLong(0L), + new AtomicLong(0L)); + } + + StorageMetricsContext( + String operationName, + Attributes baseAttributes, + long startTimeNanos, + Context otelContext, + boolean recordOperationOnRetrierExit, + AtomicBoolean operationRecorded, + AtomicInteger attemptCount, + AtomicLong requestBytes, + AtomicLong responseBytes) { + this.operationName = requireNonNull(operationName, "operationName cannot be null"); + this.baseAttributes = requireNonNull(baseAttributes, "baseAttributes cannot be null"); + this.otelContext = requireNonNull(otelContext, "otelContext cannot be null"); + this.startTimeNanos = startTimeNanos; + this.recordOperationOnRetrierExit = recordOperationOnRetrierExit; + this.operationRecorded = requireNonNull(operationRecorded, "operationRecorded cannot be null"); + this.attemptCount = requireNonNull(attemptCount, "attemptCount cannot be null"); + this.requestBytes = requireNonNull(requestBytes, "requestBytes cannot be null"); + this.responseBytes = requireNonNull(responseBytes, "responseBytes cannot be null"); + } + + static StorageMetricsContext create(String operationName) { + return create(operationName, Attributes.empty()); + } + + static StorageMetricsContext create(String operationName, Attributes baseAttributes) { + return create(operationName, baseAttributes, Context.current()); + } + + static StorageMetricsContext create( + String operationName, Attributes baseAttributes, Context otelContext) { + return new StorageMetricsContext( + operationName, + baseAttributes, + System.nanoTime(), + otelContext, + true, + new AtomicBoolean(false), + new AtomicInteger(0), + new AtomicLong(0L), + new AtomicLong(0L)); + } + + static StorageMetricsContext createForMultiStep(String operationName) { + return createForMultiStep(operationName, Attributes.empty()); + } + + static StorageMetricsContext createForMultiStep(String operationName, Attributes baseAttributes) { + return createForMultiStep(operationName, baseAttributes, Context.current()); + } + + static StorageMetricsContext createForMultiStep( + String operationName, Attributes baseAttributes, Context otelContext) { + return new StorageMetricsContext( + operationName, + baseAttributes, + System.nanoTime(), + otelContext, + false, + new AtomicBoolean(false), + new AtomicInteger(0), + new AtomicLong(0L), + new AtomicLong(0L)); + } + + StorageMetricsContext asSubAttemptContext() { + return new StorageMetricsContext( + operationName, + baseAttributes, + startTimeNanos, + otelContext, + false, + operationRecorded, + attemptCount, + requestBytes, + responseBytes); + } + + boolean shouldRecordOperationOnRetrierExit() { + return recordOperationOnRetrierExit; + } + + void recordOperation( + @Nullable StorageClientMetrics metrics, @Nullable Throwable terminalThrowable) { + if (metrics == null || !operationRecorded.compareAndSet(false, true)) { + return; + } + try (Scope ignore = otelContext.makeCurrent()) { + String terminalErrorType = StorageErrorTypeMapper.mapThrowable(terminalThrowable); + String clientMethod = baseAttributes.get(StorageClientMetrics.KEY_GCP_CLIENT_METHOD); + if (clientMethod == null || clientMethod.isEmpty()) { + clientMethod = operationName; + } + + AttributesBuilder opAttrBuilder = Attributes.builder(); + opAttrBuilder.putAll(metrics.getClientAttributes()); + baseAttributes.forEach( + (key, val) -> { + if (!key.equals(StorageClientMetrics.KEY_RPC_METHOD) + && !key.equals(StorageClientMetrics.KEY_NETWORK_PATH) + && !key.equals(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE) + && !key.equals(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) { + @SuppressWarnings("unchecked") + AttributeKey objKey = (AttributeKey) key; + opAttrBuilder.put(objKey, val); + } + }); + opAttrBuilder.put(StorageClientMetrics.KEY_GCP_STORAGE_OPERATION, operationName); + opAttrBuilder.put(StorageClientMetrics.KEY_GCP_CLIENT_METHOD, clientMethod); + opAttrBuilder.put(StorageClientMetrics.KEY_ERROR_TYPE, terminalErrorType); + Attributes operationAttributes = opAttrBuilder.build(); + + if (metrics.getOperations() != null) { + metrics.getOperations().add(1, operationAttributes); + } + + if (metrics.getGcpClientRequestDuration() != null) { + String rpcMethod = baseAttributes.get(StorageClientMetrics.KEY_RPC_METHOD); + if (rpcMethod == null || !rpcMethod.contains("/")) { + rpcMethod = "google.storage.v2.Storage/" + operationName; + } + Attributes durationAttributes = + operationAttributes.toBuilder() + .put(StorageClientMetrics.KEY_RPC_METHOD, rpcMethod) + .build(); + metrics.getGcpClientRequestDuration().record(getElapsedSeconds(), durationAttributes); + } + } + } + + static @Nullable StorageMetricsContext from(@Nullable CallOptions callOptions) { + return callOptions != null ? callOptions.getOption(GRPC_CALL_OPTIONS_KEY) : null; + } + + static @Nullable StorageMetricsContext from(@Nullable GrpcCallContext context) { + if (context == null) { + return null; + } + StorageMetricsContext result = context.getOption(API_CALL_CONTEXT_KEY); + if (result != null) { + return result; + } + return from(context.getCallOptions()); + } + + static @Nullable StorageMetricsContext from(@Nullable ApiCallContext context) { + if (context == null) { + return null; + } + StorageMetricsContext result = context.getOption(API_CALL_CONTEXT_KEY); + if (result != null) { + return result; + } + if (context instanceof GrpcCallContext) { + return from(((GrpcCallContext) context).getCallOptions()); + } + return null; + } + + static @Nullable StorageMetricsContext from(@Nullable Map properties) { + if (properties == null) { + return null; + } + Object obj = properties.get(HTTP_REQUEST_PROPERTY_KEY); + return (obj instanceof StorageMetricsContext) ? (StorageMetricsContext) obj : null; + } + + CallOptions attach(@Nullable CallOptions callOptions) { + CallOptions options = callOptions != null ? callOptions : CallOptions.DEFAULT; + return options.withOption(GRPC_CALL_OPTIONS_KEY, this); + } + + GrpcCallContext attach(@Nullable GrpcCallContext context) { + GrpcCallContext callContext = context != null ? context : GrpcCallContext.createDefault(); + return callContext + .withCallOptions(attach(callContext.getCallOptions())) + .withOption(API_CALL_CONTEXT_KEY, this); + } + + ApiCallContext attach(@Nullable ApiCallContext context) { + if (context == null) { + return null; + } + if (context instanceof GrpcCallContext) { + return attach((GrpcCallContext) context); + } + return context.withOption(API_CALL_CONTEXT_KEY, this); + } + + Map attach(@Nullable Map properties) { + if (properties != null) { + properties.put(HTTP_REQUEST_PROPERTY_KEY, this); + } + return properties; + } + + int incrementAttempts() { + return attemptCount.incrementAndGet(); + } + + int getAttemptCount() { + return attemptCount.get(); + } + + long addRequestBytes(long bytes) { + return requestBytes.addAndGet(bytes); + } + + long getRequestBytes() { + return requestBytes.get(); + } + + long addResponseBytes(long bytes) { + return responseBytes.addAndGet(bytes); + } + + long getResponseBytes() { + return responseBytes.get(); + } + + String getOperationName() { + return operationName; + } + + long getStartTimeNanos() { + return startTimeNanos; + } + + Context getOtelContext() { + return otelContext; + } + + double getElapsedSeconds() { + return getElapsedSeconds(System.nanoTime()); + } + + double getElapsedSeconds(long currentTimeNanos) { + return (currentTimeNanos - startTimeNanos) / 1_000_000_000.0; + } + + Attributes getBaseAttributes() { + return baseAttributes; + } + + AttributesBuilder attributesBuilder() { + return Attributes.builder().putAll(baseAttributes); + } +} diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java new file mode 100644 index 000000000000..0a09523b5689 --- /dev/null +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java @@ -0,0 +1,234 @@ +/* + * 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.gax.grpc.GrpcCallContext; +import com.google.api.gax.rpc.ApiCallContext; +import io.grpc.CallOptions; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.context.Context; +import java.util.HashMap; +import java.util.Map; +import org.junit.Test; + +public final class StorageMetricsContextTest { + + @Test + public void create_initializesDefaultState() { + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + + assertThat(context.getOperationName()).isEqualTo("storage.objects.get"); + assertThat(context.getStartTimeNanos()).isGreaterThan(0L); + assertThat(context.getElapsedSeconds()).isAtLeast(0.0); + assertThat(context.getAttemptCount()).isEqualTo(0); + assertThat(context.getRequestBytes()).isEqualTo(0L); + assertThat(context.getResponseBytes()).isEqualTo(0L); + assertThat(context.getBaseAttributes()).isEqualTo(Attributes.empty()); + assertThat(context.getOtelContext()).isSameInstanceAs(Context.current()); + } + + @Test + public void create_withOtelContext() { + Context customContext = Context.root(); + StorageMetricsContext context = + StorageMetricsContext.create("storage.objects.get", Attributes.empty(), customContext); + assertThat(context.getOtelContext()).isSameInstanceAs(customContext); + } + + @Test + public void constructor_validatesNonNullArguments() { + assertThrows( + NullPointerException.class, () -> new StorageMetricsContext(null, Attributes.empty())); + assertThrows(NullPointerException.class, () -> new StorageMetricsContext("op", null)); + assertThrows( + NullPointerException.class, + () -> new StorageMetricsContext("op", Attributes.empty(), 0L, null)); + } + + @Test + public void create_withBaseAttributes() { + Attributes attrs = + Attributes.of( + AttributeKey.stringKey("gcp.client.service"), + "storage", + AttributeKey.stringKey("gcp.client.version"), + "2.74.0"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.insert", attrs); + + assertThat(context.getOperationName()).isEqualTo("storage.objects.insert"); + assertThat(context.getBaseAttributes()).isEqualTo(attrs); + assertThat(context.attributesBuilder().build()).isEqualTo(attrs); + } + + @Test + public void stateMutations_accumulateValues() { + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.write"); + + assertThat(context.incrementAttempts()).isEqualTo(1); + assertThat(context.incrementAttempts()).isEqualTo(2); + assertThat(context.getAttemptCount()).isEqualTo(2); + + assertThat(context.addRequestBytes(1024L)).isEqualTo(1024L); + assertThat(context.addRequestBytes(2048L)).isEqualTo(3072L); + assertThat(context.getRequestBytes()).isEqualTo(3072L); + + assertThat(context.addResponseBytes(512L)).isEqualTo(512L); + assertThat(context.addResponseBytes(512L)).isEqualTo(1024L); + assertThat(context.getResponseBytes()).isEqualTo(1024L); + } + + @Test + public void elapsedSeconds_computesCorrectly() { + long startTime = 1_000_000_000L; + StorageMetricsContext context = + new StorageMetricsContext("storage.objects.get", Attributes.empty(), startTime); + + long endTime = 2_500_000_000L; + double elapsed = context.getElapsedSeconds(endTime); + assertThat(elapsed).isEqualTo(1.5); + assertThat(context.getElapsedSeconds()).isAtLeast(0.0); + } + + @Test + public void transportAttachment_grpcCallOptions() { + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + CallOptions options = CallOptions.DEFAULT; + + CallOptions attachedOptions = context.attach(options); + assertThat(attachedOptions).isNotNull(); + + StorageMetricsContext extracted = StorageMetricsContext.from(attachedOptions); + assertThat(extracted).isSameInstanceAs(context); + + assertThat(StorageMetricsContext.from((CallOptions) null)).isNull(); + assertThat(StorageMetricsContext.from(CallOptions.DEFAULT)).isNull(); + } + + @Test + public void transportAttachment_grpcCallContext() { + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + GrpcCallContext callContext = GrpcCallContext.createDefault(); + + GrpcCallContext attachedContext = context.attach(callContext); + assertThat(attachedContext).isNotNull(); + + StorageMetricsContext extracted = StorageMetricsContext.from(attachedContext); + assertThat(extracted).isSameInstanceAs(context); + + assertThat(StorageMetricsContext.from((GrpcCallContext) null)).isNull(); + assertThat(StorageMetricsContext.from(GrpcCallContext.createDefault())).isNull(); + } + + @Test + public void transportAttachment_apiCallContext() { + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + ApiCallContext callContext = GrpcCallContext.createDefault(); + + ApiCallContext attachedContext = context.attach(callContext); + assertThat(attachedContext).isNotNull(); + + StorageMetricsContext extracted = StorageMetricsContext.from(attachedContext); + assertThat(extracted).isSameInstanceAs(context); + + assertThat(StorageMetricsContext.from((ApiCallContext) null)).isNull(); + } + + @Test + public void transportAttachment_apiCallContext_whenGrpcCallContext_attachesCallOptions() { + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + ApiCallContext callContext = GrpcCallContext.createDefault(); + + ApiCallContext attachedContext = context.attach(callContext); + assertThat(attachedContext).isNotNull(); + assertThat(attachedContext).isInstanceOf(GrpcCallContext.class); + + GrpcCallContext grpcCallContext = (GrpcCallContext) attachedContext; + assertThat(grpcCallContext.getOption(StorageMetricsContext.API_CALL_CONTEXT_KEY)) + .isSameInstanceAs(context); + assertThat( + grpcCallContext.getCallOptions().getOption(StorageMetricsContext.GRPC_CALL_OPTIONS_KEY)) + .isSameInstanceAs(context); + + StorageMetricsContext extracted = StorageMetricsContext.from(attachedContext); + assertThat(extracted).isSameInstanceAs(context); + + GrpcCallContext callOptionsOnlyContext = + GrpcCallContext.createDefault().withCallOptions(context.attach(CallOptions.DEFAULT)); + assertThat(StorageMetricsContext.from((ApiCallContext) callOptionsOnlyContext)) + .isSameInstanceAs(context); + } + + @Test + public void transportAttachment_propertiesMap() { + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + Map properties = new HashMap<>(); + + Map attachedMap = context.attach(properties); + assertThat(attachedMap).isSameInstanceAs(properties); + + StorageMetricsContext extracted = StorageMetricsContext.from(properties); + assertThat(extracted).isSameInstanceAs(context); + + assertThat(StorageMetricsContext.from((Map) null)).isNull(); + Map unattached = new HashMap<>(); + assertThat(StorageMetricsContext.from(unattached)).isNull(); + } + + @Test + public void multiStepContext_behavior() { + StorageMetricsContext parent = StorageMetricsContext.createForMultiStep("WriteObject"); + assertThat(parent.shouldRecordOperationOnRetrierExit()).isFalse(); + + StorageMetricsContext subAttempt = parent.asSubAttemptContext(); + assertThat(subAttempt.shouldRecordOperationOnRetrierExit()).isFalse(); + assertThat(subAttempt.getOperationName()).isEqualTo("WriteObject"); + + subAttempt.incrementAttempts(); + assertThat(parent.getAttemptCount()).isEqualTo(1); + assertThat(subAttempt.getAttemptCount()).isEqualTo(1); + } + + @Test + public void recordOperation_isIdempotent() { + io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader reader = + io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader.create(); + io.opentelemetry.sdk.metrics.SdkMeterProvider provider = + io.opentelemetry.sdk.metrics.SdkMeterProvider.builder() + .registerMetricReader(reader) + .build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + StorageMetricsContext context = StorageMetricsContext.createForMultiStep("WriteObject"); + context.recordOperation(metrics, null); + context.recordOperation(metrics, null); + + java.util.Collection collected = + reader.collectAllMetrics(); + long opCount = + collected.stream() + .filter( + m -> m.getName().equals(StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_OPERATIONS)) + .flatMap(m -> m.getLongSumData().getPoints().stream()) + .mapToLong(p -> p.getValue()) + .sum(); + assertThat(opCount).isEqualTo(1L); + } +} From 900248a15661ea2478f826f611d06577b7422aff Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 07:02:06 +0000 Subject: [PATCH 2/6] refactor(storage): address PR #14728 review comments on StorageMetricsContext [Generated-by: AI] --- .../cloud/storage/StorageMetricsContext.java | 46 +++++++------------ .../storage/StorageMetricsContextTest.java | 10 ++-- 2 files changed, 22 insertions(+), 34 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java index bae073e1e19f..48f644c15d53 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java @@ -54,29 +54,7 @@ final class StorageMetricsContext { private final boolean recordOperationOnRetrierExit; private final AtomicBoolean operationRecorded; - StorageMetricsContext(String operationName, Attributes baseAttributes) { - this(operationName, baseAttributes, System.nanoTime(), Context.current()); - } - - StorageMetricsContext(String operationName, Attributes baseAttributes, long startTimeNanos) { - this(operationName, baseAttributes, startTimeNanos, Context.current()); - } - - StorageMetricsContext( - String operationName, Attributes baseAttributes, long startTimeNanos, Context otelContext) { - this( - operationName, - baseAttributes, - startTimeNanos, - otelContext, - true, - new AtomicBoolean(false), - new AtomicInteger(0), - new AtomicLong(0L), - new AtomicLong(0L)); - } - - StorageMetricsContext( + private StorageMetricsContext( String operationName, Attributes baseAttributes, long startTimeNanos, @@ -97,6 +75,20 @@ final class StorageMetricsContext { this.responseBytes = requireNonNull(responseBytes, "responseBytes cannot be null"); } + static StorageMetricsContext create( + String operationName, Attributes baseAttributes, long startTimeNanos) { + return new StorageMetricsContext( + operationName, + baseAttributes, + startTimeNanos, + Context.current(), + true, + new AtomicBoolean(false), + new AtomicInteger(0), + new AtomicLong(0L), + new AtomicLong(0L)); + } + static StorageMetricsContext create(String operationName) { return create(operationName, Attributes.empty()); } @@ -225,14 +217,10 @@ void recordOperation( if (context == null) { return null; } - StorageMetricsContext result = context.getOption(API_CALL_CONTEXT_KEY); - if (result != null) { - return result; - } if (context instanceof GrpcCallContext) { - return from(((GrpcCallContext) context).getCallOptions()); + return from((GrpcCallContext) context); } - return null; + return context.getOption(API_CALL_CONTEXT_KEY); } static @Nullable StorageMetricsContext from(@Nullable Map properties) { diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java index 0a09523b5689..7ad089bfa9f5 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java @@ -54,13 +54,13 @@ public void create_withOtelContext() { } @Test - public void constructor_validatesNonNullArguments() { + public void create_validatesNonNullArguments() { assertThrows( - NullPointerException.class, () -> new StorageMetricsContext(null, Attributes.empty())); - assertThrows(NullPointerException.class, () -> new StorageMetricsContext("op", null)); + NullPointerException.class, () -> StorageMetricsContext.create(null, Attributes.empty())); + assertThrows(NullPointerException.class, () -> StorageMetricsContext.create("op", null)); assertThrows( NullPointerException.class, - () -> new StorageMetricsContext("op", Attributes.empty(), 0L, null)); + () -> StorageMetricsContext.create("op", Attributes.empty(), null)); } @Test @@ -99,7 +99,7 @@ public void stateMutations_accumulateValues() { public void elapsedSeconds_computesCorrectly() { long startTime = 1_000_000_000L; StorageMetricsContext context = - new StorageMetricsContext("storage.objects.get", Attributes.empty(), startTime); + StorageMetricsContext.create("storage.objects.get", Attributes.empty(), startTime); long endTime = 2_500_000_000L; double elapsed = context.getElapsedSeconds(endTime); From 4728da2e42a1ae2f9a016b662335aa2425c51f26 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 10:02:08 +0000 Subject: [PATCH 3/6] feat(storage): add attempt recording methods to StorageMetricsContext [Generated-by: AI] --- .../cloud/storage/StorageMetricsContext.java | 148 ++++++- .../storage/StorageMetricsContextTest.java | 373 +++++++++++++++++- 2 files changed, 490 insertions(+), 31 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java index 48f644c15d53..9f0b046507f2 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java @@ -133,19 +133,6 @@ static StorageMetricsContext createForMultiStep( new AtomicLong(0L)); } - StorageMetricsContext asSubAttemptContext() { - return new StorageMetricsContext( - operationName, - baseAttributes, - startTimeNanos, - otelContext, - false, - operationRecorded, - attemptCount, - requestBytes, - responseBytes); - } - boolean shouldRecordOperationOnRetrierExit() { return recordOperationOnRetrierExit; } @@ -198,6 +185,141 @@ void recordOperation( } } + void recordAttemptSuccess(@Nullable StorageClientMetrics metrics) { + if (metrics == null || metrics.getAttempts() == null) { + return; + } + try (Scope ignore = otelContext.makeCurrent()) { + Attributes attemptBase = buildAttemptBaseAttributes(metrics); + AttributesBuilder builder = + attemptBase.toBuilder() + .put(StorageClientMetrics.KEY_ERROR_TYPE, StorageErrorTypeMapper.STATUS_OK) + .put(StorageClientMetrics.KEY_NETWORK_PATH, resolveNetworkPath()); + String rpcSystemName = baseAttributes.get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME); + if ("grpc".equals(rpcSystemName)) { + builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "OK"); + } else if ("http".equals(rpcSystemName)) { + builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 200L); + } + String rpcCode = baseAttributes.get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE); + if (rpcCode != null) { + builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcCode); + } + Long httpCode = baseAttributes.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE); + if (httpCode != null) { + builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, httpCode); + } + metrics.getAttempts().add(1, builder.build()); + } + } + + void recordAttemptFailure(@Nullable StorageClientMetrics metrics, Throwable e) { + if (metrics == null) { + return; + } + try (Scope ignore = otelContext.makeCurrent()) { + String errorType = StorageErrorTypeMapper.mapThrowable(e); + String networkPath = resolveNetworkPath(); + Attributes attemptBase = buildAttemptBaseAttributes(metrics); + if (metrics.getErrors() != null) { + Attributes errorAttributes = + attemptBase.toBuilder() + .put(StorageClientMetrics.KEY_ERROR_TYPE, errorType) + .put(StorageClientMetrics.KEY_NETWORK_PATH, networkPath) + .build(); + metrics.getErrors().add(1, errorAttributes); + } + if (metrics.getAttempts() != null) { + AttributesBuilder builder = + attemptBase.toBuilder() + .put(StorageClientMetrics.KEY_ERROR_TYPE, errorType) + .put(StorageClientMetrics.KEY_NETWORK_PATH, networkPath); + String rpcStatus = extractRpcStatusCode(e); + if (rpcStatus != null) { + builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcStatus); + } + Long httpStatus = extractHttpStatusCode(e); + if (httpStatus != null) { + builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, httpStatus); + } + metrics.getAttempts().add(1, builder.build()); + } + } + } + + private Attributes buildAttemptBaseAttributes(StorageClientMetrics metrics) { + AttributesBuilder baseBuilder = Attributes.builder(); + baseBuilder.putAll(metrics.getClientAttributes()); + baseAttributes.forEach( + (key, val) -> { + if (!key.equals(StorageClientMetrics.KEY_RPC_METHOD) + && !key.equals(StorageClientMetrics.KEY_NETWORK_PATH) + && !key.equals(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE) + && !key.equals(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) { + @SuppressWarnings("unchecked") + AttributeKey objKey = (AttributeKey) key; + baseBuilder.put(objKey, val); + } + }); + baseBuilder.put(StorageClientMetrics.KEY_GCP_STORAGE_OPERATION, operationName); + return baseBuilder.build(); + } + + private String resolveNetworkPath() { + String path = baseAttributes.get(StorageClientMetrics.KEY_NETWORK_PATH); + if (path != null && !path.isEmpty()) { + return path; + } + return StorageClientMetrics.NETWORK_PATH_CLOUDPATH; + } + + @Nullable + private String extractRpcStatusCode(Throwable e) { + String fromContext = baseAttributes.get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE); + if (fromContext != null) { + return fromContext; + } + Throwable curr = e; + while (curr != null) { + if (curr instanceof io.grpc.StatusRuntimeException) { + return ((io.grpc.StatusRuntimeException) curr).getStatus().getCode().name(); + } + if (curr instanceof io.grpc.StatusException) { + return ((io.grpc.StatusException) curr).getStatus().getCode().name(); + } + if (curr instanceof com.google.api.gax.rpc.ApiException) { + return ((com.google.api.gax.rpc.ApiException) curr).getStatusCode().getCode().name(); + } + curr = curr.getCause(); + } + return null; + } + + @Nullable + private Long extractHttpStatusCode(Throwable e) { + Long fromContext = baseAttributes.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE); + if (fromContext != null) { + return fromContext; + } + Throwable curr = e; + while (curr != null) { + if (curr instanceof StorageException) { + int code = ((StorageException) curr).getCode(); + if (code > 0) { + return (long) code; + } + } + if (curr instanceof com.google.api.client.http.HttpResponseException) { + int code = ((com.google.api.client.http.HttpResponseException) curr).getStatusCode(); + if (code > 0) { + return (long) code; + } + } + curr = curr.getCause(); + } + return null; + } + static @Nullable StorageMetricsContext from(@Nullable CallOptions callOptions) { return callOptions != null ? callOptions.getOption(GRPC_CALL_OPTIONS_KEY) : null; } diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java index 7ad089bfa9f5..e8beec487a8e 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java @@ -19,14 +19,27 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; +import com.google.api.client.http.HttpHeaders; +import com.google.api.client.http.HttpResponseException; import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.grpc.GrpcStatusCode; import com.google.api.gax.rpc.ApiCallContext; +import com.google.api.gax.rpc.ApiException; import io.grpc.CallOptions; +import io.grpc.Status; +import io.grpc.StatusException; +import io.grpc.StatusRuntimeException; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.context.Context; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.data.LongPointData; +import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; +import java.util.Collection; import java.util.HashMap; import java.util.Map; +import org.checkerframework.checker.nullness.qual.Nullable; import org.junit.Test; public final class StorageMetricsContextTest { @@ -194,41 +207,365 @@ public void transportAttachment_propertiesMap() { @Test public void multiStepContext_behavior() { - StorageMetricsContext parent = StorageMetricsContext.createForMultiStep("WriteObject"); - assertThat(parent.shouldRecordOperationOnRetrierExit()).isFalse(); + StorageMetricsContext context1 = StorageMetricsContext.createForMultiStep("WriteObject"); + assertThat(context1.shouldRecordOperationOnRetrierExit()).isFalse(); + assertThat(context1.getOperationName()).isEqualTo("WriteObject"); + assertThat(context1.getBaseAttributes()).isEqualTo(Attributes.empty()); + assertThat(context1.getOtelContext()).isSameInstanceAs(Context.current()); + + Attributes customAttrs = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext context2 = + StorageMetricsContext.createForMultiStep("ReadObject", customAttrs); + assertThat(context2.shouldRecordOperationOnRetrierExit()).isFalse(); + assertThat(context2.getOperationName()).isEqualTo("ReadObject"); + assertThat(context2.getBaseAttributes()).isEqualTo(customAttrs); - StorageMetricsContext subAttempt = parent.asSubAttemptContext(); - assertThat(subAttempt.shouldRecordOperationOnRetrierExit()).isFalse(); - assertThat(subAttempt.getOperationName()).isEqualTo("WriteObject"); - - subAttempt.incrementAttempts(); - assertThat(parent.getAttemptCount()).isEqualTo(1); - assertThat(subAttempt.getAttemptCount()).isEqualTo(1); + Context customContext = Context.root(); + StorageMetricsContext context3 = + StorageMetricsContext.createForMultiStep("DeleteObject", customAttrs, customContext); + assertThat(context3.shouldRecordOperationOnRetrierExit()).isFalse(); + assertThat(context3.getOperationName()).isEqualTo("DeleteObject"); + assertThat(context3.getBaseAttributes()).isEqualTo(customAttrs); + assertThat(context3.getOtelContext()).isSameInstanceAs(customContext); + + context1.incrementAttempts(); + assertThat(context1.getAttemptCount()).isEqualTo(1); } @Test public void recordOperation_isIdempotent() { - io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader reader = - io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader.create(); - io.opentelemetry.sdk.metrics.SdkMeterProvider provider = - io.opentelemetry.sdk.metrics.SdkMeterProvider.builder() - .registerMetricReader(reader) - .build(); + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); StorageMetricsContext context = StorageMetricsContext.createForMultiStep("WriteObject"); context.recordOperation(metrics, null); context.recordOperation(metrics, null); - java.util.Collection collected = - reader.collectAllMetrics(); + Collection collected = reader.collectAllMetrics(); long opCount = collected.stream() .filter( m -> m.getName().equals(StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_OPERATIONS)) .flatMap(m -> m.getLongSumData().getPoints().stream()) - .mapToLong(p -> p.getValue()) + .mapToLong(LongPointData::getValue) .sum(); assertThat(opCount).isEqualTo(1L); } + + @Test + public void recordAttemptSuccess_nullMetrics_noOp() { + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + context.recordAttemptSuccess(null); + } + + @Test + public void recordAttemptSuccess_whenAttemptsNull_noOp() { + SdkMeterProvider provider = SdkMeterProvider.builder().build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, false, false); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + context.recordAttemptSuccess(metrics); + } + + @Test + public void recordAttemptSuccess_defaultAttributes() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + context.recordAttemptSuccess(metrics); + + Collection collected = reader.collectAllMetrics(); + LongPointData point = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(point.getValue()).isEqualTo(1L); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo(StorageErrorTypeMapper.STATUS_OK); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_GCP_STORAGE_OPERATION)) + .isEqualTo("storage.objects.get"); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isNull(); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isNull(); + // Errors counter must not be recorded on success + assertThat(findMetric(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS)) + .isNull(); + } + + @Test + public void recordAttemptSuccess_grpcSystemName_setsRpcStatusCodeOk() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + context.recordAttemptSuccess(metrics); + + Collection collected = reader.collectAllMetrics(); + LongPointData point = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("OK"); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isNull(); + } + + @Test + public void recordAttemptSuccess_httpSystemName_setsHttpStatusCode200() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + context.recordAttemptSuccess(metrics); + + Collection collected = reader.collectAllMetrics(); + LongPointData point = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isEqualTo(200L); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isNull(); + } + + @Test + public void recordAttemptSuccess_overridesFromBaseAttributes() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + Attributes base = + Attributes.builder() + .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc") + .put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "ALREADY_EXISTS") + .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 204L) + .put( + StorageClientMetrics.KEY_NETWORK_PATH, StorageClientMetrics.NETWORK_PATH_DIRECTPATH) + .build(); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + context.recordAttemptSuccess(metrics); + + Collection collected = reader.collectAllMetrics(); + LongPointData point = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("ALREADY_EXISTS"); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isEqualTo(204L); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + } + + @Test + public void recordAttemptFailure_nullMetrics_noOp() { + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + context.recordAttemptFailure(null, new RuntimeException("boom")); + } + + @Test + public void recordAttemptFailure_whenInstrumentsNull_noOp() { + SdkMeterProvider provider = SdkMeterProvider.builder().build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, false, false); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + context.recordAttemptFailure(metrics, new RuntimeException("err")); + } + + @Test + public void recordAttemptFailure_storageException() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + StorageException se = new StorageException(404, "Not Found"); + context.recordAttemptFailure(metrics, se); + + Collection collected = reader.collectAllMetrics(); + + // Verify attempts counter + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getValue()).isEqualTo(1L); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("NOT_FOUND"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isEqualTo(404L); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); + + // Verify errors counter + LongPointData errorPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + assertThat(errorPoint.getValue()).isEqualTo(1L); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("NOT_FOUND"); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_GCP_STORAGE_OPERATION)) + .isEqualTo("storage.objects.get"); + } + + @Test + public void recordAttemptFailure_statusRuntimeException() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + StatusRuntimeException sre = Status.NOT_FOUND.asRuntimeException(); + context.recordAttemptFailure(metrics, sre); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("NOT_FOUND"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("NOT_FOUND"); + + LongPointData errorPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("NOT_FOUND"); + } + + @Test + public void recordAttemptFailure_statusException() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + StatusException se = Status.PERMISSION_DENIED.asException(); + context.recordAttemptFailure(metrics, se); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("PERMISSION_DENIED"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("PERMISSION_DENIED"); + } + + @Test + public void recordAttemptFailure_apiException() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + ApiException apiException = + new ApiException( + new StatusRuntimeException(Status.ALREADY_EXISTS), + GrpcStatusCode.of(Status.Code.ALREADY_EXISTS), + false); + context.recordAttemptFailure(metrics, apiException); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("ALREADY_EXISTS"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("ALREADY_EXISTS"); + } + + @Test + public void recordAttemptFailure_httpResponseException() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + HttpResponseException hre = + new HttpResponseException.Builder(429, "Too Many Requests", new HttpHeaders()).build(); + context.recordAttemptFailure(metrics, hre); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("RESOURCE_EXHAUSTED"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isEqualTo(429L); + } + + @Test + public void recordAttemptFailure_nestedCause() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + Exception wrappedGrpc = + new RuntimeException(new IllegalStateException(Status.UNAVAILABLE.asRuntimeException())); + context.recordAttemptFailure(metrics, wrappedGrpc); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("UNAVAILABLE"); + + reader = InMemoryMetricReader.create(); + provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + metrics = StorageClientMetrics.create(provider, true, false); + Exception wrappedHttp = new RuntimeException(new StorageException(503, "Service Unavailable")); + context.recordAttemptFailure(metrics, wrappedHttp); + collected = reader.collectAllMetrics(); + attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isEqualTo(503L); + } + + @Test + public void recordAttemptFailure_overridesFromBaseAttributes() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + Attributes base = + Attributes.builder() + .put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "OVERRIDDEN_RPC") + .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 999L) + .put( + StorageClientMetrics.KEY_NETWORK_PATH, StorageClientMetrics.NETWORK_PATH_DIRECTPATH) + .build(); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + context.recordAttemptFailure(metrics, new StorageException(404, "Not Found")); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("OVERRIDDEN_RPC"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isEqualTo(999L); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + + LongPointData errorPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + } + + private static @Nullable MetricData findMetric( + Collection metrics, String metricName) { + return metrics.stream().filter(m -> m.getName().equals(metricName)).findFirst().orElse(null); + } + + private static LongPointData getSinglePoint(Collection metrics, String metricName) { + MetricData metric = findMetric(metrics, metricName); + assertThat(metric).isNotNull(); + assertThat(metric.getLongSumData().getPoints()).hasSize(1); + return metric.getLongSumData().getPoints().iterator().next(); + } } From 74d10a621a931178980ebb682e70c2c978cdfc5d Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 14:54:40 +0000 Subject: [PATCH 4/6] fix(storage): gate status code extraction on rpcSystemName in attempt recording [Generated-by: AI] --- .../cloud/storage/StorageMetricsContext.java | 34 ++-- .../storage/StorageMetricsContextTest.java | 173 +++++++++++++++++- 2 files changed, 181 insertions(+), 26 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java index 9f0b046507f2..8c09d93c62ae 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java @@ -197,17 +197,13 @@ void recordAttemptSuccess(@Nullable StorageClientMetrics metrics) { .put(StorageClientMetrics.KEY_NETWORK_PATH, resolveNetworkPath()); String rpcSystemName = baseAttributes.get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME); if ("grpc".equals(rpcSystemName)) { - builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "OK"); + String rpcCode = baseAttributes.get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE); + builder.put( + StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcCode != null ? rpcCode : "OK"); } else if ("http".equals(rpcSystemName)) { - builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 200L); - } - String rpcCode = baseAttributes.get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE); - if (rpcCode != null) { - builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcCode); - } - Long httpCode = baseAttributes.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE); - if (httpCode != null) { - builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, httpCode); + Long httpCode = baseAttributes.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE); + builder.put( + StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, httpCode != null ? httpCode : 200L); } metrics.getAttempts().add(1, builder.build()); } @@ -234,13 +230,17 @@ void recordAttemptFailure(@Nullable StorageClientMetrics metrics, Throwable e) { attemptBase.toBuilder() .put(StorageClientMetrics.KEY_ERROR_TYPE, errorType) .put(StorageClientMetrics.KEY_NETWORK_PATH, networkPath); - String rpcStatus = extractRpcStatusCode(e); - if (rpcStatus != null) { - builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcStatus); - } - Long httpStatus = extractHttpStatusCode(e); - if (httpStatus != null) { - builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, httpStatus); + String rpcSystemName = baseAttributes.get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME); + if ("grpc".equals(rpcSystemName)) { + String rpcStatus = extractRpcStatusCode(e); + builder.put( + StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, + rpcStatus != null ? rpcStatus : "UNKNOWN"); + } else if ("http".equals(rpcSystemName)) { + Long httpStatus = extractHttpStatusCode(e); + builder.put( + StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, + httpStatus != null ? httpStatus : 0L); } metrics.getAttempts().add(1, builder.build()); } diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java index e8beec487a8e..5dce84926ca5 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java @@ -334,7 +334,7 @@ public void recordAttemptSuccess_httpSystemName_setsHttpStatusCode200() { } @Test - public void recordAttemptSuccess_overridesFromBaseAttributes() { + public void recordAttemptSuccess_overridesFromBaseAttributes_grpc() { InMemoryMetricReader reader = InMemoryMetricReader.create(); SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); @@ -355,8 +355,36 @@ public void recordAttemptSuccess_overridesFromBaseAttributes() { getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) .isEqualTo("ALREADY_EXISTS"); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isNull(); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + } + + @Test + public void recordAttemptSuccess_overridesFromBaseAttributes_http() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + Attributes base = + Attributes.builder() + .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http") + .put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "ALREADY_EXISTS") + .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 204L) + .put( + StorageClientMetrics.KEY_NETWORK_PATH, StorageClientMetrics.NETWORK_PATH_DIRECTPATH) + .build(); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + context.recordAttemptSuccess(metrics); + + Collection collected = reader.collectAllMetrics(); + LongPointData point = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isEqualTo(204L); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isNull(); assertThat(point.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); } @@ -381,7 +409,8 @@ public void recordAttemptFailure_storageException() { SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); StorageException se = new StorageException(404, "Not Found"); context.recordAttemptFailure(metrics, se); @@ -395,6 +424,8 @@ public void recordAttemptFailure_storageException() { .isEqualTo("NOT_FOUND"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isEqualTo(404L); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isNull(); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); @@ -416,7 +447,8 @@ public void recordAttemptFailure_statusRuntimeException() { SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); StatusRuntimeException sre = Status.NOT_FOUND.asRuntimeException(); context.recordAttemptFailure(metrics, sre); @@ -427,6 +459,8 @@ public void recordAttemptFailure_statusRuntimeException() { .isEqualTo("NOT_FOUND"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) .isEqualTo("NOT_FOUND"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isNull(); LongPointData errorPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); @@ -440,7 +474,8 @@ public void recordAttemptFailure_statusException() { SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); StatusException se = Status.PERMISSION_DENIED.asException(); context.recordAttemptFailure(metrics, se); @@ -451,6 +486,8 @@ public void recordAttemptFailure_statusException() { .isEqualTo("PERMISSION_DENIED"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) .isEqualTo("PERMISSION_DENIED"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isNull(); } @Test @@ -459,7 +496,8 @@ public void recordAttemptFailure_apiException() { SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); ApiException apiException = new ApiException( new StatusRuntimeException(Status.ALREADY_EXISTS), @@ -474,6 +512,8 @@ public void recordAttemptFailure_apiException() { .isEqualTo("ALREADY_EXISTS"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) .isEqualTo("ALREADY_EXISTS"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isNull(); } @Test @@ -482,7 +522,8 @@ public void recordAttemptFailure_httpResponseException() { SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); HttpResponseException hre = new HttpResponseException.Builder(429, "Too Many Requests", new HttpHeaders()).build(); context.recordAttemptFailure(metrics, hre); @@ -494,6 +535,8 @@ public void recordAttemptFailure_httpResponseException() { .isEqualTo("RESOURCE_EXHAUSTED"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isEqualTo(429L); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isNull(); } @Test @@ -502,7 +545,8 @@ public void recordAttemptFailure_nestedCause() { SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + Attributes grpcBase = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", grpcBase); Exception wrappedGrpc = new RuntimeException(new IllegalStateException(Status.UNAVAILABLE.asRuntimeException())); context.recordAttemptFailure(metrics, wrappedGrpc); @@ -512,27 +556,35 @@ public void recordAttemptFailure_nestedCause() { getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) .isEqualTo("UNAVAILABLE"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isNull(); reader = InMemoryMetricReader.create(); provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); metrics = StorageClientMetrics.create(provider, true, false); + Attributes httpBase = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); + StorageMetricsContext httpContext = + StorageMetricsContext.create("storage.objects.get", httpBase); Exception wrappedHttp = new RuntimeException(new StorageException(503, "Service Unavailable")); - context.recordAttemptFailure(metrics, wrappedHttp); + httpContext.recordAttemptFailure(metrics, wrappedHttp); collected = reader.collectAllMetrics(); attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isEqualTo(503L); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isNull(); } @Test - public void recordAttemptFailure_overridesFromBaseAttributes() { + public void recordAttemptFailure_overridesFromBaseAttributes_grpc() { InMemoryMetricReader reader = InMemoryMetricReader.create(); SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); Attributes base = Attributes.builder() + .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc") .put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "OVERRIDDEN_RPC") .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 999L) .put( @@ -546,8 +598,41 @@ public void recordAttemptFailure_overridesFromBaseAttributes() { getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) .isEqualTo("OVERRIDDEN_RPC"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isNull(); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + + LongPointData errorPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + } + + @Test + public void recordAttemptFailure_overridesFromBaseAttributes_http() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + Attributes base = + Attributes.builder() + .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http") + .put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "OVERRIDDEN_RPC") + .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 999L) + .put( + StorageClientMetrics.KEY_NETWORK_PATH, StorageClientMetrics.NETWORK_PATH_DIRECTPATH) + .build(); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + context.recordAttemptFailure(metrics, new StorageException(404, "Not Found")); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isEqualTo(999L); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isNull(); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); @@ -557,6 +642,76 @@ public void recordAttemptFailure_overridesFromBaseAttributes() { .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); } + @Test + public void recordAttemptFailure_grpc_neverSetsHttpCode_evenForStorageException() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + + ApiException apiException = + new ApiException( + new StatusRuntimeException(Status.FAILED_PRECONDITION), + GrpcStatusCode.of(Status.Code.FAILED_PRECONDITION), + false); + StorageException se = new StorageException(412, "Precondition Failed", apiException); + context.recordAttemptFailure(metrics, se); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("FAILED_PRECONDITION"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isNull(); + } + + @Test + public void recordAttemptFailure_grpc_defaultsToUnknown_whenRpcStatusNull() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + + // RuntimeException without any gRPC or ApiException cause + context.recordAttemptFailure(metrics, new RuntimeException("generic failure")); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("UNKNOWN"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isNull(); + } + + @Test + public void recordAttemptFailure_http_neverSetsRpcCode_andDefaultsToZero_whenHttpStatusNull() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + + // StatusRuntimeException has a gRPC status code, but rpcSystemName is "http", + // and it has no HTTP status code (extractHttpStatusCode returns null). + StatusRuntimeException sre = Status.NOT_FOUND.asRuntimeException(); + context.recordAttemptFailure(metrics, sre); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isEqualTo(0L); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isNull(); + } + private static @Nullable MetricData findMetric( Collection metrics, String metricName) { return metrics.stream().filter(m -> m.getName().equals(metricName)).findFirst().orElse(null); From 198a662834a8b5e1bf9a8c4ca8558a93e51168df Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 15:14:11 +0000 Subject: [PATCH 5/6] test(storage): consolidate StorageMetricsContextTest and remove unused status overrides --- .../cloud/storage/StorageMetricsContext.java | 16 +- .../storage/StorageMetricsContextTest.java | 462 ++++++------------ 2 files changed, 154 insertions(+), 324 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java index 8c09d93c62ae..1db52c3677e3 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java @@ -197,13 +197,9 @@ void recordAttemptSuccess(@Nullable StorageClientMetrics metrics) { .put(StorageClientMetrics.KEY_NETWORK_PATH, resolveNetworkPath()); String rpcSystemName = baseAttributes.get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME); if ("grpc".equals(rpcSystemName)) { - String rpcCode = baseAttributes.get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE); - builder.put( - StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcCode != null ? rpcCode : "OK"); + builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "OK"); } else if ("http".equals(rpcSystemName)) { - Long httpCode = baseAttributes.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE); - builder.put( - StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, httpCode != null ? httpCode : 200L); + builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 200L); } metrics.getAttempts().add(1, builder.build()); } @@ -275,10 +271,6 @@ private String resolveNetworkPath() { @Nullable private String extractRpcStatusCode(Throwable e) { - String fromContext = baseAttributes.get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE); - if (fromContext != null) { - return fromContext; - } Throwable curr = e; while (curr != null) { if (curr instanceof io.grpc.StatusRuntimeException) { @@ -297,10 +289,6 @@ private String extractRpcStatusCode(Throwable e) { @Nullable private Long extractHttpStatusCode(Throwable e) { - Long fromContext = baseAttributes.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE); - if (fromContext != null) { - return fromContext; - } Throwable curr = e; while (curr != null) { if (curr instanceof StorageException) { diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java index 5dce84926ca5..c6a02a669c2c 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java @@ -27,7 +27,6 @@ import com.google.api.gax.rpc.ApiException; import io.grpc.CallOptions; import io.grpc.Status; -import io.grpc.StatusException; import io.grpc.StatusRuntimeException; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; @@ -40,10 +39,26 @@ import java.util.HashMap; import java.util.Map; import org.checkerframework.checker.nullness.qual.Nullable; +import org.junit.Before; import org.junit.Test; public final class StorageMetricsContextTest { + private InMemoryMetricReader reader; + private SdkMeterProvider provider; + private StorageClientMetrics metrics; + + @Before + public void setUp() { + resetMetrics(); + } + + private void resetMetrics() { + reader = InMemoryMetricReader.create(); + provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + metrics = StorageClientMetrics.create(provider, true, false); + } + @Test public void create_initializesDefaultState() { StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); @@ -155,20 +170,6 @@ public void transportAttachment_apiCallContext() { StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); ApiCallContext callContext = GrpcCallContext.createDefault(); - ApiCallContext attachedContext = context.attach(callContext); - assertThat(attachedContext).isNotNull(); - - StorageMetricsContext extracted = StorageMetricsContext.from(attachedContext); - assertThat(extracted).isSameInstanceAs(context); - - assertThat(StorageMetricsContext.from((ApiCallContext) null)).isNull(); - } - - @Test - public void transportAttachment_apiCallContext_whenGrpcCallContext_attachesCallOptions() { - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); - ApiCallContext callContext = GrpcCallContext.createDefault(); - ApiCallContext attachedContext = context.attach(callContext); assertThat(attachedContext).isNotNull(); assertThat(attachedContext).isInstanceOf(GrpcCallContext.class); @@ -187,6 +188,8 @@ public void transportAttachment_apiCallContext_whenGrpcCallContext_attachesCallO GrpcCallContext.createDefault().withCallOptions(context.attach(CallOptions.DEFAULT)); assertThat(StorageMetricsContext.from((ApiCallContext) callOptionsOnlyContext)) .isSameInstanceAs(context); + + assertThat(StorageMetricsContext.from((ApiCallContext) null)).isNull(); } @Test @@ -227,17 +230,10 @@ public void multiStepContext_behavior() { assertThat(context3.getOperationName()).isEqualTo("DeleteObject"); assertThat(context3.getBaseAttributes()).isEqualTo(customAttrs); assertThat(context3.getOtelContext()).isSameInstanceAs(customContext); - - context1.incrementAttempts(); - assertThat(context1.getAttemptCount()).isEqualTo(1); } @Test public void recordOperation_isIdempotent() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - StorageMetricsContext context = StorageMetricsContext.createForMultiStep("WriteObject"); context.recordOperation(metrics, null); context.recordOperation(metrics, null); @@ -254,25 +250,21 @@ public void recordOperation_isIdempotent() { } @Test - public void recordAttemptSuccess_nullMetrics_noOp() { + public void recordAttempt_nullOrDisabledMetrics_noOp() { StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + context.recordAttemptSuccess(null); - } + context.recordAttemptFailure(null, new RuntimeException("boom")); - @Test - public void recordAttemptSuccess_whenAttemptsNull_noOp() { - SdkMeterProvider provider = SdkMeterProvider.builder().build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, false, false); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); - context.recordAttemptSuccess(metrics); + StorageClientMetrics disabledMetrics = StorageClientMetrics.create(provider, false, false); + context.recordAttemptSuccess(disabledMetrics); + context.recordAttemptFailure(disabledMetrics, new RuntimeException("err")); + + assertThat(reader.collectAllMetrics()).isEmpty(); } @Test public void recordAttemptSuccess_defaultAttributes() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); context.recordAttemptSuccess(metrics); @@ -297,11 +289,12 @@ public void recordAttemptSuccess_defaultAttributes() { @Test public void recordAttemptSuccess_grpcSystemName_setsRpcStatusCodeOk() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - - Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + Attributes base = + Attributes.of( + StorageClientMetrics.KEY_RPC_SYSTEM_NAME, + "grpc", + StorageClientMetrics.KEY_NETWORK_PATH, + StorageClientMetrics.NETWORK_PATH_DIRECTPATH); StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); context.recordAttemptSuccess(metrics); @@ -312,14 +305,12 @@ public void recordAttemptSuccess_grpcSystemName_setsRpcStatusCodeOk() { .isEqualTo("OK"); assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isNull(); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); } @Test public void recordAttemptSuccess_httpSystemName_setsHttpStatusCode200() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); context.recordAttemptSuccess(metrics); @@ -334,222 +325,174 @@ public void recordAttemptSuccess_httpSystemName_setsHttpStatusCode200() { } @Test - public void recordAttemptSuccess_overridesFromBaseAttributes_grpc() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - - Attributes base = - Attributes.builder() - .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc") - .put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "ALREADY_EXISTS") - .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 204L) - .put( - StorageClientMetrics.KEY_NETWORK_PATH, StorageClientMetrics.NETWORK_PATH_DIRECTPATH) - .build(); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); - context.recordAttemptSuccess(metrics); - - Collection collected = reader.collectAllMetrics(); - LongPointData point = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); - assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isEqualTo("ALREADY_EXISTS"); - assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isNull(); - assertThat(point.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - } - - @Test - public void recordAttemptSuccess_overridesFromBaseAttributes_http() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - + public void recordAttemptFailure_grpc_extractsRpcStatusAndNeverSetsHttpCode() { Attributes base = - Attributes.builder() - .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http") - .put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "ALREADY_EXISTS") - .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 204L) - .put( - StorageClientMetrics.KEY_NETWORK_PATH, StorageClientMetrics.NETWORK_PATH_DIRECTPATH) - .build(); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); - context.recordAttemptSuccess(metrics); - - Collection collected = reader.collectAllMetrics(); - LongPointData point = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); - assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isEqualTo(204L); - assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isNull(); - assertThat(point.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - } - - @Test - public void recordAttemptFailure_nullMetrics_noOp() { - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); - context.recordAttemptFailure(null, new RuntimeException("boom")); - } - - @Test - public void recordAttemptFailure_whenInstrumentsNull_noOp() { - SdkMeterProvider provider = SdkMeterProvider.builder().build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, false, false); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); - context.recordAttemptFailure(metrics, new RuntimeException("err")); - } - - @Test - public void recordAttemptFailure_storageException() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - - Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); + Attributes.of( + StorageClientMetrics.KEY_RPC_SYSTEM_NAME, + "grpc", + StorageClientMetrics.KEY_NETWORK_PATH, + StorageClientMetrics.NETWORK_PATH_DIRECTPATH); StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); - StorageException se = new StorageException(404, "Not Found"); - context.recordAttemptFailure(metrics, se); + // StatusRuntimeException (NOT_FOUND) + context.recordAttemptFailure(metrics, Status.NOT_FOUND.asRuntimeException()); Collection collected = reader.collectAllMetrics(); - - // Verify attempts counter LongPointData attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); - assertThat(attemptPoint.getValue()).isEqualTo(1L); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) .isEqualTo("NOT_FOUND"); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isEqualTo(404L); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("NOT_FOUND"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isNull(); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - // Verify errors counter LongPointData errorPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); - assertThat(errorPoint.getValue()).isEqualTo(1L); assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) .isEqualTo("NOT_FOUND"); assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_GCP_STORAGE_OPERATION)) - .isEqualTo("storage.objects.get"); - } + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - @Test - public void recordAttemptFailure_statusRuntimeException() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + // StatusException (PERMISSION_DENIED) + resetMetrics(); + context.recordAttemptFailure(metrics, Status.PERMISSION_DENIED.asException()); + collected = reader.collectAllMetrics(); + attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("PERMISSION_DENIED"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("PERMISSION_DENIED"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isNull(); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); - StatusRuntimeException sre = Status.NOT_FOUND.asRuntimeException(); - context.recordAttemptFailure(metrics, sre); + errorPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("PERMISSION_DENIED"); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - Collection collected = reader.collectAllMetrics(); - LongPointData attemptPoint = + // ApiException (ALREADY_EXISTS) + resetMetrics(); + ApiException apiException = + new ApiException( + new StatusRuntimeException(Status.ALREADY_EXISTS), + GrpcStatusCode.of(Status.Code.ALREADY_EXISTS), + false); + context.recordAttemptFailure(metrics, apiException); + collected = reader.collectAllMetrics(); + attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) - .isEqualTo("NOT_FOUND"); + .isEqualTo("ALREADY_EXISTS"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isEqualTo("NOT_FOUND"); + .isEqualTo("ALREADY_EXISTS"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isNull(); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - LongPointData errorPoint = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + errorPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) - .isEqualTo("NOT_FOUND"); - } - - @Test - public void recordAttemptFailure_statusException() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + .isEqualTo("ALREADY_EXISTS"); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); - StatusException se = Status.PERMISSION_DENIED.asException(); + // StorageException (412 wrapping ApiException FAILED_PRECONDITION) + resetMetrics(); + ApiException innerApiException = + new ApiException( + new StatusRuntimeException(Status.FAILED_PRECONDITION), + GrpcStatusCode.of(Status.Code.FAILED_PRECONDITION), + false); + StorageException se = new StorageException(412, "Precondition Failed", innerApiException); context.recordAttemptFailure(metrics, se); - - Collection collected = reader.collectAllMetrics(); - LongPointData attemptPoint = + collected = reader.collectAllMetrics(); + attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) - .isEqualTo("PERMISSION_DENIED"); + .isEqualTo("FAILED_PRECONDITION"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isEqualTo("PERMISSION_DENIED"); + .isEqualTo("FAILED_PRECONDITION"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isNull(); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + + errorPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("FAILED_PRECONDITION"); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); } @Test - public void recordAttemptFailure_apiException() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - - Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + public void recordAttemptFailure_http_extractsHttpStatusAndNeverSetsRpcCode() { + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); - ApiException apiException = - new ApiException( - new StatusRuntimeException(Status.ALREADY_EXISTS), - GrpcStatusCode.of(Status.Code.ALREADY_EXISTS), - false); - context.recordAttemptFailure(metrics, apiException); + + // StorageException with HTTP code 404 (NOT_FOUND) + StorageException se = new StorageException(404, "Not Found"); + context.recordAttemptFailure(metrics, se); Collection collected = reader.collectAllMetrics(); LongPointData attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getValue()).isEqualTo(1L); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) - .isEqualTo("ALREADY_EXISTS"); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isEqualTo("ALREADY_EXISTS"); + .isEqualTo("NOT_FOUND"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isEqualTo(404L); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) .isNull(); - } + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); - @Test - public void recordAttemptFailure_httpResponseException() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); + LongPointData errorPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + assertThat(errorPoint.getValue()).isEqualTo(1L); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("NOT_FOUND"); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_GCP_STORAGE_OPERATION)) + .isEqualTo("storage.objects.get"); - Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + // HttpResponseException with HTTP code 429 (RESOURCE_EXHAUSTED) + resetMetrics(); HttpResponseException hre = new HttpResponseException.Builder(429, "Too Many Requests", new HttpHeaders()).build(); context.recordAttemptFailure(metrics, hre); - Collection collected = reader.collectAllMetrics(); - LongPointData attemptPoint = + collected = reader.collectAllMetrics(); + attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getValue()).isEqualTo(1L); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) .isEqualTo("RESOURCE_EXHAUSTED"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isEqualTo(429L); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) .isNull(); + + errorPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + assertThat(errorPoint.getValue()).isEqualTo(1L); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("RESOURCE_EXHAUSTED"); } @Test - public void recordAttemptFailure_nestedCause() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - + public void recordAttemptFailure_nestedCause_extractsStatusFromCauseChain() { Attributes grpcBase = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", grpcBase); + StorageMetricsContext grpcContext = + StorageMetricsContext.create("storage.objects.get", grpcBase); Exception wrappedGrpc = new RuntimeException(new IllegalStateException(Status.UNAVAILABLE.asRuntimeException())); - context.recordAttemptFailure(metrics, wrappedGrpc); + grpcContext.recordAttemptFailure(metrics, wrappedGrpc); Collection collected = reader.collectAllMetrics(); LongPointData attemptPoint = @@ -559,14 +502,13 @@ public void recordAttemptFailure_nestedCause() { assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isNull(); - reader = InMemoryMetricReader.create(); - provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - metrics = StorageClientMetrics.create(provider, true, false); + resetMetrics(); Attributes httpBase = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); StorageMetricsContext httpContext = StorageMetricsContext.create("storage.objects.get", httpBase); Exception wrappedHttp = new RuntimeException(new StorageException(503, "Service Unavailable")); httpContext.recordAttemptFailure(metrics, wrappedHttp); + collected = reader.collectAllMetrics(); attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); @@ -577,108 +519,13 @@ public void recordAttemptFailure_nestedCause() { } @Test - public void recordAttemptFailure_overridesFromBaseAttributes_grpc() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - - Attributes base = - Attributes.builder() - .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc") - .put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "OVERRIDDEN_RPC") - .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 999L) - .put( - StorageClientMetrics.KEY_NETWORK_PATH, StorageClientMetrics.NETWORK_PATH_DIRECTPATH) - .build(); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); - context.recordAttemptFailure(metrics, new StorageException(404, "Not Found")); - - Collection collected = reader.collectAllMetrics(); - LongPointData attemptPoint = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isEqualTo("OVERRIDDEN_RPC"); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isNull(); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - - LongPointData errorPoint = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - } - - @Test - public void recordAttemptFailure_overridesFromBaseAttributes_http() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - - Attributes base = - Attributes.builder() - .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http") - .put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "OVERRIDDEN_RPC") - .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 999L) - .put( - StorageClientMetrics.KEY_NETWORK_PATH, StorageClientMetrics.NETWORK_PATH_DIRECTPATH) - .build(); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); - context.recordAttemptFailure(metrics, new StorageException(404, "Not Found")); - - Collection collected = reader.collectAllMetrics(); - LongPointData attemptPoint = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isEqualTo(999L); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isNull(); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - - LongPointData errorPoint = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - } - - @Test - public void recordAttemptFailure_grpc_neverSetsHttpCode_evenForStorageException() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - - Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); - - ApiException apiException = - new ApiException( - new StatusRuntimeException(Status.FAILED_PRECONDITION), - GrpcStatusCode.of(Status.Code.FAILED_PRECONDITION), - false); - StorageException se = new StorageException(412, "Precondition Failed", apiException); - context.recordAttemptFailure(metrics, se); - - Collection collected = reader.collectAllMetrics(); - LongPointData attemptPoint = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isEqualTo("FAILED_PRECONDITION"); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isNull(); - } - - @Test - public void recordAttemptFailure_grpc_defaultsToUnknown_whenRpcStatusNull() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - - Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + public void recordAttemptFailure_missingStatusCode_defaultsToUnknownOrZero() { + Attributes grpcBase = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext grpcContext = + StorageMetricsContext.create("storage.objects.get", grpcBase); // RuntimeException without any gRPC or ApiException cause - context.recordAttemptFailure(metrics, new RuntimeException("generic failure")); + grpcContext.recordAttemptFailure(metrics, new RuntimeException("generic failure")); Collection collected = reader.collectAllMetrics(); LongPointData attemptPoint = @@ -687,24 +534,19 @@ public void recordAttemptFailure_grpc_defaultsToUnknown_whenRpcStatusNull() { .isEqualTo("UNKNOWN"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isNull(); - } - - @Test - public void recordAttemptFailure_http_neverSetsRpcCode_andDefaultsToZero_whenHttpStatusNull() { - InMemoryMetricReader reader = InMemoryMetricReader.create(); - SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); - Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); - StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + resetMetrics(); + Attributes httpBase = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); + StorageMetricsContext httpContext = + StorageMetricsContext.create("storage.objects.get", httpBase); // StatusRuntimeException has a gRPC status code, but rpcSystemName is "http", // and it has no HTTP status code (extractHttpStatusCode returns null). StatusRuntimeException sre = Status.NOT_FOUND.asRuntimeException(); - context.recordAttemptFailure(metrics, sre); + httpContext.recordAttemptFailure(metrics, sre); - Collection collected = reader.collectAllMetrics(); - LongPointData attemptPoint = + collected = reader.collectAllMetrics(); + attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) .isEqualTo(0L); From 1be4ebac0f46d3b6c283afc0feed47734e2a4707 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 16:01:00 +0000 Subject: [PATCH 6/6] feat(storage): string-type HTTP response status code and set empty string for unused transport [Generated-by: AI] --- .../cloud/storage/StorageClientMetrics.java | 4 +- .../cloud/storage/StorageMetricsContext.java | 49 ++- .../storage/StorageMetricsContextTest.java | 303 +++++++++--------- 3 files changed, 193 insertions(+), 163 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageClientMetrics.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageClientMetrics.java index 659fb698dcd4..d7e1d2308e67 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageClientMetrics.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageClientMetrics.java @@ -52,8 +52,8 @@ final class StorageClientMetrics { AttributeKey.stringKey("rpc.response.status_code"); static final AttributeKey KEY_HTTP_REQUEST_METHOD = AttributeKey.stringKey("http.request.method"); - static final AttributeKey KEY_HTTP_RESPONSE_STATUS_CODE = - AttributeKey.longKey("http.response.status_code"); + static final AttributeKey KEY_HTTP_RESPONSE_STATUS_CODE = + AttributeKey.stringKey("http.response.status_code"); static final AttributeKey KEY_URL_TEMPLATE = AttributeKey.stringKey("url.template"); static final AttributeKey KEY_SERVER_ADDRESS = AttributeKey.stringKey("server.address"); static final AttributeKey KEY_SERVER_PORT = AttributeKey.longKey("server.port"); diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java index 1db52c3677e3..81332c7aeca1 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java @@ -197,9 +197,23 @@ void recordAttemptSuccess(@Nullable StorageClientMetrics metrics) { .put(StorageClientMetrics.KEY_NETWORK_PATH, resolveNetworkPath()); String rpcSystemName = baseAttributes.get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME); if ("grpc".equals(rpcSystemName)) { - builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "OK"); + String rpcCode = baseAttributes.get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE); + builder.put( + StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcCode != null ? rpcCode : "OK"); + builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, ""); } else if ("http".equals(rpcSystemName)) { - builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, 200L); + String httpCode = baseAttributes.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE); + builder.put( + StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, + httpCode != null ? httpCode : "200"); + builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, ""); + } else { + String rpcCode = baseAttributes.get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE); + String httpCode = baseAttributes.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE); + builder.put( + StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcCode != null ? rpcCode : ""); + builder.put( + StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, httpCode != null ? httpCode : ""); } metrics.getAttempts().add(1, builder.build()); } @@ -231,12 +245,23 @@ void recordAttemptFailure(@Nullable StorageClientMetrics metrics, Throwable e) { String rpcStatus = extractRpcStatusCode(e); builder.put( StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, - rpcStatus != null ? rpcStatus : "UNKNOWN"); + rpcStatus != null ? rpcStatus : ""); + builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, ""); } else if ("http".equals(rpcSystemName)) { - Long httpStatus = extractHttpStatusCode(e); + String httpStatus = extractHttpStatusCode(e); builder.put( StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, - httpStatus != null ? httpStatus : 0L); + httpStatus != null ? httpStatus : ""); + builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, ""); + } else { + String rpcStatus = extractRpcStatusCode(e); + String httpStatus = extractHttpStatusCode(e); + builder.put( + StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, + rpcStatus != null ? rpcStatus : ""); + builder.put( + StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, + httpStatus != null ? httpStatus : ""); } metrics.getAttempts().add(1, builder.build()); } @@ -271,6 +296,10 @@ private String resolveNetworkPath() { @Nullable private String extractRpcStatusCode(Throwable e) { + String fromContext = baseAttributes.get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE); + if (fromContext != null) { + return fromContext; + } Throwable curr = e; while (curr != null) { if (curr instanceof io.grpc.StatusRuntimeException) { @@ -288,19 +317,23 @@ private String extractRpcStatusCode(Throwable e) { } @Nullable - private Long extractHttpStatusCode(Throwable e) { + private String extractHttpStatusCode(Throwable e) { + String fromContext = baseAttributes.get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE); + if (fromContext != null) { + return fromContext; + } Throwable curr = e; while (curr != null) { if (curr instanceof StorageException) { int code = ((StorageException) curr).getCode(); if (code > 0) { - return (long) code; + return Integer.toString(code); } } if (curr instanceof com.google.api.client.http.HttpResponseException) { int code = ((com.google.api.client.http.HttpResponseException) curr).getStatusCode(); if (code > 0) { - return (long) code; + return Integer.toString(code); } } curr = curr.getCause(); diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java index c6a02a669c2c..3b1d0192a05a 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java @@ -27,6 +27,7 @@ import com.google.api.gax.rpc.ApiException; import io.grpc.CallOptions; import io.grpc.Status; +import io.grpc.StatusException; import io.grpc.StatusRuntimeException; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; @@ -170,6 +171,20 @@ public void transportAttachment_apiCallContext() { StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); ApiCallContext callContext = GrpcCallContext.createDefault(); + ApiCallContext attachedContext = context.attach(callContext); + assertThat(attachedContext).isNotNull(); + + StorageMetricsContext extracted = StorageMetricsContext.from(attachedContext); + assertThat(extracted).isSameInstanceAs(context); + + assertThat(StorageMetricsContext.from((ApiCallContext) null)).isNull(); + } + + @Test + public void transportAttachment_apiCallContext_whenGrpcCallContext_attachesCallOptions() { + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + ApiCallContext callContext = GrpcCallContext.createDefault(); + ApiCallContext attachedContext = context.attach(callContext); assertThat(attachedContext).isNotNull(); assertThat(attachedContext).isInstanceOf(GrpcCallContext.class); @@ -188,8 +203,6 @@ public void transportAttachment_apiCallContext() { GrpcCallContext.createDefault().withCallOptions(context.attach(CallOptions.DEFAULT)); assertThat(StorageMetricsContext.from((ApiCallContext) callOptionsOnlyContext)) .isSameInstanceAs(context); - - assertThat(StorageMetricsContext.from((ApiCallContext) null)).isNull(); } @Test @@ -250,17 +263,17 @@ public void recordOperation_isIdempotent() { } @Test - public void recordAttempt_nullOrDisabledMetrics_noOp() { + public void recordAttemptSuccess_nullMetrics_noOp() { StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); - context.recordAttemptSuccess(null); - context.recordAttemptFailure(null, new RuntimeException("boom")); - - StorageClientMetrics disabledMetrics = StorageClientMetrics.create(provider, false, false); - context.recordAttemptSuccess(disabledMetrics); - context.recordAttemptFailure(disabledMetrics, new RuntimeException("err")); + } - assertThat(reader.collectAllMetrics()).isEmpty(); + @Test + public void recordAttemptSuccess_whenAttemptsNull_noOp() { + SdkMeterProvider provider = SdkMeterProvider.builder().build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, false, false); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + context.recordAttemptSuccess(metrics); } @Test @@ -279,9 +292,9 @@ public void recordAttemptSuccess_defaultAttributes() { assertThat(point.getAttributes().get(StorageClientMetrics.KEY_GCP_STORAGE_OPERATION)) .isEqualTo("storage.objects.get"); assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isNull(); + .isEqualTo(""); assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isNull(); + .isEqualTo(""); // Errors counter must not be recorded on success assertThat(findMetric(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS)) .isNull(); @@ -304,7 +317,7 @@ public void recordAttemptSuccess_grpcSystemName_setsRpcStatusCodeOk() { assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) .isEqualTo("OK"); assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isNull(); + .isEqualTo(""); assertThat(point.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); } @@ -319,174 +332,166 @@ public void recordAttemptSuccess_httpSystemName_setsHttpStatusCode200() { LongPointData point = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isEqualTo(200L); + .isEqualTo("200"); assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isNull(); + .isEqualTo(""); } @Test - public void recordAttemptFailure_grpc_extractsRpcStatusAndNeverSetsHttpCode() { + public void recordAttemptSuccess_overridesFromBaseAttributes() { Attributes base = - Attributes.of( - StorageClientMetrics.KEY_RPC_SYSTEM_NAME, - "grpc", - StorageClientMetrics.KEY_NETWORK_PATH, - StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + Attributes.builder() + .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc") + .put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "ALREADY_EXISTS") + .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, "204") + .put( + StorageClientMetrics.KEY_NETWORK_PATH, StorageClientMetrics.NETWORK_PATH_DIRECTPATH) + .build(); StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + context.recordAttemptSuccess(metrics); - // StatusRuntimeException (NOT_FOUND) - context.recordAttemptFailure(metrics, Status.NOT_FOUND.asRuntimeException()); Collection collected = reader.collectAllMetrics(); + LongPointData point = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("ALREADY_EXISTS"); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isEqualTo(""); + assertThat(point.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + } + + @Test + public void recordAttemptFailure_nullMetrics_noOp() { + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + context.recordAttemptFailure(null, new RuntimeException("boom")); + } + + @Test + public void recordAttemptFailure_whenInstrumentsNull_noOp() { + StorageClientMetrics disabledMetrics = StorageClientMetrics.create(provider, false, false); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get"); + context.recordAttemptFailure(disabledMetrics, new RuntimeException("err")); + } + + @Test + public void recordAttemptFailure_storageException() { + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + StorageException se = new StorageException(404, "Not Found"); + context.recordAttemptFailure(metrics, se); + + Collection collected = reader.collectAllMetrics(); + + // Verify attempts counter LongPointData attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getValue()).isEqualTo(1L); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) .isEqualTo("NOT_FOUND"); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isEqualTo("NOT_FOUND"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isNull(); + .isEqualTo("404"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo(""); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); + // Verify errors counter LongPointData errorPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + assertThat(errorPoint.getValue()).isEqualTo(1L); assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) .isEqualTo("NOT_FOUND"); assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_GCP_STORAGE_OPERATION)) + .isEqualTo("storage.objects.get"); + } - // StatusException (PERMISSION_DENIED) - resetMetrics(); - context.recordAttemptFailure(metrics, Status.PERMISSION_DENIED.asException()); - collected = reader.collectAllMetrics(); - attemptPoint = + @Test + public void recordAttemptFailure_statusRuntimeException() { + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + StatusRuntimeException sre = Status.NOT_FOUND.asRuntimeException(); + context.recordAttemptFailure(metrics, sre); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) - .isEqualTo("PERMISSION_DENIED"); + .isEqualTo("NOT_FOUND"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isEqualTo("PERMISSION_DENIED"); + .isEqualTo("NOT_FOUND"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isNull(); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + .isEqualTo(""); - errorPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + LongPointData errorPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) + .isEqualTo("NOT_FOUND"); + } + + @Test + public void recordAttemptFailure_statusException() { + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + StatusException se = Status.PERMISSION_DENIED.asException(); + context.recordAttemptFailure(metrics, se); + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) .isEqualTo("PERMISSION_DENIED"); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) + .isEqualTo("PERMISSION_DENIED"); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) + .isEqualTo(""); + } - // ApiException (ALREADY_EXISTS) - resetMetrics(); + @Test + public void recordAttemptFailure_apiException() { + Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); ApiException apiException = new ApiException( new StatusRuntimeException(Status.ALREADY_EXISTS), GrpcStatusCode.of(Status.Code.ALREADY_EXISTS), false); context.recordAttemptFailure(metrics, apiException); - collected = reader.collectAllMetrics(); - attemptPoint = + + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) .isEqualTo("ALREADY_EXISTS"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) .isEqualTo("ALREADY_EXISTS"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isNull(); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - - errorPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) - .isEqualTo("ALREADY_EXISTS"); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - - // StorageException (412 wrapping ApiException FAILED_PRECONDITION) - resetMetrics(); - ApiException innerApiException = - new ApiException( - new StatusRuntimeException(Status.FAILED_PRECONDITION), - GrpcStatusCode.of(Status.Code.FAILED_PRECONDITION), - false); - StorageException se = new StorageException(412, "Precondition Failed", innerApiException); - context.recordAttemptFailure(metrics, se); - collected = reader.collectAllMetrics(); - attemptPoint = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) - .isEqualTo("FAILED_PRECONDITION"); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isEqualTo("FAILED_PRECONDITION"); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isNull(); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - - errorPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) - .isEqualTo("FAILED_PRECONDITION"); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); + .isEqualTo(""); } @Test - public void recordAttemptFailure_http_extractsHttpStatusAndNeverSetsRpcCode() { + public void recordAttemptFailure_httpResponseException() { Attributes base = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); - - // StorageException with HTTP code 404 (NOT_FOUND) - StorageException se = new StorageException(404, "Not Found"); - context.recordAttemptFailure(metrics, se); - - Collection collected = reader.collectAllMetrics(); - LongPointData attemptPoint = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); - assertThat(attemptPoint.getValue()).isEqualTo(1L); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) - .isEqualTo("NOT_FOUND"); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isEqualTo(404L); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isNull(); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); - - LongPointData errorPoint = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); - assertThat(errorPoint.getValue()).isEqualTo(1L); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) - .isEqualTo("NOT_FOUND"); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) - .isEqualTo(StorageClientMetrics.NETWORK_PATH_CLOUDPATH); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_GCP_STORAGE_OPERATION)) - .isEqualTo("storage.objects.get"); - - // HttpResponseException with HTTP code 429 (RESOURCE_EXHAUSTED) - resetMetrics(); HttpResponseException hre = new HttpResponseException.Builder(429, "Too Many Requests", new HttpHeaders()).build(); context.recordAttemptFailure(metrics, hre); - collected = reader.collectAllMetrics(); - attemptPoint = + Collection collected = reader.collectAllMetrics(); + LongPointData attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); - assertThat(attemptPoint.getValue()).isEqualTo(1L); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) .isEqualTo("RESOURCE_EXHAUSTED"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isEqualTo(429L); + .isEqualTo("429"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isNull(); - - errorPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); - assertThat(errorPoint.getValue()).isEqualTo(1L); - assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_ERROR_TYPE)) - .isEqualTo("RESOURCE_EXHAUSTED"); + .isEqualTo(""); } @Test - public void recordAttemptFailure_nestedCause_extractsStatusFromCauseChain() { + public void recordAttemptFailure_nestedCause() { Attributes grpcBase = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); StorageMetricsContext grpcContext = StorageMetricsContext.create("storage.objects.get", grpcBase); @@ -500,7 +505,7 @@ public void recordAttemptFailure_nestedCause_extractsStatusFromCauseChain() { assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) .isEqualTo("UNAVAILABLE"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isNull(); + .isEqualTo(""); resetMetrics(); Attributes httpBase = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); @@ -508,50 +513,42 @@ public void recordAttemptFailure_nestedCause_extractsStatusFromCauseChain() { StorageMetricsContext.create("storage.objects.get", httpBase); Exception wrappedHttp = new RuntimeException(new StorageException(503, "Service Unavailable")); httpContext.recordAttemptFailure(metrics, wrappedHttp); - collected = reader.collectAllMetrics(); attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isEqualTo(503L); + .isEqualTo("503"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isNull(); + .isEqualTo(""); } @Test - public void recordAttemptFailure_missingStatusCode_defaultsToUnknownOrZero() { - Attributes grpcBase = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc"); - StorageMetricsContext grpcContext = - StorageMetricsContext.create("storage.objects.get", grpcBase); - - // RuntimeException without any gRPC or ApiException cause - grpcContext.recordAttemptFailure(metrics, new RuntimeException("generic failure")); + public void recordAttemptFailure_overridesFromBaseAttributes() { + Attributes base = + Attributes.builder() + .put(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "grpc") + .put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, "OVERRIDDEN_RPC") + .put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, "999") + .put( + StorageClientMetrics.KEY_NETWORK_PATH, StorageClientMetrics.NETWORK_PATH_DIRECTPATH) + .build(); + StorageMetricsContext context = StorageMetricsContext.create("storage.objects.get", base); + context.recordAttemptFailure(metrics, new StorageException(404, "Not Found")); Collection collected = reader.collectAllMetrics(); LongPointData attemptPoint = getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isEqualTo("UNKNOWN"); + .isEqualTo("OVERRIDDEN_RPC"); assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isNull(); - - resetMetrics(); - Attributes httpBase = Attributes.of(StorageClientMetrics.KEY_RPC_SYSTEM_NAME, "http"); - StorageMetricsContext httpContext = - StorageMetricsContext.create("storage.objects.get", httpBase); - - // StatusRuntimeException has a gRPC status code, but rpcSystemName is "http", - // and it has no HTTP status code (extractHttpStatusCode returns null). - StatusRuntimeException sre = Status.NOT_FOUND.asRuntimeException(); - httpContext.recordAttemptFailure(metrics, sre); + .isEqualTo(""); + assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); - collected = reader.collectAllMetrics(); - attemptPoint = - getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)) - .isEqualTo(0L); - assertThat(attemptPoint.getAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)) - .isNull(); + LongPointData errorPoint = + getSinglePoint(collected, StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS); + assertThat(errorPoint.getAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH)) + .isEqualTo(StorageClientMetrics.NETWORK_PATH_DIRECTPATH); } private static @Nullable MetricData findMetric(