Repository navigation
Conversation
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Pull request dashboard statusWaiting on the author · refreshed 2026-10-06 06:59 UTC Respond to 2 review items (e.g. link a commit, explain why not, ask a follow-up): Status above doesn't look right?
|
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Signed-off-by: 1fanwang <1fannnw@gmail.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Add integer type assertions for exported usage attributes in the regression tests.
Pull request overview
Adds OpenAI Chat Completions cache-write and modality token usage recording for synchronous, asynchronous, and streaming calls.
Changes:
- Extracts detailed usage fields into telemetry.
- Adds local HTTP/SSE regression coverage.
- Adds a changelog entry.
File summaries
| File | Description |
|---|---|
instrumentation/opentelemetry-instrumentation-genai-openai/tests/test_chat_token_usage.py |
Tests usage-detail variants across call modes. |
instrumentation/opentelemetry-instrumentation-genai-openai/src/opentelemetry/instrumentation/genai/openai/utils.py |
Extracts detailed usage fields. |
instrumentation/opentelemetry-instrumentation-genai-openai/src/opentelemetry/instrumentation/genai/openai/patch.py |
Applies extraction to non-streaming responses. |
instrumentation/opentelemetry-instrumentation-genai-openai/src/opentelemetry/instrumentation/genai/openai/chat_wrappers.py |
Applies extraction to streaming responses. |
instrumentation/opentelemetry-instrumentation-genai-openai/.changelog/603.added |
Documents the feature. |
Review details
Suppressed comments (1)
instrumentation/opentelemetry-instrumentation-genai-openai/tests/test_chat_token_usage.py:222
- This only checks equality, so a
boolorfloatusage value would still pass (True == 1and1.0 == 1). The new semconv usage attributes must be integers; assert the types of the exported values here so the regression test catches invalid attribute values.
assert actual == expected
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
lmolkova
left a comment
There was a problem hiding this comment.
Please make sure to follow existing test practices in this repo
| ) | ||
| invocation.text_input_tokens = get_property_value( | ||
| prompt_details, "text_tokens" | ||
| ) |
There was a problem hiding this comment.
OpenAI Chat Completions does not expose text_tokens or image_tokens in prompt_tokens_details or completion_tokens_details. Only cache_write_tokens and audio_tokens exist on PromptTokensDetails, and audio_tokens on CompletionTokensDetails. Please remove the non-existent modality mappings.
There was a problem hiding this comment.
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Signed-off-by: 1fanwang <1fannnw@gmail.com>
|
Hi @1fanwang — just a friendly reminder that this pull request is waiting on you. The dashboard status comment has the open items and is kept current.
|
| invocation.thinking_tokens = get_property_value( | ||
| obj=completion_details, property_name="reasoning_tokens" | ||
| ) | ||
| invocation.set_input_tokens( |
There was a problem hiding this comment.
OpenAI Chat Completions does not expose text_tokens or image_tokens in prompt_tokens_details or completion_tokens_details. Only audio_tokens exists. Please remove text and image from the modality extractions and only pass audio.
Signed-off-by: 1fanwang <1fannnw@gmail.com>
|
This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment. |
Assisted-by: GitHub Copilot CLI (Auto) Signed-off-by: 1fanwang <1fannnw@gmail.com>
Description
Chat Completions telemetry omits cache-write and audio token counts, so users cannot observe those reported usage breakdowns. This records them for normal and streamed calls without changing aggregate input/output totals; streamed usage is applied during cleanup.
Text/image mappings are excluded as requested in review. The SDK schema defines those fields, but I have not verified that Chat Completions returns them.
Fixes #603.
Type of change
How has this been tested?
Testing Done
The real OpenAI 3.13.0 SDK parsed JSON/SSE through a mock HTTP transport on macOS with Python 3.14.7. These synthetic responses check extraction, not provider behavior. No live API call was made.
Save the source below as sdk_probe.py. From the repository root:
Both probe runs exited 0. Before:
{"path": "sync-response", "usage": {"gen_ai.usage.cache_read.input_tokens": 5, "gen_ai.usage.input_tokens": 100, "gen_ai.usage.output_tokens": 20, "gen_ai.usage.reasoning.output_tokens": 3}} {"path": "sync-stream", "usage": {"gen_ai.usage.cache_read.input_tokens": 5, "gen_ai.usage.input_tokens": 100, "gen_ai.usage.output_tokens": 20, "gen_ai.usage.reasoning.output_tokens": 3}} {"path": "async-response", "usage": {"gen_ai.usage.cache_read.input_tokens": 5, "gen_ai.usage.input_tokens": 100, "gen_ai.usage.output_tokens": 20, "gen_ai.usage.reasoning.output_tokens": 3}} {"path": "async-stream", "usage": {"gen_ai.usage.cache_read.input_tokens": 5, "gen_ai.usage.input_tokens": 100, "gen_ai.usage.output_tokens": 20, "gen_ai.usage.reasoning.output_tokens": 3}}After:
{"path": "sync-response", "usage": {"gen_ai.usage.audio.input_tokens": 10, "gen_ai.usage.audio.output_tokens": 2, "gen_ai.usage.cache_read.input_tokens": 5, "gen_ai.usage.cache_write.input_tokens": 10, "gen_ai.usage.input_tokens": 100, "gen_ai.usage.output_tokens": 20, "gen_ai.usage.reasoning.output_tokens": 3}} {"path": "sync-stream", "usage": {"gen_ai.usage.audio.input_tokens": 10, "gen_ai.usage.audio.output_tokens": 2, "gen_ai.usage.cache_read.input_tokens": 5, "gen_ai.usage.cache_write.input_tokens": 10, "gen_ai.usage.input_tokens": 100, "gen_ai.usage.output_tokens": 20, "gen_ai.usage.reasoning.output_tokens": 3}} {"path": "async-response", "usage": {"gen_ai.usage.audio.input_tokens": 10, "gen_ai.usage.audio.output_tokens": 2, "gen_ai.usage.cache_read.input_tokens": 5, "gen_ai.usage.cache_write.input_tokens": 10, "gen_ai.usage.input_tokens": 100, "gen_ai.usage.output_tokens": 20, "gen_ai.usage.reasoning.output_tokens": 3}} {"path": "async-stream", "usage": {"gen_ai.usage.audio.input_tokens": 10, "gen_ai.usage.audio.output_tokens": 2, "gen_ai.usage.cache_read.input_tokens": 5, "gen_ai.usage.cache_write.input_tokens": 10, "gen_ai.usage.input_tokens": 100, "gen_ai.usage.output_tokens": 20, "gen_ai.usage.reasoning.output_tokens": 3}}Reproducer source: sdk_probe.py
Local conformance scenarios skipped because Weaver is unavailable.
Checklist
See CONTRIBUTING.md
for the style guide, changelog guidance, and more.