fix(gax): update error messages to be set to span.status.message - #9347
Draft
shivanee-p wants to merge 5 commits into
Draft
shivanee-p wants to merge 5 commits into
shivanee-p wants to merge 5 commits into
Conversation
shivanee-p
added this pull request to stack #9345
September 15, 2026 22:29
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request enhances telemetry tracing and adds robust deadline/timeout enforcement for the REST fallback transport in google-gax. It integrates OpenTelemetry tracing into API calls, maps transport-specific timeout failures to DEADLINE_EXCEEDED errors, and tracks HTTP status codes on fallback errors. The review feedback highlights critical compatibility and robustness improvements: avoiding AbortSignal.any to maintain Node.js 18 compatibility, handling invalid Date objects for deadlines to prevent runtime crashes, and filtering out null or undefined metadata values to avoid sending literal string headers.
shivanee-p
force-pushed
the
shivaneep-o11y-errors-exceptions
branch
2 times, most recently
from
September 17, 2026 18:27
b325b67 to
116e15f
Compare
shivanee-p
force-pushed
the
shivaneep-o11y-errors-exceptions
branch
2 times, most recently
from
September 17, 2026 23:02
fc6d7b8 to
6563ab7
Compare
shivanee-p
force-pushed
the
shivaneep-o11y-errors-exceptions
branch
from
September 17, 2026 23:04
6563ab7 to
7855ea3
Compare
Three deviations from the semantic conventions, all in traceCall: - error.message was set on every failed span. semconv deprecated the attribute and calls it NOT RECOMMENDED on spans, because it has unbounded cardinality and restates the span status description that already carries the message. The status description is now its only home. - Successful calls set the span status to OK. semconv requires the status to be left unset when an operation ends without any errors; OK is reserved for an application explicitly overriding the instrumentation's judgement, which a library must never claim on the application's behalf. - A non-Error throw reported no error.type at all, leaving the failure invisible to any error-rate query that groups on it. It now reports the semconv-defined _OTHER fallback. The accompanying exception event is dropped: recordException on a bare string yields an event with no exception.type and no stacktrace, which adds nothing the status description does not already carry. rpc.response.status_code is unaffected. It is a domain-specific RPC status rather than the span status, and semconv recommends reporting a domain-specific attribute alongside error.type.
The previous commit changed which signal carries what, but the existing tests only assert each attribute where it happens to be used. Nothing pinned the split itself, so the deprecated attribute or an instrumented OK status could return without a single failure. Adds a suite covering the contract directly: - error information (status + error.type) and exception information (the event) stay on their own signal, with neither leaking onto the other - error.message is never set, while the message stays reachable via the status description and the exception event - the exception event carries a stacktrace, the one detail no span attribute may hold - error.type agrees with the RPC status resolved for the same call - exactly one exception event is recorded however many completion signals a stream emits - a non-Error, a coded non-Error and a thrown null all still produce a usable error.type and RPC status - a successful call reports no error information at all The first case also documents that OTel derives exception.type from an error's code property before its name property, so a coded gax error reports '5' on the event where the span reports 'NOT_FOUND'. That asymmetry is the clearest argument for resolving error.type separately.
Forward the deadline and server-stream cancel signals onto the request's cancel controller by hand, and make the request with that controller's signal, instead of composing them with AbortSignal.any. Behaviour is unchanged: both paths still set their bookkeeping flag before aborting, so the handlers can tell a deadline expiry apart from a cancel(). AbortSignal.any is safe on the supported Node versions, but this module is also the browser entry point and it is a recent addition there (Chrome 116, Safari 17.4).
shivanee-p
force-pushed
the
shivaneep-o11y-errors-exceptions
branch
from
September 17, 2026 23:27
e35f033 to
4f58eb6
Compare
The span status description is the only place a failure's message lives, but a non-Error went through String(), so an object carrying a perfectly good message reported '[object Object]'. resolveRpcStatusName and resolveHttpStatusCode already read their fields off whatever was thrown rather than off an Error; read the message the same way, falling back to String() when there is no string message to use. Also adds status description coverage for the paths that had none: the write-only (client-streaming) failure route, an Error with an empty message, a second distinct failure not overwriting the first description, and a late error leaving the description unset.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.