feat(openai-messages)!: emit invoke_agent, chat and execute_tool spans - #32
Draft
apucacao wants to merge 4 commits into
Draft
feat(openai-messages)!: emit invoke_agent, chat and execute_tool spans#32apucacao wants to merge 4 commits into
apucacao wants to merge 4 commits into
Conversation
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 5b44877. Configure here.
apucacao
force-pushed
the
ag/py-telemetry-openai-messages
branch
from
August 11, 2026 20:43
5b44877 to
b2f5fa1
Compare
Author
|
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 b2f5fa1. Configure here.
apucacao
force-pushed
the
ag/py-telemetry-openai-messages
branch
from
August 11, 2026 21:01
b2f5fa1 to
77d1074
Compare
Author
|
bugbot run |
One flat span named openai.response 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 `openai.response`
and `openai.response.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 were absent entirely. OpenAI reports them under
input_tokens_details.cached_tokens, which nothing here read, so every span
understated what a prompt-cached call actually reused. They are now reported in
gen_ai.usage.cache_read.input_tokens and, unlike Anthropic, not added on top of
the input figure: OpenAI already counts them inside it, and adding them would
double-count. Cache creation is always zero, because OpenAI has no such
concept.
This handler is the only one of the six that reports the model which actually
answered rather than the one requested, on both the root and the chat spans.
OpenAI resolves an alias like gpt-4o to a dated snapshot, and this handler has
the resolved value to hand.
Finish reasons are derived, not mapped. The Responses API has no finish_reason
field, so the shared mapping table does not apply and is deliberately not
imported. The value comes from a closed three-way check: a function call in the
output means tool_calls, an incomplete status means length or content_filter
depending on the reported cause, a completed status means stop, and anything
else writes no attribute at all. The function-call check comes first because
status alone reports completed for a turn that stopped to call a tool.
The streaming path gets a finally, so a consumer that breaks out of the
iteration no longer leaves the root span unended and unexported, taking the
whole run out of AI Config Monitoring along with the feature_flag event it
carries.
Tests: 62 to 80.
…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.
… 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.
…t ends it The content writes on both sides of the provider call sat outside the try that fails the chat span, so a raise while serialising conversation content failed only the root. The chat span was never ended and never exported: a run showed an errored root with no sign a model call had happened. Reachable through capture_content, where serialising any non-JSON-serialisable value raises TypeError. The tool path in this same file already kept its serialisation inside the guard, which is what makes the model path's omission look accidental rather than considered. It was. Found by Bugbot on #32.
apucacao
force-pushed
the
ag/py-telemetry-openai-messages
branch
from
August 11, 2026 21:18
77d1074 to
910e8a7
Compare
Author
|
bugbot run |
| from opentelemetry import trace | ||
| from opentelemetry.trace import StatusCode as SpanStatusCode | ||
| from opentelemetry import trace # noqa: F401 | ||
| from opentelemetry.trace import StatusCode as SpanStatusCode # noqa: F401 |
There was a problem hiding this comment.
Dead OTel gate left in handler
Medium Severity
OTel usage moved to spans.py, but handler.py still defines _HAS_OTEL and imports trace / SpanStatusCode under # noqa: F401. Nothing in the handler reads them anymore, and tests that patch handler._HAS_OTEL no longer disable telemetry.
Reviewed by Cursor Bugbot for commit 910e8a7. Configure here.
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 910e8a7. Configure here.
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
1 participant
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.
Replaces one flat span per call with the tree the TypeScript SDK emits, for
openai-messages.Cached tokens were absent entirely
OpenAI reports them under
input_tokens_details.cached_tokens, which nothing here read, so every span understated what a prompt-cached call actually reused.They now appear in
gen_ai.usage.cache_read.input_tokensand, unlike Anthropic, are not added on top of the input figure: OpenAI already counts them inside it, and adding them would double-count. Cache creation is always zero, because OpenAI has no such concept.Two things specific to this handler
It is the only one of the six that reports the model which actually answered rather than the one requested, on both the root and the chat spans. OpenAI resolves an alias like
gpt-4oto a dated snapshot, and this handler has the resolved value to hand.Finish reasons are derived, not mapped. The Responses API has no
finish_reasonfield, so the shared mapping table does not apply and is deliberately not imported. The value comes from a closed three-way check: a function call in the output meanstool_calls, an incomplete status meanslengthorcontent_filterdepending on the reported cause, a completed status meansstop, and anything else writes no attribute. The function-call check comes first, because status alone reportscompletedfor a turn that stopped to call a tool.Other changes
The streaming path gets a
finally, so a consumer that breaks out of the iteration no longer leaves the root span unended and unexported, taking the whole run out of AI Config Monitoring along with thefeature_flagevent it carries.Breaking change
The span is renamed from
openai.responsetoinvoke_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: 763 to 781.
Note
Overview
Breaking: replaces the flat
openai.responsespan with the TypeScript-aligned tree (invoke_agentroot, onechat {model}per turn, siblingexecute_toolspans). Prompt/completion content is off by default; passcapture_content=Trueto emit it.Adds a dedicated
spans.pymodule and rewires both blocking and streaming paths around shared helpers. OpenAI cached tokens now surface ascache_readwithout double-counting into input. Finish reasons are derived from Responses status/output (tool_calls/stop/length/content_filter). Root and chat spans report the model that answered, not just the requested alias.Streaming gets a
finallyso abandoned consumers still end and export spans (marked abandoned, not ERROR). Failed runs still report spend from completed turns.Reviewed by Cursor Bugbot for commit 910e8a7. Bugbot is set up for automated code reviews on this repo. Configure here.