-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Feature/disable telemetry #4032
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
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @mportdata, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request provides a robust and flexible way to disable telemetry across the Agent Development Kit (ADK). Previously, telemetry spans were always emitted, but now users can control this behavior through a new Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Response from ADK Triaging Agent Hello @mportdata, thank you for your contribution! Before we can review your pull request, you'll need to sign our Contributor License Agreement (CLA). It seems that the CLA check has failed. You can review and sign the CLA here: https://cla.developers.google.com/ Once the CLA is signed, we can proceed with the review. Thanks! |
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.
Code Review
This pull request introduces a new disable_telemetry flag to BaseAgent and Gemini LLM classes, allowing users to explicitly control OpenTelemetry tracing for agents and LLM interactions. The changes involve adding the is_telemetry_enabled utility function and refactoring existing code to conditionally apply tracing spans based on this flag. Specifically, run_async and run_live methods in base_agent.py, _call_llm_async in base_llm_flow.py, and tool execution in functions.py now check this flag before creating spans. The GeminiContextCacheManager and Gemini LLM also incorporate this flag to conditionally trace cache creation and LLM calls. Additionally, many string literals across various files were updated from single quotes to double quotes for consistency, and minor formatting adjustments were made to import statements. Review comments highlighted opportunities to further reduce code duplication by using contextlib.nullcontext for conditional tracing and corrected a typo in a docstring example.
| if not self.disable_telemetry: | ||
| from ..telemetry.tracing import tracer | ||
|
|
||
| with tracer.start_as_current_span("create_cache") as span: | ||
| return await self._create_gemini_cache_body( | ||
| llm_request=llm_request, | ||
| cache_contents_count=cache_contents_count, | ||
| span=span, | ||
| ) | ||
| else: | ||
| return await self._create_gemini_cache_body( | ||
| llm_request=llm_request, cache_contents_count=cache_contents_count | ||
| ) |
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.
There's some code duplication here. You can use contextlib.nullcontext to conditionally apply the tracing span, which will make the code cleaner and more maintainable.
You'll need to add import contextlib at the top of the file.
span_context = contextlib.nullcontext()
if not self.disable_telemetry:
from ..telemetry.tracing import tracer
span_context = tracer.start_as_current_span("create_cache")
with span_context as span:
return await self._create_gemini_cache_body(
llm_request=llm_request,
cache_contents_count=cache_contents_count,
span=span,
)| if llm_request.cache_config: | ||
| from ..telemetry.tracing import tracer | ||
| from .gemini_context_cache_manager import GeminiContextCacheManager | ||
|
|
||
| with tracer.start_as_current_span('handle_context_caching') as span: | ||
| cache_manager = GeminiContextCacheManager(self.api_client) | ||
| if not self.disable_telemetry: | ||
| from ..telemetry.tracing import tracer | ||
|
|
||
| with tracer.start_as_current_span("handle_context_caching") as span: | ||
| cache_manager = GeminiContextCacheManager( | ||
| self.api_client, disable_telemetry=self.disable_telemetry | ||
| ) | ||
| cache_metadata = await cache_manager.handle_context_caching( | ||
| llm_request | ||
| ) | ||
| if cache_metadata: | ||
| if cache_metadata.cache_name: | ||
| span.set_attribute("cache_action", "active_cache") | ||
| span.set_attribute("cache_name", cache_metadata.cache_name) | ||
| else: | ||
| span.set_attribute("cache_action", "fingerprint_only") | ||
| else: | ||
| cache_manager = GeminiContextCacheManager( | ||
| self.api_client, disable_telemetry=self.disable_telemetry | ||
| ) | ||
| cache_metadata = await cache_manager.handle_context_caching(llm_request) | ||
| if cache_metadata: | ||
| if cache_metadata.cache_name: | ||
| span.set_attribute('cache_action', 'active_cache') | ||
| span.set_attribute('cache_name', cache_metadata.cache_name) | ||
| else: | ||
| span.set_attribute('cache_action', 'fingerprint_only') | ||
|
|
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.
There's some code duplication in this block. You can refactor it to avoid repeating the logic for creating GeminiContextCacheManager and calling handle_context_caching. Using contextlib.nullcontext can help conditionally apply the tracing span.
You'll need to add import contextlib at the top of the file.
| if llm_request.cache_config: | |
| from ..telemetry.tracing import tracer | |
| from .gemini_context_cache_manager import GeminiContextCacheManager | |
| with tracer.start_as_current_span('handle_context_caching') as span: | |
| cache_manager = GeminiContextCacheManager(self.api_client) | |
| if not self.disable_telemetry: | |
| from ..telemetry.tracing import tracer | |
| with tracer.start_as_current_span("handle_context_caching") as span: | |
| cache_manager = GeminiContextCacheManager( | |
| self.api_client, disable_telemetry=self.disable_telemetry | |
| ) | |
| cache_metadata = await cache_manager.handle_context_caching( | |
| llm_request | |
| ) | |
| if cache_metadata: | |
| if cache_metadata.cache_name: | |
| span.set_attribute("cache_action", "active_cache") | |
| span.set_attribute("cache_name", cache_metadata.cache_name) | |
| else: | |
| span.set_attribute("cache_action", "fingerprint_only") | |
| else: | |
| cache_manager = GeminiContextCacheManager( | |
| self.api_client, disable_telemetry=self.disable_telemetry | |
| ) | |
| cache_metadata = await cache_manager.handle_context_caching(llm_request) | |
| if cache_metadata: | |
| if cache_metadata.cache_name: | |
| span.set_attribute('cache_action', 'active_cache') | |
| span.set_attribute('cache_name', cache_metadata.cache_name) | |
| else: | |
| span.set_attribute('cache_action', 'fingerprint_only') | |
| if llm_request.cache_config: | |
| from .gemini_context_cache_manager import GeminiContextCacheManager | |
| cache_manager = GeminiContextCacheManager( | |
| self.api_client, disable_telemetry=self.disable_telemetry | |
| ) | |
| span_context = contextlib.nullcontext() | |
| if not self.disable_telemetry: | |
| from ..telemetry.tracing import tracer | |
| span_context = tracer.start_as_current_span("handle_context_caching") | |
| with span_context as span: | |
| cache_metadata = await cache_manager.handle_context_caching(llm_request) | |
| if span and cache_metadata: | |
| if cache_metadata.cache_name: | |
| span.set_attribute("cache_action", "active_cache") | |
| span.set_attribute("cache_name", cache_metadata.cache_name) | |
| else: | |
| span.set_attribute("cache_action", "fingerprint_only") |
| async def _run_with_optional_trace( | ||
| agent: BaseAgent, | ||
| new_message: Optional[types.Content] = None, | ||
| invocation_id: Optional[str] = None, | ||
| ) -> AsyncGenerator[Event, None]: | ||
| if is_telemetry_enabled(agent): | ||
| with tracer.start_as_current_span("invocation"): | ||
| async with Aclosing( | ||
| _run_body(new_message=new_message, invocation_id=invocation_id) | ||
| ) as agen: | ||
| async for e in agen: | ||
| yield e | ||
| else: | ||
| async with Aclosing( | ||
| self._exec_with_plugin( | ||
| invocation_context=invocation_context, | ||
| session=session, | ||
| execute_fn=execute, | ||
| is_live_call=False, | ||
| ) | ||
| _run_body(new_message=new_message, invocation_id=invocation_id) | ||
| ) as agen: | ||
| async for event in agen: | ||
| yield event | ||
| # Run compaction after all events are yielded from the agent. | ||
| # (We don't compact in the middle of an invocation, we only compact at | ||
| # the end of an invocation.) | ||
| if self.app and self.app.events_compaction_config: | ||
| logger.debug('Running event compactor.') | ||
| await _run_compaction_for_sliding_window( | ||
| self.app, session, self.session_service | ||
| ) | ||
| async for e in agen: | ||
| yield e |
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.
The async with Aclosing(...) block is duplicated. You can refactor this using contextlib.nullcontext to conditionally apply the tracing span, which will make the code more DRY (Don't Repeat Yourself).
You'll need to add import contextlib at the top of the file.
async def _run_with_optional_trace(
agent: BaseAgent,
new_message: Optional[types.Content] = None,
invocation_id: Optional[str] = None,
) -> AsyncGenerator[Event, None]:
span_context = contextlib.nullcontext()
if is_telemetry_enabled(agent):
span_context = tracer.start_as_current_span("invocation")
with span_context:
async with Aclosing(
_run_body(new_message=new_message, invocation_id=invocation_id)
) as agen:
async for e in agen:
yield eAdd unit tests for agent and Gemini telemetry disable flags including env var gating. Add integration test asserting spans are emitted when enabled and omitted when ADK_TELEMETRY_DISABLED is set.
Add integration coverage for OTEL_SDK_DISABLED, ADK_TELEMETRY_DISABLED, and agent.disable_telemetry.
Add back _create_gemini_cache wrapper for compatibility after telemetry refactor. Apply autoformat across touched files.
… incorrectly said disabled
2c1e727 to
ba0ce7a
Compare
Use a nullcontext fallback to avoid duplicating the async run loop when telemetry is disabled.
Use a nullcontext fallback to keep run_live telemetry handling consistent with run_async.
Use nullcontext fallbacks for tracing in base_agent and base_llm_flow. Run autoformat on telemetry utilities.
Use a nullcontext fallback in _call_llm_with_optional_tracing to avoid branching.
Use early return when telemetry is disabled to avoid branching in execute_tool spans.
Use a nullcontext fallback for cache creation tracing. Update GEPA sample formatting.
Restore GEPA sample files to upstream main to avoid unrelated formatting changes.
Reuse callback runner with a nullcontext span and avoid quote-only churn.
Reapply only the model telemetry toggle without quote-style churn.
Use nullcontext for live send_data/call_llm spans and skip tool tracing when telemetry is disabled. Add a shared context_manager_with_span fixture for telemetry tests.
Keep only telemetry logic in google_llm and runners without quote/indent churn.
Use _create_gemini_cache name consistently and update tests. Revert tracing.py quote-only churn.
Link to Issue or Description of Change
Problem:
Telemetry spans are always emitted. There wasn’t a
consistent, ADK‑level switch to disable all
telemetry.
Solution:
I've added a disable_telemetry attribute to the BaseAgent class and 2 environment variables OTEL_SDK_DISABLED and ADK_TELEMETRY_DISABLED. When any of the above are set to true then spans are no longer started resulting in tracing being disabled.
Testing Plan
Unit Tests:
change.
Integration Tests:
I have created unit tests in the following file /tests/unittests/telemetry/test_telemetry_disable_agent.py
These tests monkey patch the start_as_current_span method used for tracing throughout Google ADK and ensures it is only called when tracing is enabled both via the agent attribute or the environment variables. Conversely I have also tested here that telemetry is returned when expected (when not disabled by either the attribute or the environment variables).
Summary:
Manual End-to-End (E2E) Tests:
A simple ADK agent has been tested by running
uv run adk webwith thedisable_telemetry=Truebehaved as expected.Checklist
code.
hard-to-understand areas.
effective or that my feature works.
with my changes.
end.
published in downstream modules.