Skip to content

Conversation

@arloc
Copy link

@arloc arloc commented Nov 11, 2025

Description

This PR fixes a bug in the opentelemetry-instrumentation-google-genai package where passing an uninstantiated Pydantic model class (instead of an instance) to dict conversion functions would cause a TypeError:

BaseModel.model_dump() missing 1 required positional argument: 'self'

The issue occurred in three functions that attempt to convert objects to dictionaries:

  • _to_dict() in generate_content.py
  • _to_otel_value() in tool_call_wrapper.py
  • _flatten_dict() in dict_util.py

These functions check hasattr(value, "model_dump") which returns True for both Pydantic classes and instances, but model_dump() is an instance method that cannot be called on the class itself.

Solution:

  • Added isinstance(value, type) check to distinguish between classes and instances before calling model_dump()
  • When a Pydantic class is detected, the code now calls model_json_schema() (a classmethod) to get the schema representation
  • Added comprehensive test coverage for both class and instance scenarios

Fixes #3596

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Added unit tests to verify the fix:

  1. test_flatten_with_pydantic_class_not_instance in tests/utils/test_dict_util.py

    • Verifies that passing a Pydantic class (not instance) doesn't raise the model_dump() error
    • Confirms the class is properly serialized using model_json_schema()
  2. test_handles_pydantic_class_not_instance in tests/utils/test_tool_call_wrapper.py

    • Tests the same scenario for the tool call wrapper functionality
    • Ensures proper handling of Pydantic model classes in tool call contexts

Does This PR Require a Core Repo Change?

  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated (not needed - internal implementation fix)

@arloc arloc requested a review from a team as a code owner November 11, 2025 03:42
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 11, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: arloc / name: Davi Campos (6045ec2)

@arloc arloc marked this pull request as draft November 11, 2025 03:50
@arloc arloc marked this pull request as ready for review November 11, 2025 03:51
@arloc arloc force-pushed the fix-google-genai-pydantic branch from 3f10d20 to eeed5d7 Compare November 11, 2025 03:54
@arloc arloc force-pushed the fix-google-genai-pydantic branch from eeed5d7 to 6045ec2 Compare November 11, 2025 03:57
@aabmass
Copy link
Member

aabmass commented Nov 11, 2025

@DylanRussell this covers some of the same things as #3905, but also includes the JSON schema. WDYT?

@DylanRussell
Copy link
Contributor

I just put str(Class) when I came across this. This also works, but it's a lot more information.. Do we want the whole json schema each time ?

@aabmass
Copy link
Member

aabmass commented Nov 11, 2025

We have a semconv PR for tool schemas open-telemetry/semantic-conventions#2942. I think it's generally useful but maybe best to hold off until we have conventions. @arloc was capturing the schema in particular important to you or mostly just avoiding this exception?

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.

BaseModel.model_dump() called on class instead of instance in opentelemetry-instrumentation-google-genai

7 participants