docs(api-admin): clarify budget enforcement is SaaS-only - #225
Merged
Conversation
The §4.4 prose previously said "the proxy reads the budget at request start and adds the cost at end of request" — true neither in standalone mode (proxy doesn't read the field at all) nor in SaaS mode (proxy doesn't compute cost; cp-api accumulates from telemetry). Architectural review confirmed budget is a SaaS-tier feature: cp-api is the single source of truth, the DP delegates per-request decisions via `/dp/budget_check` and pushes usage via `/dp/telemetry`. Rewrite §4.4 to: - State explicitly that DP never reads `max_budget_usd` from the etcd snapshot; the field is informational on the DP side. - Explain the SaaS pull (pre-check) + push (telemetry) flows with exact cache TTL, batch size, and worst-case ≈ 10 s propagation. - Document the cp-api unreachable fall-back (sticky cache up to `AISIX_DP_BUDGET_STALE_MAX_SECONDS`, then `fail_mode`). - State plainly that standalone mode does not enforce — admin POST accepts the field for SaaS wire compatibility (CP writes ApiKey rows including this field directly into etcd) but it's a no-op on the DP side. Closes #189 (wontfix — by design, not a gap). Closes #190 (depends on #189; field is unenforced DP-side, so admin lower-bound tightening has no practical effect).
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdmin API docs expand Budgets: ChangesBudget Enforcement Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
There was a problem hiding this comment.
Pull request overview
Updates the admin API documentation (§4.4 “Budgets”) to reflect that budget enforcement in the data-plane proxy is managed via cp-api in SaaS/Managed deployments and is not implemented in standalone deployments.
Changes:
- Rewrites §4.4 to describe the managed-mode budget pre-check (
/dp/budget_check) and usage push (/dp/telemetry) flows, including timing expectations and fallback behavior. - Clarifies that standalone mode does not enforce
max_budget_usd, and that the field remains for wire compatibility with managed mode.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+192
to
+193
| with a 5 s LRU cache per api_key. `allow=false` becomes a 429 | ||
| `BudgetExceeded` before upstream dispatch. |
| decision sticks for up to `AISIX_DP_BUDGET_STALE_MAX_SECONDS` | ||
| (default 600 s); past that the proxy applies the `fail_mode` | ||
| (`open` / `closed` / `sticky`) the most recent successful response | ||
| carried. |
6 tasks
PR #225 audit (general-purpose agent) flagged that the "Usage push" bullet overgeneralized — only /v1/chat/completions and /v1/messages emit UsageEvents today; embeddings, audio, images, responses, and rerank don't (per the comment at chat.rs:977-980). The previous prose implied complete coverage, which would mislead anyone reading §4.4 to plan SaaS budgets. Fixes: - §4.4 "Usage push" — pin the two emitting endpoints; explicitly list the five that don't and reference the chat.rs comment; link the carve-out to the new tracking issue #226. - §4.2 — add a one-line caveat under the apikey example so a reader landing on §4.2 alone sees that max_budget_usd is SaaS-only and points at §4.4 for the propagation model.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The §4.4 `Budgets` prose previously said "the proxy reads the
budget at request start and adds the cost at end of request" —
true in neither mode:
etcd snapshot at all (`grep -rn 'max_budget' crates/aisix-proxy/`
→ 0 non-test matches).
spend; cp-api owns both. The DP only calls cp-api
(`/dp/budget_check`) and pushes usage to it
(`/dp/telemetry`) — and only for chat-completion and Anthropic
messages traffic.
Architectural review confirmed budget is a SaaS-tier feature: cp-api
is the single source of truth, multiple DP instances stay
consistent without DP-side coordination, and the DP avoids
duplicating cp-api's pricing/accumulation logic.
This PR rewrites §4.4 to describe what actually happens in each
mode, with concrete file paths, cache TTLs, batch sizes, and
worst-case propagation latency. No code changes.
Changes
(`/dp/budget_check`, 5 s LRU cache) + push (`/dp/telemetry`,
5 s / 100-event batches) flows; pin worst-case ≈ 10 s
propagation; document the cp-api-unreachable fall-back ladder
(`AISIX_DP_BUDGET_STALE_MAX_SECONDS` → `fail_mode`).
UsageEvents today (`/v1/chat/completions`, `/v1/messages`)
and explicitly list the five that don't; cross-reference Non-chat endpoints don't emit UsageEvents — cp-api's spend ledger misses spend from /v1/responses, /v1/embeddings, /v1/audio*, /v1/images*, /v1/rerank #226.
implemented; explain why the admin schema still accepts the
field (SaaS CP writes ApiKey rows including this field directly
into etcd, so the wire shape must stay compatible).
landing on §4.2 alone sees max_budget_usd is SaaS-only and is
pointed at §4.4.
Audit history
PR went through one round of independent audit (general-purpose
agent, no shared context). Findings:
overgeneralized; only chat completions + Anthropic messages
emit. Fixed in d1b4379 by pinning the two emitting
endpoints and explicitly listing the five that don't.
caveat. Fixed in d1b4379 by adding a cross-reference note.
within the 5 s cache window". Folded into the prose during the
fix.
Audit also surfaced a real product gap (not introduced by this
PR): non-chat endpoints don't push usage to cp-api, so SaaS
budgets undercount spend on embeddings/audio/images/responses/
rerank traffic. Filed as #226 (HIGH for SaaS budget
correctness and billing accuracy) and linked from §4.4.
Closes
spend locally (avoids cross-DP coordination, avoids duplicating
cp-api's pricing logic).
no practical effect while the field is unenforced DP-side.
Test plan
`crates/aisix-server/src/telemetry.rs`, and `crates/aisix-proxy/src/chat.rs:977-980`
to confirm cache TTL, batch size, fail-mode, and emitting-endpoints
match the code.
Summary by CodeRabbit