Repository navigation
feat(storage): add gcp.storage.client.response.body.size metric for read operations - #14747
nidhiii-27 wants to merge 7 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements response body size tracking for the 'ReadObject' operation across both gRPC and JSON implementations of Google Cloud Storage, updating BaseStorageReadChannel, GrpcStorageImpl, StorageImpl, and StorageMetricsContext, along with adding corresponding unit tests. The review feedback suggests refactoring a long chain of excluded key checks in StorageMetricsContext into a static Set constant to improve maintainability, and replacing fully qualified class names with imports in the new test cases to enhance readability.
There was a problem hiding this comment.
Code Review
This pull request introduces tracking and recording of response body size metrics for "ReadObject" operations across both GRPC and JSON/HTTP storage implementations. It updates BaseStorageReadChannel, GrpcStorageImpl, and StorageImpl to capture the read or downloaded bytes and set them in the StorageMetricsContext, which now records the response body size metric. Corresponding unit tests are also added to StorageMetricsContextTest. The reviewer suggested avoiding the use of fully qualified class names in the new test cases to improve code readability, recommending importing them instead.
…se body size - Extract BODY_SIZE_EXCLUDED_KEYS and buildBodySizeAttributes(StorageClientMetrics) in StorageMetricsContext to deduplicate request/response body size attribute filtering. - Guard GrpcStorageImpl.readAllBytes response byte recording with error == null and move countBytes helper to the private helper section. - Wire GrpcStorageImpl.blobReadSession and BlobReadSessionAdapter to track delivered response bytes across all ReadProjectionConfig types and record gcp.storage.client.response.body.size on session close. - Track lastReadError in BaseStorageReadChannel so mid-stream read errors propagate to recordOperation on close while preserving partial read byte counts. - Replace inline FQCNs with imports in StorageMetricsContextTest and add unit tests for channel reads, StorageImpl, GrpcStorageImpl, and BlobReadSessionAdapter. [Generated-by: AI]
…rdResponseBodySize
…ge.operation on response.body.size [Generated-by: AI]
cpriti-os
left a comment
cpriti-os
left a comment
There was a problem hiding this comment.
Mostly looks good, left a few comments inline.
…e-response-body-size [Generated-by: AI]
[Generated-by: AI]
…nse-body-size [Generated-by: AI]
Summary
This PR adds the
gcp.storage.client.response.body.sizemetric for read operations.Changes
StorageMetricsContext:gcp.storage.client.response.body.sizeinsiderecordOperation(...)whenoperationNameis"ReadObject"andresponseBytes > 0.BODY_SIZE_EXCLUDED_KEYSandbuildBodySizeAttributes(StorageClientMetrics)to share attribute filtering betweenrequest.body.sizeandresponse.body.size.BaseStorageReadChannel/BlobReadChannelV2/GrpcBlobReadChannel:BaseStorageReadChannel.read(ByteBuffer)viametricsContext.addResponseBytes(read).lastReadErroracrossread(ByteBuffer)so mid-stream errors are passed torecordOperation(...)onclose()while preserving partial byte counts.StorageImpl:readAllBytesanddownloadTo(usingCountingOutputStream.getCount()across retries without double-counting).GrpcStorageImpl&BlobReadSessionAdapter:GrpcStorageImpl.readAllBytes(whenerror == null) and accumulates written bytes inGrpcStorageImpl.downloadToviacountBytes.GrpcStorageImpl.blobReadSessionandBlobReadSessionAdapterto accumulate delivered bytes across allReadProjectionConfigtypes (asFutureBytes(),asFutureByteString(),asChannel(),asSeekableChannel()) and record the operation onclose()(or on session open failure).StorageMetricsContextTest:recordOperationattribute filtering,BaseStorageReadChannel(full read, partial read + early close, 0-byte close, mid-stream error),StorageImpl(readAllBytes,downloadTowith retries),BlobReadSessionAdapter(all projections and open failure), andGrpcStorageImpl(readAllBytes,downloadTo,blobReadSession).