Skip to content

Conversation

vgrozdanic
Copy link
Member

@vgrozdanic vgrozdanic commented Oct 6, 2025

Adds support for python-genai integrations. It supports both sync and async clients, and both regular and streaming modes for interacting with models and building agents.

Closes PY-1733: Add agent monitoring support for google-genai

@vgrozdanic vgrozdanic force-pushed the vg/add-python-genai-integration branch 7 times, most recently from e265d62 to 48a0c71 Compare October 7, 2025 10:57
@vgrozdanic vgrozdanic marked this pull request as ready for review October 7, 2025 10:58
@vgrozdanic vgrozdanic requested a review from a team as a code owner October 7, 2025 10:58
Copy link

linear bot commented Oct 7, 2025

("frequency_penalty", SPANDATA.GEN_AI_REQUEST_FREQUENCY_PENALTY),
("seed", SPANDATA.GEN_AI_REQUEST_SEED),
]:
if hasattr(config, param):
Copy link

Choose a reason for hiding this comment

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

Potential bug: The function set_span_data_for_request calls hasattr(config, ...) without checking if config is None, causing an AttributeError if the optional config parameter is omitted.
  • Description: In set_span_data_for_request, the config object is retrieved from kwargs using kwargs.get("config"). If a user does not provide a config object in their API call, which is a valid use case, config will be None. The subsequent loop attempts to access attributes on this None object with hasattr(config, param), which raises an AttributeError. Because this integration code runs within a user's application, this unhandled exception will crash the application.

  • Suggested fix: Before the loop that iterates over configuration parameters in set_span_data_for_request, add a check to ensure config is not None. If config is None, the entire loop that sets span data from the config should be skipped.
    severity: 0.85, confidence: 0.98

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Contributor

Choose a reason for hiding this comment

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

hasattr(None, 'something') won't raise an AttributeError, it'll simply be False

Copy link
Contributor

@sentrivana sentrivana left a comment

Choose a reason for hiding this comment

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

TY for the PR Vjeran!

I'll do a full review shortly, just a couple of things I noticed on the fly:

Lmk if anything is unclear.

cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

@sentrivana sentrivana left a comment

Choose a reason for hiding this comment

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

👋🏻 Back with a big review. Overall looks nice and clean! Don't be fooled by the number of comments, it's just that it's a big PR, and I'm pointing out some stuff multiple times.

"sentry_sdk.integrations.flask.FlaskIntegration",
"sentry_sdk.integrations.gql.GQLIntegration",
"sentry_sdk.integrations.graphene.GrapheneIntegration",
"sentry_sdk.integrations.google_genai.GoogleGenAIIntegration",
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding the integration here will make it auto-enabling (the user wouldn't have to add GoogleGenAIIntegration manually to their init) -- however for new integrations, I strongly prefer to make them opt-in first, in order to iron out issues, and only make them auto-enabling after some time.

setup.py Outdated
"statsig": ["statsig>=0.55.3"],
"tornado": ["tornado>=6"],
"unleash": ["UnleashClient>=6.0.1"],
"google-genai": ["google-genai"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"google-genai": ["google-genai"],
"google-genai": ["google-genai>=1.0.0"],

We should also have the lower bound here, to make sure it's not possible to install the sdk with an unsupported version of google-genai.

from sentry_sdk.tracing import Span


def prepare_generate_content_args(args, kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this function to google_genai/utils.py since it's also used by the non-streaming wrappers?

("frequency_penalty", SPANDATA.GEN_AI_REQUEST_FREQUENCY_PENALTY),
("seed", SPANDATA.GEN_AI_REQUEST_SEED),
]:
if hasattr(config, param):
Copy link
Contributor

Choose a reason for hiding this comment

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

hasattr(None, 'something') won't raise an AttributeError, it'll simply be False

Comment on lines 371 to 372
if kwargs.get("stream", False):
span.set_data(SPANDATA.GEN_AI_RESPONSE_STREAMING, True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we also want to add the attribute for non-streaming responses?

Suggested change
if kwargs.get("stream", False):
span.set_data(SPANDATA.GEN_AI_RESPONSE_STREAMING, True)
span.set_data(SPANDATA.GEN_AI_RESPONSE_STREAMING, kwargs.get("stream") or False)

@vgrozdanic vgrozdanic force-pushed the vg/add-python-genai-integration branch from f0f32d2 to 9e03a64 Compare October 8, 2025 15:37
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: Token Count Inflation in Streaming Responses

The accumulate_streaming_response function incorrectly sums token counts from streaming chunks. Since these counts (especially input and cached tokens) are often cumulative or constant across chunks, summing them leads to inflated and inaccurate usage metrics.

sentry_sdk/integrations/google_genai/streaming.py#L507-L526

https://github.com/getsentry/sentry-python/blob/1c5aa1c08955f7271a7d8b5397874e616b2f3819/sentry_sdk/integrations/google_genai/streaming.py#L507-L526

sentry_sdk/integrations/google_genai/streaming.py#L69-L76

# Accumulate token usage
extracted_usage_data = extract_usage_data(chunk)
total_input_tokens += extracted_usage_data["input_tokens"]
total_output_tokens += extracted_usage_data["output_tokens"]
total_cached_tokens += extracted_usage_data["input_tokens_cached"]
total_reasoning_tokens += extracted_usage_data["output_tokens_reasoning"]
total_tokens += extracted_usage_data["total_tokens"]

Fix in Cursor Fix in Web


texts = []
for candidate in response.candidates:
if not hasattr(candidate, "content") or not hasattr(candidate.content, "parts"):
continue
Copy link

Choose a reason for hiding this comment

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

Bug: Nested Attribute Access Fails

The code can raise an AttributeError when accessing nested attributes, such as candidate.content.parts. This occurs if an intermediate attribute (like content) is missing, or if the final attribute (parts) is None when an iterable is expected. This pattern is visible in _extract_response_text.

Additional Locations (1)

Fix in Cursor Fix in Web

@vgrozdanic
Copy link
Member Author

@sentrivana i have made the changes regarding all the feedback, and did some more refactoring, to make the code more consistent and a bit easier to read.

Also, now the pipeline is finally green 🥳

@vgrozdanic vgrozdanic requested a review from sentrivana October 9, 2025 12:39
@sentrivana
Copy link
Contributor

Taking a look at this now and will take it for a spin with the test scripts 👀

Copy link
Contributor

@sentrivana sentrivana left a comment

Choose a reason for hiding this comment

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

TY for addressing the feedback, let's get this merged!

Is there already a docs PR/do you plan to make one or should I?

@vgrozdanic
Copy link
Member Author

Thank you!

Is there already a docs PR/do you plan to make one or should I?

We will take care of it, Ogi will prepare both the in-product onboarding and the docs, we track it here:

@vgrozdanic vgrozdanic merged commit 149a7da into master Oct 10, 2025
111 of 112 checks passed
@vgrozdanic vgrozdanic deleted the vg/add-python-genai-integration branch October 10, 2025 10:09
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.

2 participants