fix(proxy): attribute org spend via team when key has no org_id - #30799
fix(proxy): attribute org spend via team when key has no org_id#30799mateo-berri wants to merge 3 commits into
Conversation
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes a spend-attribution gap where keys created under a team (with no direct
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to the async spend-tracking path, uses the existing cached get_team_object helper, does not touch auth or access-control logic, and is guarded by a full exception handler that logs at warning level and falls back gracefully. The fix is a straightforward parity change: spend tracking now resolves org the same way enforcement already does. The lazy team lookup is cache-backed, the error path is safe, existing tests are untouched, and six new tests including an end-to-end behavioral assertion cover all branches. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/hooks/proxy_track_cost_callback.py | Adds _resolve_spend_tracking_org_id to fall back to the team's organization_id when the key carries no org_id, matching the existing enforcement-time fallback in _organization_max_budget_check. |
| tests/test_litellm/proxy/hooks/test_proxy_track_cost_callback.py | Adds six new unit tests covering all branches of _resolve_spend_tracking_org_id and one end-to-end behavioral test verifying org_id propagates to update_database and increment_spend_counters. No existing tests modified. |
Reviews (3): Last reviewed commit: "fix(proxy): log team-lookup failures in ..." | Re-trigger Greptile
|
|
…rage Add cases for the no-db-client early return and the swallowed team-lookup failure in _resolve_spend_tracking_org_id so every new line in the patch is exercised, satisfying codecov's 100% patch coverage gate.
…ng level A team lookup failure during spend tracking silently dropped org attribution. At debug level this gap is invisible in production, so an unenforced org budget has no signal to investigate. Log at warning with the impact, and assert the warning in the regression test so it can't quietly fall back to debug.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 51e7fd4. Configure here.
|
I think this might be a no-op. Closing for now pending more investigation |
Relevant issues
Surfaced by the new every-12h e2e suite in #30790 (
tests/e2e_tests/budgets/test_budget_enforcement_e2e.py::test_organization_budget_blocks), which is the "org level budget gap" flagged in #eng.Linear ticket
Pre-Submission checklist
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer review (5/5)Changes
Org budget enforcement and org spend tracking disagreed on how they resolve a request's organization. Enforcement (
auth_checks._organization_max_budget_checkandbudget_reservation._get_org_budget_counter) falls back to the team'sorganization_idwhen the key itself has noorg_id, but spend tracking did not: inproxy_track_cost_callback._PROXY_track_cost_callbackthe org came straight frommetadata["user_api_key_org_id"](i.e.valid_token.org_id), which is null for a key created under a team that belongs to an org. Keys generated with only ateam_iddo not inherit the team's org onto the key row, so that value is null in the common case.The result is that for a key whose org comes from its team,
_update_org_db, the daily org transaction, and the unreservedspend:org:{org_id}counter were all skipped (org_id None). The org'sLiteLLM_OrganizationTable.spendcolumn, which is the persisted floor that read-time enforcement reads back throughget_current_spend(..., fallback_spend=org_table.spend), therefore stayed at zero. Org budgets then fail to enforce whenever the in-memory reservation counter isn't the source of truth: across workers that don't share it (the e2e proxy runs with--workers), after the counter's TTL, or when reservation is disabled / the model is unpriced.The fix resolves the spend-tracking org the same way enforcement already does: when the request has no
org_idbut does have ateam_id, look up the (cached) team and use itsorganization_id. This is confined to the spend-tracking path and deliberately does not mutatevalid_token.org_id, so org-scoped access control (resource ownership, MCP, rate limiting) is untouched.A team-lookup failure during this resolution is swallowed so cost tracking keeps working, but it now logs at
warning(notdebug) with the attribution impact; at debug level that gap would be invisible in production and an unenforced org budget would have no signal to investigate. Follow-up commits also add regression coverage for the no-DB-client and lookup-failure branches so the patch is fully exercised.Where the gap is documented
The PR #30790 budget matrices describe the org row as enforcing off real-time reservation counters; they do not call out that the persisted org floor is never written for team-inherited orgs. The closest existing pointer is the unit suite
tests/test_litellm/proxy/auth/test_organization_budget_enforcement.py, which only passes because it injects orgspenddirectly and mocksget_org_object, so it never exercises the team->org spend write path that the live e2e test does. There is no public docs page describing this gap; it lives in those test matrices plus the live suite.Screenshots / Proof of Fix
Unit + regression (the two new behavioral tests fail before the fix with
org_id == None, pass after):Mateo: to reproduce the original gap on a live proxy without waiting 12h, run the proxy with
--workers 2(no shared redis) and the budget suite's org case against a real model; pre-fix the org never blocks becauseorganization.spendstays 0, post-fix it blocks once accumulated org spend crossesmax_budget. I did not run the full live suite here since it needs the multi-worker proxy + real provider keys; happy to attach run logs if you want them before the deploy.Type
🐛 Bug Fix
Slack Thread