Skip to content

fix(cli): Stop unbounded debug trace span retention in ApiServer - #6696

Open
chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix-unbounded-debug-trace-exporters
Open

fix(cli): Stop unbounded debug trace span retention in ApiServer#6696
chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix-unbounded-debug-trace-exporters

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:
ApiServer.get_fast_api_app() unconditionally registered two in-memory
span processors (ApiServerSpanExporter, InMemoryExporter) on the global
tracer provider. InMemoryExporter.export() appends every span to an
unbounded list and ApiServerSpanExporter.export() writes every span's
attributes into a dict, neither of which is ever pruned. clear() exists
but has no callers. The only reader of this retained data is DevServer's
/debug/trace endpoint (used by adk web), so any server started with
web=False (e.g. via adk api_server or get_fast_api_app(..., web=False))
leaked memory for the life of the process with no way to opt out.

Solution:
Added a _registers_debug_trace_exporters class attribute on ApiServer
(default False), and only build/register the two debug-trace span
processors when it's set. DevServer overrides it to True, since it's
the only place that reads the retained data (AdkWebServer inherits this
from DevServer). Plain ApiServer (production/web=False) now sets up
telemetry with no internal exporters, so no spans are retained and nothing
observable changes for DevServer/adk web users.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Added test_debug_trace_exporters_only_registered_for_dev_server in
tests/unittests/cli/test_fast_api.py, which patches
google.adk.cli.api_server._setup_telemetry and asserts it is called with
internal_exporters=[] when web=False and with the two debug-trace span
processors when web=True.

Verified the new test fails without the fix (checked out
src/google/adk/cli/api_server.py/dev_server.py from the parent commit
and reran):

FAILED tests/unittests/cli/test_fast_api.py::test_debug_trace_exporters_only_registered_for_dev_server
  assert [<opentelemetry...>, <opentelemetry...>] == []
1 failed, 113 deselected, 5 warnings in 2.46s

With the fix applied:

$ pytest tests/unittests/cli/test_fast_api.py -q
107 passed, 5 skipped, 2 xfailed, 11 warnings in 21.47s

$ pytest tests/unittests/cli -q
783 passed, 5 skipped, 4 xfailed, 180 warnings in 40.07s

Also ran isort --check-only and pyink --check on the changed files;
both report no issues.

Manual End-to-End (E2E) Tests:

Followed the repro from the issue: get_fast_api_app(agents_dir=..., web=False)
now sets up the tracer provider with zero internal exporters, so
InMemoryExporter/ApiServerSpanExporter are never instantiated as span
processors and len(memory_exporter._spans) no longer grows as requests
are served. adk web (web=True, DevServer) is unaffected — the
/debug/trace/{event_id} endpoint continues to work exactly as before,
covered by the existing test_debug_trace test.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

This PR was prepared with AI assistance (Claude Code), reviewed and
verified by me before pushing.

ApiServer.get_fast_api_app() unconditionally registered two in-memory
span exporters (ApiServerSpanExporter, InMemoryExporter) on the global
tracer provider. Their retained span/attribute dicts are never pruned,
so any long-lived server serving through ApiServer.get_fast_api_app()
(web=False) leaked memory for the life of the process.

The only reader of this data is DevServer's /debug/trace endpoint
(web=True), so the exporters are now only registered when
_registers_debug_trace_exporters is set, which DevServer overrides to
True. ApiServer no longer registers them at all.

Fixes google#6692
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tracing [Component] This issue is related to OpenTelemetry tracing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ApiServer.get_fast_api_app() registers unbounded in-memory span exporters (memory leak on long-lived servers)

3 participants