Repository navigation
feat(storage): add OpenTelemetry tracing support for gRPC bidi APIs - #18397
Open
shradhakatyal wants to merge 57 commits into
Open
shradhakatyal wants to merge 57 commits into
shradhakatyal wants to merge 57 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
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.
shradhakatyal
force-pushed
the
feat/zb-traces
branch
from
September 17, 2026 09:03
50474ec to
e572fb0
Compare
shradhakatyal
marked this pull request as ready for review
September 28, 2026 05:26
cpriti-os
self-requested a review
September 28, 2026 06:02
…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]
shradhakatyal
force-pushed
the
feat/zb-traces
branch
from
September 29, 2026 09:37
bb092c8 to
6a01c38
Compare
Contributor
Author
|
/gcbrun 6a01c38 |
Contributor
|
@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]
Contributor
Author
|
/gcbrun (bd89f56) |
cpriti-os
requested changes
Oct 19, 2026
Contributor
Author
|
/gcbrun (bd89f56) |
[Generated-by: AI]
[Generated-by: AI]
Contributor
Author
|
/gcbrun (7b87f4b) |
Contributor
Author
Fixed, please check. |
🤖 I have created a release *beep* *boop* --- <details><summary>google-shopping-merchant-reviews: 0.6.2</summary> ## [0.6.2](googleapis/google-cloud-python@google-shopping-merchant-reviews-v0.6.1...google-shopping-merchant-reviews-v0.6.2) (2026-10-23) ### Features * **google/shopping/merchant/reviews/v1:** add google-shopping-merchant-reviews ([googleapis#18801](googleapis#18801)) ([c4a6a21](googleapis@c4a6a21)) </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>
…googleapis#18683) Replaces live-instance DDL system tests in `test_dbapi.py` with fast, deterministic in-memory mock server tests in `test_dbapi_ddl.py`. Previously, `test_dbapi.py` dynamically created and dropped ephemeral tables (e.g. `Singers`, `DdlExecute`, `DdlExecuteMany`, `JsonDetails`) against live Cloud Spanner instances to verify client-side DDL buffering and flushing. Each table creation/drop triggered a long-running operation (LRO), adding ~1.5 to 2.0 minutes of latency and causing test flakiness. Changes: - Add `test_dbapi_ddl.py` in `tests/mockserver_tests/` with 9 mock server tests covering immediate autocommit DDL execution, non-autocommit DDL buffering, multi-statement DDL batches, DDL flushing before subsequent queries and DML, and client-side rejection of DDL in `executemany()`. - Remove 7 redundant DDL tests from `tests/system/test_dbapi.py`. - Update `test_autocommit_with_json_data` in `test_dbapi.py` to reuse the existing `all_types` table with row cleanup instead of creating and dropping a dedicated table. - Eliminate all dynamic table churn in DB-API system tests, saving ~1.5 to 2.0 minutes in standard CI while executing the replacement mock tests in < 1 second.
…ase churn (googleapis#18693) Consolidate administrative system tests in `test_database_api.py` to eliminate redundant database creations, long-running operations (LROs), and schema churn: 1. Consolidate Create & Update Tests: - Combine PITR database creation and subsequent DDL update into a single test (`test_create_and_update_database_pitr`), asserting both client properties and Information Schema catalog values. - Combine default leader creation and subsequent update into a single multi-region database test (`test_create_and_update_database_with_default_leader`). - Merge `test_update_ddl_w_operation_id` into `test_create_database_and_update_ddl`. 2. Consolidate Database Roles & Permissions: - Merge `test_create_role_grant_access_success` and `test_list_database_role_success` into `test_create_role_grant_access_and_list_roles`. - Strip the 20-table schema down to only the single required table (`contacts`). - Eliminate redundant teardown DDL (revoking permissions and dropping roles) immediately prior to database deletion. - Fix a lazy evaluation bug where the parent role query was never iterated and therefore never dispatched over the wire. 3. Remove Pure Server-Side DDL Parser Tests: - Drop `test_create_database_pitr_invalid_retention_period` and `test_update_ddl_w_pitr_invalid`, which tested Spanner backend syntax rejection of '0d' rather than client validation logic. 4. Fix Multi-Region Database Timeout: - Replace the hardcoded 30-second timeout on multi-region database creation with `DBAPI_OPERATION_TIMEOUT` (240s) to prevent CI flakes under load. Savings: - Eliminates 5 full database creations and drops (including 1 multi-region DB). - Eliminates ~8 long-running DDL operations.
🤖 I have created a release *beep* *boop* --- ## 0.1.0 (2026-10-26) ### Features * **google/cloud/observability/v1:** add google-cloud-observability ([googleapis#18807](googleapis#18807)) ([0a446d5](googleapis@0a446d5)) --- 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>
… engine with tidy data (googleapis#18852)
Contributor
Author
Done! |
Contributor
Author
|
/gcbrun (1df0886) |
…es (googleapis#18408) Optimize request ID generation and RPC retry hot paths across Database, Transaction, Snapshot, and Batch: - Use `int.from_bytes(os.urandom(8), "big")` for process-level random ID generation and pre-format the static process prefix once at import time. - Cache per-database request ID prefixes directly in `instance.__dict__` via a non-data descriptor (`_CachedPrefixDescriptor`), lazily computing on first access and invalidating when `_channel_id` changes. - Inline `Database.with_error_augmentation` and construct gRPC metadata lists with unpacking (`[*prior_metadata, ...]`) instead of intermediate list copies and `.append()`. - Replace thread-safe `AtomicCounter` instances with local integer counters (`attempt = 0`) inside sequential retry closures, and pass active OpenTelemetry spans directly to avoid repeated `contextvars` lookups. - Eliminate transient `functools.partial` allocations and redundant `*args, **kwargs` unpacking inside retry closures. - Make `wrap_with_request_id` idempotent on repeated wrapping and replace stale request IDs when retrying failed calls.
cpriti-os
reviewed
Nov 2, 2026
chandra-siri
approved these changes
Nov 2, 2026
…oogleapis#18920) When signing AWS STS GetCallerIdentity requests against regional FIPS endpoints (`sts-fips.{region}.amazonaws.com`), the `Host` header must remain `sts-fips.{region}.amazonaws.com`, whereas the AWS SigV4 credential scope and signing key derivation require the canonical service name (`sts`). Previously, `_generate_authentication_header_map` derived `service_name = host.split(".")[0]`, which produced `sts-fips` and caused AWS STS to reject the signed request with `SignatureDoesNotMatch: Credential should be scoped to correct service: 'sts'`. This change strips any trailing `-fips` suffix from the extracted hostname label when computing `service_name`. ## Manual Testing End-to-end reproduction script and observed pre-fix / post-fix output against GCP STS (`https://sts.googleapis.com/v1/token`) and AWS STS (`https://sts-fips.us-east-2.amazonaws.com`): - https://paste.googleplex.com/6755399436486780
🤖 I have created a release *beep* *boop* --- ## [2.62.1](googleapis/google-cloud-python@google-auth-v2.62.0...google-auth-v2.62.1) (2026-11-02) ### Bug Fixes * **auth:** strip -fips suffix from AWS service name in SigV4 signing ([googleapis#18920](googleapis#18920)) ([2e1899e](googleapis@2e1899e)) * **auth:** use an unencrypted key for mTLS certificate rotation ([googleapis#18709](googleapis#18709)) ([8166325](googleapis@8166325)) --- 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>
…ppend span Aligns the payload size attribute on Storage.AsyncAppendableObjectWriter.append with the cross-SDK trace attributes specification (gcp.storage.payload.size_bytes). [Generated-by: AI]
…leapis#18641) ## Summary `AsyncAppendableObjectWriter` overloads `generation`: `None` / `0` creates a new object via `WriteObjectSpec(if_generation_match=generation)`, while `N > 0` reopens an existing object via `AppendObjectSpec(generation=N)`. There was no way to pass `if_generation_match=N` (`N > 0`) on `WriteObjectSpec` (overwrite only if the current generation matches `N`) or other write preconditions (`if_generation_not_match`, `if_metageneration_match`, `if_metageneration_not_match`). This adds a `Preconditions` dataclass (`if_generation_match`, `if_generation_not_match`, `if_metageneration_match`, `if_metageneration_not_match`) accepted via `preconditions: Optional[Preconditions] = None` on `AsyncAppendableObjectWriter.__init__`, `AsyncAppendableObjectWriter.from_blob`, and `_AsyncWriteObjectStream.__init__`: - Generation preconditions (`if_generation_match`, `if_generation_not_match`) require `generation=None`. Combining them with `generation` raises `ValueError`, including `from_blob` when `blob.generation` is set. To overwrite a fetched Blob, pass `generation=None` to `from_blob`, for example with `Preconditions(if_generation_match=blob.generation)`. `generation=0` without `preconditions` still means `if_generation_match=0`. - Creating or overwriting (`generation` is `None` or `0`) sets all 4 fields on `WriteObjectSpec`. Appending to an existing object (`generation > 0`) sets the metageneration preconditions on `AppendObjectSpec`. Metageneration preconditions cannot be combined with `write_handle` (raises `ValueError`), because GCS only checks them when no write handle is sent. - After the writer creates a generation, reconnects send `AppendObjectSpec` without preconditions, because they describe the object that was replaced. Motivation: fsspec/gcsfs#1101 (compare-and-swap writes in gcsfs). ## Test plan ``` cd packages/google-cloud-storage PYTHONPATH=$PWD python -m pytest tests/unit/asyncio/test_async_write_object_stream.py tests/unit/asyncio/test_async_appendable_object_writer.py # 86 passed ruff check --select I --target-version=py310 --line-length=88 <changed files> ruff format --check --target-version=py310 --line-length=88 <changed files> flake8 <changed files> ``` > This change was authored with an AI assistant; the description above was reviewed by the author.
…thread (googleapis#18628) ## Summary Hash read chunks of 1 MiB or more on a small worker pool instead of the asyncio event-loop thread, and write each chunk to its buffer only after its checksum matches. On `main`, `google_crc32c.value()` is 10-12% of the event-loop thread's samples at 2 GB/s per process (py-spy, 1 process x 16 streams), time during which no other stream on that loop is served. `google-crc32c` 1.9.0 and later release the GIL while hashing `bytes` of 1 MiB or more, so the workers run in parallel with the loop thread; this PR raises the minimum `google-crc32c` to 1.9.0. ## Changes - **`asyncio/retry/reads_resumption_strategy.py`:** - Chunks of at least `_CRC32C_OFFLOAD_MIN_BYTES` (defaults to `getattr(google_crc32c, "GIL_RELEASE_THRESHOLD", 1024 * 1024)`; configurable via `GCS_PYTHON_CLIENT_CRC32C_OFFLOAD_MIN_BYTES`, where `0` verifies every chunk inline) that are not part of a full-object read are hashed on a 2-thread `ThreadPoolExecutor`; smaller chunks and full-object reads (which feed the rolling checksum) are hashed inline as before. - Offloaded chunks (and any chunks arriving behind one) are held in `state["pending_checksums"]` and written in arrival order once their checksum matches (`verify_pending_checksums`). `next_expected_offset` advances on receipt and `bytes_written` only on write, so a retry resumes right after the last verified byte. - On a chunk checksum mismatch (which raises `DataCorruption`, as inline verification does) or an exception on the worker (which is re-raised), `discard_pending_checksums` drops every held chunk, cancels queued hashes and rewinds each read to its written end. `recover_state_on_failure` verifies held chunks before a stream is reopened. - The pool is replaced in forked children (`os.register_at_fork`); once interpreter shutdown refuses new work, chunks are hashed inline. - **`setup.py`, `testing/constraints-3.10.txt`, `storage-integration.yaml`:** require `google-crc32c >= 1.9.0`, the first release that hashes large `bytes` without holding the GIL. - **`asyncio/async_multi_range_downloader.py`:** `download_ranges` writes verified chunks after each received message (holding at most `_CRC32C_MAX_PENDING = 32`), drains fully when an attempt ends or fails with a stream error, discards held chunks on cancellation or `DataCorruption`, and documents all of this. ## Usage Enabled automatically for non-full-object read chunks of at least 1 MiB (`getattr(google_crc32c, "GIL_RELEASE_THRESHOLD", 1024 * 1024)`). Set `GCS_PYTHON_CLIENT_CRC32C_OFFLOAD_MIN_BYTES` to override the threshold in bytes, or `0` to verify every chunk inline on the event-loop thread. ## Memory `download_ranges` bounds `pending_checksums` to `_CRC32C_MAX_PENDING = 32` chunks per call before draining on the event loop. For 2 MiB chunks, each held `_PendingChunk` retains both the `data` slice and the enclosing `BidiReadObjectResponse`, bounding pending memory per `download_ranges` call to at most ~128 MiB with the generated `proto-plus`/`upb` parser (~4 MiB per chunk across the `upb` arena copy and `.content` `bytes` copy) or ~64 MiB with the zero-copy parser in googleapis#18627 (~2 MiB per chunk sharing the gRPC buffer). ## Tests `tests/unit/asyncio/retry/test_reads_resumption_strategy.py` and `tests/unit/asyncio/test_async_multi_range_downloader.py`: offload threshold, zero threshold and full-object exclusion; in-order writes behind a pending chunk; mismatch on an offloaded chunk (nothing from it on is written, offsets rewound, the retry requests the bytes again); a worker exception (queue emptied, queued futures cancelled); drain before a retry and at the end of an attempt; discard on cancellation; fork and shutdown behavior of the pool. ``` nox -s unit lint cover ``` ## Benchmark Setup: GCE `c4-standard-192`, Rapid bucket in the same zone, 8 processes x 16 concurrent `AsyncMultiRangeDownloader` streams (128 streams total), `4 x 16 MiB` ranges per `download_ranges()` call into preallocated buffers, 5 s warmup + 15 s timed window per round, 8 interleaved rounds per arm. ### 1. Off-Loop CRC32C Only vs. `main` (Neutral Alone) | Arm | Mean (GB/s) | Median (GB/s) | Min - Max (GB/s) | vs. `main` (Mean / Median) | | :--- | :---: | :---: | :---: | :---: | | `main` (generated parser + inline CRC32C) | 9.62 | 9.83 | 8.25 - 10.70 | - | | `offloop` only (`GCS_PYTHON_CLIENT_ZERO_COPY_BIDI_READ=0`) | 9.64 | 9.96 | 7.32 - 10.85 | +0.2% / +1.3% | With the generated `proto-plus`/`upb` parser active, each 2 MiB chunk is still copied twice on the event-loop thread under the GIL (~254 us per message), so the loop remains saturated and offloading CRC32C alone is neutral. ### 2. Zero-Copy Only vs. Zero-Copy + Off-Loop CRC32C (Additive on Top of googleapis#18627) | Arm | Mean (GB/s) | Median (GB/s) | Min - Max (GB/s) | vs. `main` (Mean / Median) | vs. Zero-Copy Only (Mean / Median) | | :--- | :---: | :---: | :---: | :---: | :---: | | `main` (generated parser + inline CRC32C) | 9.47 | 9.48 | 8.07 - 10.51 | - | - | | `zero-copy` only (googleapis#18626 + googleapis#18627, inline CRC32C) | 11.24 | 11.07 | 9.62 - 13.73 | +18.6% / +16.7% | - | | `zero-copy + offloop` (googleapis#18626 + googleapis#18627 + this PR) | 13.30 | 12.96 | 10.35 - 16.02 | +40.4% / +36.7% | +18.3% / +17.1% | Once googleapis#18627 removes the two `upb`/`proto-plus` copies from the event-loop thread (254 us -> 4.7 us per 2 MiB message, +18.6% mean / +16.7% median over `main`), synchronous `google_crc32c.value()` (~100-200 us per 2 MiB chunk) becomes the largest remaining per-message work on the loop thread; offloading >= 1 MiB chunks onto the worker pool adds another +18.3% mean / +17.1% median (+40.4% mean / +36.7% median total over `main`).
…xtend() (googleapis#18626) ## Summary `google_crc32c.value()`, `extend()` and `Checksum.update()` accept any contiguous bytes-like object (`memoryview`, `bytearray`, `array`, `bytes`) instead of `bytes` only, and the C extension releases the GIL for inputs of 1 MiB or more whose memory belongs to a `bytes` object (a `bytes` or a `memoryview` of one). That threshold is exported as `google_crc32c.GIL_RELEASE_THRESHOLD` (1 MiB). Motivation: the storage client wants to verify chunks it receives as `memoryview` slices of the gRPC message without copying them into `bytes` first; today `value(memoryview(...))` raises `TypeError: argument 1 must be read-only bytes-like object`. ## Changes - `src/google_crc32c/_crc32c.c`: parse the data argument with `y*` into a `Py_buffer` (released after hashing) instead of `y#`, which rejects every exporter with a release function. The 1 MiB GIL release applies when the exported memory is owned by a `bytes` object; a `bytearray` or other writable owner keeps the GIL, since another thread could write to it mid-hash. - `src/google_crc32c/python.py`: the pure-Python implementation hashes the raw bytes of the same inputs (a multi-byte `array` or cast view is hashed byte for byte, as the C extension does, instead of item by item). `Checksum(initial_value)` no longer compares the initial value with `b""`, which raised for numpy arrays. - `GIL_RELEASE_THRESHOLD` (1 MiB) is exported from `_crc32c.c`, `cext.py`, `python.py` and the package, so callers such as the storage client (googleapis#18628) can size the chunks they hash off the event loop. - Docstrings in `cext.py`, `_checksum.py` and `python.py` say bytes-like. - `tests/test___init__.py`: `value`/`extend` on `bytes`, `bytearray`, `memoryview` (sliced, writable, read-only, cast, 2-D) and `array` inputs, `BufferError` on non-contiguous views, and GIL probes: held for `bytearray` and read-only views of it, released for `bytes`, `memoryview(bytes)` and a slice of it. ## Tests ``` nox -s unit lint cover ``` 184 passed, 4 skipped (numpy not installed) with the compiled extension; 89 passed, 99 skipped, 100% coverage in pure-Python mode (`nox -s unit cover`). ## Notes - Behavior change in the pure-Python implementation for multi-byte `array`/cast inputs (previously coerced item by item). It now matches the C extension. - The GIL rule (`bytes`-owned memory only) is a choice: `hashlib` and `zlib.crc32` release the GIL for any buffer. Holding it for writable owners avoids hashing memory another thread may be writing to; happy to drop that restriction if you prefer parity with `hashlib`.
This PR was created by running `zizmor --fix=all` I also had to update all comments for `actions/setup-python@` to point to v7.0.0 to address this error ``` warning[ref-version-mismatch]: action's hash pin has mismatched or missing version comment --> .github/workflows/version_scanner.yml:26:79 | 26 | uses: actions/setup-python@5fda3b9 # v7 | ------------------------------------------------------------------- ^^ points to commit 6d4fd22b2046 | | | is pointed to by tag v7.0.0 | = note: audit confidence → High = note: this finding has an auto-fix ```
…ync reads (googleapis#18627) ## Summary An opt-in parser for `BidiReadObjectResponse` data messages that reads each message in place over the `bytes` object gRPC hands to the deserializer, instead of copying every chunk twice on the event-loop thread (`upb` arena decode, then `.content` into a new `bytes`; 257 us per 2 MiB message vs. 5 us with this parser). Off by default; set `GCS_PYTHON_CLIENT_ZERO_COPY_BIDI_READ=1` (or `true`/`yes`/`on`) to enable it. - Existing callers passing a `.write()` destination (`io.BytesIO`, binary files, custom streams) keep receiving `bytes`. A destination receives the chunk as a `memoryview` of the received message only when both the parser is enabled and the caller passes `download_ranges(..., accepts_memoryview=True)`; either switch alone leaves it `bytes`. - Callers passing a writable buffer (`bytearray` or a writable `memoryview` slice) as the destination get each chunk copied straight into it, without an intermediate `bytes` when the parser is enabled and the installed `google-crc32c` accepts `memoryview`. - Works with any `google-crc32c`: when the installed release accepts `memoryview` (googleapis#18626), chunks are views of the wire buffer; with `google-crc32c <= 1.9.0` they are `bytes` slices, which still saves the `upb` arena copy. - When opted in via `GCS_PYTHON_CLIENT_ZERO_COPY_BIDI_READ=1`, `_AsyncReadObjectStream.recv()` and `DataCorruption.response` carry the duck-typed `_ZeroCopyBidiReadObjectResponse` on data messages instead of the generated `proto-plus` wrapper. ## Changes - **`asyncio/_zero_copy_bidi_read.py` (new):** - `deserialize(buf)` walks the top-level wire tags of data responses (`object_data_ranges`, `read_handle`), slices `checksummed_data.content` as `memoryview(buf)[start:i]` (or `buf[start:i]` when `google-crc32c` only takes `bytes`), and parses `ReadRange` and `BidiReadHandle` sub-messages with the generated parser. Varint, int64, tag, and length-prefix handling match `upb`. - A response carrying `metadata`, input that raises `(_Malformed, IndexError, DecodeError)`, or a buffer that is not an immutable `bytes` object is handed to `BidiReadObjectResponse.deserialize`, which returns the real message or raises its usual `DecodeError`. - `is_enabled()` reads `GCS_PYTHON_CLIENT_ZERO_COPY_BIDI_READ`; `is_supported()` probes both `google_crc32c.value(memoryview(...))` and `google_crc32c.Checksum().update(memoryview(...))` once per process. - `wrapped_rpc(transport)` builds and caches the transport's `BidiReadObject` callable with `deserialize` swapped in and the same error mapping and `x-goog-api-client` header as `_wrapped_methods`. - **`asyncio/async_read_object_stream.py`:** `_AsyncReadObjectStream` uses `wrapped_rpc` when enabled, the generated method otherwise. - **`asyncio/async_multi_range_downloader.py`:** - `download_ranges(..., accepts_memoryview=False)`: `.write()` destinations are wrapped in `_BytesWriter`, which converts `memoryview` chunks to `bytes`, unless the caller opts in. - Writable, C-contiguous buffer-protocol destinations (`bytes_to_read > 0`) are wrapped in `_BufferWriter`, which copies each chunk into the first `bytes_to_read` bytes of the buffer, raises `BufferError` if fewer than `bytes_to_read` bytes are returned, and releases its view when `download_ranges` exits. - **`asyncio/retry/reads_resumption_strategy.py`:** `_write_chunk` checks `bytes_written + len(data) > initial_length` before writing to `user_buffer`, so server over-delivery raises `DataCorruption` and closes the stream before touching the caller's buffer. ## Tests `tests/unit/asyncio/test_zero_copy_bidi_read.py`, `test_async_read_object_stream.py`, `test_async_multi_range_downloader.py`: parametrized equivalence against the generated parser (`_pb` / `HasField`, unknown fields, split sub-messages, over-long varints and length prefixes), malformed and fallback inputs, the environment switch and cached `value` / `Checksum.update` probe, `_BytesWriter` and `_BufferWriter` validation / retry / checksum-mismatch / full-object / view-release paths, an unpatched `google-crc32c` check, and an in-process `grpc.aio` server header parity test. 100% line and branch coverage of `_zero_copy_bidi_read.py` under both `upb` and `PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python`. ``` nox -s unit lint cover ``` ## Benchmark GCE `c4-standard-192`, Rapid bucket in the same zone, 8 processes x 16 concurrent `AsyncMultiRangeDownloader` streams, `4 x 16 MiB` ranges per `download_ranges()` call into preallocated buffers (`accepts_memoryview=True`), `google-crc32c` with googleapis#18626; 8 interleaved rounds per arm, aggregate GB/s: | Arm | Mean | Median | Min - Max | | :--- | :---: | :---: | :---: | | `main` | 9.47 | 9.48 | 8.07 - 10.51 | | this PR, parser enabled | 11.24 (+18.6%) | 11.07 (+16.7%) | 9.62 - 13.73 |
Resolve merge conflicts in AsyncAppendableObjectWriter.open and AsyncMultiRangeDownloader.download_ranges while preserving OpenTelemetry bidi tracing spans, Preconditions support, and off-loop CRC32C checks. [Generated-by: AI]
shradhakatyal
requested review from
julianxu-goog
and removed request for
a team
November 3, 2026 08:23
Resolve merge conflicts in async_multi_range_downloader.py and async_read_object_stream.py with opt-in zero-copy BidiReadObjectResponse parsing (googleapis#18627) while preserving OpenTelemetry bidi tracing spans. [Generated-by: AI]
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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