openai: convert list-of-parts message content to semconv message parts - #358
openai: convert list-of-parts message content to semconv message parts#358HQidea wants to merge 1 commit into
Conversation
|
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pull request dashboard statusWaiting on reviewers · refreshed 2026-08-13 04:33 UTC Review the latest changes. Status above doesn't look right?
|
There was a problem hiding this comment.
Pull request overview
Fixes OpenAI Chat Completions message content handling so list-of-parts (multimodal / typed parts) is converted into semconv MessagePart models instead of being silently dropped, improving gen_ai.input.messages / gen_ai.output.messages fidelity in the OpenAI GenAI instrumentation.
Changes:
- Add per-part conversion for OpenAI
contentvalues (string or list-of-parts) into semconvText/Uri/Blob/File. - Apply the same conversion for both input messages and output messages.
- Add focused unit tests covering the supported OpenAI part variants and edge cases, plus a changelog fragment.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| instrumentation/opentelemetry-instrumentation-genai-openai/src/opentelemetry/instrumentation/genai/openai/utils.py | Adds OpenAI content-part → semconv MessagePart conversion and uses it in input/output message preparation. |
| instrumentation/opentelemetry-instrumentation-genai-openai/tests/test_prepare_input_messages_unit.py | Adds unit tests to pin expected conversion for text/image/audio/file/refusal parts and mixed cases. |
| instrumentation/opentelemetry-instrumentation-genai-openai/.changelog/358.fixed | Adds a changelog fragment documenting the bug fix. |
| def _decode_base64(data: str) -> bytes | None: | ||
| try: | ||
| return base64.b64decode(data) | ||
| except Exception: # pylint: disable=broad-exception-caught | ||
| return None |
There was a problem hiding this comment.
Narrowed to except ValueError (binascii.Error is a ValueError subclass). For context, the blanket form was copied from the anthropic package's _decode_base64; that one may deserve the same narrowing separately.
| def _content_to_parts(content: Any) -> list[MessagePart]: | ||
| """Convert an OpenAI message ``content`` value — a plain string or a | ||
| list of content parts — to semconv message parts.""" | ||
| if isinstance(content, str): | ||
| return [Text(content=content)] | ||
| if isinstance(content, Iterable): | ||
| parts: list[MessagePart] = [] | ||
| for item in content: | ||
| part = _convert_content_part(item) | ||
| if part is not None: | ||
| parts.append(part) | ||
| return parts | ||
| return [] |
There was a problem hiding this comment.
Added an explicit Mapping guard that records no parts — a bare dict is not a valid content shape, and iterating it would have produced Text parts for its keys. Covered by a unit test.
| @@ -0,0 +1 @@ | |||
| fix chat message content being dropped from `gen_ai.input.messages`/`gen_ai.output.messages` when it is a list of content parts | |||
There was a problem hiding this comment.
Reworded to match the existing fragments' style: capitalized sentence with RST double-backtick literals.
lmolkova
left a comment
lmolkova
left a comment
There was a problem hiding this comment.
LGTM, but let's write more realistic tests. Thanks!
| def test_string_content_is_single_text_part(): | ||
| messages = [{"role": "user", "content": "Say this is a test"}] | ||
|
|
||
| result = _prepare_input_messages(messages) |
There was a problem hiding this comment.
please write realistic tests against real instrumentation flow - this is a private method that might or might not be called
There was a problem hiding this comment.
Rewrote the coverage to drive the real instrumentation flow: test_chat_completion_multiturn_content_parts and test_chat_completion_multimodal_content_parts (in test_chat_completions.py) make instrumented chat.completions.create calls against cassettes and assert gen_ai.input.messages / gen_ai.output.messages on the exported span. Both fail against the unpatched code. A slim unit module remains only for degenerate content shapes a well-formed request wouldn't carry (unknown part types, mapping-shaped content, invalid base64, None).
|
Hi @HQidea — just a friendly reminder that this pull request is waiting on you. There are still items that need your attention. See the dashboard status comment for the full list. You don't need to push a code change to hand it back — replying to move each discussion forward is enough, whether that's answering a question, explaining why no change is needed, or asking a follow-up. The dashboard then automatically routes it back to reviewers. If you believe this pull request is incorrectly routed as waiting on the author, comment |
Chat Completions `content` may be a plain string or a list of typed
content parts. `_prepare_input_messages` (and `_prepare_output_messages`)
gated content on `_is_text_part`, which only accepts `str` or an iterable
of `str`, so the list form was dropped entirely and such messages were
recorded in `gen_ai.input.messages` as `{"role": ..., "parts": []}`.
Replace the gate with a per-part converter mirroring the anthropic
package's `convert_content_to_parts`:
- `{"type": "text"}` parts -> `Text` (one per part)
- `{"type": "image_url"}` -> `Uri` (modality `image`; data: URLs recorded
as sent, not decoded)
- `{"type": "input_audio"}` -> `Blob` (modality `audio`, base64-decoded)
- `{"type": "file"}` with `file_id` -> `File`
- `{"type": "refusal"}` -> `Text` (the message's user-visible text)
- unrecognized part types are skipped instead of nuking the message
Plain-string content behaves exactly as before. A list of plain strings
now yields one `Text` part per string (previously the whole list was
stringified into a single part). Mapping-shaped content is explicitly
guarded (iterating it would have produced Text parts for its keys), and
base64 decoding failures only catch ValueError.
Coverage is driven through the real instrumentation flow
(test_chat_completion_multiturn_content_parts and
test_chat_completion_multimodal_content_parts, cassette-based, asserting
gen_ai.input.messages on the exported span); a slim unit module keeps
the degenerate shapes a well-formed request would not carry.
Implemented with Claude (Anthropic) assistance.
Fixes open-telemetry#357
b8e3bb9 to
39aa107
Compare
|
/easycla |
|
@lmolkova sorry for the slow follow-up, and thanks for the review — updated as suggested. Ready for another look whenever you have a chance. |
Description
Chat Completions
contentmay be a plain string or a list of typed content parts (the standard OpenAI shape for multi-part text and multimodal requests)._prepare_input_messages(and_prepare_output_messages) gated content on_is_text_part, which only acceptsstror an iterable ofstr, so the list form was silently dropped and such messages were recorded ingen_ai.input.messagesas{"role": ..., "parts": []}even with content capture enabled.This replaces the gate with a per-part converter mirroring the anthropic package's
convert_content_to_parts, using the semconv part models fromopentelemetry-util-genai:{"type": "text"}parts → oneTextpart each{"type": "image_url"}→Uri(modalityimage;data:URLs are recorded as sent, not decoded){"type": "input_audio"}→Blob(modalityaudio, base64-decoded, mime type fromformat){"type": "file"}with afile_id→File{"type": "refusal"}→Text(the refusal string is the message's user-visible text)Behavior notes:
Text(content=<string>)).Textpart per string; previously the whole list was stringified into a single part. That shape is not a valid OpenAI request anyway; the new behavior seems strictly more useful.Textparts for its keys); base64 decode failures catchValueErroronly.get_property_value, so both TypedDict/dict parts and attribute objects work.Fixes #357
Type of change
How has this been tested?
test_chat_completion_multiturn_content_parts(multi-turn conversation whose messages use{"type": "text"}part lists) andtest_chat_completion_multimodal_content_parts(text + remote/data image_url + input_audio + file parts in one request) drive instrumentedchat.completions.createcalls against cassettes and assertgen_ai.input.messages/gen_ai.output.messageson the exported span. Both fail against the unpatched code (verified by revertingutils.pytomain).test_prepare_input_messages_unit.py) covers degenerate content shapes a well-formed request would not carry: unknown part types, mapping-shaped content, invalid base64 audio, attribute-object parts, list-of-strings,None.pytest tests/→ 262 passed, 8 skipped.ruff checkandruff format --checkclean with the repo-pinned ruff.Checklist