fix(genai-openai): capture multimodal content parts as semconv media parts - #522
fix(genai-openai): capture multimodal content parts as semconv media parts#522FeelKyoun wants to merge 5 commits into
Conversation
…essages
When a chat message uses the OpenAI content-part array form
([{"type": "text", ...}, {"type": "image_url", ...}]), _is_text_part
rejects it and _prepare_input_messages had no fallback branch, so the
message was captured with empty parts — the text part was dropped along
with the non-text parts, even with content capture opted in.
Map "text" parts to TextPart and preserve any other part type
("image_url", "input_audio", ...) as a GenericPart carrying the
provider-specific type discriminator and payload, per the GenericPart
contract in opentelemetry-util-genai.
Fixes open-telemetry#521
Pull request dashboard statusWaiting on reviewers · refreshed 2026-09-02 01:30 UTC Review the latest changes. Status above doesn't look right?
|
There was a problem hiding this comment.
Pull request overview
This PR fixes captured gen_ai.input.messages for opentelemetry-instrumentation-genai-openai when OpenAI chat messages use the multimodal “content-part array” form, ensuring text parts are preserved and non-text parts are retained via GenericPart instead of being dropped.
Changes:
- Add
_extract_content_partsto map OpenAI content-part arrays intoTextPartandGenericPartinstances. - Update
_prepare_input_messagesto use the new extraction logic for assistant and non-tool roles whencontentis iterable. - Add focused unit tests and a changelog fragment for the multimodal capture behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| instrumentation/opentelemetry-instrumentation-genai-openai/src/opentelemetry/instrumentation/genai/openai/utils.py | Adds content-part extraction helpers and wires them into input message capture. |
| instrumentation/opentelemetry-instrumentation-genai-openai/tests/test_prepare_input_messages_unit.py | Adds unit tests covering multimodal content-part arrays and JSON serializability. |
| instrumentation/opentelemetry-instrumentation-genai-openai/.changelog/522.fixed | Documents the bug fix in a towncrier fragment. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if _is_text_part(content): | ||
| chat_message.parts.append(TextPart(content=str(content))) | ||
| elif content is not None and isinstance(content, Iterable): | ||
| chat_message.parts += _extract_content_parts(content) |
There was a problem hiding this comment.
Good catch — addressed in the latest commit. content is now routed through a single _content_to_parts helper that materializes non-string, non-mapping iterables exactly once before branching, so single-pass iterables are no longer partially consumed, and mappings keep the previous _is_text_part behavior (string-keyed mappings captured as str(mapping)) instead of being iterated as content-part arrays. Added unit tests for both cases (generator content and mapping content).
There was a problem hiding this comment.
Correction to my earlier reply here: "materialize once" was the wrong fix. The SDK accepts any iterable for content and materializes it itself, so consuming a generator inside the instrumentation (which runs before wrapped()) drained the caller's input and the SDK sent content: []. As of 7d339a8 only Sequence content is walked; other iterables are left untouched and simply not captured, and the unit test now asserts the generator is still unconsumed afterwards.
…hape Address review feedback: _is_text_part could partially consume a single-pass iterable before _extract_content_parts iterated the remainder, and mappings fell into the content-part-array branch via key iteration. Route content through _content_to_parts, which materializes non-string, non-mapping iterables exactly once and keeps the previous behavior for strings and string-keyed mappings.
lmolkova
left a comment
There was a problem hiding this comment.
Thank for the contribution! Please use specialized parts for images, URLs and others.
Also please consider adding conformance test for new modalities - this is where we validate value schema.
| ) | ||
| else: | ||
| parts.append( | ||
| GenericPart(type=str(item_type), value=_as_plain_value(item)) |
There was a problem hiding this comment.
image_url parts should map to semconv standard media parts (UriPart or BlobPart) rather than GenericPart.
opentelemetry-util-genai provides the image_from_url helper in opentelemetry.util.genai.utils for this exact purpose (it handles external URLs as UriPart and base64 data URLs as BlobPart). Please reuse image_from_url(url) here. GenericPart should only be used for provider-specific parts that have no standard semconv representation.
There was a problem hiding this comment.
Done in d7e6b9a — image_url parts now go through image_from_url(url): an external URL becomes a UriPart and a data: URL becomes a BlobPart.
While at it I mapped the other standard OpenAI parts too, so GenericPart is only used for typed parts that have no semconv representation:
input_audio→ audioBlobPart(mime type derived fromformat)file→FilePartforfile_id, or a documentBlobPartfor inlinefile_data
| return [TextPart(content=content)] | ||
| if content is None: | ||
| return [] | ||
| if isinstance(content, Mapping): |
There was a problem hiding this comment.
Stringifying mappings as TextPart(content=str(content)) turns arbitrary dicts into malformed text parts like TextPart(content="{'unexpected': 'shape'}").
Similarly, all(isinstance(item, str) for item in items): return [TextPart(content=str(items))] produces stringified Python list syntax (TextPart(content="['a', 'b']")). Iterables should route through _extract_content_parts so string items become individual TextPart instances.
There was a problem hiding this comment.
Agreed — that was a bad carry-over from the old _is_text_part behaviour. Both stringifications are gone in d7e6b9a:
- a bare mapping is treated as a single content part, so
{"type": "text", "text": ...}becomes aTextPartand an untyped dict is dropped instead of becoming a bogus text part; - string items in a list each become their own
TextPart.
Everything now routes through one _convert_content_part and the iterable is consumed exactly once.
Address review feedback on open-telemetry#522: - `image_url` parts map to `UriPart` / `BlobPart` via the shared `image_from_url` helper, `input_audio` to an audio `BlobPart`, and `file` to `FilePart` (file_id) / `BlobPart` (inline file_data). `GenericPart` is now used only for typed parts with no semconv representation. - Stop stringifying content: a bare mapping is treated as a single content part and string items become individual `TextPart`s instead of `str(list)` / `str(dict)` text parts. - Add a `MultimodalScenario` conformance test (text + external image URL + inline base64 image) recorded against gpt-4o-mini and validated with weaver live-check, asserting `text` / `uri` / `blob` parts round-trip onto the input message. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QsXFvtQoFP6DwdBgyEA3xB
|
Thanks for the review, @lmolkova — addressed in d7e6b9a:
Verification: the full openai conformance suite (9 scenarios) passes locally in replay mode with weaver 0.25.1, the unit suite passes (260 passed), and pre-commit is clean.
|
Address review feedback on open-telemetry#522: - `image_url` parts map to `UriPart` / `BlobPart` via the shared `image_from_url` helper, `input_audio` to an audio `BlobPart`, and `file` to `FilePart` (file_id) / `BlobPart` (inline file_data). `GenericPart` is now used only for typed parts with no semconv representation. - Stop stringifying content: a bare mapping is treated as a single content part and string items become individual `TextPart`s instead of `str(list)` / `str(dict)` text parts. - Add a `MultimodalScenario` conformance test (text + external image URL + inline base64 image) recorded against gpt-4o-mini and validated with weaver live-check, asserting `text` / `uri` / `blob` parts round-trip onto the input message. Claude-Session: https://claude.ai/code/session_01QsXFvtQoFP6DwdBgyEA3xB
|
/easycla |
508971c to
d7e6b9a
Compare
…capturing content Self-review follow-ups on open-telemetry#522: - Only walk `Sequence` content as a content-part array. Iterating any other iterable (e.g. a generator, which the SDK accepts and materializes itself) drained the caller's input before the request was sent, so the SDK ended up sending empty content. Such content is now left untouched and not captured. - Content conversion never raises: each part is converted under a broad guard and skipped on failure, and `GenericPart.value` is made JSON-safe (pydantic `model_dump(mode="json")`, `json` round-trip with `default=str`) so span export cannot fail on `datetime`/`set`/`Enum` payloads. - `file_data` is base64 per the SDK: decode plain base64 into a document `BlobPart` (mime type from `filename`), keep data-URL support, and use `filename` for `FilePart.mime_type` too. - `input_audio.data` given as a data URL is decoded instead of dropped; unmapped audio formats get `mime_type=None` instead of a synthesized type. - `get_property_value` accepts any `Mapping`, so non-dict mappings are no longer accepted as content and then silently dropped. - Non-string `text` values are dropped rather than stringified. - Replace the dispatch table with an if-chain matching the sibling instrumentations and trim docstrings. Claude-Session: https://claude.ai/code/session_01QsXFvtQoFP6DwdBgyEA3xB
|
Follow-up in 7d339a8 after a self-review pass on
Unit tests: 24/24 (new cases for each of the above); full openai suite 270 passed; conformance suite (9 scenarios incl. |
Description
Fixes #521.
When a chat message uses the OpenAI content-part array form for multimodal input, e.g.
{"role": "user", "content": [ {"type": "text", "text": "What is in this image?"}, {"type": "image_url", "image_url": {"url": "https://example.com/cat.png"}}, ]}_is_text_partrejects the array (it only acceptsstror an iterable ofstr) and_prepare_input_messageshad no fallback branch, so the message was captured with emptyparts— the text part was silently dropped along with the non-text parts, even when the user explicitly opted in to content capture.This PR routes message
contentthrough a single_content_to_parts/_convert_content_partpath used by both theassistantanduser/system/fallback branches of_prepare_input_messages, mapping each OpenAI content part to its semconv message part:str/{"type": "text", ...}TextPartimage_url(external URL)UriPart(viaimage_from_url)image_url(data:URL)BlobPart(viaimage_from_url)input_audio(base64 or data URL)BlobPart(mime type forwav/mp3, elseNone)filewithfile_idFilePartfilewith inlinefile_data(base64 or data URL)BlobPart(mime type fromfilename)GenericPart(provider-specific, no semconv mapping)A bare string is a single text part and a bare mapping is a single content part. Only sequences (
list/tuple) are walked as content-part arrays: the SDK accepts any iterable and materializes it itself, so iterating a generator here would drain the caller's input before the request is sent; such content is left untouched and not captured. Items without atypediscriminator,textparts whosetextis not a string, and media parts whose payload can't be decoded are skipped.Content capture never raises into the caller: each part is converted under a guard and skipped on failure, and
GenericPart.valueis made JSON-safe (pydanticmodel_dump(mode="json"),jsonround-trip withdefault=str) so span export cannot fail ondatetime/set/Enumpayloads.Behavior for plain string content,
Nonecontent, tool messages, and tool calls is unchanged.Type of change
How Has This Been Tested?
MultimodalScenariointests/conformance/multimodal.py— a chat turn with text + an external image URL + an inline base64 PNG, cassette recorded against the realgpt-4o-miniAPI, validated with weaver live-check, and assertingtext/uri(image) /blob(image) parts land on the input message. Full openai conformance suite (9 scenarios) passes in replay mode with weaver 0.25.1.tests/test_prepare_input_messages_unit.pycovers plain string content (regression),image_urlURL →UriPart,image_urldata URL →BlobPart,input_audio(base64 / data URL / unknown format) →BlobPart,file(file_id/ data-URLfile_data/ plain-base64file_data/ undecodable), unknown typed part →GenericPartwith JSON-safe non-aliased value, untyped or non-string-text parts dropped, a part that raises during conversion is skipped without breaking the message, assistant history,Nonecontent, string lists, tuples, dict and non-dict mapping content, generator content left unconsumed, and JSON serializability viagen_ai_json_dumps— 24/24 pass.pre-commit(ruff / ruff-format) clean on changed files.Does This PR Require a Core Repo Change?
Checklist
.changelog/522.fixed)🤖 Generated with Claude Code
https://claude.ai/code/session_01QsXFvtQoFP6DwdBgyEA3xB