From 2b123b7f8395efa504632a9a141abc471cc47c6d Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 05:41:02 +0000 Subject: [PATCH 1/2] feat(storage): wire logical operation metrics in Retrying and StorageOptions [Generated-by: AI] --- .../cloud/storage/HttpStorageOptions.java | 5 + .../com/google/cloud/storage/Retrying.java | 359 ++++++++++++-- .../cloud/storage/StorageClientMetrics.java | 38 +- .../google/cloud/storage/StorageOptions.java | 186 ++++++- .../cloud/storage/RetryingMetricsTest.java | 462 ++++++++++++++++++ .../storage/StorageClientMetricsTest.java | 62 ++- .../cloud/storage/StorageOptionsTest.java | 101 ++++ 7 files changed, 1159 insertions(+), 54 deletions(-) create mode 100644 java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/RetryingMetricsTest.java diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java index 8dff1c8185c1..3fabc6ba197b 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java @@ -551,5 +551,10 @@ public RetrySettings getRetrySettings() { public ApiClock getClock() { return HttpStorageOptions.this.getClock(); } + + @Override + public @Nullable StorageClientMetrics getStorageClientMetrics() { + return HttpStorageOptions.this.getStorageClientMetrics(); + } } } diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/Retrying.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/Retrying.java index d2f46d959e4c..940efc59076e 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/Retrying.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/Retrying.java @@ -22,18 +22,25 @@ import com.google.api.gax.retrying.BasicResultRetryAlgorithm; import com.google.api.gax.retrying.ResultRetryAlgorithm; import com.google.api.gax.retrying.RetrySettings; +import com.google.cloud.BaseServiceException; import com.google.cloud.storage.Backoff.Jitterer; import com.google.cloud.storage.Conversions.Decoder; import com.google.cloud.storage.spi.v1.HttpRpcContext; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.api.metrics.LongCounter; +import io.opentelemetry.context.Scope; import java.util.UUID; import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.UnaryOperator; import org.checkerframework.checker.nullness.qual.NonNull; +import org.checkerframework.checker.nullness.qual.Nullable; final class Retrying { @@ -43,7 +50,7 @@ final class Retrying { */ interface Retrier { /** - * A convenience method to invoke a callable, and possibly retry it again if and exception is + * A convenience method to invoke a callable, and possibly retry it again if an exception is * thrown. If the result of {@code c} is non-null, Decoder {@code d} will be applied to the * result before returning. * @@ -54,18 +61,23 @@ interface Retrier { * T}, can optionally return null * @param d A post process mapping {@link Function} which can be used to transform the result * from {@code c} if it is successful and non-null + * @param metricsContext optional metrics context for tracking logical operation metrics * @return A {@code U} (possibly null) after applying {@code f} to the result of {@code c} * @throws StorageException if {@code c} fails due to any retry exhaustion */ Model run( - ResultRetryAlgorithm alg, Callable c, Decoder d); + ResultRetryAlgorithm alg, + Callable c, + Decoder d, + @Nullable StorageMetricsContext metricsContext); + + default Model run( + ResultRetryAlgorithm alg, Callable c, Decoder d) { + return run(alg, c, d, null); + } - /** - * Produce a new {@link RetrierWithAlg} where the provided ResultRetryAlgorithm is bound for the - * life of the produced instance. - */ default RetrierWithAlg withAlg(ResultRetryAlgorithm alg) { - return new RetrierWithAlgImpl(this, alg); + return new DefaultRetrierWithAlg(this, alg); } static Retrier attemptOnce() { @@ -80,22 +92,15 @@ static Retrier attemptOnce() { */ interface RetrierWithAlg extends Retrier { - /** - * A convenience method to invoke a callable, and possibly retry it again if and exception is - * thrown. If the result of {@code c} is non-null, Decoder {@code d} will be applied to the - * result before returning. - * - * @param The result type of {@code c} - * @param The result type of any mapping that takes place via {@code d} - * @param c The {@link Callable} which will be passed to runWithRetries producing some {@code - * T}, can optionally return null - * @param d A post process mapping {@link Function} which can be used to transform the result - * from {@code c} if it is successful and non-null - * @return A {@code U} (possibly null) after applying {@code f} to the result of {@code c} - * @throws StorageException if {@code c} fails due to any retry exhaustion - */ Model run(Callable c, Decoder d); + default Model run( + Callable c, + Decoder d, + @Nullable StorageMetricsContext metricsContext) { + return run(neverRetry(), c, d, metricsContext); + } + static RetrierWithAlg attemptOnce() { return AttemptOnceRetrier.INSTANCE_WITH_ALG; } @@ -107,7 +112,10 @@ static final class AttemptOnceRetrier implements Retrier { @Override public Model run( - ResultRetryAlgorithm alg, Callable c, Decoder d) { + ResultRetryAlgorithm alg, + Callable c, + Decoder d, + @Nullable StorageMetricsContext metricsContext) { try { Response call = c.call(); return call == null ? null : d.decode(call); @@ -117,38 +125,273 @@ public Model run( } } - static final class RetrierWithAlgImpl implements RetrierWithAlg { + static final class DefaultRetrierWithAlg implements RetrierWithAlg { private final Retrier retrier; private final ResultRetryAlgorithm alg; - private RetrierWithAlgImpl(Retrier retrier, ResultRetryAlgorithm alg) { + private DefaultRetrierWithAlg(Retrier retrier, ResultRetryAlgorithm alg) { this.retrier = retrier; this.alg = alg; } @Override public Model run( - ResultRetryAlgorithm alg, Callable c, Decoder d) { - return retrier.run(alg, c, d); + ResultRetryAlgorithm alg, + Callable c, + Decoder d, + @Nullable StorageMetricsContext metricsContext) { + return retrier.run(alg, c, d, metricsContext); } @Override public Model run(Callable c, Decoder d) { - return retrier.run(alg, c, d); + return retrier.run(alg, c, d, null); + } + + @Override + public Model run( + Callable c, + Decoder d, + @Nullable StorageMetricsContext metricsContext) { + return retrier.run(alg, c, d, metricsContext); } } static final class DefaultRetrier implements Retrier { private final UnaryOperator decorator; private final RetryingDependencies deps; + @Nullable private final StorageClientMetrics metrics; DefaultRetrier(UnaryOperator decorator, RetryingDependencies deps) { + this(decorator, deps, deps != null ? deps.getStorageClientMetrics() : null); + } + + DefaultRetrier( + UnaryOperator decorator, + RetryingDependencies deps, + @Nullable StorageClientMetrics metrics) { this.decorator = decorator; this.deps = deps; + this.metrics = + metrics != null ? metrics : (deps != null ? deps.getStorageClientMetrics() : null); } @Override public Model run( + ResultRetryAlgorithm alg, + Callable c, + Decoder d, + @Nullable StorageMetricsContext metricsContext) { + if (metrics == null || metricsContext == null) { + return runWithoutMetrics(alg, c, d); + } + + StorageMetricsContext activeContext = metricsContext; + Attributes clientAttributes = metrics.getClientAttributes(); + + AttributesBuilder baseBuilder = Attributes.builder(); + baseBuilder.putAll(clientAttributes); + activeContext + .getBaseAttributes() + .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, activeContext.getOperationName()); + Attributes baseAttributes = baseBuilder.build(); + + LongCounter attemptsCounter = metrics.getAttempts(); + LongCounter errorsCounter = metrics.getErrors(); + + try (Scope ignore = activeContext.getOtelContext().makeCurrent()) { + RetryContext ctx = + decorator.apply( + RetryContext.of( + RetryContext.directScheduledExecutorService(), + deps, + alg, + Jitterer.threadLocalRandom())); + AtomicReference failure = new AtomicReference<>(); + AtomicBoolean attemptAgain = new AtomicBoolean(false); + Throwable terminalThrowable = null; + + try { + do { + attemptAgain.set(false); + activeContext.incrementAttempts(); + try { + Response result = c.call(); + recordAttemptSuccess(attemptsCounter, baseAttributes, activeContext); + return result == null ? null : d.decode(result); + } catch (StorageException se) { + String errorType = StorageErrorTypeMapper.mapThrowable(se); + recordAttemptFailure( + attemptsCounter, errorsCounter, baseAttributes, activeContext, se, errorType); + ctx.recordError(se, () -> attemptAgain.set(true), failure::set); + } catch (IllegalArgumentException iae) { + String errorType = StorageErrorTypeMapper.mapThrowable(iae); + recordAttemptFailure( + attemptsCounter, errorsCounter, baseAttributes, activeContext, iae, errorType); + ctx.recordError(iae, () -> attemptAgain.set(true), failure::set); + } catch (Exception e) { + String errorType = StorageErrorTypeMapper.mapThrowable(e); + recordAttemptFailure( + attemptsCounter, errorsCounter, baseAttributes, activeContext, e, errorType); + BaseServiceException coalesced = StorageException.coalesce(e); + ctx.recordError(coalesced, () -> attemptAgain.set(true), failure::set); + } + } while (attemptAgain.get()); + + Exception throwable = failure.get(); + terminalThrowable = throwable; + if (throwable instanceof StorageException) { + throw (StorageException) throwable; + } else if (throwable != null) { + throw StorageException.coalesce(throwable); + } else { + throw new StorageException(0, "Retry failed without exception"); + } + } catch (Throwable t) { + terminalThrowable = t; + throw t; + } finally { + if (activeContext.shouldRecordOperationOnRetrierExit()) { + activeContext.recordOperation(metrics, terminalThrowable); + } + } + } + } + + private static void recordAttemptSuccess( + @Nullable LongCounter attemptsCounter, + Attributes baseAttributes, + StorageMetricsContext activeContext) { + if (attemptsCounter == null) { + return; + } + AttributesBuilder builder = + baseAttributes.toBuilder() + .put(StorageClientMetrics.KEY_ERROR_TYPE, StorageErrorTypeMapper.STATUS_OK) + .put(StorageClientMetrics.KEY_NETWORK_PATH, resolveNetworkPath(activeContext)); + String rpcSystemName = + activeContext.getBaseAttributes().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 = + activeContext.getBaseAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE); + if (rpcCode != null) { + builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcCode); + } + Long httpCode = + activeContext.getBaseAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE); + if (httpCode != null) { + builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, httpCode); + } + attemptsCounter.add(1, builder.build()); + } + + private static void recordAttemptFailure( + @Nullable LongCounter attemptsCounter, + @Nullable LongCounter errorsCounter, + Attributes baseAttributes, + StorageMetricsContext activeContext, + Throwable e, + String errorType) { + String networkPath = resolveNetworkPath(activeContext); + if (errorsCounter != null) { + Attributes errorAttributes = + baseAttributes.toBuilder() + .put(StorageClientMetrics.KEY_ERROR_TYPE, errorType) + .put(StorageClientMetrics.KEY_NETWORK_PATH, networkPath) + .build(); + errorsCounter.add(1, errorAttributes); + } + if (attemptsCounter != null) { + AttributesBuilder builder = + baseAttributes.toBuilder() + .put(StorageClientMetrics.KEY_ERROR_TYPE, errorType) + .put(StorageClientMetrics.KEY_NETWORK_PATH, networkPath); + String rpcStatus = extractRpcStatusCode(e, activeContext); + if (rpcStatus != null) { + builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcStatus); + } + Long httpStatus = extractHttpStatusCode(e, activeContext); + if (httpStatus != null) { + builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, httpStatus); + } + attemptsCounter.add(1, builder.build()); + } + } + + private static String resolveNetworkPath(StorageMetricsContext ctx) { + String path = ctx.getBaseAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH); + if (path != null && !path.isEmpty()) { + return path; + } + return StorageClientMetrics.NETWORK_PATH_CLOUDPATH; + } + + @Nullable + private static String extractRpcStatusCode(Throwable e, StorageMetricsContext activeContext) { + String fromContext = + activeContext.getBaseAttributes().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 static Long extractHttpStatusCode(Throwable e, StorageMetricsContext activeContext) { + Long fromContext = + activeContext.getBaseAttributes().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; + } + + private Model runWithoutMetrics( ResultRetryAlgorithm alg, Callable c, Decoder d) { RetryContext ctx = decorator.apply( @@ -159,25 +402,19 @@ public Model run( Jitterer.threadLocalRandom())); AtomicReference failure = new AtomicReference<>(); AtomicBoolean attemptAgain = new AtomicBoolean(false); + do { attemptAgain.set(false); try { Response result = c.call(); return result == null ? null : d.decode(result); } catch (StorageException se) { - // we hope for this case ctx.recordError(se, () -> attemptAgain.set(true), failure::set); } catch (IllegalArgumentException iae) { - // IllegalArgumentException can happen if there is no json in the body and we try - // to parse it Our retry algorithms have special case for this, so in an effort to - // keep compatibility with those existing behaviors, explicitly rethrow an - // IllegalArgumentException that may have happened ctx.recordError(iae, () -> attemptAgain.set(true), failure::set); } catch (Exception e) { - // Wire in this fall through just in case. - // all of our retry algorithms are centered around StorageException so this helps - // those be more effective - ctx.recordError(StorageException.coalesce(e), () -> attemptAgain.set(true), failure::set); + BaseServiceException coalesced = StorageException.coalesce(e); + ctx.recordError(coalesced, () -> attemptAgain.set(true), failure::set); } } while (attemptAgain.get()); @@ -199,11 +436,14 @@ static final class HttpRetrier implements Retrier { @Override public Model run( - ResultRetryAlgorithm alg, Callable c, Decoder d) { + ResultRetryAlgorithm alg, + Callable c, + Decoder d, + @Nullable StorageMetricsContext metricsContext) { HttpRpcContext httpRpcContext = HttpRpcContext.getInstance(); try { httpRpcContext.newInvocationId(); - return delegate.run(alg, c, d); + return delegate.run(alg, c, d, metricsContext); } finally { httpRpcContext.clearInvocationId(); } @@ -212,10 +452,21 @@ public Model run( @NonNull static GrpcCallContext newCallContext() { - return GrpcCallContext.createDefault() - .withExtraHeaders( - ImmutableMap.of( - "x-goog-gcs-idempotency-token", ImmutableList.of(UUID.randomUUID().toString()))); + return newCallContext(null); + } + + @NonNull + static GrpcCallContext newCallContext(@Nullable StorageMetricsContext metricsContext) { + GrpcCallContext context = + GrpcCallContext.createDefault() + .withExtraHeaders( + ImmutableMap.of( + "x-goog-gcs-idempotency-token", + ImmutableList.of(UUID.randomUUID().toString()))); + if (metricsContext != null) { + context = metricsContext.attach(context); + } + return context; } static ResultRetryAlgorithm neverRetry() { @@ -246,23 +497,37 @@ interface RetryingDependencies { ApiClock getClock(); + default @Nullable StorageClientMetrics getStorageClientMetrics() { + return null; + } + static RetryingDependencies attemptOnce() { return RetryingDependencies.simple( NanoClock.getDefaultClock(), RetrySettings.newBuilder().setMaxAttempts(1).build()); } static RetryingDependencies simple(ApiClock clock, RetrySettings retrySettings) { - return new SimpleRetryingDependencies(clock, retrySettings); + return new SimpleRetryingDependencies(clock, retrySettings, null); + } + + static RetryingDependencies simple( + ApiClock clock, RetrySettings retrySettings, @Nullable StorageClientMetrics metrics) { + return new SimpleRetryingDependencies(clock, retrySettings, metrics); } } private static final class SimpleRetryingDependencies implements RetryingDependencies { private final ApiClock clock; private final RetrySettings retrySettings; + @Nullable private final StorageClientMetrics storageClientMetrics; - private SimpleRetryingDependencies(ApiClock clock, RetrySettings retrySettings) { + private SimpleRetryingDependencies( + ApiClock clock, + RetrySettings retrySettings, + @Nullable StorageClientMetrics storageClientMetrics) { this.retrySettings = retrySettings; this.clock = clock; + this.storageClientMetrics = storageClientMetrics; } @Override @@ -275,11 +540,17 @@ public RetrySettings getRetrySettings() { return retrySettings; } + @Override + public @Nullable StorageClientMetrics getStorageClientMetrics() { + return storageClientMetrics; + } + @Override public String toString() { return MoreObjects.toStringHelper(this) .add("clock", clock) .add("retrySettings", retrySettings) + .add("storageClientMetrics", storageClientMetrics) .toString(); } } 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..0877ba1fcb52 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 @@ -25,11 +25,11 @@ import io.opentelemetry.api.metrics.LongUpDownCounter; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.api.metrics.MeterProvider; -import java.util.UUID; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; import org.checkerframework.checker.nullness.qual.Nullable; /** Package-private instrument registry for OpenTelemetry client metrics. */ -final class StorageClientMetrics { +final class StorageClientMetrics implements AutoCloseable { static final String METER_NAME = "com.google.cloud.storage"; // Standard client attribute keys and values @@ -121,11 +121,12 @@ final class StorageClientMetrics { @Nullable private final DoubleHistogram credentialRefreshDuration; private final Attributes clientAttributes; + @Nullable private final SdkMeterProvider ownedMeterProvider; static StorageClientMetrics create( MeterProvider meterProvider, boolean enableOtelMetrics, boolean enableOtelDebugMetrics) { return create( - meterProvider, UUID.randomUUID().toString(), enableOtelMetrics, enableOtelDebugMetrics); + meterProvider, (SdkMeterProvider) null, enableOtelMetrics, enableOtelDebugMetrics); } static StorageClientMetrics create( @@ -133,6 +134,15 @@ static StorageClientMetrics create( String instanceId, boolean enableOtelMetrics, boolean enableOtelDebugMetrics) { + return create( + meterProvider, (SdkMeterProvider) null, enableOtelMetrics, enableOtelDebugMetrics); + } + + static StorageClientMetrics create( + MeterProvider meterProvider, + @Nullable SdkMeterProvider ownedMeterProvider, + boolean enableOtelMetrics, + boolean enableOtelDebugMetrics) { Meter meter = meterProvider .meterBuilder(METER_NAME) @@ -143,10 +153,9 @@ static StorageClientMetrics create( .put(KEY_GCP_CLIENT_SERVICE, SERVICE_STORAGE) .put(KEY_GCP_CLIENT_VERSION, StorageOptions.version()) .put(KEY_GCP_CLIENT_ARTIFACT, ARTIFACT_STORAGE) - .put(KEY_GCP_CLIENT_INSTANCE_ID, instanceId) .build(); return new StorageClientMetrics( - meter, clientAttributes, enableOtelMetrics, enableOtelDebugMetrics); + meter, clientAttributes, ownedMeterProvider, enableOtelMetrics, enableOtelDebugMetrics); } StorageClientMetrics(Meter meter, boolean enableOtelMetrics, boolean enableOtelDebugMetrics) { @@ -156,8 +165,8 @@ static StorageClientMetrics create( .put(KEY_GCP_CLIENT_SERVICE, SERVICE_STORAGE) .put(KEY_GCP_CLIENT_VERSION, StorageOptions.version()) .put(KEY_GCP_CLIENT_ARTIFACT, ARTIFACT_STORAGE) - .put(KEY_GCP_CLIENT_INSTANCE_ID, UUID.randomUUID().toString()) .build(), + null, enableOtelMetrics, enableOtelDebugMetrics); } @@ -167,7 +176,17 @@ static StorageClientMetrics create( Attributes clientAttributes, boolean enableOtelMetrics, boolean enableOtelDebugMetrics) { + this(meter, clientAttributes, null, enableOtelMetrics, enableOtelDebugMetrics); + } + + StorageClientMetrics( + Meter meter, + Attributes clientAttributes, + @Nullable SdkMeterProvider ownedMeterProvider, + boolean enableOtelMetrics, + boolean enableOtelDebugMetrics) { this.clientAttributes = clientAttributes; + this.ownedMeterProvider = ownedMeterProvider; if (enableOtelMetrics) { this.rpcClientCallDuration = @@ -396,4 +415,11 @@ Attributes attachClientAttributes(@Nullable Attributes other) { @Nullable DoubleHistogram getCredentialRefreshDuration() { return credentialRefreshDuration; } + + @Override + public void close() { + if (ownedMeterProvider != null) { + ownedMeterProvider.close(); + } + } } diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java index 672c9ce34041..c2311a5b584e 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java @@ -35,7 +35,11 @@ import com.google.cloud.storage.TransportCompatibility.Transport; import com.google.cloud.storage.spi.StorageRpcFactory; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.MeterProvider; +import io.opentelemetry.contrib.gcp.resource.GCPResourceProvider; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; import java.io.IOException; import java.io.InputStream; import java.io.ObjectInputStream; @@ -81,7 +85,7 @@ public abstract class StorageOptions extends ServiceOptions= 0) { + s = s.substring(0, slashIdx); + } + if (s.startsWith("[") && s.contains("]")) { + int closeBracket = s.indexOf(']'); + int colonIdx = s.indexOf(':', closeBracket); + if (colonIdx >= 0) { + s = s.substring(0, colonIdx); + } + if (s.startsWith("[") && s.endsWith("]")) { + s = s.substring(1, s.length() - 1); + } + } else { + int colonIdx = s.indexOf(':'); + if (colonIdx >= 0) { + s = s.substring(0, colonIdx); + } + } + return s.isEmpty() ? null : s; + } + /** * The metric export interval configured for periodic metric reading. * diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/RetryingMetricsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/RetryingMetricsTest.java new file mode 100644 index 000000000000..ddd054e4c10e --- /dev/null +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/RetryingMetricsTest.java @@ -0,0 +1,462 @@ +/* + * 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.cloud.storage.StorageClientMetrics.KEY_ERROR_TYPE; +import static com.google.cloud.storage.StorageClientMetrics.KEY_GCP_CLIENT_METHOD; +import static com.google.cloud.storage.StorageClientMetrics.KEY_GCP_CLIENT_SERVICE; +import static com.google.cloud.storage.StorageClientMetrics.KEY_GCP_STORAGE_OPERATION; +import static com.google.cloud.storage.StorageClientMetrics.KEY_RPC_METHOD; +import static com.google.cloud.storage.StorageClientMetrics.METRIC_GCP_CLIENT_REQUEST_DURATION; +import static com.google.cloud.storage.StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS; +import static com.google.cloud.storage.StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS; +import static com.google.cloud.storage.StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_OPERATIONS; +import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_DNS_FAILURE; +import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_NOT_FOUND; +import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_OK; +import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_TIMEOUT; +import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_UNAVAILABLE; +import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_UNKNOWN; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import com.google.api.core.NanoClock; +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.retrying.RetrySettings; +import com.google.cloud.storage.Conversions.Decoder; +import com.google.cloud.storage.Retrying.DefaultRetrier; +import com.google.cloud.storage.Retrying.Retrier; +import com.google.cloud.storage.Retrying.RetrierWithAlg; +import com.google.cloud.storage.Retrying.RetryingDependencies; +import io.grpc.Status; +import io.grpc.StatusRuntimeException; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.context.Context; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.data.HistogramPointData; +import io.opentelemetry.sdk.metrics.data.LongPointData; +import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; +import java.net.SocketTimeoutException; +import java.net.UnknownHostException; +import java.time.Duration; +import java.util.Collection; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.UnaryOperator; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.junit.Before; +import org.junit.Test; + +public final class RetryingMetricsTest { + + private static final RetrySettings RETRY_SETTINGS = + RetrySettings.newBuilder() + .setMaxAttempts(3) + .setInitialRetryDelayDuration(Duration.ofMillis(10)) + .setMaxRetryDelayDuration(Duration.ofMillis(50)) + .setRetryDelayMultiplier(1.0) + .build(); + + private InMemoryMetricReader reader; + private SdkMeterProvider provider; + private StorageClientMetrics metrics; + private RetryingDependencies deps; + private Retrier baseRetrier; + private RetrierWithAlg alwaysRetrier; + private RetrierWithAlg neverRetrier; + @Nullable private Collection collected; + + @Before + public void setUp() { + reader = InMemoryMetricReader.create(); + provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + metrics = StorageClientMetrics.create(provider, true, false); + deps = RetryingDependencies.simple(NanoClock.getDefaultClock(), RETRY_SETTINGS, metrics); + baseRetrier = new DefaultRetrier(UnaryOperator.identity(), deps); + alwaysRetrier = baseRetrier.withAlg(Retrying.alwaysRetry()); + neverRetrier = baseRetrier.withAlg(Retrying.neverRetry()); + collected = null; + } + + @Test + public void successfulCallOnFirstAttempt_recordsExpectedMetrics() { + String result = + alwaysRetrier.run( + () -> "success", Decoder.identity(), StorageMetricsContext.create("GetObject")); + assertThat(result).isEqualTo("success"); + assertOutcomeMetrics(STATUS_OK, 1L, 0L); + assertThat(assertDuration(STATUS_OK).getAttributes().get(KEY_GCP_CLIENT_SERVICE)) + .isEqualTo("storage"); + } + + @Test + public void retriedCallSuccessOnThirdAttempt_recordsRetriesAndErrors() { + AtomicInteger attemptCounter = new AtomicInteger(0); + String result = + alwaysRetrier.run( + () -> { + int count = attemptCounter.incrementAndGet(); + if (count < 3) { + throw new StorageException(503, "Service Unavailable attempt " + count); + } + return "recovered"; + }, + Decoder.identity(), + StorageMetricsContext.create("GetObject")); + + assertThat(result).isEqualTo("recovered"); + assertThat(attemptCounter.get()).isEqualTo(3); + assertOutcomeMetrics(STATUS_OK, 3L, 2L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_UNAVAILABLE, 2L); + assertDuration(STATUS_OK); + } + + @Test + public void terminalFailureAllAttemptsFail_recordsExhaustedRetries() { + AtomicInteger attemptCounter = new AtomicInteger(0); + StorageException se = + assertThrows( + StorageException.class, + () -> + alwaysRetrier.run( + () -> { + int count = attemptCounter.incrementAndGet(); + throw new StorageException(503, "Permanent failure attempt " + count); + }, + Decoder.identity(), + StorageMetricsContext.create("GetObject"))); + + assertThat(attemptCounter.get()).isEqualTo(3); + assertThat(se.getCode()).isEqualTo(503); + assertOutcomeMetrics(STATUS_UNAVAILABLE, 3L, 3L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_UNAVAILABLE, 3L); + assertDuration(STATUS_UNAVAILABLE); + } + + @Test + public void metricsDisabled_recordsNoMetrics() { + StorageClientMetrics disabledMetrics = StorageClientMetrics.create(provider, false, false); + RetryingDependencies disabledDeps = + RetryingDependencies.simple(NanoClock.getDefaultClock(), RETRY_SETTINGS, disabledMetrics); + RetrierWithAlg retrier = + new DefaultRetrier(UnaryOperator.identity(), disabledDeps).withAlg(Retrying.alwaysRetry()); + + assertThat(retrier.run(() -> "no-op metrics", Decoder.identity())).isEqualTo("no-op metrics"); + assertThat(collect()).isEmpty(); + } + + @Test + public void nullMetrics_executesWithoutRecording() { + RetryingDependencies nullDeps = + RetryingDependencies.simple(NanoClock.getDefaultClock(), RETRY_SETTINGS, null); + RetrierWithAlg retrier = + new DefaultRetrier(UnaryOperator.identity(), nullDeps).withAlg(Retrying.alwaysRetry()); + + assertThat(retrier.run(() -> "null-metrics", Decoder.identity())).isEqualTo("null-metrics"); + } + + @Test + public void callingRunWithoutMetricsContext_whenMetricsEnabled_recordsNoMetrics() { + assertThat(alwaysRetrier.run(() -> "no-context", Decoder.identity())).isEqualTo("no-context"); + assertThat(collect()).isEmpty(); + } + + @Test + public void httpStorageOptions_createRetrier_wiresMetricsWhenEnabled() { + HttpStorageOptions options = + HttpStorageOptions.newBuilder() + .setMeterProvider(provider) + .setEnableOtelMetrics(true) + .build(); + + assertThat(options.getStorageClientMetrics()).isNotNull(); + Retrier retrier = options.createRetrier(); + assertThat(retrier).isNotNull(); + + StorageMetricsContext ctx = options.createMetricsContext("GetObject"); + assertThat(retrier.run(Retrying.neverRetry(), () -> "options-success", Decoder.identity(), ctx)) + .isEqualTo("options-success"); + assertAttr(METRIC_GCP_STORAGE_CLIENT_OPERATIONS, KEY_ERROR_TYPE, STATUS_OK, 1L); + } + + @Test + public void httpStorageOptions_createRetrier_noMetricsWhenDisabled() { + HttpStorageOptions options = + HttpStorageOptions.newBuilder() + .setMeterProvider(provider) + .setEnableOtelMetrics(false) + .build(); + + assertThat(options.getStorageClientMetrics()).isNull(); + Retrier retrier = options.createRetrier(); + assertThat(retrier.run(Retrying.neverRetry(), () -> "options-disabled", Decoder.identity())) + .isEqualTo("options-disabled"); + assertThat(collect()).isEmpty(); + } + + @Test + public void grpcStorageOptions_createRetrier_wiresMetricsWhenEnabled() { + GrpcStorageOptions options = + GrpcStorageOptions.newBuilder() + .setMeterProvider(provider) + .setEnableOtelMetrics(true) + .build(); + + assertThat(options.getStorageClientMetrics()).isNotNull(); + Retrier retrier = options.createRetrier(); + assertThat(retrier).isNotNull(); + + StorageMetricsContext ctx = options.createMetricsContext("GetObject"); + String res = + retrier.run(Retrying.neverRetry(), () -> "grpc-options-success", Decoder.identity(), ctx); + assertThat(res).isEqualTo("grpc-options-success"); + assertAttr(METRIC_GCP_STORAGE_CLIENT_OPERATIONS, KEY_ERROR_TYPE, STATUS_OK, 1L); + } + + @Test + public void grpcStorageOptions_createRetrier_noMetricsWhenDisabled() { + GrpcStorageOptions options = + GrpcStorageOptions.newBuilder() + .setMeterProvider(provider) + .setEnableOtelMetrics(false) + .build(); + + assertThat(options.getStorageClientMetrics()).isNull(); + Retrier retrier = options.createRetrier(); + String res = + retrier.run(Retrying.neverRetry(), () -> "grpc-options-disabled", Decoder.identity()); + assertThat(res).isEqualTo("grpc-options-disabled"); + assertThat(collect()).isEmpty(); + } + + @Test + public void nonRetryableError_singleAttemptTerminalFailure_recordsErrorAndTerminalOperation() { + StorageException se = + assertThrows( + StorageException.class, + () -> + neverRetrier.run( + () -> { + throw new StorageException(404, "Not Found"); + }, + Decoder.identity(), + StorageMetricsContext.create("GetObject"))); + + assertThat(se.getCode()).isEqualTo(404); + assertOutcomeMetrics(STATUS_NOT_FOUND, 1L, 1L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_NOT_FOUND, 1L); + assertDuration(STATUS_NOT_FOUND); + } + + @Test + public void recordAttemptError_specificExceptions_mappedToCanonicalTypes() { + AtomicInteger attemptCounter = new AtomicInteger(0); + StorageException se = + assertThrows( + StorageException.class, + () -> + alwaysRetrier.run( + () -> { + int count = attemptCounter.incrementAndGet(); + if (count == 1) { + throw new StatusRuntimeException(Status.DEADLINE_EXCEEDED); + } else if (count == 2) { + throw new SocketTimeoutException("Read timed out"); + } else { + throw new UnknownHostException("storage.googleapis.com"); + } + }, + Decoder.identity(), + StorageMetricsContext.create("GetObject"))); + + assertThat(attemptCounter.get()).isEqualTo(3); + assertThat(se).isNotNull(); + assertOutcomeMetrics(STATUS_DNS_FAILURE, 3L, 3L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_TIMEOUT, 2L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_DNS_FAILURE, 1L); + } + + @Test + public void illegalArgumentException_parsingFailure_recordedAndRethrown() { + StorageException se = + assertThrows( + StorageException.class, + () -> + neverRetrier.run( + () -> { + throw new IllegalArgumentException("malformed json response"); + }, + Decoder.identity(), + StorageMetricsContext.create("GetObject"))); + + assertThat(se.getCause()).isInstanceOf(IllegalArgumentException.class); + assertThat(se.getCause()).hasMessageThat().contains("malformed json response"); + assertOutcomeMetrics(STATUS_UNKNOWN, 1L, 1L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_UNKNOWN, 1L); + } + + @Test + public void run_withCustomMetricsContext_recordsSpecificOperationAttribute() { + StorageMetricsContext customContext = StorageMetricsContext.create("GetObject"); + String result = + baseRetrier.run( + Retrying.neverRetry(), () -> "custom-op-result", Decoder.identity(), customContext); + assertThat(result).isEqualTo("custom-op-result"); + + assertAttr(METRIC_GCP_STORAGE_CLIENT_OPERATIONS, KEY_GCP_STORAGE_OPERATION, "GetObject", 1L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_OPERATIONS, KEY_GCP_CLIENT_METHOD, "GetObject", 1L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_OPERATIONS, KEY_RPC_METHOD, "GetObject", 0L); + + HistogramPointData durationPoint = assertDuration(STATUS_OK); + assertThat(durationPoint.getAttributes().get(KEY_GCP_STORAGE_OPERATION)).isEqualTo("GetObject"); + assertThat(durationPoint.getAttributes().get(KEY_GCP_CLIENT_METHOD)).isEqualTo("GetObject"); + assertThat(durationPoint.getAttributes().get(KEY_RPC_METHOD)) + .isEqualTo("google.storage.v2.Storage/GetObject"); + + assertAttr(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS, KEY_GCP_STORAGE_OPERATION, "GetObject", 1L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS, KEY_RPC_METHOD, "GetObject", 0L); + } + + @Test + public void withAlg_withCustomMetricsContext_propagatesContext() { + StorageMetricsContext customContext = StorageMetricsContext.create("GetObject"); + String result = neverRetrier.run(() -> "custom-op-result", Decoder.identity(), customContext); + assertThat(result).isEqualTo("custom-op-result"); + + assertAttr(METRIC_GCP_STORAGE_CLIENT_OPERATIONS, KEY_GCP_STORAGE_OPERATION, "GetObject", 1L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS, KEY_GCP_STORAGE_OPERATION, "GetObject", 1L); + } + + @Test + public void newCallContext_attachesMetricsContext() { + StorageMetricsContext customContext = StorageMetricsContext.create("GetObject"); + GrpcCallContext callContext = Retrying.newCallContext(customContext); + assertThat(StorageMetricsContext.from(callContext)).isSameInstanceAs(customContext); + assertThat(StorageMetricsContext.from(Retrying.newCallContext(null))).isNull(); + } + + @Test + public void run_withCustomMetricsContext_recordsOperationOnRetriesAndErrors() { + StorageMetricsContext customContext = StorageMetricsContext.create("GetObject"); + AtomicInteger attempts = new AtomicInteger(0); + String result = + baseRetrier.run( + Retrying.alwaysRetry(), + () -> { + if (attempts.incrementAndGet() < 2) { + throw new StorageException(503, "Unavailable"); + } + return "recovered"; + }, + Decoder.identity(), + customContext); + assertThat(result).isEqualTo("recovered"); + + assertAttr(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS, KEY_GCP_STORAGE_OPERATION, "GetObject", 2L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS, KEY_RPC_METHOD, "GetObject", 0L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_GCP_STORAGE_OPERATION, "GetObject", 1L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_RPC_METHOD, "GetObject", 0L); + } + + @Test + public void run_withCustomMetricsContext_propagatesOtelContextScope() { + StorageMetricsContext customContext = StorageMetricsContext.create("GetObject"); + Context[] observedContext = new Context[1]; + + baseRetrier.run( + Retrying.neverRetry(), + () -> { + observedContext[0] = Context.current(); + return "ok"; + }, + Decoder.identity(), + customContext); + + assertThat(observedContext[0]).isEqualTo(customContext.getOtelContext()); + } + + private Collection collect() { + if (collected == null) { + collected = reader.collectAllMetrics(); + } + return collected; + } + + private void assertOutcomeMetrics( + String terminalStatus, long expectedAttempts, long expectedErrors) { + assertAttr(METRIC_GCP_STORAGE_CLIENT_OPERATIONS, KEY_ERROR_TYPE, terminalStatus, 1L); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ATTEMPTS)) + .isEqualTo(expectedAttempts); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ERRORS)) + .isEqualTo(expectedErrors); + } + + private void assertAttr( + String metricName, AttributeKey key, String expectedValue, long expectedCount) { + assertThat(getCounterValueWithAttribute(collect(), metricName, key, expectedValue)) + .isEqualTo(expectedCount); + } + + private HistogramPointData assertDuration(String expectedTerminalStatus) { + HistogramPointData point = getHistogramPoint(collect(), METRIC_GCP_CLIENT_REQUEST_DURATION); + assertThat(point).isNotNull(); + assertThat(point.getCount()).isEqualTo(1L); + assertThat(point.getAttributes().get(KEY_ERROR_TYPE)).isEqualTo(expectedTerminalStatus); + return point; + } + + private static @Nullable MetricData findMetric( + Collection metrics, String metricName) { + for (MetricData md : metrics) { + if (md.getName().equals(metricName) || md.getName().equals(metricName.replace(".", "/"))) { + return md; + } + } + return null; + } + + private static long getCounterValue(Collection metrics, String metricName) { + MetricData md = findMetric(metrics, metricName); + return md == null + ? 0L + : md.getLongSumData().getPoints().stream().mapToLong(LongPointData::getValue).sum(); + } + + private static long getCounterValueWithAttribute( + Collection metrics, + String metricName, + AttributeKey key, + String expectedValue) { + MetricData md = findMetric(metrics, metricName); + if (md == null) { + return 0L; + } + return md.getLongSumData().getPoints().stream() + .filter(p -> expectedValue.equals(p.getAttributes().get(key))) + .mapToLong(LongPointData::getValue) + .sum(); + } + + private static @Nullable HistogramPointData getHistogramPoint( + Collection metrics, String metricName) { + MetricData md = findMetric(metrics, metricName); + if (md == null || !md.getHistogramData().getPoints().iterator().hasNext()) { + return null; + } + return md.getHistogramData().getPoints().iterator().next(); + } +} diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageClientMetricsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageClientMetricsTest.java index 9d3b8fa95e4d..f9f90e7cf005 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageClientMetricsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageClientMetricsTest.java @@ -21,14 +21,21 @@ import com.google.common.collect.ImmutableList; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.sdk.common.CompletableResultCode; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; +import io.opentelemetry.sdk.metrics.data.AggregationTemporality; import io.opentelemetry.sdk.metrics.data.HistogramPointData; import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.metrics.export.CollectionRegistration; +import io.opentelemetry.sdk.metrics.export.MetricReader; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import java.util.stream.Collectors; import org.junit.Before; @@ -80,7 +87,7 @@ public void standardMetrics_initializedWithoutDebug() { assertThat(metrics.getClientAttributes().get(StorageClientMetrics.KEY_GCP_CLIENT_ARTIFACT)) .isEqualTo("com.google.cloud:google-cloud-storage"); assertThat(metrics.getClientAttributes().get(StorageClientMetrics.KEY_GCP_CLIENT_INSTANCE_ID)) - .isNotEmpty(); + .isNull(); assertInstruments(metrics, DEBUG_INSTRUMENTS, /* isNull= */ true); } @@ -287,8 +294,7 @@ public void attachClientAttributes_mergesAdditionalAttributes() { Attributes merged = metrics.attachClientAttributes(extra); - assertThat(merged.get(StorageClientMetrics.KEY_GCP_CLIENT_INSTANCE_ID)) - .isEqualTo("test-instance"); + assertThat(merged.get(StorageClientMetrics.KEY_GCP_CLIENT_INSTANCE_ID)).isNull(); assertThat(merged.get(StorageClientMetrics.KEY_GCP_CLIENT_VERSION)) .isEqualTo(StorageOptions.version()); assertThat(merged.get(StorageClientMetrics.KEY_GCP_CLIENT_ARTIFACT)) @@ -341,6 +347,56 @@ private static void assertHistogramBoundaries( assertThat(pointData.getBoundaries()).isEqualTo(expectedBoundaries); } + @Test + public void close_closesOwnedMeterProvider() { + TestMetricReader reader = new TestMetricReader(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, provider, true, false); + metrics.close(); + assertThat(reader.shutdownCalled.get()).isTrue(); + } + + @Test + public void close_doesNotThrowWhenOwnedMeterProviderIsNull() { + TestMetricReader reader = new TestMetricReader(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + StorageClientMetrics metrics = + StorageClientMetrics.create(provider, (SdkMeterProvider) null, true, false); + metrics.close(); + assertThat(reader.shutdownCalled.get()).isFalse(); + } + + static final class TestMetricReader implements MetricReader { + private final MetricReader delegate = InMemoryMetricReader.create(); + final AtomicBoolean shutdownCalled = new AtomicBoolean(false); + + @Override + public void register(CollectionRegistration registration) { + delegate.register(registration); + } + + @Override + public AggregationTemporality getAggregationTemporality(InstrumentType instrumentType) { + return delegate.getAggregationTemporality(instrumentType); + } + + @Override + public Aggregation getDefaultAggregation(InstrumentType instrumentType) { + return delegate.getDefaultAggregation(instrumentType); + } + + @Override + public CompletableResultCode forceFlush() { + return delegate.forceFlush(); + } + + @Override + public CompletableResultCode shutdown() { + shutdownCalled.set(true); + return delegate.shutdown(); + } + } + private static void assertInstruments( StorageClientMetrics metrics, List> instruments, diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java index f2dc6f3fda8d..dfe074b80753 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java @@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertWithMessage; import static org.junit.Assert.assertThrows; +import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.MeterProvider; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import java.io.ByteArrayInputStream; @@ -458,4 +459,104 @@ public void deserializationWithNullMetricInterval_defaultsToSixtySeconds() throw assertThat(deserialized.hashCode()).isEqualTo(defaultOptions.hashCode()); }); } + + @Test + public void extractServerAddress_parsesVariousHostFormats() { + assertThat(StorageOptions.extractServerAddress("https://storage.googleapis.com")) + .isEqualTo("storage.googleapis.com"); + assertThat(StorageOptions.extractServerAddress("http://localhost:8080")).isEqualTo("localhost"); + assertThat(StorageOptions.extractServerAddress("google-c2p:///storage.googleapis.com")) + .isEqualTo("storage.googleapis.com"); + assertThat(StorageOptions.extractServerAddress("https://storage.googleapis.com/path")) + .isEqualTo("storage.googleapis.com"); + assertThat(StorageOptions.extractServerAddress("http://[::1]:8080")).isEqualTo("::1"); + assertThat(StorageOptions.extractServerAddress("[::1]")).isEqualTo("::1"); + assertThat(StorageOptions.extractServerAddress(null)).isNull(); + assertThat(StorageOptions.extractServerAddress(" ")).isNull(); + } + + @Test + public void createMetricsContext_populatesRpcSystemNameAndServerAddress() { + MeterProvider meterProvider = SdkMeterProvider.builder().build(); + + HttpStorageOptions httpOptions = + StorageOptions.http() + .setEnableOtelMetrics(true) + .setMeterProvider(meterProvider) + .setHost("https://storage.googleapis.com") + .build(); + StorageMetricsContext httpContext = httpOptions.createMetricsContext("GetObject"); + assertThat(httpContext).isNotNull(); + assertThat(httpContext.getBaseAttributes().get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME)) + .isEqualTo("http"); + assertThat(httpContext.getBaseAttributes().get(StorageClientMetrics.KEY_SERVER_ADDRESS)) + .isEqualTo("storage.googleapis.com"); + + GrpcStorageOptions grpcOptions = + StorageOptions.grpc() + .setEnableOtelMetrics(true) + .setMeterProvider(meterProvider) + .setHost("google-c2p:///storage.googleapis.com") + .build(); + StorageMetricsContext grpcContext = grpcOptions.createMetricsContext("GetObject"); + assertThat(grpcContext).isNotNull(); + assertThat(grpcContext.getBaseAttributes().get(StorageClientMetrics.KEY_RPC_SYSTEM_NAME)) + .isEqualTo("grpc"); + assertThat(grpcContext.getBaseAttributes().get(StorageClientMetrics.KEY_SERVER_ADDRESS)) + .isEqualTo("storage.googleapis.com"); + } + + @Test + public void meterProvider_usedWhenExplicitlySet() { + MeterProvider customMeterProvider = SdkMeterProvider.builder().build(); + HttpStorageOptions options = + StorageOptions.http() + .setEnableOtelMetrics(true) + .setMeterProvider(customMeterProvider) + .build(); + StorageClientMetrics metrics = options.getStorageClientMetrics(); + assertThat(metrics).isNotNull(); + } + + @Test + public void customOpenTelemetry_meterProviderUsedWhenConfigured() { + SdkMeterProvider sdkMeterProvider = SdkMeterProvider.builder().build(); + OpenTelemetry customOtel = + io.opentelemetry.sdk.OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); + HttpStorageOptions options = + StorageOptions.http().setEnableOtelMetrics(true).setOpenTelemetry(customOtel).build(); + assertThat(options.isCustomOpenTelemetryConfigured()).isTrue(); + StorageClientMetrics metrics = options.getStorageClientMetrics(); + assertThat(metrics).isNotNull(); + } + + @Test + public void closeStorageClientMetrics_cleansUpMetricsAndClosesSdkOwnedProvider() { + StorageClientMetricsTest.TestMetricReader reader = + new StorageClientMetricsTest.TestMetricReader(); + SdkMeterProvider mockOwnedProvider = + SdkMeterProvider.builder().registerMetricReader(reader).build(); + HttpStorageOptions options = + StorageOptions.http() + .setEnableOtelMetrics(true) + .setMeterProvider(mockOwnedProvider) + .build(); + StorageClientMetrics metrics = + StorageClientMetrics.create(mockOwnedProvider, mockOwnedProvider, true, false); + options.storageClientMetrics = metrics; + + options.closeStorageClientMetrics(); + assertThat(options.storageClientMetrics).isNull(); + assertThat(reader.shutdownCalled.get()).isTrue(); + } + + @Test + public void closeStorageClientMetrics_doesNotFailOnUserSuppliedMeterProvider() { + MeterProvider userProvider = SdkMeterProvider.builder().build(); + HttpStorageOptions options = + StorageOptions.http().setEnableOtelMetrics(true).setMeterProvider(userProvider).build(); + assertThat(options.getStorageClientMetrics()).isNotNull(); + options.closeStorageClientMetrics(); + assertThat(options.storageClientMetrics).isNull(); + } } From f5d0dbe284e99c26ae96a91408642a7d6e72b544 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 10:26:07 +0000 Subject: [PATCH 2/2] feat(storage): bind metrics context to retrier and delegate attempt recording [Generated-by: AI] --- .../cloud/storage/GrpcStorageOptions.java | 6 + .../com/google/cloud/storage/Retrying.java | 309 ++++++------------ .../cloud/storage/StorageClientMetrics.java | 9 - .../google/cloud/storage/StorageOptions.java | 22 +- .../cloud/storage/RetryingMetricsTest.java | 103 ++++++ .../storage/StorageClientMetricsTest.java | 6 +- 6 files changed, 216 insertions(+), 239 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 0793d4614223..ce0ec9fbc487 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -1460,6 +1460,12 @@ public static boolean isReady() { } } + @InternalApi + @Override + public StorageClientMetrics getStorageClientMetrics() { + return super.getStorageClientMetrics(); + } + private static class ReadObjectRangeResultRetryAlgorithmDecorator extends BasicResultRetryAlgorithm { diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/Retrying.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/Retrying.java index 940efc59076e..6552dd427d2a 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/Retrying.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/Retrying.java @@ -29,10 +29,6 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.common.AttributesBuilder; -import io.opentelemetry.api.metrics.LongCounter; import io.opentelemetry.context.Scope; import java.util.UUID; import java.util.concurrent.Callable; @@ -77,7 +73,16 @@ default Model run( } default RetrierWithAlg withAlg(ResultRetryAlgorithm alg) { - return new DefaultRetrierWithAlg(this, alg); + return withAlg(alg, null); + } + + default RetrierWithAlg withAlg( + ResultRetryAlgorithm alg, @Nullable StorageMetricsContext metricsContext) { + return new DefaultRetrierWithAlg(this, alg, metricsContext); + } + + default Retrier withMetricsContext(@Nullable StorageMetricsContext metricsContext) { + return metricsContext == null ? this : new ContextBoundRetrier(this, metricsContext); } static Retrier attemptOnce() { @@ -125,35 +130,91 @@ public Model run( } } + static final class ContextBoundRetrier implements Retrier { + private final Retrier delegate; + private final StorageMetricsContext metricsContext; + + ContextBoundRetrier(Retrier delegate, StorageMetricsContext metricsContext) { + this.delegate = delegate; + this.metricsContext = metricsContext; + } + + @Override + public Model run( + ResultRetryAlgorithm alg, + Callable c, + Decoder d, + @Nullable StorageMetricsContext explicitCtx) { + return delegate.run(alg, c, d, explicitCtx != null ? explicitCtx : metricsContext); + } + + @Override + public Model run( + ResultRetryAlgorithm alg, Callable c, Decoder d) { + return delegate.run(alg, c, d, metricsContext); + } + + @Override + public RetrierWithAlg withAlg(ResultRetryAlgorithm alg) { + return delegate.withAlg(alg, metricsContext); + } + + @Override + public RetrierWithAlg withAlg( + ResultRetryAlgorithm alg, @Nullable StorageMetricsContext explicitCtx) { + return delegate.withAlg(alg, explicitCtx != null ? explicitCtx : metricsContext); + } + + @Override + public Retrier withMetricsContext(@Nullable StorageMetricsContext explicitCtx) { + return explicitCtx != null ? new ContextBoundRetrier(delegate, explicitCtx) : this; + } + } + static final class DefaultRetrierWithAlg implements RetrierWithAlg { private final Retrier retrier; private final ResultRetryAlgorithm alg; + @Nullable private final StorageMetricsContext boundMetricsContext; - private DefaultRetrierWithAlg(Retrier retrier, ResultRetryAlgorithm alg) { + private DefaultRetrierWithAlg( + Retrier retrier, + ResultRetryAlgorithm alg, + @Nullable StorageMetricsContext boundMetricsContext) { this.retrier = retrier; this.alg = alg; + this.boundMetricsContext = boundMetricsContext; + } + + private DefaultRetrierWithAlg(Retrier retrier, ResultRetryAlgorithm alg) { + this(retrier, alg, null); + } + + @Override + public Model run(Callable c, Decoder d) { + return retrier.run(alg, c, d, boundMetricsContext); } @Override public Model run( - ResultRetryAlgorithm alg, - Callable c, - Decoder d, - @Nullable StorageMetricsContext metricsContext) { - return retrier.run(alg, c, d, metricsContext); + ResultRetryAlgorithm alg, Callable c, Decoder d) { + return retrier.run(alg, c, d, boundMetricsContext); } @Override - public Model run(Callable c, Decoder d) { - return retrier.run(alg, c, d, null); + public Model run( + Callable c, + Decoder d, + @Nullable StorageMetricsContext metricsContext) { + return retrier.run(alg, c, d, metricsContext != null ? metricsContext : boundMetricsContext); } @Override public Model run( + ResultRetryAlgorithm alg, Callable c, Decoder d, @Nullable StorageMetricsContext metricsContext) { - return retrier.run(alg, c, d, metricsContext); + return retrier.run(alg, c, d, metricsContext != null ? metricsContext : boundMetricsContext); } } @@ -182,36 +243,8 @@ public Model run( Callable c, Decoder d, @Nullable StorageMetricsContext metricsContext) { - if (metrics == null || metricsContext == null) { - return runWithoutMetrics(alg, c, d); - } - - StorageMetricsContext activeContext = metricsContext; - Attributes clientAttributes = metrics.getClientAttributes(); - - AttributesBuilder baseBuilder = Attributes.builder(); - baseBuilder.putAll(clientAttributes); - activeContext - .getBaseAttributes() - .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, activeContext.getOperationName()); - Attributes baseAttributes = baseBuilder.build(); - - LongCounter attemptsCounter = metrics.getAttempts(); - LongCounter errorsCounter = metrics.getErrors(); - - try (Scope ignore = activeContext.getOtelContext().makeCurrent()) { + Scope scope = metricsContext != null ? metricsContext.getOtelContext().makeCurrent() : null; + try { RetryContext ctx = decorator.apply( RetryContext.of( @@ -226,25 +259,29 @@ public Model run( try { do { attemptAgain.set(false); - activeContext.incrementAttempts(); + if (metricsContext != null) { + metricsContext.incrementAttempts(); + } try { Response result = c.call(); - recordAttemptSuccess(attemptsCounter, baseAttributes, activeContext); + if (metricsContext != null) { + metricsContext.recordAttemptSuccess(metrics); + } return result == null ? null : d.decode(result); } catch (StorageException se) { - String errorType = StorageErrorTypeMapper.mapThrowable(se); - recordAttemptFailure( - attemptsCounter, errorsCounter, baseAttributes, activeContext, se, errorType); + if (metricsContext != null) { + metricsContext.recordAttemptFailure(metrics, se); + } ctx.recordError(se, () -> attemptAgain.set(true), failure::set); } catch (IllegalArgumentException iae) { - String errorType = StorageErrorTypeMapper.mapThrowable(iae); - recordAttemptFailure( - attemptsCounter, errorsCounter, baseAttributes, activeContext, iae, errorType); + if (metricsContext != null) { + metricsContext.recordAttemptFailure(metrics, iae); + } ctx.recordError(iae, () -> attemptAgain.set(true), failure::set); } catch (Exception e) { - String errorType = StorageErrorTypeMapper.mapThrowable(e); - recordAttemptFailure( - attemptsCounter, errorsCounter, baseAttributes, activeContext, e, errorType); + if (metricsContext != null) { + metricsContext.recordAttemptFailure(metrics, e); + } BaseServiceException coalesced = StorageException.coalesce(e); ctx.recordError(coalesced, () -> attemptAgain.set(true), failure::set); } @@ -263,166 +300,14 @@ public Model run( terminalThrowable = t; throw t; } finally { - if (activeContext.shouldRecordOperationOnRetrierExit()) { - activeContext.recordOperation(metrics, terminalThrowable); - } - } - } - } - - private static void recordAttemptSuccess( - @Nullable LongCounter attemptsCounter, - Attributes baseAttributes, - StorageMetricsContext activeContext) { - if (attemptsCounter == null) { - return; - } - AttributesBuilder builder = - baseAttributes.toBuilder() - .put(StorageClientMetrics.KEY_ERROR_TYPE, StorageErrorTypeMapper.STATUS_OK) - .put(StorageClientMetrics.KEY_NETWORK_PATH, resolveNetworkPath(activeContext)); - String rpcSystemName = - activeContext.getBaseAttributes().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 = - activeContext.getBaseAttributes().get(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE); - if (rpcCode != null) { - builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcCode); - } - Long httpCode = - activeContext.getBaseAttributes().get(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE); - if (httpCode != null) { - builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, httpCode); - } - attemptsCounter.add(1, builder.build()); - } - - private static void recordAttemptFailure( - @Nullable LongCounter attemptsCounter, - @Nullable LongCounter errorsCounter, - Attributes baseAttributes, - StorageMetricsContext activeContext, - Throwable e, - String errorType) { - String networkPath = resolveNetworkPath(activeContext); - if (errorsCounter != null) { - Attributes errorAttributes = - baseAttributes.toBuilder() - .put(StorageClientMetrics.KEY_ERROR_TYPE, errorType) - .put(StorageClientMetrics.KEY_NETWORK_PATH, networkPath) - .build(); - errorsCounter.add(1, errorAttributes); - } - if (attemptsCounter != null) { - AttributesBuilder builder = - baseAttributes.toBuilder() - .put(StorageClientMetrics.KEY_ERROR_TYPE, errorType) - .put(StorageClientMetrics.KEY_NETWORK_PATH, networkPath); - String rpcStatus = extractRpcStatusCode(e, activeContext); - if (rpcStatus != null) { - builder.put(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE, rpcStatus); - } - Long httpStatus = extractHttpStatusCode(e, activeContext); - if (httpStatus != null) { - builder.put(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE, httpStatus); - } - attemptsCounter.add(1, builder.build()); - } - } - - private static String resolveNetworkPath(StorageMetricsContext ctx) { - String path = ctx.getBaseAttributes().get(StorageClientMetrics.KEY_NETWORK_PATH); - if (path != null && !path.isEmpty()) { - return path; - } - return StorageClientMetrics.NETWORK_PATH_CLOUDPATH; - } - - @Nullable - private static String extractRpcStatusCode(Throwable e, StorageMetricsContext activeContext) { - String fromContext = - activeContext.getBaseAttributes().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 static Long extractHttpStatusCode(Throwable e, StorageMetricsContext activeContext) { - Long fromContext = - activeContext.getBaseAttributes().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; + if (metricsContext != null && metricsContext.shouldRecordOperationOnRetrierExit()) { + metricsContext.recordOperation(metrics, terminalThrowable); } } - curr = curr.getCause(); - } - return null; - } - - private Model runWithoutMetrics( - ResultRetryAlgorithm alg, Callable c, Decoder d) { - RetryContext ctx = - decorator.apply( - RetryContext.of( - RetryContext.directScheduledExecutorService(), - deps, - alg, - Jitterer.threadLocalRandom())); - AtomicReference failure = new AtomicReference<>(); - AtomicBoolean attemptAgain = new AtomicBoolean(false); - - do { - attemptAgain.set(false); - try { - Response result = c.call(); - return result == null ? null : d.decode(result); - } catch (StorageException se) { - ctx.recordError(se, () -> attemptAgain.set(true), failure::set); - } catch (IllegalArgumentException iae) { - ctx.recordError(iae, () -> attemptAgain.set(true), failure::set); - } catch (Exception e) { - BaseServiceException coalesced = StorageException.coalesce(e); - ctx.recordError(coalesced, () -> attemptAgain.set(true), failure::set); + } finally { + if (scope != null) { + scope.close(); } - } while (attemptAgain.get()); - - Exception throwable = failure.get(); - if (throwable instanceof StorageException) { - throw (StorageException) throwable; - } else { - throw StorageException.coalesce(throwable); } } } 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 0877ba1fcb52..3f00f1cfb9ad 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 @@ -129,15 +129,6 @@ static StorageClientMetrics create( meterProvider, (SdkMeterProvider) null, enableOtelMetrics, enableOtelDebugMetrics); } - static StorageClientMetrics create( - MeterProvider meterProvider, - String instanceId, - boolean enableOtelMetrics, - boolean enableOtelDebugMetrics) { - return create( - meterProvider, (SdkMeterProvider) null, enableOtelMetrics, enableOtelDebugMetrics); - } - static StorageClientMetrics create( MeterProvider meterProvider, @Nullable SdkMeterProvider ownedMeterProvider, diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java index c2311a5b584e..330be593875f 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java @@ -380,9 +380,8 @@ public boolean isEnableOtelDebugMetrics() { return meterProvider; } - @InternalApi @Nullable - public synchronized StorageClientMetrics getStorageClientMetrics() { + synchronized StorageClientMetrics getStorageClientMetrics() { if (!isEnableOtelMetrics()) { return null; } @@ -493,17 +492,7 @@ synchronized void closeStorageClientMetrics() { if (metrics == null) { return null; } - io.opentelemetry.api.common.AttributesBuilder builder = - metrics - .clientAttributesBuilder() - .put( - StorageClientMetrics.KEY_RPC_SYSTEM_NAME, - this instanceof GrpcStorageOptions ? "grpc" : "http"); - String serverAddress = extractServerAddress(getHost()); - if (serverAddress != null && !serverAddress.isEmpty()) { - builder.put(StorageClientMetrics.KEY_SERVER_ADDRESS, serverAddress); - } - return StorageMetricsContext.create(operationName, builder.build()); + return StorageMetricsContext.create(operationName, buildBaseMetricsAttributes(metrics)); } @Nullable StorageMetricsContext createMultiStepMetricsContext(String operationName) { @@ -511,6 +500,11 @@ synchronized void closeStorageClientMetrics() { if (metrics == null) { return null; } + return StorageMetricsContext.createForMultiStep( + operationName, buildBaseMetricsAttributes(metrics)); + } + + private Attributes buildBaseMetricsAttributes(StorageClientMetrics metrics) { io.opentelemetry.api.common.AttributesBuilder builder = metrics .clientAttributesBuilder() @@ -521,7 +515,7 @@ synchronized void closeStorageClientMetrics() { if (serverAddress != null && !serverAddress.isEmpty()) { builder.put(StorageClientMetrics.KEY_SERVER_ADDRESS, serverAddress); } - return StorageMetricsContext.createForMultiStep(operationName, builder.build()); + return builder.build(); } @Nullable diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/RetryingMetricsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/RetryingMetricsTest.java index ddd054e4c10e..5ab6f367d82b 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/RetryingMetricsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/RetryingMetricsTest.java @@ -389,6 +389,109 @@ public void run_withCustomMetricsContext_propagatesOtelContextScope() { assertThat(observedContext[0]).isEqualTo(customContext.getOtelContext()); } + @Test + public void withAlg_withMultiStepMetricsContext_runTwoArgs_recordsAttemptsAndErrors() { + StorageMetricsContext multiContext = StorageMetricsContext.createForMultiStep("MultiStepOp"); + RetrierWithAlg retrier = baseRetrier.withAlg(Retrying.alwaysRetry(), multiContext); + AtomicInteger attempts = new AtomicInteger(0); + + String result = + retrier.run( + () -> { + if (attempts.incrementAndGet() < 2) { + throw new StorageException(503, "Unavailable"); + } + return "recovered"; + }, + Decoder.identity()); + + assertThat(result).isEqualTo("recovered"); + assertThat(attempts.get()).isEqualTo(2); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_OPERATIONS)).isEqualTo(0L); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ATTEMPTS)).isEqualTo(2L); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ERRORS)).isEqualTo(1L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS, KEY_GCP_STORAGE_OPERATION, "MultiStepOp", 2L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_GCP_STORAGE_OPERATION, "MultiStepOp", 1L); + } + + @Test + public void withAlg_withMultiStepMetricsContext_runThreeArgs_recordsAttemptsAndErrors() { + StorageMetricsContext multiContext = StorageMetricsContext.createForMultiStep("MultiStepOp"); + RetrierWithAlg retrier = baseRetrier.withAlg(Retrying.neverRetry(), multiContext); + AtomicInteger attempts = new AtomicInteger(0); + + String result = + retrier.run( + Retrying.alwaysRetry(), + () -> { + if (attempts.incrementAndGet() < 2) { + throw new StorageException(503, "Unavailable"); + } + return "recovered"; + }, + Decoder.identity()); + + assertThat(result).isEqualTo("recovered"); + assertThat(attempts.get()).isEqualTo(2); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_OPERATIONS)).isEqualTo(0L); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ATTEMPTS)).isEqualTo(2L); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ERRORS)).isEqualTo(1L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS, KEY_GCP_STORAGE_OPERATION, "MultiStepOp", 2L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_GCP_STORAGE_OPERATION, "MultiStepOp", 1L); + } + + @Test + public void withMetricsContext_runThreeArgs_recordsAttemptsAndErrors() { + StorageMetricsContext multiContext = StorageMetricsContext.createForMultiStep("MultiStepOp"); + Retrier contextBoundRetrier = baseRetrier.withMetricsContext(multiContext); + AtomicInteger attempts = new AtomicInteger(0); + + String result = + contextBoundRetrier.run( + Retrying.alwaysRetry(), + () -> { + if (attempts.incrementAndGet() < 2) { + throw new StorageException(503, "Unavailable"); + } + return "recovered"; + }, + Decoder.identity()); + + assertThat(result).isEqualTo("recovered"); + assertThat(attempts.get()).isEqualTo(2); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_OPERATIONS)).isEqualTo(0L); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ATTEMPTS)).isEqualTo(2L); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ERRORS)).isEqualTo(1L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS, KEY_GCP_STORAGE_OPERATION, "MultiStepOp", 2L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_GCP_STORAGE_OPERATION, "MultiStepOp", 1L); + } + + @Test + public void withMetricsContext_withAlg_runTwoArgs_recordsAttemptsAndErrors() { + StorageMetricsContext multiContext = StorageMetricsContext.createForMultiStep("MultiStepOp"); + Retrier contextBoundRetrier = baseRetrier.withMetricsContext(multiContext); + RetrierWithAlg retrierWithAlg = contextBoundRetrier.withAlg(Retrying.alwaysRetry()); + AtomicInteger attempts = new AtomicInteger(0); + + String result = + retrierWithAlg.run( + () -> { + if (attempts.incrementAndGet() < 2) { + throw new StorageException(503, "Unavailable"); + } + return "recovered"; + }, + Decoder.identity()); + + assertThat(result).isEqualTo("recovered"); + assertThat(attempts.get()).isEqualTo(2); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_OPERATIONS)).isEqualTo(0L); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ATTEMPTS)).isEqualTo(2L); + assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ERRORS)).isEqualTo(1L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS, KEY_GCP_STORAGE_OPERATION, "MultiStepOp", 2L); + assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_GCP_STORAGE_OPERATION, "MultiStepOp", 1L); + } + private Collection collect() { if (collected == null) { collected = reader.collectAllMetrics(); diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageClientMetricsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageClientMetricsTest.java index f9f90e7cf005..1f26551d95cf 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageClientMetricsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageClientMetricsTest.java @@ -282,8 +282,7 @@ public void latencyHistogram_recordsSecondsScaleValuesIntoExpectedBuckets() { @Test public void attachClientAttributes_mergesAdditionalAttributes() { SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = - StorageClientMetrics.create(provider, "test-instance", true, false); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); Attributes extra = Attributes.of( @@ -313,8 +312,7 @@ public void attachClientAttributes_mergesAdditionalAttributes() { @Test public void clientAttributesBuilder_containsBaseClientAttributes() { SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); - StorageClientMetrics metrics = - StorageClientMetrics.create(provider, "test-instance", true, false); + StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false); Attributes built = metrics.clientAttributesBuilder().build(); assertThat(built).isEqualTo(metrics.getClientAttributes());