Repository navigation
fix(spanner): record streaming operation completion when stream finishes - #18704
sinhasubham wants to merge 1 commit into
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 refactors the Spanner client's metrics tracking to ensure that streaming operation latencies are recorded immediately upon stream completion and exclude consumer processing delays. This is achieved by tracking stream completion in both synchronous and asynchronous snapshot iterators and recording metrics accordingly. However, the reviewer identified a critical issue in metrics_interceptor.py where the code attempts to check for a last attribute on the yielded stream items. Since the production PartialResultSet protobuf messages do not contain a last field, this check will always evaluate to False and act as dead code. It is recommended to remove these redundant checks.
fcc2c32 to
1b8a925
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the Spanner client to record operation completion metrics immediately when the stream finishes or when a PartialResultSet with last=True is observed, rather than waiting for background stream draining. The feedback suggests simplifying the boolean checks in metrics_interceptor.py by removing the explicit 'is True' comparisons to align with PEP 8 guidelines.
1b8a925 to
31b7a08
Compare
…empt latency upon stream completion Wait until the streaming response completes (either via PartialResultSet.last=True or iterator exhaustion/closure) before recording operation completion and attempt completion metrics in _restart_on_unavailable and _StreamingResponseWrapper / _AsyncStreamingResponseWrapper. Fixes #18656
31b7a08 to
e37b728
Compare
Description
Fixes an issue where
operation_latenciesandoperation_countclient metrics were not being emitted for streaming queries (ExecuteStreamingSql).Key Changes
_restart_on_unavailablein atry...finallyblock (sync and async insnapshot.py) to guaranteemetrics_tracer.record_operation_completion()is called when streams finish normally, exit early viabreak, or raise unhandled exceptions._StreamingResponseWrapperand_AsyncStreamingResponseWrapperinmetrics_interceptor.pyto trigger attempt metric recording when encounteringlast=Trueon partial result sets."OK"upon caught gRPC stream exceptions.last=Truechunk handling across both sync and async paths.Validation
Verified using live test runs on project
span-cloud-testing(instance=ranjithmrtest,database=gsql_enterprise_final) confirming thatoperation_latenciesandattempt_latenciesare actively emitted to Cloud Monitoring forExecuteStreamingSql.operation_latencies&attempt_latencies(p50)operation_latencies&attempt_latencies(Mean)