[nidhiii-27]

Instruments read operations across HTTP and gRPC veneers to record gcp.storage.client.response.body.size OpenTelemetry histogram metrics.

• Adds byte counting tracking in StorageMetricsContext (addBytesReceived) and records gcp.storage.client.response.body.size on operation completion.
• Instruments BaseStorageReadChannel, StorageImpl, and GrpcStorageImpl to track downloaded payload bytes across unary and streaming reads.
• Initializes gcp.storage.client.response.body.size in StorageClientMetrics and adds unit tests in StorageMetricsContextTest, StorageByteCountingTest, and StorageClientMetricsTest.

Stacked on #14746.

[gemini-code-assist]

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.

[gemini-code-assist]

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);
              }
            });

[gemini-code-assist]

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.

[gemini-code-assist]

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.

[gemini-code-assist]

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.