Skip to content

feat(openai-agents)!: emit invoke_agent, chat and execute_tool spans - #33

Merged
apucacao merged 14 commits into
ag/py-telemetry-openai-messagesfrom
ag/py-telemetry-openai-agents
Aug 14, 2026
Merged

feat(openai-agents)!: emit invoke_agent, chat and execute_tool spans#33
apucacao merged 14 commits into
ag/py-telemetry-openai-messagesfrom
ag/py-telemetry-openai-agents

Conversation

@apucacao

@apucacao apucacao commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Replaces one flat span per call with the tree the TypeScript SDK emits, for openai-agents.

invoke_agent                     one per call, carries the LD identity and the run total
├── chat {model}                 one per model turn, with that turn's own tokens
└── execute_tool {name}          one per tool call, a sibling of chat

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

  1. gen_ai.response.model stays the requested name, on both the root and the chat spans, which is deliberately different from openai-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.

  2. 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.

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.run to invoke_agent. Queries selecting on the old name will not match. Prompt and completion content is no longer on spans unless the caller passes capture_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 stack is linear.

Tests: 781 to 799.


Note

Overview
Replaces the single flat openai.agent.run span with the same three-level tree as the TypeScript handler: an invoke_agent root (LD identity + run totals), chat {model} per model turn, and execute_tool {name} siblings per tool call—driven by RunHooks (_SpanningHooks) instead of ad-hoc OTel on the root.

Span construction moves to spans.py (usage, finish-reason derivation, Responses API message shaping). create_openai_agent_handler(capture_content=False) gates prompts/completions/tool args/results on spans; openai_agents now forwards capture_content correctly.

Usage and failure paths prefer the Runner’s context_wrapper.usage over hook accumulation (older SDKs), keep billing when OTel is off, avoid double-counting on SDK exceptions, and skip writing all-zero usage on first-call failures. finally / streaming teardown end stranded spans on cancel/abandon, distinguish cancelled vs abandoned, and streamed.cancel() the Runner when the consumer stops early (not gated on spans).

Breaking: span name invoke_agent; content off by default. gen_ai.response.model stays the requested name; finish reasons are partially derived (Python SDK drops response status).

Reviewed by Cursor Bugbot for commit 2b246d0. Bugbot is set up for automated code reviews on this repo. Configure here.

@apucacao

Copy link
Copy Markdown
Contributor Author

bugbot run

Comment thread packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py Outdated
@apucacao
apucacao force-pushed the ag/py-telemetry-openai-agents branch from efe4f71 to aec3684 Compare August 11, 2026 20:43
@apucacao

Copy link
Copy Markdown
Contributor Author

bugbot run

Comment thread packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py Outdated
@apucacao
apucacao force-pushed the ag/py-telemetry-openai-agents branch from aec3684 to 60c2410 Compare August 11, 2026 21:01
@apucacao

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@apucacao
apucacao force-pushed the ag/py-telemetry-openai-agents branch from 60c2410 to 169ab0c Compare August 11, 2026 21:18
@apucacao

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@apucacao

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@apucacao
apucacao force-pushed the ag/py-telemetry-openai-agents branch from 169ab0c to 6bda783 Compare August 12, 2026 17:23
@apucacao

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@apucacao
apucacao force-pushed the ag/py-telemetry-openai-agents branch from 6bda783 to ca6774a Compare August 12, 2026 17:45
@apucacao

Copy link
Copy Markdown
Contributor Author

bugbot run

Comment thread packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py
Comment thread packages/openai-agents/src/launchdarkly_ai_openai_agents/handler.py
@apucacao

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@apucacao
apucacao force-pushed the ag/py-telemetry-openai-agents branch from 1ad37a7 to a2d3d69 Compare August 12, 2026 18:07
@apucacao

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 9dde276. Configure here.

@apucacao
apucacao force-pushed the ag/py-telemetry-openai-agents branch from 9dde276 to cf6d4f9 Compare August 14, 2026 19:39
@apucacao

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 cf6d4f9. 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.
…ring

The Agents SDK carries function_call.arguments and context.tool_arguments as an
opaque JSON string. Every other handler puts a parsed object on a tool_call part,
because Anthropic and LangChain hand over an object already.

Passing the string through left the content carriers encoding it a second time, so
a reader saw "arguments": "{\"q\":1}" on an OpenAI span and "arguments": {"q": 1}
on an Anthropic span describing the same kind of call.

The comment on the hook path made this look deliberate. It cited section 7 of the
contract as a reason to pass the string, but that section describes what the writer
does with a value it is given, not what a call site should hand it. Reading it as a
mandate is what produced the mismatch. The comment now cites the call-site rule and
records the misreading, so the next reader does not repeat it.

A string that does not parse comes back verbatim rather than raising: a truncated
stream is worth reporting as it arrived, and raising inside the telemetry path would
end a run the provider has already billed.

Two tests. Removing the helper fails one, removing its guard fails the other.

