Repository navigation
feat(storage): OpenTelemetry client metrics configuration and options - #14484
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces OpenTelemetry client metrics configuration across GrpcStorageOptions, HttpStorageOptions, and StorageOptions, supported by a new StorageMetricsConfig class and comprehensive unit tests. The review feedback highlights critical issues regarding backward compatibility during deserialization, where the newly added metricInterval field could be null and cause a NullPointerException, and recommends declaring the static resolvers in StorageMetricsConfig as volatile to ensure thread safety during parallel test execution.
cpriti-os
left a comment
There was a problem hiding this comment.
Requesting changes mainly for serialization behaviour and the enablement configuration (see inline comments).
cpriti-os
left a comment
There was a problem hiding this comment.
LGTM once the getMeterProvider() default behavior below is addressed.
c21ff47 to
f1d51f3
Compare
Dhriti07
left a comment
There was a problem hiding this comment.
Suggestion: We repeat the same test cases for both HttpStorageOptions and GrpcStorageOptions across most of StorageOptionsTest. Could we deduplicate these via a small helper or parameterization to reduce duplication across code. This prevents copy-paste drift between HTTP and gRPC and makes future test maintenance easier.
…nfiguration - Add configuration options to StorageOptions, GrpcStorageOptions, and HttpStorageOptions (enableOtelMetrics, enableOtelDebugMetrics, meterProvider, metricInterval). - Add development gate checking system properties and environment variables in StorageMetricsConfig. - Implement package-private instrument registry in StorageClientMetrics. - Update OpenTelemetryBootstrappingUtils to register client views with custom histogram boundaries and create client meter provider. - Add comprehensive unit tests in StorageOptionsTest, StorageClientMetricsTest, and OpenTelemetryBootstrappingUtilsTest.
Apply spotify fmt-maven-plugin formatting to StorageMetricsConfig, StorageClientMetrics, StorageOptionsTest, and StorageClientMetricsTest. [Generated-by: AI]
Use simple class name Duration instead of fully qualified java.time.Duration in GrpcStorageOptions and OpenTelemetryBootstrappingUtils. [Generated-by: AI]
…trappingUtils Apply spotify fmt-maven-plugin formatting to OpenTelemetryBootstrappingUtils. [Generated-by: AI]
…nd views [Generated-by: AI]
… metrics Refactor addClientHistogramView in OpenTelemetryBootstrappingUtils to remove unit selector and custom view description, preserving instrument default descriptions. Remove deferred Phase 2 connection metrics (dnsLookupDuration, tcpConnectDuration, tlsHandshakeDuration) from StorageClientMetrics, CLIENT_LATENCY_HISTOGRAMS, and StorageClientMetricsTest. Ensure clean Duration imports. [Generated-by: AI]
…rProvider Default api parameter to "grpc" instead of "storage" in createClientMeterProvider, allow specifying "json" / "http" transport protocols for Cloud Monitoring monitored resource storage.googleapis.com/Client, and add unit test coverage in OpenTelemetryBootstrappingUtilsTest. [Generated-by: AI]
Update createClientMeterProvider to use prefix custom.googleapis.com with default monitored resource mapping. Add KEY_URL_TEMPLATE attribute key constant in StorageClientMetrics. [Generated-by: AI]
…for default monitored resource
…geClientMetricsTest [Generated-by: AI]
…m boundary helpers
…ions [Generated-by: AI]
…rialization [Generated-by: AI]
… options - In GrpcStorageOptions and HttpStorageOptions: - Add null check in getMetricInterval() defaulting to 60 seconds - Use getMetricInterval() in equals() and hashCode() - Update Javadoc for setMeterProvider to document nullable/default behavior - In StorageOptions: - Update Javadoc for setMeterProvider on Builder - In StorageMetricsConfig: - Make sysPropResolver and envResolver volatile for thread safety - In StorageOptionsTest: - Add unit test verifying deserialization with null metricInterval defaults to 60s and equals/hashCode match - Verify explicit client options take precedence over env vars and sysprops [Generated-by: AI]
[Generated-by: AI]
Format StorageOptions, GrpcStorageOptions, HttpStorageOptions, and StorageOptionsTest using com.spotify.fmt:fmt-maven-plugin:2.25:format under JDK 17 to resolve CI lint check failure. [Generated-by: AI]
[Generated-by: AI]
…iew comments Clamp metric export interval to at least 60 seconds when meterProvider is null and no custom OpenTelemetry is configured, preventing invalid sub-60s intervals during deserialization when falling back to default Cloud Monitoring exporter. [Generated-by: AI]
…ig and tests Deduplicate flag resolution in StorageMetricsConfig by extracting resolveFlag helper, separate multi-behavior tests per go/tott/726, and add input diagnostic messages in looped assertions. [Generated-by: AI]
…Test [Generated-by: AI]
4c7531d to
e7a3588
Compare
…iews (#14398) This PR implements Phase 1 (Core Metrics Registry & Views) of OpenTelemetry Client Metrics in the Java Cloud Storage SDK (`google-cloud-storage`). Following review feedback and architectural alignment across Storage SDKs, this PR focuses strictly on the package-private instrument registry and OpenTelemetry view/histogram bootstrapping, decoupling core infrastructure from public configuration options (handled in follow-up PR #14484). ## Summary of Changes - **Instrument Registry (`StorageClientMetrics`)**: - Implemented package-private registry defining standard and debug OpenTelemetry instruments (call/request durations, operation and retry counters, transfer sizes, TTFB, active requests, stall duration, etc.). - Configured standardized attribute keys and UCUM unit strings (e.g., `s`, `By`, `{operation}`, `{attempt}`, `{retry}`). - **OpenTelemetry Views & Exporter Bootstrapping (`OpenTelemetryBootstrappingUtils`)**: - Registered custom explicit-bucket histogram boundaries for latency (2ms resolution) and size distributions (128KiB to 16GiB). - Configured metric name view translation (`.` to `/`). - Added `createClientMeterProvider` to bootstrap Google Cloud Monitoring export under `custom.googleapis.com` with client attributes (`instance_id`, `api`). - **Testing**: - Added unit test suites (`StorageClientMetricsTest`, `OpenTelemetryBootstrappingUtilsTest`) verifying metric registrations, custom view bucket distributions, and exporter bootstrapping using `InMemoryMetricReader`. --- [Generated-by: AI] --------- Co-authored-by: Dhriti07 <56169283+Dhriti07@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- <details><summary>1.95.0</summary> ## [1.95.0](v1.94.0...v1.95.0) (2026-11-02) ### Features * **bigquery-jdbc:** expose BigQueryResultSetMetadata and getField(int) ([#14761](#14761)) ([f01245a](f01245a)) * **gax:** align resumable upload chunk size to server granularity and surface effective chunk size ([#14670](#14670)) ([6cbb096](6cbb096)) * **google/cloud/designcenter/v1:** add designcenter ([#14823](#14823)) ([01d27dc](01d27dc)) * **google/cloud/observability/v1:** add observability ([#14722](#14722)) ([624f28e](624f28e)) * **oauth2:** support certificate-bound OAuth2 tokens and mTLS rotation for Workload Identity Federation ([#14781](#14781)) ([dcb29b0](dcb29b0)) * **storage:** add bucket IP filtering samples and tests ([#14328](#14328)) ([cb2b726](cb2b726)) * **storage:** add support for DirectPath over Interconnect ([#14006](#14006)) ([ad0eb94](ad0eb94)) * **storage:** OpenTelemetry client metrics configuration and options ([#14484](#14484)) ([7c29e48](7c29e48)) * **storage:** OpenTelemetry client metrics instrument registry and views ([#14398](#14398)) ([60233b7](60233b7)) ### Bug Fixes * add user agent to session path ([#14835](#14835)) ([a653638](a653638)) * **auth:** align mTLS transport and bound token certificate startup polling and well-known discovery ([#14748](#14748)) ([0bae268](0bae268)) * **bigquery-jdbc:** expand handling to OOM errors ([#14759](#14759)) ([7d4b921](7d4b921)) * **bigquery-jdbc:** fix naive fallback of parameter count ([#14779](#14779)) ([c3e4a5b](c3e4a5b)) * **bigquery-jdbc:** format RANGE bounds consistently across REST and Arrow ([#14807](#14807)) ([2108b8e](2108b8e)) * **bigquery-jdbc:** parse scientific-notation epoch TIMESTAMPs ([#14777](#14777)) ([71a31b1](71a31b1)) * **bigquery-jdbc:** preserve pre-1582 and pre-standard-time DATE/DATETIME digits ([#14787](#14787)) ([3dc394f](3dc394f)) * **bigtable:** avoid mutating Query in-place in createPointReadCallable ([#14797](#14797)) ([1b1c6a7](1b1c6a7)) * **bigtable:** check isOk and getRejected first in RetryingVRpc.shouldRetry ([#14795](#14795)) ([d821bd3](d821bd3)) * **bigtable:** check remaining deadline on client-initiated retries in RetryingVRpc ([#14796](#14796)) ([a04e24c](a04e24c)) * **bigtable:** Fix empty RowResult semantics in TestProxy. ([#14810](#14810)) ([1d2fb9a](1d2fb9a)) * **bigtable:** include reversed in Query equals, hashCode, and toString ([#14799](#14799)) ([fe9232b](fe9232b)) * **bigtable:** pass cell value size to RowBuilder.startCell in RowBuilderShim ([#14800](#14800)) ([cfa7949](cfa7949)) * **bigtable:** reject empty split points in RowSetUtil and clarify QueryPaginator Javadoc ([#14811](#14811)) ([1bc507f](1bc507f)) * **bigtable:** remove legacy PollingConfiguration migration in ClientConfigurationManager ([#14822](#14822)) ([95ead45](95ead45)) * **bigtable:** resolve session diversion through a single shared rule ([#14738](#14738)) ([00f7bea](00f7bea)) * **bigtable:** respect query limit smaller than pageSize and avoid mutating Query in QueryPaginator ([#14821](#14821)) ([cecd09e](cecd09e)) * **firestore:** graduate pipeline search from preview to GA ([#14741](#14741)) ([a993026](a993026)) * **gax:** do not treat a SocketTimeoutException as a thread interrupt ([#14778](#14778)) ([7b91805](7b91805)) * **jdbc:** populate `ResultSetMetaData` on empty `DatabaseMetaData` resultsets ([#14809](#14809)) ([4c6a7e3](4c6a7e3)) * **pubsub:** make publish hedging robust to clock jumps, queue errors, and shutdown ([#14860](#14860)) ([9b9e2a5](9b9e2a5)) * **spanner:** attach routing hints to read-write transaction requests ([#14786](#14786)) ([54e0585](54e0585)) * **spanner:** cancel queries promptly on close and synchronize error handling ([#14887](#14887)) ([323a8f7](323a8f7)) * **spanner:** do not cache retryable errors in AbstractLazyInitializer ([#14752](#14752)) ([6a9ac76](6a9ac76)) * **spanner:** restore setCall overload for stream test compatibility ([#14885](#14885)) ([cfe728e](cfe728e)) * **spanner:** synchronize stream lifecycle and retry buffering in ResumableStreamIterator ([#14770](#14770)) ([4edefc5](4edefc5)) * **spanner:** trigger best-effort rollback on indeterminate commit failures ([#14831](#14831)) ([b9d1b95](b9d1b95)) * **storage:** preserve HTTP status for XML multipart upload errors so retries work ([#14804](#14804)) ([3c03034](3c03034)) * **storage:** send destination on gRPC copy when target BlobInfo is provided ([#14784](#14784)) ([43a67a0](43a67a0)) * **storage:** validate CRC32C on HTTP downloads that were not decompressed ([#14805](#14805)) ([61aa22f](61aa22f)) ### Documentation * **bigtable:** document that skipped large rows count towards row limit ([#14801](#14801)) ([91d60e0](91d60e0)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Description
This PR implements Phase 2 (Configuration & Options API) of the OpenTelemetry Client Metrics feature in the Java Cloud Storage SDK (
google-cloud-storage), building on top of PR #14398 (feat/storage-client-metrics-infra).Key Changes
Public Options API (
StorageOptions,GrpcStorageOptions,HttpStorageOptions):isEnableOtelMetrics()(default:false)isEnableOtelDebugMetrics()(default:false)getMeterProvider()returningio.opentelemetry.api.metrics.MeterProvider(fromopentelemetry-api, avoiding leaking SDK implementation types in public API surface)getMetricInterval()(default: 60 seconds; clamped to 60 seconds when using default Cloud Monitoring exporter)@BetaApi:setEnableOtelMetrics(boolean)setEnableOtelDebugMetrics(boolean)setMeterProvider(MeterProvider)setMetricInterval(Duration)with precondition check requiring positive, non-zero durations (and >= 60 seconds unless a custom MeterProvider or OpenTelemetry is configured).Feature Gates & Environment Variables (
StorageMetricsConfig):GCP_STORAGE_JAVA_ENABLE_OTEL_METRICSGCP_STORAGE_JAVA_ENABLE_OTEL_DEBUG_METRICSfalse).1/t/trueand0/f/false).Serialization Resiliency for Distributed Runtimes:
StorageOptionsis serialized across worker nodes:meterProvideras transient (non-serializable).readObject()when deserializing an instance configured with a custom MeterProvider, indicating fallback to the default provider.getMetricInterval(), clamped interval to minimum 60 seconds ifmeterProvider == null && !customOpenTelemetryConfiguredto ensure compatibility with Cloud Monitoring.Testing:
StorageOptionsTest:IllegalArgumentExceptionon null, zero, negative, or invalid (< 60s without custom provider) metric intervals.ByteArrayOutputStream/ObjectInputStream) ensuring transient meterProvider handling, fallback defaults, and interval clamping post-deserialization.[Generated-by: AI]