Repository navigation
feat(storage): add OpenTelemetry standard metrics recorder, views, and helpers - #18753
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]
![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 introduces comprehensive OpenTelemetry metrics instrumentation for the Google Cloud Storage client, including custom histogram buckets, canonical error classification, URL template normalization, logical operation context tracking, and a metrics recorder class, along with extensive unit tests. The review feedback suggests several robustness improvements to the error classification logic, such as validating that exception args are iterable, handling mock objects gracefully during MRO and gRPC status name resolution, performing more precise module prefix matching, and classifying concurrent.futures.CancelledError as a cancellation.
…branch coverage [Generated-by: AI]
…ncellations [Generated-by: AI]
cpriti-os
left a comment
cpriti-os
left a comment
There was a problem hiding this comment.
Please address the comments regarding the standard metric inventory, base attributes, size histogram buckets, and per-metric attributes.
- 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]
|
Re: review comment Done — addressed all four inline comments in Co-authored by AI Agent |
…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]
|
/gcbrun (cdea802) |
|
/gcbrun (0e344fe) |
cpriti-os
left a comment
cpriti-os
left a comment
There was a problem hiding this comment.
Thanks for addressing the earlier feedback! Just two small follow-ups below.
- Drop _RPC_V2_METHODS and format all short operation names under google.storage.v2.Storage/ - Record attempt-scoped gcp.storage.client.ttfb even when start_operation(..., suppress=True) is active
|
/gcbrun (2d7c269) |
Summary
Implements the core OpenTelemetry standard metrics module (
_opentelemetry_metrics.py) forgoogle-cloud-storage, aligned with the unified 10-metric GCS client schema.Key Changes
_MetricsRecorder): Manages all 10 standard instruments (gcp.client.request.duration,gcp.storage.client.operations,rpc.client.call.duration,http.client.request.duration,gcp.storage.client.attempts,gcp.storage.client.retries,gcp.storage.client.errors,gcp.storage.client.request.body.size,gcp.storage.client.response.body.size,gcp.storage.client.ttfb), per-metric attribute scoping, and customMeterProviderflushing onshutdown().get_histogram_views): Defines custom bucket boundaries for latency (165 buckets) and byte size (48 buckets matching Go and Java SDKs).classify_error_type,normalize_url_template,normalize_logical_operation,resolve_rpc_method): Maps exceptions, HTTP/gRPC status codes, URLs, and RPC paths to bounded low-cardinality attributes.start_operation,record_pending_retry,handle_unrecorded_retryable_failure): Tracks outermost user-invoked Veneer methods and retry state viaContextVar.tests/unit/test__opentelemetry_metrics.pywith 100% statement and branch coverage.[Generated-by: AI]