feat(a2asrv): add CostLimiter for budget-aware execution control - #378
feat(a2asrv): add CostLimiter for budget-aware execution control#378kuangmi-bit wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the costlimiter package, which implements cost-based budget enforcement for A2A server request handling using a pluggable interceptor and budget store. The review feedback highlights several critical improvements: ensuring budget release operations do not fail on cancelled contexts by using context.WithoutCancel, implementing the documented fallback to limiter.AttachScope when no custom scope function is provided, and removing a redundant Available check in Before to prevent race conditions and double round-trips. Additionally, the feedback points out a bug in InMemoryCostStore.Reserve where storing negative costs for unlimited scopes can incorrectly restrict budgets, and suggests adding corresponding unit tests.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if resp != nil && resp.Err != nil && errors.Is(resp.Err, context.Canceled) { | ||
| _ = cl.store.Release(ctx, res.scope, res.cost) | ||
| } |
There was a problem hiding this comment.
When a request is cancelled, the context ctx is already cancelled. If cl.store.Release performs any network or database I/O (e.g., in a distributed store implementation), passing the cancelled ctx will cause the release call to fail immediately, preventing the budget from being restored. Use context.WithoutCancel(ctx) to ensure the cleanup/release operation can complete successfully while preserving trace and logging context.
| if resp != nil && resp.Err != nil && errors.Is(resp.Err, context.Canceled) { | |
| _ = cl.store.Release(ctx, res.scope, res.cost) | |
| } | |
| if resp != nil && resp.Err != nil && errors.Is(resp.Err, context.Canceled) { | |
| _ = cl.store.Release(context.WithoutCancel(ctx), res.scope, res.cost) | |
| } |
There was a problem hiding this comment.
Done. After() now uses context.WithoutCancel(ctx) for the Release call when the request was cancelled.
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "sync" | ||
|
|
||
| "github.com/a2aproject/a2a-go/v2/a2asrv" | ||
| ) |
There was a problem hiding this comment.
Import the limiter package to support retrieving the scope attached to the context via limiter.AttachScope.
| import ( | |
| "context" | |
| "errors" | |
| "fmt" | |
| "sync" | |
| "github.com/a2aproject/a2a-go/v2/a2asrv" | |
| ) | |
| import ( | |
| "context" | |
| "errors" | |
| "fmt" | |
| "sync" | |
| "github.com/a2aproject/a2a-go/v2/a2asrv" | |
| "github.com/a2aproject/a2a-go/v2/a2asrv/limiter" | |
| ) |
There was a problem hiding this comment.
Done. limiter package is now imported.
| func NewCostLimiter(store CostStore, costFn CostFunc, opts ...Option) *CostLimiter { | ||
| cl := &CostLimiter{ | ||
| store: store, | ||
| costFn: costFn, | ||
| scopeFn: func(_ *a2asrv.CallContext) string { | ||
| return "default" | ||
| }, | ||
| } | ||
| for _, opt := range opts { | ||
| opt(cl) | ||
| } | ||
| return cl | ||
| } |
There was a problem hiding this comment.
Do not initialize scopeFn to a default function in NewCostLimiter. Leaving it as nil allows us to detect if a custom scope function was provided, and fall back to the scope attached to the context via limiter.AttachScope as documented.
func NewCostLimiter(store CostStore, costFn CostFunc, opts ...Option) *CostLimiter {
cl := &CostLimiter{
store: store,
costFn: costFn,
}
for _, opt := range opts {
opt(cl)
}
return cl
}There was a problem hiding this comment.
Done. scopeFn is left nil in NewCostLimiter; fallback is handled in resolveScope().
| func (cl *CostLimiter) Before(ctx context.Context, callCtx *a2asrv.CallContext, req *a2asrv.Request) (context.Context, any, error) { | ||
| scope := cl.scopeFn(callCtx) | ||
| cost := cl.costFn(ctx, callCtx, req) | ||
| if cost <= 0 { | ||
| return ctx, nil, nil | ||
| } | ||
|
|
||
| available, err := cl.store.Available(ctx, scope) | ||
| if err != nil { | ||
| return ctx, nil, fmt.Errorf("costlimiter: checking budget: %w", err) | ||
| } | ||
| if available >= 0 && available < cost { | ||
| return ctx, nil, fmt.Errorf("%w: scope %q: need %d, available %d", | ||
| ErrBudgetExceeded, scope, cost, available) | ||
| } | ||
|
|
||
| ok, err := cl.store.Reserve(ctx, scope, cost) | ||
| if err != nil { | ||
| return ctx, nil, fmt.Errorf("costlimiter: reserving budget: %w", err) | ||
| } | ||
| if !ok { | ||
| return ctx, nil, fmt.Errorf("%w: scope %q: budget exhausted", ErrBudgetExceeded, scope) | ||
| } | ||
|
|
||
| return context.WithValue(ctx, reservationKeyType{}, reservation{scope: scope, cost: cost}), nil, nil | ||
| } |
There was a problem hiding this comment.
This refactoring addresses two key issues:
- Fallback to
limiter.AttachScope: Implements the documented fallback behavior whenscopeFnis not provided, by checkinglimiter.ScopeFrom(ctx)before defaulting to"default". - Performance Optimization: Removes the redundant
Availablecheck before callingReserve. SinceReserveatomically checks and deducts the budget, callingAvailablebeforehand introduces an unnecessary double round-trip to the store (which is a major performance bottleneck for distributed stores like Redis or SQL) and creates a check-then-act race condition.
func (cl *CostLimiter) Before(ctx context.Context, callCtx *a2asrv.CallContext, req *a2asrv.Request) (context.Context, any, error) {
var scope string
if cl.scopeFn != nil {
scope = cl.scopeFn(callCtx)
} else if sc, ok := limiter.ScopeFrom(ctx); ok {
scope = sc
} else {
scope = "default"
}
cost := cl.costFn(ctx, callCtx, req)
if cost <= 0 {
return ctx, nil, nil
}
ok, err := cl.store.Reserve(ctx, scope, cost)
if err != nil {
return ctx, nil, fmt.Errorf("costlimiter: reserving budget: %w", err)
}
if !ok {
return ctx, nil, fmt.Errorf("%w: scope %q: budget exhausted", ErrBudgetExceeded, scope)
}
return context.WithValue(ctx, reservationKeyType{}, reservation{scope: scope, cost: cost}), nil, nil
}There was a problem hiding this comment.
Done. Added resolveScope() with fallback to limiter.AttachScope. Removed the redundant Available() check — Reserve() now called directly.
| if !ok { | ||
| s.budget[scope] = -cost | ||
| return true, nil | ||
| } |
There was a problem hiding this comment.
If a scope is not present in the map, it defaults to unlimited. Storing -cost in the map is problematic:
- If
costis0, it stores0, which turns the unlimited scope into a limited scope with0budget (exhausted) on subsequent calls. - It causes unnecessary map growth and memory leaks for unlimited scopes.
Since unlimited scopes do not need to track or deduct budget, we should simply return true, nil without modifying the map.
if !ok {
return true, nil
}There was a problem hiding this comment.
Done. Reserve() returns true, nil for unlimited scopes without storing in the map. Added TestInMemoryCostStore_UnlimitedScopeNotStored to verify.
| import ( | ||
| "context" | ||
| "errors" | ||
| "testing" | ||
|
|
||
| "github.com/a2aproject/a2a-go/v2/a2asrv" | ||
| ) |
There was a problem hiding this comment.
Import the limiter package to support testing the fallback to limiter.AttachScope.
| import ( | |
| "context" | |
| "errors" | |
| "testing" | |
| "github.com/a2aproject/a2a-go/v2/a2asrv" | |
| ) | |
| import ( | |
| "context" | |
| "errors" | |
| "testing" | |
| "github.com/a2aproject/a2a-go/v2/a2asrv" | |
| "github.com/a2aproject/a2a-go/v2/a2asrv/limiter" | |
| ) |
There was a problem hiding this comment.
Done. limiter package imported in test file.
| if !errors.Is(err, ErrBudgetExceeded) { | ||
| t.Fatalf("agent-a should be out of budget, got: %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Add a unit test to verify that the CostLimiter correctly falls back to the scope attached to the context via limiter.AttachScope when no custom ScopeFunc is provided.
if !errors.Is(err, ErrBudgetExceeded) {
t.Fatalf("agent-a should be out of budget, got: %v", err)
}
}
func TestCostLimiter_FallbackToLimiterScope(t *testing.T) {
store := NewInMemoryCostStore(map[string]int64{"custom-scope": 10})
cl := NewCostLimiter(store, func(ctx context.Context, callCtx *a2asrv.CallContext, req *a2asrv.Request) int64 {
return 10
})
ctx := context.Background()
ctx = limiter.AttachScope(ctx, "custom-scope")
ctx, callCtx = a2asrv.NewCallContext(ctx, nil)
ctx, _, err := cl.Before(ctx, callCtx, &a2asrv.Request{})
if err != nil {
t.Fatalf("should have successfully reserved from custom-scope, got: %v", err)
}
// Second call should fail as budget is exhausted
_, _, err = cl.Before(ctx, callCtx, &a2asrv.Request{})
if !errors.Is(err, ErrBudgetExceeded) {
t.Fatalf("expected ErrBudgetExceeded, got: %v", err)
}
}There was a problem hiding this comment.
Done. Added TestCostLimiter_FallbackToLimiterScope verifying the AttachScope fallback.
Add a new a2asrv/costlimiter package that provides cost-based budget enforcement as a CallInterceptor. This complements the existing ConcurrencyConfig (which limits in-flight executions) with cost-aware budgeting — useful for LLM token budgets, compute-time quotas, or monetary credit limits. Key components: - CostLimiter: server-side CallInterceptor with pluggable CostFunc/CostStore - CostStore interface: Available/Reserve/Release with atomic semantics - InMemoryCostStore: thread-safe, unknown scopes treated as unlimited - ScopeFunc: per-agent/per-tenant scoping, falls back to limiter.AttachScope - Cancelled requests release budget via context.WithoutCancel - Removed redundant Available check before Reserve (race condition fix) Addresses review feedback: context.WithoutCancel on release, limiter.AttachScope fallback, InMemoryCostStore negative budget fix. 8 tests pass.
3edd3a0 to
82b30ff
Compare
…ored tests - Import limiter package in test file - Add TestCostLimiter_FallbackToLimiterScope: verifies CostLimiter falls back to limiter.AttachScope when no custom ScopeFunc is provided - Add TestInMemoryCostStore_UnlimitedScopeNotStored: verifies unlimited scopes are not stored in the budget map
Summary
Add a new
a2asrv/costlimiterpackage that provides cost-based budget enforcement as aCallInterceptor.Motivation
The existing
limiterpackage provides concurrency control (MaxExecutions), but there is no built-in mechanism for cost-based quotas. This is useful for:Design
CostLimiter: a server-sideCallInterceptor. Before each request, it estimates the cost via a pluggableCostFuncand reserves budget via aCostStore. If budget is insufficient, it returns an error before the executor runs.CostStore: an interface withAvailable/Reserve/Release. Negative availability = unlimited.InMemoryCostStore: thread-safe in-memory implementation usingsync.Mutex.ScopeFunc: derives a scope (e.g., per-tenant, per-user) fromCallContextfor independent budget tracking.Example
Testing
All 8 tests pass, including:
ScopeFuncNo existing tests affected (verified with
go test ./a2asrv/...).