feat(openai-agents)!: emit invoke_agent, chat and execute_tool spans - #33
feat(openai-agents)!: emit invoke_agent, chat and execute_tool spans#33apucacao wants to merge 9 commits into
Conversation
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
efe4f71 to
aec3684
Compare
|
bugbot run |
aec3684 to
60c2410
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 60c2410. Configure here.
60c2410 to
169ab0c
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 169ab0c. Configure here.
|
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 169ab0c. Configure here.
169ab0c to
6bda783
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 6bda783. Configure here.
6bda783 to
ca6774a
Compare
|
bugbot run |
|
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 1ad37a7. Configure here.
1ad37a7 to
a2d3d69
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 a2d3d69. Configure here.
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 a11074c. Configure here.
a11074c to
eab1621
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 eab1621. Configure here.
|
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 eab1621. Configure here.
eab1621 to
4cf0be7
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 4cf0be7. Configure here.
4cf0be7 to
e5bc277
Compare
|
bugbot run |
|
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 7771be4. Configure here.
One flat span named openai.agent.run 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. The per-turn data was already in
hand: this handler walked the Runner's raw responses to sum usage, and simply
never opened a span per turn.
BREAKING CHANGE: the span this handler emits is renamed from `openai.agent.run`
and `openai.agent.run.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.
Cached tokens are now reported, read from the cached-tokens detail and left out
of the input total, because OpenAI already counts them inside it. Cache creation
is always zero.
gen_ai.response.model stays the requested name here, on both the root and the
chat spans, which is deliberately different from openai-messages. The
TypeScript twin has never resolved the answering model in this handler and no
test pins it, so reporting one would invent behaviour rather than match it.
Finish reasons are derived from the Responses API's status rather than mapped
through the shared table, which does not apply: there is no finish_reason field
to map. A function call in the output takes precedence over status, because a
live capture put `completed` on every turn including the six that stopped to
call a tool.
Abandoning the stream needed more than ending our spans. The old path iterated
the Runner's event stream with no cleanup at all, and breaking out of that loop
only stops us reading: the Runner's own background task keeps calling the model
and spending tokens until told to stop. Teardown now cancels the streamed run
as well as closing the span tree, so an abandoned stream stops costing money.
Tests: 53 to 71.
Two sources describe the same spend and they overlap. The run hooks add each turn as it finishes, so by the time the run raises they already hold every completed turn, and the exception carries the SDK's own aggregate over those same turns. The error path added the aggregate to the accumulator, so any run that failed after paid turns reported roughly twice what it cost. MaxTurnsExceeded does that by definition, which makes this the common case rather than an edge one. A three-turn run reporting 70 input tokens reported 140. The aggregate is the authoritative figure, so it now replaces the accumulator rather than adding to it, matching what the TypeScript handler does. When the error carries no aggregate, which is what a tool handler's own error looks like, the accumulator is all there is and is used instead. Neither having anything still writes nothing, because all-zero attributes would assert the run cost nothing. Three tests, one per branch. The double-count one fails with 140 against 70 when the fix is reverted, which is how I checked it pins the bug rather than the behaviour. Found by Bugbot on #33, severity High.
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. Two tests, one per branch, asserting the flag reaches the factory and does not reach config(). Found by Bugbot on #33. It flagged this handler; five of the six wrappers have it, and the other four are fixed in their own layers.
…eads 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. That mattered because the tests patched the dead one. 3 tests set 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.
Two ways a run reported the wrong spend. on_llm_end returned early when there was no open chat span, before it accumulated the turn's usage. On an install without the otel extra no span is ever created, so every turn's tokens were dropped and the handler handed the caller zeros. That is a billing figure rather than a telemetry one: it is what the caller reads back and what LaunchDarkly's own metrics record for the AI Config. The accounting now runs first and unconditionally, and only the span writes depend on the span. _write_failed_run_usage then wrote a full set of zeros for a run that died before its first paid call. RunContextWrapper.usage defaults to an empty Usage, so the aggregate the exception carries is present from the moment the run starts, and the function treated present as authoritative. Zeros on the root assert the run cost nothing, which is different from not knowing what it cost, and this function's own docstring already said it must not make that claim. Four tests: the usage bag with telemetry off, the bag agreeing with the root span with telemetry on, no usage attributes for a run that died first call, and real spend still reported for one that died after a paid turn. Found by Bugbot on #33.
The cancel sat inside a span is not None guard. Without the otel extra there is no root span at all, so an early consumer break never cancelled the Runner: its background task kept calling the model and spending money. That is the exact failure this teardown exists to prevent, reintroduced by an install choice that has nothing to do with tracing. Stopping the vendor's run is not telemetry, so it no longer sits behind a span. It is gated on whether the stream ran to the end, tracked in its own local. Two tests: an abandoned stream cancels with telemetry off, and a stream read to the end is not cancelled afterwards. Found by Bugbot on #33.
The prompt write ran before the try that fails the root, so a raise while serialising it left the root open: never ended, never exported, so the run disappeared from AI Config Monitoring along with the feature_flag event it carries. Both paths had it. The chat span's own input write is already safe, because on_llm_start records the span in open_model_span before writing, so close_open_spans can still reach it. Two tests, one per path. Found by Bugbot on #34, which is this shape in langchain-messages.
on_llm_end clears open_model_span before it writes, so close_open_spans can no longer reach that span: it is the only code left that can end it. A serialisation failure in the content write left the chat span open with nothing tracking it, never ended and never exported, while the run reported an error on the root with no sign a model call had happened. The langchain-agents callback guards the identical shape for the identical reason, and says so in a comment. This one did not. One test, and it fails on the reverted code. Found by auditing the handlers for the guard Bugbot reported missing on #30.
…hooks The run total and the returned usage bag came only from the accumulator the LLM lifecycle hooks fill. openai-agents added those hooks in 0.2.11 and this package allows anything from 0.0.1, so on an older SDK no hook fires, the accumulator stays empty, and a run that really did spend handed its caller zeros. LaunchDarkly's metrics recorded the same figure. The Runner keeps its own aggregate in context_wrapper.usage, and that is what the TypeScript handler reads for both the root and the returned bag. It is also the object _usage_from_error already reads on the failure path, whose docstring claimed the success path read it too. It did not, until now. The hook accumulator remains for per-turn chat spans, and stands in for the total only when the aggregate reports nothing, so a provider that fills one and not the other is still counted once. Three tests: a run whose hooks never fired still reports its spend, the aggregate wins when both are present so nothing is double-counted, and the accumulator still stands in when there is no aggregate. Found by Bugbot on this PR.
7771be4 to
7f0607f
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 7f0607f. Configure here.
1 participant
Replaces one flat span per call with the tree the TypeScript SDK emits, for
openai-agents.The per-turn data was already in hand: this handler walked the Runner's raw responses to sum usage, and simply never opened a span per turn.
Abandoning the stream was costing money
The old path iterated the Runner's event stream with no cleanup at all. Breaking out of that loop only stops us reading: the Runner's own background task keeps calling the model and spending tokens until told to stop.
Teardown now cancels the streamed run as well as closing the span tree.
Two things specific to this handler
gen_ai.response.modelstays the requested name, on both the root and the chat spans, which is deliberately different fromopenai-messages(feat(openai-messages)!: emit invoke_agent, chat and execute_tool spans #32). The TypeScript twin has never resolved the answering model here and no test pins it, so reporting one would invent behaviour rather than match it.Finish reasons are derived from the Responses API's status rather than mapped through the shared table, which does not apply: there is no
finish_reasonfield to map. A function call in the output takes precedence over status, because a live capture putcompletedon every turn including the six that stopped to call a tool.Other changes
Cached tokens are now reported, read from the cached-tokens detail and left out of the input total, because OpenAI already counts them inside it. Cache creation is always zero.
Breaking change
The span is renamed from
openai.agent.runtoinvoke_agent. Queries selecting on the old name will not match. 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: 781 to 799.
Note
Overview
Replaces the single flat
openai.agent.runspan with the same tree the TypeScript handler emits: aninvoke_agentroot, onechat {model}child per model turn, andexecute_tool {name}siblings per tool call. Span construction moves tospans.py;RunHooks(on_llm_start/on_llm_end,on_tool_start/on_tool_end) drive per-turn and per-tool boundaries on both blocking and streaming paths.Usage and billing no longer depend on OTel being installed: hooks accumulate turn usage into
run_usage, success paths preferRunResult.context_wrapper.usage, and failures use SDK aggregates on exceptions without double-counting. Cached read tokens are reported separately;capture_content(default off) gates prompts, completions, and tool args/results via shared content helpers instead of legacy OpenLLmetry events.Streaming now mirrors blocking telemetry, abandons open spans with
launchdarkly.stream.abandoned, andfinallycancelsRunner.run_streamedwhen the consumer stops early so background model calls do not keep spending.openai_agents()forwardscapture_contentto the factory instead ofconfig().Breaking: span name
invoke_agent; content on spans only whencapture_content=True.Reviewed by Cursor Bugbot for commit 7f0607f. Bugbot is set up for automated code reviews on this repo. Configure here.