-
Notifications
You must be signed in to change notification settings - Fork 440
feat(google_genai): [MLOB-2932] add apm tracing for google-genai #13650
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
base: main
Are you sure you want to change the base?
Conversation
|
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 275 ± 3 ms. The average import time from base is: 280 ± 4 ms. The import time difference between this PR and base is: -4.6 ± 0.2 ms. Import time breakdownThe following import paths have shrunk:
|
BenchmarksBenchmark execution time: 2025-06-17 23:33:48 Comparing candidate commit 23ab599 in PR branch Found 0 performance improvements and 3 performance regressions! Performance is the same for 558 metrics, 3 unstable metrics. scenario:iastaspectsospath-ospathbasename_aspect
scenario:iastaspectsospath-ospathjoin_aspect
scenario:iastaspectsospath-ospathnormcase_aspect
|
# https://cloud.google.com/vertex-ai/generative-ai/docs/model-garden/quickstart | ||
# for vertex, it seems like the best way to associate provider name with each call is based on the model name prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked this link and it doesn't seem to show the model names in the below context. Is this the correct link?
# https://cloud.google.com/vertex-ai/generative-ai/docs/model-garden/quickstart | |
# for vertex, it seems like the best way to associate provider name with each call is based on the model name prefix | |
# https://cloud.google.com/vertex-ai/generative-ai/docs/model-garden/quickstart | |
# VertexAI: the best way to associate provider name with each call is checking the model name prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't able to find a definitive source for providers.
Unlike what we initially thought, its hard to get the provider from the full path since it is not required and users can simply provide a model name. : https://github.com/googleapis/python-genai/blob/main/google/genai/models.py#L6005
So this code is a bit more best-effort. Gemini only exports google provided models whereas vertex lists supported models on the left side of the provided link. I just manually mapped supported models to providers.
Let me know if you have any suggestions on how to improve this part.
|
||
@pytest.mark.snapshot(token="tests.contrib.google_genai.test_google_genai.test_google_genai_generate_content_async") | ||
async def test_google_genai_generate_content_async(google_genai_vcr, genai): | ||
with google_genai_vcr.use_cassette("generate_content_async.yaml"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the request/response from sync and async generate_content methods are the same. Can we just reuse the same snapshot and cassette files? For the sake of minimizing test files we need to maintain
i.e.
@pytest.mark.snapshot(token="tests.contrib.google_genai.test_google_genai.test_google_genai_generate_content")
...
with google_genai_vcr.use_cassette("generate_content.yaml"):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was able to deduplicate cassettes and snapshots. but for snapshots, I had to add an ignore on resource to prevent this:
span mismatch on 'resource': got 'AsyncModels.generate_content_stream' which does not match expected 'Models.generate_content_stream'..
is this a good idea?
Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
This PR adds support for APM tracing of Google's GenAI Python SDK. Traces currently only contain UST tags as well as provider and model (LLMObs tracing of inputs/outputs and metadata will be done in a later PR).
Traced calls:
Checklist
Reviewer Checklist