[issue-5374] [P SDK] Fix ADK OTel tracer re-patching on deep copy#5381
Closed
ollieagent[bot] wants to merge 8 commits intomainfrom
Closed
[issue-5374] [P SDK] Fix ADK OTel tracer re-patching on deep copy#5381ollieagent[bot] wants to merge 8 commits intomainfrom
ollieagent[bot] wants to merge 8 commits intomainfrom
Conversation
…zation Add idempotency guard to _patch_adk_opentelemetry_tracers using a module-level flag to prevent re-patching when OpikTracer is deep-copied per request. Fix __setstate__ to reinitialize only non-patching state (client, tracking dicts) instead of calling _init_internal_attributes() which unconditionally re-invoked patchers.patch_adk(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
Bot
added
python
…ve-otel-span-patching-941773
Update _patch_adk_opentelemetry_tracers to refresh opik_client reference when called after initial patching. The global flag prevents re-patching the ADK tracers (correct), but the stale opik_client in OpikADKOtelTracer caused failures when the client was ended and recreated between tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ve-otel-span-patching-941773
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add pytestmark = pytest.mark.usefixtures("ensure_vertexai_configured") to
ADK test files that make real Google AI API calls. This matches the pattern
used by genai, crewai, langchain_vertexai, anthropic, and bedrock test suites.
Without this fixture, the ensure_vertexai_configured session fixture is never
called, so GCP_CREDENTIALS_JSON is never written to a file and
GOOGLE_APPLICATION_CREDENTIALS is never set. The Google client then fails to
find credentials and falls back to requiring GOOGLE_API_KEY, which is absent.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
Bot
added
the
tests
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Python SDK E2E Tests Results (Python 3.11)243 tests ±0 241 ✅ ±0 8m 15s ⏱️ +14s Results for commit a70c4af. ± Comparison against base commit d2e999a. This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Python SDK E2E Tests Results (Python 3.13)243 tests ±0 241 ✅ ±0 7m 59s ⏱️ -8s Results for commit a70c4af. ± Comparison against base commit d2e999a. This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Python SDK E2E Tests Results (Python 3.14)243 tests ±0 241 ✅ ±0 8m 36s ⏱️ +17s Results for commit a70c4af. ± Comparison against base commit d2e999a. This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Python SDK E2E Tests Results (Python 3.10)243 tests ±0 241 ✅ ±0 8m 7s ⏱️ -8s Results for commit a70c4af. ± Comparison against base commit d2e999a. This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…y patched Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Python SDK E2E Tests Results (Python 3.12)243 tests ±0 241 ✅ ±0 7m 29s ⏱️ -27s Results for commit a70c4af. ± Comparison against base commit d2e999a. This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both. |
Member
|
Hi! Thanks for your contribution 🙌 We’re going to close this PR for now since it has merge conflicts and appears to be inactive. That said, feel free to reopen it anytime if you’d like to continue working on it—we’d be happy to take another look. Thanks again! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
The Opik ADK integration was re-applying its OpenTelemetry tracer patch on every deserialization of
OpikTracer, which occurs whenever an agent is deep-copied (e.g.,model_copy(deep=True)per request in frameworks likeag-ui-adk). This caused the Opik tracer to be re-installed over the ADK module-level tracers on every request, making it impossible to work around.Two bugs were fixed:
1. No idempotency guard on
_patch_adk_opentelemetry_tracersUnlike
_patch_adk_lite_llm()which checkshasattr(old_function, "opik_patched")before patching,_patch_adk_opentelemetry_tracersunconditionally replaced the ADK module-level tracer methods on every call. A module-level boolean flag_adk_opentelemetry_tracers_patchedis now set on first application and checked at entry, making the function idempotent.2.
__setstate__triggeredpatch_adk()on every deserialization__setstate__called_init_internal_attributes(), which in turn calledpatchers.patch_adk(). This re-applied the OTel tracer patch on everypickle.loadsordeepcopyof anOpikTracerinstance, undoing any workarounds applied at startup.__setstate__now directly reinitializes only the runtime state that needs restoring after deserialization (_opik_client,_last_model_output,_ttft_tracking), without triggering any patching.Note:
OpikADKOtelTracerstill returnsINVALID_SPANto OTel by design — it is the mechanism by which Opik creates its own traces and spans from ADK's span lifecycle events. The OTel patching is necessary for Opik's tracing to function; removing it entirely would causebefore_agent_callbackand related callbacks to fail silently (they update existing spans but do not create them).Change checklist
Issues
Testing
Run existing ADK integration tests:
cd sdks/python python -m pytest tests/library_integration/adk/test_adk_sync.py::test_adk__track_adk_agent_recursive__idempotent_calls_make_no_duplicated_callbacks tests/library_integration/adk/test_adk_sync.py::test_adk__opik_tracer__unpickled_object_works_as_expected -vVerify the idempotency guard manually:
Documentation
No documentation updates required.