Skip to content

fix(prometheus): track spend + suppress failure counter for cancellations - #82

Merged
songkuan-zheng merged 2 commits into
ship/v1.87.0from
fix/prometheus-cancel-spend-undercount
Jun 10, 2026
Merged

fix(prometheus): track spend + suppress failure counter for cancellations#82
songkuan-zheng merged 2 commits into
ship/v1.87.0from
fix/prometheus-cancel-spend-undercount

Conversation

@songkuan-zheng

@songkuan-zheng songkuan-zheng commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Tier classification

  • D — Universal mechanism + company opinion in litellm/ core

Tried upstream first?

Summary

After PR #78 / #81, the SpendLogs DB row for a cancelled request has correct status="success" + cancellation_indicator + positive spend (via compute_prompt_only_cost). But Prometheus was still divergent: cancelled requests counted as failures AND litellm_spend_metric undercounted.

Two layers fixed in one PR:

Layer 1 — _failure_handler_helper_fn (litellm_logging.py)

response_cost was hardcoded to 0 before building the StandardLoggingPayload. Every SLP-reading callback (Prometheus, Langfuse, Custom Callback API, OTel, Helicone, …) saw 0 for cancellations even though the DB row carried a positive spend.

Fix: detect asyncio.CancelledError and call compute_prompt_only_cost inline. Best-effort — falls back to 0 on cost-map miss or tokenizer failure, never raises.

Layer 2 — async_log_failure_event (prometheus.py)

The failure-event path didn't increment litellm_spend_metric at all, AND it bumped litellm_llm_api_failed_requests_metric (which pollutes failure-rate alerting with what are really client cancellations).

Fix: branch on standard_logging_payload.metadata.cancellation_indicator. When present:

  • Route the SLP's response_cost (now correctly populated by Layer 1) into litellm_spend_metric so Prometheus spend stays in lockstep with the SpendLogs spend column
  • SKIP litellm_llm_api_failed_requests_metric and set_llm_deployment_failure_metrics — cancels are not system failures
  • Track the cancel cost on the org-budget metric too

Non-cancel failures: original behaviour preserved exactly (gated on the indicator).

Verification

Unit tests (Layer 1 — SLP cost population)

  • 3 new tests in test_litellm_logging.py::TestFailureHandlerCancelCost:
    • test_cancelled_error_populates_prompt_only_cost — Anthropic Haiku + 20×repeated long prompt → response_cost > 0
    • test_cancelled_error_falls_back_to_zero_on_unknown_model — unknown model, no raise, finite result
    • test_non_cancel_exception_keeps_response_cost_zeroValueError keeps 0
  • 217/217 cancel + spend_tracking regression suite stays green
  • ✅ One pre-existing failure in test_logfire_logger_accepts_env_vars_for_base_url is unrelated — missing opentelemetry optional dep on the local Python env (confirmed reproducible BEFORE this PR via git stash).

E2E (Layer 2 — Prometheus routing topology)

  • New e2e/cases/data/35_cancel_prometheus_metrics.sh verifies the routing against the real Prometheus client state inside the running proxy (no Counter mocking — would have been theater). Two-probe discrimination:
    • Probe A — streaming cancel → litellm_llm_api_failed_requests_metric_total delta = 0 (cancel suppressed) + SpendLogs row with cancellation marker
    • Probe BX-Mock-Fail: 503 control → delta ≥ 1 (real failure DOES count) + SpendLogs row status=failure
  • ✅ Wired into run-all-cases after case 32. Full cancel suite (26/27/28/29/30/32/35) all PASS end-to-end after the fix.

Case 35 deliberately doesn't assert spend-metric VALUES because the mock provider's mock-claude isn't in the litellm cost map (cost=0 makes Counter.inc(0) a no-op that's indistinguishable from "Layer 2 didn't fire"). Spend-value coverage lives in:

  • The unit tests above (Anthropic Haiku in the cost map → positive response_cost)
  • e2e/cases/data/33_real_anthropic_cancel.sh (real provider, asserts the billed SpendLogs spend column)

Impact

Metric Before After
Cancelled request → litellm_spend_metric ❌ 0 (cancel via failure_hook) compute_prompt_only_cost
Cancelled request → litellm_llm_api_failed_requests_metric ❌ +1 (counted as failure) ✅ skipped
Cancelled request → org budget metric ❌ 0 compute_prompt_only_cost
Cancelled request → SLP response_cost for OTHER callbacks (Langfuse, Custom Callback API, OTel) ❌ 0 ✅ real prompt-only cost
Non-cancel failure → everything unchanged unchanged ✓

Conflict resolutions

N/A — single commit, branch linear from ship/v1.87.0 HEAD 083f325241 (post-#81 merge).

Pre-submission

  • Tests in tests/test_litellm/
  • make test-unit equivalent passes (217/217 cancel + spend tests; one pre-existing unrelated logfire failure documented)
  • PR scope: 1 commit, 2 cohesive layers of the same metric-gap fix

…ions

The cancel-finalize zero-chunk / shield-timeout / no-completion paths
all route through proxy_logging_obj.post_call_failure_hook. That
dispatch reaches every callback in litellm.callbacks, but the failure
side of the integration was systemically undercounting cancel revenue:

Layer 1 — SLP construction (litellm_logging.py):
  _failure_handler_helper_fn hardcoded model_call_details["response_cost"]
  = 0 BEFORE building the StandardLoggingPayload. Every callback that
  reads response_cost off the SLP (Prometheus litellm_spend_metric,
  Langfuse generation cost, Custom Callback API body, OTel spans, etc.)
  saw 0 for client cancellations even though the DB row carried a
  positive spend from proxy_track_cost_callback's compute_prompt_only_cost.

  Fix: detect asyncio.CancelledError and run compute_prompt_only_cost
  inline (best-effort — falls back to 0 on cost-map miss or tokenizer
  failure, never raises).

Layer 2 — Prometheus integration (prometheus.py):
  async_log_failure_event unconditionally bumped
  litellm_llm_api_failed_requests_metric and did NOT call
  _increment_top_level_request_and_spend_metrics at all. Even with
  Layer 1 fixed, the spend counter would still be wrong because the
  failure-event path never touches it.

  Fix: branch on standard_logging_payload.metadata.cancellation_indicator.
  When present:
    • Route SLP.response_cost to litellm_spend_metric.inc(amount=cost)
      — keeps Prometheus spend in lockstep with the SpendLogs spend
      column.
    • SKIP litellm_llm_api_failed_requests_metric and the
      set_llm_deployment_failure_metrics call — cancels are not
      system failures, must not pollute failure-rate dashboards.
    • Org-budget metric tracks the cancel cost too.
  Otherwise (real failures) the original behaviour is preserved
  exactly.

Tests:
  - test_litellm_logging.py: 3 new tests covering the cancel
    response_cost pre-population (positive cost for known model,
    zero/no-raise on unknown model, no change for non-cancel
    exceptions).
  - 217/217 cancel-billing + spend_tracking + cancel_finalize tests
    pass. (The one pre-existing failure in
    test_logfire_logger_accepts_env_vars_for_base_url is a missing-
    `opentelemetry` package on the local Python env, unaffected by
    this change.)

Tier: D — refines the Phase-3 cancel taxonomy by closing a metric
gap inherited from upstream's failure-event design. Both layers are
gated on the cancellation_indicator marker so non-cancel failures
keep identical behaviour. To be folded into the same upstream issue
as PR #78's Tier-D candidate after one internal release dogfood.
End-to-end coverage for PR #82's Layer 2 routing topology. Tests the
real Prometheus client state inside the running proxy (no Counter
mocking, no async_log_failure_event monkey-patching — both would have
been theater).

Two probes against `model=mock-claude`:

  Probe A — streaming cancel (curl --max-time 3 mid-stream):
    expect: litellm_llm_api_failed_requests_metric_total delta = 0
            SpendLogs row status=success + cancellation_indicator

  Probe B — forced X-Mock-Fail: 503 (the control):
    expect: litellm_llm_api_failed_requests_metric_total delta >= 1
            SpendLogs row status=failure + no marker

The A/B discrimination is the load-bearing assertion: both probes
share the same `model` label so the counter delta is purely a
function of which branch of async_log_failure_event fired. Probe B's
positive delta also locks in that the fix isn't over-suppressing.

Spend-VALUE verification (Layer 1's compute_prompt_only_cost
populating SLP.response_cost) lives in
test_litellm_logging.py::TestFailureHandlerCancelCost (uses
anthropic/claude-haiku-4-5 in the cost map) and
e2e/cases/data/33_real_anthropic_cancel.sh (real provider) — both
land positive cost figures the mock provider can't produce because
mock-claude isn't in the cost map.

Wired into run-all-cases under the cancel-billing block (26 → 35).
Verified end-to-end: 7/7 cancel-suite cases PASS including the new
case 35.
@songkuan-zheng
songkuan-zheng merged commit b51fd79 into ship/v1.87.0 Jun 10, 2026
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.

1 participant