Fix: record the thrown error on the invocation, invoke_agent and call_llm spans - #817
Open
AmaadMartin wants to merge 3 commits into
Open
Fix: record the thrown error on the invocation, invoke_agent and call_llm spans#817AmaadMartin wants to merge 3 commits into
AmaadMartin wants to merge 3 commits into
Conversation
added 3 commits
August 8, 2026 11:26
…and call_llm spans OpenTelemetry JS does not record anything when a span body throws, so a crashed ADK run exported spans with status UNSET and no exception event. The call_llm span also ended with a statement after the delegation, so a throw skipped span.end() and the span never reached an exporter. Add recordSpanError() to the telemetry module and resolveErrorType() to error_utils, and catch-record-rethrow at the four span sites. The call_llm span now ends from a finally.
Drive each span through its production call path with the real OTel SDK and an in-memory exporter, and pin both the failure and the success path. The call_llm case also pins that the span is exported at all, which it was not before.
…InSpan The start-bind-record-end sequence was copy-pasted at all four span sites. Move it into one helper next to runAsyncGeneratorWithOtelContext, so each site is a single delegation and recordSpanError stays private to the telemetry module. The runner keeps its own finally for the toolset teardown, which still runs after span.end(). Also drop the errorType branch of resolveErrorType. Nothing in adk-js, @google/genai or the MCP SDK sets that field, so the branch classified a shape the runtime never produces.
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
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.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
No existing issue.
Problem: A crashed ADK run does not look crashed in a trace backend. OpenTelemetry JS records nothing when a span body throws, so
invocation,invoke_agentandcall_llmall finish withUNSETstatus, noexceptionevent and noerror.type. Thecall_llmspan is worse: it ended with a statement placed after the delegation, so a throw skippedspan.end()and the span never reached an exporter at all. adk-python does not have this gap, because its SDK records the exception for astart_as_current_spancontext manager.Solution: Add
runAsyncGeneratorInSpan(spanName, thisArg, fn)tocore/src/telemetry/tracing.ts, which starts the span, binds it as the active OTEL context, records any error that ends the generator, rethrows it unchanged, and always ends the span. All four sites now delegate to it, so the span lifecycle lives in one place.resolveErrorType(error)incore/src/utils/error_utils.tsderives the label: a duck-typed HTTP status first, otherwise the class name. The status message carries the error type, not the message, because the message can hold user or model content thatADK_CAPTURE_MESSAGE_CONTENT_IN_SPANSdoes not gate there.Notes for the reviewer:
===). One test asserts identity.base_agent.tsshows a large hunk, butgit diff main -wreports 5 added and 19 removed lines. The rest is one level of de-indentation from dropping the wrapper.runner.tskeeps its ownfinallyfor the toolset teardown. The helper'sfinallyis inner, sospan.end()still runs first.resolveErrorTypeomits the self-classifiederrorTypebranch that adk-python has. Nothing in adk-js,@google/genaior the MCP SDK sets that field, so the branch would classify a shape the JS runtime never produces.execute_tool(PR Fix: record the tool exception on the execute_tool span #710) andrunAndHandleError, which absorbs anError-typed model failure into an error event instead of rethrowing. Widening that changes behaviour, not telemetry.resolveErrorTypeto the same file, so whichever lands second reconciles that one function.Testing Plan
Unit Tests:
New file
core/test/telemetry/span_error_recording_test.ts(11 tests) drives each span through its real call path with aNodeTracerProviderand anInMemorySpanExporter.core/test/utils/error_utils_test.tsgains aresolveErrorTypeblock (7 tests); the existingformatErrortests are untouched.Every line this change adds is covered.
error_utils.tsreports 100% line and branch coverage, and every line ofrunAsyncGeneratorInSpanandrecordSpanErrorreports a non-zero hit count in the v8 report.Proof the tests can fail. Each mutation was applied to the source, the tests were run, and the source was restored:
catchblocks inbase_agent.tsexpected +0 to be 2(UNSET, not ERROR)runLivecatchrunAsynccase still passedcatchinrunner.tsexpected +0 to be 2llm_agent.tstospan.end()after the delegationexpected [] to have a length of 1 but got +0catchbut dropspan.end()from thefinallyerror.constructor.name->error.nameexpected 'Error' to be 'QuotaExceededError'expected '0' to be 'QuotaExceededError'errorTypeprecedenceexpected '429' to be 'RATE_LIMIT'Manual End-to-End (E2E) Tests:
Build the package, then run a script that registers a
NodeTracerProviderwith anInMemorySpanExporter, drivesRunner.runAsyncover an agent that throws, over an agent whose model throws the stringMODEL_UNAVAILABLE, and over a model that answers, and prints each finished span.Before this change:
The
call_llmspan is absent from the failing model run.After this change:
The success-path output is identical to
main.CI note: the first macOS run failed on
tests/integration/app_loader/app_loader_test.tswithTest timed out in 40000ms. That test is unrelated to this change, it passed on Ubuntu and Windows in the same run, and it passed on a re-run of the same commit.Checklist