Skip to content

refactor(proxy): inject tracing receiver and access context - #44035

Merged
yujonglee-berri merged 12 commits into
mainfrom
litellm_tracing_dependencies
Oct 1, 2026
Merged

yujonglee-berri merged 12 commits into
mainfrom
litellm_tracing_dependencies

Conversation

@yujonglee-berri

@yujonglee-berri yujonglee-berri commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Trace handlers share one process-global receiver
  • Background initialization races incoming requests
  • Tracing's periodic logging task outlives application shutdown

How it solves it:

  • Create tracing inside the FastAPI application lifespan
  • Inject lifespan state into tracing and Lens routes
  • Flush queued spend logs and stop the logging task
  • Preserve validation and authorization error ordering

Intentional product change: startup waits for tracing initialization. If initialization fails, the proxy starts with tracing disabled and trace routes return the existing 501 response

Relevant issues

Prerequisite for #43975, which is stacked on this branch. Merge this PR first, then rebase #43975 and retarget it to main

Implementation

The tracing context manager yields its receiver. The proxy assembles ProxyLifespanState and yields it to Starlette before serving requests. The state schema lives with the existing proxy types. Tracing routes retrieve the receiver from request state. Lens gets the same application's storage through a FastAPI dependency. The shared ClickHouse bridge is named ClickHouseStorage; trace-specific queries live in TraceStore. The global receiver and no-argument accessor are removed

litellm-storage-clickhouse now exports generic Storage and Connection types plus bounded read and insert transport. litellm-traces keeps OTLP decoding, trace schema, row encoding, and named query selection. The PyO3 trace binding composes the shared Storage while preserving its Python API

The access dependency derives read scope and write tenant in one role-policy match. The context enforces those permissions inside each handler, after parameter validation. Shutdown removes its spend callback, signals the periodic worker to stop, awaits its current insert, and drains queued rows. The worker wakes immediately when idle and exits without cancellation during normal shutdown. This follows FastAPI lifespan and Starlette lifespan state

The existing SDK callback registry remains process-wide. This PR scopes the trace receiver and tracing/Lens reads to the application; it does not make the rest of the proxy's global configuration independent between applications

Validation

At 4921a1c81b, all eight batch logger tests pass and make check passes. New regression cases verify shutdown recovery after a transient failure and accounting after retry exhaustion across multiple batches. Removing the drain loop makes both cases fail

Review triage confirmed the stacked branch already uses the renamed storage classes. Multiple independently configured proxies inside one process remain unsupported because SDK configuration and callbacks are process-global. The transport's existing production callers enforce the encoded byte limit before sending

Live curl checks at 4921a1c81b against PostgreSQL and ClickHouse returned 200 for protobuf export and span detail, an empty list for another team, 401 without authentication, and 422 for invalid time input

Greptile reviewed 4921a1c81b at 5/5. Hosted checks remain pending. No Bugbot review is available on this tip Earlier coverage reported 90.56% patch coverage with uncovered lifespan and Lens branches; this is not a completed coverage gate

Pre-Submission checklist

  • Meaningful tests cover injection, tenant identity, and error ordering
  • Focused tests and local checks pass
  • All required CI checks pass
  • Scope is limited to tracing ownership and dependency injection
  • Greptile confidence is at least 4/5

Type

Refactoring

Caveats

Medium

  • Independently configured proxies sharing one process remain unsupported

Low

  • Full current merge-base live comparison has not been refreshed

  • Existing team-key detail-route rejection remains unchanged

  • Dependency scan flags existing pypdf vulnerabilities in unchanged uv.lock

  • Hosted checks and final coverage remain pending

  • Bugbot review is unavailable on this tip

Final Attestation

  • Tests exercise application isolation, tenant stamping, failure precedence, and shutdown cleanup

Devin Review

@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.60773% with 17 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
litellm/proxy/proxy_server.py 77.55% 11 Missing ⚠️
litellm/proxy/lens/endpoints.py 66.66% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

@yujonglee-berri
yujonglee-berri added this pull request to stack #44039 October 1, 2026 17:07
@codspeed

codspeed Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_tracing_dependencies (4921a1c) with main (ca05eca)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (0da00d4) during the generation of this report, so ca05eca was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@yujonglee-berri
yujonglee-berri marked this pull request as ready for review October 1, 2026 19:34
@yujonglee-berri
yujonglee-berri requested a review from a team October 1, 2026 19:34

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Devin Review

Comment thread litellm/proxy/tracing_runtime.py
Comment thread litellm/integrations/clickhouse/clickhouse_batch_logger.py Outdated
@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[High risk] Extracts ClickHouse storage into a shared crate and refactors trace code to use it.

The PR appears safe to merge based on the reviewed changes.

Summary

The PR moves tracing ownership into the proxy lifespan, injects application-scoped tracing access into trace and Lens routes, separates shared ClickHouse transport from trace-specific code, and drains queued spend rows on shutdown.

Reviews (2) · Last reviewed commit: "fix(tracing): drain spend retries during..."

Comment thread litellm/proxy/tracing_runtime.py
Comment thread litellm/integrations/clickhouse/clickhouse_batch_logger.py Outdated
Comment thread litellm/rust_bridge/traces.py
Comment thread litellm-rust/crates/storage-clickhouse/src/insert.rs
@yujonglee-berri

Copy link
Copy Markdown
Contributor Author

@greptile-app review Please review the current tip after conflict resolution and report actionable findings on this pull request.

@yujonglee-berri

Copy link
Copy Markdown
Contributor Author

@greptile-app review The branch tip changed after the migration test fix. Please review this latest commit and report actionable findings.

@yujonglee-berri

Copy link
Copy Markdown
Contributor Author

@greptileai Please review 4921a1c. Shutdown retries are fixed, and other findings have evidence explaining unsupported scenarios, existing bounds, and updated imports

@yujonglee-berri
yujonglee-berri merged commit be67fce into main Oct 1, 2026
113 of 115 checks passed
@yujonglee-berri
yujonglee-berri deleted the litellm_tracing_dependencies branch October 1, 2026 20:45

This branch is waiting to be deployed

1 waiting deployment
e2e-changed — 4921a1c8 Waiting Oct 1, 2026 by yujonglee-berri via oauth #2392
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants