Skip to content

fix(spanner): scope server-timing metrics per call and guard interceptor lifecycle callbacks - #14053

Merged
olavloite merged 1 commit into
mainfrom
spanner-fix-race-in-header-interceptor
Aug 12, 2026
Merged

fix(spanner): scope server-timing metrics per call and guard interceptor lifecycle callbacks#14053
olavloite merged 1 commit into
mainfrom
spanner-fix-race-in-header-interceptor

Conversation

@olavloite

Copy link
Copy Markdown
Contributor
  • Scope gfeLatency and afeLatency to per-call listener instances in HeaderInterceptor to eliminate data races and cross-RPC telemetry pollution.
  • Guard onHeaders and onClose in HeaderInterceptor with try...finally to guarantee downstream callback propagation (preventing hung futures) and RequestIdTargetTracker cleanup.
  • Catch Throwable in SpannerErrorInterceptor.onClose to prevent unexpected metadata parsing errors from escaping into gRPC transport threads.

The above changes should guarantee that all interceptors in the entire chain of interceptors are always executed, and that no exceptions escape to the gRPC thread executing them.

@olavloite
olavloite requested review from a team as code owners August 12, 2026 08:22

@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 refactors HeaderInterceptor to make it thread-safe by moving latency state variables from the class level to the call listener level. It also enhances robustness in both HeaderInterceptor and SpannerErrorInterceptor by wrapping header and close processing in try-catch-finally blocks, ensuring downstream gRPC notifications are always executed even if unexpected exceptions occur. Additionally, comprehensive unit tests are introduced for both interceptors to verify their behavior and error handling. I have no feedback to provide as there are no review comments.

…tor lifecycle callbacks

- Scope gfeLatency and afeLatency to per-call listener instances in HeaderInterceptor to eliminate data races and cross-RPC telemetry pollution.
- Guard onHeaders and onClose in HeaderInterceptor with try...finally to guarantee downstream callback propagation (preventing hung futures) and RequestIdTargetTracker cleanup.
- Catch Throwable in SpannerErrorInterceptor.onClose to prevent unexpected metadata parsing errors from escaping into gRPC transport threads.

The above changes should guarantee that all interceptors in the entire chain of interceptors
are always executed, and that no exceptions escape to the gRPC thread executing them.
@olavloite
olavloite force-pushed the spanner-fix-race-in-header-interceptor branch from ff9bece to 01b12f6 Compare August 12, 2026 08:37
@olavloite olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 12, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 12, 2026
@olavloite
olavloite merged commit f35c570 into main Aug 12, 2026
205 checks passed
@olavloite
olavloite deleted the spanner-fix-race-in-header-interceptor branch August 12, 2026 09:58
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.

3 participants