Skip to content

feat(storage): add gcp.storage.client.response.body.size metric for read operations #14747

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-request-body-size
Choose a base branch
from
Draft

feat(storage): add gcp.storage.client.response.body.size metric for read operations #14747

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 ...e/google-cloud-storage/src/main/java/com/google/cloud/storage/BaseStorageReadChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ public final int read(ByteBuffer dst) throws IOException {
int read = tmp.read(dst);
if (read != -1) {
byteRangeSpec = byteRangeSpec.withShiftBeginOffset(read);
if (read > 0 && metricsContext != null) {
metricsContext.addResponseBytes(read);
}
}
return read;
} catch (StorageException e) {
Expand Down
32 changes: 30 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 @@ -819,6 +819,7 @@ public byte[] readAllBytes(BlobId blob, BlobSourceOption... options) {
throw e;
} finally {
if (metricsContext != null) {
metricsContext.setResponseBytes(baos.size());
metricsContext.recordOperation(getOptions().getStorageClientMetrics(), error);
}
}
Expand Down Expand Up @@ -864,7 +865,7 @@ public void downloadTo(BlobId blob, Path path, BlobSourceOption... options) {

Throwable error = null;
try (UnbufferedReadableByteChannel r = session.open();
WritableByteChannel w = Files.newByteChannel(path, WRITE_OPS)) {
WritableByteChannel w = countBytes(Files.newByteChannel(path, WRITE_OPS), metricsContext)) {
ByteStreams.copy(r, w);
} catch (ApiException | IOException e) {
error = e;
Expand All @@ -887,7 +888,7 @@ public void downloadTo(BlobId blob, OutputStream outputStream, BlobSourceOption.

Throwable error = null;
try (UnbufferedReadableByteChannel r = session.open();
WritableByteChannel w = Channels.newChannel(outputStream)) {
WritableByteChannel w = countBytes(Channels.newChannel(outputStream), metricsContext)) {
ByteStreams.copy(r, w);
} catch (ApiException | IOException e) {
error = e;
Expand All @@ -902,6 +903,33 @@ public void downloadTo(BlobId blob, OutputStream outputStream, BlobSourceOption.
}
}

private static WritableByteChannel countBytes(
WritableByteChannel delegate, @Nullable StorageMetricsContext metricsContext) {
if (metricsContext == null) {
return delegate;
}
return new WritableByteChannel() {
@Override
public int write(ByteBuffer src) throws IOException {
int written = delegate.write(src);
if (written > 0) {
metricsContext.addResponseBytes(written);
}
return written;
}

@Override
public boolean isOpen() {
return delegate.isOpen();
}

@Override
public void close() throws IOException {
delegate.close();
}
};
}

@Override
public GrpcBlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options) {
Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo).prepend(defaultOpts);
Expand Down
32 changes: 23 additions & 9 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 @@ -761,11 +761,18 @@ public byte[] readAllBytes(BlobId blob, BlobSourceOption... options) {
ImmutableMap<StorageRpc.Option, ?> optionsMap = resolve.getRpcOptions();
ResultRetryAlgorithm<?> algorithm =
retryAlgorithmManager.getForObjectsGet(storageObject, optionsMap);
return run(
"ReadObject",
StorageMetricsContext metricsContext = getOptions().createMetricsContext("ReadObject");
return retrier.run(
algorithm,
() -> storageRpc.load(storageObject, optionsMap),
Decoder.identity());
() -> {
byte[] bytes = storageRpc.load(storageObject, optionsMap);
if (metricsContext != null && bytes != null) {
metricsContext.setResponseBytes(bytes.length);
}
return bytes;
},
Decoder.identity(),
metricsContext);
}

@Override
Expand Down Expand Up @@ -804,15 +811,22 @@ public void downloadTo(BlobId blob, OutputStream outputStream, BlobSourceOption.
ImmutableMap<StorageRpc.Option, ?> optionsMap =
Opts.unwrap(options).resolveFrom(blob).getRpcOptions();
ResultRetryAlgorithm<?> algorithm = retryAlgorithmManager.getForObjectsGet(pb, optionsMap);
run(
"ReadObject",
StorageMetricsContext metricsContext = getOptions().createMetricsContext("ReadObject");
retrier.run(
algorithm,
callable(
() -> {
storageRpc.read(
pb, optionsMap, countingOutputStream.getCount(), countingOutputStream);
try {
storageRpc.read(
pb, optionsMap, countingOutputStream.getCount(), countingOutputStream);
} finally {
if (metricsContext != null) {
metricsContext.setResponseBytes(countingOutputStream.getCount());
}
}
}),
Decoder.identity());
Decoder.identity(),
metricsContext);
}

@Override
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 @@ -205,6 +205,29 @@ void recordOperation(
bodyAttrBuilder.put(StorageClientMetrics.KEY_RPC_METHOD, operationName);
metrics.getRequestBodySize().record(reqBytes, bodyAttrBuilder.build(), otelContext);
}

long respBytes = responseBytes.get();
if ("ReadObject".equals(operationName)
&& respBytes > 0
&& metrics.getResponseBodySize() != 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);
}
});
Comment on lines +215 to +227

