(Openinference Migration: Langchain): Capture multimodal image content (OpenAI image_url and Anthropic image blocks) as Blob/Uri message parts. - #296
Conversation
Pull request dashboard statusWaiting on the author · refreshed 2026-08-14 16:40 UTC Respond to 5 review items (e.g. link a commit, explain why not, ask a follow-up): Status above doesn't look right?
|
There was a problem hiding this comment.
Pull request overview
Adds multimodal image support to the LangChain GenAI instrumentation by converting OpenAI image_url and Anthropic image blocks into Blob / Uri message parts, and extends the unit test suite to validate the new parsing behavior.
Changes:
- Add multimodal image parsing helpers (
_media_part,_image_from_url) to convert LangChain image blocks intoBlob/Uriparts. - Extend callback-handler tests to cover OpenAI and Anthropic image content shapes (including data URIs and base64 sources).
- Add a changelog fragment documenting the new capability.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| instrumentation/opentelemetry-instrumentation-genai-langchain/src/opentelemetry/instrumentation/genai/langchain/utils.py | Adds image block parsing and base64 decoding to emit Blob/Uri message parts. |
| instrumentation/opentelemetry-instrumentation-genai-langchain/tests/test_callback_handler.py | Adds unit tests for data-URI, HTTP-URI, and Anthropic image/source parsing into message parts. |
| instrumentation/opentelemetry-instrumentation-genai-langchain/.changelog/296.added | Documents the new multimodal image capture support. |
aa26c4f to
ed3949d
Compare
d72b64e to
501725d
Compare
|
@rads-1996 One thing I wanted to check on ordering. In That means a config that doesn't record content still copies large images. If that isn't intended, either skip the media decode when capture is off, or add a test asserting If the parse is meant to run unconditionally so the completion hook has something to work with, that's fine, I just wanted to confirm that's the assumption. |
@AgentGymLeader Thanks for the callout. I have gated decoding and will skipped when the content capture mode is |
9cd26fe to
1b98bc4
Compare
aaea6b5 to
d9fe119
Compare
9ae1c71 to
0c19a81
Compare
| gen_ai_attributes as GenAIAttributes, | ||
| ) | ||
| from opentelemetry.util.genai.types import ( | ||
| Blob, |
There was a problem hiding this comment.
it's the first time we're using this type in this repo, mind renaming it to BlobPart? Same with Uri, can we rename it to UriPart - that's how they are called in semconv and it seems we're inconsistent - https://github.com/open-telemetry/semantic-conventions-genai/blob/main/docs/gen-ai/non-normative/models.py
There was a problem hiding this comment.
Sounds good. I will make changes in the utils. I believe the same change is needed for some other data classes as well. I think I will make a separate PR to address that.
There was a problem hiding this comment.
@lmolkova Blob and Uri both have previously been used in other instrumentations, Blob has been used in the anthropic and google-genai instrumentation and Uri in google-genai. I have a PR out which aligns the names with semconv - #365. Should we have backward compatibility aliases for downstream users with deprecation messages?
There was a problem hiding this comment.
oh, good catch! not sure how I missed them. Yes, let's do aliases
eb9e373 to
c57a7b7
Compare
…mage` blocks) as `Blob`/`Uri` message parts.
f0b8684 to
837f846
Compare
| invocation.output_messages = make_last_output_message( | ||
| outputs, capture_content | ||
| ) |
There was a problem hiding this comment.
why not
| invocation.output_messages = make_last_output_message( | |
| outputs, capture_content | |
| ) | |
| if capture_content: | |
| invocation.output_messages = make_last_output_message( | |
| outputs | |
| ) |
?
| flattened: list[BaseMessage] = [msg for sub in messages for msg in sub] | ||
| input_messages = to_input_messages(flattened) | ||
| capture_content = self._telemetry_handler.should_capture_content() | ||
| input_messages = to_input_messages(flattened, capture_content) |
There was a problem hiding this comment.
| input_messages = to_input_messages(flattened, capture_content) | |
| if capture_content: | |
| input_messages = to_input_messages(flattened) |
?
| return ContentCapturingMode.NO_CONTENT | ||
|
|
||
|
|
||
| def decode_base64(data: str, capture_content: bool = False) -> bytes | None: |
There was a problem hiding this comment.
does it need to be public? also it seems we can just avoid calling to_input / to_output if content is disabled and add a docstring for public utils methods that is expected from caller to check if content is enabled and not call this method if it's not.
| def decode_base64(data: str, capture_content: bool = False) -> bytes | None: | |
| def _decode_base64(data: str) -> bytes | None: |
Description
Part of the langchain migration PRs - #272
Type of change
Please delete options that are not relevant.
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide
instructions so we can reproduce. List any relevant details for your test
configuration.
Checklist
See CONTRIBUTING.md
for the style guide, changelog guidance, and more.