Repository navigation
feat(storage): add OpenTelemetry tracing support for gRPC bidi APIs - #18397
shradhakatyal wants to merge 9 commits into
Conversation
![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 OpenTelemetry tracing support for zonal buckets (Rapid Storage) in async gRPC classes. It refactors tracing span creation to support both synchronous and asynchronous context managers, injects traceparent metadata into gRPC requests, and updates the bucket metadata cache to support asynchronous gRPC fetches. The review feedback recommends defensively checking for __aenter__ and __aexit__ in _TraceSpanHelperContext to ensure compatibility with synchronous context managers (such as mocked spans in unit tests) and refactoring the project number extraction in update_from_bucket to use getattr more idiomatically.
50474ec to
e572fb0
Compare
…adata_cache.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…te leakage across unit tests
[Generated-by: AI]
…ache - Add T2 trace spans to AsyncAppendableObjectWriter.simple_flush and finalize - Prevent nested T2 spans during internal stream reconnects and finalize_on_close - Simplify _is_otel_traces_enabled() and clean up fixture teardown - Avoid duplicate exception events on GoogleAPICallError in _TraceSpanContext - Track background asyncio tasks in BucketMetadataCache and refresh cache on GetBucket - Add unit tests for 403/404 cache handling and span error recording [Generated-by: AI]
bb092c8 to
6a01c38
Compare
|
/gcbrun 6a01c38 |
|
@shradhakatyal , can you check the performance regression caused because of this PR - screen/228ingun6e470 |
… is disabled - Add _NoOpTraceSpanContext singleton and fast-path create_trace_span and create_trace_span_helper when tracing is disabled - Add __slots__ to _TraceSpanContext and _TraceSpanHelperContext and avoid async coroutine wrapping for sync span context managers - Replace @asynccontextmanager in AsyncAppendableObjectWriter._trace_span with slotted _WriterTraceSpanContext and no-op fast path - Fast-path inject_traceparent_to_metadata without copying metadata when disabled and cache TraceContextTextMapPropagator - Avoid duplicate cache lookup in BucketMetadataCache.get_or_queue_fetch [Generated-by: AI]
|
/gcbrun (bd89f56) |
|
/gcbrun (bd89f56) |
[Generated-by: AI]
[Generated-by: AI]
|
/gcbrun (7b87f4b) |
Fixed, please check. |
|
nit: modify the title to "feat(storage): add OpenTelemetry tracing support for gRPC bidi APIs" reason: grpc bidi includes Rapid Storage ( zonal buckets) & they also started supporting Standard buckets. |
cpriti-os
left a comment
cpriti-os
left a comment
There was a problem hiding this comment.
LGTM, thanks for addressing the feedback!
…tracing [Generated-by: AI]
shradhakatyal
changed the title
Done Co-authored by AI Agent |
Done! |
|
/gcbrun (1df0886) |
| async with self._trace_span( | ||
| "Storage.AsyncAppendableObjectWriter.append", | ||
| attributes={"gcp.storage.append.bytes": len(data)}, | ||
| ): |
There was a problem hiding this comment.
In the cross-SDK trace attributes spec, the standard attribute name for payload size on gRPC spans is gcp.storage.payload.size_bytes — could we use gcp.storage.payload.size_bytes here instead of gcp.storage.append.bytes?
This PR adds OpenTelemetry tracing support for gRPC bidi APIs (Rapid Storage and Standard buckets) across the asynchronous gRPC client and streaming classes (
AsyncGrpcClient,AsyncAppendableObjectWriter, andAsyncMultiRangeDownloader).The implementation aligns with existing regional bucket tracing conventions:
Storage.<Class>.<methodName>.ENABLE_GCS_PYTHON_CLIENT_OTEL_TRACESandDISABLE_GCS_PYTHON_CLIENT_OTEL_BUCKET_METADATA).rpc.system="grpc"and attaches App-Centric Observability (ACO) attributes (gcp.resource.destination.idandgcp.resource.destination.location).traceparentheaders directly into gRPC request metadata tuples.Fixes: b/489239033