Repository navigation
fix(adk): let Google GenAI own LLM spans for ADK model calls - #897
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5964cf6cb8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| with _start_stream_span( | ||
| name="llm_call", | ||
| type=SpanTypeAttribute.LLM, | ||
| type=SpanTypeAttribute.TASK, |
There was a problem hiding this comment.
When google-adk==1.14.1 is installed with its permitted google-genai>=1.21.1,<2.0.0 dependency below 1.30 (package metadata), GoogleGenAIIntegration.setup() silently returns false because its min_version is 1.30.0. Unconditionally changing this span to task while removing ADK's usage extraction therefore leaves successful model calls with no LLM span or token/cost metrics, even though setup_adk() reports success. Preserve the former ADK LLM/usage fallback whenever provider setup is unavailable.
AGENTS.md reference: AGENTS.md:L132-L141
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this is fine, we can just document accordingly
With auto_instrument(), each Gemini request made through Google ADK produced two nested `llm` spans with identical usage: ADK's `llm_call [...]` and Google GenAI's `generate_content`. Trace summaries counted the request's tokens and LLM-call count twice. ADK always dispatches model calls through a separately instrumented provider client, so that integration owns the `llm` leaf and its usage, matching the JS SDK's ADK plugin and the pydantic-ai, DSPy, and LlamaIndex integrations: - Type ADK's `llm_call [...]` span as `task` and stop logging token metrics and usage metadata on it. It keeps its input, output, model/provider/tools metadata, `llm_call_type`, and time to first token. - Have `setup_adk()` also enable the Google GenAI integration, so users who only call `setup_adk()` still get a Gemini `llm` span with usage. `auto_instrument()` already enables both and still respects `google_genai=False`. Tests replay the existing cassettes; no re-recording was needed: - `test_adk_captures_metrics` covers `setup_adk()`: one usage-bearing `generate_content` span under a `task` `llm_call`. - `test_auto_adk.py` covers `auto_instrument()`, the reported scenario. - The other ADK tests assert the same ownership rule. Fixes SDK-525. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With Google GenAI owning the `llm` span and usage for ADK model calls, some ADK tests only re-checked GenAI behavior: - Remove `test_adk_usage_metadata_metrics` and its cassettes. ADK no longer computes usage, and `test_google_search_grounding` already covers tool-use prompt tokens, reasoning tokens and `usage_by_modality` for the same Google Search response shape in four call modes. - Trim `test_adk_captures_metrics` to the ownership rule and the ADK `llm_call` span's shape (task type, call type, time to first token). - Check only ADK's own model-call spans for raw binary in `test_adk_binary_data_attachment_conversion`; GenAI's attachment handling is covered in its own tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5964cf6 to
e80ebcc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e80ebcc3b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
|
|
||
| from braintrust.integrations.google_genai import GoogleGenAIIntegration | ||
|
|
||
| GoogleGenAIIntegration.setup() |
There was a problem hiding this comment.
When google-genai is installed but google-adk is absent or outside the supported version range, this call still globally patches Google GenAI before ADKIntegration.setup() returns False. A caller that uses the return value to treat ADK instrumentation as unavailable will therefore unexpectedly trace unrelated direct GenAI calls; enable the provider integration only after confirming that ADK setup succeeded.
Useful? React with 👍 / 👎.
2 participants
AI Summary
With
auto_instrument(), each Gemini request made through Google ADK produced two nestedllmspans with the same usage: ADK'sllm_call [...]and Google GenAI'sgenerate_content. Trace totals counted tokens and LLM calls twice.Now the provider integration owns the
llmspan and its usage, as in the JS SDK's ADK plugin and our pydantic-ai, DSPy and LlamaIndex integrations:llm_call [...]is now ataskspan with no token metrics or usage metadata. It keeps input, output, model/provider/tools metadata,llm_call_typeandtime_to_first_token.setup_adk()also enables Google GenAI, sosetup_adk()-only users still get a Geminillmspan with tokens.auto_instrument(google_genai=False)still turns GenAI off.Fixes #896 (SDK-525).
llmspans (prompt / completion tokens)auto_instrument()setup_adk()onlyBehavior changes
type = llmnow match GenAI'sgenerate_contentinstead ofllm_call [...].auto_instrument(). Same for users calling onlywrap_flow()withoutsetup_adk().Tests
No cassettes re-recorded. GenAI instrumentation doesn't change the outgoing request.
test_auto_adk.py(the reportedauto_instrument()case) andtest_adk_captures_metrics(setup_adk()only) assert one usage-bearinggenerate_contentspan under ataskllm_call. Other ADK tests share an_assert_provider_owns_llm_spanshelper.test_adk_usage_metadata_metricsand its cassettes: ADK no longer computes usage, and GenAI'stest_google_search_groundingcovers the same usage fields.main's ADK source, 5 tests fail, including both regression tests.Validation, all replaying cassettes:
test_google_adk(latest)16 passed;test_google_adk(2.6.3)and(1.14.1)13 passed, 3 skipped (workflow tests need ADK 2.10+);test_google_genai(latest)41 passed, 2 skipped;test_core -k lazypassed;make lintpasses; no unused ADK cassettes.🤖 Generated with Claude Code
Co-authored by StarfolkAI (@starfolkai)[bot]