-
-
Notifications
You must be signed in to change notification settings - Fork 432
feat: add per-user API rate limiting (Phase 1 — tracking only) #932
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
AchoArnold
wants to merge
16
commits into
main
Choose a base branch
from
feat/api-rate-limiting
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
97b1508
docs: add API rate limiting Phase 1 design spec
AchoArnold 672d3d9
docs: add API rate limiting implementation plan
AchoArnold d282440
feat(entities): add RateLimit() method to SubscriptionName
AchoArnold 0d671c2
feat(events): add rate.limit.exceeded CloudEvent type
AchoArnold 65ca2dd
feat(services): implement RateLimitService with in-memory counters an…
AchoArnold 5cc7db3
feat(middlewares): add rate limit middleware for tracking API usage
AchoArnold e2a75e6
feat(di): wire rate limit service and middleware into DI container
AchoArnold 04ef44f
fix: add graceful shutdown for RateLimitService and handle Redis noti…
AchoArnold d2ece4f
feat(services): add 1s Redis timeout, fail-open behavior, and shutdow…
AchoArnold df3368f
refactor: use Fiber OnPreShutdown hook for RateLimitService cleanup
AchoArnold f15306e
refactor: remove nil checks for logger in RateLimitService, use OnPre…
AchoArnold 3f8b8d1
refactor: gate rate limiting at container level, skip middleware enti…
AchoArnold f35d4cc
refactor: rename rate limiter service
AchoArnold 6b388e5
perf: batch all Redis flush operations into a single pipeline call
AchoArnold 214163e
refactor(api): use ctxLogger with span context in RateLimitService
AchoArnold 736697c
refactor: improve logging
AchoArnold File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package events | ||
|
|
||
| import ( | ||
| "time" | ||
|
|
||
| "github.com/NdoleStudio/httpsms/pkg/entities" | ||
| ) | ||
|
|
||
| // RateLimitExceeded is raised when a user exceeds their daily API rate limit. | ||
| const RateLimitExceeded = "rate.limit.exceeded" | ||
|
|
||
| // RateLimitExceededPayload stores the data for the RateLimitExceeded event | ||
| type RateLimitExceededPayload struct { | ||
| UserID entities.UserID `json:"user_id"` | ||
| Count int64 `json:"count"` | ||
| Limit uint `json:"limit"` | ||
| Plan string `json:"plan"` | ||
| Timestamp time.Time `json:"timestamp"` | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| package middlewares | ||
|
|
||
| import ( | ||
| "strconv" | ||
| "strings" | ||
|
|
||
| "github.com/NdoleStudio/httpsms/pkg/entities" | ||
| "github.com/NdoleStudio/httpsms/pkg/repositories" | ||
| "github.com/NdoleStudio/httpsms/pkg/services" | ||
| "github.com/NdoleStudio/httpsms/pkg/telemetry" | ||
| "github.com/gofiber/fiber/v3" | ||
| ) | ||
|
|
||
| const rateLimitCostCap = 100 | ||
|
|
||
| // RateLimit tracks per-user API request counts without blocking requests. | ||
| func RateLimit( | ||
| tracer telemetry.Tracer, | ||
| logger telemetry.Logger, | ||
| service *services.RateLimitService, | ||
| userRepository repositories.UserRepository, | ||
| excludePaths []string, | ||
| ) fiber.Handler { | ||
| logger = logger.WithService("middlewares.RateLimit") | ||
|
|
||
| return func(c fiber.Ctx) error { | ||
| path := c.Path() | ||
| for _, excluded := range excludePaths { | ||
| if strings.HasPrefix(path, excluded) { | ||
| return c.Next() | ||
| } | ||
| } | ||
|
|
||
| ctx, span := tracer.StartFromFiberCtx(c, "middlewares.RateLimit") | ||
| defer span.End() | ||
|
|
||
| authUser, ok := c.Locals(ContextKeyAuthUserID).(entities.AuthContext) | ||
| if !ok || authUser.IsNoop() { | ||
| return c.Next() | ||
| } | ||
|
|
||
| cost := 1 | ||
| if c.Method() == fiber.MethodGet { | ||
| if limitParam := c.Query("limit"); limitParam != "" { | ||
| if parsed, err := strconv.Atoi(limitParam); err == nil && parsed > 0 { | ||
| cost = min(parsed, rateLimitCostCap) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| user, err := userRepository.Load(ctx, authUser.ID) | ||
| if err != nil { | ||
| ctxLogger := tracer.CtxLogger(logger, span) | ||
| ctxLogger.Error(err) | ||
| return c.Next() | ||
| } | ||
|
|
||
| _, _, _ = service.Increment(ctx, authUser.ID, user.SubscriptionName, cost) | ||
|
|
||
| return c.Next() | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
userRepository.Loadis called on every request solely to retrieveuser.SubscriptionName. With ~20M requests/month this adds an equivalent number of DB queries — one per API call — just for rate-limit plan lookups. The auth middlewares that run before this one (BearerAuth,APIKeyAuth) already load and validate the user; if theAuthContextstored inc.Localsalready carries the subscription plan (or if there is a short-lived cache in the repository), this load could be avoided entirely. IfAuthContextdoesn't include the plan today, it would be worth adding it there rather than performing a separate load per request.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!