feat(langchain-messages)!: emit invoke_agent, chat and execute_tool spans - #34
feat(langchain-messages)!: emit invoke_agent, chat and execute_tool spans#34apucacao wants to merge 5 commits into
Conversation
|
bugbot run |
6e89feb to
36f4e28
Compare
|
bugbot run |
36f4e28 to
9131182
Compare
|
bugbot run |
…pans
One flat span named langchain.invoke becomes the tree the TypeScript SDK emits:
an invoke_agent root, one `chat {model}` child per model turn, one
`execute_tool {name}` child per tool call.
BREAKING CHANGE: the span this handler emits is renamed from `langchain.invoke`
and `langchain.stream` to `invoke_agent`. Queries selecting on the old names
will not match. Prompt and completion content is no longer on spans unless the
caller passes capture_content=True. gen_ai.system changes value; see below.
Both provider attributes were wrong, in different ways.
gen_ai.system was the configured provider name lower-cased, so an
Anthropic-backed config reported `anthropic` where the TypeScript SDK reports
`langchain`. That key names the instrumentation, and for these two handlers the
instrumentation is the framework. It is now the literal string.
gen_ai.provider.name now exists here at all, and it is not a passthrough of the
configured name. It names who served the model, and its semantic-convention
enum has no langchain member, so it follows the client the handler actually
instantiates: ChatAnthropic for a configured provider of anthropic, ChatOpenAI
for everything else. A Bedrock or Azure config therefore reports openai, which
looks wrong and is right, because an OpenAI client is what made the request.
Cached tokens are now read from usage_metadata.input_token_details and reported
per turn, without being added to the input figure, which LangChain already
reports inclusive of them.
Finish reasons go through the shared LangChain helper, which reads the reason
from generation_info or response_metadata depending on which vendor answered and
maps it onto the shared vocabulary. This handler is one of the places where the
same code path serves either vendor, so an untranslated passthrough is least
defensible here.
The streaming path gets a finally. The per-chunk usage accumulation is a
faithful port of the TypeScript, summing each field as chunks arrive rather than
reading a single terminal figure.
Tests: 59 to 79.
…ros as spend Two defects, both introduced by this port. The structured-output paths were fighting. Python already bound response_format for OpenAI when both tools and outputFormat were set, and broke out of the tool loop with the model's own reply. The port added the TypeScript handler's structured follow-up turn on top of that, so for OpenAI the loop produced a structured reply and then a second call threw it away and billed another turn. Neither SDK does both. The follow-up now runs only when response_format was not bound, which is what carries Anthropic and every other non-OpenAI provider, since binding response_format is an OpenAI-only mechanism. The streaming path marked every turn as having reported usage, including turns where no chunk carried any. That defeats the flag: a later failure or abandonment then wrote all-zero totals on the root and claimed the run cost nothing, which is a different claim from unknown and the one thing the flag exists to prevent. The blocking path gets this right for free, because lang_chain_span_usage returns None for a bag the provider never filled. Two tests, and the first needs two turns to be meaningful: a turn that dies mid-iteration never reaches the accumulator, so only a turn that completes without usage followed by one that fails can exercise it. My first attempt passed with the fix reverted, which is how I found that out. Found by Bugbot on #34.
… wrapper The wrapper never passed capture_content to the factory, so it stayed in kwargs and reached config(), which takes no such argument. A caller asking for content on spans got a TypeError rather than content. Lifted out alongside variables, which was already handled the same way and for the same reason: one configures the handler, the other belongs to the invocation, and config() accepts neither. This wrapper also takes llm, so the flag joins it on the factory call rather than replacing the argument list. Found by Bugbot on #33 against openai-agents. Five of the six wrappers had it; each is fixed in its own layer.
…nds its span The success-side content write and the span finish sat outside the try, so a raise while recording the result skipped both the finish and the failure path. The tool span was never ended, so the exporter never saw it: the run showed a root marked ERROR and no sign the tool had been called. Reachable rather than theoretical. Serialising a tool result raises TypeError whenever capture_content is on and the result is not JSON-serialisable, which is any object a handler happens to return. Inherited from the claude-messages handler this one was modelled on, which had it in the wrong place. The TypeScript handlers have always done this inside the try. Found by Bugbot on #34.
Both reachable through capture_content, where serialising any non-JSON-serialisable value raises TypeError. The output write and the span finish sat outside the guard that fails the chat span, so a raise there left it open with nothing able to recover it: the blocking path has no finally. Now inside the try. The streaming finally awaited the vendor generator's aclose() before touching any span. aclose() can raise, and doing it first took the whole teardown with it: the root never ended, never exported, and the run disappeared from AI Config Monitoring along with the feature_flag event that block exists to protect. Spans close first now, and the vendor teardown is contained, because its failure is not worth losing the trace over. Two tests, each failing on its own defect when reverted. Found by Bugbot on #34.
9131182 to
c3de18e
Compare
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c3de18e. Configure here.
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c3de18e. Configure here.
| lang_chain_span_usage(raw_usage) or SpanUsage(), | ||
| lang_chain_finish_reasons(raw), | ||
| ) | ||
| run_usage.add(lang_chain_span_usage(raw_usage)) |
There was a problem hiding this comment.
Structured turn can leak chat span
Medium Severity
In _run_structured_turn, only the model invoke is inside the try/fail_span guard. Output content writes and finish_model_span sit after it, so a raise while serializing parsed (especially with capture_content=True) leaves the chat span open. The tool-loop path in this same change already guards that work for this reason, and nothing else can recover this child span.
Reviewed by Cursor Bugbot for commit c3de18e. Configure here.
| # finish_model_span ends the span. Clearing open_model_span is what stops the | ||
| # `finally` from ending it a second time. | ||
| finish_model_span(model_span, config, turn_usage, finish_reasons) | ||
| open_model_span = None |
There was a problem hiding this comment.
Stream error marks chat abandoned
Medium Severity
After the chunk loop, output content attribution and finish_model_span run outside the inner try that calls fail_span on the open chat span. A raise there leaves open_model_span set, so finally ends that span as abandoned instead of recording the real error.
Reviewed by Cursor Bugbot for commit c3de18e. Configure here.
| ], | ||
| ) | ||
| finish_root_span(span, config, run_usage.total) | ||
| succeed_span(span) |
There was a problem hiding this comment.
Success writes zero root usage
Medium Severity
Successful blocking and streaming completions always call finish_root_span with run_usage.total, even when run_usage.reported is false. That writes all-zero root usage attributes and claims the run cost nothing when LangChain never supplied usage, which the failure path and telemetry contract both avoid.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit c3de18e. Configure here.