Matches launchdarkly/js-ai-sdk#23. Found by Bugbot on the sibling PR.
on_tool_start filed an open tool span under context.tool_call_id, falling back to
the tool name when that was absent. on_tool_end read str(context.tool_call_id) with
no fallback, so an absent id produced the string "None" and the pop missed.

The span then never closed on success. It stayed open until process teardown, and a
reader saw a tool that started and never returned. Only the crash and abandonment
paths closed it, because those iterate the whole map rather than looking a key up.

Both hooks now call one _tool_span_key helper, which mirrors the single callId
helper the TypeScript handler shares between its two hooks. The port had lost that
sharing and recomputed the key twice, differently.

One test, driving a tool call whose id is None. It fails with the old lookup.

Found by Bugbot on this PR.
A timeout or a task.cancel() raises asyncio.CancelledError, which inherits from
BaseException, so it walks past the except Exception clause in _call_impl. The blocking
path ended its spans only from that clause, so a cancelled run exported nothing at all:
not a wrong attribute, no span. The root carries the feature_flag event and every
launchdarkly.* attribute, so the whole run vanished from AI Config Monitoring rather
than showing as incomplete.

A finally in _call_impl now owns the ends the except clause cannot reach: it closes
whatever the RunHooks still had open (the chat span, any execute_tool span), then the
root. This is the shape the streaming path has had since the earlier rounds, so both
paths of this handler now agree.

The root is tracked with a local, open_root_span, cleared by whichever path already
ended it, rather than by asking the span. A mock span answers is_recording() truthily
and the test suite is built on mock spans, so asking would have made the finally fire a
second end on every successful run. The hooks object already tracked its own open
spans the same way, by clearing them once on_llm_end or on_tool_end fires.

A cancelled root still reports the spend of the turns that completed, for the same
reason the failure path does: those turns were billed.

Spans are left at UNSET and marked launchdarkly.run.cancelled. Nothing failed, the
caller went away.

Two tests, driving a real task.cancel() against a provider call that never returns.
Gutting the finally fails both.

Found by Bugbot on the langchain-messages layer, then found here by audit.
…not serialise

A tool result comes from the caller's own function, so it can be any Python object,
including one json.dumps refuses. Both content writes on the tool path could raise,
and neither was positioned so the span survived it.

on_tool_start wrote the arguments before filing the span in open_tool_spans. A raise
there left a span nothing knew about: close_open_spans and abandon_open_spans both
walk that dict, so a span missing from it can never be ended and never exports.
Filing now happens first.

on_tool_end popped the span, then wrote the result outside any guard. Once popped,
ending it is that method's job alone, and a raise meant nobody did it.

The argument read also moved back inside its capture_content branch. It had come to
rely on `and` short-circuiting to stay bound, which works and reads like a mistake.

langchain-agents already had both of these right, and is what this now matches.

One test, driving a tool whose result cannot be serialised. Removing the guard fails
it.

Found by Bugbot on this PR.
…oned

A CancelledError never enters except Exception, so the streaming teardown always ran
its abandonment path and marked launchdarkly.stream.abandoned on the root, the chat
span and every open tool span.

A consumer that stops reading abandoned the stream, and that word is right for it. A
CancelledError is not a choice: something cancelled the run, usually a timeout, and
the consumer was still reading. The blocking path already reports
launchdarkly.run.cancelled for that, so the two paths disagreed about the same event.

abandon_open_spans carries the distinction through to the tool spans, so a cancelled
run's children agree with its root about why they stopped.

The new test suspends inside the fake stream rather than in the consumer's loop body.
Cancelling a consumer that is awaiting the next chunk raises CancelledError inside the
generator, which is the real timeout shape. Sleeping in the loop body unwinds as a
GeneratorExit instead, which is abandonment and would have tested the wrong thing.

The existing abandonment test breaks out of the loop and still asserts
stream.abandoned, and needed no change.

No new attribute. Both keys already exist and are in the vocabulary lock.

Setting the flag to False fails the new test.

Found by Bugbot on this PR.
@apucacao
apucacao force-pushed the ag/py-telemetry-openai-agents branch from cf6d4f9 to 2b246d0 Compare August 14, 2026 19:56
@apucacao

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 2b246d0. Configure here.

@apucacao
apucacao marked this pull request as ready for review August 14, 2026 21:18
@apucacao
apucacao merged commit 6a82ef5 into main Aug 14, 2026
12 checks passed
@apucacao
apucacao deleted the ag/py-telemetry-openai-agents branch August 14, 2026 21:20
apucacao added a commit that referenced this pull request Aug 14, 2026
…apper

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 against openai-agents. Five of the six wrappers had it;
each is fixed in its own layer.
apucacao added a commit that referenced this pull request Aug 14, 2026
…apper

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 against openai-agents. Five of the six wrappers had it;
each is fixed in its own layer.
apucacao added a commit that referenced this pull request Aug 14, 2026
… 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.
apucacao added a commit that referenced this pull request Aug 14, 2026
…rapper

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 against openai-agents. Five of the six wrappers had it;
each is fixed in its own layer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants