Repository navigation
perf(spanner): short-circuit trace_call when tracing is inactive - #18356
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 a fast-path optimization in OpenTelemetry tracing for Google Cloud Spanner, bypassing attribute dictionary construction and span creation when a no-op tracer is active. It also expands type checking for observability options to support general mappings and adds comprehensive unit tests. The review feedback suggests further simplifying the fast-path implementation to eliminate nested context manager overhead when tracing is inactive, and recommends streamlining type checks by using isinstance(observability_options, Mapping) instead of checking for both dict and Mapping.
de84a9d to
ab4c846
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a fast-path optimization in trace_call to bypass attribute dictionary construction and span creation when a no-op tracer is active or no TracerProvider is registered. Feedback on the changes highlights two critical issues: first, the helper _is_tracer_noop should check _real_tracer instead of _tracer on ProxyTracer to correctly handle lazy initialization; second, the unit tests should assign to the private _tracer attribute instead of the read-only _real_tracer property to avoid raising an AttributeError at runtime.
Skip building the trace attribute dictionary and creating spans in `trace_call` when OpenTelemetry is not configured or uses a no-op tracer. This reduces overhead by ~4.8 µs per call (~10 µs per query). If a valid ambient span is present in the caller's context, a child span is still started on the no-op tracer to prevent Spanner-internal attributes and events from attaching to the parent span. Also update `observability_options` validation to check `Mapping`.
ab4c846 to
2b5821a
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a fast-path optimization in the OpenTelemetry tracing implementation for Google Cloud Spanner. When no TracerProvider is registered or a no-op tracer is active, the code now skips attribute dictionary construction and span creation entirely, yielding a non-recording span or an invalid span instead. Additionally, observability_options type checking was updated to support any Mapping subclass. Comprehensive unit tests were added to verify these changes under various tracing configurations. There are no review comments, and I have no feedback to provide.
olavloite
added
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
|
Could you please share the benchmark? The code path I see is one trace_call per single-use query, so I'm curious what makes up the ~10 µs. |
Many of these changes are so small that we do not run separate benchmarks for them (this is one such example). Instead, this is more about shaving away a few microseconds everywhere you can find them, as in a single-threaded language like Python, that makes a much bigger difference for overall throughput than in multi-threaded languages. The 10us in this case purely an estimate. The overall picture is clearer when you take all of these together and compare with what the performance was before (see screenshots shared separately). |
🤖 I have created a release *beep* *boop* --- ## [3.72.0](google-cloud-spanner-v3.71.0...google-cloud-spanner-v3.72.0) (2026-11-12) ### Features * **dbapi:** wire timeout parameter through Connection to execute_sql ([#18484](#18484)) ([98fc304](98fc304)) * declare Python3.15 support ([8775794](8775794)) * **spanner:** auth login support for Spanner Omni endpoints ([#18273](#18273)) ([9e0548d](9e0548d)) * update API sources and regenerate ([#18797](#18797)) ([be4d954](be4d954)) ### Bug Fixes * **google-cloud-spanner:** clear buffered DDL statements on rollback and flush on autocommit ([#18636](#18636)) ([15d6a2c](15d6a2c)) * **google-cloud-spanner:** do not replay DDL statements on transaction retry ([#18625](#18625)) ([78b9da2](78b9da2)) * **spanner:** correctly wrap async streaming calls in metrics interceptor ([#18581](#18581)) ([2d19b0a](2d19b0a)) * **spanner:** eliminate duplicate metrics capture and measure full streaming latency ([#18605](#18605)) ([a7678fd](a7678fd)) * **spanner:** record streaming operation completion when stream finishes ([#18704](#18704)) ([ddeae47](ddeae47)) * **spanner:** release transaction lock if inline begin fails ([#18409](#18409)) ([dd24029](dd24029)) * **spanner:** stop deleting multiplexed sessions on rotation and close ([#18332](#18332)) ([c38e960](c38e960)) ### Performance Improvements * **spanner:** add fast-path for multiplexed session acquisition ([#18317](#18317)) ([9569b39](9569b39)) * **spanner:** avoid allocating empty RequestOptions on read and query paths ([#18422](#18422)) ([3743ff0](3743ff0)) * **spanner:** build ExecuteSqlRequest on the raw protobuf message ([#18379](#18379)) ([6d91fec](6d91fec)) * **spanner:** eliminate redundant tracing span in run_in_transaction ([#18357](#18357)) ([181e26a](181e26a)) * **spanner:** optimize built-in metrics hot path and harden concurrency ([#18329](#18329)) ([f2a791f](f2a791f)) * **spanner:** optimize query option merging and prevent in-place mutation ([#18358](#18358)) ([8cb3c6a](8cb3c6a)) * **spanner:** optimize query parameter encoding with direct type dispatch ([#18359](#18359)) ([f06497c](f06497c)) * **spanner:** optimize query result decoding and type resolution ([#18330](#18330)) ([ce72986](ce72986)) * **spanner:** optimize request ID header generation and retry closures ([#18408](#18408)) ([ad03b95](ad03b95)) * **spanner:** optimize row popping in StreamedResultSet ([#18316](#18316)) ([e66103d](e66103d)) * **spanner:** optimize single-chunk results and single-row lookups ([#18602](#18602)) ([05b3775](05b3775)) * **spanner:** prune lock and begin event allocations on single-use snapshots ([#18420](#18420)) ([ab56473](ab56473)) * **spanner:** short-circuit trace_call when tracing is inactive ([#18356](#18356)) ([58031be](58031be)) * **spanner:** support PartialResultSet.last with background stream draining ([#18320](#18320)) ([80efd63](80efd63)) ### Documentation * fix typos ([#18979](#18979)) ([5c8baeb](5c8baeb)) --- 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>
Skip building the trace attribute dictionary and creating spans in
trace_callwhen OpenTelemetry is not configured or uses a no-op tracer.This reduces overhead by ~4.8 µs per call (~10 µs per query).
If a valid ambient span is present in the caller's context, a child span
is still started on the no-op tracer to prevent Spanner-internal attributes
and events from attaching to the parent span.
Also update
observability_optionsvalidation to checkMapping.