From 354f72b4423e6dbe2efbb859d77c31dac2a00efe Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 05:49:55 +0000 Subject: [PATCH 1/9] feat(storage): wire operation metrics in gRPC veneer [Generated-by: AI] --- .../google/cloud/storage/GapicCopyWriter.java | 31 +- .../cloud/storage/GrpcBlobReadChannel.java | 14 +- .../cloud/storage/GrpcBlobWriteChannel.java | 16 +- .../google/cloud/storage/GrpcStorageImpl.java | 591 ++++++++++++++---- 4 files changed, 509 insertions(+), 143 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java index dff801871c51..48a2d44bfc80 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java @@ -23,6 +23,7 @@ import com.google.cloud.storage.Retrying.RetrierWithAlg; import com.google.storage.v2.RewriteObjectRequest; import com.google.storage.v2.RewriteResponse; +import org.checkerframework.checker.nullness.qual.Nullable; final class GapicCopyWriter extends CopyWriter { @@ -32,6 +33,7 @@ final class GapicCopyWriter extends CopyWriter { private final RetrierWithAlg retrier; private final RewriteObjectRequest originalRequest; private final RewriteResponse initialResponse; + @Nullable private final StorageMetricsContext metricsContext; private RewriteResponse mostRecentResponse; @@ -41,12 +43,23 @@ final class GapicCopyWriter extends CopyWriter { RetrierWithAlg retrier, RewriteObjectRequest originalRequest, RewriteResponse initialResponse) { + this(storage, callable, retrier, originalRequest, initialResponse, null); + } + + GapicCopyWriter( + GrpcStorageImpl storage, + UnaryCallable callable, + RetrierWithAlg retrier, + RewriteObjectRequest originalRequest, + RewriteResponse initialResponse, + @Nullable StorageMetricsContext metricsContext) { this.storage = storage; this.callable = callable; this.retrier = retrier; this.initialResponse = initialResponse; this.mostRecentResponse = initialResponse; this.originalRequest = originalRequest; + this.metricsContext = metricsContext; } @Override @@ -54,6 +67,9 @@ public Blob getResult() { while (!isDone()) { copyChunk(); } + if (metricsContext != null) { + metricsContext.recordOperation(storage.getOptions().getStorageClientMetrics(), null); + } BlobInfo info = Conversions.grpc().blobInfo().decode(mostRecentResponse.getResource()); return info.asBlob(storage); } @@ -78,8 +94,19 @@ public void copyChunk() { if (!isDone()) { RewriteObjectRequest req = originalRequest.toBuilder().setRewriteToken(mostRecentResponse.getRewriteToken()).build(); - GrpcCallContext retryContext = Retrying.newCallContext(); - mostRecentResponse = retrier.run(() -> callable.call(req, retryContext), Decoder.identity()); + StorageMetricsContext chunkContext = + metricsContext != null + ? metricsContext.asSubAttemptContext() + : storage.getOptions().createMetricsContext("RewriteObject"); + GrpcCallContext retryContext = Retrying.newCallContext(chunkContext); + RewriteResponse resp = + retrier.run(() -> callable.call(req, retryContext), Decoder.identity(), chunkContext); + if (resp != null) { + mostRecentResponse = resp; + } + if (isDone() && metricsContext != null) { + metricsContext.recordOperation(storage.getOptions().getStorageClientMetrics(), null); + } } } diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java index 9dd39dfe47ea..0b30d9f4376a 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java @@ -26,6 +26,7 @@ import com.google.storage.v2.ReadObjectRequest; import com.google.storage.v2.ReadObjectResponse; import org.checkerframework.checker.nullness.qual.NonNull; +import org.checkerframework.checker.nullness.qual.Nullable; final class GrpcBlobReadChannel extends BaseStorageReadChannel { @@ -41,7 +42,18 @@ final class GrpcBlobReadChannel extends BaseStorageReadChannel { ResultRetryAlgorithm resultRetryAlgorithm, ReadObjectRequest request, boolean autoGzipDecompression) { - super(Conversions.grpc().blobInfo()); + this(read, retrier, resultRetryAlgorithm, request, autoGzipDecompression, null, null); + } + + GrpcBlobReadChannel( + ZeroCopyServerStreamingCallable read, + Retrier retrier, + ResultRetryAlgorithm resultRetryAlgorithm, + ReadObjectRequest request, + boolean autoGzipDecompression, + @Nullable StorageMetricsContext metricsContext, + @Nullable StorageClientMetrics storageClientMetrics) { + super(Conversions.grpc().blobInfo(), metricsContext, storageClientMetrics); this.read = read; this.retrier = retrier; this.resultRetryAlgorithm = resultRetryAlgorithm; diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobWriteChannel.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobWriteChannel.java index 3a7bc2a790c1..23c690ff7841 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobWriteChannel.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobWriteChannel.java @@ -24,6 +24,7 @@ import com.google.storage.v2.WriteObjectRequest; import com.google.storage.v2.WriteObjectResponse; import java.util.function.Supplier; +import org.checkerframework.checker.nullness.qual.Nullable; final class GrpcBlobWriteChannel extends BaseStorageWriteChannel { @@ -37,7 +38,20 @@ final class GrpcBlobWriteChannel extends BaseStorageWriteChannel> start, Hasher hasher) { - super(Conversions.grpc().blobInfo().compose(WriteObjectResponse::getResource)); + this(write, retrier, start, hasher, null, null); + } + + GrpcBlobWriteChannel( + ClientStreamingCallable write, + RetrierWithAlg retrier, + Supplier> start, + Hasher hasher, + @Nullable StorageMetricsContext metricsContext, + @Nullable StorageClientMetrics storageClientMetrics) { + super( + Conversions.grpc().blobInfo().compose(WriteObjectResponse::getResource), + metricsContext, + storageClientMetrics); this.write = write; this.retrier = retrier; this.start = start; diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index 120b7a269724..9160a671f260 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java @@ -209,6 +209,8 @@ public void close() throws Exception { s.shutdownNow(); java.time.Duration terminationAwaitDuration = getOptions().getTerminationAwaitDuration(); s.awaitTermination(terminationAwaitDuration.toMillis(), TimeUnit.MILLISECONDS); + } finally { + getOptions().closeStorageClientMetrics(); } } @@ -227,11 +229,13 @@ public Bucket create(BucketInfo bucketInfo, BucketTargetOption... options) { builder.getBucketBuilder().setProject(projectNameCodec.encode(getOptions().getProjectId())); } CreateBucketRequest req = opts.createBucketsRequest().apply(builder).build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("CreateBucket"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.createBucketCallable().call(req, merge), - syntaxDecoders.bucket); + syntaxDecoders.bucket, + metricsContext); } @Override @@ -255,6 +259,8 @@ public Blob create( @Override public Blob create(BlobInfo blobInfo, InputStream content, BlobWriteOption... options) { + StorageMetricsContext metricsContext = getOptions().createMetricsContext("WriteObject"); + Throwable error = null; try { requireNonNull(blobInfo, "blobInfo must be non null"); InputStream inputStreamParam = firstNonNull(content, new ByteArrayInputStream(ZERO_BYTES)); @@ -264,7 +270,12 @@ public Blob create(BlobInfo blobInfo, InputStream content, BlobWriteOption... op optsWithDefaults.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); WriteObjectRequest req = getWriteObjectRequest(blobInfo, optsWithDefaults); Hasher hasher = optsWithDefaults.getHasher(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + GrpcCallContext merge = + Utils.merge( + grpcCallContext, + metricsContext != null + ? Retrying.newCallContext(metricsContext) + : Retrying.newCallContext()); UnbufferedWritableByteChannelSession session = ResumableMedia.gapic() .write() @@ -282,7 +293,15 @@ public Blob create(BlobInfo blobInfo, InputStream content, BlobWriteOption... op ApiFuture responseApiFuture = session.getResult(); return this.getBlob(responseApiFuture); } catch (IOException | ApiException e) { + error = e; throw StorageException.coalesce(e); + } catch (Exception e) { + error = e; + throw e; + } finally { + if (metricsContext != null) { + metricsContext.recordOperation(getOptions().getStorageClientMetrics(), error); + } } } @@ -307,37 +326,48 @@ public Blob internalCreateFrom(Path path, BlobInfo info, Opts o throw new StorageException(0, path + " is a directory"); } - GrpcCallContext grpcCallContext = - opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); - WriteObjectRequest req = getWriteObjectRequest(info, opts); - - ClientStreamingCallable write = - storageClient.writeObjectCallable().withDefaultCallContext(grpcCallContext); - - ApiFuture start = startResumableWrite(grpcCallContext, req, opts); - ApiFuture session2 = - ApiFutures.transform( - start, - rw -> - ResumableSession.grpc( - retrier.withAlg(retryAlgorithmManager.idempotent()), - write, - storageClient.queryWriteStatusCallable(), - rw, - opts.getHasher()), - MoreExecutors.directExecutor()); + StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("WriteObject"); + Throwable error = null; try { - GrpcResumableSession got = session2.get(); - ResumableOperationResult<@Nullable Object> put = got.put(RewindableContent.of(path)); - Object object = put.getObject(); - if (object == null) { - // if by some odd chance the put didn't get the Object, query for it - ResumableOperationResult<@Nullable Object> query = got.query(); - object = query.getObject(); + GrpcCallContext grpcCallContext = + opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); + WriteObjectRequest req = getWriteObjectRequest(info, opts); + + ClientStreamingCallable write = + storageClient.writeObjectCallable().withDefaultCallContext(grpcCallContext); + + ApiFuture start = startResumableWrite(grpcCallContext, req, opts); + ApiFuture session2 = + ApiFutures.transform( + start, + rw -> + ResumableSession.grpc( + retrier.withAlg(retryAlgorithmManager.idempotent()), + write, + storageClient.queryWriteStatusCallable(), + rw, + opts.getHasher()), + MoreExecutors.directExecutor()); + try { + GrpcResumableSession got = session2.get(); + ResumableOperationResult<@Nullable Object> put = got.put(RewindableContent.of(path)); + Object object = put.getObject(); + if (object == null) { + // if by some odd chance the put didn't get the Object, query for it + ResumableOperationResult<@Nullable Object> query = got.query(); + object = query.getObject(); + } + return codecs.blobInfo().decode(object).asBlob(this); + } catch (InterruptedException | ExecutionException e) { + throw StorageException.coalesce(e); + } + } catch (Throwable t) { + error = t; + throw t; + } finally { + if (multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), error); } - return codecs.blobInfo().decode(object).asBlob(this); - } catch (InterruptedException | ExecutionException e) { - throw StorageException.coalesce(e); } } @@ -353,35 +383,46 @@ public Blob createFrom( throws IOException { requireNonNull(blobInfo, "blobInfo must be non null"); - Opts opts = Opts.unwrap(options).resolveFrom(blobInfo).prepend(defaultOpts); - GrpcCallContext grpcCallContext = - opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); - WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts); + StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("WriteObject"); + Throwable error = null; + try { + Opts opts = Opts.unwrap(options).resolveFrom(blobInfo).prepend(defaultOpts); + GrpcCallContext grpcCallContext = + opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); + WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts); - ApiFuture start = startResumableWrite(grpcCallContext, req, opts); - - BufferedWritableByteChannelSession session = - ResumableMedia.gapic() - .write() - .byteChannel( - storageClient.writeObjectCallable().withDefaultCallContext(grpcCallContext)) - .setHasher(opts.getHasher()) - .setByteStringStrategy(ByteStringStrategy.noCopy()) - .resumable() - .withRetryConfig(retrier.withAlg(retryAlgorithmManager.idempotent())) - .buffered(Buffers.allocateAligned(bufferSize, _256KiB)) - .setStartAsync(start) - .build(); + ApiFuture start = startResumableWrite(grpcCallContext, req, opts); - // Specifically not in the try-with, so we don't close the provided stream - ReadableByteChannel src = - Channels.newChannel(firstNonNull(in, new ByteArrayInputStream(ZERO_BYTES))); - try (BufferedWritableByteChannel dst = session.open()) { - ByteStreams.copy(src, dst); - } catch (Exception e) { - throw StorageException.coalesce(e); + BufferedWritableByteChannelSession session = + ResumableMedia.gapic() + .write() + .byteChannel( + storageClient.writeObjectCallable().withDefaultCallContext(grpcCallContext)) + .setHasher(opts.getHasher()) + .setByteStringStrategy(ByteStringStrategy.noCopy()) + .resumable() + .withRetryConfig(retrier.withAlg(retryAlgorithmManager.idempotent())) + .buffered(Buffers.allocateAligned(bufferSize, _256KiB)) + .setStartAsync(start) + .build(); + + // Specifically not in the try-with, so we don't close the provided stream + ReadableByteChannel src = + Channels.newChannel(firstNonNull(in, new ByteArrayInputStream(ZERO_BYTES))); + try (BufferedWritableByteChannel dst = session.open()) { + ByteStreams.copy(src, dst); + } catch (Exception e) { + throw StorageException.coalesce(e); + } + return getBlob(session.getResult()); + } catch (Throwable t) { + error = t; + throw t; + } finally { + if (multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), error); + } } - return getBlob(session.getResult()); } @Override @@ -400,11 +441,14 @@ public Bucket lockRetentionPolicy(BucketInfo bucket, BucketTargetOption... optio .setBucket(bucketNameCodec.encode(bucket.getName())); LockBucketRetentionPolicyRequest req = opts.lockBucketRetentionPolicyRequest().apply(builder).build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = + getOptions().createMetricsContext("LockBucketRetentionPolicy"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.lockBucketRetentionPolicyCallable().call(req, merge), - syntaxDecoders.bucket); + syntaxDecoders.bucket, + metricsContext); } @Override @@ -439,14 +483,16 @@ private Blob internalObjectRestore(BlobId blobId, Opts opts) { .setObject(blobId.getName()); ifNonNull(blobId.getGeneration(), builder::setGeneration); RestoreObjectRequest req = finalOpts.restoreObjectRequest().apply(builder).build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("RestoreObject"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.restoreObjectCallable().call(req, merge), resp -> { BlobInfo tmp = codecs.blobInfo().decode(resp); return finalOpts.clearBlobFields().decode(tmp).asBlob(this); - }); + }, + metricsContext); } @Override @@ -465,7 +511,9 @@ public Page list(BucketListOption... options) { if (!request.getReturnPartialSuccess()) { try { - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListBuckets"); + GrpcCallContext merge = + Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(request), () -> storageClient.listBucketsPagedCallable().call(request, merge), @@ -474,7 +522,9 @@ public Page list(BucketListOption... options) { resp.getPage(), syntaxDecoders.bucket.andThen(opts.clearBucketFields()), retrier, - retryAlgorithmManager.getFor(request))); + retryAlgorithmManager.getFor(request), + () -> metricsContext != null ? metricsContext.asSubAttemptContext() : null), + metricsContext); } catch (Exception e) { throw StorageException.coalesce(e); } @@ -497,11 +547,15 @@ public Page list(String bucket, BlobListOption... options) { ListObjectsRequest.newBuilder().setParent(bucketNameCodec.encode(bucket)); ListObjectsRequest req = opts.listObjectsRequest().apply(builder).build(); try { - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListObjects"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.listObjectsCallable().call(req, merge), - resp -> new ListObjectsWithSyntheticDirectoriesPage(grpcCallContext, req, resp)); + resp -> + new ListObjectsWithSyntheticDirectoriesPage( + grpcCallContext, req, resp, metricsContext), + metricsContext); } catch (Exception e) { throw StorageException.coalesce(e); } @@ -526,11 +580,13 @@ public Bucket update(BucketInfo bucketInfo, BucketTargetOption... options) { .map(NamedField::getGrpcName) .collect(ImmutableList.toImmutableList())); UpdateBucketRequest req = builder.build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("UpdateBucket"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.updateBucketCallable().call(req, merge), - syntaxDecoders.bucket); + syntaxDecoders.bucket, + metricsContext); } @Override @@ -552,11 +608,13 @@ public Blob update(BlobInfo blobInfo, BlobTargetOption... options) { .map(NamedField::getGrpcName) .collect(ImmutableList.toImmutableList())); UpdateObjectRequest req = builder.build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("UpdateObject"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.updateObjectCallable().call(req, merge), - syntaxDecoders.blob); + syntaxDecoders.blob, + metricsContext); } @Override @@ -572,7 +630,8 @@ public boolean delete(String bucket, BucketSourceOption... options) { DeleteBucketRequest.Builder builder = DeleteBucketRequest.newBuilder().setName(bucketNameCodec.encode(bucket)); DeleteBucketRequest req = opts.deleteBucketsRequest().apply(builder).build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("DeleteBucket"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return Boolean.TRUE.equals( retrier.run( retryAlgorithmManager.getFor(req), @@ -584,7 +643,8 @@ public boolean delete(String bucket, BucketSourceOption... options) { return false; } }, - Decoder.identity())); + Decoder.identity(), + metricsContext)); } @Override @@ -624,14 +684,16 @@ public Void internalObjectDelete(BlobId id, Opts opts) { .setObject(id.getName()); ifNonNull(id.getGeneration(), builder::setGeneration); DeleteObjectRequest req = finalOpts.deleteObjectsRequest().apply(builder).build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("DeleteObject"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> { storageClient.deleteObjectCallable().call(req, merge); return null; }, - Decoder.identity()); + Decoder.identity(), + metricsContext); } @Override @@ -647,11 +709,13 @@ public Blob compose(ComposeRequest composeRequest) { builder.setDestination(target); builder.setDeleteSourceObjects(composeRequest.isDeleteSourceObjects()); ComposeObjectRequest req = opts.composeObjectsRequest().apply(builder).build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("ComposeObject"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.composeObjectCallable().call(req, merge), - syntaxDecoders.blob); + syntaxDecoders.blob, + metricsContext); } @Override @@ -708,12 +772,26 @@ public CopyWriter copy(CopyRequest copyRequest) { GrpcCallContext grpcCallContext = grpcCallContextMapper.apply(GrpcCallContext.createDefault()); UnaryCallable callable = storageClient.rewriteObjectCallable().withDefaultCallContext(grpcCallContext); - GrpcCallContext retryContext = Retrying.newCallContext(); + StorageMetricsContext multiContext = + getOptions().createMultiStepMetricsContext("RewriteObject"); + StorageMetricsContext initialContext = + multiContext != null + ? multiContext.asSubAttemptContext() + : getOptions().createMetricsContext("RewriteObject"); + GrpcCallContext retryContext = Retrying.newCallContext(initialContext); RetrierWithAlg retrierWithAlg = retrier.withAlg(retryAlgorithmManager.idempotent()); return retrier.run( retryAlgorithmManager.getFor(req), () -> callable.call(req, retryContext), - (resp) -> new GapicCopyWriter(this, callable, retrierWithAlg, req, resp)); + (resp) -> { + GapicCopyWriter writer = + new GapicCopyWriter(this, callable, retrierWithAlg, req, resp, multiContext); + if (resp.getDone() && multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), null); + } + return writer; + }, + initialContext); } @Override @@ -723,14 +801,25 @@ public byte[] readAllBytes(String bucket, String blob, BlobSourceOption... optio @Override public byte[] readAllBytes(BlobId blob, BlobSourceOption... options) { - UnbufferedReadableByteChannelSession session = unbufferedReadSession(blob, options); + StorageMetricsContext metricsContext = getOptions().createMultiStepMetricsContext("ReadObject"); + UnbufferedReadableByteChannelSession session = + unbufferedReadSession(blob, options, metricsContext); ByteArrayOutputStream baos = new ByteArrayOutputStream(); + Throwable error = null; try (UnbufferedReadableByteChannel r = session.open(); WritableByteChannel w = Channels.newChannel(baos)) { ByteStreams.copy(r, w); } catch (ApiException | IOException e) { + error = e; throw StorageException.coalesce(e); + } catch (Exception e) { + error = e; + throw e; + } finally { + if (metricsContext != null) { + metricsContext.recordOperation(getOptions().getStorageClientMetrics(), error); + } } return baos.toByteArray(); } @@ -749,39 +838,66 @@ public GrpcBlobReadChannel reader(String bucket, String blob, BlobSourceOption.. public GrpcBlobReadChannel reader(BlobId blob, BlobSourceOption... options) { Opts opts = Opts.unwrap(options).resolveFrom(blob).prepend(defaultOpts); ReadObjectRequest request = getReadObjectRequest(blob, opts); - GrpcCallContext grpcCallContext = opts.grpcMetadataMapper().apply(Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMultiStepMetricsContext("ReadObject"); + GrpcCallContext subAttemptContext = + metricsContext != null + ? Retrying.newCallContext(metricsContext.asSubAttemptContext()) + : Retrying.newCallContext(); + GrpcCallContext grpcCallContext = opts.grpcMetadataMapper().apply(subAttemptContext); return new GrpcBlobReadChannel( readObjectCallable(grpcCallContext), retrier, retryAlgorithmManager.getFor(request), request, - !opts.autoGzipDecompression()); + !opts.autoGzipDecompression(), + metricsContext, + getOptions().getStorageClientMetrics()); } @Override public void downloadTo(BlobId blob, Path path, BlobSourceOption... options) { + StorageMetricsContext metricsContext = getOptions().createMultiStepMetricsContext("ReadObject"); + UnbufferedReadableByteChannelSession session = + unbufferedReadSession(blob, options, metricsContext); - UnbufferedReadableByteChannelSession session = unbufferedReadSession(blob, options); - + Throwable error = null; try (UnbufferedReadableByteChannel r = session.open(); WritableByteChannel w = Files.newByteChannel(path, WRITE_OPS)) { ByteStreams.copy(r, w); } catch (ApiException | IOException e) { + error = e; throw StorageException.coalesce(e); + } catch (Exception e) { + error = e; + throw e; + } finally { + if (metricsContext != null) { + metricsContext.recordOperation(getOptions().getStorageClientMetrics(), error); + } } } @Override public void downloadTo(BlobId blob, OutputStream outputStream, BlobSourceOption... options) { + StorageMetricsContext metricsContext = getOptions().createMultiStepMetricsContext("ReadObject"); + UnbufferedReadableByteChannelSession session = + unbufferedReadSession(blob, options, metricsContext); - UnbufferedReadableByteChannelSession session = unbufferedReadSession(blob, options); - + Throwable error = null; try (UnbufferedReadableByteChannel r = session.open(); WritableByteChannel w = Channels.newChannel(outputStream)) { ByteStreams.copy(r, w); } catch (ApiException | IOException e) { + error = e; throw StorageException.coalesce(e); + } catch (Exception e) { + error = e; + throw e; + } finally { + if (metricsContext != null) { + metricsContext.recordOperation(getOptions().getStorageClientMetrics(), error); + } } } @@ -792,10 +908,16 @@ public GrpcBlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts); Hasher hasher = opts.getHasher(); + StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("WriteObject"); + GrpcCallContext startContext = + multiContext != null + ? Retrying.newCallContext(multiContext.asSubAttemptContext()) + : Retrying.newCallContext(); + GrpcCallContext mergedStart = Utils.merge(grpcCallContext, startContext); // in JSON, the starting of the resumable session happens before the invocation of write can // happen. Emulate the same thing here. // 1. create the future - ApiFuture startResumableWrite = startResumableWrite(grpcCallContext, req, opts); + ApiFuture startResumableWrite = startResumableWrite(mergedStart, req, opts); // 2. await the result of the future ResumableWrite resumableWrite = ApiFutureUtils.await(startResumableWrite); // 3. wrap the result in another future container before constructing the BlobWriteChannel @@ -804,7 +926,9 @@ public GrpcBlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options storageClient.writeObjectCallable().withDefaultCallContext(grpcCallContext), retrier.withAlg(retryAlgorithmManager.idempotent()), () -> wrapped, - hasher); + hasher, + multiContext, + getOptions().getStorageClientMetrics()); } @Override @@ -817,7 +941,8 @@ public BlobInfo internalDirectUpload( optsWithDefaults.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); WriteObjectRequest req = getWriteObjectRequest(blobInfo, optsWithDefaults); Hasher hasher = opts.getHasher(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("WriteObject"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); RewindableContent content = RewindableContent.of(buf); return retrier.run( retryAlgorithmManager.getFor(req), @@ -839,7 +964,8 @@ public BlobInfo internalDirectUpload( } return session.getResult(); }, - this::getBlob); + this::getBlob, + metricsContext); } @Override @@ -950,9 +1076,10 @@ public List delete(Iterable blobIds) { @Override public Acl getAcl(String bucket, Entity entity, BucketSourceOption... options) { + StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetBucketACL"); try { Opts opts = Opts.unwrap(options).prepend(defaultOpts); - com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts); + com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts, metricsContext); Predicate entityPredicate = bucketAclEntityOrAltEq(codecs.entity().encode(entity)); @@ -980,9 +1107,14 @@ public Acl getAcl(String bucket, Entity entity) { @Override public boolean deleteAcl(String bucket, Entity entity, BucketSourceOption... options) { + StorageMetricsContext multiContext = + getOptions().createMultiStepMetricsContext("DeleteBucketACL"); + StorageMetricsContext subContext = + multiContext != null ? multiContext.asSubAttemptContext() : null; + Throwable error = null; try { Opts opts = Opts.unwrap(options).prepend(defaultOpts); - com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts); + com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts, subContext); String encode = codecs.entity().encode(entity); Predicate entityPredicate = bucketAclEntityOrAltEq(encode); @@ -1000,7 +1132,7 @@ public boolean deleteAcl(String bucket, Entity entity, BucketSourceOption... opt UpdateBucketRequest req = createUpdateBucketAclRequest(bucket, newAcls, metageneration); - com.google.storage.v2.Bucket updateResult = updateBucket(req); + com.google.storage.v2.Bucket updateResult = updateBucket(req, subContext); // read the response to ensure there is no longer an acl for the specified entity Optional first = updateResult.getAclList().stream().filter(entityPredicate).findFirst(); @@ -1012,8 +1144,16 @@ public boolean deleteAcl(String bucket, Entity entity, BucketSourceOption... opt if (se.getCode() == 404) { return false; } else { + error = se; throw se; } + } catch (Exception e) { + error = e; + throw e; + } finally { + if (multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), error); + } } } @@ -1024,7 +1164,7 @@ public boolean deleteAcl(String bucket, Entity entity) { @Override public Acl createAcl(String bucket, Acl acl, BucketSourceOption... options) { - return updateAcl(bucket, acl, options); + return internalUpdateAcl("CreateBucketACL", bucket, acl, options); } @Override @@ -1034,9 +1174,18 @@ public Acl createAcl(String bucket, Acl acl) { @Override public Acl updateAcl(String bucket, Acl acl, BucketSourceOption... options) { + return internalUpdateAcl("UpdateBucketACL", bucket, acl, options); + } + + private Acl internalUpdateAcl( + String operationName, String bucket, Acl acl, BucketSourceOption... options) { + StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext(operationName); + StorageMetricsContext subContext = + multiContext != null ? multiContext.asSubAttemptContext() : null; + Throwable error = null; try { Opts opts = Opts.unwrap(options).prepend(defaultOpts); - com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts); + com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts, subContext); BucketAccessControl encode = codecs.bucketAcl().encode(acl); String entity = encode.getEntity(); @@ -1050,7 +1199,7 @@ public Acl updateAcl(String bucket, Acl acl, BucketSourceOption... options) { UpdateBucketRequest req = createUpdateBucketAclRequest(bucket, newDefaultAcls, resp.getMetageneration()); - com.google.storage.v2.Bucket updateResult = updateBucket(req); + com.google.storage.v2.Bucket updateResult = updateBucket(req, subContext); Optional first = updateResult.getAclList().stream() @@ -1061,7 +1210,15 @@ public Acl updateAcl(String bucket, Acl acl, BucketSourceOption... options) { return first.orElseThrow( () -> new StorageException(0, "Acl update call success, but not in response")); } catch (NotFoundException e) { + error = e; throw StorageException.coalesce(e); + } catch (Exception e) { + error = e; + throw e; + } finally { + if (multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), error); + } } } @@ -1072,9 +1229,10 @@ public Acl updateAcl(String bucket, Acl acl) { @Override public List listAcls(String bucket, BucketSourceOption... options) { + StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListBucketACLs"); try { Opts opts = Opts.unwrap(options).prepend(defaultOpts); - com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts); + com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts, metricsContext); return resp.getAclList().stream() .map(codecs.bucketAcl()::decode) .collect(ImmutableList.toImmutableList()); @@ -1090,8 +1248,9 @@ public List listAcls(String bucket) { @Override public Acl getDefaultAcl(String bucket, Entity entity) { + StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetDefaultObjectACL"); try { - com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket); + com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, metricsContext); Predicate entityPredicate = objectAclEntityOrAltEq(codecs.entity().encode(entity)); @@ -1114,8 +1273,13 @@ public Acl getDefaultAcl(String bucket, Entity entity) { @Override public boolean deleteDefaultAcl(String bucket, Entity entity) { + StorageMetricsContext multiContext = + getOptions().createMultiStepMetricsContext("DeleteDefaultObjectACL"); + StorageMetricsContext subContext = + multiContext != null ? multiContext.asSubAttemptContext() : null; + Throwable error = null; try { - com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket); + com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, subContext); String encode = codecs.entity().encode(entity); Predicate entityPredicate = objectAclEntityOrAltEq(encode); @@ -1134,7 +1298,7 @@ public boolean deleteDefaultAcl(String bucket, Entity entity) { UpdateBucketRequest req = createUpdateDefaultAclRequest(bucket, newDefaultAcls, metageneration); - com.google.storage.v2.Bucket updateResult = updateBucket(req); + com.google.storage.v2.Bucket updateResult = updateBucket(req, subContext); // read the response to ensure there is no longer an acl for the specified entity Optional first = updateResult.getDefaultObjectAclList().stream().filter(entityPredicate).findFirst(); @@ -1146,20 +1310,36 @@ public boolean deleteDefaultAcl(String bucket, Entity entity) { if (se.getCode() == 404) { return false; } else { + error = se; throw se; } + } catch (Exception e) { + error = e; + throw e; + } finally { + if (multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), error); + } } } @Override public Acl createDefaultAcl(String bucket, Acl acl) { - return updateDefaultAcl(bucket, acl); + return internalUpdateDefaultAcl("CreateDefaultObjectACL", bucket, acl); } @Override public Acl updateDefaultAcl(String bucket, Acl acl) { + return internalUpdateDefaultAcl("UpdateDefaultObjectACL", bucket, acl); + } + + private Acl internalUpdateDefaultAcl(String operationName, String bucket, Acl acl) { + StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext(operationName); + StorageMetricsContext subContext = + multiContext != null ? multiContext.asSubAttemptContext() : null; + Throwable error = null; try { - com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket); + com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, subContext); ObjectAccessControl encode = codecs.objectAcl().encode(acl); String entity = encode.getEntity(); @@ -1174,7 +1354,7 @@ public Acl updateDefaultAcl(String bucket, Acl acl) { UpdateBucketRequest req = createUpdateDefaultAclRequest(bucket, newDefaultAcls, resp.getMetageneration()); - com.google.storage.v2.Bucket updateResult = updateBucket(req); + com.google.storage.v2.Bucket updateResult = updateBucket(req, subContext); Optional first = updateResult.getDefaultObjectAclList().stream() @@ -1185,14 +1365,24 @@ public Acl updateDefaultAcl(String bucket, Acl acl) { return first.orElseThrow( () -> new StorageException(0, "Acl update call success, but not in response")); } catch (NotFoundException e) { + error = e; throw StorageException.coalesce(e); + } catch (Exception e) { + error = e; + throw e; + } finally { + if (multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), error); + } } } @Override public List listDefaultAcls(String bucket) { + StorageMetricsContext metricsContext = + getOptions().createMetricsContext("ListDefaultObjectACLs"); try { - com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket); + com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, metricsContext); return resp.getDefaultObjectAclList().stream() .map(codecs.objectAcl()::decode) .collect(ImmutableList.toImmutableList()); @@ -1203,9 +1393,10 @@ public List listDefaultAcls(String bucket) { @Override public Acl getAcl(BlobId blob, Entity entity) { + StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetObjectACL"); try { Object req = codecs.blobId().encode(blob); - Object resp = getObjectWithAcls(req); + Object resp = getObjectWithAcls(req, metricsContext); Predicate entityPredicate = objectAclEntityOrAltEq(codecs.entity().encode(entity)); @@ -1228,9 +1419,14 @@ public Acl getAcl(BlobId blob, Entity entity) { @Override public boolean deleteAcl(BlobId blob, Entity entity) { + StorageMetricsContext multiContext = + getOptions().createMultiStepMetricsContext("DeleteObjectACL"); + StorageMetricsContext subContext = + multiContext != null ? multiContext.asSubAttemptContext() : null; + Throwable error = null; try { Object obj = codecs.blobId().encode(blob); - Object resp = getObjectWithAcls(obj); + Object resp = getObjectWithAcls(obj, subContext); String encode = codecs.entity().encode(entity); Predicate entityPredicate = objectAclEntityOrAltEq(encode); @@ -1248,7 +1444,7 @@ public boolean deleteAcl(BlobId blob, Entity entity) { UpdateObjectRequest req = createUpdateObjectAclRequest(obj, newDefaultAcls, metageneration); - Object updateResult = updateObject(req); + Object updateResult = updateObject(req, subContext); // read the response to ensure there is no longer an acl for the specified entity Optional first = updateResult.getAclList().stream().filter(entityPredicate).findFirst(); @@ -1260,21 +1456,37 @@ public boolean deleteAcl(BlobId blob, Entity entity) { if (se.getCode() == 404) { return false; } else { + error = se; throw se; } + } catch (Exception e) { + error = e; + throw e; + } finally { + if (multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), error); + } } } @Override public Acl createAcl(BlobId blob, Acl acl) { - return updateAcl(blob, acl); + return internalUpdateObjectAcl("CreateObjectACL", blob, acl); } @Override public Acl updateAcl(BlobId blob, Acl acl) { + return internalUpdateObjectAcl("UpdateObjectACL", blob, acl); + } + + private Acl internalUpdateObjectAcl(String operationName, BlobId blob, Acl acl) { + StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext(operationName); + StorageMetricsContext subContext = + multiContext != null ? multiContext.asSubAttemptContext() : null; + Throwable error = null; try { Object obj = codecs.blobId().encode(blob); - Object resp = getObjectWithAcls(obj); + Object resp = getObjectWithAcls(obj, subContext); ObjectAccessControl encode = codecs.objectAcl().encode(acl); String entity = encode.getEntity(); @@ -1288,7 +1500,7 @@ public Acl updateAcl(BlobId blob, Acl acl) { UpdateObjectRequest req = createUpdateObjectAclRequest(obj, newDefaultAcls, resp.getMetageneration()); - Object updateResult = updateObject(req); + Object updateResult = updateObject(req, subContext); Optional first = updateResult.getAclList().stream() @@ -1299,15 +1511,24 @@ public Acl updateAcl(BlobId blob, Acl acl) { return first.orElseThrow( () -> new StorageException(0, "Acl update call success, but not in response")); } catch (NotFoundException e) { + error = e; throw StorageException.coalesce(e); + } catch (Exception e) { + error = e; + throw e; + } finally { + if (multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), error); + } } } @Override public List listAcls(BlobId blob) { + StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListObjectACLs"); try { Object req = codecs.blobId().encode(blob); - Object resp = getObjectWithAcls(req); + Object resp = getObjectWithAcls(req, metricsContext); return resp.getAclList().stream() .map(codecs.objectAcl()::decode) .collect(ImmutableList.toImmutableList()); @@ -1350,11 +1571,13 @@ public Policy getIamPolicy(String bucket, BucketSourceOption... options) { GetIamPolicyRequest.Builder builder = GetIamPolicyRequest.newBuilder().setResource(bucketNameCodec.encode(bucket)); GetIamPolicyRequest req = opts.getIamPolicyRequest().apply(builder).build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetIamPolicy"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.getIamPolicyCallable().call(req, merge), - codecs.policyCodec()); + codecs.policyCodec(), + metricsContext); } @Override @@ -1367,11 +1590,13 @@ public Policy setIamPolicy(String bucket, Policy policy, BucketSourceOption... o .setResource(bucketNameCodec.encode(bucket)) .setPolicy(codecs.policyCodec().encode(policy)) .build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("SetIamPolicy"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.setIamPolicyCallable().call(req, merge), - codecs.policyCodec()); + codecs.policyCodec(), + metricsContext); } @Override @@ -1385,7 +1610,8 @@ public List testIamPermissions( .setResource(bucketNameCodec.encode(bucket)) .addAllPermissions(permissions) .build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("TestIamPermissions"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.testIamPermissionsCallable().call(req, merge), @@ -1394,7 +1620,8 @@ public List testIamPermissions( return permissions.stream() .map(heldPermissions::contains) .collect(ImmutableList.toImmutableList()); - }); + }, + metricsContext); } @Override @@ -1458,10 +1685,12 @@ public Blob moveBlob(MoveBlobRequest request) { dstOpts.moveObjectsRequest().apply(b); MoveObjectRequest req = b.build(); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("MoveObject"); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.moveObjectCallable().call(req), - syntaxDecoders.blob); + syntaxDecoders.blob, + metricsContext); } @Override @@ -1562,12 +1791,22 @@ private final class ListObjectsWithSyntheticDirectoriesPage implements Page getNextPage() { ListObjectsRequest nextPageReq = req.toBuilder().setPageToken(resp.getNextPageToken()).build(); try { - GrpcCallContext merge = Utils.merge(ctx, Retrying.newCallContext()); + StorageMetricsContext metricsContext = + parentMetricsContext != null + ? parentMetricsContext.asSubAttemptContext() + : getOptions().createMetricsContext("ListObjects"); + GrpcCallContext merge = Utils.merge(ctx, Retrying.newCallContext(metricsContext)); ListObjectsResponse nextPageResp = retrier.run( retryAlgorithmManager.getFor(nextPageReq), () -> storageClient.listObjectsCallable().call(nextPageReq, merge), - Decoder.identity()); - return new ListObjectsWithSyntheticDirectoriesPage(ctx, nextPageReq, nextPageResp); + Decoder.identity(), + metricsContext); + return new ListObjectsWithSyntheticDirectoriesPage( + ctx, nextPageReq, nextPageResp, parentMetricsContext); } catch (Exception e) { throw StorageException.coalesce(e); } @@ -1715,16 +1960,27 @@ static final class TransformingPageDecorator< private final Decoder translator; private final Retrier retrier; private final ResultRetryAlgorithm resultRetryAlgorithm; + private final Supplier metricsContextSupplier; TransformingPageDecorator( PageT page, Decoder translator, Retrier retrier, ResultRetryAlgorithm resultRetryAlgorithm) { + this(page, translator, retrier, resultRetryAlgorithm, () -> null); + } + + TransformingPageDecorator( + PageT page, + Decoder translator, + Retrier retrier, + ResultRetryAlgorithm resultRetryAlgorithm, + Supplier metricsContextSupplier) { this.page = page; this.translator = translator; this.retrier = retrier; this.resultRetryAlgorithm = resultRetryAlgorithm; + this.metricsContextSupplier = metricsContextSupplier; } @Override @@ -1740,7 +1996,7 @@ public String getNextPageToken() { @Override public Page getNextPage() { return new TransformingPageDecorator<>( - page.getNextPage(), translator, retrier, resultRetryAlgorithm); + page.getNextPage(), translator, retrier, resultRetryAlgorithm, metricsContextSupplier); } @SuppressWarnings({"Convert2MethodRef"}) @@ -1760,7 +2016,8 @@ public Iterable iterateAll() { // prevent a javac 1.8 exception // https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8056984 Callable c = () -> prev.getNextPage(); - return retrier.run(resultRetryAlgorithm, c, Decoder.identity()); + return retrier.run( + resultRetryAlgorithm, c, Decoder.identity(), metricsContextSupplier.get()); }) .filter(Objects::nonNull) .flatMap(p -> StreamSupport.stream(p.getValues().spliterator(), false)) @@ -1894,10 +2151,19 @@ BidiWriteObjectRequest getBidiWriteObjectRequestForTakeover( private UnbufferedReadableByteChannelSession unbufferedReadSession( BlobId blob, BlobSourceOption[] options) { + return unbufferedReadSession(blob, options, null); + } + + private UnbufferedReadableByteChannelSession unbufferedReadSession( + BlobId blob, BlobSourceOption[] options, @Nullable StorageMetricsContext metricsContext) { Opts opts = Opts.unwrap(options).resolveFrom(blob).prepend(defaultOpts); ReadObjectRequest readObjectRequest = getReadObjectRequest(blob, opts); - GrpcCallContext grpcCallContext = opts.grpcMetadataMapper().apply(Retrying.newCallContext()); + GrpcCallContext subAttemptContext = + metricsContext != null + ? Retrying.newCallContext(metricsContext.asSubAttemptContext()) + : Retrying.newCallContext(); + GrpcCallContext grpcCallContext = opts.grpcMetadataMapper().apply(subAttemptContext); return ResumableMedia.gapic() .read() .byteChannel( @@ -1944,14 +2210,21 @@ private SourceObject sourceObjectEncode(SourceBlob from) { private com.google.storage.v2.ListBucketsResponse listBuckets( GrpcCallContext grpcCallContext, ListBucketsRequest request) { - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListBuckets"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(request), () -> storageClient.listBucketsCallable().call(request, merge), - Decoder.identity()); + Decoder.identity(), + metricsContext); } private com.google.storage.v2.Bucket getBucketWithDefaultAcls(String bucketName) { + return getBucketWithDefaultAcls(bucketName, null); + } + + private com.google.storage.v2.Bucket getBucketWithDefaultAcls( + String bucketName, @Nullable StorageMetricsContext metricsContext) { Fields fields = UnifiedOpts.fields( ImmutableSet.of( @@ -1966,15 +2239,25 @@ private com.google.storage.v2.Bucket getBucketWithDefaultAcls(String bucketName) .setName(bucketNameCodec.encode(bucketName)) .build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext activeContext = + metricsContext != null ? metricsContext : getOptions().createMetricsContext("GetBucket"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(activeContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.getBucketCallable().call(req, merge), - Decoder.identity()); + Decoder.identity(), + activeContext); } private com.google.storage.v2.Bucket getBucketWithAcls( String bucketName, Opts opts) { + return getBucketWithAcls(bucketName, opts, null); + } + + private com.google.storage.v2.Bucket getBucketWithAcls( + String bucketName, + Opts opts, + @Nullable StorageMetricsContext metricsContext) { Fields fields = UnifiedOpts.fields(ImmutableSet.of(BucketField.ACL, BucketField.METAGENERATION)); GrpcCallContext grpcCallContext = GrpcCallContext.createDefault(); @@ -1985,20 +2268,31 @@ private com.google.storage.v2.Bucket getBucketWithAcls( .setName(bucketNameCodec.encode(bucketName)) .build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext activeContext = + metricsContext != null ? metricsContext : getOptions().createMetricsContext("GetBucket"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(activeContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.getBucketCallable().call(req, merge), - Decoder.identity()); + Decoder.identity(), + activeContext); } private com.google.storage.v2.Bucket updateBucket(UpdateBucketRequest req) { + return updateBucket(req, null); + } + + private com.google.storage.v2.Bucket updateBucket( + UpdateBucketRequest req, @Nullable StorageMetricsContext metricsContext) { GrpcCallContext grpcCallContext = GrpcCallContext.createDefault(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext activeContext = + metricsContext != null ? metricsContext : getOptions().createMetricsContext("UpdateBucket"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(activeContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.updateBucketCallable().call(req, merge), - Decoder.identity()); + Decoder.identity(), + activeContext); } private static UpdateBucketRequest createUpdateDefaultAclRequest( @@ -2036,6 +2330,10 @@ private static UpdateBucketRequest createUpdateBucketAclRequest( } private Object getObjectWithAcls(Object obj) { + return getObjectWithAcls(obj, null); + } + + private Object getObjectWithAcls(Object obj, @Nullable StorageMetricsContext metricsContext) { Fields fields = UnifiedOpts.fields(ImmutableSet.of(BucketField.ACL, BucketField.METAGENERATION)); GrpcCallContext grpcCallContext = GrpcCallContext.createDefault(); @@ -2047,11 +2345,14 @@ private Object getObjectWithAcls(Object obj) { .setObject(obj.getName()) .build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext activeContext = + metricsContext != null ? metricsContext : getOptions().createMetricsContext("GetObject"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(activeContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.getObjectCallable().call(req, merge), - Decoder.identity()); + Decoder.identity(), + activeContext); } private static UpdateObjectRequest createUpdateObjectAclRequest( @@ -2073,12 +2374,20 @@ private static UpdateObjectRequest createUpdateObjectAclRequest( } private Object updateObject(UpdateObjectRequest req) { + return updateObject(req, null); + } + + private Object updateObject( + UpdateObjectRequest req, @Nullable StorageMetricsContext metricsContext) { GrpcCallContext grpcCallContext = GrpcCallContext.createDefault(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext activeContext = + metricsContext != null ? metricsContext : getOptions().createMetricsContext("UpdateObject"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(activeContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.updateObjectCallable().call(req, merge), - Decoder.identity()); + Decoder.identity(), + activeContext); } @NonNull @@ -2093,7 +2402,8 @@ public BlobInfo internalObjectGet(BlobId blobId, Opts opts) { .setObject(blobId.getName()); ifNonNull(blobId.getGeneration(), builder::setGeneration); GetObjectRequest req = finalOpts.getObjectsRequest().apply(builder).build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetObject"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); //noinspection DataFlowIssue return retrier.run( retryAlgorithmManager.getFor(req), @@ -2101,7 +2411,8 @@ public BlobInfo internalObjectGet(BlobId blobId, Opts opts) { resp -> { BlobInfo tmp = codecs.blobInfo().decode(resp); return finalOpts.clearBlobFields().decode(tmp); - }); + }, + metricsContext); } @Nullable @@ -2128,7 +2439,8 @@ private Bucket internalBucketGet(String bucket, Opts unwrap) { GetBucketRequest.Builder builder = GetBucketRequest.newBuilder().setName(bucketNameCodec.encode(bucket)); GetBucketRequest req = opts.getBucketsRequest().apply(builder).build(); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetBucket"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> { @@ -2138,7 +2450,8 @@ private Bucket internalBucketGet(String bucket, Opts unwrap) { return null; } }, - syntaxDecoders.bucket.andThen(opts.clearBucketFields())); + syntaxDecoders.bucket.andThen(opts.clearBucketFields()), + metricsContext); } private ZeroCopyServerStreamingCallable readObjectCallable( From 93b51b5dbc3be0eb2f1134e9cd641ff90b2803e6 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 07:11:20 +0000 Subject: [PATCH 2/9] fix(storage): address PR #14729 review comments on gRPC veneer metrics [Generated-by: AI] --- .../google/cloud/storage/GapicCopyWriter.java | 57 +++++++++++------ .../google/cloud/storage/GrpcStorageImpl.java | 61 ++++++++++++------- 2 files changed, 76 insertions(+), 42 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java index 48a2d44bfc80..c5d3ed703388 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java @@ -34,6 +34,7 @@ final class GapicCopyWriter extends CopyWriter { private final RewriteObjectRequest originalRequest; private final RewriteResponse initialResponse; @Nullable private final StorageMetricsContext metricsContext; + private boolean operationRecorded; private RewriteResponse mostRecentResponse; @@ -60,15 +61,26 @@ final class GapicCopyWriter extends CopyWriter { this.mostRecentResponse = initialResponse; this.originalRequest = originalRequest; this.metricsContext = metricsContext; + this.operationRecorded = initialResponse.getDone(); + } + + private void recordOperation(@Nullable Throwable error) { + if (metricsContext != null && !operationRecorded) { + metricsContext.recordOperation(storage.getOptions().getStorageClientMetrics(), error); + operationRecorded = true; + } } @Override public Blob getResult() { - while (!isDone()) { - copyChunk(); - } - if (metricsContext != null) { - metricsContext.recordOperation(storage.getOptions().getStorageClientMetrics(), null); + try { + while (!isDone()) { + copyChunk(); + } + recordOperation(null); + } catch (Throwable t) { + recordOperation(t); + throw t; } BlobInfo info = Conversions.grpc().blobInfo().decode(mostRecentResponse.getResource()); return info.asBlob(storage); @@ -92,20 +104,27 @@ public long getTotalBytesCopied() { @Override public void copyChunk() { if (!isDone()) { - RewriteObjectRequest req = - originalRequest.toBuilder().setRewriteToken(mostRecentResponse.getRewriteToken()).build(); - StorageMetricsContext chunkContext = - metricsContext != null - ? metricsContext.asSubAttemptContext() - : storage.getOptions().createMetricsContext("RewriteObject"); - GrpcCallContext retryContext = Retrying.newCallContext(chunkContext); - RewriteResponse resp = - retrier.run(() -> callable.call(req, retryContext), Decoder.identity(), chunkContext); - if (resp != null) { - mostRecentResponse = resp; - } - if (isDone() && metricsContext != null) { - metricsContext.recordOperation(storage.getOptions().getStorageClientMetrics(), null); + try { + RewriteObjectRequest req = + originalRequest.toBuilder() + .setRewriteToken(mostRecentResponse.getRewriteToken()) + .build(); + StorageMetricsContext chunkContext = + metricsContext != null + ? metricsContext.asSubAttemptContext() + : storage.getOptions().createMetricsContext("RewriteObject"); + GrpcCallContext retryContext = Retrying.newCallContext(chunkContext); + RewriteResponse resp = + retrier.run(() -> callable.call(req, retryContext), Decoder.identity(), chunkContext); + if (resp != null) { + mostRecentResponse = resp; + } + if (isDone()) { + recordOperation(null); + } + } catch (Throwable t) { + recordOperation(t); + throw t; } } } diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index 9160a671f260..4e4100a4a5d3 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java @@ -780,18 +780,25 @@ public CopyWriter copy(CopyRequest copyRequest) { : getOptions().createMetricsContext("RewriteObject"); GrpcCallContext retryContext = Retrying.newCallContext(initialContext); RetrierWithAlg retrierWithAlg = retrier.withAlg(retryAlgorithmManager.idempotent()); - return retrier.run( - retryAlgorithmManager.getFor(req), - () -> callable.call(req, retryContext), - (resp) -> { - GapicCopyWriter writer = - new GapicCopyWriter(this, callable, retrierWithAlg, req, resp, multiContext); - if (resp.getDone() && multiContext != null) { - multiContext.recordOperation(getOptions().getStorageClientMetrics(), null); - } - return writer; - }, - initialContext); + try { + return retrier.run( + retryAlgorithmManager.getFor(req), + () -> callable.call(req, retryContext), + (resp) -> { + GapicCopyWriter writer = + new GapicCopyWriter(this, callable, retrierWithAlg, req, resp, multiContext); + if (resp.getDone() && multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), null); + } + return writer; + }, + initialContext); + } catch (Throwable t) { + if (multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), t); + } + throw t; + } } @Override @@ -909,17 +916,25 @@ public GrpcBlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts); Hasher hasher = opts.getHasher(); StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("WriteObject"); - GrpcCallContext startContext = - multiContext != null - ? Retrying.newCallContext(multiContext.asSubAttemptContext()) - : Retrying.newCallContext(); - GrpcCallContext mergedStart = Utils.merge(grpcCallContext, startContext); - // in JSON, the starting of the resumable session happens before the invocation of write can - // happen. Emulate the same thing here. - // 1. create the future - ApiFuture startResumableWrite = startResumableWrite(mergedStart, req, opts); - // 2. await the result of the future - ResumableWrite resumableWrite = ApiFutureUtils.await(startResumableWrite); + ResumableWrite resumableWrite; + try { + GrpcCallContext startContext = + multiContext != null + ? Retrying.newCallContext(multiContext.asSubAttemptContext()) + : Retrying.newCallContext(); + GrpcCallContext mergedStart = Utils.merge(grpcCallContext, startContext); + // in JSON, the starting of the resumable session happens before the invocation of write can + // happen. Emulate the same thing here. + // 1. create the future + ApiFuture startResumableWrite = startResumableWrite(mergedStart, req, opts); + // 2. await the result of the future + resumableWrite = ApiFutureUtils.await(startResumableWrite); + } catch (Throwable t) { + if (multiContext != null) { + multiContext.recordOperation(getOptions().getStorageClientMetrics(), t); + } + throw t; + } // 3. wrap the result in another future container before constructing the BlobWriteChannel ApiFuture wrapped = ApiFutures.immediateFuture(resumableWrite); return new GrpcBlobWriteChannel( From bb8cd418643eadff2dde094c74ba0149a337b9c9 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 22 Oct 2026 11:10:25 +0000 Subject: [PATCH 3/9] fix(storage): refine gRPC veneer operation metrics and attempt tracking --- .../google/cloud/storage/GapicCopyWriter.java | 7 +- .../cloud/storage/GrpcBlobReadChannel.java | 3 +- .../google/cloud/storage/GrpcStorageImpl.java | 227 ++++++------------ 3 files changed, 84 insertions(+), 153 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java index c5d3ed703388..21c1aad1b59e 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java @@ -34,7 +34,6 @@ final class GapicCopyWriter extends CopyWriter { private final RewriteObjectRequest originalRequest; private final RewriteResponse initialResponse; @Nullable private final StorageMetricsContext metricsContext; - private boolean operationRecorded; private RewriteResponse mostRecentResponse; @@ -61,13 +60,11 @@ final class GapicCopyWriter extends CopyWriter { this.mostRecentResponse = initialResponse; this.originalRequest = originalRequest; this.metricsContext = metricsContext; - this.operationRecorded = initialResponse.getDone(); } private void recordOperation(@Nullable Throwable error) { - if (metricsContext != null && !operationRecorded) { + if (metricsContext != null) { metricsContext.recordOperation(storage.getOptions().getStorageClientMetrics(), error); - operationRecorded = true; } } @@ -111,7 +108,7 @@ public void copyChunk() { .build(); StorageMetricsContext chunkContext = metricsContext != null - ? metricsContext.asSubAttemptContext() + ? metricsContext : storage.getOptions().createMetricsContext("RewriteObject"); GrpcCallContext retryContext = Retrying.newCallContext(chunkContext); RewriteResponse resp = diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java index 0b30d9f4376a..927ac7c65e04 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java @@ -73,7 +73,8 @@ protected LazyReadChannel newLazyReadChannel() { ReadableByteChannelSessionBuilder b = ResumableMedia.gapic() .read() - .byteChannel(read, retrier, resultRetryAlgorithm) + .byteChannel( + read, retrier.withMetricsContext(getMetricsContext()), resultRetryAlgorithm) .setHasher(Hasher.readHasher()) .setAutoGzipDecompression(autoGzipDecompression); BufferHandle bufferHandle = getBufferHandle(); diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index 4e4100a4a5d3..39152543420e 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java @@ -259,50 +259,38 @@ public Blob create( @Override public Blob create(BlobInfo blobInfo, InputStream content, BlobWriteOption... options) { - StorageMetricsContext metricsContext = getOptions().createMetricsContext("WriteObject"); - Throwable error = null; - try { - requireNonNull(blobInfo, "blobInfo must be non null"); - InputStream inputStreamParam = firstNonNull(content, new ByteArrayInputStream(ZERO_BYTES)); + requireNonNull(blobInfo, "blobInfo must be non null"); + InputStream inputStreamParam = firstNonNull(content, new ByteArrayInputStream(ZERO_BYTES)); - Opts optsWithDefaults = Opts.unwrap(options).prepend(defaultOpts); - GrpcCallContext grpcCallContext = - optsWithDefaults.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); - WriteObjectRequest req = getWriteObjectRequest(blobInfo, optsWithDefaults); - Hasher hasher = optsWithDefaults.getHasher(); - GrpcCallContext merge = - Utils.merge( - grpcCallContext, - metricsContext != null - ? Retrying.newCallContext(metricsContext) - : Retrying.newCallContext()); - UnbufferedWritableByteChannelSession session = - ResumableMedia.gapic() - .write() - .byteChannel(storageClient.writeObjectCallable().withDefaultCallContext(merge)) - .setByteStringStrategy(ByteStringStrategy.noCopy()) - .setHasher(hasher) - .direct() - .unbuffered() - .setRequest(req) - .build(); + Opts optsWithDefaults = Opts.unwrap(options).prepend(defaultOpts); + GrpcCallContext grpcCallContext = + optsWithDefaults.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); + WriteObjectRequest req = getWriteObjectRequest(blobInfo, optsWithDefaults); + Hasher hasher = optsWithDefaults.getHasher(); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("WriteObject"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); + return retrier.run( + Retrying.neverRetry(), + () -> { + UnbufferedWritableByteChannelSession session = + ResumableMedia.gapic() + .write() + .byteChannel(storageClient.writeObjectCallable().withDefaultCallContext(merge)) + .setByteStringStrategy(ByteStringStrategy.noCopy()) + .setHasher(hasher) + .direct() + .unbuffered() + .setRequest(req) + .build(); - try (UnbufferedWritableByteChannel c = session.open()) { - ByteStreams.copy(Channels.newChannel(inputStreamParam), c); - } - ApiFuture responseApiFuture = session.getResult(); - return this.getBlob(responseApiFuture); - } catch (IOException | ApiException e) { - error = e; - throw StorageException.coalesce(e); - } catch (Exception e) { - error = e; - throw e; - } finally { - if (metricsContext != null) { - metricsContext.recordOperation(getOptions().getStorageClientMetrics(), error); - } - } + try (UnbufferedWritableByteChannel c = session.open()) { + ByteStreams.copy(Channels.newChannel(inputStreamParam), c); + } + ApiFuture responseApiFuture = session.getResult(); + return this.getBlob(responseApiFuture); + }, + Decoder.identity(), + metricsContext); } @Override @@ -336,13 +324,14 @@ public Blob internalCreateFrom(Path path, BlobInfo info, Opts o ClientStreamingCallable write = storageClient.writeObjectCallable().withDefaultCallContext(grpcCallContext); - ApiFuture start = startResumableWrite(grpcCallContext, req, opts); + ApiFuture start = + startResumableWrite(grpcCallContext, req, opts, multiContext); ApiFuture session2 = ApiFutures.transform( start, rw -> ResumableSession.grpc( - retrier.withAlg(retryAlgorithmManager.idempotent()), + retrier.withAlg(retryAlgorithmManager.idempotent(), multiContext), write, storageClient.queryWriteStatusCallable(), rw, @@ -391,7 +380,8 @@ public Blob createFrom( opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts); - ApiFuture start = startResumableWrite(grpcCallContext, req, opts); + ApiFuture start = + startResumableWrite(grpcCallContext, req, opts, multiContext); BufferedWritableByteChannelSession session = ResumableMedia.gapic() @@ -401,7 +391,7 @@ public Blob createFrom( .setHasher(opts.getHasher()) .setByteStringStrategy(ByteStringStrategy.noCopy()) .resumable() - .withRetryConfig(retrier.withAlg(retryAlgorithmManager.idempotent())) + .withRetryConfig(retrier.withAlg(retryAlgorithmManager.idempotent(), multiContext)) .buffered(Buffers.allocateAligned(bufferSize, _256KiB)) .setStartAsync(start) .build(); @@ -522,8 +512,7 @@ public Page list(BucketListOption... options) { resp.getPage(), syntaxDecoders.bucket.andThen(opts.clearBucketFields()), retrier, - retryAlgorithmManager.getFor(request), - () -> metricsContext != null ? metricsContext.asSubAttemptContext() : null), + retryAlgorithmManager.getFor(request)), metricsContext); } catch (Exception e) { throw StorageException.coalesce(e); @@ -552,9 +541,7 @@ public Page list(String bucket, BlobListOption... options) { return retrier.run( retryAlgorithmManager.getFor(req), () -> storageClient.listObjectsCallable().call(req, merge), - resp -> - new ListObjectsWithSyntheticDirectoriesPage( - grpcCallContext, req, resp, metricsContext), + resp -> new ListObjectsWithSyntheticDirectoriesPage(grpcCallContext, req, resp), metricsContext); } catch (Exception e) { throw StorageException.coalesce(e); @@ -775,9 +762,7 @@ public CopyWriter copy(CopyRequest copyRequest) { StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("RewriteObject"); StorageMetricsContext initialContext = - multiContext != null - ? multiContext.asSubAttemptContext() - : getOptions().createMetricsContext("RewriteObject"); + multiContext != null ? multiContext : getOptions().createMetricsContext("RewriteObject"); GrpcCallContext retryContext = Retrying.newCallContext(initialContext); RetrierWithAlg retrierWithAlg = retrier.withAlg(retryAlgorithmManager.idempotent()); try { @@ -848,7 +833,7 @@ public GrpcBlobReadChannel reader(BlobId blob, BlobSourceOption... options) { StorageMetricsContext metricsContext = getOptions().createMultiStepMetricsContext("ReadObject"); GrpcCallContext subAttemptContext = metricsContext != null - ? Retrying.newCallContext(metricsContext.asSubAttemptContext()) + ? Retrying.newCallContext(metricsContext) : Retrying.newCallContext(); GrpcCallContext grpcCallContext = opts.grpcMetadataMapper().apply(subAttemptContext); @@ -918,15 +903,11 @@ public GrpcBlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("WriteObject"); ResumableWrite resumableWrite; try { - GrpcCallContext startContext = - multiContext != null - ? Retrying.newCallContext(multiContext.asSubAttemptContext()) - : Retrying.newCallContext(); - GrpcCallContext mergedStart = Utils.merge(grpcCallContext, startContext); // in JSON, the starting of the resumable session happens before the invocation of write can // happen. Emulate the same thing here. // 1. create the future - ApiFuture startResumableWrite = startResumableWrite(mergedStart, req, opts); + ApiFuture startResumableWrite = + startResumableWrite(grpcCallContext, req, opts, multiContext); // 2. await the result of the future resumableWrite = ApiFutureUtils.await(startResumableWrite); } catch (Throwable t) { @@ -939,7 +920,7 @@ public GrpcBlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options ApiFuture wrapped = ApiFutures.immediateFuture(resumableWrite); return new GrpcBlobWriteChannel( storageClient.writeObjectCallable().withDefaultCallContext(grpcCallContext), - retrier.withAlg(retryAlgorithmManager.idempotent()), + retrier.withAlg(retryAlgorithmManager.idempotent(), multiContext), () -> wrapped, hasher, multiContext, @@ -1124,12 +1105,10 @@ public Acl getAcl(String bucket, Entity entity) { public boolean deleteAcl(String bucket, Entity entity, BucketSourceOption... options) { StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("DeleteBucketACL"); - StorageMetricsContext subContext = - multiContext != null ? multiContext.asSubAttemptContext() : null; Throwable error = null; try { Opts opts = Opts.unwrap(options).prepend(defaultOpts); - com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts, subContext); + com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts, multiContext); String encode = codecs.entity().encode(entity); Predicate entityPredicate = bucketAclEntityOrAltEq(encode); @@ -1147,7 +1126,7 @@ public boolean deleteAcl(String bucket, Entity entity, BucketSourceOption... opt UpdateBucketRequest req = createUpdateBucketAclRequest(bucket, newAcls, metageneration); - com.google.storage.v2.Bucket updateResult = updateBucket(req, subContext); + com.google.storage.v2.Bucket updateResult = updateBucket(req, multiContext); // read the response to ensure there is no longer an acl for the specified entity Optional first = updateResult.getAclList().stream().filter(entityPredicate).findFirst(); @@ -1195,12 +1174,10 @@ public Acl updateAcl(String bucket, Acl acl, BucketSourceOption... options) { private Acl internalUpdateAcl( String operationName, String bucket, Acl acl, BucketSourceOption... options) { StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext(operationName); - StorageMetricsContext subContext = - multiContext != null ? multiContext.asSubAttemptContext() : null; Throwable error = null; try { Opts opts = Opts.unwrap(options).prepend(defaultOpts); - com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts, subContext); + com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts, multiContext); BucketAccessControl encode = codecs.bucketAcl().encode(acl); String entity = encode.getEntity(); @@ -1214,7 +1191,7 @@ private Acl internalUpdateAcl( UpdateBucketRequest req = createUpdateBucketAclRequest(bucket, newDefaultAcls, resp.getMetageneration()); - com.google.storage.v2.Bucket updateResult = updateBucket(req, subContext); + com.google.storage.v2.Bucket updateResult = updateBucket(req, multiContext); Optional first = updateResult.getAclList().stream() @@ -1290,11 +1267,9 @@ public Acl getDefaultAcl(String bucket, Entity entity) { public boolean deleteDefaultAcl(String bucket, Entity entity) { StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("DeleteDefaultObjectACL"); - StorageMetricsContext subContext = - multiContext != null ? multiContext.asSubAttemptContext() : null; Throwable error = null; try { - com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, subContext); + com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, multiContext); String encode = codecs.entity().encode(entity); Predicate entityPredicate = objectAclEntityOrAltEq(encode); @@ -1313,7 +1288,7 @@ public boolean deleteDefaultAcl(String bucket, Entity entity) { UpdateBucketRequest req = createUpdateDefaultAclRequest(bucket, newDefaultAcls, metageneration); - com.google.storage.v2.Bucket updateResult = updateBucket(req, subContext); + com.google.storage.v2.Bucket updateResult = updateBucket(req, multiContext); // read the response to ensure there is no longer an acl for the specified entity Optional first = updateResult.getDefaultObjectAclList().stream().filter(entityPredicate).findFirst(); @@ -1350,11 +1325,9 @@ public Acl updateDefaultAcl(String bucket, Acl acl) { private Acl internalUpdateDefaultAcl(String operationName, String bucket, Acl acl) { StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext(operationName); - StorageMetricsContext subContext = - multiContext != null ? multiContext.asSubAttemptContext() : null; Throwable error = null; try { - com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, subContext); + com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, multiContext); ObjectAccessControl encode = codecs.objectAcl().encode(acl); String entity = encode.getEntity(); @@ -1369,7 +1342,7 @@ private Acl internalUpdateDefaultAcl(String operationName, String bucket, Acl ac UpdateBucketRequest req = createUpdateDefaultAclRequest(bucket, newDefaultAcls, resp.getMetageneration()); - com.google.storage.v2.Bucket updateResult = updateBucket(req, subContext); + com.google.storage.v2.Bucket updateResult = updateBucket(req, multiContext); Optional first = updateResult.getDefaultObjectAclList().stream() @@ -1436,12 +1409,10 @@ public Acl getAcl(BlobId blob, Entity entity) { public boolean deleteAcl(BlobId blob, Entity entity) { StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("DeleteObjectACL"); - StorageMetricsContext subContext = - multiContext != null ? multiContext.asSubAttemptContext() : null; Throwable error = null; try { Object obj = codecs.blobId().encode(blob); - Object resp = getObjectWithAcls(obj, subContext); + Object resp = getObjectWithAcls(obj, multiContext); String encode = codecs.entity().encode(entity); Predicate entityPredicate = objectAclEntityOrAltEq(encode); @@ -1459,7 +1430,7 @@ public boolean deleteAcl(BlobId blob, Entity entity) { UpdateObjectRequest req = createUpdateObjectAclRequest(obj, newDefaultAcls, metageneration); - Object updateResult = updateObject(req, subContext); + Object updateResult = updateObject(req, multiContext); // read the response to ensure there is no longer an acl for the specified entity Optional first = updateResult.getAclList().stream().filter(entityPredicate).findFirst(); @@ -1496,12 +1467,10 @@ public Acl updateAcl(BlobId blob, Acl acl) { private Acl internalUpdateObjectAcl(String operationName, BlobId blob, Acl acl) { StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext(operationName); - StorageMetricsContext subContext = - multiContext != null ? multiContext.asSubAttemptContext() : null; Throwable error = null; try { Object obj = codecs.blobId().encode(blob); - Object resp = getObjectWithAcls(obj, subContext); + Object resp = getObjectWithAcls(obj, multiContext); ObjectAccessControl encode = codecs.objectAcl().encode(acl); String entity = encode.getEntity(); @@ -1515,7 +1484,7 @@ private Acl internalUpdateObjectAcl(String operationName, BlobId blob, Acl acl) UpdateObjectRequest req = createUpdateObjectAclRequest(obj, newDefaultAcls, resp.getMetageneration()); - Object updateResult = updateObject(req, subContext); + Object updateResult = updateObject(req, multiContext); Optional first = updateResult.getAclList().stream() @@ -1700,10 +1669,13 @@ public Blob moveBlob(MoveBlobRequest request) { dstOpts.moveObjectsRequest().apply(b); MoveObjectRequest req = b.build(); + GrpcCallContext grpcCallContext = + dstOpts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); StorageMetricsContext metricsContext = getOptions().createMetricsContext("MoveObject"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), - () -> storageClient.moveObjectCallable().call(req), + () -> storageClient.moveObjectCallable().call(req, merge), syntaxDecoders.blob, metricsContext); } @@ -1806,22 +1778,12 @@ private final class ListObjectsWithSyntheticDirectoriesPage implements Page getNextPage() { ListObjectsRequest nextPageReq = req.toBuilder().setPageToken(resp.getNextPageToken()).build(); try { - StorageMetricsContext metricsContext = - parentMetricsContext != null - ? parentMetricsContext.asSubAttemptContext() - : getOptions().createMetricsContext("ListObjects"); + StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListObjects"); GrpcCallContext merge = Utils.merge(ctx, Retrying.newCallContext(metricsContext)); ListObjectsResponse nextPageResp = retrier.run( @@ -1850,8 +1809,7 @@ public Page getNextPage() { () -> storageClient.listObjectsCallable().call(nextPageReq, merge), Decoder.identity(), metricsContext); - return new ListObjectsWithSyntheticDirectoriesPage( - ctx, nextPageReq, nextPageResp, parentMetricsContext); + return new ListObjectsWithSyntheticDirectoriesPage(ctx, nextPageReq, nextPageResp); } catch (Exception e) { throw StorageException.coalesce(e); } @@ -1975,27 +1933,16 @@ static final class TransformingPageDecorator< private final Decoder translator; private final Retrier retrier; private final ResultRetryAlgorithm resultRetryAlgorithm; - private final Supplier metricsContextSupplier; TransformingPageDecorator( PageT page, Decoder translator, Retrier retrier, ResultRetryAlgorithm resultRetryAlgorithm) { - this(page, translator, retrier, resultRetryAlgorithm, () -> null); - } - - TransformingPageDecorator( - PageT page, - Decoder translator, - Retrier retrier, - ResultRetryAlgorithm resultRetryAlgorithm, - Supplier metricsContextSupplier) { this.page = page; this.translator = translator; this.retrier = retrier; this.resultRetryAlgorithm = resultRetryAlgorithm; - this.metricsContextSupplier = metricsContextSupplier; } @Override @@ -2011,7 +1958,7 @@ public String getNextPageToken() { @Override public Page getNextPage() { return new TransformingPageDecorator<>( - page.getNextPage(), translator, retrier, resultRetryAlgorithm, metricsContextSupplier); + page.getNextPage(), translator, retrier, resultRetryAlgorithm); } @SuppressWarnings({"Convert2MethodRef"}) @@ -2031,8 +1978,7 @@ public Iterable iterateAll() { // prevent a javac 1.8 exception // https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8056984 Callable c = () -> prev.getNextPage(); - return retrier.run( - resultRetryAlgorithm, c, Decoder.identity(), metricsContextSupplier.get()); + return retrier.run(resultRetryAlgorithm, c, Decoder.identity()); }) .filter(Objects::nonNull) .flatMap(p -> StreamSupport.stream(p.getValues().spliterator(), false)) @@ -2164,11 +2110,6 @@ BidiWriteObjectRequest getBidiWriteObjectRequestForTakeover( return opts.bidiWriteObjectRequest().apply(requestBuilder).build(); } - private UnbufferedReadableByteChannelSession unbufferedReadSession( - BlobId blob, BlobSourceOption[] options) { - return unbufferedReadSession(blob, options, null); - } - private UnbufferedReadableByteChannelSession unbufferedReadSession( BlobId blob, BlobSourceOption[] options, @Nullable StorageMetricsContext metricsContext) { @@ -2176,14 +2117,14 @@ private UnbufferedReadableByteChannelSession unbufferedReadSession( ReadObjectRequest readObjectRequest = getReadObjectRequest(blob, opts); GrpcCallContext subAttemptContext = metricsContext != null - ? Retrying.newCallContext(metricsContext.asSubAttemptContext()) + ? Retrying.newCallContext(metricsContext) : Retrying.newCallContext(); GrpcCallContext grpcCallContext = opts.grpcMetadataMapper().apply(subAttemptContext); return ResumableMedia.gapic() .read() .byteChannel( readObjectCallable(grpcCallContext), - retrier, + retrier.withMetricsContext(metricsContext), retryAlgorithmManager.getFor(readObjectRequest)) .setAutoGzipDecompression(!opts.autoGzipDecompression()) .unbuffered() @@ -2194,14 +2135,27 @@ private UnbufferedReadableByteChannelSession unbufferedReadSession( @VisibleForTesting ApiFuture startResumableWrite( GrpcCallContext grpcCallContext, WriteObjectRequest req, Opts opts) { - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext()); + return startResumableWrite(grpcCallContext, req, opts, null); + } + + ApiFuture startResumableWrite( + GrpcCallContext grpcCallContext, + WriteObjectRequest req, + Opts opts, + @Nullable StorageMetricsContext metricsContext) { + GrpcCallContext merge = + Utils.merge( + grpcCallContext, + metricsContext != null + ? Retrying.newCallContext(metricsContext) + : Retrying.newCallContext()); return ResumableMedia.gapic() .write() .resumableWrite( storageClient.startResumableWriteCallable().withDefaultCallContext(merge), req, opts, - retrier.withAlg(retryAlgorithmManager.getFor(req))); + retrier.withAlg(retryAlgorithmManager.getFor(req), metricsContext)); } ApiFuture startResumableWrite( @@ -2234,10 +2188,6 @@ private com.google.storage.v2.ListBucketsResponse listBuckets( metricsContext); } - private com.google.storage.v2.Bucket getBucketWithDefaultAcls(String bucketName) { - return getBucketWithDefaultAcls(bucketName, null); - } - private com.google.storage.v2.Bucket getBucketWithDefaultAcls( String bucketName, @Nullable StorageMetricsContext metricsContext) { Fields fields = @@ -2264,11 +2214,6 @@ private com.google.storage.v2.Bucket getBucketWithDefaultAcls( activeContext); } - private com.google.storage.v2.Bucket getBucketWithAcls( - String bucketName, Opts opts) { - return getBucketWithAcls(bucketName, opts, null); - } - private com.google.storage.v2.Bucket getBucketWithAcls( String bucketName, Opts opts, @@ -2293,10 +2238,6 @@ private com.google.storage.v2.Bucket getBucketWithAcls( activeContext); } - private com.google.storage.v2.Bucket updateBucket(UpdateBucketRequest req) { - return updateBucket(req, null); - } - private com.google.storage.v2.Bucket updateBucket( UpdateBucketRequest req, @Nullable StorageMetricsContext metricsContext) { GrpcCallContext grpcCallContext = GrpcCallContext.createDefault(); @@ -2344,10 +2285,6 @@ private static UpdateBucketRequest createUpdateBucketAclRequest( .build(); } - private Object getObjectWithAcls(Object obj) { - return getObjectWithAcls(obj, null); - } - private Object getObjectWithAcls(Object obj, @Nullable StorageMetricsContext metricsContext) { Fields fields = UnifiedOpts.fields(ImmutableSet.of(BucketField.ACL, BucketField.METAGENERATION)); @@ -2388,10 +2325,6 @@ private static UpdateObjectRequest createUpdateObjectAclRequest( .build(); } - private Object updateObject(UpdateObjectRequest req) { - return updateObject(req, null); - } - private Object updateObject( UpdateObjectRequest req, @Nullable StorageMetricsContext metricsContext) { GrpcCallContext grpcCallContext = GrpcCallContext.createDefault(); From 6b27e7f2f5b69559243941abf615c33ebff27bf2 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Tue, 27 Oct 2026 08:01:29 +0000 Subject: [PATCH 4/9] refactor(storage): use StorageClientMetrics for context creation in gRPC veneer [Generated-by: AI] --- .../google/cloud/storage/GapicCopyWriter.java | 3 +- .../google/cloud/storage/GrpcStorageImpl.java | 166 +++++++++++++----- 2 files changed, 122 insertions(+), 47 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java index 21c1aad1b59e..c1be891367ee 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java @@ -109,7 +109,8 @@ public void copyChunk() { StorageMetricsContext chunkContext = metricsContext != null ? metricsContext - : storage.getOptions().createMetricsContext("RewriteObject"); + : StorageClientMetrics.createContext( + storage.getOptions().getStorageClientMetrics(), "RewriteObject"); GrpcCallContext retryContext = Retrying.newCallContext(chunkContext); RewriteResponse resp = retrier.run(() -> callable.call(req, retryContext), Decoder.identity(), chunkContext); diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index 39152543420e..6f80e4125189 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java @@ -229,7 +229,8 @@ public Bucket create(BucketInfo bucketInfo, BucketTargetOption... options) { builder.getBucketBuilder().setProject(projectNameCodec.encode(getOptions().getProjectId())); } CreateBucketRequest req = opts.createBucketsRequest().apply(builder).build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("CreateBucket"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "CreateBucket"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -267,7 +268,8 @@ public Blob create(BlobInfo blobInfo, InputStream content, BlobWriteOption... op optsWithDefaults.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); WriteObjectRequest req = getWriteObjectRequest(blobInfo, optsWithDefaults); Hasher hasher = optsWithDefaults.getHasher(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("WriteObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "WriteObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( Retrying.neverRetry(), @@ -314,7 +316,9 @@ public Blob internalCreateFrom(Path path, BlobInfo info, Opts o throw new StorageException(0, path + " is a directory"); } - StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("WriteObject"); + StorageMetricsContext multiContext = + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "WriteObject"); Throwable error = null; try { GrpcCallContext grpcCallContext = @@ -372,7 +376,9 @@ public Blob createFrom( throws IOException { requireNonNull(blobInfo, "blobInfo must be non null"); - StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("WriteObject"); + StorageMetricsContext multiContext = + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "WriteObject"); Throwable error = null; try { Opts opts = Opts.unwrap(options).resolveFrom(blobInfo).prepend(defaultOpts); @@ -432,7 +438,8 @@ public Bucket lockRetentionPolicy(BucketInfo bucket, BucketTargetOption... optio LockBucketRetentionPolicyRequest req = opts.lockBucketRetentionPolicyRequest().apply(builder).build(); StorageMetricsContext metricsContext = - getOptions().createMetricsContext("LockBucketRetentionPolicy"); + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "LockBucketRetentionPolicy"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -473,7 +480,8 @@ private Blob internalObjectRestore(BlobId blobId, Opts opts) { .setObject(blobId.getName()); ifNonNull(blobId.getGeneration(), builder::setGeneration); RestoreObjectRequest req = finalOpts.restoreObjectRequest().apply(builder).build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("RestoreObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "RestoreObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -501,7 +509,9 @@ public Page list(BucketListOption... options) { if (!request.getReturnPartialSuccess()) { try { - StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListBuckets"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "ListBuckets"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( @@ -536,7 +546,8 @@ public Page list(String bucket, BlobListOption... options) { ListObjectsRequest.newBuilder().setParent(bucketNameCodec.encode(bucket)); ListObjectsRequest req = opts.listObjectsRequest().apply(builder).build(); try { - StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListObjects"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "ListObjects"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -567,7 +578,8 @@ public Bucket update(BucketInfo bucketInfo, BucketTargetOption... options) { .map(NamedField::getGrpcName) .collect(ImmutableList.toImmutableList())); UpdateBucketRequest req = builder.build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("UpdateBucket"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "UpdateBucket"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -595,7 +607,8 @@ public Blob update(BlobInfo blobInfo, BlobTargetOption... options) { .map(NamedField::getGrpcName) .collect(ImmutableList.toImmutableList())); UpdateObjectRequest req = builder.build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("UpdateObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "UpdateObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -617,7 +630,8 @@ public boolean delete(String bucket, BucketSourceOption... options) { DeleteBucketRequest.Builder builder = DeleteBucketRequest.newBuilder().setName(bucketNameCodec.encode(bucket)); DeleteBucketRequest req = opts.deleteBucketsRequest().apply(builder).build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("DeleteBucket"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "DeleteBucket"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return Boolean.TRUE.equals( retrier.run( @@ -671,7 +685,8 @@ public Void internalObjectDelete(BlobId id, Opts opts) { .setObject(id.getName()); ifNonNull(id.getGeneration(), builder::setGeneration); DeleteObjectRequest req = finalOpts.deleteObjectsRequest().apply(builder).build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("DeleteObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "DeleteObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -696,7 +711,8 @@ public Blob compose(ComposeRequest composeRequest) { builder.setDestination(target); builder.setDeleteSourceObjects(composeRequest.isDeleteSourceObjects()); ComposeObjectRequest req = opts.composeObjectsRequest().apply(builder).build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("ComposeObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "ComposeObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -760,9 +776,13 @@ public CopyWriter copy(CopyRequest copyRequest) { UnaryCallable callable = storageClient.rewriteObjectCallable().withDefaultCallContext(grpcCallContext); StorageMetricsContext multiContext = - getOptions().createMultiStepMetricsContext("RewriteObject"); + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "RewriteObject"); StorageMetricsContext initialContext = - multiContext != null ? multiContext : getOptions().createMetricsContext("RewriteObject"); + multiContext != null + ? multiContext + : StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "RewriteObject"); GrpcCallContext retryContext = Retrying.newCallContext(initialContext); RetrierWithAlg retrierWithAlg = retrier.withAlg(retryAlgorithmManager.idempotent()); try { @@ -793,7 +813,9 @@ public byte[] readAllBytes(String bucket, String blob, BlobSourceOption... optio @Override public byte[] readAllBytes(BlobId blob, BlobSourceOption... options) { - StorageMetricsContext metricsContext = getOptions().createMultiStepMetricsContext("ReadObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "ReadObject"); UnbufferedReadableByteChannelSession session = unbufferedReadSession(blob, options, metricsContext); @@ -830,7 +852,9 @@ public GrpcBlobReadChannel reader(String bucket, String blob, BlobSourceOption.. public GrpcBlobReadChannel reader(BlobId blob, BlobSourceOption... options) { Opts opts = Opts.unwrap(options).resolveFrom(blob).prepend(defaultOpts); ReadObjectRequest request = getReadObjectRequest(blob, opts); - StorageMetricsContext metricsContext = getOptions().createMultiStepMetricsContext("ReadObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "ReadObject"); GrpcCallContext subAttemptContext = metricsContext != null ? Retrying.newCallContext(metricsContext) @@ -849,7 +873,9 @@ public GrpcBlobReadChannel reader(BlobId blob, BlobSourceOption... options) { @Override public void downloadTo(BlobId blob, Path path, BlobSourceOption... options) { - StorageMetricsContext metricsContext = getOptions().createMultiStepMetricsContext("ReadObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "ReadObject"); UnbufferedReadableByteChannelSession session = unbufferedReadSession(blob, options, metricsContext); @@ -872,7 +898,9 @@ public void downloadTo(BlobId blob, Path path, BlobSourceOption... options) { @Override public void downloadTo(BlobId blob, OutputStream outputStream, BlobSourceOption... options) { - StorageMetricsContext metricsContext = getOptions().createMultiStepMetricsContext("ReadObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "ReadObject"); UnbufferedReadableByteChannelSession session = unbufferedReadSession(blob, options, metricsContext); @@ -900,7 +928,9 @@ public GrpcBlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts); Hasher hasher = opts.getHasher(); - StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("WriteObject"); + StorageMetricsContext multiContext = + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "WriteObject"); ResumableWrite resumableWrite; try { // in JSON, the starting of the resumable session happens before the invocation of write can @@ -937,7 +967,8 @@ public BlobInfo internalDirectUpload( optsWithDefaults.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); WriteObjectRequest req = getWriteObjectRequest(blobInfo, optsWithDefaults); Hasher hasher = opts.getHasher(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("WriteObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "WriteObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); RewindableContent content = RewindableContent.of(buf); return retrier.run( @@ -1072,7 +1103,8 @@ public List delete(Iterable blobIds) { @Override public Acl getAcl(String bucket, Entity entity, BucketSourceOption... options) { - StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetBucketACL"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "GetBucketACL"); try { Opts opts = Opts.unwrap(options).prepend(defaultOpts); com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts, metricsContext); @@ -1104,7 +1136,8 @@ public Acl getAcl(String bucket, Entity entity) { @Override public boolean deleteAcl(String bucket, Entity entity, BucketSourceOption... options) { StorageMetricsContext multiContext = - getOptions().createMultiStepMetricsContext("DeleteBucketACL"); + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "DeleteBucketACL"); Throwable error = null; try { Opts opts = Opts.unwrap(options).prepend(defaultOpts); @@ -1173,7 +1206,9 @@ public Acl updateAcl(String bucket, Acl acl, BucketSourceOption... options) { private Acl internalUpdateAcl( String operationName, String bucket, Acl acl, BucketSourceOption... options) { - StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext(operationName); + StorageMetricsContext multiContext = + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), operationName); Throwable error = null; try { Opts opts = Opts.unwrap(options).prepend(defaultOpts); @@ -1221,7 +1256,9 @@ public Acl updateAcl(String bucket, Acl acl) { @Override public List listAcls(String bucket, BucketSourceOption... options) { - StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListBucketACLs"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "ListBucketACLs"); try { Opts opts = Opts.unwrap(options).prepend(defaultOpts); com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts, metricsContext); @@ -1240,7 +1277,9 @@ public List listAcls(String bucket) { @Override public Acl getDefaultAcl(String bucket, Entity entity) { - StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetDefaultObjectACL"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "GetDefaultObjectACL"); try { com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, metricsContext); @@ -1266,7 +1305,8 @@ public Acl getDefaultAcl(String bucket, Entity entity) { @Override public boolean deleteDefaultAcl(String bucket, Entity entity) { StorageMetricsContext multiContext = - getOptions().createMultiStepMetricsContext("DeleteDefaultObjectACL"); + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "DeleteDefaultObjectACL"); Throwable error = null; try { com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, multiContext); @@ -1324,7 +1364,9 @@ public Acl updateDefaultAcl(String bucket, Acl acl) { } private Acl internalUpdateDefaultAcl(String operationName, String bucket, Acl acl) { - StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext(operationName); + StorageMetricsContext multiContext = + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), operationName); Throwable error = null; try { com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, multiContext); @@ -1368,7 +1410,8 @@ private Acl internalUpdateDefaultAcl(String operationName, String bucket, Acl ac @Override public List listDefaultAcls(String bucket) { StorageMetricsContext metricsContext = - getOptions().createMetricsContext("ListDefaultObjectACLs"); + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "ListDefaultObjectACLs"); try { com.google.storage.v2.Bucket resp = getBucketWithDefaultAcls(bucket, metricsContext); return resp.getDefaultObjectAclList().stream() @@ -1381,7 +1424,8 @@ public List listDefaultAcls(String bucket) { @Override public Acl getAcl(BlobId blob, Entity entity) { - StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetObjectACL"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "GetObjectACL"); try { Object req = codecs.blobId().encode(blob); Object resp = getObjectWithAcls(req, metricsContext); @@ -1408,7 +1452,8 @@ public Acl getAcl(BlobId blob, Entity entity) { @Override public boolean deleteAcl(BlobId blob, Entity entity) { StorageMetricsContext multiContext = - getOptions().createMultiStepMetricsContext("DeleteObjectACL"); + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "DeleteObjectACL"); Throwable error = null; try { Object obj = codecs.blobId().encode(blob); @@ -1466,7 +1511,9 @@ public Acl updateAcl(BlobId blob, Acl acl) { } private Acl internalUpdateObjectAcl(String operationName, BlobId blob, Acl acl) { - StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext(operationName); + StorageMetricsContext multiContext = + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), operationName); Throwable error = null; try { Object obj = codecs.blobId().encode(blob); @@ -1509,7 +1556,9 @@ private Acl internalUpdateObjectAcl(String operationName, BlobId blob, Acl acl) @Override public List listAcls(BlobId blob) { - StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListObjectACLs"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "ListObjectACLs"); try { Object req = codecs.blobId().encode(blob); Object resp = getObjectWithAcls(req, metricsContext); @@ -1555,7 +1604,8 @@ public Policy getIamPolicy(String bucket, BucketSourceOption... options) { GetIamPolicyRequest.Builder builder = GetIamPolicyRequest.newBuilder().setResource(bucketNameCodec.encode(bucket)); GetIamPolicyRequest req = opts.getIamPolicyRequest().apply(builder).build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetIamPolicy"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "GetIamPolicy"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -1574,7 +1624,8 @@ public Policy setIamPolicy(String bucket, Policy policy, BucketSourceOption... o .setResource(bucketNameCodec.encode(bucket)) .setPolicy(codecs.policyCodec().encode(policy)) .build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("SetIamPolicy"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "SetIamPolicy"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -1594,7 +1645,9 @@ public List testIamPermissions( .setResource(bucketNameCodec.encode(bucket)) .addAllPermissions(permissions) .build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("TestIamPermissions"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "TestIamPermissions"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -1671,7 +1724,8 @@ public Blob moveBlob(MoveBlobRequest request) { MoveObjectRequest req = b.build(); GrpcCallContext grpcCallContext = dstOpts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("MoveObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "MoveObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -1801,7 +1855,9 @@ public Page getNextPage() { ListObjectsRequest nextPageReq = req.toBuilder().setPageToken(resp.getNextPageToken()).build(); try { - StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListObjects"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "ListObjects"); GrpcCallContext merge = Utils.merge(ctx, Retrying.newCallContext(metricsContext)); ListObjectsResponse nextPageResp = retrier.run( @@ -2179,7 +2235,8 @@ private SourceObject sourceObjectEncode(SourceBlob from) { private com.google.storage.v2.ListBucketsResponse listBuckets( GrpcCallContext grpcCallContext, ListBucketsRequest request) { - StorageMetricsContext metricsContext = getOptions().createMetricsContext("ListBuckets"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "ListBuckets"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(request), @@ -2205,7 +2262,10 @@ private com.google.storage.v2.Bucket getBucketWithDefaultAcls( .build(); StorageMetricsContext activeContext = - metricsContext != null ? metricsContext : getOptions().createMetricsContext("GetBucket"); + metricsContext != null + ? metricsContext + : StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "GetBucket"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(activeContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -2229,7 +2289,10 @@ private com.google.storage.v2.Bucket getBucketWithAcls( .build(); StorageMetricsContext activeContext = - metricsContext != null ? metricsContext : getOptions().createMetricsContext("GetBucket"); + metricsContext != null + ? metricsContext + : StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "GetBucket"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(activeContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -2242,7 +2305,10 @@ private com.google.storage.v2.Bucket updateBucket( UpdateBucketRequest req, @Nullable StorageMetricsContext metricsContext) { GrpcCallContext grpcCallContext = GrpcCallContext.createDefault(); StorageMetricsContext activeContext = - metricsContext != null ? metricsContext : getOptions().createMetricsContext("UpdateBucket"); + metricsContext != null + ? metricsContext + : StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "UpdateBucket"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(activeContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -2298,7 +2364,10 @@ private Object getObjectWithAcls(Object obj, @Nullable StorageMetricsContext met .build(); StorageMetricsContext activeContext = - metricsContext != null ? metricsContext : getOptions().createMetricsContext("GetObject"); + metricsContext != null + ? metricsContext + : StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "GetObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(activeContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -2329,7 +2398,10 @@ private Object updateObject( UpdateObjectRequest req, @Nullable StorageMetricsContext metricsContext) { GrpcCallContext grpcCallContext = GrpcCallContext.createDefault(); StorageMetricsContext activeContext = - metricsContext != null ? metricsContext : getOptions().createMetricsContext("UpdateObject"); + metricsContext != null + ? metricsContext + : StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "UpdateObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(activeContext)); return retrier.run( retryAlgorithmManager.getFor(req), @@ -2350,7 +2422,8 @@ public BlobInfo internalObjectGet(BlobId blobId, Opts opts) { .setObject(blobId.getName()); ifNonNull(blobId.getGeneration(), builder::setGeneration); GetObjectRequest req = finalOpts.getObjectsRequest().apply(builder).build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetObject"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "GetObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); //noinspection DataFlowIssue return retrier.run( @@ -2387,7 +2460,8 @@ private Bucket internalBucketGet(String bucket, Opts unwrap) { GetBucketRequest.Builder builder = GetBucketRequest.newBuilder().setName(bucketNameCodec.encode(bucket)); GetBucketRequest req = opts.getBucketsRequest().apply(builder).build(); - StorageMetricsContext metricsContext = getOptions().createMetricsContext("GetBucket"); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "GetBucket"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), From 36fea56c7b2db4458a8c173af5243ac8af7f6525 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 29 Oct 2026 10:29:14 +0000 Subject: [PATCH 5/9] fix(storage): address review feedback on gRPC veneer operation metrics [Generated-by: AI] --- .../google/cloud/storage/GapicCopyWriter.java | 24 +- .../cloud/storage/GrpcBlobReadChannel.java | 2 +- .../cloud/storage/GrpcBlobWriteChannel.java | 2 +- .../google/cloud/storage/GrpcStorageImpl.java | 65 ++-- .../cloud/storage/GapicCopyWriterTest.java | 212 +++++++++++++ .../cloud/storage/GrpcStorageImplTest.java | 295 ++++++++++++++++++ 6 files changed, 563 insertions(+), 37 deletions(-) create mode 100644 java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java create mode 100644 java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java index c1be891367ee..7fbd90a5370f 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GapicCopyWriter.java @@ -23,8 +23,10 @@ import com.google.cloud.storage.Retrying.RetrierWithAlg; import com.google.storage.v2.RewriteObjectRequest; import com.google.storage.v2.RewriteResponse; -import org.checkerframework.checker.nullness.qual.Nullable; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; +@NullMarked final class GapicCopyWriter extends CopyWriter { // needed for #getResult @@ -36,15 +38,7 @@ final class GapicCopyWriter extends CopyWriter { @Nullable private final StorageMetricsContext metricsContext; private RewriteResponse mostRecentResponse; - - GapicCopyWriter( - GrpcStorageImpl storage, - UnaryCallable callable, - RetrierWithAlg retrier, - RewriteObjectRequest originalRequest, - RewriteResponse initialResponse) { - this(storage, callable, retrier, originalRequest, initialResponse, null); - } + private boolean operationRecorded; GapicCopyWriter( GrpcStorageImpl storage, @@ -60,11 +54,13 @@ final class GapicCopyWriter extends CopyWriter { this.mostRecentResponse = initialResponse; this.originalRequest = originalRequest; this.metricsContext = metricsContext; + this.operationRecorded = initialResponse.getDone(); } private void recordOperation(@Nullable Throwable error) { - if (metricsContext != null) { + if (metricsContext != null && !operationRecorded) { metricsContext.recordOperation(storage.getOptions().getStorageClientMetrics(), error); + operationRecorded = true; } } @@ -106,11 +102,7 @@ public void copyChunk() { originalRequest.toBuilder() .setRewriteToken(mostRecentResponse.getRewriteToken()) .build(); - StorageMetricsContext chunkContext = - metricsContext != null - ? metricsContext - : StorageClientMetrics.createContext( - storage.getOptions().getStorageClientMetrics(), "RewriteObject"); + StorageMetricsContext chunkContext = metricsContext; GrpcCallContext retryContext = Retrying.newCallContext(chunkContext); RewriteResponse resp = retrier.run(() -> callable.call(req, retryContext), Decoder.identity(), chunkContext); diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java index 927ac7c65e04..98a0a6faca72 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobReadChannel.java @@ -26,7 +26,7 @@ import com.google.storage.v2.ReadObjectRequest; import com.google.storage.v2.ReadObjectResponse; import org.checkerframework.checker.nullness.qual.NonNull; -import org.checkerframework.checker.nullness.qual.Nullable; +import org.jspecify.annotations.Nullable; final class GrpcBlobReadChannel extends BaseStorageReadChannel { diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobWriteChannel.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobWriteChannel.java index 23c690ff7841..d8fa127b5fd4 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobWriteChannel.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcBlobWriteChannel.java @@ -24,7 +24,7 @@ import com.google.storage.v2.WriteObjectRequest; import com.google.storage.v2.WriteObjectResponse; import java.util.function.Supplier; -import org.checkerframework.checker.nullness.qual.Nullable; +import org.jspecify.annotations.Nullable; final class GrpcBlobWriteChannel extends BaseStorageWriteChannel { diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index 6f80e4125189..0f1e4c1fcf7b 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java @@ -143,8 +143,7 @@ import java.util.function.UnaryOperator; import java.util.stream.Stream; import java.util.stream.StreamSupport; -import org.checkerframework.checker.nullness.qual.NonNull; -import org.checkerframework.checker.nullness.qual.Nullable; +import org.jspecify.annotations.Nullable; final class GrpcStorageImpl extends BaseService implements Storage, StorageInternal { @@ -655,18 +654,31 @@ public boolean delete(String bucket, String blob, BlobSourceOption... options) { @Override public boolean delete(BlobId blob, BlobSourceOption... options) { - Opts opts = Opts.unwrap(options); - try { - internalObjectDelete(blob, opts); - return true; - } catch (NotFoundException e) { - return false; - } catch (StorageException e) { - if (e.getCode() == 404) { - return false; - } - throw e; - } + Opts opts = Opts.unwrap(options).resolveFrom(blob).prepend(defaultOpts); + GrpcCallContext grpcCallContext = + opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); + DeleteObjectRequest.Builder builder = + DeleteObjectRequest.newBuilder() + .setBucket(bucketNameCodec.encode(blob.getBucket())) + .setObject(blob.getName()); + ifNonNull(blob.getGeneration(), builder::setGeneration); + DeleteObjectRequest req = opts.deleteObjectsRequest().apply(builder).build(); + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "DeleteObject"); + GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); + return Boolean.TRUE.equals( + retrier.run( + retryAlgorithmManager.getFor(req), + () -> { + try { + storageClient.deleteObjectCallable().call(req, merge); + return true; + } catch (NotFoundException e) { + return false; + } + }, + Decoder.identity(), + metricsContext)); } @Override @@ -691,7 +703,11 @@ public Void internalObjectDelete(BlobId id, Opts opts) { return retrier.run( retryAlgorithmManager.getFor(req), () -> { - storageClient.deleteObjectCallable().call(req, merge); + try { + storageClient.deleteObjectCallable().call(req, merge); + } catch (NotFoundException e) { + // Non-existent object is treated as non-failure matching delete(Bucket) + } return null; }, Decoder.identity(), @@ -784,7 +800,8 @@ public CopyWriter copy(CopyRequest copyRequest) { : StorageClientMetrics.createContext( getOptions().getStorageClientMetrics(), "RewriteObject"); GrpcCallContext retryContext = Retrying.newCallContext(initialContext); - RetrierWithAlg retrierWithAlg = retrier.withAlg(retryAlgorithmManager.idempotent()); + RetrierWithAlg retrierWithAlg = + retrier.withAlg(retryAlgorithmManager.idempotent(), multiContext); try { return retrier.run( retryAlgorithmManager.getFor(req), @@ -2410,7 +2427,7 @@ private Object updateObject( activeContext); } - @NonNull + @Nullable @Override public BlobInfo internalObjectGet(BlobId blobId, Opts opts) { Opts finalOpts = opts.prepend(defaultOpts).prepend(ALL_BLOB_FIELDS); @@ -2428,8 +2445,17 @@ public BlobInfo internalObjectGet(BlobId blobId, Opts opts) { //noinspection DataFlowIssue return retrier.run( retryAlgorithmManager.getFor(req), - () -> storageClient.getObjectCallable().call(req, merge), + () -> { + try { + return storageClient.getObjectCallable().call(req, merge); + } catch (NotFoundException e) { + return null; + } + }, resp -> { + if (resp == null) { + return null; + } BlobInfo tmp = codecs.blobInfo().decode(resp); return finalOpts.clearBlobFields().decode(tmp); }, @@ -2440,7 +2466,8 @@ public BlobInfo internalObjectGet(BlobId blobId, Opts opts) { private Blob internalBlobGet(BlobId blob, Opts unwrap) { Opts opts = unwrap.resolveFrom(blob); try { - return internalObjectGet(blob, opts).asBlob(this); + BlobInfo blobInfo = internalObjectGet(blob, opts); + return blobInfo != null ? blobInfo.asBlob(this) : null; } catch (StorageException e) { if (e.getCause() instanceof NotFoundException) { return null; diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java new file mode 100644 index 000000000000..d447d2823c02 --- /dev/null +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java @@ -0,0 +1,212 @@ +/* + * 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.METRIC_GCP_STORAGE_CLIENT_OPERATIONS; +import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_OK; +import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_UNAVAILABLE; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.rpc.UnaryCallable; +import com.google.cloud.NoCredentials; +import com.google.cloud.storage.Retrying.Retrier; +import com.google.cloud.storage.Retrying.RetrierWithAlg; +import com.google.cloud.storage.UnifiedOpts.Opts; +import com.google.storage.v2.Object; +import com.google.storage.v2.RewriteObjectRequest; +import com.google.storage.v2.RewriteResponse; +import com.google.storage.v2.StorageClient; +import com.google.storage.v2.stub.StorageStub; +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 org.junit.Before; +import org.junit.Test; + +public final class GapicCopyWriterTest { + + private static final String BUCKET = "test-bucket"; + private static final String OBJECT = "test-object"; + private static final RewriteObjectRequest REQUEST = + RewriteObjectRequest.newBuilder() + .setSourceBucket("projects/_/buckets/" + BUCKET) + .setSourceObject(OBJECT) + .setDestinationBucket("projects/_/buckets/" + BUCKET) + .setDestinationName("test-dest") + .build(); + private static final Object RESOURCE = + Object.newBuilder() + .setBucket("projects/_/buckets/" + BUCKET) + .setName("test-dest") + .setGeneration(1L) + .build(); + private static final RewriteResponse NOT_DONE_RESPONSE = + RewriteResponse.newBuilder() + .setDone(false) + .setObjectSize(100L) + .setTotalBytesRewritten(50L) + .setRewriteToken("token-1") + .build(); + private static final RewriteResponse DONE_RESPONSE = + RewriteResponse.newBuilder() + .setDone(true) + .setObjectSize(100L) + .setTotalBytesRewritten(100L) + .setResource(RESOURCE) + .build(); + + private InMemoryMetricReader reader; + private StorageClientMetrics metrics; + private GrpcStorageImpl storage; + private UnaryCallable callable; + private RetrierWithAlg retrier; + + @Before + @SuppressWarnings("unchecked") + public void setUp() { + reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + GrpcStorageOptions options = + GrpcStorageOptions.newBuilder() + .setMeterProvider(provider) + .setEnableOtelMetrics(true) + .setProjectId("test-project") + .setCredentials(NoCredentials.getInstance()) + .build(); + metrics = options.getStorageClientMetrics(); + + StorageStub storageStub = mock(StorageStub.class); + StorageClient storageClient = StorageClient.create(storageStub); + storage = + new GrpcStorageImpl( + options, + storageClient, + null, + ResponseContentLifecycleManager.noop(), + null, + options.createRetrier(), + Opts.empty()); + + callable = mock(UnaryCallable.class); + retrier = Retrier.attemptOnce().withAlg(Retrying.neverRetry()); + } + + @Test + public void copyChunk_doesNotDoubleRecordOperationCompletionWhenDone() { + StorageMetricsContext metricsContext = + StorageClientMetrics.createMultiStepContext(metrics, "RewriteObject"); + when(callable.call(any(RewriteObjectRequest.class), any(GrpcCallContext.class))) + .thenReturn(DONE_RESPONSE); + + GapicCopyWriter writer = + new GapicCopyWriter(storage, callable, retrier, REQUEST, NOT_DONE_RESPONSE, metricsContext); + + writer.copyChunk(); + assertThat(writer.isDone()).isTrue(); + assertOperationCount(STATUS_OK, 1L); + + // Subsequent copyChunk should be a no-op and not record another operation + writer.copyChunk(); + assertOperationCount(STATUS_OK, 1L); + + // getResult should return blob and not record another operation + Blob blob = writer.getResult(); + assertThat(blob).isNotNull(); + assertOperationCount(STATUS_OK, 1L); + } + + @Test + public void getResult_doesNotDoubleRecordWhenInitialResponseIsAlreadyDone() { + StorageMetricsContext metricsContext = + StorageClientMetrics.createMultiStepContext(metrics, "RewriteObject"); + + GapicCopyWriter writer = + new GapicCopyWriter(storage, callable, retrier, REQUEST, DONE_RESPONSE, metricsContext); + + assertThat(writer.isDone()).isTrue(); + Blob blob = writer.getResult(); + assertThat(blob).isNotNull(); + assertNoOperationsRecorded(); + } + + @Test + public void whenMetricsContextIsNull_internalChunksDoNotEmitTopLevelOperationMetrics() { + when(callable.call(any(RewriteObjectRequest.class), any(GrpcCallContext.class))) + .thenReturn(DONE_RESPONSE); + + GapicCopyWriter writer = + new GapicCopyWriter(storage, callable, retrier, REQUEST, NOT_DONE_RESPONSE, null); + + writer.copyChunk(); + assertThat(writer.isDone()).isTrue(); + Blob blob = writer.getResult(); + assertThat(blob).isNotNull(); + + assertNoOperationsRecorded(); + } + + @Test + public void errorsInCopyChunk_recordFailureOnMetricsContext() { + StorageMetricsContext metricsContext = + StorageClientMetrics.createContext(metrics, "RewriteObject"); + when(callable.call(any(RewriteObjectRequest.class), any(GrpcCallContext.class))) + .thenThrow(new StorageException(503, "Service Unavailable")); + + GapicCopyWriter writer = + new GapicCopyWriter(storage, callable, retrier, REQUEST, NOT_DONE_RESPONSE, metricsContext); + + assertThrows(StorageException.class, writer::copyChunk); + assertOperationCount(STATUS_UNAVAILABLE, 1L); + } + + private void assertOperationCount(String errorType, long expectedCount) { + Collection collected = reader.collectAllMetrics(); + long count = 0; + for (MetricData md : collected) { + if (md.getName().equals(METRIC_GCP_STORAGE_CLIENT_OPERATIONS)) { + for (LongPointData point : md.getLongSumData().getPoints()) { + String val = point.getAttributes().get(KEY_ERROR_TYPE); + if (errorType.equals(val)) { + count += point.getValue(); + } + } + } + } + assertThat(count).isEqualTo(expectedCount); + } + + private void assertNoOperationsRecorded() { + Collection collected = reader.collectAllMetrics(); + long count = 0; + for (MetricData md : collected) { + if (md.getName().equals(METRIC_GCP_STORAGE_CLIENT_OPERATIONS)) { + for (LongPointData point : md.getLongSumData().getPoints()) { + count += point.getValue(); + } + } + } + assertThat(count).isEqualTo(0L); + } +} diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java new file mode 100644 index 000000000000..71c8bed015af --- /dev/null +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java @@ -0,0 +1,295 @@ +/* + * 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_STORAGE_OPERATION; +import static com.google.cloud.storage.StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS; +import static com.google.cloud.storage.StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_OPERATIONS; +import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_NOT_FOUND; +import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_OK; +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.grpc.GrpcStatusCode; +import com.google.api.gax.rpc.NotFoundException; +import com.google.api.gax.rpc.UnaryCallable; +import com.google.cloud.NoCredentials; +import com.google.cloud.storage.Retrying.Retrier; +import com.google.cloud.storage.Storage.CopyRequest; +import com.google.cloud.storage.Storage.MoveBlobRequest; +import com.google.cloud.storage.UnifiedOpts.Opts; +import com.google.protobuf.Empty; +import com.google.storage.v2.Bucket; +import com.google.storage.v2.CreateBucketRequest; +import com.google.storage.v2.DeleteObjectRequest; +import com.google.storage.v2.GetObjectRequest; +import com.google.storage.v2.MoveObjectRequest; +import com.google.storage.v2.Object; +import com.google.storage.v2.RestoreObjectRequest; +import com.google.storage.v2.RewriteObjectRequest; +import com.google.storage.v2.RewriteResponse; +import com.google.storage.v2.StorageClient; +import com.google.storage.v2.UpdateObjectRequest; +import com.google.storage.v2.stub.StorageStub; +import io.grpc.Status; +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 org.junit.Before; +import org.junit.Test; + +public final class GrpcStorageImplTest { + + private static final String BUCKET_NAME = "test-bucket"; + private static final String BLOB_NAME = "test-blob"; + + private InMemoryMetricReader reader; + private GrpcStorageOptions options; + private StorageStub storageStub; + private GrpcStorageImpl grpcStorage; + + @Before + public void setUp() { + reader = InMemoryMetricReader.create(); + SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + options = + GrpcStorageOptions.newBuilder() + .setMeterProvider(provider) + .setEnableOtelMetrics(true) + .setProjectId("test-project") + .setCredentials(NoCredentials.getInstance()) + .build(); + + storageStub = mock(StorageStub.class); + StorageClient storageClient = StorageClient.create(storageStub); + Retrier retrier = options.createRetrier(); + + grpcStorage = + new GrpcStorageImpl( + options, + storageClient, + null, + ResponseContentLifecycleManager.noop(), + null, + retrier, + Opts.empty()); + } + + @Test + @SuppressWarnings("unchecked") + public void unaryCalls_recordOperationMetrics() { + // 1. Create (Bucket) + UnaryCallable createBucketCallable = mock(UnaryCallable.class); + when(storageStub.createBucketCallable()).thenReturn(createBucketCallable); + when(createBucketCallable.call(any(CreateBucketRequest.class), any(GrpcCallContext.class))) + .thenReturn(Bucket.newBuilder().setName("projects/_/buckets/" + BUCKET_NAME).build()); + + com.google.cloud.storage.Bucket createdBucket = grpcStorage.create(BucketInfo.of(BUCKET_NAME)); + assertThat(createdBucket).isNotNull(); + assertOperationCount("CreateBucket", STATUS_OK, 1L); + + // 2. Get (Blob) + UnaryCallable getObjectCallable = mock(UnaryCallable.class); + when(storageStub.getObjectCallable()).thenReturn(getObjectCallable); + when(getObjectCallable.call(any(GetObjectRequest.class), any(GrpcCallContext.class))) + .thenReturn( + Object.newBuilder() + .setBucket("projects/_/buckets/" + BUCKET_NAME) + .setName(BLOB_NAME) + .build()); + + Blob retrievedBlob = grpcStorage.get(BlobId.of(BUCKET_NAME, BLOB_NAME)); + assertThat(retrievedBlob).isNotNull(); + assertOperationCount("GetObject", STATUS_OK, 1L); + + // 3. Delete (Blob) + UnaryCallable deleteObjectCallable = mock(UnaryCallable.class); + when(storageStub.deleteObjectCallable()).thenReturn(deleteObjectCallable); + when(deleteObjectCallable.call(any(DeleteObjectRequest.class), any(GrpcCallContext.class))) + .thenReturn(Empty.getDefaultInstance()); + + boolean deleted = grpcStorage.delete(BlobId.of(BUCKET_NAME, BLOB_NAME)); + assertThat(deleted).isTrue(); + assertOperationCount("DeleteObject", STATUS_OK, 1L); + + // 4. Update (Blob) + UnaryCallable updateObjectCallable = mock(UnaryCallable.class); + when(storageStub.updateObjectCallable()).thenReturn(updateObjectCallable); + when(updateObjectCallable.call(any(UpdateObjectRequest.class), any(GrpcCallContext.class))) + .thenReturn( + Object.newBuilder() + .setBucket("projects/_/buckets/" + BUCKET_NAME) + .setName(BLOB_NAME) + .setContentType("text/plain") + .build()); + + Blob updatedBlob = + grpcStorage.update( + BlobInfo.newBuilder(BUCKET_NAME, BLOB_NAME).setContentType("text/plain").build()); + assertThat(updatedBlob).isNotNull(); + assertOperationCount("UpdateObject", STATUS_OK, 1L); + + // 5. Restore (Blob) + UnaryCallable restoreObjectCallable = mock(UnaryCallable.class); + when(storageStub.restoreObjectCallable()).thenReturn(restoreObjectCallable); + when(restoreObjectCallable.call(any(RestoreObjectRequest.class), any(GrpcCallContext.class))) + .thenReturn( + Object.newBuilder() + .setBucket("projects/_/buckets/" + BUCKET_NAME) + .setName(BLOB_NAME) + .setGeneration(1L) + .build()); + + Blob restoredBlob = grpcStorage.restore(BlobId.of(BUCKET_NAME, BLOB_NAME, 1L)); + assertThat(restoredBlob).isNotNull(); + assertOperationCount("RestoreObject", STATUS_OK, 1L); + + // 6. MoveBlob + UnaryCallable moveObjectCallable = mock(UnaryCallable.class); + when(storageStub.moveObjectCallable()).thenReturn(moveObjectCallable); + when(moveObjectCallable.call(any(MoveObjectRequest.class), any(GrpcCallContext.class))) + .thenReturn( + Object.newBuilder() + .setBucket("projects/_/buckets/" + BUCKET_NAME) + .setName("dst-blob") + .build()); + + Blob movedBlob = + grpcStorage.moveBlob( + MoveBlobRequest.newBuilder() + .setSource(BlobId.of(BUCKET_NAME, BLOB_NAME)) + .setTarget(BlobId.of(BUCKET_NAME, "dst-blob")) + .build()); + assertThat(movedBlob).isNotNull(); + assertOperationCount("MoveObject", STATUS_OK, 1L); + } + + @Test + @SuppressWarnings("unchecked") + public void deleteBlob_onNonExistentObject_returnsFalseWithErrorTypeOk() { + UnaryCallable deleteObjectCallable = mock(UnaryCallable.class); + when(storageStub.deleteObjectCallable()).thenReturn(deleteObjectCallable); + when(deleteObjectCallable.call(any(DeleteObjectRequest.class), any(GrpcCallContext.class))) + .thenThrow( + new NotFoundException( + "Not Found", null, GrpcStatusCode.of(Status.Code.NOT_FOUND), false)); + + boolean deleted = grpcStorage.delete(BlobId.of(BUCKET_NAME, "non-existent")); + assertThat(deleted).isFalse(); + + // Verify operations metric records STATUS_OK and not NOT_FOUND + assertOperationCount("DeleteObject", STATUS_OK, 1L); + assertOperationCount("DeleteObject", STATUS_NOT_FOUND, 0L); + } + + @Test + @SuppressWarnings("unchecked") + public void copy_propagatesMultiContextToRetrierWithAlg() { + UnaryCallable rewriteObjectCallable = + mock(UnaryCallable.class); + when(storageStub.rewriteObjectCallable()).thenReturn(rewriteObjectCallable); + when(rewriteObjectCallable.withDefaultCallContext(any(GrpcCallContext.class))) + .thenReturn(rewriteObjectCallable); + + RewriteResponse initialResp = + RewriteResponse.newBuilder() + .setDone(false) + .setRewriteToken("token-step-1") + .setTotalBytesRewritten(50L) + .setObjectSize(100L) + .build(); + RewriteResponse doneResp = + RewriteResponse.newBuilder() + .setDone(true) + .setTotalBytesRewritten(100L) + .setObjectSize(100L) + .setResource( + Object.newBuilder() + .setBucket("projects/_/buckets/" + BUCKET_NAME) + .setName("dst-blob") + .build()) + .build(); + + when(rewriteObjectCallable.call(any(RewriteObjectRequest.class), any(GrpcCallContext.class))) + .thenReturn(initialResp, doneResp); + + CopyWriter writer = + grpcStorage.copy( + CopyRequest.newBuilder() + .setSource(BUCKET_NAME, BLOB_NAME) + .setTarget(BlobId.of(BUCKET_NAME, "dst-blob")) + .build()); + + // Initial step returned done=false, so top-level operation is not recorded yet + assertThat(writer.isDone()).isFalse(); + assertOperationCount("RewriteObject", STATUS_OK, 0L); + + // Copying chunk completes rewrite using bound multiContext + writer.copyChunk(); + assertThat(writer.isDone()).isTrue(); + + // Multi-step operation is completed once + assertOperationCount("RewriteObject", STATUS_OK, 1L); + + // Calling getResult returns blob without double-recording + Blob result = writer.getResult(); + assertThat(result).isNotNull(); + assertOperationCount("RewriteObject", STATUS_OK, 1L); + + // Verify 2 attempts were recorded under RewriteObject + assertAttemptCount("RewriteObject", 2L); + } + + private void assertOperationCount(String operation, String errorType, long expectedCount) { + Collection collected = reader.collectAllMetrics(); + long count = 0; + for (MetricData md : collected) { + if (md.getName().equals(METRIC_GCP_STORAGE_CLIENT_OPERATIONS)) { + for (LongPointData point : md.getLongSumData().getPoints()) { + String op = point.getAttributes().get(KEY_GCP_STORAGE_OPERATION); + String err = point.getAttributes().get(KEY_ERROR_TYPE); + if (operation.equals(op) && errorType.equals(err)) { + count += point.getValue(); + } + } + } + } + assertThat(count).isEqualTo(expectedCount); + } + + private void assertAttemptCount(String operation, long expectedCount) { + Collection collected = reader.collectAllMetrics(); + long count = 0; + for (MetricData md : collected) { + if (md.getName().equals(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS)) { + for (LongPointData point : md.getLongSumData().getPoints()) { + String op = point.getAttributes().get(KEY_GCP_STORAGE_OPERATION); + if (operation.equals(op)) { + count += point.getValue(); + } + } + } + } + assertThat(count).isEqualTo(expectedCount); + } +} From 295833dd2bd32e0cab7a841c760a8e7ca67c4522 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 29 Oct 2026 16:29:13 +0000 Subject: [PATCH 6/9] fix(storage): replace StorageStub mock with static TestStorageStub for Java 8 compatibility [Generated-by: AI] --- .../cloud/storage/GapicCopyWriterTest.java | 3 +- .../cloud/storage/GrpcStorageImplTest.java | 21 ++-- .../google/cloud/storage/TestStorageStub.java | 101 ++++++++++++++++++ 3 files changed, 112 insertions(+), 13 deletions(-) create mode 100644 java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java index d447d2823c02..397319d17697 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java @@ -36,7 +36,6 @@ import com.google.storage.v2.RewriteObjectRequest; import com.google.storage.v2.RewriteResponse; import com.google.storage.v2.StorageClient; -import com.google.storage.v2.stub.StorageStub; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.data.LongPointData; import io.opentelemetry.sdk.metrics.data.MetricData; @@ -97,7 +96,7 @@ public void setUp() { .build(); metrics = options.getStorageClientMetrics(); - StorageStub storageStub = mock(StorageStub.class); + TestStorageStub storageStub = new TestStorageStub(); StorageClient storageClient = StorageClient.create(storageStub); storage = new GrpcStorageImpl( diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java index 71c8bed015af..0830430d5cfb 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java @@ -48,7 +48,6 @@ import com.google.storage.v2.RewriteResponse; import com.google.storage.v2.StorageClient; import com.google.storage.v2.UpdateObjectRequest; -import com.google.storage.v2.stub.StorageStub; import io.grpc.Status; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.data.LongPointData; @@ -65,7 +64,7 @@ public final class GrpcStorageImplTest { private InMemoryMetricReader reader; private GrpcStorageOptions options; - private StorageStub storageStub; + private TestStorageStub storageStub; private GrpcStorageImpl grpcStorage; @Before @@ -80,7 +79,7 @@ public void setUp() { .setCredentials(NoCredentials.getInstance()) .build(); - storageStub = mock(StorageStub.class); + storageStub = new TestStorageStub(); StorageClient storageClient = StorageClient.create(storageStub); Retrier retrier = options.createRetrier(); @@ -100,7 +99,7 @@ public void setUp() { public void unaryCalls_recordOperationMetrics() { // 1. Create (Bucket) UnaryCallable createBucketCallable = mock(UnaryCallable.class); - when(storageStub.createBucketCallable()).thenReturn(createBucketCallable); + storageStub.createBucketCallable = createBucketCallable; when(createBucketCallable.call(any(CreateBucketRequest.class), any(GrpcCallContext.class))) .thenReturn(Bucket.newBuilder().setName("projects/_/buckets/" + BUCKET_NAME).build()); @@ -110,7 +109,7 @@ public void unaryCalls_recordOperationMetrics() { // 2. Get (Blob) UnaryCallable getObjectCallable = mock(UnaryCallable.class); - when(storageStub.getObjectCallable()).thenReturn(getObjectCallable); + storageStub.getObjectCallable = getObjectCallable; when(getObjectCallable.call(any(GetObjectRequest.class), any(GrpcCallContext.class))) .thenReturn( Object.newBuilder() @@ -124,7 +123,7 @@ public void unaryCalls_recordOperationMetrics() { // 3. Delete (Blob) UnaryCallable deleteObjectCallable = mock(UnaryCallable.class); - when(storageStub.deleteObjectCallable()).thenReturn(deleteObjectCallable); + storageStub.deleteObjectCallable = deleteObjectCallable; when(deleteObjectCallable.call(any(DeleteObjectRequest.class), any(GrpcCallContext.class))) .thenReturn(Empty.getDefaultInstance()); @@ -134,7 +133,7 @@ public void unaryCalls_recordOperationMetrics() { // 4. Update (Blob) UnaryCallable updateObjectCallable = mock(UnaryCallable.class); - when(storageStub.updateObjectCallable()).thenReturn(updateObjectCallable); + storageStub.updateObjectCallable = updateObjectCallable; when(updateObjectCallable.call(any(UpdateObjectRequest.class), any(GrpcCallContext.class))) .thenReturn( Object.newBuilder() @@ -151,7 +150,7 @@ public void unaryCalls_recordOperationMetrics() { // 5. Restore (Blob) UnaryCallable restoreObjectCallable = mock(UnaryCallable.class); - when(storageStub.restoreObjectCallable()).thenReturn(restoreObjectCallable); + storageStub.restoreObjectCallable = restoreObjectCallable; when(restoreObjectCallable.call(any(RestoreObjectRequest.class), any(GrpcCallContext.class))) .thenReturn( Object.newBuilder() @@ -166,7 +165,7 @@ public void unaryCalls_recordOperationMetrics() { // 6. MoveBlob UnaryCallable moveObjectCallable = mock(UnaryCallable.class); - when(storageStub.moveObjectCallable()).thenReturn(moveObjectCallable); + storageStub.moveObjectCallable = moveObjectCallable; when(moveObjectCallable.call(any(MoveObjectRequest.class), any(GrpcCallContext.class))) .thenReturn( Object.newBuilder() @@ -188,7 +187,7 @@ public void unaryCalls_recordOperationMetrics() { @SuppressWarnings("unchecked") public void deleteBlob_onNonExistentObject_returnsFalseWithErrorTypeOk() { UnaryCallable deleteObjectCallable = mock(UnaryCallable.class); - when(storageStub.deleteObjectCallable()).thenReturn(deleteObjectCallable); + storageStub.deleteObjectCallable = deleteObjectCallable; when(deleteObjectCallable.call(any(DeleteObjectRequest.class), any(GrpcCallContext.class))) .thenThrow( new NotFoundException( @@ -207,7 +206,7 @@ public void deleteBlob_onNonExistentObject_returnsFalseWithErrorTypeOk() { public void copy_propagatesMultiContextToRetrierWithAlg() { UnaryCallable rewriteObjectCallable = mock(UnaryCallable.class); - when(storageStub.rewriteObjectCallable()).thenReturn(rewriteObjectCallable); + storageStub.rewriteObjectCallable = rewriteObjectCallable; when(rewriteObjectCallable.withDefaultCallContext(any(GrpcCallContext.class))) .thenReturn(rewriteObjectCallable); diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java new file mode 100644 index 000000000000..008702081b35 --- /dev/null +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java @@ -0,0 +1,101 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage; + +import com.google.api.gax.rpc.UnaryCallable; +import com.google.protobuf.Empty; +import com.google.storage.v2.Bucket; +import com.google.storage.v2.CreateBucketRequest; +import com.google.storage.v2.DeleteObjectRequest; +import com.google.storage.v2.GetObjectRequest; +import com.google.storage.v2.MoveObjectRequest; +import com.google.storage.v2.Object; +import com.google.storage.v2.RestoreObjectRequest; +import com.google.storage.v2.RewriteObjectRequest; +import com.google.storage.v2.RewriteResponse; +import com.google.storage.v2.UpdateObjectRequest; +import com.google.storage.v2.stub.StorageStub; +import java.util.concurrent.TimeUnit; + +class TestStorageStub extends StorageStub { + UnaryCallable createBucketCallable; + UnaryCallable getObjectCallable; + UnaryCallable deleteObjectCallable; + UnaryCallable updateObjectCallable; + UnaryCallable restoreObjectCallable; + UnaryCallable moveObjectCallable; + UnaryCallable rewriteObjectCallable; + + @Override + public UnaryCallable createBucketCallable() { + return createBucketCallable; + } + + @Override + public UnaryCallable getObjectCallable() { + return getObjectCallable; + } + + @Override + public UnaryCallable deleteObjectCallable() { + return deleteObjectCallable; + } + + @Override + public UnaryCallable updateObjectCallable() { + return updateObjectCallable; + } + + @Override + public UnaryCallable restoreObjectCallable() { + return restoreObjectCallable; + } + + @Override + public UnaryCallable moveObjectCallable() { + return moveObjectCallable; + } + + @Override + public UnaryCallable rewriteObjectCallable() { + return rewriteObjectCallable; + } + + @Override + public void shutdown() {} + + @Override + public boolean isShutdown() { + return false; + } + + @Override + public boolean isTerminated() { + return false; + } + + @Override + public void shutdownNow() {} + + @Override + public boolean awaitTermination(long duration, TimeUnit unit) throws InterruptedException { + return false; + } + + @Override + public void close() {} +} From ac00bf40460c7b871fcc2f6182359aa3cbdabb06 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 29 Oct 2026 17:09:11 +0000 Subject: [PATCH 7/9] fix(storage): mock UnaryCallable without annotations for Java 8 compatibility Configure Mockito to mock UnaryCallable without annotations on Java 8 to prevent ArrayStoreException from EnumConstantNotPresentExceptionProxy. [Generated-by: AI] --- .../cloud/storage/GapicCopyWriterTest.java | 3 +-- .../cloud/storage/GrpcStorageImplTest.java | 20 ++++++++++--------- .../google/cloud/storage/TestStorageStub.java | 6 ++++++ 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java index 397319d17697..2d2198be7b1e 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GapicCopyWriterTest.java @@ -23,7 +23,6 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.google.api.gax.grpc.GrpcCallContext; @@ -108,7 +107,7 @@ public void setUp() { options.createRetrier(), Opts.empty()); - callable = mock(UnaryCallable.class); + callable = TestStorageStub.mockCallable(); retrier = Retrier.attemptOnce().withAlg(Retrying.neverRetry()); } diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java index 0830430d5cfb..13a2f15c6d8e 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java @@ -24,7 +24,6 @@ import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_OK; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.google.api.gax.grpc.GrpcCallContext; @@ -98,7 +97,8 @@ public void setUp() { @SuppressWarnings("unchecked") public void unaryCalls_recordOperationMetrics() { // 1. Create (Bucket) - UnaryCallable createBucketCallable = mock(UnaryCallable.class); + UnaryCallable createBucketCallable = + TestStorageStub.mockCallable(); storageStub.createBucketCallable = createBucketCallable; when(createBucketCallable.call(any(CreateBucketRequest.class), any(GrpcCallContext.class))) .thenReturn(Bucket.newBuilder().setName("projects/_/buckets/" + BUCKET_NAME).build()); @@ -108,7 +108,7 @@ public void unaryCalls_recordOperationMetrics() { assertOperationCount("CreateBucket", STATUS_OK, 1L); // 2. Get (Blob) - UnaryCallable getObjectCallable = mock(UnaryCallable.class); + UnaryCallable getObjectCallable = TestStorageStub.mockCallable(); storageStub.getObjectCallable = getObjectCallable; when(getObjectCallable.call(any(GetObjectRequest.class), any(GrpcCallContext.class))) .thenReturn( @@ -122,7 +122,7 @@ public void unaryCalls_recordOperationMetrics() { assertOperationCount("GetObject", STATUS_OK, 1L); // 3. Delete (Blob) - UnaryCallable deleteObjectCallable = mock(UnaryCallable.class); + UnaryCallable deleteObjectCallable = TestStorageStub.mockCallable(); storageStub.deleteObjectCallable = deleteObjectCallable; when(deleteObjectCallable.call(any(DeleteObjectRequest.class), any(GrpcCallContext.class))) .thenReturn(Empty.getDefaultInstance()); @@ -132,7 +132,8 @@ public void unaryCalls_recordOperationMetrics() { assertOperationCount("DeleteObject", STATUS_OK, 1L); // 4. Update (Blob) - UnaryCallable updateObjectCallable = mock(UnaryCallable.class); + UnaryCallable updateObjectCallable = + TestStorageStub.mockCallable(); storageStub.updateObjectCallable = updateObjectCallable; when(updateObjectCallable.call(any(UpdateObjectRequest.class), any(GrpcCallContext.class))) .thenReturn( @@ -149,7 +150,8 @@ public void unaryCalls_recordOperationMetrics() { assertOperationCount("UpdateObject", STATUS_OK, 1L); // 5. Restore (Blob) - UnaryCallable restoreObjectCallable = mock(UnaryCallable.class); + UnaryCallable restoreObjectCallable = + TestStorageStub.mockCallable(); storageStub.restoreObjectCallable = restoreObjectCallable; when(restoreObjectCallable.call(any(RestoreObjectRequest.class), any(GrpcCallContext.class))) .thenReturn( @@ -164,7 +166,7 @@ public void unaryCalls_recordOperationMetrics() { assertOperationCount("RestoreObject", STATUS_OK, 1L); // 6. MoveBlob - UnaryCallable moveObjectCallable = mock(UnaryCallable.class); + UnaryCallable moveObjectCallable = TestStorageStub.mockCallable(); storageStub.moveObjectCallable = moveObjectCallable; when(moveObjectCallable.call(any(MoveObjectRequest.class), any(GrpcCallContext.class))) .thenReturn( @@ -186,7 +188,7 @@ public void unaryCalls_recordOperationMetrics() { @Test @SuppressWarnings("unchecked") public void deleteBlob_onNonExistentObject_returnsFalseWithErrorTypeOk() { - UnaryCallable deleteObjectCallable = mock(UnaryCallable.class); + UnaryCallable deleteObjectCallable = TestStorageStub.mockCallable(); storageStub.deleteObjectCallable = deleteObjectCallable; when(deleteObjectCallable.call(any(DeleteObjectRequest.class), any(GrpcCallContext.class))) .thenThrow( @@ -205,7 +207,7 @@ public void deleteBlob_onNonExistentObject_returnsFalseWithErrorTypeOk() { @SuppressWarnings("unchecked") public void copy_propagatesMultiContextToRetrierWithAlg() { UnaryCallable rewriteObjectCallable = - mock(UnaryCallable.class); + TestStorageStub.mockCallable(); storageStub.rewriteObjectCallable = rewriteObjectCallable; when(rewriteObjectCallable.withDefaultCallContext(any(GrpcCallContext.class))) .thenReturn(rewriteObjectCallable); diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java index 008702081b35..4dcc91da4cc1 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java @@ -30,6 +30,7 @@ import com.google.storage.v2.UpdateObjectRequest; import com.google.storage.v2.stub.StorageStub; import java.util.concurrent.TimeUnit; +import org.mockito.Mockito; class TestStorageStub extends StorageStub { UnaryCallable createBucketCallable; @@ -40,6 +41,11 @@ class TestStorageStub extends StorageStub { UnaryCallable moveObjectCallable; UnaryCallable rewriteObjectCallable; + @SuppressWarnings("unchecked") + static UnaryCallable mockCallable() { + return Mockito.mock(UnaryCallable.class, Mockito.withSettings().withoutAnnotations()); + } + @Override public UnaryCallable createBucketCallable() { return createBucketCallable; From 12449884d24eb5b057214831751df487728abbd5 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Mon, 2 Nov 2026 10:42:18 +0000 Subject: [PATCH 8/9] fix(storage): record NOT_FOUND on 404, suppress PCU sub-op metrics, and bind list pagination retrier - Let NotFoundException propagate out of retrier.run and catch 404 outside in delete, internalBlobGet, and internalBucketGet, and set error on 404 in deleteAcl/deleteDefaultAcl so NOT_FOUND is recorded instead of OK. - Run StorageInternal hooks (internalDirectUpload, internalObjectDelete, internalObjectGet) with null metricsContext so PCU sub-operations do not emit top-level operations, and keep internalObjectGet @NonNull. - Bind TransformingPageDecorator retrier with multi-step ListBuckets context and use createMultiStepContext for page 2+ in ListBucketsWithPartialSuccessPage and ListObjectsWithSyntheticDirectoriesPage. [Generated-by: AI] --- .../google/cloud/storage/GrpcStorageImpl.java | 201 +++++++++-------- .../cloud/storage/GrpcStorageImplTest.java | 206 +++++++++++++++++- .../google/cloud/storage/TestStorageStub.java | 37 ++++ 3 files changed, 355 insertions(+), 89 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index 0f1e4c1fcf7b..6a3b97d05a9f 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java @@ -143,6 +143,7 @@ import java.util.function.UnaryOperator; import java.util.stream.Stream; import java.util.stream.StreamSupport; +import org.checkerframework.checker.nullness.qual.NonNull; import org.jspecify.annotations.Nullable; final class GrpcStorageImpl extends BaseService @@ -253,7 +254,12 @@ public Blob create(BlobInfo blobInfo, byte[] content, BlobTargetOption... option public Blob create( BlobInfo blobInfo, byte[] content, int offset, int length, BlobTargetOption... options) { Opts opts = Opts.unwrap(options).resolveFrom(blobInfo); - return internalDirectUpload(blobInfo, opts, ByteBuffer.wrap(content, offset, length)) + return internalDirectUpload( + blobInfo, + opts, + ByteBuffer.wrap(content, offset, length), + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "WriteObject")) .asBlob(this); } @@ -520,7 +526,9 @@ public Page list(BucketListOption... options) { new TransformingPageDecorator<>( resp.getPage(), syntaxDecoders.bucket.andThen(opts.clearBucketFields()), - retrier, + retrier.withMetricsContext( + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "ListBuckets")), retryAlgorithmManager.getFor(request)), metricsContext); } catch (Exception e) { @@ -528,7 +536,12 @@ public Page list(BucketListOption... options) { } } else { try { - com.google.storage.v2.ListBucketsResponse response = listBuckets(grpcCallContext, request); + com.google.storage.v2.ListBucketsResponse response = + listBuckets( + grpcCallContext, + request, + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "ListBuckets")); return new ListBucketsWithPartialSuccessPage(grpcCallContext, request, response, opts); } catch (Exception e) { throw StorageException.coalesce(e); @@ -632,19 +645,24 @@ public boolean delete(String bucket, BucketSourceOption... options) { StorageMetricsContext metricsContext = StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "DeleteBucket"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); - return Boolean.TRUE.equals( - retrier.run( - retryAlgorithmManager.getFor(req), - () -> { - try { + try { + return Boolean.TRUE.equals( + retrier.run( + retryAlgorithmManager.getFor(req), + () -> { storageClient.deleteBucketCallable().call(req, merge); return true; - } catch (NotFoundException e) { - return false; - } - }, - Decoder.identity(), - metricsContext)); + }, + Decoder.identity(), + metricsContext)); + } catch (NotFoundException e) { + return false; + } catch (StorageException e) { + if (e.getCode() == 404 || e.getCause() instanceof NotFoundException) { + return false; + } + throw e; + } } @Override @@ -654,31 +672,22 @@ public boolean delete(String bucket, String blob, BlobSourceOption... options) { @Override public boolean delete(BlobId blob, BlobSourceOption... options) { - Opts opts = Opts.unwrap(options).resolveFrom(blob).prepend(defaultOpts); - GrpcCallContext grpcCallContext = - opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); - DeleteObjectRequest.Builder builder = - DeleteObjectRequest.newBuilder() - .setBucket(bucketNameCodec.encode(blob.getBucket())) - .setObject(blob.getName()); - ifNonNull(blob.getGeneration(), builder::setGeneration); - DeleteObjectRequest req = opts.deleteObjectsRequest().apply(builder).build(); - StorageMetricsContext metricsContext = - StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "DeleteObject"); - GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); - return Boolean.TRUE.equals( - retrier.run( - retryAlgorithmManager.getFor(req), - () -> { - try { - storageClient.deleteObjectCallable().call(req, merge); - return true; - } catch (NotFoundException e) { - return false; - } - }, - Decoder.identity(), - metricsContext)); + Opts opts = Opts.unwrap(options); + try { + internalObjectDelete( + blob, + opts, + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "DeleteObject")); + return true; + } catch (NotFoundException e) { + return false; + } catch (StorageException e) { + if (e.getCode() == 404 || e.getCause() instanceof NotFoundException) { + return false; + } + throw e; + } } @Override @@ -688,6 +697,11 @@ public boolean delete(BlobId blob) { @Override public Void internalObjectDelete(BlobId id, Opts opts) { + return internalObjectDelete(id, opts, null); + } + + private Void internalObjectDelete( + BlobId id, Opts opts, @Nullable StorageMetricsContext metricsContext) { Opts finalOpts = opts.resolveFrom(id).prepend(defaultOpts); GrpcCallContext grpcCallContext = finalOpts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); @@ -697,17 +711,11 @@ public Void internalObjectDelete(BlobId id, Opts opts) { .setObject(id.getName()); ifNonNull(id.getGeneration(), builder::setGeneration); DeleteObjectRequest req = finalOpts.deleteObjectsRequest().apply(builder).build(); - StorageMetricsContext metricsContext = - StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "DeleteObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(req), () -> { - try { - storageClient.deleteObjectCallable().call(req, merge); - } catch (NotFoundException e) { - // Non-existent object is treated as non-failure matching delete(Bucket) - } + storageClient.deleteObjectCallable().call(req, merge); return null; }, Decoder.identity(), @@ -977,6 +985,14 @@ public GrpcBlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options @Override public BlobInfo internalDirectUpload( BlobInfo blobInfo, Opts opts, ByteBuffer buf) { + return internalDirectUpload(blobInfo, opts, buf, null); + } + + private BlobInfo internalDirectUpload( + BlobInfo blobInfo, + Opts opts, + ByteBuffer buf, + @Nullable StorageMetricsContext metricsContext) { requireNonNull(blobInfo, "blobInfo must be non null"); requireNonNull(buf, "content must be non null"); Opts optsWithDefaults = opts.prepend(defaultOpts); @@ -984,8 +1000,6 @@ public BlobInfo internalDirectUpload( optsWithDefaults.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); WriteObjectRequest req = getWriteObjectRequest(blobInfo, optsWithDefaults); Hasher hasher = opts.getHasher(); - StorageMetricsContext metricsContext = - StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "WriteObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); RewindableContent content = RewindableContent.of(buf); return retrier.run( @@ -1183,12 +1197,13 @@ public boolean deleteAcl(String bucket, Entity entity, BucketSourceOption... opt return !first.isPresent(); } catch (NotFoundException e) { // HttpStorageRpc returns false if the bucket doesn't exist :( + error = e; return false; } catch (StorageException se) { + error = se; if (se.getCode() == 404) { return false; } else { - error = se; throw se; } } catch (Exception e) { @@ -1352,12 +1367,13 @@ public boolean deleteDefaultAcl(String bucket, Entity entity) { return !first.isPresent(); } catch (NotFoundException e) { // HttpStorageRpc returns false if the bucket doesn't exist :( + error = e; return false; } catch (StorageException se) { + error = se; if (se.getCode() == 404) { return false; } else { - error = se; throw se; } } catch (Exception e) { @@ -1499,12 +1515,13 @@ public boolean deleteAcl(BlobId blob, Entity entity) { return !first.isPresent(); } catch (NotFoundException e) { // HttpStorageRpc returns false if the bucket doesn't exist :( + error = e; return false; } catch (StorageException se) { + error = se; if (se.getCode() == 404) { return false; } else { - error = se; throw se; } } catch (Exception e) { @@ -1873,7 +1890,7 @@ public Page getNextPage() { req.toBuilder().setPageToken(resp.getNextPageToken()).build(); try { StorageMetricsContext metricsContext = - StorageClientMetrics.createContext( + StorageClientMetrics.createMultiStepContext( getOptions().getStorageClientMetrics(), "ListObjects"); GrpcCallContext merge = Utils.merge(ctx, Retrying.newCallContext(metricsContext)); ListObjectsResponse nextPageResp = @@ -1958,7 +1975,12 @@ public Page getNextPage() { ListBucketsRequest nextPageReq = req.toBuilder().setPageToken(resp.getNextPageToken()).build(); try { - com.google.storage.v2.ListBucketsResponse nextPageResp = listBuckets(ctx, nextPageReq); + com.google.storage.v2.ListBucketsResponse nextPageResp = + listBuckets( + ctx, + nextPageReq, + StorageClientMetrics.createMultiStepContext( + getOptions().getStorageClientMetrics(), "ListBuckets")); return new ListBucketsWithPartialSuccessPage(ctx, nextPageReq, nextPageResp, opts); } catch (Exception e) { throw StorageException.coalesce(e); @@ -2028,10 +2050,17 @@ public String getNextPageToken() { return page.getNextPageToken(); } + @SuppressWarnings({"Convert2MethodRef"}) @Override public Page getNextPage() { - return new TransformingPageDecorator<>( - page.getNextPage(), translator, retrier, resultRetryAlgorithm); + if (!hasNextPage()) { + return null; + } + Callable c = () -> page.getNextPage(); + PageT nextPage = retrier.run(resultRetryAlgorithm, c, Decoder.identity()); + return nextPage == null + ? null + : new TransformingPageDecorator<>(nextPage, translator, retrier, resultRetryAlgorithm); } @SuppressWarnings({"Convert2MethodRef"}) @@ -2251,9 +2280,9 @@ private SourceObject sourceObjectEncode(SourceBlob from) { } private com.google.storage.v2.ListBucketsResponse listBuckets( - GrpcCallContext grpcCallContext, ListBucketsRequest request) { - StorageMetricsContext metricsContext = - StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "ListBuckets"); + GrpcCallContext grpcCallContext, + ListBucketsRequest request, + @Nullable StorageMetricsContext metricsContext) { GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); return retrier.run( retryAlgorithmManager.getFor(request), @@ -2427,9 +2456,15 @@ private Object updateObject( activeContext); } - @Nullable + @NonNull @Override public BlobInfo internalObjectGet(BlobId blobId, Opts opts) { + return internalObjectGet(blobId, opts, null); + } + + @NonNull + private BlobInfo internalObjectGet( + BlobId blobId, Opts opts, @Nullable StorageMetricsContext metricsContext) { Opts finalOpts = opts.prepend(defaultOpts).prepend(ALL_BLOB_FIELDS); GrpcCallContext grpcCallContext = finalOpts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); @@ -2439,23 +2474,12 @@ public BlobInfo internalObjectGet(BlobId blobId, Opts opts) { .setObject(blobId.getName()); ifNonNull(blobId.getGeneration(), builder::setGeneration); GetObjectRequest req = finalOpts.getObjectsRequest().apply(builder).build(); - StorageMetricsContext metricsContext = - StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "GetObject"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); //noinspection DataFlowIssue return retrier.run( retryAlgorithmManager.getFor(req), - () -> { - try { - return storageClient.getObjectCallable().call(req, merge); - } catch (NotFoundException e) { - return null; - } - }, + () -> storageClient.getObjectCallable().call(req, merge), resp -> { - if (resp == null) { - return null; - } BlobInfo tmp = codecs.blobInfo().decode(resp); return finalOpts.clearBlobFields().decode(tmp); }, @@ -2466,10 +2490,14 @@ public BlobInfo internalObjectGet(BlobId blobId, Opts opts) { private Blob internalBlobGet(BlobId blob, Opts unwrap) { Opts opts = unwrap.resolveFrom(blob); try { - BlobInfo blobInfo = internalObjectGet(blob, opts); - return blobInfo != null ? blobInfo.asBlob(this) : null; + return internalObjectGet( + blob, + opts, + StorageClientMetrics.createContext( + getOptions().getStorageClientMetrics(), "GetObject")) + .asBlob(this); } catch (StorageException e) { - if (e.getCause() instanceof NotFoundException) { + if (e.getCode() == 404 || e.getCause() instanceof NotFoundException) { return null; } else { throw e; @@ -2490,17 +2518,20 @@ private Bucket internalBucketGet(String bucket, Opts unwrap) { StorageMetricsContext metricsContext = StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "GetBucket"); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); - return retrier.run( - retryAlgorithmManager.getFor(req), - () -> { - try { - return storageClient.getBucketCallable().call(req, merge); - } catch (NotFoundException e) { - return null; - } - }, - syntaxDecoders.bucket.andThen(opts.clearBucketFields()), - metricsContext); + try { + return retrier.run( + retryAlgorithmManager.getFor(req), + () -> storageClient.getBucketCallable().call(req, merge), + syntaxDecoders.bucket.andThen(opts.clearBucketFields()), + metricsContext); + } catch (StorageException e) { + if (e.getCode() == 404 || e.getCause() instanceof NotFoundException) { + return null; + } + throw e; + } catch (NotFoundException e) { + return null; + } } private ZeroCopyServerStreamingCallable readObjectCallable( diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java index 13a2f15c6d8e..197917b59e7a 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java @@ -23,29 +23,42 @@ import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_NOT_FOUND; import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_OK; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcStatusCode; +import com.google.api.gax.paging.Page; import com.google.api.gax.rpc.NotFoundException; import com.google.api.gax.rpc.UnaryCallable; import com.google.cloud.NoCredentials; +import com.google.cloud.storage.Acl.User; import com.google.cloud.storage.Retrying.Retrier; +import com.google.cloud.storage.Storage.BucketListOption; import com.google.cloud.storage.Storage.CopyRequest; import com.google.cloud.storage.Storage.MoveBlobRequest; import com.google.cloud.storage.UnifiedOpts.Opts; +import com.google.common.collect.ImmutableList; import com.google.protobuf.Empty; import com.google.storage.v2.Bucket; import com.google.storage.v2.CreateBucketRequest; +import com.google.storage.v2.DeleteBucketRequest; import com.google.storage.v2.DeleteObjectRequest; +import com.google.storage.v2.GetBucketRequest; import com.google.storage.v2.GetObjectRequest; +import com.google.storage.v2.ListBucketsRequest; +import com.google.storage.v2.ListBucketsResponse; +import com.google.storage.v2.ListObjectsRequest; +import com.google.storage.v2.ListObjectsResponse; import com.google.storage.v2.MoveObjectRequest; import com.google.storage.v2.Object; import com.google.storage.v2.RestoreObjectRequest; import com.google.storage.v2.RewriteObjectRequest; import com.google.storage.v2.RewriteResponse; import com.google.storage.v2.StorageClient; +import com.google.storage.v2.StorageClient.ListBucketsPage; +import com.google.storage.v2.StorageClient.ListBucketsPagedResponse; import com.google.storage.v2.UpdateObjectRequest; import io.grpc.Status; import io.opentelemetry.sdk.metrics.SdkMeterProvider; @@ -55,6 +68,7 @@ import java.util.Collection; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; public final class GrpcStorageImplTest { @@ -187,7 +201,7 @@ public void unaryCalls_recordOperationMetrics() { @Test @SuppressWarnings("unchecked") - public void deleteBlob_onNonExistentObject_returnsFalseWithErrorTypeOk() { + public void deleteBlob_onNonExistentObject_returnsFalseWithErrorTypeNotFound() { UnaryCallable deleteObjectCallable = TestStorageStub.mockCallable(); storageStub.deleteObjectCallable = deleteObjectCallable; when(deleteObjectCallable.call(any(DeleteObjectRequest.class), any(GrpcCallContext.class))) @@ -198,9 +212,193 @@ public void deleteBlob_onNonExistentObject_returnsFalseWithErrorTypeOk() { boolean deleted = grpcStorage.delete(BlobId.of(BUCKET_NAME, "non-existent")); assertThat(deleted).isFalse(); - // Verify operations metric records STATUS_OK and not NOT_FOUND - assertOperationCount("DeleteObject", STATUS_OK, 1L); - assertOperationCount("DeleteObject", STATUS_NOT_FOUND, 0L); + // Verify operations metric records STATUS_NOT_FOUND and not STATUS_OK + assertOperationCount("DeleteObject", STATUS_NOT_FOUND, 1L); + assertOperationCount("DeleteObject", STATUS_OK, 0L); + } + + @Test + public void deleteBucket_onNonExistentBucket_returnsFalseWithErrorTypeNotFound() { + UnaryCallable deleteBucketCallable = TestStorageStub.mockCallable(); + storageStub.deleteBucketCallable = deleteBucketCallable; + when(deleteBucketCallable.call(any(DeleteBucketRequest.class), any(GrpcCallContext.class))) + .thenThrow( + new NotFoundException( + "Not Found", null, GrpcStatusCode.of(Status.Code.NOT_FOUND), false)); + + boolean deleted = grpcStorage.delete(BUCKET_NAME); + assertThat(deleted).isFalse(); + + assertOperationCount("DeleteBucket", STATUS_NOT_FOUND, 1L); + assertOperationCount("DeleteBucket", STATUS_OK, 0L); + } + + @Test + public void getBucketAndBlob_onNonExistent_returnsNullWithErrorTypeNotFound() { + UnaryCallable getBucketCallable = TestStorageStub.mockCallable(); + storageStub.getBucketCallable = getBucketCallable; + when(getBucketCallable.call(any(GetBucketRequest.class), any(GrpcCallContext.class))) + .thenThrow( + new NotFoundException( + "Not Found", null, GrpcStatusCode.of(Status.Code.NOT_FOUND), false)); + + assertThat(grpcStorage.get(BUCKET_NAME)).isNull(); + assertOperationCount("GetBucket", STATUS_NOT_FOUND, 1L); + assertOperationCount("GetBucket", STATUS_OK, 0L); + + UnaryCallable getObjectCallable = TestStorageStub.mockCallable(); + storageStub.getObjectCallable = getObjectCallable; + when(getObjectCallable.call(any(GetObjectRequest.class), any(GrpcCallContext.class))) + .thenThrow( + new NotFoundException( + "Not Found", null, GrpcStatusCode.of(Status.Code.NOT_FOUND), false)); + + assertThat(grpcStorage.get(BlobId.of(BUCKET_NAME, "non-existent"))).isNull(); + assertOperationCount("GetObject", STATUS_NOT_FOUND, 1L); + assertOperationCount("GetObject", STATUS_OK, 0L); + } + + @Test + public void deleteAclAndDefaultAcl_onNonExistent_returnsFalseWithErrorTypeNotFound() { + UnaryCallable getBucketCallable = TestStorageStub.mockCallable(); + storageStub.getBucketCallable = getBucketCallable; + when(getBucketCallable.call(any(GetBucketRequest.class), any(GrpcCallContext.class))) + .thenThrow( + new NotFoundException( + "Not Found", null, GrpcStatusCode.of(Status.Code.NOT_FOUND), false)); + + UnaryCallable getObjectCallable = TestStorageStub.mockCallable(); + storageStub.getObjectCallable = getObjectCallable; + when(getObjectCallable.call(any(GetObjectRequest.class), any(GrpcCallContext.class))) + .thenThrow( + new NotFoundException( + "Not Found", null, GrpcStatusCode.of(Status.Code.NOT_FOUND), false)); + + User entity = User.ofAllAuthenticatedUsers(); + assertThat(grpcStorage.deleteAcl(BUCKET_NAME, entity)).isFalse(); + assertOperationCount("DeleteBucketACL", STATUS_NOT_FOUND, 1L); + assertOperationCount("DeleteBucketACL", STATUS_OK, 0L); + + assertThat(grpcStorage.deleteDefaultAcl(BUCKET_NAME, entity)).isFalse(); + assertOperationCount("DeleteDefaultObjectACL", STATUS_NOT_FOUND, 1L); + assertOperationCount("DeleteDefaultObjectACL", STATUS_OK, 0L); + + assertThat(grpcStorage.deleteAcl(BlobId.of(BUCKET_NAME, BLOB_NAME), entity)).isFalse(); + assertOperationCount("DeleteObjectACL", STATUS_NOT_FOUND, 1L); + assertOperationCount("DeleteObjectACL", STATUS_OK, 0L); + } + + @Test + public void pcuSubOperations_doNotEmitTopLevelOperations_andInternalObjectGetThrowsOn404() { + UnaryCallable getObjectCallable = TestStorageStub.mockCallable(); + storageStub.getObjectCallable = getObjectCallable; + when(getObjectCallable.call(any(GetObjectRequest.class), any(GrpcCallContext.class))) + .thenReturn( + Object.newBuilder() + .setBucket("projects/_/buckets/" + BUCKET_NAME) + .setName(BLOB_NAME) + .build()) + .thenThrow( + new NotFoundException( + "Not Found", null, GrpcStatusCode.of(Status.Code.NOT_FOUND), false)); + + UnaryCallable deleteObjectCallable = TestStorageStub.mockCallable(); + storageStub.deleteObjectCallable = deleteObjectCallable; + when(deleteObjectCallable.call(any(DeleteObjectRequest.class), any(GrpcCallContext.class))) + .thenReturn(Empty.getDefaultInstance()); + + BlobId blobId = BlobId.of(BUCKET_NAME, BLOB_NAME); + BlobInfo info = grpcStorage.internalObjectGet(blobId, Opts.empty()); + assertThat(info).isNotNull(); + grpcStorage.internalObjectDelete(blobId, Opts.empty()); + + assertOperationCount("GetObject", STATUS_OK, 0L); + assertOperationCount("DeleteObject", STATUS_OK, 0L); + + // Verify internalObjectGet throws on 404 rather than returning null + assertThrows(StorageException.class, () -> grpcStorage.internalObjectGet(blobId, Opts.empty())); + assertOperationCount("GetObject", STATUS_NOT_FOUND, 0L); + } + + @Test + @SuppressWarnings("unchecked") + public void listBucketsAndObjects_paginationRecordsSingleOperationAndMultipleAttempts() { + // 1. list(BucketListOption...) via TransformingPageDecorator (getNextPage & iterateAll) + UnaryCallable listBucketsPagedCallable = + TestStorageStub.mockCallable(); + storageStub.listBucketsPagedCallable = listBucketsPagedCallable; + + ListBucketsPagedResponse pagedResp = + Mockito.mock(ListBucketsPagedResponse.class, Mockito.withSettings().withoutAnnotations()); + ListBucketsPage page1 = + Mockito.mock(ListBucketsPage.class, Mockito.withSettings().withoutAnnotations()); + ListBucketsPage page2 = + Mockito.mock(ListBucketsPage.class, Mockito.withSettings().withoutAnnotations()); + Bucket bucketProto1 = Bucket.newBuilder().setName("projects/_/buckets/b1").build(); + Bucket bucketProto2 = Bucket.newBuilder().setName("projects/_/buckets/b2").build(); + + when(pagedResp.getPage()).thenReturn(page1); + when(page1.hasNextPage()).thenReturn(true); + when(page1.getNextPage()).thenReturn(page2); + when(page1.getValues()).thenReturn(ImmutableList.of(bucketProto1)); + when(page2.hasNextPage()).thenReturn(false); + when(page2.getValues()).thenReturn(ImmutableList.of(bucketProto2)); + when(listBucketsPagedCallable.call(any(ListBucketsRequest.class), any(GrpcCallContext.class))) + .thenReturn(pagedResp); + + Page firstBucketPage = grpcStorage.list(); + Page secondBucketPage = firstBucketPage.getNextPage(); + assertThat(secondBucketPage).isNotNull(); + assertOperationCount("ListBuckets", STATUS_OK, 1L); + assertAttemptCount("ListBuckets", 2L); + + // IterateAll also records attempt via bound retrier without incrementing operations + ImmutableList allBuckets = + ImmutableList.copyOf(firstBucketPage.iterateAll()); + assertThat(allBuckets).hasSize(2); + assertOperationCount("ListBuckets", STATUS_OK, 1L); + assertAttemptCount("ListBuckets", 3L); + + // 2. list(BucketListOption.returnPartialSuccess(true)) via ListBucketsWithPartialSuccessPage + UnaryCallable listBucketsCallable = + TestStorageStub.mockCallable(); + storageStub.listBucketsCallable = listBucketsCallable; + when(listBucketsCallable.call(any(ListBucketsRequest.class), any(GrpcCallContext.class))) + .thenReturn( + ListBucketsResponse.newBuilder() + .addBuckets(bucketProto1) + .setNextPageToken("token-2") + .build(), + ListBucketsResponse.newBuilder().addBuckets(bucketProto2).build()); + + Page partialPage1 = + grpcStorage.list(BucketListOption.returnPartialSuccess(true)); + Page partialPage2 = partialPage1.getNextPage(); + assertThat(partialPage2).isNotNull(); + assertOperationCount("ListBuckets", STATUS_OK, 2L); + assertAttemptCount("ListBuckets", 5L); + + // 3. list(String bucket, BlobListOption...) via ListObjectsWithSyntheticDirectoriesPage + UnaryCallable listObjectsCallable = + TestStorageStub.mockCallable(); + storageStub.listObjectsCallable = listObjectsCallable; + Object obj1 = + Object.newBuilder().setBucket("projects/_/buckets/" + BUCKET_NAME).setName("o1").build(); + Object obj2 = + Object.newBuilder().setBucket("projects/_/buckets/" + BUCKET_NAME).setName("o2").build(); + when(listObjectsCallable.call(any(ListObjectsRequest.class), any(GrpcCallContext.class))) + .thenReturn( + ListObjectsResponse.newBuilder() + .addObjects(obj1) + .setNextPageToken("obj-token-2") + .build(), + ListObjectsResponse.newBuilder().addObjects(obj2).build()); + + Page firstObjectPage = grpcStorage.list(BUCKET_NAME); + Page secondObjectPage = firstObjectPage.getNextPage(); + assertThat(secondObjectPage).isNotNull(); + assertOperationCount("ListObjects", STATUS_OK, 1L); + assertAttemptCount("ListObjects", 2L); } @Test diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java index 4dcc91da4cc1..d113e469f216 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/TestStorageStub.java @@ -20,13 +20,20 @@ import com.google.protobuf.Empty; import com.google.storage.v2.Bucket; import com.google.storage.v2.CreateBucketRequest; +import com.google.storage.v2.DeleteBucketRequest; import com.google.storage.v2.DeleteObjectRequest; +import com.google.storage.v2.GetBucketRequest; import com.google.storage.v2.GetObjectRequest; +import com.google.storage.v2.ListBucketsRequest; +import com.google.storage.v2.ListBucketsResponse; +import com.google.storage.v2.ListObjectsRequest; +import com.google.storage.v2.ListObjectsResponse; import com.google.storage.v2.MoveObjectRequest; import com.google.storage.v2.Object; import com.google.storage.v2.RestoreObjectRequest; import com.google.storage.v2.RewriteObjectRequest; import com.google.storage.v2.RewriteResponse; +import com.google.storage.v2.StorageClient.ListBucketsPagedResponse; import com.google.storage.v2.UpdateObjectRequest; import com.google.storage.v2.stub.StorageStub; import java.util.concurrent.TimeUnit; @@ -34,12 +41,17 @@ class TestStorageStub extends StorageStub { UnaryCallable createBucketCallable; + UnaryCallable getBucketCallable; + UnaryCallable deleteBucketCallable; + UnaryCallable listBucketsCallable; + UnaryCallable listBucketsPagedCallable; UnaryCallable getObjectCallable; UnaryCallable deleteObjectCallable; UnaryCallable updateObjectCallable; UnaryCallable restoreObjectCallable; UnaryCallable moveObjectCallable; UnaryCallable rewriteObjectCallable; + UnaryCallable listObjectsCallable; @SuppressWarnings("unchecked") static UnaryCallable mockCallable() { @@ -51,6 +63,26 @@ public UnaryCallable createBucketCallable() { return createBucketCallable; } + @Override + public UnaryCallable getBucketCallable() { + return getBucketCallable; + } + + @Override + public UnaryCallable deleteBucketCallable() { + return deleteBucketCallable; + } + + @Override + public UnaryCallable listBucketsCallable() { + return listBucketsCallable; + } + + @Override + public UnaryCallable listBucketsPagedCallable() { + return listBucketsPagedCallable; + } + @Override public UnaryCallable getObjectCallable() { return getObjectCallable; @@ -81,6 +113,11 @@ public UnaryCallable rewriteObjectCallabl return rewriteObjectCallable; } + @Override + public UnaryCallable listObjectsCallable() { + return listObjectsCallable; + } + @Override public void shutdown() {} From eaa05b94495c6654a6aa385700f2d8afbb72dba7 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Mon, 2 Nov 2026 12:41:19 +0000 Subject: [PATCH 9/9] fix(storage): record UpdateBucket and UpdateObject metrics on no-op gRPC updates - Pass UpdateBucket and UpdateObject operation names into internalBucketGet and internalBlobGet when BucketInfo or BlobInfo has no modified fields in GrpcStorageImpl.update(...), while keeping GetBucket and GetObject for get(...) callers. - Add unit test in GrpcStorageImplTest verifying no-op update(BucketInfo) and update(BlobInfo) record UpdateBucket and UpdateObject operation metrics. [Generated-by: AI] --- .../google/cloud/storage/GrpcStorageImpl.java | 21 ++++++++++--- .../cloud/storage/GrpcStorageImplTest.java | 31 +++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index 6a3b97d05a9f..9103b847c80a 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java @@ -575,7 +575,8 @@ public Page list(String bucket, BlobListOption... options) { public Bucket update(BucketInfo bucketInfo, BucketTargetOption... options) { Opts unwrap = Opts.unwrap(options); if (bucketInfo.getModifiedFields().isEmpty()) { - return internalBucketGet(bucketInfo.getName(), unwrap.constrainTo(BucketSourceOpt.class)); + return internalBucketGet( + "UpdateBucket", bucketInfo.getName(), unwrap.constrainTo(BucketSourceOpt.class)); } Opts opts = unwrap.resolveFrom(bucketInfo).prepend(defaultOpts); GrpcCallContext grpcCallContext = @@ -604,7 +605,8 @@ public Bucket update(BucketInfo bucketInfo, BucketTargetOption... options) { public Blob update(BlobInfo blobInfo, BlobTargetOption... options) { Opts unwrap = Opts.unwrap(options); if (blobInfo.getModifiedFields().isEmpty()) { - return internalBlobGet(blobInfo.getBlobId(), unwrap.constrainTo(ObjectSourceOpt.class)); + return internalBlobGet( + "UpdateObject", blobInfo.getBlobId(), unwrap.constrainTo(ObjectSourceOpt.class)); } Opts opts = unwrap.resolveFrom(blobInfo).prepend(defaultOpts); GrpcCallContext grpcCallContext = @@ -2488,13 +2490,18 @@ private BlobInfo internalObjectGet( @Nullable private Blob internalBlobGet(BlobId blob, Opts unwrap) { + return internalBlobGet("GetObject", blob, unwrap); + } + + @Nullable + private Blob internalBlobGet(String operationName, BlobId blob, Opts unwrap) { Opts opts = unwrap.resolveFrom(blob); try { return internalObjectGet( blob, opts, StorageClientMetrics.createContext( - getOptions().getStorageClientMetrics(), "GetObject")) + getOptions().getStorageClientMetrics(), operationName)) .asBlob(this); } catch (StorageException e) { if (e.getCode() == 404 || e.getCause() instanceof NotFoundException) { @@ -2509,6 +2516,12 @@ private Blob internalBlobGet(BlobId blob, Opts unwrap) { @Nullable private Bucket internalBucketGet(String bucket, Opts unwrap) { + return internalBucketGet("GetBucket", bucket, unwrap); + } + + @Nullable + private Bucket internalBucketGet( + String operationName, String bucket, Opts unwrap) { Opts opts = unwrap.prepend(defaultOpts).prepend(ALL_BUCKET_FIELDS); GrpcCallContext grpcCallContext = opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault()); @@ -2516,7 +2529,7 @@ private Bucket internalBucketGet(String bucket, Opts unwrap) { GetBucketRequest.newBuilder().setName(bucketNameCodec.encode(bucket)); GetBucketRequest req = opts.getBucketsRequest().apply(builder).build(); StorageMetricsContext metricsContext = - StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), "GetBucket"); + StorageClientMetrics.createContext(getOptions().getStorageClientMetrics(), operationName); GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext)); try { return retrier.run( diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java index 197917b59e7a..396361de1436 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/GrpcStorageImplTest.java @@ -401,6 +401,37 @@ public void listBucketsAndObjects_paginationRecordsSingleOperationAndMultipleAtt assertAttemptCount("ListObjects", 2L); } + @Test + public void updateBucketAndBlob_withNoModifiedFields_recordsUpdateOperationMetrics() { + UnaryCallable getBucketCallable = TestStorageStub.mockCallable(); + storageStub.getBucketCallable = getBucketCallable; + when(getBucketCallable.call(any(GetBucketRequest.class), any(GrpcCallContext.class))) + .thenReturn(Bucket.newBuilder().setName("projects/_/buckets/" + BUCKET_NAME).build()); + + UnaryCallable getObjectCallable = TestStorageStub.mockCallable(); + storageStub.getObjectCallable = getObjectCallable; + when(getObjectCallable.call(any(GetObjectRequest.class), any(GrpcCallContext.class))) + .thenReturn( + Object.newBuilder() + .setBucket("projects/_/buckets/" + BUCKET_NAME) + .setName(BLOB_NAME) + .build()); + + BucketInfo unmodifiedBucket = BucketInfo.of(BUCKET_NAME); + assertThat(unmodifiedBucket.getModifiedFields()).isEmpty(); + com.google.cloud.storage.Bucket updatedBucket = grpcStorage.update(unmodifiedBucket); + assertThat(updatedBucket).isNotNull(); + assertOperationCount("UpdateBucket", STATUS_OK, 1L); + assertOperationCount("GetBucket", STATUS_OK, 0L); + + BlobInfo unmodifiedBlob = BlobInfo.newBuilder(BUCKET_NAME, BLOB_NAME).build(); + assertThat(unmodifiedBlob.getModifiedFields()).isEmpty(); + Blob updatedBlob = grpcStorage.update(unmodifiedBlob); + assertThat(updatedBlob).isNotNull(); + assertOperationCount("UpdateObject", STATUS_OK, 1L); + assertOperationCount("GetObject", STATUS_OK, 0L); + } + @Test @SuppressWarnings("unchecked") public void copy_propagatesMultiContextToRetrierWithAlg() {