Skip to content

perf(spanner): optimize built-in metrics hot path and harden concurrency - #18329

Open
olavloite wants to merge 1 commit into
mainfrom
spanner-optimize-built-in-metrics-hot-path
Open

perf(spanner): optimize built-in metrics hot path and harden concurrency#18329
olavloite wants to merge 1 commit into
mainfrom
spanner-optimize-built-in-metrics-hot-path

Conversation

@olavloite

Copy link
Copy Markdown
Contributor

Optimize hot-path execution in the Spanner client's built-in metrics subsystem and resolve subtle regex boundary and concurrency issues:

  • Correctness & Regex Parsing:

    • Add negative lookbehind (?<![a-zA-Z0-9_-]) to GFE and AFE timing regexes to prevent false-positive matches (e.g. safe; dur=55).
    • Guard regex evaluations in extract_front_end_latencies with fast string containment checks ("gfet4t7" in text, "afe" in text).
    • Extract helper _extract_metric_latency and flatten header detection.
  • Caching & Allocations:

    • Add bounded LRU cache (maxsize=128) for resource path parsing, yielding a ~9x speedup matching the Java client's caching strategy.
    • Add bounded LRU cache (maxsize=64) for RPC method name formatting.
    • Introduce _ObservableDict to achieve zero-allocation steady-state OpenTelemetry attribute caching with O(1) invalidation on mutation.
    • Consolidate RPC attempt preparation in MetricsInterceptor._prepare_attempt.
  • Concurrency & Resource Safety:

    • Extract _BaseAsyncResponseWrapper and synchronize _metrics_recorded via threading.Lock across cancel(), __del__(), and _record_metrics().
    • Safely close unawaited initial metadata coroutines on cancellation.
    • Guarantee ContextVar token reset in MetricsCapture.__exit__ via finally.
    • Harden _safe_decode_utf8 to return "" when passed None.

Optimize hot-path execution in the Spanner client's built-in metrics
subsystem and resolve subtle regex boundary and concurrency issues:
- Correctness & Regex Parsing:
  - Add negative lookbehind `(?<![a-zA-Z0-9_-])` to GFE and AFE timing
    regexes to prevent false-positive matches (e.g. `safe; dur=55`).
  - Guard regex evaluations in `extract_front_end_latencies` with fast string
    containment checks (`"gfet4t7" in text`, `"afe" in text`).
  - Extract helper `_extract_metric_latency` and flatten header detection.

- Caching & Allocations:
  - Add bounded LRU cache (`maxsize=128`) for resource path parsing,
    yielding a ~9x speedup matching the Java client's caching strategy.
  - Add bounded LRU cache (`maxsize=64`) for RPC method name formatting.
  - Introduce `_ObservableDict` to achieve zero-allocation steady-state
    OpenTelemetry attribute caching with O(1) invalidation on mutation.
  - Consolidate RPC attempt preparation in `MetricsInterceptor._prepare_attempt`.

- Concurrency & Resource Safety:
  - Extract `_BaseAsyncResponseWrapper` and synchronize `_metrics_recorded`
    via `threading.Lock` across `cancel()`, `__del__()`, and `_record_metrics()`.
  - Safely close unawaited initial metadata coroutines on cancellation.
  - Guarantee ContextVar token reset in `MetricsCapture.__exit__` via `finally`.
  - Harden `_safe_decode_utf8` to return `""` when passed `None`.
@olavloite
olavloite requested a review from a team as a code owner September 10, 2026 08:30

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request optimizes and hardens the Spanner metrics collection system. Key changes include caching resource path parsing and method name formatting using lru_cache, introducing thread-safe locking and cancellation handling in the response wrappers, caching OpenTelemetry attributes with an _ObservableDict to invalidate the cache on modifications, and refactoring metadata parsing to safely handle various formats. Extensive unit tests have been added to cover these optimizations and edge cases. There are no review comments to address, and I have no feedback to provide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant