-
Notifications
You must be signed in to change notification settings - Fork 200
Feat:2395 Per-key rate-limit overrides: configure distinct rate, burst, and period for specific keys/subgraphs; applied at runtime. #2409
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds per-key rate-limit overrides: new config types and schema, conversion helper in graph server, override storage and lookup in the rate limiter (including new interface and generateKey return), and tests updated to exercise suffix and override behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
router/core/graph_server.go(3 hunks)router/core/ratelimiter.go(4 hunks)router/core/ratelimiter_test.go(2 hunks)router/pkg/config/config.go(2 hunks)router/pkg/config/config.schema.json(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-08-20T10:08:17.857Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2155
File: router/core/router.go:1857-1866
Timestamp: 2025-08-20T10:08:17.857Z
Learning: router/pkg/config/config.schema.json forbids null values for traffic_shaping.subgraphs: additionalProperties references $defs.traffic_shaping_subgraph_request_rule with type "object". Therefore, in core.NewSubgraphTransportOptions, dereferencing each subgraph rule pointer is safe under schema-validated configs, and a nil-check is unnecessary.
Applied to files:
router/pkg/config/config.gorouter/core/graph_server.gorouter/pkg/config/config.schema.json
📚 Learning: 2025-09-17T20:55:39.456Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2172
File: router/core/graph_server.go:0-0
Timestamp: 2025-09-17T20:55:39.456Z
Learning: The Initialize method in router/internal/retrytransport/manager.go has been updated to properly handle feature-flag-only subgraphs by collecting subgraphs from both routerConfig.GetSubgraphs() and routerConfig.FeatureFlagConfigs.ConfigByFeatureFlagName, ensuring all subgraphs receive retry configuration.
Applied to files:
router/pkg/config/config.gorouter/core/graph_server.go
📚 Learning: 2025-11-12T18:38:46.619Z
Learnt from: StarpTech
Repo: wundergraph/cosmo PR: 2327
File: graphqlmetrics/core/metrics_service.go:435-442
Timestamp: 2025-11-12T18:38:46.619Z
Learning: In graphqlmetrics/core/metrics_service.go, the opGuardCache TTL (30 days) is intentionally shorter than the gql_metrics_operations database retention period (90 days). This design reduces memory usage while relying on database constraints to prevent duplicate inserts after cache expiration.
Applied to files:
router/core/graph_server.go
📚 Learning: 2025-07-21T15:06:36.664Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
Applied to files:
router/pkg/config/config.schema.json
📚 Learning: 2025-07-21T14:46:34.879Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Applied to files:
router/pkg/config/config.schema.json
📚 Learning: 2025-06-30T20:39:02.387Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 1929
File: router/internal/circuit/manager.go:16-25
Timestamp: 2025-06-30T20:39:02.387Z
Learning: In the Cosmo router project, parameter validation for circuit breaker configuration is handled at the JSON schema level rather than through runtime validation methods on structs. The config.schema.json file contains comprehensive validation constraints for circuit breaker parameters.
Applied to files:
router/pkg/config/config.schema.json
📚 Learning: 2025-10-01T20:39:16.113Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2252
File: router-tests/telemetry/telemetry_test.go:9684-9693
Timestamp: 2025-10-01T20:39:16.113Z
Learning: Repo preference: In router-tests/telemetry/telemetry_test.go, keep strict > 0 assertions for request.operation.*Time (parsingTime, normalizationTime, validationTime, planningTime) in telemetry-related tests; do not relax to >= 0 unless CI flakiness is observed.
Applied to files:
router/core/ratelimiter_test.go
📚 Learning: 2025-08-26T17:19:28.178Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2167
File: router/internal/expr/retry_context.go:3-10
Timestamp: 2025-08-26T17:19:28.178Z
Learning: In the Cosmo router codebase, prefer using netErr.Timeout() checks over explicit context.DeadlineExceeded checks for timeout detection in retry logic, as Go's networking stack typically wraps context deadline exceeded errors in proper net.Error implementations.
Applied to files:
router/core/ratelimiter_test.go
📚 Learning: 2025-09-24T12:54:00.765Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2222
File: router-tests/websocket_test.go:2238-2302
Timestamp: 2025-09-24T12:54:00.765Z
Learning: The wundergraph/cosmo project uses Go 1.25 (Go 1.23+ minimum), so fmt.Appendf and other newer Go standard library functions are available and can be used without compatibility concerns.
Applied to files:
router/core/ratelimiter.go
📚 Learning: 2025-09-24T12:54:00.765Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2222
File: router-tests/websocket_test.go:2238-2302
Timestamp: 2025-09-24T12:54:00.765Z
Learning: The wundergraph/cosmo project uses Go 1.25 (Go 1.25 minimum), so fmt.Appendf and other newer Go standard library functions are available and can be used without compatibility concerns.
Applied to files:
router/core/ratelimiter.go
🧬 Code graph analysis (3)
router/core/graph_server.go (4)
router/pkg/config/config.go (1)
RateLimitSimpleOverride(566-570)router/core/ratelimiter.go (1)
RateLimitOverride(89-93)router/pkg/logging/logging.go (1)
BufferedLogger(129-132)router/internal/graphqlmetrics/prometheus_exporter.go (1)
PrometheusMetricsExporter(14-16)
router/core/ratelimiter_test.go (1)
router/core/ratelimiter.go (3)
NewCosmoRateLimiter(38-72)CosmoRateLimiterOptions(27-36)RateLimitOverride(89-93)
router/core/ratelimiter.go (3)
router/internal/expr/expr.go (1)
Context(35-39)router/internal/expr/resolvers.go (1)
ResolveStringExpression(22-34)demo/pkg/subgraphs/family/subgraph/employees.go (1)
String(8-10)
🔇 Additional comments (14)
router/core/graph_server.go (3)
1422-1422: Wire-up looks correct.The overrides are properly converted and passed to the rate limiter options during initialization.
537-542: LGTM! Formatting changes only.The alignment changes to the
graphMuxstruct fields have no semantic impact.
117-130: LGTM! Clean conversion function with appropriate nil handling.The function correctly converts config-level overrides to runtime overrides with proper map pre-allocation and nil return for empty inputs. The configuration schema validates all override fields—rate and burst must be integers ≥ 1, and period must be a string (parsed to time.Duration), with all three fields required—making the direct field copy safe.
router/pkg/config/config.go (3)
556-564: LGTM! Clean addition of override support.The
Overridesfield properly extendsRateLimitSimpleStrategyto support per-key rate limit configuration with appropriate YAML/JSON tags.
362-382: LGTM! Formatting changes only.The alignment adjustments to
EngineDebugConfigurationstruct tags improve readability with no functional impact.
566-570: LGTM! Struct definition is clean and appropriate.The
RateLimitSimpleOverridestruct correctly defines the per-key override configuration with appropriate field types. Configuration schema validation inconfig.schema.jsonenforces constraints:rateandburstrequire minimum values of 1, andperiodis a required string field, preventing invalid override values at the configuration level.router/core/ratelimiter.go (4)
38-72: LGTM! Constructor properly handles override initialization.The override map is correctly trimmed of whitespace keys, converted to
redis_rate.Limitvalues, and stored in the limiter. Empty keys are properly skipped.
85-93: LGTM! Interface and override type are well-designed.The
redisLimiterinterface enables clean unit testing with fakes, andRateLimitOverridemirrors theredis_rate.Limitstructure appropriately.
119-136: LGTM! Key generation correctly returns both key and suffix.The updated signature enables tests to verify suffix extraction independently. The suffix represents the raw expression result before being appended to the base key.
138-148: LGTM! Override lookup with fallback is correctly implemented.The method properly checks the override map first and falls back to the context's default rate limit options.
router/core/ratelimiter_test.go (4)
43-157: LGTM! Existing tests properly updated for new signature.All test cases correctly capture and assert both the key and suffix values from the updated
generateKeyreturn signature. Coverage includes header-derived, claim-derived, and fallback scenarios.
160-248: LGTM! Override tests provide good coverage.The tests correctly verify:
- Override is applied when full key matches (including prefix)
- Fallback to defaults when override key doesn't match
- Claim-based suffix extraction with overrides
250-282: LGTM! End-to-end test validates override behavior with Redis.The test correctly verifies that:
- Override limits (Rate: 1, Burst: 1) take precedence over defaults (Rate: 5, Burst: 5)
- First request succeeds (uses burst allowance)
- Second request is rate-limited
Using
miniredis.RunT(t)ensures proper cleanup and test isolation.
284-297: LGTM! Clean test double implementation.The
fakeLimiterproperly implements theredisLimiterinterface and captures all parameters for test assertions.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
router/pkg/config/config.schema.json (1)
1988-1993: Add description to the override period field for consistency.The
periodfield in the override object (lines 1988-1993) is missing the description present in the parentsimple_strategy.period(lines 1954-1958). Including this description would improve user clarity about the expected format and supported units.Apply this diff to add the description:
"period": { "type": "string", + "description": "The period of time over which the rate limit is enforced. The period is specified as a string with a number and a unit, e.g. 10ms, 1s, 1m, 1h. The supported units are 'ms', 's', 'm', 'h'.", "duration": { "minimum": "1s" } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
router/pkg/config/config.schema.json(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-07-21T15:06:36.664Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
Applied to files:
router/pkg/config/config.schema.json
📚 Learning: 2025-07-21T14:46:34.879Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Applied to files:
router/pkg/config/config.schema.json
📚 Learning: 2025-06-30T20:39:02.387Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 1929
File: router/internal/circuit/manager.go:16-25
Timestamp: 2025-06-30T20:39:02.387Z
Learning: In the Cosmo router project, parameter validation for circuit breaker configuration is handled at the JSON schema level rather than through runtime validation methods on structs. The config.schema.json file contains comprehensive validation constraints for circuit breaker parameters.
Applied to files:
router/pkg/config/config.schema.json
📚 Learning: 2025-07-30T09:29:46.660Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2090
File: router/pkg/config/config.schema.json:0-0
Timestamp: 2025-07-30T09:29:46.660Z
Learning: The "operation_name_trim_limit" configuration property in router/pkg/config/config.schema.json should be placed at the security level as a sibling to complexity_limits, not inside the complexity_limits object.
Applied to files:
router/pkg/config/config.schema.json
📚 Learning: 2025-08-20T10:08:17.857Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2155
File: router/core/router.go:1857-1866
Timestamp: 2025-08-20T10:08:17.857Z
Learning: router/pkg/config/config.schema.json forbids null values for traffic_shaping.subgraphs: additionalProperties references $defs.traffic_shaping_subgraph_request_rule with type "object". Therefore, in core.NewSubgraphTransportOptions, dereferencing each subgraph rule pointer is safe under schema-validated configs, and a nil-check is unnecessary.
Applied to files:
router/pkg/config/config.schema.json
|
Example config |
|
End to end test file |
|
wundergraph/cosmo-docs#211 - docs |
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist