Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

fix(metrics): correct GFE metrics extraction and enable by default#17561

Draft
sinhasubham wants to merge 1 commit into
maingoogleapis/google-cloud-python:mainfrom
gfe-metrics-fixgoogleapis/google-cloud-python:gfe-metrics-fixCopy head branch name to clipboard
Draft

fix(metrics): correct GFE metrics extraction and enable by default#17561
sinhasubham wants to merge 1 commit into
maingoogleapis/google-cloud-python:mainfrom
gfe-metrics-fixgoogleapis/google-cloud-python:gfe-metrics-fixCopy head branch name to clipboard

Conversation

@sinhasubham

Copy link
Copy Markdown
Contributor

Description

This PR resolves a critical issue where Spanner GFE (Google Front End) latency metrics were not being properly captured, and ensures these metrics are always enabled by default.

Key Changes:

  • Fix Metadata Extraction Logic: Fixed a truthiness bug in MetricsInterceptor where initial_metadata() returning standard headers (e.g., content-type) masked trailing_metadata(). The server-timing header is now properly extracted from both initial and trailing metadata.
  • Support for Streaming RPCs: Added _StreamingResponseWrapper and _AsyncStreamingResponseWrapper to the interceptor. This correctly defers metrics recording until the streaming iterators finish, ensuring trailing_metadata is fully populated and available before attempting extraction.
  • Enable GFE Metrics by Default: Removed the gfe_enabled toggle in SpannerMetricsTracerFactory. GFE metrics capture is now always-on whenever OpenTelemetry tracing is enabled.
  • Testing: Added unit tests and a new mockserver test to ensure GFE metrics are correctly published end-to-end.

@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 introduces deferred metrics recording for streaming and async streaming RPC responses in Google Cloud Spanner by wrapping responses in specialized wrappers. It also implements GFE latency extraction from response metadata and adds corresponding tests. The review feedback highlights several critical improvements for robustness: refining streaming response detection to check for iterator methods (next and anext) rather than iterable methods to prevent incorrect wrapping of standard iterables; wrapping telemetry and metrics recording blocks in try-except blocks to ensure telemetry failures do not disrupt the main application flow; defensively validating metadata elements before unpacking to avoid unpacking errors; and properly decoding bytes metadata values before regex matching.

Comment thread packages/google-cloud-spanner/google/cloud/spanner_v1/metrics/metrics_tracer.py Outdated
Comment thread packages/google-cloud-spanner/google/cloud/spanner_v1/metrics/metrics_tracer.py Outdated
@parthea

parthea commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

@sinhasubham, Please merge main into this branch. The presubmit failures were addressed in #17578

@sinhasubham sinhasubham force-pushed the gfe-metrics-fix branch 3 times, most recently from b0c3ca5 to 4226490 Compare June 29, 2026 07:43
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.