feat: ✨ Cost emission on event stream - #907
Conversation
Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com>
Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe LiteLLM budget tracking plugin emits a ChangesLiteLLM cost event emission
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Priced LiteLLM responses now expose per-response cost, pricing source, daily total, and configured cap to session-event consumers. The supported pricing and no-cost paths are covered, with no concrete merge-blocking risk identified. Sequence Diagram(s)sequenceDiagram
participant ResponseHandler
participant BudgetLedger
participant SessionExtensions
participant SessionEvent
ResponseHandler->>BudgetLedger: Accumulate response cost
BudgetLedger-->>ResponseHandler: Return daily total and added status
ResponseHandler->>SessionExtensions: Store costEvent
SessionExtensions->>SessionEvent: Promote plugin event
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
huang195
left a comment
There was a problem hiding this comment.
Clean, well-scoped change. Surfacing the already-computed cost is the right move, and making provenance explicit (gateway-header vs usage-fallback) is genuinely new information for consumers — a display can now distinguish an authoritative gateway figure from a token-derived estimate rather than presenting both as equally trustworthy.
What I verified:
- Convention: the
pctx.Extensions.Custom[p.Name()+pipeline.PluginEventSuffix]keying matchestoolprune's pattern exactly, and a struct value marshals cleanly throughSnapshotPlugins. - Correctness of
source: traced all four paths.sourceis only reassigned inside the!present || isEventStream(pctx)fallback branch, so a header-priced response keepsgateway-headerand a present-0non-streamed response (genuine free call) still emits nothing. The doc's claim that unpriced responses produce no event holds on every path. - Concurrency: no new race.
accumulatecaptures the post-add total inside the existing mutex,p.cfgis read-only afterConfigure, andTestConcurrentOnResponsegives each goroutine its ownpipeline.Context— so the new unsynchronizedExtensions.Customwrite isn't shared. CI runsgo test -raceand is green. - Docs: the
plugin-reference.md#emitting-session-eventsanchor resolves.
The added bool return threading emission through the same chokepoint that guards the ledger is a nice touch — it means the event stream and the ledger can't disagree about whether a call was charged.
One suggestion worth considering (below) about the 429 deny path emitting nothing; the other two comments are minor.
Author: evaline-ju (MEMBER — maintainer)
Areas reviewed: Go (authlib plugin + tests), Docs
Agent/IDE config (.claude/.vscode): none
Commits: 2 commits, all signed-off: yes
CI status: passing (23 checks pass, Spellcheck skipped)
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
|
|
||
| // emitCost writes the costEvent to pctx.Extensions.Custom; the listener | ||
| // forwards it to SessionEvent.Plugins under the plugin name. | ||
| func (p *BudgetTrack) emitCost(pctx *pipeline.Context, cost float64, source string, dailyTotal float64) { |
There was a problem hiding this comment.
suggestion — This plugin calls pctx.Record(pipeline.Invocation{...}) nowhere, unlike every other gate plugin (ibac, jwtvalidation, cpex all record explicitly). Two consequences:
litellm-budget-tracknever appears in the per-plugin invocation timeline, so abctl shows the cost payload underPluginsbut no row saying this plugin ran.- More pointedly: the
OnRequest429budget.exceededdeny path emits neither an Invocation nor a cost event. Per the session-API contract, rejected requests only land asphase: "denied"events when at least one plugin appended an Invocation before rejecting — so the single moment an operator most wants the spend number ("why was I cut off, and at what total?") is the one moment nothing is published.
Both are pre-existing and not introduced by this PR, but (2) is directly adjacent to its stated goal of making per-response USD visible to SessionEvent.Plugins consumers. A pctx.Record(pipeline.Invocation{Action: pipeline.ActionDeny, Reason: "budget.exceeded"}) alongside the DenyStatus — and an ActionObserve record here in emitCost — would close it. Reasonable as a follow-up if you'd rather keep this PR to the success path.
Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com>
Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com>
Summary
The litellm-budget-track plugin already computes each response's cost but keeps it inside the plugin. This change surfaces the cost value on the session-event stream and how it was derived (gateway header vs. token-fallback), the post-add daily total, and the configured cap.
Enables:
/v1/usagePricer can be wired to this streamSessionEvent.Plugins(abctl, external observability, future summary endpoints) sees per-response USDAssisted-By: Claude (Anthropic AI) noreply@anthropic.com
Related issue(s)
(Optional) Testing Instructions
Summary by CodeRabbit
New Features
Documentation