Skip to content

fix(anthropic): serialize assistant message pydantic models #3041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 24, 2025

Conversation

dinmukhamedm
Copy link
Collaborator

@dinmukhamedm dinmukhamedm commented Jun 23, 2025

Fixes #3040

  • I have added tests that cover my changes.
  • If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
  • PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
  • (If applicable) I have updated the documentation accordingly.

Important

Enhances assistant message serialization in opentelemetry-instrumentation-anthropic by using model_as_dict() and adds tests for image content handling.

  • Behavior:
    • Updates _dump_content in __init__.py to use model_as_dict() for serializing assistant messages.
    • Handles different Pydantic model versions in model_as_dict() in utils.py.
  • Testing:
    • Adds test_anthropic_image_with_history in test_messages.py to test image content handling in message history.
    • Includes a new VCR cassette test_anthropic_image_with_history.yaml for the test.
  • Misc:
    • Adds image_content_block in test_messages.py for image data setup.

This description was created by Ellipsis for 6b200db. You can customize this summary. It will automatically update as commits are pushed.

@dinmukhamedm dinmukhamedm requested a review from nirga June 23, 2025 11:58
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed everything up to 6b200db in 2 minutes and 11 seconds. Click for details.
  • Reviewed 460 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_Hxc7xjWpqiaJU1Ii

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@@ -134,16 +135,16 @@ async def _dump_content(message_index, content, span):
elif isinstance(content, list):
# If the content is a list of text blocks, concatenate them.
# This is more commonly used in prompt caching.
if all([item.get("type") == "text" for item in content]):
return "".join([item.get("text") for item in content])
if all([model_as_dict(item).get("type") == "text" for item in content]):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix to use model_as_dict() for serialization. Consider caching the conversion per item so that each list comprehension doesn’t call model_as_dict(item) multiple times. For example, assign the result of model_as_dict(item) to a variable within the loop before checking its 'type'. This minor optimization improves readability and performance.

@nirga nirga merged commit c2c1697 into traceloop:main Jun 24, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Bug Report: Anthropic instrumentation fails to serialize pydantic messages from previous assistant responses
2 participants