fix(logging): dedupe success logging for non-streaming anthropic_messages (/v1/messages) - #31172
Conversation
Congrats! CodSpeed is installed 🎉
You will start to see performance impacts in the reports once the benchmarks are run from your default branch.
|
Greptile SummaryThis PR fixes double-invocation of
Confidence Score: 5/5Safe to merge — the change is a minimal, targeted guard extension in the sync callback dispatcher with a direct regression test. The one-line condition extension is well-understood: No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/litellm_logging.py | Extends the CustomLogger re-dispatch exclusion in success_handler to cover anthropic_messages in addition to pass_through; change is surgical and well-scoped. |
| tests/test_litellm/litellm_core_utils/test_litellm_logging.py | Adds a new test verifying the sync CustomLogger path is skipped for anthropic_messages; no existing tests modified. |
Reviews (1): Last reviewed commit: "fix(logging): dedupe success logging for..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
724216e to
e6d3d8a
Compare
…ages The /v1/messages (anthropic_messages) route is always dispatched through the async wrapper, which logs via async_success_handler. But unlike acompletion, anthropic_messages sets no async flag in litellm_params, so _is_sync_litellm_request returns True in the sync success_handler and it re-dispatches every CustomLogger. The result for a single non-streaming request was two litellm_request OTEL spans and double cost/success callbacks. Skip the sync CustomLogger pass for anthropic_messages, mirroring how pass_through is already handled, so logging runs once via the canonical async path -- matching acompletion behaviour.
e6d3d8a to
2776dec
Compare
|
Rebased onto the current |
|
@Sameerlite could you take a look at this one when you get a chance? It dedupes the double success logging (and double OTEL span) for non-streaming /v1/messages by extending the existing pass_through skip to anthropic_messages. Rebased onto current litellm_internal_staging; the two red checks are the fork-base guard and a pre-existing openapi_compliance failure on base, both unrelated. |
Summary
A single non-streaming
POST /v1/messages(anthropic_messages) emitted twolitellm_requestOTEL spans and fired the success/cost callbacks twice. Plain/chat/completionswas unaffected.Closes #31121
Root cause
/v1/messagesis always dispatched through the async wrapper, so logging runs viaasync_success_handler. The async path also schedules the syncsuccess_handler(viahandle_sync_success_callbacks_for_async_calls) for legacy sync callbacks. That sync pass is supposed to skipCustomLoggerdispatch when async already handled it — but the guard only excludescall_type == pass_through.Unlike
acompletion,anthropic_messagessets no async flag inlitellm_params, so_is_sync_litellm_request()returnsTrueandis_sync_requestisTruein the sync handler. Withanthropic_messagesnot excluded, the sync pass re-dispatches everyCustomLogger(OpenTelemetry,_ProxyDBLoggercost tracking, …) on top of the async one → twolitellm_requestspans and double cost/success callbacks. This is the non-streaming counterpart of #29550 / #29598, whose fix is gated onstream is True.Change
Extend the existing
pass_throughskip insuccess_handlerto also coveranthropic_messages, so the sync pass no longer re-dispatchesCustomLoggers — logging happens once via the canonical async path, matchingacompletion.Verifying it doesn't drop logging
End-to-end with
litellm.anthropic_messages(..., mock_response=...)and a countingCustomLogger:log_success_eventasync_log_success_eventCallbacks that reach the async path (otel via
success_callback: ["otel"],_ProxyDBLogger, any logger inlitellm.callbacks) keep their single async invocation and lose only the duplicate. ACustomLoggerreachable only via the syncsuccess_callbacklist is already not invoked onacompletioneither, so this change makesanthropic_messagesconsistent with the other async paths rather than regressing them.Tests
Added
test_success_handler_skips_sync_callbacks_for_anthropic_messages, mirroring the existing sync/async callback tests. It fails before the change (sync callback fires) and passes after. Fulltests/test_litellm/litellm_core_utils/test_litellm_logging.pypasses (104).