Skip to content

feat: add o11y gRPC tracing - #9267

Open
shivanee-p wants to merge 3 commits into
shivaneep-o11y-rest-timeoutfrom
shivaneep-o11y-grpc-tracing
Open

feat: add o11y gRPC tracing#9267
shivanee-p wants to merge 3 commits into
shivaneep-o11y-rest-timeoutfrom
shivaneep-o11y-grpc-tracing

Conversation

@shivanee-p

@shivanee-p shivanee-p commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Call TracerHelper within createApiCall() and update unit tests accordingly

  • build StaticTraceContext from the internal telemetry information from CallSettings
  • determine if calls are streaming or not
  • add unit tests for fallback detection

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 8, 2026
View reviewed changes

@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 integrates OpenTelemetry tracing into API calls within google-gax. It updates createApiCall to wrap invocations with traceAttempt when telemetry is enabled, captures static and dynamic trace contexts, and propagates the internal method name via constructSettings. Additionally, traceAttempt is refactored to synchronously handle both synchronous results and Promises. The feedback highlights two important issues: first, checking result instanceof Promise is fragile and fails to correctly track the lifetime of streaming calls (which return streams rather than Promises); second, the unit tests use a synchronous callback mock that masks this premature span termination. It is recommended to check for thenables and streams to properly manage span lifetimes, and to update the tests to invoke callbacks asynchronously.

Comment thread core/packages/gax/src/observability/TracerHelper.ts Outdated
Comment thread core/packages/gax/test/unit/apiCallable.ts
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch 2 times, most recently from fa84a3f to 0706fc7 Compare September 8, 2026 23:31
@shivanee-p
shivanee-p changed the base branch from main to shivaneep-o11y-tracer-helper-updates September 8, 2026 23:33
@shivanee-p shivanee-p changed the title Shivaneep o11y grpc tracing feat: add o11y gRPC tracing Sep 8, 2026
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch 2 times, most recently from d231ce3 to 6ae7e02 Compare September 8, 2026 23:53
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch from 6ae7e02 to 57d1e9a Compare September 9, 2026 00:03
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch 4 times, most recently from 6ed7d57 to 98c43f2 Compare September 9, 2026 00:39
@shivanee-p
shivanee-p removed this pull request from stack #9270 September 9, 2026 15:29
@shivanee-p
shivanee-p added this pull request to stack #9275 September 9, 2026 15:29
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch from 98c43f2 to 5691da9 Compare September 9, 2026 15:34
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch 2 times, most recently from add101e to d7b3c8e Compare September 9, 2026 20:59
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch 2 times, most recently from 16cfa1e to 4a51201 Compare September 9, 2026 21:23
@shivanee-p
shivanee-p marked this pull request as ready for review September 9, 2026 21:30
@shivanee-p
shivanee-p requested a review from a team as a code owner September 9, 2026 21:30
@shivanee-p

shivanee-p commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch from 2f47ead to f54aaa4 Compare September 10, 2026 18:37
@shivanee-p
shivanee-p removed this pull request from stack #9275 September 10, 2026 18:41
@shivanee-p
shivanee-p added this pull request to stack #9282 September 10, 2026 18:42
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch from c428d77 to 9ded6a0 Compare September 11, 2026 18:08
quirogas
quirogas reviewed Sep 14, 2026
View reviewed changes

@quirogas quirogas 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.

Thanks for wiring traceCall, the callback wrapper, and getTracingBackstopMs through createApiCall, along with the new unit tests in apiCallable.ts. One small nit inline.

Comment thread core/packages/gax/src/createApiCall.ts Outdated
@quirogas quirogas mentioned this pull request Sep 14, 2026
Draft
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch 2 times, most recently from 1405674 to ae927fe Compare September 15, 2026 00:53
@shivanee-p
shivanee-p removed this pull request from stack #9282 September 15, 2026 01:46
@shivanee-p
shivanee-p changed the base branch from shivaneep-o11y-tracer-helper-updates to shivaneep-o11y-rest-timeout September 15, 2026 01:51
@shivanee-p
shivanee-p added this pull request to stack #9335 September 15, 2026 01:51
@shivanee-p

shivanee-p commented Sep 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Thanks for wiring traceCall, the callback wrapper, and getTracingBackstopMs through createApiCall, along with the new unit tests in apiCallable.ts. One small nit inline.

Ty for the comment! I updated the logic in the stack, but I removed the backstop parts. PR#9334 has the fix instead. The TLDR is that we never actually verified that REST calls were complete.

@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch from 5518beb to f67243e Compare September 15, 2026 16:11
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch from f67243e to b604bf2 Compare September 15, 2026 16:24
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch from b604bf2 to 5bd9df0 Compare September 15, 2026 17:45
@shivanee-p
shivanee-p removed this pull request from stack #9335 September 15, 2026 21:12
@shivanee-p
shivanee-p added this pull request to stack #9345 September 15, 2026 21:12
@shivanee-p
feat(gax): trace gRPC api calls via traceCall in createApiCall
249995b 
Wire TracerHelper.traceCall into createApiCall so gRPC calls emit spans when telemetry tracing is enabled. Covers unary, streaming, and callback-style calls, passing the isStreamingCall flag and the maxDurationMs backstop, and keeps the _fallback parameter type intact.

Adds unit tests for the createApiCall tracing branch, stream retries, listener cleanup, and premature span closure.

Squashed from 42 commits (24 of which were stale duplicates of shivaneep-o11y-tracer-helper-updates work) to restore linear history across the stack. Content is identical to the previous branch tip.
@shivanee-p
refactor(gax): guard otherArgs in internalTelemetryInfo optional chain
7ce5198 
The staticArgs block started its optional chain at internalTelemetryInfo, leaving otherArgs itself unguarded, while internalMethodName a few lines below already used settings.otherArgs?.*

This is not currently reachable: checkTelemetryEnabled(settings) guarantees otherArgs is defined before the tracing branch runs. It is also invisible to the compiler, since CallSettings declares otherArgs as required (CallOptions declares it optional), so tsc accepts the unguarded access. That combination means a refactor of the gating would surface this as a runtime TypeError with no compile-time warning.

No behavior change.
@shivanee-p
docs(gax): correct the traced callback contract in createApiCall
49458c5 
traceCall now wraps the user's callback for stream calls as well, so the
comment describing it as non-streaming only no longer holds. The
tracedCallback ?? callback fallback is unchanged and still correct.
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-grpc-tracing branch from 5bd9df0 to 49458c5 Compare September 15, 2026 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@quirogas quirogas quirogas approved these changes
@bshaffer bshaffer Awaiting requested review from bshaffer
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shivanee-p @quirogas