Replaces one flat span per call with the tree the TypeScript SDK emits, for
langchain-messages.Both provider attributes were wrong, in different ways
gen_ai.systemwas the configured provider name lower-cased, so an Anthropic-backed config reportedanthropicwhere the TypeScript SDK reportslangchain. That key names the instrumentation, and for the two LangChain handlers the instrumentation is the framework. It is now the literal string.gen_ai.provider.namedid not exist here at all, and it is not a passthrough of the configured name. It names who served the model, and its semantic-convention enum has nolangchainmember, so it follows the client the handler actually instantiates:ChatAnthropicfor a configured provider ofanthropic,ChatOpenAIfor everything else.A Bedrock or Azure config therefore reports
openai. That looks wrong and is right, because an OpenAI client is what made the request. There is a test pinning it, because a passthrough reads as obviously correct.Other changes
usage_metadata.input_token_detailsand reported per turn, without being added to the input figure, which LangChain already reports inclusive of them.generation_infoorresponse_metadatadepending on which vendor answered. This handler is one of the places where the same code path serves either vendor, so an untranslated passthrough is least defensible here.finally. The per-chunk usage accumulation is a faithful port of the TypeScript, summing each field as chunks arrive rather than reading a single terminal figure.Breaking change
The span is renamed from
langchain.invoketoinvoke_agent. Queries selecting on the old name will not match.gen_ai.systemchanges value, as above. Prompt and completion content is no longer on spans unless the caller passescapture_content=True.Where this sits
Needs the usage layer (#28) and the content layer (#29). Independent of the other five handler PRs; the stack orders them only because
gh stackis linear.Tests: 799 to 824.
Note
Overview
Replaces one flat span per call with the TypeScript SDK's span tree for both blocking and streaming paths:
Breaking telemetry changes: span renamed from
langchain.invoketoinvoke_agent;gen_ai.systemis now always the literallangchain(was the configured provider lower-cased); prompt/completion content is off by default and requirescapture_content=True.Also corrects
gen_ai.provider.nameto follow the client actually instantiated (anthropiconly for Anthropic, otherwiseopenai), reports cache tokens and mapped finish reasons per turn, and adds streamingfinallycleanup so abandoned streams still export spans without marking them ERROR.Reviewed by Cursor Bugbot for commit c3de18e. Bugbot is set up for automated code reviews on this repo. Configure here.