docs: rate limiting in features, interceptor chain, config, security hardening#68
docs: rate limiting in features, interceptor chain, config, security hardening#68
Conversation
- Replace deprecated Resilience (Hystrix) with Rate Limiting in Index.md - Update architecture.md interceptor chain: add Default Timeout, Rate Limiting, Debug Log (was missing 3 interceptors) - Add RATE_LIMIT_PER_SECOND, RATE_LIMIT_BURST, DISABLE_RATE_LIMIT to config-reference.md - Add rate limiting howto with per-API custom limiter example - Add rate limiting to security hardening env block and checklist - Update production.md: per-pod rate limiting is now built-in
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 27 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughDocumentation introduces a per-pod, in-memory token-bucket gRPC rate limiter (disabled by default), documents env vars for configuration, adds the rate-limiting interceptor to the server interceptor chain, and describes how to plug in a custom/distributed limiter via the exported Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
Updates ColdBrew documentation to reflect the new built-in per-pod rate limiting feature, including how to enable/configure it, how it fits into the interceptor chain, and how it changes production/security guidance.
Changes:
- Replace the deprecated “Resilience” homepage feature entry with “Rate Limiting”.
- Document the full interceptor chain order (now including Default Timeout, Rate Limiting, Debug Log).
- Add rate limiting environment variables and guidance across config reference, interceptor how-to, and production/security hardening docs.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Index.md | Updates the homepage feature table to highlight rate limiting instead of deprecated resilience entry. |
| howto/production.md | Adds recommended rate limiting env vars to the security hardening example and updates “not built-in” guidance + checklist. |
| howto/interceptors.md | Adds a new rate limiting section with enablement instructions and a per-API custom limiter example. |
| config-reference.md | Documents RATE_LIMIT_PER_SECOND, RATE_LIMIT_BURST, and DISABLE_RATE_LIMIT. |
| architecture.md | Corrects/expands interceptor chain table to include all interceptors in order, including rate limiting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…commendations Address review: link Limiter interface to pkg.go.dev, add import and compile-time check to example. Add table of distributed rate limiting libraries (mennanov/limiters, go-redis/redis_rate, sethvargo/go-limiter) and large-scale services (gubernator, Envoy ratelimit).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
howto/interceptors.md (1)
216-221: Consider handling the case when grpc.Method returns false.While the current code works for typical scenarios,
grpc.Method(ctx)returns(method string, ok bool). Whenokis false (e.g., non-gRPC requests), the method will be an empty string. Consider explicitly handling this case for robustness:func (l *perMethodLimiter) Limit(ctx context.Context) error { method, ok := grpc.Method(ctx) if !ok { method = "" // explicit fallback for non-gRPC contexts } limiter, ok := l.limiters[method] if !ok { limiter = l.fallback } if !limiter.Allow() { return fmt.Errorf("rate limit exceeded for %s", method) } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/interceptors.md` around lines 216 - 221, Handle the case where grpc.Method(ctx) returns ok == false inside perMethodLimiter.Limit: call grpc.Method(ctx) capturing both method and ok, and if ok is false set method to a safe default (e.g., empty string) before looking up l.limiters[method]; then proceed to select limiter or l.fallback and use limiter.Allow() as before so non-gRPC contexts do not accidentally use an empty or unexpected key.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@howto/interceptors.md`:
- Around line 197-237: The example uses rate.NewLimiter with plain ints which
mismatches its signature; update the calls in the init block where
interceptors.SetRateLimiter is invoked to cast the first argument to rate.Limit
(e.g., pass rate.Limit(10), rate.Limit(100), rate.Limit(50)) so the map entries
in perMethodLimiter and the fallback use rate.NewLimiter(rate.Limit(...), burst)
matching the required types.
---
Nitpick comments:
In `@howto/interceptors.md`:
- Around line 216-221: Handle the case where grpc.Method(ctx) returns ok ==
false inside perMethodLimiter.Limit: call grpc.Method(ctx) capturing both method
and ok, and if ok is false set method to a safe default (e.g., empty string)
before looking up l.limiters[method]; then proceed to select limiter or
l.fallback and use limiter.Allow() as before so non-gRPC contexts do not
accidentally use an empty or unexpected key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1eb137e7-ee32-4420-836b-df2004463e9a
📒 Files selected for processing (2)
Index.mdhowto/interceptors.md
🚧 Files skipped from review as they are similar to previous changes (1)
- Index.md
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…n burst - Per-API limiter example: fall back to runtime.RPCMethod for HTTP→gRPC calls where grpc.Method returns empty (no transport stream) - Add enable_debug to architecture chain table - Mention RATE_LIMIT_BURST in production checklist
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
howto/interceptors.md (1)
237-246:⚠️ Potential issue | 🔴 CriticalFix
rate.NewLimitertype conversion in code example.The example uses
rate.NewLimiter(10, 5)but the signature requires the first parameter to be typerate.Limit, notint. Cast the values:rate.NewLimiter(rate.Limit(10), 5),rate.NewLimiter(rate.Limit(100), 50), andrate.NewLimiter(rate.Limit(50), 25).Run this script to confirm the signature:
#!/bin/bash # Description: Verify rate.NewLimiter signature in golang.org/x/time/rate # Check the signature in go.dev documentation echo "Checking rate.NewLimiter signature..."What is the signature of rate.NewLimiter in golang.org/x/time/rate package in Go?🔧 Corrected code snippet
func init() { interceptors.SetRateLimiter(&perMethodLimiter{ limiters: map[string]*rate.Limiter{ - "/myservice.v1.UserService/CreateUser": rate.NewLimiter(10, 5), // 10 rps - "/myservice.v1.UserService/ListUsers": rate.NewLimiter(100, 50), // 100 rps + "/myservice.v1.UserService/CreateUser": rate.NewLimiter(rate.Limit(10), 5), // 10 rps + "/myservice.v1.UserService/ListUsers": rate.NewLimiter(rate.Limit(100), 50), // 100 rps }, - fallback: rate.NewLimiter(50, 25), // 50 rps default + fallback: rate.NewLimiter(rate.Limit(50), 25), // 50 rps default }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/interceptors.md` around lines 237 - 246, The code example using rate.NewLimiter incorrectly passes ints for the first parameter; update each call to cast the first argument to rate.Limit (e.g. rate.NewLimiter(rate.Limit(10), 5), rate.NewLimiter(rate.Limit(100), 50), rate.NewLimiter(rate.Limit(50), 25)) so the signature matches; change these instances where perMethodLimiter.limiters and perMethodLimiter.fallback are constructed (the calls to rate.NewLimiter referenced in the init block and passed to interceptors.SetRateLimiter).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@howto/interceptors.md`:
- Around line 248-259: Update the "Distributed rate limiting" section to either
remove the "go-redis/redis_rate" row or add a clear maintenance warning next to
the library name; specifically edit the table entry for "go-redis/redis_rate"
under the "Distributed rate limiting" heading so it indicates that the project
is unmaintained/stale (e.g., "unmaintained — last release 2023") or delete that
row entirely, leaving the other recommendations ("mennanov/limiters" and
"sethvargo/go-limiter") intact.
---
Duplicate comments:
In `@howto/interceptors.md`:
- Around line 237-246: The code example using rate.NewLimiter incorrectly passes
ints for the first parameter; update each call to cast the first argument to
rate.Limit (e.g. rate.NewLimiter(rate.Limit(10), 5),
rate.NewLimiter(rate.Limit(100), 50), rate.NewLimiter(rate.Limit(50), 25)) so
the signature matches; change these instances where perMethodLimiter.limiters
and perMethodLimiter.fallback are constructed (the calls to rate.NewLimiter
referenced in the init block and passed to interceptors.SetRateLimiter).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f410bc6-575c-4180-8a2b-1a3ca38b1d54
📒 Files selected for processing (3)
architecture.mdhowto/interceptors.mdhowto/production.md
✅ Files skipped from review due to trivial changes (1)
- howto/production.md
🚧 Files skipped from review as they are similar to previous changes (1)
- architecture.md
Summary
RATE_LIMIT_PER_SECOND,RATE_LIMIT_BURST,DISABLE_RATE_LIMITto config referenceSetRateLimiter())Test plan
Summary by CodeRabbit
New Features
Documentation