@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.

medium

To improve maintainability and readability, avoid duplicating the long chain of !key.equals(...) checks. Instead, define a private static final Set<AttributeKey<?>> EXCLUDED_KEYS constant containing the excluded keys, and use !EXCLUDED_KEYS.contains(key) here.

        baseAttributes.forEach(
            (key, val) -> {
              if (!EXCLUDED_KEYS.contains(key)) {
                @SuppressWarnings("unchecked")
                AttributeKey<Object> objKey = (AttributeKey<Object>) key;
                bodyAttrBuilder.put(objKey, val);
              }
            });

bodyAttrBuilder.put(StorageClientMetrics.KEY_RPC_METHOD, operationName);
metrics.getResponseBodySize().record(respBytes, bodyAttrBuilder.build(), otelContext);
}
}
}

Expand Down Expand Up @@ -429,6 +452,10 @@ long addResponseBytes(long bytes) {
return responseBytes.addAndGet(bytes);
}

void setResponseBytes(long bytes) {
responseBytes.set(bytes);
}

long getResponseBytes() {
return responseBytes.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 @@ -659,4 +659,95 @@ public void recordOperation_doesNotRecordRequestBodySize_whenNotWriteObject() {

assertThat(bodySizeData).isNull();
}

@Test
public void recordOperation_recordsResponseBodySize_whenReadObjectAndBytesGreaterThanZero() {
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.setResponseBytes(4096L);
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_RESPONSE_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();
Comment on lines +665 to +691

@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.

medium

Avoid using fully qualified class names (such as io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader) when they can be imported. This unnecessarily reduces code readability and violates the project's general rules.

    InMemoryMetricReader reader = InMemoryMetricReader.create();
    SdkMeterProvider provider =
        SdkMeterProvider.builder()
            .registerMetricReader(reader)
            .build();
    StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false);

    StorageMetricsContext context = StorageMetricsContext.create("ReadObject");
    context.setResponseBytes(4096L);
    context.recordOperation(metrics, null);

    Collection<MetricData> collected = reader.collectAllMetrics();
    MetricData bodySizeData =
        collected.stream()
            .filter(
                m ->
                    m.getName()
                        .equals(StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_RESPONSE_BODY_SIZE))
            .findFirst()
            .orElse(null);

    assertThat(bodySizeData).isNotNull();
    assertThat(bodySizeData.getHistogramData().getPoints()).hasSize(1);
    HistogramPointData point =
        bodySizeData.getHistogramData().getPoints().iterator().next();
References
  1. Do not use fully qualified class names if there is no class name conflict in the file and the class is already imported, as it unnecessarily reduces code readability.

assertThat(point.getCount()).isEqualTo(1L);
assertThat(point.getSum()).isEqualTo(4096.0);
assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_METHOD))
.isEqualTo("ReadObject");
Comment on lines +665 to +695

@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.

medium

Avoid using fully qualified class names (such as io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader, io.opentelemetry.sdk.metrics.SdkMeterProvider, java.util.Collection, io.opentelemetry.sdk.metrics.data.MetricData, and io.opentelemetry.sdk.metrics.data.HistogramPointData) when there is no class name conflict in the file and the classes are already imported or can be imported. This unnecessarily reduces code readability. Please import these classes and use their simple names instead. This also applies to the other new tests in this file.

    InMemoryMetricReader reader =
        InMemoryMetricReader.create();
    SdkMeterProvider provider =
        SdkMeterProvider.builder()
            .registerMetricReader(reader)
            .build();
    StorageClientMetrics metrics = StorageClientMetrics.create(provider, true, false);

    StorageMetricsContext context = StorageMetricsContext.create("ReadObject");
    context.setResponseBytes(4096L);
    context.recordOperation(metrics, null);

    Collection<MetricData> collected =
        reader.collectAllMetrics();
    MetricData bodySizeData =
        collected.stream()
            .filter(
                m ->
                    m.getName()
                        .equals(StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_RESPONSE_BODY_SIZE))
            .findFirst()
            .orElse(null);

    assertThat(bodySizeData).isNotNull();
    assertThat(bodySizeData.getHistogramData().getPoints()).hasSize(1);
    HistogramPointData point =
        bodySizeData.getHistogramData().getPoints().iterator().next();
    assertThat(point.getCount()).isEqualTo(1L);
    assertThat(point.getSum()).isEqualTo(4096.0);
    assertThat(point.getAttributes().get(StorageClientMetrics.KEY_RPC_METHOD))
        .isEqualTo("ReadObject");
References
  1. Do not use fully qualified class names (such as java.time.Duration) if there is no class name conflict in the file and the class is already imported, as it unnecessarily reduces code readability.

}

@Test
public void recordOperation_doesNotRecordResponseBodySize_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("ReadObject");
context.setResponseBytes(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_RESPONSE_BODY_SIZE))
.findFirst()
.orElse(null);

assertThat(bodySizeData).isNull();
}

@Test
public void recordOperation_doesNotRecordResponseBodySize_whenNotReadObject() {
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.setResponseBytes(4096L);
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_RESPONSE_BODY_SIZE))
.findFirst()
.orElse(null);

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