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 13 commits into
Conversation
|
bugbot run |
6e89feb to
36f4e28
Compare
|
bugbot run |
36f4e28 to
9131182
Compare
|
bugbot run |
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 |
c3de18e to
3b4ac11
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 3b4ac11. Configure here.
3b4ac11 to
777418b
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 777418b. Configure here.
777418b to
115c02f
Compare
|
bugbot run |
115c02f to
9b5d9b2
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 7914463. Configure here.
7914463 to
302d3ff
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 302d3ff. Configure here.
302d3ff to
9803d6d
Compare
|
bugbot run |
9803d6d to
9f3dcca
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 9f3dcca. Configure here.
…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.
…ger reads Span construction moved to spans.py, which holds the real _HAS_OTEL. The handler kept its own copy, plus the two imports it needed, alive only by a noqa. Nothing read any of it. No tests aimed at this one, so only the dead code goes. handler._HAS_OTEL to False and believed they were exercising the install without the otel extra; the flag was unread, so they exercised nothing and passed either way. They now patch spans._HAS_OTEL, which is the flag start_root_span actually consults: with it patched, span creation returns None, and with it set it does not. Found by Bugbot on #32. Five of the six handlers carried the dead gate, and four had tests aimed at it.
…open The streaming finally closed the model span and the root, but the in-flight execute_tool span was held only by a local. except Exception does not see a CancelledError or a GeneratorExit, so a tool cancelled mid-flight left its span open and unexported: the trace showed a closed parent above a child that never arrived. Tracked in open_tool_span and abandoned in the finally, the same way the model span already was. The tracker is cleared on the two paths that end the span and deliberately not in a finally, because a finally would also clear it for the BaseException case, which is the one case where the outer finally is the only thing left to close it. Found by Bugbot on the openai-messages layer, and shared by four of the six handlers.
…guard The output content write and the span finish sat outside the try that fails the chat span, and this path has no finally that could recover it. A raise while serialising the parsed object left the span open and unexported, and dropped the turn from the run total even though the provider had already billed it. Reachable through capture_content with any parsed object json.dumps refuses. The usage is now accumulated straight after the provider returns, before anything that can raise, and every span write happens inside the guard. Two tests: the span still ends and reports ERROR, and the tokens survive. Found while checking whether the openai-messages defect Bugbot reported on #32 reached the other handlers. It reached three of them.
The tool loop added its usage only after the try that serialises completion content, so a content failure dropped a turn the provider had already billed. The structured turn already accumulated before any content work, which is what made the tool loop's ordering look accidental rather than considered. It was. Both now accumulate straight after the provider returns. The structured result path then serialised the parsed object with json.dumps before handing it to set_output_content_attributes, including when capture_content was off. That helper is a no-op without the flag and json.dumps is not, so a parsed object json.dumps refuses turned a successful run into a raised TypeError for a caller who had asked for no content at all. The work now sits behind the flag that decides whether anyone will read it. Two tests, each failing on its own defect when reverted. Found by Bugbot on #34.
…okens The content write and the span finish sat outside the try that fails the chat span, and the usage was accumulated after both. A raise while serialising completion content left the span for the finally to end as abandoned, which reads as a consumer who walked away rather than as the failure it was, and dropped a turn the provider had already billed. Both now sit inside a guard, and the accumulation happens first. Two tests: the span is failed rather than abandoned, and the tokens survive. Found by Bugbot on #34.
The tool loop's own return serialised the run output with json.dumps before handing it to set_output_content_attributes, including when capture_content was off. That helper is a no-op without the flag and json.dumps is not, so with tools and an outputFormat on a non-OpenAI provider, where the output is the parsed object, a value json.dumps refuses turned a successful run into a raised TypeError for a caller who had asked for no content at all. The outputFormat-only path was already fixed the same way. This is its sibling exit, which the earlier fix missed. Found by Bugbot on #34.
… too The prompt write ran before the try that fails the span it writes to. Serialising conversation content raises on anything that is not JSON-serialisable, so a raise there left the chat span open on the structured path, and on both root paths left the root open: never ended, never exported, so the run disappeared from AI Config Monitoring along with the feature_flag event it carries. The output writes were moved inside their guards earlier in this stack. The input writes were not, which is the same defect at the other end of the same span. Two tests, one per root path, each failing when the fix is reverted. Found by Bugbot on #34. Four other handlers share the shape and are fixed in their own layers.
The input write for each chat span ran before the try that fails it. On the blocking path a raise left the child span open and unexported while the root was failed, and there is no finally on that path to recover it. On the streaming path the raise reached the outer finally with open_model_span still set, so the span was ended as abandoned: a content failure that reads as a consumer walking away. The root's own input writes were moved inside their guards earlier in this stack, and the structured turn already kept its input write inside one. The per-turn writes in both loops were missed. Two tests, one per path, each failing when the fix is reverted. Found by Bugbot on this PR.
9f3dcca to
449b7e1
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 449b7e1. 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 OTel span per call with the same tree the TypeScript SDK emits: an
invoke_agentroot (LaunchDarkly identity + run token totals), onechat {model}child per model turn, andexecute_tool {name}siblings parented to the root—not underchat.Span construction moves to
spans.py, wired throughlaunchdarkly_ai_serverhelpers (set_input_content_attributes,create_run_usage,lang_chain_span_usage, etc.) instead of hand-rolled OpenLLmetry events on a single span.Provider attributes are corrected:
gen_ai.systemis always the literallangchain;gen_ai.provider.namereflects the client the handler actually builds (anthropicvsopenai, not a config passthrough). Finish reasons and message roles go through shared LangChain helpers, with a_with_get_typeshim solangchain-coremessages aren’t misclassified asuser.capture_content(default off) gates prompts, completions, and tool I/O on spans. Blocking and streaming paths accumulate usage before content serialization so billed tokens still land on the root when serialization fails; streaming addsfinallyteardown (abandoned streams, innerastreamaclose(), no double span ends).Breaking: span name
langchain.invoke→invoke_agent;gen_ai.systemvalue changes; span content requirescapture_content=True.langchain_messages()now popscapture_contentso it reaches the handler instead ofconfig().Reviewed by Cursor Bugbot for commit 449b7e1. Bugbot is set up for automated code reviews on this repo. Configure here.