Skip to content

feat(storage): add gcp.storage.client.request.body.size metric for write operations #14746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
nidhiii-27 wants to merge 1 commit into
base: feat/storage-ttfb-metric
Choose a base branch
from
Draft

feat(storage): add gcp.storage.client.request.body.size metric for write operations #14746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .../google-cloud-storage/src/main/java/com/google/cloud/storage/BaseStorageWriteChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ public final int write(ByteBuffer src) throws IOException {
return 0;
}
int write = tmp.write(src);
if (write > 0 && metricsContext != null) {
metricsContext.addRequestBytes(write);
}
return write;
} catch (StorageException e) {
throw new IOException(e);
Expand Down
17 changes: 15 additions & 2 deletions ...-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,10 @@ public Blob create(BlobInfo blobInfo, InputStream content, BlobWriteOption... op
.build();

try (UnbufferedWritableByteChannel c = session.open()) {
ByteStreams.copy(Channels.newChannel(inputStreamParam), c);
long copied = ByteStreams.copy(Channels.newChannel(inputStreamParam), c);
if (metricsContext != null) {
metricsContext.setRequestBytes(copied);
}
}
ApiFuture<WriteObjectResponse> responseApiFuture = session.getResult();
return this.getBlob(responseApiFuture);
Expand Down Expand Up @@ -315,6 +318,9 @@ public Blob internalCreateFrom(Path path, BlobInfo info, Opts<ObjectTargetOpt> o
}

StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("WriteObject");
if (multiContext != null) {
multiContext.setRequestBytes(Files.size(path));
}
Throwable error = null;
try {
Comment on lines +321 to 325

@gemini-code-assist gemini-code-assist Bot Oct 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Calling Files.size(path) throws a checked IOException. Since internalCreateFrom does not declare throws IOException, calling it outside of the try block will cause a compilation error. Additionally, if an I/O error occurs, it should be caught within the try-catch block so that the exception is properly translated to a StorageException and the metrics context can record the failure in its finally block. Moving this call inside the try block resolves both issues.

Suggested change
if (multiContext != null) {
multiContext.setRequestBytes(Files.size(path));
}
Throwable error = null;
try {
Throwable error = null;
try {
if (multiContext != null) {
multiContext.setRequestBytes(Files.size(path));
}

Comment on lines +321 to 325

@gemini-code-assist gemini-code-assist Bot Oct 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Calling Files.size(path) outside of the try block can cause issues. Since Files.size(path) throws a checked IOException, calling it outside the try block means any thrown exception will bypass the error handling and metrics recording logic. Moving this call inside the try block ensures that any IOException is properly caught, recorded as an error in the metrics context, and handled consistently.

Suggested change
if (multiContext != null) {
multiContext.setRequestBytes(Files.size(path));
}
Throwable error = null;
try {
Throwable error = null;
try {
if (multiContext != null) {
multiContext.setRequestBytes(Files.size(path));
}

GrpcCallContext grpcCallContext =
Expand Down Expand Up @@ -400,7 +406,10 @@ public Blob createFrom(
ReadableByteChannel src =
Channels.newChannel(firstNonNull(in, new ByteArrayInputStream(ZERO_BYTES)));
try (BufferedWritableByteChannel dst = session.open()) {
ByteStreams.copy(src, dst);
long copied = ByteStreams.copy(src, dst);
if (multiContext != null) {
multiContext.setRequestBytes(copied);
}
} catch (Exception e) {
throw StorageException.coalesce(e);
}
Expand Down Expand Up @@ -938,6 +947,10 @@ public BlobInfo internalDirectUpload(
WriteObjectRequest req = getWriteObjectRequest(blobInfo, optsWithDefaults);
Hasher hasher = opts.getHasher();
StorageMetricsContext metricsContext = getOptions().createMetricsContext("WriteObject");
int totalBytes = buf.remaining();
if (metricsContext != null) {
metricsContext.setRequestBytes(totalBytes);
}
GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext(metricsContext));
RewindableContent content = RewindableContent.of(buf);
return retrier.run(
Expand Down
14 changes: 11 additions & 3 deletions java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@ public Blob createFrom(BlobInfo blobInfo, Path path, int bufferSize, BlobWriteOp
return create(blobInfo, null, options);
}
StorageMetricsContext multiContext = getOptions().createMultiStepMetricsContext("WriteObject");
if (multiContext != null) {
multiContext.setRequestBytes(size);
}
Throwable error = null;
try {
Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo);
Expand Down Expand Up @@ -1921,6 +1924,7 @@ public BlobInfo internalCreateFrom(Path path, BlobInfo info, Opts<ObjectTargetOp

@Override
public BlobInfo internalDirectUpload(BlobInfo info, Opts<ObjectTargetOpt> opts, ByteBuffer buf) {
int totalBytes = buf.remaining();
BlobInfo.Builder builder =
opts.blobInfoMapper().apply(info.toBuilder().clearMd5().clearCrc32c());
@Nullable Crc32cLengthKnown hash = opts.getHasher().hash(buf.duplicate());
Expand All @@ -1934,14 +1938,18 @@ public BlobInfo internalDirectUpload(BlobInfo info, Opts<ObjectTargetOpt> opts,
ResultRetryAlgorithm<?> algorithm =
retryAlgorithmManager.getForObjectsCreate(encoded, optionsMap);
RewindableContent content = RewindableContent.of(buf);
return run(
"WriteObject",
StorageMetricsContext metricsContext = getOptions().createMetricsContext("WriteObject");
if (metricsContext != null) {
metricsContext.setRequestBytes(totalBytes);
}
return retrier.run(
algorithm,
() -> {
content.rewindTo(0);
return storageRpc.create(encoded, new RewindableContentInputStream(content), optionsMap);
},
Conversions.json().blobInfo());
Conversions.json().blobInfo(),
metricsContext);
}

/**
Expand Down
27 changes: 27 additions & 0 deletions ...ge/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,29 @@ void recordOperation(
.build();
metrics.getGcpClientRequestDuration().record(getElapsedSeconds(), durationAttributes);
}

long reqBytes = requestBytes.get();
if ("WriteObject".equals(operationName)
&& reqBytes > 0
&& metrics.getRequestBodySize() != null) {
AttributesBuilder bodyAttrBuilder = Attributes.builder();
bodyAttrBuilder.putAll(metrics.getClientAttributes());
baseAttributes.forEach(
(key, val) -> {
if (!key.equals(StorageClientMetrics.KEY_NETWORK_PATH)
&& !key.equals(StorageClientMetrics.KEY_RPC_RESPONSE_STATUS_CODE)
&& !key.equals(StorageClientMetrics.KEY_HTTP_RESPONSE_STATUS_CODE)
&& !key.equals(StorageClientMetrics.KEY_ERROR_TYPE)
&& !key.equals(StorageClientMetrics.KEY_GCP_STORAGE_OPERATION)
&& !key.equals(StorageClientMetrics.KEY_GCP_CLIENT_METHOD)) {
@SuppressWarnings("unchecked")
AttributeKey<Object> objKey = (AttributeKey<Object>) key;
bodyAttrBuilder.put(objKey, val);
}
});
bodyAttrBuilder.put(StorageClientMetrics.KEY_RPC_METHOD, operationName);
metrics.getRequestBodySize().record(reqBytes, bodyAttrBuilder.build(), otelContext);
}
}
}

Expand Down Expand Up @@ -394,6 +417,10 @@ long addRequestBytes(long bytes) {
return requestBytes.addAndGet(bytes);
}

void setRequestBytes(long bytes) {
requestBytes.set(bytes);
}

long getRequestBytes() {
return requestBytes.get();
}
Expand Down
91 changes: 91 additions & 0 deletions ...oogle-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,4 +568,95 @@ private static LongPointData getSinglePoint(Collection<MetricData> metrics, Stri
assertThat(metric.getLongSumData().getPoints()).hasSize(1);
return metric.getLongSumData().getPoints().iterator().next();
}

@Test
public void recordOperation_recordsRequestBodySize_whenWriteObjectAndBytesGreaterThanZero() {
io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader reader =
io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader.create();
io.opentelemetry.sdk.metrics.SdkMeterProvider provider =
io.opentelemetry.sdk.metrics.SdkMeterProvider.builder()
.registerMetricReader(reader)
.build();
StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false);

StorageMetricsContext context = StorageMetricsContext.create("WriteObject");
context.setRequestBytes(2048L);
context.recordOperation(metrics, null);

java.util.Collection<io.opentelemetry.sdk.metrics.data.MetricData> collected =
reader.collectAllMetrics();
io.opentelemetry.sdk.metrics.data.MetricData bodySizeData =
collected.stream()
.filter(
m ->
m.getName()
.equals(StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_REQUEST_BODY_SIZE))
.findFirst()
.orElse(null);

assertThat(bodySizeData).isNotNull();
assertThat(bodySizeData.getHistogramData().getPoints()).hasSize(1);
io.opentelemetry.sdk.metrics.data.HistogramPointData point =
bodySizeData.getHistogramData().getPoints().iterator().next();
assertThat(point.getCount()).isEqualTo(1L);
assertThat(point.getSum()).isEqualTo(2048.0);
assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_METHOD))
.isEqualTo("WriteObject");
}

@Test
public void recordOperation_doesNotRecordRequestBodySize_whenBytesIsZero() {
io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader reader =
io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader.create();
io.opentelemetry.sdk.metrics.SdkMeterProvider provider =
io.opentelemetry.sdk.metrics.SdkMeterProvider.builder()
.registerMetricReader(reader)
.build();
StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false);

StorageMetricsContext context = StorageMetricsContext.create("WriteObject");
context.setRequestBytes(0L);
context.recordOperation(metrics, null);

java.util.Collection<io.opentelemetry.sdk.metrics.data.MetricData> collected =
reader.collectAllMetrics();
io.opentelemetry.sdk.metrics.data.MetricData bodySizeData =
collected.stream()
.filter(
m ->
m.getName()
.equals(StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_REQUEST_BODY_SIZE))
.findFirst()
.orElse(null);

assertThat(bodySizeData).isNull();
}

@Test
public void recordOperation_doesNotRecordRequestBodySize_whenNotWriteObject() {
io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader reader =
io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader.create();
io.opentelemetry.sdk.metrics.SdkMeterProvider provider =
io.opentelemetry.sdk.metrics.SdkMeterProvider.builder()
.registerMetricReader(reader)
.build();
StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false);

StorageMetricsContext context = StorageMetricsContext.create("ReadObject");
context.setRequestBytes(2048L);
context.recordOperation(metrics, null);

java.util.Collection<io.opentelemetry.sdk.metrics.data.MetricData> collected =
reader.collectAllMetrics();
io.opentelemetry.sdk.metrics.data.MetricData bodySizeData =
collected.stream()
.filter(
m ->
m.getName()
.equals(StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_REQUEST_BODY_SIZE))
.findFirst()
.orElse(null);

assertThat(bodySizeData).isNull();
}
}
Loading