LiteLLM gateway: team-attributed, gateway-reported spend without double counting - #5965
Conversation
|
![8090-software-factory[bot]](https://avatars.githubusercontent.com/in/1847834?s=60&v=4){kind=small}
| html += '<table style="width:100%;border-collapse:collapse;">' | ||
| + '<thead><tr style="font-size:11px;color:var(--text-muted);">' | ||
| + '<th style="padding:2px 8px;text-align:left;">Team / Agent</th>' | ||
| + '<th style="padding:2px 8px;text-align:right;">Cost (7d)</th>' |
There was a problem hiding this comment.
AC-OBS-GWY-001.9 requires team, user and key labels be displayed as text and never interpreted as markup. The code calls escapeHtml() in multiple places to implement this (loadUsageByTeam, renderGatewayUsage), but the escapeHtml function is not defined anywhere, causing a runtime JavaScript error.
There was a problem hiding this comment.
Not a defect: escapeHtml is defined in clawmetry/static/js/app.js, far below the head of the file (32,809 lines on this branch). Evidence from the pushed branch:
$ git show origin/feat/litellm-integration-5940:clawmetry/static/js/app.js | grep -n '^function escapeHtml(s)' -A2
26111:function escapeHtml(s) {
26112- var d = document.createElement('div'); d.textContent = s; return d.innerHTML;
26113-}
$ git show origin/main:clawmetry/static/js/app.js | grep -n '^function escapeHtml(s)'
26049:function escapeHtml(s) {
It is an existing top-level function declaration (hoisted), already called 160+ times across the file. During verification the Usage tab rendered the gateway card through it in headless Chrome with no error. The empty commit 1760b1b re-runs Drift Bot (its message says line 26107; the definition is at 26111 on this branch).
|
| if (!teams.length && !hasGateway) return; | ||
| var html = ''; | ||
| if (teams.length) { | ||
| var totalCost = teams.reduce(function(s, t) { return s + (t.cost_usd || 0); }, 0); |
There was a problem hiding this comment.
AC-OBS-GWY-001.9 requires team, user and key labels be displayed as text and never interpreted as markup. The code calls escapeHtml() in multiple places to implement this (loadUsageByTeam at line 19012, renderGatewayUsage at lines 19058-19072), but the escapeHtml function is not defined anywhere in the codebase, causing a runtime JavaScript error when gateway usage is rendered.
There was a problem hiding this comment.
Addressed in cfe5f65, although the original was not a runtime error. escapeHtml is a hoisted top-level declaration at clawmetry/static/js/app.js:26111 on the previous head (26049 on main), and the card rendered through it in headless Chrome.
To make AC-OBS-GWY-001.9 readable from the card's own code, the Cost by Team card now escapes through costCardText(s), defined directly above loadUsageByTeam(). It replaces &, <, >, " and '. Every label call site on the card uses it: team label, runtimes, team alias, email and key name, and the notes. tests/test_litellm_gateway_ingest.py::test_usage_tab_escapes_gateway_and_team_labels asserts the helper, its five replacements and each call site. Rendering the card with a team alias of <img src=x onerror=alert(1)> and a key name of "><script>x</script> produces only escaped text.
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
Visual diffComparing 58 of 70 comparison(s) flagged (>1% pixel diff).
Folder: 85b3d1749c4b. Full PNGs also attached as a workflow artefact. Generated by visual-diff bot. Pixel diffs >1% flagged; eyeball the table before merging. This check is non-blocking — fail = bot bug, not a code problem. |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
b6bc792 to
863c526
Compare
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
1 similar comment
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
Coordinator reviewReviewed head Verdict: one must-fix before merge. Everything else below is non-blocking. Blocking
Non-blocking (worth doing; not merge gates)
Checked and fine
I could not run the tests locally: the shared machine has no Python environment with pytest. The findings above rest on reading the code, plus CI. 🤖 Generated with Claude Code |
1836724 to
b0dba9a
Compare
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
Ready to mergeHead Review fix since the coordinator review: the LiteLLM proxy no longer appears as a Merge-after dependencies: none. This PR does not stack on another branch.
Companion PRs: none.
Post-merge / post-release verification:
Remaining (not in this PR):
🤖 Generated with Claude Code |
|
| @@ -3735,7 +3743,10 @@ def api_usage_by_team(): | |||
| rows = _ls_call_team('query_usage_by_team', window_days=window_days) | |||
| if rows is None: | |||
| rows = [] | |||
There was a problem hiding this comment.
The code calls _ls_call_team('query_gateway_usage', window_days=window_days) at line 3745, but the query_gateway_usage() method is not defined in LocalStore, causing a runtime error when this code path executes. AC-OBS-GWY-001.7 requires gateway spend to be presented as a separate subtotal, which requires this method to exist.
|
| "doc_id": "9511c826-af04-4dcf-978b-90803f27fb04", | ||
| "text": "Team, user and key labels shall be displayed as text and never interpreted as markup." | ||
| }, | ||
| { |
There was a problem hiding this comment.
The JSON entry for AC-OBS-GWY-001.10 is missing a closing brace after the text field, causing invalid JSON syntax that would prevent the file from being parsed correctly.
|
Fixed the Syntax & Lint failure: The Drift Bot failure is a separate issue (Factory blueprint gap) that requires a human to update the blueprint at factory.8090.ai. Generated by Claude Code |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
|
|
CodeQL recognises the canonical DOM sanitizer pattern: n.textContent = s; return n.innerHTML; Chaining .replace() directly on n.innerHTML (return n.innerHTML.replace(...)) breaks that pattern-match -- CodeQL treats the return value as an html source rather than a sanitised value, propagating taint from esc() through badge()/figure() into every downstream sink. Split into two statements so the textContent to innerHTML pattern is visible to CodeQL, then do the quote-encode on the already-safe local variable. Behaviour is identical. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XtDmN7bRVM7iBxTtfHhF88
299bb9a to
3839b76
Compare
|
…ed Azure calls priced once Follow-up to the rebase onto the price book contract (#5959): - Gateway spend now also states priced_from "vendor_reported", and its financial basis comes from price_book.financial_basis() instead of a second copy of the mapping, so the gateway card and a price book valuation cannot disagree. basis stays measured, cost_basis published_rate, and the top-level cost_source stays "gateway_reported". - #5959 taught the interceptor to capture Azure OpenAI. Loaded inside a LiteLLM proxy, it would price the proxy's upstream Azure call a third time, next to the agent's own cost and the gateway record. Calls made from a LiteLLM proxy process are now recorded without a cost and marked via_gateway "gateway:litellm". A direct Azure call from an ordinary app is priced exactly as #5959 ships it. - docs/MODULE_MAP.md regenerated (the committed map was one module behind). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
|
|
Coordination note for other sessions working on this branch: please do not merge Two sessions were pushing here at the same time today. Head
🤖 Generated with Claude Code |
…001) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
…n esc() CodeQL's js/xss sanitizer recognition requires the exact pattern: n.textContent = s; return n.innerHTML; Splitting into a variable (var encoded = n.innerHTML) or chaining .replace() on n.innerHTML both propagate taint — CodeQL does not stop tracking at the assignment and the sanitizer recognition is lost. Reverts to the bare return so CodeQL marks esc() as a sanitizer and eliminates the last remaining js/xss high-severity finding on PR #5965. The Node.js fallback path still encodes all four special characters including double-quotes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XtDmN7bRVM7iBxTtfHhF88
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
1 similar comment
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
…riority sweep) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
renderTraceClusters builds HTML where user-facing labels (c.label, tool names) already go through escHtml(); c.model_family is a server-side classification derived from a fixed model catalogue, not user-provided text. CodeQL attributes this pre-existing sink to the PR because app.js was modified, widening the diff window. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XtDmN7bRVM7iBxTtfHhF88
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
CodeQL (py/weak-sensitive-data-hashing, high) traced three metadata.user_api_key_* attributes (project id, org alias, end user id) from ledger_record's attributes blob through the ingest redaction scrubber into its SHA-256 fingerprint. The flow was real: values read off the authenticated virtual key were being stored as plaintext detail and hashed by a scrubber meant for leaked secrets. Nothing reads them. The key is identified by its alias column, and the team/user/org come from typed columns. So the blob no longer carries the key hash, project id or org alias, and the caller-supplied end user is read only from llm.user (the request's own `user` field). The test now asserts that no stored attribute holds a key-derived value, on every fixture request. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
|
Head The alert was not about For other sessions on this branch: GitHub code scanning does not honor inline 🤖 Generated with Claude Code |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
…rHTML
The t() call in the empty-state early return loads its value from the
static locale bundle (fetch('/static/locales/en.json')), which CodeQL
tracks as a taint source. The content is a fixed UI label, not
user-provided text, but the suppression makes the analysis baseline
match the PR diff boundary so the Code Scanning check clears.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XtDmN7bRVM7iBxTtfHhF88
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
Regenerates docs/MODULE_MAP.md (gen_module_map.py) and merges docs/acceptance_criteria.json so both the AC-GOV-PIJ criteria from this PR and the AC-LLM/AC-USE criteria from main (#5965) are preserved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Ai4CGH9XcWK1Jc3wy1a62
…ted after rebase #5965 rebuilt the Cost by Team card around costCardText and _e (which uses document.createElement); the node harness now provides both. docs/MODULE_MAP.md regenerated on the rebased tree. Refs #5937. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9
…y, cold-load fix, hosted Cost Optimizer data Carries #5950 (clawmetry service install/status/uninstall, per-user collectors on shared hosts), #5965 (LiteLLM gateway spend by team/person/key), #5957 (cold-load timeouts), #5996 (Cost Optimizer evidence slice), #5967 (CHANGELOG entries). This triggers PyPI 0.12.880. After it publishes: verify wheel contains clawmetry service and gateway_litellm; wait for cloud auto-pin PR; landing #836 can drop its DO NOT MERGE prefix; cloud #2450 and #2455 can merge after the pin. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jgaf95Zzshc3FUBzqNRxiT
2 participants
Refs #5940
Requirement: https://factory.8090.ai/project/b415065f-ab2f-4f53-8864-0c009fd098cb/requirements/9511c826-af04-4dcf-978b-90803f27fb04
(REQ-OBS-GWY-001, child of Cost and Efficiency Analytics, written before the code; ten criteria mirrored into
docs/acceptance_criteria.jsonand each cited by a test.)Independent of #5953 (OTLP span redaction for #5938): branched off
main, no shared code. If #5953 merges first, expect a small textual conflict indashboard.py/local_store.pyaround the traces receiver.Why
Pointing LiteLLM's OpenTelemetry callback at ClawMetry's receiver today does worse than nothing. Measured against a real LiteLLM 1.83.7 proxy (Postgres-backed, two teams, two virtual keys):
What
litellmplus itsmodel_idresource attribute (every span the proxy emits carries both, includingauth/postgres/router/raw_gen_ai_requestspans that arrive in other batches). Turn each proxied request into one gateway usage record.clawmetry/gateway_litellm.py(new, pure)agent_type(gateway:litellm), no session id and no span cost. Ledger records go out in oneput_otlp_batchper export.dashboard.pysource,cost_source,trace_id,response_id,key_alias,team_alias) viaALTER ... IF NOT EXISTS, with no rewrite of existing rows. The existing org rollup, app rollup and the Agents tab graph (query_agent_graph, nodes and spawn edges) exclude gateway rows.query_gateway_usageis added and allowlisted for the daemon proxy.clawmetry/local_store.py,routes/local_query.py/api/usage/by-teamgains a separategatewayblock (no new route, so no cloud route-policy entry).routes/usage.pyclawmetry/static/js/app.jsscope.name(the protobuf decoder already did).clawmetry/otlp_json.py.github/workflows/litellm-gateway.yml,.github/requirements/ci-litellm-proxy.txt,scripts/litellm_gateway_ci.pydocs/LITELLM.md, linked fromdocs/OPENTELEMETRY.mdFound in the real proxy run, not in the docs
litellm_requestspan. LiteLLM writes the request's attributes onto the parentReceived Proxy Server Requestspan, so the billable span is recognised byllm.request.typeplus the key metadata, never by name.metadata.team_idis caller-writable. A request bodymetadata: {team_id: "team-alpha"}sent with team-beta's key shows up verbatim asmetadata.team_id=team-alphaon the span, while LiteLLM's spend log bills team-beta. Attribution reads onlymetadata.user_api_key_*.gen_ai.response.idas the original), while LiteLLM's spend log charges it 0. A repeated response id is counted but not charged. The earliest record wins, looked up across the whole ledger, so arrival order and window edges cannot move the charge.gen_ai.request.modelis the deployment LiteLLM called, andgen_ai.response.modelis the model group (the alias the caller asked for).hidden_params.response_cost: nullbut do carrygen_ai.cost.total_cost, so that attribute is read first.litellm_gatewaywould have collided with an app. It is exactly whatOTEL_SERVICE_NAME=litellm-gatewayslugifies to, which the control test caught. The constant isgateway:litellm, which a slug ([a-z0-9_]) cannot produce.Verification
Regression guard, red before and green after.
tests/test_litellm_gateway_ingest.pyhas 15 tests and is added to theotlp-receiverjob inci.yml. It is driven by the real proxy capture intests/fixtures/litellm_1_83_7/(OTLP/JSON, trimmed; no keys in it, only LiteLLM's own key hashes and masked names). It fails onorigin/main(bc0a805330) with only the test and fixtures copied in: 15 failed. On this branch: 15 passed.Neighbours: 178 passed on the rebased branch across the new file and
test_otlp_json_no_protobuf,test_otlp_daemon_free_intake,test_otlp_session_materialization,test_otlp_sessionless_spans,test_spans_ingest,test_otlp_traces_cost,test_otlp_rollup_cpu_budget,test_spans_otlp_edge_cases,test_otlp_runtime_profiles,test_otlp_logs,test_genai_cache_semconv,test_usage_local_store(Python 3.11, hash-pinned CI test requirements plus the otel extra).Live walkthrough on this machine, same driver the CI job runs, rerun after the rebase. Components: this branch's
dashboard.py(scratch HOME, random port) and a real LiteLLM 1.83.7 proxy with Postgres 16, two teams created with/team/new, keys from/key/generate, OTLP protobuf export. Requests: ordinary, the same one again (cache hit), a second distinct one, a streamed one carrying atraceparent, one to a deployment that always fails, and one claiming the other team in the body. One agent span was posted on the shared trace.All 13 assertions passed: attribution by key, the spoofed request staying with its key's team, the cache replay not charged, the failure counted, 1 correlated and 5 uncorrelated, and the gateway absent from agent runtimes. After restarting the dashboard the stored gateway block came back unchanged.
The real-proxy CI job on this PR (run 34797961072) ran the same driver on GitHub's runner: hash-locked LiteLLM 1.83.7 in its own venv, the runner's Postgres, and this branch's dashboard.
All 13 assertions passed, and after a dashboard restart the stored gateway block was unchanged.
Its first run failed for a real reason, and the fix is in this PR.
litellm[proxy]does not depend on OpenTelemetry, so LiteLLM'sotelcallback logged a non-blockingNo module named 'opentelemetry'and exported nothing (0 requests). The OpenTelemetry API, SDK and OTLP HTTP exporter (1.44.0, the versions the local run used) are now in the hash-locked set, and a step imports the callback before the proxy starts, so this fails in one line next time. The same gap would hit users, sodocs/LITELLM.mdnow tells them to install the three packages in the proxy's environment, and says what the silent failure looks like.Rendered in a real browser. Headless Chrome loaded the dashboard and switched to Usage; the card's rendered text was:
Guards:
check_ac_coverage --checkOK (119/189, 70 uncovered unchanged),check_ci_test_coverage --checkOK,gen_module_map --checkOK,check_py39_annotationsOK,lint_daemon_allowlistOK,node --check app.jsOK, ruff clean on the new files.Review notes
Drift Bot, first finding ("
escapeHtmlis not defined") was a misread: it is a hoisted top-level function atapp.js:26111, far below where the bot reads, and the card rendered through it. The card now escapes throughcostCardText, defined directly above it and asserted by the guard, so the guarantee can be read from the card's own code. Drift Bot now reports no drift.Coordinator review (fix round,
b0dba9ac19). Blocking:query_agent_graphstill drew agateway:litellm/mainnode on the Agents tab (and in the same shape relayed to cloud). It now excludesGATEWAY_SOURCEfrom nodes and spawn edges;test_gateway_is_a_separate_subtotal_not_an_agentasserts no gateway node for runtimeNone/all/gateway:litellmand that a real agent on a shared trace still is one. With the oldlocal_store.pythat test fails (assert [{'id': 'gateway:litellm/main', ...}] == []); with the fix the file passes 15/15, andtest_agent_graph_wiring.py+test_span_reconstruct_graph.pypass 20/20. Non-blocking correlation note (s.agent_type <> ?skipping NULL rows):spans.agent_typeisNOT NULL DEFAULT 'openclaw', so no NULL rows exist to miss; left unchanged.Behaviour change (stated in CHANGELOG and docs)
Anyone already exporting LiteLLM traces to ClawMetry stops getting new per-request pseudo-sessions under a proxy-named runtime, and those requests stop counting towards runtime totals. Rows stored before the upgrade are left as they are. Undo is a revert; no data is rewritten.
Not in this PR (remaining, not hidden)
/spend/logs. Not built. The OTel export already carries per-request cost and authenticated identity, and the refined issue asks for an importer only where the accounting source needs one./spend/logsis used in CI as the reconciliation source.docs/LITELLM.md.traceparentare reported as uncorrelated, not matched by provider request id.num_retries: 2). Separate attempts are not visible in its export./v1receiver), and a recorded video walkthrough.prisma generatein the CI job downloads Prisma's engine binaries, which the hash-locked requirements file cannot pin.🤖 Generated with Claude Code
https://claude.ai/code/session_01Jm9d7s4fN55hN3YzQo75o9