feat:Implement Rate Limiting with Per-User/Per-Issuer Buckets and Met… - #35
Merged
Conversation
Contributor
|
@SheyeJDev |
…er-Issuer-Buckets-and-Metrics
Contributor
Author
|
Alrght On it |
Contributor
Author
|
Build issues resolved T for thanks |
MaryammAli
approved these changes
Jun 27, 2026
MaryammAli
left a comment
Contributor
There was a problem hiding this comment.
LGTM
thanks for ur contribution @SheyeJDev
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.
feat(rate-limit): two-tier per-issuer rate limiting with Prometheus observability
Problem
The existing rate limiter (
MetricsRateLimiterinsrc/rate_limit.rs) is a single, unkeyed token bucket shared across every caller. This means:Retry-Aftersemantics. Callers that are rejected get an opaquegovernor::NotUntilguard with no standardised way to surface the wait time in an HTTP response.This PR addresses all four gaps.
Solution overview
Replace the single global limiter with a two-tier architecture:
The global tier preserves the existing worst-case throughput cap. The per-issuer tier adds fairness. Both must pass before a request proceeds.
Files changed
src/rate_limit.rs— complete rewriteNew public surface:
Key design decisions:
Global tier is checked before per-issuer. If the global bucket is saturated the per-issuer
check_keycall is skipped entirely, so per-issuerremainingcounters are not decremented by globally-blocked traffic. This keepsstatus()readings accurate.DashMapfor zero-lock-contention concurrent access. Governor'sRateLimiter::keyed()uses aDefaultKeyedStateStore<String>(internally aDashMap) — one sharded cell per issuer address. A separateDashMap<String, IssuerEntry>tracks last-seen timestamps and approximate remaining-token counts for thestatus()API without adding aMutex.RateLimitErroris a proper algebraic type. Downstream HTTP middleware can match on the variant, extractretry_after_secs(), and construct a standards-compliantRetry-Afterresponse header without any timestamp arithmetic of its own.evict_stale()bounds memory in long-running deployments. Issuer entries whoselast_seenis older thanissuer_ttl_secondsare removed. This should be called by a periodic tokio task (e.g. every 10 minutes). Without eviction, a deployment that sees millions of unique issuer addresses would leak memory indefinitely.Legacy API retained for backward compatibility.
build_rate_limiter()and theDefaultRateLimitertype alias are kept intact. TheMetricsRateLimiterwrapper remains but is superseded; it can be removed in a follow-up once all call-sites have migrated.How to wire into an Axum middleware (follow-up PR):
How to surface the status endpoint (follow-up PR):
src/metrics.rs— new per-issuer counter familyTwo new
IntCounterVecmetrics are introduced alongside the existing counters:Dependency change
governorexposesRateLimiter::keyed()andDefaultKeyedStateStorebehind itsdashmapCargo feature on some published versions. Verify against the pinned version inCargo.lock:Distributed consistency (follow-up)
Per-issuer state is currently in-process only (
DashMap). In a single-replica deployment this is sufficient. For multi-replica (horizontal scale-out), per-issuer counts will drift between replicas and a single issuer could effectively multiply their quota by the replica count.The fix is to back the keyed state store with Redis using the existing cache layer. The
PerIssuerRateLimiterstruct is designed for this: swapDefaultKeyedStateStore<String>for a customRedisKeyedStateStorethat implements thegovernor::state::StateStoretrait. The public API (check,until_ready,status) remains unchanged. Tracked as a follow-up issue.Checklist
PerIssuerRateLimiter) — global + per-issuerRateLimitErrorwithretry_after_secs()andreason()for HTTP 429status(issuer)→RateLimitStatus(remaining quota, reset timestamp)evict_stale()for bounded memory in long-running deploymentsrate_limit_hits_total{issuer}andrate_limit_rejections_total{issuer,tier}metricsAppConfig::Debugextended (no new secrets, no redaction needed)build_rate_limiter,rate_limit_tokens_consumed_total,rate_limit_violations_totalretainedrate_limit.rs; 3 new/updated inmetrics.rs; 3 new/updated inconfig.rsdashmap = "5"added toCargo.tomlGET /v1/rate-limit/:issuerstatus endpoint (follow-up)StateStorefor distributed consistency (follow-up)Closes #12