Repository navigation
feat(storage): add OpenTelemetry standard metrics for HTTP JSON and Veneer APIs - #18898
shradhakatyal wants to merge 7 commits into
Conversation
…d helpers Implement core OpenTelemetry standard metrics infrastructure for GCS: - Add _MetricsRecorder with the 9 standard metric instruments - Add custom latency (165 buckets) and byte-size (45 buckets) histogram views - Add classify_error_type for low-cardinality error.type mapping - Add normalize_url_template for bounded HTTP url.template routing - Add start_operation ContextVar for outermost logical operation tracking and nested call suppression [Generated-by: AI]
…branch coverage [Generated-by: AI]
…ncellations [Generated-by: AI]
- Add gcp.storage.client.retries counter and rename TTFB metric to gcp.storage.client.ttfb - Remove gcp.client.repo from _COMMON_ATTRIBUTES - Update _build_size_buckets to generate 48 buckets matching Go and Java SDKs - Align per-metric attributes across logical operation, RPC/HTTP attempt, error, retry, and TTFB recorders [Generated-by: AI]
…fy CONNECTION_ERROR - Scope histogram View registrations to meter_name="google.cloud.storage" - Guard normalize_url_template and _extract_server_address against malformed URIs and gRPC authority schemes - Consolidate CONNECTION_REFUSED, CONNECTION_RESET, and CONNECTIVITY error labels into CONNECTION_ERROR [Generated-by: AI]
…eneer APIs - Wire _MetricsRecorder lifecycle and meter_provider configuration into Client - Instrument _StorageConnection.api_request for per-attempt HTTP duration, TTFB, retries, and errors - Add logical operation instrumentation across Client, Bucket, Blob, ACL, Batch, HMACKeyMetadata, and BucketNotification - Defer Batch sub-request operations until Batch.finish() and track lazy HTTPIterator page metrics - Suppress internal background bucket metadata cache lookups from emitting user-facing metrics
![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 OpenTelemetry standard metrics across the Google Cloud Storage Python client, introducing metrics instrumentation, custom histogram views, canonical error classification, and route normalization. It integrates metrics recording into HTTP transport, logical operations (such as Bucket, Blob, ACL, and Batch), and background tasks, backed by comprehensive unit tests. The review feedback suggests falling back to self._client in Connection.api_request when the client argument is None to ensure metrics are consistently recorded, along with adding corresponding test cases.
| recorder = ( | ||
| None | ||
| if ( | ||
| getattr(self, "_IS_BATCH", False) is True | ||
| or _helpers._SUPPRESS_INTERNAL_METRICS.get() | ||
| ) | ||
| else _opentelemetry_metrics.get_active_recorder(client) | ||
| ) |
There was a problem hiding this comment.
In Connection.api_request, if the client argument is None, get_active_recorder(client) will return None and metrics will not be recorded, even if self._client has an active recorder. Consider falling back to self._client when client is None to ensure metrics are consistently recorded. Additionally, ensure there are test cases covering both when the client is explicitly provided and when it is omitted.
| recorder = ( | |
| None | |
| if ( | |
| getattr(self, "_IS_BATCH", False) is True | |
| or _helpers._SUPPRESS_INTERNAL_METRICS.get() | |
| ) | |
| else _opentelemetry_metrics.get_active_recorder(client) | |
| ) | |
| recorder = ( | |
| None | |
| if ( | |
| getattr(self, "_IS_BATCH", False) is True | |
| or _helpers._SUPPRESS_INTERNAL_METRICS.get() | |
| ) | |
| else _opentelemetry_metrics.get_active_recorder(client or self._client) | |
| ) |
References
- When testing functions that accept an optional client or resource parameter, ensure there are test cases covering both when the client is explicitly provided and when it is omitted (defaulting to None).
Summary
Adds experimental OpenTelemetry standard metrics support for the HTTP JSON transport and logical Veneer APIs in
google-cloud-storage, aligned with the unified 10-metric GCS client schema.Key Changes
client.py): Addsenable_metrics,enable_debug_metrics, andmeter_providerparameters toClient.__init__, wires_MetricsRecorderinitialization, and calls_metrics_recorder.shutdown()inClient.close()._http.py): Instruments_StorageConnection.api_requestfor per-attempthttp.client.request.duration,gcp.storage.client.attempts,gcp.storage.client.ttfb,gcp.storage.client.retries, andgcp.storage.client.errorswith boundedurl.templatenormalization._helpers.py,client.py,bucket.py,blob.py,acl.py,hmac_key.py,notification.py): Instruments user-facing Veneer methods to emit outermostgcp.client.request.durationandgcp.storage.client.operationswhile normalizing expected404existence/lookup checks toOK.batch.py,client.py,bucket.py,blob.py): DefersBatchsub-request logical operation metrics untilBatch.finish()and tracks per-page lazyHTTPIteratorfetches (ListBuckets,ListObjects,ListHmacKeys,ListNotifications)._bucket_metadata_cache.py): Suppresses background bucket metadata cache lookups used for tracing attributes from emitting user-facing metrics.