mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
* feat(logging): add opt-in session_id/trace_id correlation to JSON log records via contextvars Adds two ContextVar instances (session_id_var, trace_id_var) to litellm/_logging.py and two setter functions (set_session_id, set_trace_id). Logging.__init__() now calls both setters after assigning litellm_trace_id so every JSON log record emitted within the async request context carries trace_id and, when provided, session_id — enabling log correlation in Loki, CloudWatch Logs Insights, and other structured-log sinks without any changes to individual log call sites. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(logging): guard session_id/trace_id injection against overwriting caller-supplied extra fields * fix(logging): always reset session_id_var to empty string when no session_id provided * feat: gate request correlation IDs in logs behind request_correlation_in_logs flag * refactor: move correlation ID injection into CorrelationContextFilter * feat(logging): extend request_correlation_in_logs to plaintext logs and StandardLoggingPayload Plaintext log lines (json_logs off) now get the same trace_id/session_id suffix as JSON logs via a new CorrelationPlainFormatter, so the flag has a visible effect regardless of log format. StandardLoggingPayload gets a new independent session_id field, populated from litellm_session_id. trace_id's existing session_id-first fallback is preserved when request_correlation_in_logs is off; with the flag on, an explicit litellm_trace_id now takes priority over litellm_session_id so the two fields carry genuinely independent values. * fix(logging): restore correlation context after nested calls; sanitize correlation ids Addresses two review findings on this PR. CorrelationContextFilter's trace_id/session_id contextvars were set on every Logging.__init__ but never reset, so a nested LiteLLM call sharing the same asyncio Task as an outer request (e.g. a guardrail's own LLM-as-judge call, an MCP sampling call) would leave the outer request's subsequent log lines stamped with the nested call's ids instead of its own. set_trace_id/ set_session_id now return their contextvars.Token, and Logging stores them and resets both once its own success/failure handler actually completes, via a new idempotent _restore_correlation_context() called from all four terminal handlers. set_trace_id/set_session_id also now strip control characters and bound length before storing a caller-controlled trace_id/session_id, since these values can originate from request input (litellm_session_id, x-litellm- trace-id) and get interpolated into plain-text log lines - without this, a caller could embed \r/\n or escape sequences to forge fake log entries. * fix(logging): restore correlation context after nested calls, not before The previous commit called _restore_correlation_context() as the first line of each terminal handler, before that handler's own callback dispatch loop runs. That's backwards: a nested LiteLLM call triggered from within a callback (e.g. a guardrail's own LLM-as-judge call) would then capture the *already-reset* value as its own pre-call baseline, and its own reset would restore to that instead of the true outer value - verified live to still leak. success_handler/async_success_handler/failure_handler/async_failure_handler are now thin wrappers: the original bodies move to _success_handler_body/etc, called inside a try/finally that restores context only once the full body - including any nested calls its own callback dispatch triggers - has actually finished, mirroring proper stack-scoped nesting semantics. * test(logging): cover async_failure_handler's correlation-context restore Codecov flagged the new async_failure_handler wrapper (try/finally around _async_failure_handler_body) as uncovered - the method had no direct test at all before this PR's refactor split it into a wrapper. Adds a test that awaits it directly and asserts both that async_log_failure_event still fires and that _restore_correlation_context() puts the pre-call trace_id/session_id back. * fix(logging): restore correlation context by value, not by contextvars.Token veria-ai correctly flagged that contextvars.Token.reset() only works in the exact Context it was created in, and litellm's async success path (and streaming failure path) dispatch async_success_handler/async_failure_handler via asyncio.create_task and the global logging worker - a different Context than Logging.__init__ ran in. reset_trace_id/reset_session_id silently swallowed the resulting ValueError, so the restore was a no-op for exactly those paths. Verified independently: reproduced the raw contextvars behavior, then confirmed litellm's async success dispatch really does go through asyncio.create_task + GLOBAL_LOGGING_WORKER (litellm/utils.py). Logging now captures the pre-call *value* (not a Token) and restores via a plain set_trace_id()/set_session_id() call, which works regardless of which Task/Context calls it. reset_trace_id/reset_session_id are removed as dead/unreliable code. Added a regression test that spawns __init__ and the restore in different asyncio Tasks - confirmed it fails against the prior Token-based commit and passes here. * fix(logging): restore correlation context in the originating task too Greptile's re-review correctly identified a remaining gap: for a successful acompletion(), async_success_handler is dispatched via asyncio.create_task + the global logging worker into a *different* Task than the one wrapper_async/Logging.__init__ ran in. The prior fix (43c164a) only restored the handler's own (detached, throwaway) Task - it never touched the originating request Task, which keeps this call's trace_id/ session_id set for the rest of its own execution (e.g. nested calls made via the same Task). wrapper()/wrapper_async() in litellm/utils.py now restore the originating Task's correlation context in a finally block once the whole call is done, regardless of what detached logging tasks it spawned along the way. Since the wrapped body rebinds its own `kwargs` local via function_setup(), sharing the dict object doesn't work here; a small mutable holder carries the constructed Logging instance back out to the outer wrapper instead. _restore_correlation_context() is no longer guarded against repeat calls: with value-based (not Token-based) restoration, each distinct Task that calls it needs its own restore to take effect in that Task's own view of the contextvars, so multiple calls (once per Task involved in an attempt) are required, not just tolerated. Added a regression test using mock_response to exercise the real success dispatch path (asyncio.create_task + GLOBAL_LOGGING_WORKER) without a live provider call, asserting the *test's own* (originating) task context is restored after the call - this is exactly the case Greptile flagged and the prior commit didn't cover. * fix(logging): restore correlation context when function_setup itself fails Greptile's 4th finding: if function_setup() constructs Logging() (whose __init__ already mutates trace_id_var/session_id_var) and then raises before returning - e.g. update_environment_variables() throws - the caller's wrapper()/wrapper_async() never receives a logging_obj reference, so its own restore-on-finally never fires. The correlation ids leak into every subsequent log line on that thread/task with no way to clear them. function_setup()'s own except block now restores the context itself in that case, using whatever logging_obj it managed to construct before failing (locals().get(), safe against the earlier failure modes where logging_obj was never assigned at all). Added a regression test that monkeypatches Logging.update_environment_variables to raise after construction, confirmed it fails without this fix (the leaked ids show up directly in the raised exception's own log line) and passes with it. Broader sweep (test_utils.py, test_router.py, test_main_module_header.py, streaming handler tests, plus all logging-specific tests): 722 passed. * fix(logging): don't assume every litellm_logging_obj is a real Logging instance CI caught a real regression from the last commit: tests/test_litellm/llms/xai/test_xai_key_fallback.py injects a minimal FakeLogging stand-in (only implementing update_from_kwargs) as litellm_logging_obj for a narrow realtime-config unit test, bypassing the real Logging class entirely. wrapper()/ wrapper_async()'s finally block and function_setup()'s except block both unconditionally called _restore_correlation_context() on whatever ended up in the holder, which doesn't exist on that stand-in. _restore_correlation_context is new plumbing specific to this PR's feature, not part of any pre-existing stand-in's expected interface, so callers of it can't assume every object playing the litellm_logging_obj role implements it. Added _restore_correlation_context_if_supported(), a small getattr-guarded helper, and used it at all three call sites. * fix(logging): don't restore context too early on setup failure or streaming Two more findings from Greptile's 5th review round. 1. function_setup()'s except block restored correlation context *after* logging the "Error in function_setup" exception, so that diagnostic log line itself was stamped with the doomed call's ids instead of the outer ids - misleading, since the failed call never produces anything else to attribute those ids to. Restore now happens before the log call. 2. wrapper()/wrapper_async() restored the originating task's context as soon as a streaming call returned, before the caller ever starts iterating the CustomStreamWrapper it just got back. Any log lines emitted while iterating (in the same thread/task) incorrectly showed the pre-call ids instead of this call's own ones. The wrapper finally block now skips the restore when the return value is a stream wrapper, deferring to the terminal handler that already fires once the stream is actually assembled/exhausted. Both verified with tests that fail against the prior commit and pass against this one. Broader sweep unchanged at 829 passing. * fix(logging): best-effort correlation cleanup on abandoned streams Greptile's 7th finding: if a caller returns a streaming response and never fully consumes it - stops iterating early, drops the reference, cancels it - the terminal handler that normally restores the originating task's trace_id/session_id never fires, since it only runs once the stream is actually assembled/exhausted. The ids leak into every subsequent log line in that thread/task with no bound. There's no reliable Python hook for "this was abandoned without being closed" - CustomStreamWrapper has no close()/__aexit__/context-manager convention today, and the only automatic option is __del__, whose timing is inherently unpredictable (delayed by cyclic GC, not guaranteed at interpreter shutdown, can run on a different thread). This is a best-effort safety net, not a guarantee, and is documented as such in the docstring. Testing this via real garbage collection proved unreliable in practice: per-chunk logging submits work to a thread pool executor whose worker thread transiently holds its own bound-method reference to the wrapper until that task completes, so refcount doesn't hit zero on a deterministic schedule even with polling. Tests call __del__ directly instead - a plain method, safe to invoke early - which exercises exactly the restore logic real garbage collection would eventually trigger, plus a case confirming a broken logging_obj can never make __del__ raise. * fix(logging): restore consumer's context at every real stream exit point Two more findings from this round. Veria AI: even a *fully consumed* stream never restored the actual consuming thread/task's correlation context. The terminal success dispatch (dispatch_success_handlers via asyncio.create_task for async, or success_handler via the shared executor for sync) only restores whatever detached context it runs in - never the caller's own thread/task that's running the for/async for loop. Same root cause as the wrapper-level fix two rounds ago, just missed for the streaming-completion path. Greptile: explicit aclose() (client disconnect, router fallback aborting a partial stream) closed the underlying stream without restoring correlation context either, since request wrappers intentionally skip restoration for returned streams and no terminal handler runs on this path. Added CustomStreamWrapper._restore_consumer_correlation_context(), called from every point control genuinely returns to the consumer: the final raise StopIteration/StopAsyncIteration on natural exhaustion (both sync branches, both async branches), _handle_stream_fallback_error (the shared choke point for all three failure-raising call sites), and aclose(). __del__ now delegates to the same helper instead of duplicating it. Verified with tests extending the existing streaming-exhaustion cases to assert the consuming context is restored after the loop completes (fails against the prior commit, passes now), plus a dedicated aclose() test. Broader sweep: 832 passing. * fix(logging): don't let a delayed __del__ finalizer clobber a newer active call If an abandoned stream's __del__ fires late (after cyclic GC delay), a different call may have already taken over the correlation contextvars in the same Task/thread. Restoring unconditionally would stomp that active call's trace_id/session_id with the abandoned stream's stale pre-call snapshot. __del__ now only restores when the contextvars still hold the ids this call itself set. * fix(logging): compare sanitized ids in the __del__ ownership guard set_trace_id()/set_session_id() sanitize (strip control chars, bound length) before storing, so the contextvar's value can differ from the raw litellm_trace_id/litellm_session_id. The __del__ ownership guard was comparing against the raw values, so a caller-supplied id containing control characters or exceeding 256 chars would never match, permanently skipping cleanup. Capture what set_trace_id()/set_session_id() actually stored and compare against that instead. * fix(logging): restore consumer context on the synthesized finish_reason chunk Both __next__ and _finalize_completed_stream() have a branch that fires when the underlying stream ends without ever emitting an explicit finish_reason chunk: they synthesize one via finish_reason_handler() and return it. A consumer that stops as soon as it sees finish_reason - a common pattern - never calls __next__()/__anext__() again, so the existing restore in the sent_last_chunk-is-True StopIteration branch never runs for them. The underlying stream is already exhausted at this point regardless of whether the caller keeps iterating, so restoring here is safe. * fix(logging): don't restore correlation context before the caller receives the final chunk The previous fix (5147c69186) restored context immediately before returning the synthesized finish_reason chunk from __next__/_finalize_completed_stream, reasoning that completion_stream was already exhausted. But that chunk is still this call's own data, and the caller's own application-level log statements processing it run in the same synchronous frame right after the return - restoring first made those lines carry the wrong (outer) ids, exactly what wrapper()/wrapper_async() deliberately avoid by not restoring while a stream is being iterated. Revert to not restoring there. A caller that keeps iterating still gets a correct, deterministic restore on its very next __next__()/__anext__() call (completion_stream is exhausted, so that immediately re-raises StopIteration/StopAsyncIteration through the already-restoring branch). A caller that stops right after finish_reason relies on aclose() or the best-effort __del__ guard, same as any other stream the caller doesn't fully exhaust. * refactor(logging): hoist a safely-hoistable function-body import to module top CorrelationContextFilter.filter()'s `import litellm` was a function-body import; verified it can move to module top without a circular-import failure (litellm/__init__.py already imports from litellm._logging before setting request_correlation_in_logs, but a bare `import litellm` only binds the already-in-sys.modules module object - the attribute itself isn't read until filter() actually runs, by which point litellm is fully initialized). * test(logging): move correlation tests into their conventionally-mapped files tests/test_litellm/ mirrors litellm/ in a parallel path. Correlation tests for the Logging class (litellm_logging.py), function_setup/wrapper_async (utils.py), and CustomStreamWrapper (streaming_handler.py) had all landed in test_logging.py, which only maps to litellm/_logging.py itself. Moving each group to its correctly-mapped file: test_litellm_logging.py (Logging class init/restore), test_utils.py (function_setup, wrapper_async), and test_streaming_handler.py (CustomStreamWrapper) in the next commit. test_logging.py keeps only what actually exercises _logging.py's own contextvars/filters/formatters/sanitization. No behavior change - same assertions, same coverage, just relocated. * fix(logging): restore correlation context unconditionally in wrapper()'s sync path Blocking finding from review: a caller-visible correlation feature was silently misattributing one request's logs to a different, unrelated one on the sync/threaded path. wrapper()/wrapper_async() both left trace_id/session_id "open" across a stream's entire iteration so the caller's own log lines while consuming it would carry the right ids. That's safe for wrapper_async(): each async call gets its own asyncio Task with its own copy of the contextvars, and Tasks are never recycled across requests, so a leftover value can only ever affect that one already-abandoned Task. It is not safe for wrapper() (sync): a plain OS thread has no such per-call isolation, and a thread pool's worker threads *are* recycled across unrelated requests. If a sync stream was abandoned (client disconnect, early break, an uncaught exception) without ever being exhausted or closed, nothing restored its contextvars, and a pool could later hand that same thread to a completely different call, which would inherit the abandoned request's ids as its own "pre-call" baseline and then restore back to that poison when it finished - permanently misattributing every subsequent log line on that thread, including its own, to the abandoned request. Strengthening the __del__ finalizer already added for this can't fix it: finalizer timing is exactly what a permanently-reused thread can't rely on. wrapper() now restores unconditionally in its own finally, before a sync stream is ever handed back to the caller. The trade-off: a sync stream consumer's own application-level log statements while iterating no longer automatically carry this call's ids (litellm's own internal per-chunk logging is unaffected, since it's dispatched separately). That's an acceptable cost for eliminating a silent cross-request misattribution bug. wrapper_async() keeps the existing conditional (skip-if-streaming) behavior, justified by the Task-isolation argument above; CustomStreamWrapper's __del__/aclose()/next-iteration restore machinery remains meaningful and necessary there. This also simplifies wrapper()/wrapper_async() back toward their original shape: both previously used a mutable-dict-holder split into a separate _body function to smuggle logging_obj/result out to an outer finally, working around function_setup() rebinding its own local `kwargs`. That restructuring is no longer needed - `logging_obj` (and, for wrapper_async(), `result`) were already function-level locals in scope for a plain try/finally; three of wrapper_async()'s retry-return statements now assign through `result` first so it accurately reflects what's actually returned even on a retry path. Regression test: test_abandoned_sync_stream_does_not_contaminate_a_later_call_on_the_same_thread in test_streaming_handler.py reproduces the exact reported scenario with a real single-worker ThreadPoolExecutor - confirmed it fails with the prior (skip-restore-on-stream) wrapper() and passes with this fix. * refactor(logging): use Mapping instead of bare dict for read-only params _get_standard_logging_payload_trace_id/_session_id only read litellm_params (.get() calls, no mutation) - annotate it as Mapping[str, Any] rather than a bare mutable dict, per the repo's no-mutable-collection-in-annotation rule. * fix(logging): scope request_correlation_in_logs to the async/proxy path only Blocking review finding: wrapper() (the sync entry point) used the same skip-restore-on-stream design as wrapper_async(), but a plain OS thread has no per-call context isolation the way an asyncio Task does, and a thread pool's worker threads are recycled across unrelated requests - an abandoned sync stream could leave its ids stuck on a thread a pool later hands to a completely different request, misattributing that request's logs. A fix existed and was tested (restore unconditionally in wrapper()'s own finally), but it doesn't benefit this feature's primary consumer - the proxy only ever calls the async entry point - and carries sync-specific complexity this PR doesn't need. Scope the feature to async only instead: Logging.__init__() takes a new supports_correlation_logging parameter (default True), threaded down from a new function_setup(..., is_async_call: bool = True) parameter. wrapper() is the one caller that passes is_async_call=False; every other function_setup() call site (wrapper_async(), the router, and proxy/MCP-internal call sites) is already async and keeps the default. With supports_correlation_logging=False, Logging.__init__() never calls set_trace_id()/set_session_id() at all, so a sync call has nothing to leak in the first place. wrapper() reverts to its pre-review shape with no correlation-specific code at all. StandardLoggingPayload's own trace_id/session_id fields are unaffected either way - they're a deterministic per-call read of self.litellm_trace_id/self.litellm_session_id, not ambient contextvar state, so they were never exposed to the cross-request bug. Full sync/direct-SDK support (stamping + its own safe-restore mechanism) is deferred to a follow-up PR; the fix and its regression test already exist in this branch's history at commit9f3a20f4b2and can be resurrected there. Tests: replaced the two wrapper()-level tests with ones proving the new invariant (sync calls, streaming and non-streaming, never touch trace_id_var/session_id_var even when the caller explicitly passes litellm_trace_id/litellm_session_id), and added a direct unit test for the supports_correlation_logging=False gate on Logging.__init__ itself. Verified live: a real proxy (Postgres-backed, real OpenAI calls) shows clean trace_id/session_id isolation across two concurrent sessions with no cross-contamination; a standalone script confirms real sync SDK calls against a real model never touch the correlation contextvars. * feat(logging): fall back to W3C traceparent/baggage for trace_id/session_id request_correlation_in_logs previously only resolved trace_id/session_id from litellm-specific sources: x-litellm-trace-id/x-litellm-session-id headers, a generic x-<vendor>-session-id header, or Anthropic-style metadata.user_id. If none were present, trace_id fell back to an auto-generated UUID unrelated to anything else, and session_id stayed empty - even when the caller already had real distributed-tracing instrumentation sending the actual industry-standard headers for this. Add a fallback to the W3C Trace Context traceparent header (trace-id component) and W3C Baggage header (session.id entry), so a request already carrying real OpenTelemetry trace context correlates litellm's own logs with the same trace in the caller's observability backend (Datadog, Honeycomb, Tempo, etc.) instead of getting an unrelated generated id. Precedence is unchanged for existing sources: explicit litellm headers and the Anthropic metadata path both still win over this new fallback, which only fires when neither found anything. trace_id and session_id are resolved independently here (unlike the existing chain_id mechanism, which uses one shared value for both), since traceparent and baggage are semantically distinct W3C concepts. New helpers _trace_id_from_traceparent/_session_id_from_baggage in litellm_pre_call_utils.py parse the header formats directly (no new dependency - both are simple fixed-width/delimited strings), wired into LiteLLMProxyRequestSetup.add_litellm_metadata_from_request_headers() only when the corresponding litellm_trace_id/litellm_session_id key isn't already set by the existing paths. Verified live against a real proxy: a bare traceparent header produces a log trace_id exactly matching its trace-id component; a traceparent alongside an explicit x-litellm-trace-id header (different value) produces a log showing the explicit header's value, proving precedence. * fix(logging): reserve trace_id/session_id in JsonFormatter against message-content spoofing JsonFormatter merges keys parsed from the message body before applying extra record attributes, and the extra-attributes loop skips a key that's already present. A caller-controlled log message that happens to parse as JSON/dict with a "trace_id"/"session_id" key (e.g. the proxy logging a raw request-header dict) could therefore make the JSON record carry the attacker-supplied value instead of the real correlation context set via CorrelationContextFilter. trace_id/session_id are now applied from the LogRecord's own attributes after message-content parsing, unconditionally overwriting anything the message body claimed for those two keys. * style(logging): fix import order (ruff I001) in _logging.py and litellm_logging.py - _logging.py: import litellm belongs after the stdlib from-imports, grouped with the other litellm.* imports, not before them. - litellm_logging.py: the refactor to Mapping introduced a second, separate `from collections.abc import Mapping` instead of merging it into the existing `from collections.abc import Callable` import. Caught by the strict-rule budget gate (ruff-strict-budget.json caps I001 at 0 new violations); both auto-fixed with `ruff check --fix --select I001`. * style(logging): freeze mutable-collection constructions flagged by LIT002 Five sites in this PR's diff built a mutable list/dict literal instead of a frozen value: a plain list of optional strings in CorrelationPlainFormatter, a `kwargs or {}` fallback, a `metadata or {}` fallback, two `[...]` candidate orderings, and a `dict(headers)` copy feeding a dict comprehension. Each is build-once/read-only, so this rewrites them as tuples, MappingProxyType, or a plain conditional `.get()` instead of seeding then reading a fresh mutable collection - no behavior change, confirmed by the existing test suite. Caught by the type-discipline budget gate (LIT002 capped at 0 new violations). * fix(logging): reserve trace_id/session_id even when no correlation context is active Live-proxy verification surfaced a gap in the earlier message-content-spoofing fix (7f390a57fc): that fix only overwrites trace_id/session_id from the LogRecord's own attribute, so it does nothing for a log line emitted before CorrelationContextFilter has stamped anything on this record (e.g. the "Request Headers" debug line, which fires before Logging.__init__() runs for the request). On such a record, a caller-supplied header literally named trace_id/session_id still got promoted into the JSON output via the embedded JSON/dict-repr parser, since there was no genuine value to protect. Fixed at the source: trace_id/session_id are now excluded unconditionally from the message-content-parsing promotion step, not just superseded afterward. Verified live against a real proxy - the exact adversarial request (headers literally named trace_id/session_id) no longer leaks into any JSON log record. Added a regression test for this no-active-context variant specifically, confirmed it fails against the prior commit and passes now. Also fixes an unrelated basedpyright regression from an earlier rebase's conflict resolution: litellm/utils.py's `logging_obj` was incorrectly re-annotated `Final` at its second assignment in function_setup() (it's first declared `None` a few lines earlier), which basedpyright correctly rejects. * fix(proxy): stop logging the raw W3C baggage session_id value _session_id_from_baggage() extracts the caller-controlled session.id entry verbatim - it isn't sanitized until set_session_id() runs later in Logging.__init__(). The debug log line for this extraction interpolated the raw value directly, so a caller could embed terminal control characters or ANSI escape sequences that forge/alter plaintext log output for anyone tailing the proxy's logs. Verified live: a baggage header with an embedded ANSI escape reached the terminal as a real, unescaped control sequence before this fix. Drops the value from the log line entirely (the extraction succeeding is enough signal on its own) rather than sanitizing-then-logging, matching veria-ai's suggestion. Added a regression test using caplog that fails against the prior commit and passes now. * fix(logging): restore consumer context only after stream-failure exception mapping _map_anthropic_exception/_map_aleph_alpha_exception synchronously log a debug diagnostic (the raw status code) as part of exception_type()'s mapping. _handle_stream_fallback_error restored the consumer's outer correlation context before calling exception_type(), so that diagnostic log line carried the outer (or empty) trace_id/session_id instead of the failing stream's own - flagged by Greptile. Moved the restore to run after mapping completes, matching the same restore-after-not-before pattern already applied elsewhere in this file for success/finish_reason handling. Added a regression test that captures the correlation context live during a mocked exception_type() call; fails against the prior commit, passes now. * fix(logging): restore consumer context only after aclose()'s stream close completes aclose() restored the consumer's outer correlation context as its first statement, before awaiting the underlying provider stream's own aclose()/ close(). If that close attempt raises, the except branch's debug diagnostic ran under the already-restored outer context instead of the closing stream's own trace_id/session_id - flagged by Greptile, same restore-too-early pattern as the stream-failure fix inf1cf9589d6. Moved the restore to the end of aclose(), after the close attempt (and its diagnostic logging) completes. Added a regression test with a fake stream whose aclose() raises, capturing the correlation context live during the diagnostic log call; fails against the prior commit, passes now. * style(logging): satisfy new strict-lint budgets introduced upstream (Final, ANN401, S110, TRY300, kwargs typing) Rebasing onto litellm_internal_staging pulled in 116 upstream commits that introduced/tightened several lint gates this PR's own code now trips: - LIT010 (every local/module-level variable must be Final): added Final annotations across _logging.py, litellm_logging.py, streaming_handler.py, litellm_pre_call_utils.py, and utils.py. Where a name is genuinely reassigned (logging_obj: starts None, later set to the real object) or branch-assigned, either restructured into a single ternary expression (ordered_candidates) or suppressed with `# rebind-ok: <reason>` matching this repo's documented escape hatch. - LIT011 (parameter mutation): suppressed the two new `data[key] = value` writes in litellm_pre_call_utils.py with `# rebind-ok`, matching the unsuppressed precedent already used for every other `data[...]` write in the same function - `data` is an intentional out-param there. - ANN001/ANN003/ANN202 (missing parameter/return type annotations): fully typed success_handler/_success_handler_body, their async twins, and failure_handler/_failure_handler_body/async variants in litellm_logging.py, plus function_setup in utils.py (added Rules to its existing TYPE_CHECKING block for the rules_obj: Rules annotation). - ANN401 (explicit Any disallowed): suppressed with `# noqa: ANN401` on the handful of genuinely-heterogeneous result/*args/**kwargs parameters, since ordinary suppression is this repo's documented path. - S110 (try/except/pass): added to the existing BLE001 noqa on the one best-effort correlation-cleanup try/except this PR added. - TRY300 (return inside try): moved two `return result` statements into `else:` blocks in the retry-fallback paths this PR's own diff touched. - reportPrivateUsage (basedpyright): renamed the two new StandardLoggingPayloadSetup static methods (get_standard_logging_payload_ trace_id/session_id) to drop their leading underscore, since they're genuinely called from a sibling module-level function in the same file. No behavior change - confirmed by the full existing test suite (819 passed) plus all four lint gates (ruff format, ruff-strict, type-discipline, basedpyright) passing clean. * fix(lint): stop RUF100 flagging noqa suppressions the strict gate needs CI's plain "ruff check" job uses the default ruff.toml, a narrower config than ruff-strict.toml (used only by the strict-rule budget gate). ANN401 and S110 aren't enabled in the default config, so RUF100 (unused-noqa) flagged the `# noqa: ANN401`/`# noqa: ...,S110` suppressions this PR added as pointless under that config, even though they're genuinely needed under ruff-strict.toml. - ANN401: added to ruff.toml's existing `lint.external` list (same mechanism already used for C901/TID251, enforced by the strict gate but not by this config) - these Any usages are genuinely dynamic/forwarded, so the suppression itself is correct and just needed registering. - S110: fixed the underlying code instead of registering another external code - the try/except/pass in CustomStreamWrapper._restore_consumer_correlation_context now logs at debug level on failure (matching the existing best-effort-cleanup pattern in _record_partial_usage_for_failure elsewhere in this file), which satisfies S110's own suggestion directly and needs no suppression at all. Verified against both ruff.toml and ruff-strict.toml directly, plus all three other gates (ruff format, type-discipline, basedpyright) and the full test suite (821 passed). * fix(lint): scope the ANN401 exemption to file level instead of a repo-wide noqa Ruff has no per-line-scoped way to register a noqa code across configs (that requires the default ruff.toml's lint.external list, which is repo-wide in scope even though the noqa itself is per-line). Since ruff does support file-level exemptions via per-file-ignores, and ANN401 only needed exempting in exactly two files, moved the exemption there instead: - ruff-strict.toml: added [lint.per-file-ignores] disabling ANN401 for litellm_logging.py and utils.py specifically, with a comment explaining why (heterogeneous response/forwarded-args parameters with no fitting concrete type - already verified by trying CostResponseTypes and hitting a real basedpyright mismatch). - ruff.toml: reverted the ANN401 entry from lint.external - no longer needed, since there's no `# noqa: ANN401` left anywhere for RUF100 to second-guess. - Removed the now-redundant `# noqa: ANN401` from the 10 affected parameters in both files, keeping the existing kwargs-ok reasons and adding a short inline comment on the `result`/`*args` lines pointing at the ruff-strict.toml exemption for context. Verified against both configs directly (ANN401 clean under ruff-strict.toml for these files, RUF100 clean under the default config), all four gates (ruff format, ruff-strict, type-discipline, basedpyright), and the full test suite (821 passed). * fix(logging): redact credential-shaped trace_id/session_id before stamping log records CorrelationContextFilter stamps trace_id/session_id onto a LogRecord after SecretRedactionFilter has already run, so a caller-controlled value (e.g. via x-litellm-trace-id or a W3C baggage header) that happens to look like a real credential reached JSON and plaintext logs unredacted. Apply the same credential redaction already used elsewhere in this module at _sanitize_correlation_id(), the single choke point both set_trace_id() and set_session_id() route through, so every caller-facing entry point is covered without depending on filter ordering. * fix(logging): restore correlation context when a stream's max-duration timeout fires CustomStreamWrapper.__anext__() called _check_max_streaming_duration() before entering its try block, so the litellm.Timeout it raises bypassed the except Exception -> _handle_stream_fallback_error path entirely, leaking the timed-out stream's own trace_id/session_id into whatever the consumer's task logs next. Move the check inside the try so it flows through the same restoration path every other stream failure already uses. * test(streaming): make dispatch_failure_handlers mock awaitable for the async max-duration test Moving _check_max_streaming_duration() inside __anext__()'s try block (prior commit) means a max-duration Timeout now dispatches failure handlers through the same path every other stream failure already uses, instead of bypassing it entirely. dispatch_failure_handlers is async on the real Logging class; the test's plain MagicMock logging_obj made asyncio.create_task() choke on a non-coroutine return value once that path actually got exercised. --------- Co-authored-by: Deepanshu <deepanshu.lulla@alpha-sense.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
642 lines
23 KiB
Python
642 lines
23 KiB
Python
import ast
|
|
import asyncio
|
|
import json
|
|
import os
|
|
import sys
|
|
from pathlib import Path
|
|
from typing import List
|
|
|
|
import pytest
|
|
|
|
sys.path.insert(
|
|
0, os.path.abspath("../../..")
|
|
) # Adds the parent directory to the system-path
|
|
import logging
|
|
import sys
|
|
|
|
import litellm
|
|
from litellm._logging import (
|
|
ALL_LOGGERS,
|
|
CorrelationContextFilter,
|
|
CorrelationPlainFormatter,
|
|
JsonFormatter,
|
|
_initialize_loggers_with_handler,
|
|
_turn_on_json,
|
|
session_id_var,
|
|
set_session_id,
|
|
set_trace_id,
|
|
trace_id_var,
|
|
verbose_logger,
|
|
verbose_proxy_logger,
|
|
verbose_router_logger,
|
|
)
|
|
from litellm.integrations.custom_logger import CustomLogger
|
|
from litellm.types.utils import StandardLoggingPayload
|
|
|
|
|
|
class CacheHitCustomLogger(CustomLogger):
|
|
def __init__(self, *args, **kwargs):
|
|
super().__init__(*args, **kwargs)
|
|
self.logged_standard_logging_payloads: List[StandardLoggingPayload] = []
|
|
|
|
async def async_log_success_event(self, kwargs, response_obj, start_time, end_time):
|
|
standard_logging_payload = kwargs.get("standard_logging_object", None)
|
|
if standard_logging_payload:
|
|
self.logged_standard_logging_payloads.append(standard_logging_payload)
|
|
|
|
|
|
def test_json_mode_emits_one_record_per_logger(capfd):
|
|
# Turn on JSON logging
|
|
_turn_on_json()
|
|
# Make sure our loggers will emit INFO-level records
|
|
for lg in (verbose_logger, verbose_router_logger, verbose_proxy_logger):
|
|
lg.setLevel(logging.INFO)
|
|
|
|
# Log one message from each logger at different levels
|
|
verbose_logger.info("first info")
|
|
verbose_router_logger.info("second info from router")
|
|
verbose_proxy_logger.info("third info from proxy")
|
|
|
|
# Capture stdout
|
|
out, err = capfd.readouterr()
|
|
print("out", out)
|
|
print("err", err)
|
|
lines = [l for l in err.splitlines() if l.strip()]
|
|
|
|
# Expect exactly three JSON lines
|
|
assert len(lines) == 3, f"got {len(lines)} lines, want 3: {lines!r}"
|
|
|
|
# Each line must be valid JSON with the required fields
|
|
for line in lines:
|
|
obj = json.loads(line)
|
|
assert "message" in obj, "`message` key missing"
|
|
assert "level" in obj, "`level` key missing"
|
|
assert "timestamp" in obj, "`timestamp` key missing"
|
|
|
|
|
|
def test_json_formatter_parses_embedded_json_message():
|
|
"""
|
|
Test that JsonFormatter parses embedded JSON in the message field and promotes
|
|
sub-fields to first-class JSON properties for downstream querying.
|
|
"""
|
|
formatter = JsonFormatter()
|
|
record = logging.LogRecord(
|
|
name="LiteLLM",
|
|
level=logging.DEBUG,
|
|
pathname="",
|
|
lineno=0,
|
|
msg='{"event": "giveup", "exception": "Connection failed", "model_name": "gpt-4"}',
|
|
args=(),
|
|
exc_info=None,
|
|
)
|
|
output = formatter.format(record)
|
|
obj = json.loads(output)
|
|
# Standard fields preserved
|
|
assert "message" in obj
|
|
assert obj["level"] == "DEBUG"
|
|
assert "timestamp" in obj
|
|
# Embedded JSON fields promoted to top-level for querying
|
|
assert obj["event"] == "giveup"
|
|
assert obj["exception"] == "Connection failed"
|
|
assert obj["model_name"] == "gpt-4"
|
|
|
|
|
|
def test_json_formatter_includes_extra_attributes():
|
|
"""
|
|
Test that JsonFormatter includes extra attributes from logger.debug("msg", extra={...}).
|
|
"""
|
|
formatter = JsonFormatter()
|
|
record = logging.LogRecord(
|
|
name="LiteLLM",
|
|
level=logging.DEBUG,
|
|
pathname="",
|
|
lineno=0,
|
|
msg="POST Request Sent from LiteLLM",
|
|
args=(),
|
|
exc_info=None,
|
|
)
|
|
record.api_base = "https://api.openai.com"
|
|
record.authorization = "Bearer sk-***"
|
|
output = formatter.format(record)
|
|
obj = json.loads(output)
|
|
assert obj["message"] == "POST Request Sent from LiteLLM"
|
|
assert obj["api_base"] == "https://api.openai.com"
|
|
assert obj["authorization"] == "Bearer sk-***"
|
|
|
|
|
|
def test_json_formatter_plain_message_unchanged():
|
|
"""
|
|
Test that non-JSON messages are passed through as-is in the message field.
|
|
"""
|
|
formatter = JsonFormatter()
|
|
record = logging.LogRecord(
|
|
name="LiteLLM",
|
|
level=logging.INFO,
|
|
pathname="",
|
|
lineno=0,
|
|
msg="Cache hit!",
|
|
args=(),
|
|
exc_info=None,
|
|
)
|
|
output = formatter.format(record)
|
|
obj = json.loads(output)
|
|
assert obj["message"] == "Cache hit!"
|
|
assert "event" not in obj
|
|
assert "exception" not in obj
|
|
|
|
|
|
def test_json_formatter_parses_embedded_python_dict_repr():
|
|
"""
|
|
Test that JsonFormatter parses Python dict repr (str/deployment) embedded in
|
|
plain text, e.g. from get_available_deployment logs.
|
|
Reproduces Roni's reported case.
|
|
"""
|
|
formatter = JsonFormatter()
|
|
msg = (
|
|
"get_available_deployment for model: text-embedding-3-large, "
|
|
"Selected deployment: {'model_name': 'text-embedding-3-large', "
|
|
"'litellm_params': {'api_key': 'sk**********', 'tpm': 1000000, 'rpm': 2000, "
|
|
"'use_in_pass_through': False, 'use_litellm_proxy': False, "
|
|
"'merge_reasoning_content_in_choices': False, 'model': 'text-embedding-3-large'}, "
|
|
"'model_info': {'id': 'a624b057aec64ada48311', 'db_model': False}} "
|
|
"for model: text-embedding-3-large"
|
|
)
|
|
record = logging.LogRecord(
|
|
name="LiteLLM Router",
|
|
level=logging.INFO,
|
|
pathname="",
|
|
lineno=0,
|
|
msg=msg,
|
|
args=(),
|
|
exc_info=None,
|
|
)
|
|
output = formatter.format(record)
|
|
obj = json.loads(output)
|
|
assert "message" in obj
|
|
assert obj["level"] == "INFO"
|
|
# Python dict parsed and promoted to first-class properties
|
|
assert obj["model_name"] == "text-embedding-3-large"
|
|
assert "litellm_params" in obj
|
|
assert obj["litellm_params"]["api_key"] == "sk**********"
|
|
assert obj["litellm_params"]["tpm"] == 1000000
|
|
assert obj["litellm_params"]["use_in_pass_through"] is False
|
|
assert "model_info" in obj
|
|
assert obj["model_info"]["id"] == "a624b057aec64ada48311"
|
|
assert obj["model_info"]["db_model"] is False
|
|
|
|
|
|
def test_json_formatter_includes_component_field():
|
|
"""
|
|
Test that JsonFormatter always emits a 'component' field equal to the logger name.
|
|
This allows filtering by component (e.g. "LiteLLM Proxy") in Datadog / third-party log services.
|
|
"""
|
|
formatter = JsonFormatter()
|
|
for logger_name in ("LiteLLM Proxy", "LiteLLM Router", "LiteLLM"):
|
|
record = logging.LogRecord(
|
|
name=logger_name,
|
|
level=logging.ERROR,
|
|
pathname="proxy_server.py",
|
|
lineno=42,
|
|
msg="something went wrong",
|
|
args=(),
|
|
exc_info=None,
|
|
)
|
|
output = formatter.format(record)
|
|
obj = json.loads(output)
|
|
assert (
|
|
obj["component"] == logger_name
|
|
), f"Expected component={logger_name!r}, got {obj.get('component')!r}"
|
|
|
|
|
|
def test_json_formatter_includes_logger_field():
|
|
"""
|
|
Test that JsonFormatter always emits a 'logger' field with filename:lineno.
|
|
This allows pinpointing the exact source of a log line in third-party services.
|
|
"""
|
|
formatter = JsonFormatter()
|
|
record = logging.LogRecord(
|
|
name="LiteLLM Proxy",
|
|
level=logging.INFO,
|
|
pathname="/app/litellm/proxy/proxy_server.py",
|
|
lineno=123,
|
|
msg="request received",
|
|
args=(),
|
|
exc_info=None,
|
|
)
|
|
output = formatter.format(record)
|
|
obj = json.loads(output)
|
|
assert (
|
|
obj["logger"] == "proxy_server.py:123"
|
|
), f"Expected logger='proxy_server.py:123', got {obj['logger']!r}"
|
|
|
|
|
|
def test_json_formatter_extra_component_not_overwritten():
|
|
"""
|
|
User-supplied extra={"component": "..."} must not be silently dropped.
|
|
"""
|
|
formatter = JsonFormatter()
|
|
record = logging.LogRecord(
|
|
name="LiteLLM Proxy",
|
|
level=logging.INFO,
|
|
pathname="proxy_server.py",
|
|
lineno=1,
|
|
msg="event",
|
|
args=(),
|
|
exc_info=None,
|
|
)
|
|
record.component = "auth-service"
|
|
obj = json.loads(formatter.format(record))
|
|
assert (
|
|
obj["component"] == "auth-service"
|
|
), f"User-supplied component was overwritten, got {obj['component']!r}"
|
|
|
|
|
|
def test_initialize_loggers_with_handler_sets_propagate_false():
|
|
"""
|
|
Test that the initialize_loggers_with_handler function sets propagate to False for all loggers
|
|
"""
|
|
# Initialize loggers with the test handler
|
|
_initialize_loggers_with_handler(logging.StreamHandler())
|
|
|
|
# Check that propagate is set to False for all loggers
|
|
for logger in ALL_LOGGERS:
|
|
assert (
|
|
logger.propagate is False
|
|
), f"Logger {logger.name} has propagate set to {logger.propagate}, expected False"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_cache_hit_includes_custom_llm_provider():
|
|
"""
|
|
Test that when there's a cache hit, the standard logging payload includes the custom_llm_provider
|
|
"""
|
|
# Set up caching and custom logger
|
|
litellm.cache = litellm.Cache()
|
|
test_custom_logger = CacheHitCustomLogger()
|
|
original_callbacks = litellm.callbacks.copy() if litellm.callbacks else []
|
|
litellm.callbacks = [test_custom_logger]
|
|
|
|
try:
|
|
# First call - should be a cache miss
|
|
response1 = await litellm.acompletion(
|
|
model="gpt-3.5-turbo",
|
|
messages=[{"role": "user", "content": "test cache hit message"}],
|
|
mock_response="test response",
|
|
caching=True,
|
|
)
|
|
|
|
# Wait for logging to complete
|
|
await asyncio.sleep(0.5)
|
|
|
|
# Second identical call - should be a cache hit
|
|
response2 = await litellm.acompletion(
|
|
model="gpt-3.5-turbo",
|
|
messages=[{"role": "user", "content": "test cache hit message"}],
|
|
mock_response="test response",
|
|
caching=True,
|
|
)
|
|
|
|
# Wait for logging to complete
|
|
await asyncio.sleep(0.5)
|
|
|
|
# Verify we have logged events
|
|
assert (
|
|
len(test_custom_logger.logged_standard_logging_payloads) >= 2
|
|
), f"Expected at least 2 logged events, got {len(test_custom_logger.logged_standard_logging_payloads)}"
|
|
|
|
# Find the cache hit event (should be the second call)
|
|
cache_hit_payload = None
|
|
for payload in test_custom_logger.logged_standard_logging_payloads:
|
|
if payload.get("cache_hit") is True:
|
|
cache_hit_payload = payload
|
|
break
|
|
|
|
# Verify cache hit event was found
|
|
assert (
|
|
cache_hit_payload is not None
|
|
), "No cache hit event found in logged payloads"
|
|
|
|
# Verify custom_llm_provider is included in the cache hit payload
|
|
assert (
|
|
"custom_llm_provider" in cache_hit_payload
|
|
), "custom_llm_provider missing from cache hit standard logging payload"
|
|
|
|
# Verify custom_llm_provider has a valid value (should be "openai" for gpt-3.5-turbo)
|
|
custom_llm_provider = cache_hit_payload["custom_llm_provider"]
|
|
assert (
|
|
custom_llm_provider is not None and custom_llm_provider != ""
|
|
), f"custom_llm_provider should not be None or empty, got: {custom_llm_provider}"
|
|
|
|
print(
|
|
f"Cache hit standard logging payload with custom_llm_provider: {custom_llm_provider}",
|
|
json.dumps(cache_hit_payload, indent=2),
|
|
)
|
|
|
|
finally:
|
|
# Clean up
|
|
litellm.callbacks = original_callbacks
|
|
litellm.cache = None
|
|
|
|
|
|
LITELLM_LOGGER_NAMES = frozenset(
|
|
{"verbose_logger", "verbose_proxy_logger", "verbose_router_logger", "logger", "logging"}
|
|
)
|
|
LOG_LEVEL_METHODS = frozenset({"debug", "info", "warning", "error", "exception", "critical"})
|
|
LITELLM_PACKAGE_ROOT = Path(__file__).resolve().parents[2] / "litellm"
|
|
|
|
|
|
def _receiver_name(node: ast.expr) -> str:
|
|
if isinstance(node, ast.Name):
|
|
return node.id
|
|
if isinstance(node, ast.Attribute):
|
|
return node.attr
|
|
return ""
|
|
|
|
|
|
def _is_logging_call(node: ast.AST) -> bool:
|
|
return (
|
|
isinstance(node, ast.Call)
|
|
and isinstance(node.func, ast.Attribute)
|
|
and node.func.attr in LOG_LEVEL_METHODS
|
|
and _receiver_name(node.func.value) in LITELLM_LOGGER_NAMES
|
|
)
|
|
|
|
|
|
def _has_format_spec(message: ast.JoinedStr) -> bool:
|
|
return any(isinstance(value, ast.FormattedValue) and value.format_spec is not None for value in message.values)
|
|
|
|
|
|
def _eager_logging_calls(source: str, path: Path) -> tuple[str, ...]:
|
|
return tuple(
|
|
f"{path}:{node.lineno}"
|
|
for node in ast.walk(ast.parse(source))
|
|
if _is_logging_call(node)
|
|
and node.args
|
|
and isinstance(node.args[0], ast.JoinedStr)
|
|
and not _has_format_spec(node.args[0])
|
|
)
|
|
|
|
|
|
def test_logging_calls_do_not_build_their_message_eagerly():
|
|
"""A discarded log record must not have cost anything to build.
|
|
|
|
`log.debug(f"payload: {body}")` interpolates before the call runs, so the message is
|
|
built and thrown away on every request the level filters out; `log.debug("payload: %s", body)`
|
|
defers that to `record.getMessage()`, which only runs once the record passes the level check.
|
|
|
|
f-strings carrying a format spec are exempt: `%`-style has no faithful equivalent for
|
|
specs like `{ratio:.1%}`, and those sites interpolate scalars rather than payloads.
|
|
"""
|
|
offenders = tuple(
|
|
offender
|
|
for path in sorted(LITELLM_PACKAGE_ROOT.rglob("*.py"))
|
|
for offender in _eager_logging_calls(
|
|
path.read_text(encoding="utf-8"), path.relative_to(LITELLM_PACKAGE_ROOT.parent)
|
|
)
|
|
)
|
|
|
|
assert offenders == (), (
|
|
"these logging calls build their message eagerly; pass the values as %-style arguments instead:\n"
|
|
+ "\n".join(offenders)
|
|
)
|
|
|
|
|
|
class _JsonCapture(logging.Handler):
|
|
def __init__(self):
|
|
super().__init__()
|
|
self.formatter = JsonFormatter()
|
|
self.records: list[dict] = []
|
|
self.addFilter(CorrelationContextFilter())
|
|
|
|
def emit(self, record):
|
|
self.records.append(json.loads(self.formatter.format(record)))
|
|
|
|
|
|
def _make_capture_logger(name: str) -> tuple[logging.Logger, _JsonCapture]:
|
|
lg = logging.getLogger(name)
|
|
cap = _JsonCapture()
|
|
lg.addHandler(cap)
|
|
lg.setLevel(logging.DEBUG)
|
|
return lg, cap
|
|
|
|
|
|
def test_trace_id_injected_into_json_record(monkeypatch):
|
|
"""trace_id set via set_trace_id() appears in every JSON record in that context."""
|
|
monkeypatch.setattr(litellm, "request_correlation_in_logs", True)
|
|
lg, cap = _make_capture_logger("test.trace_inject")
|
|
set_trace_id("trace-abc-123")
|
|
try:
|
|
lg.info("test message")
|
|
assert len(cap.records) == 1
|
|
assert cap.records[0]["trace_id"] == "trace-abc-123"
|
|
finally:
|
|
trace_id_var.set("")
|
|
|
|
|
|
def test_session_id_injected_when_set(monkeypatch):
|
|
"""session_id set via set_session_id() appears in JSON record."""
|
|
monkeypatch.setattr(litellm, "request_correlation_in_logs", True)
|
|
lg, cap = _make_capture_logger("test.session_inject")
|
|
set_session_id("sess-xyz-456")
|
|
try:
|
|
lg.info("another message")
|
|
assert cap.records[0]["session_id"] == "sess-xyz-456"
|
|
finally:
|
|
session_id_var.set("")
|
|
|
|
|
|
def test_trace_id_and_session_id_cannot_be_spoofed_by_message_content(monkeypatch):
|
|
"""A log message that happens to parse as JSON/dict with "trace_id"/"session_id"
|
|
keys (e.g. the proxy logging a raw request-header dict) must not override the
|
|
real correlation ids set via set_trace_id()/set_session_id()."""
|
|
monkeypatch.setattr(litellm, "request_correlation_in_logs", True)
|
|
lg, cap = _make_capture_logger("test.spoof_attempt")
|
|
set_trace_id("real-trace-id")
|
|
set_session_id("real-session-id")
|
|
try:
|
|
lg.info('{"trace_id": "attacker-supplied-trace", "session_id": "attacker-supplied-session"}')
|
|
assert cap.records[0]["trace_id"] == "real-trace-id"
|
|
assert cap.records[0]["session_id"] == "real-session-id"
|
|
finally:
|
|
trace_id_var.set("")
|
|
session_id_var.set("")
|
|
|
|
|
|
def test_trace_id_and_session_id_cannot_be_injected_with_no_active_context(monkeypatch):
|
|
"""A message that happens to parse as JSON/dict with "trace_id"/"session_id" keys
|
|
must not surface those fields at all when CorrelationContextFilter hasn't stamped
|
|
this record - e.g. a log line emitted before Logging.__init__() runs for a request
|
|
(request_correlation_in_logs on, but no genuine trace/session id active yet)."""
|
|
monkeypatch.setattr(litellm, "request_correlation_in_logs", True)
|
|
lg, cap = _make_capture_logger("test.no_context_spoof_attempt")
|
|
trace_id_var.set("")
|
|
session_id_var.set("")
|
|
lg.info('{"trace_id": "attacker-supplied-trace", "session_id": "attacker-supplied-session"}')
|
|
assert "trace_id" not in cap.records[0]
|
|
assert "session_id" not in cap.records[0]
|
|
|
|
|
|
def test_trace_id_and_session_id_are_redacted_when_credential_shaped(monkeypatch):
|
|
"""A caller-controlled trace_id/session_id (e.g. from x-litellm-trace-id or a W3C
|
|
baggage header) that happens to look like a real credential must not reach log
|
|
records unredacted. CorrelationContextFilter stamps trace_id/session_id onto the
|
|
record after SecretRedactionFilter has already run, so those two fields would
|
|
otherwise bypass credential redaction entirely - the fix redacts at set_trace_id()/
|
|
set_session_id() time instead, before the value ever reaches a log record."""
|
|
monkeypatch.setattr(litellm, "request_correlation_in_logs", True)
|
|
lg, cap = _make_capture_logger("test.credential_shaped_correlation_id")
|
|
poisoned_trace_id = "sk-ant-api03-" + "A" * 40
|
|
poisoned_session_id = "AKIA" + "B" * 16
|
|
set_trace_id(poisoned_trace_id)
|
|
set_session_id(poisoned_session_id)
|
|
try:
|
|
lg.info("some benign log line")
|
|
assert cap.records[0]["trace_id"] == "REDACTED"
|
|
assert cap.records[0]["session_id"] == "REDACTED"
|
|
assert poisoned_trace_id not in json.dumps(cap.records[0])
|
|
assert poisoned_session_id not in json.dumps(cap.records[0])
|
|
finally:
|
|
trace_id_var.set("")
|
|
session_id_var.set("")
|
|
|
|
|
|
def test_session_id_absent_when_not_set():
|
|
"""session_id must NOT appear in JSON record when not set for this context."""
|
|
lg, cap = _make_capture_logger("test.no_session")
|
|
session_id_var.set("")
|
|
lg.info("no session message")
|
|
assert "session_id" not in cap.records[0]
|
|
|
|
|
|
def test_trace_id_absent_when_not_set():
|
|
"""trace_id must NOT appear when not set."""
|
|
lg, cap = _make_capture_logger("test.no_trace")
|
|
trace_id_var.set("")
|
|
lg.info("no trace message")
|
|
assert "trace_id" not in cap.records[0]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_contextvar_isolation_between_tasks():
|
|
"""Two concurrent async tasks each see only their own trace_id."""
|
|
results: dict[str, str] = {}
|
|
|
|
async def task(task_id: str, trace_id: str) -> None:
|
|
set_trace_id(trace_id)
|
|
await asyncio.sleep(0)
|
|
results[task_id] = trace_id_var.get()
|
|
|
|
await asyncio.gather(
|
|
task("A", "trace-for-A"),
|
|
task("B", "trace-for-B"),
|
|
)
|
|
|
|
assert results["A"] == "trace-for-A"
|
|
assert results["B"] == "trace-for-B"
|
|
|
|
|
|
def test_trace_id_not_in_log_when_flag_disabled(monkeypatch):
|
|
"""When request_correlation_in_logs is False (default), trace_id must not appear in JSON records even when set."""
|
|
monkeypatch.setattr(litellm, "request_correlation_in_logs", False)
|
|
lg, cap = _make_capture_logger("test.no_trace_gated")
|
|
set_trace_id("trace-should-not-appear")
|
|
try:
|
|
lg.info("message")
|
|
assert "trace_id" not in cap.records[0]
|
|
finally:
|
|
trace_id_var.set("")
|
|
|
|
|
|
def test_session_id_not_in_log_when_flag_disabled(monkeypatch):
|
|
"""When request_correlation_in_logs is False (default), session_id must not appear in JSON records even when set."""
|
|
monkeypatch.setattr(litellm, "request_correlation_in_logs", False)
|
|
lg, cap = _make_capture_logger("test.no_session_gated")
|
|
set_session_id("sess-should-not-appear")
|
|
try:
|
|
lg.info("message")
|
|
assert "session_id" not in cap.records[0]
|
|
finally:
|
|
session_id_var.set("")
|
|
|
|
|
|
class _PlainCapture(logging.Handler):
|
|
def __init__(self):
|
|
super().__init__()
|
|
self.formatter = CorrelationPlainFormatter("%(message)s")
|
|
self.records: list[str] = []
|
|
self.addFilter(CorrelationContextFilter())
|
|
|
|
def emit(self, record):
|
|
self.records.append(self.formatter.format(record))
|
|
|
|
|
|
def _make_plain_capture_logger(name: str) -> tuple[logging.Logger, _PlainCapture]:
|
|
lg = logging.getLogger(name)
|
|
cap = _PlainCapture()
|
|
lg.addHandler(cap)
|
|
lg.setLevel(logging.DEBUG)
|
|
return lg, cap
|
|
|
|
|
|
def test_plain_formatter_appends_trace_id_and_session_id(monkeypatch):
|
|
"""CorrelationPlainFormatter must append trace_id/session_id to non-JSON log lines too."""
|
|
monkeypatch.setattr(litellm, "request_correlation_in_logs", True)
|
|
lg, cap = _make_plain_capture_logger("test.plain_trace_session")
|
|
set_trace_id("plain-trace-1")
|
|
set_session_id("plain-session-1")
|
|
try:
|
|
lg.info("plaintext message")
|
|
assert cap.records[0] == "plaintext message [trace_id=plain-trace-1 session_id=plain-session-1]"
|
|
finally:
|
|
trace_id_var.set("")
|
|
session_id_var.set("")
|
|
|
|
|
|
def test_plain_formatter_appends_only_trace_id_when_session_id_absent(monkeypatch):
|
|
"""Only trace_id is appended when session_id was never set."""
|
|
monkeypatch.setattr(litellm, "request_correlation_in_logs", True)
|
|
lg, cap = _make_plain_capture_logger("test.plain_trace_only")
|
|
set_trace_id("plain-trace-2")
|
|
session_id_var.set("")
|
|
try:
|
|
lg.info("plaintext message")
|
|
assert cap.records[0] == "plaintext message [trace_id=plain-trace-2]"
|
|
finally:
|
|
trace_id_var.set("")
|
|
|
|
|
|
def test_plain_formatter_unchanged_when_flag_disabled(monkeypatch):
|
|
"""When request_correlation_in_logs is False, plain log lines are unmodified even if the contextvars are set."""
|
|
monkeypatch.setattr(litellm, "request_correlation_in_logs", False)
|
|
lg, cap = _make_plain_capture_logger("test.plain_flag_off")
|
|
set_trace_id("should-not-appear")
|
|
set_session_id("should-not-appear")
|
|
try:
|
|
lg.info("plaintext message")
|
|
assert cap.records[0] == "plaintext message"
|
|
finally:
|
|
trace_id_var.set("")
|
|
session_id_var.set("")
|
|
|
|
|
|
def test_set_trace_id_strips_control_characters():
|
|
"""set_trace_id() must strip \\r/\\n/escape sequences so a caller-controlled
|
|
trace id can't forge fake log entries when interpolated into plain-text logs."""
|
|
token = set_trace_id('evil\r\n{"level": "CRITICAL", "message": "forged"}')
|
|
try:
|
|
value = trace_id_var.get()
|
|
assert "\r" not in value
|
|
assert "\n" not in value
|
|
finally:
|
|
trace_id_var.reset(token)
|
|
|
|
|
|
def test_set_session_id_bounds_length():
|
|
"""set_session_id() must bound length so an oversized caller-supplied value
|
|
isn't repeated across every log line for the request."""
|
|
token = set_session_id("a" * 1000)
|
|
try:
|
|
assert len(session_id_var.get()) == 256
|
|
finally:
|
|
session_id_var.reset(token)
|
|
|