[TT-6075] Create option to populate the X-ratelimit headers from rate limits rather than quotas #7730

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.

Sign up for GitHub

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

shults wants to merge 19 commits into master from TT-6075-create-option-to-populate-the-x-ratelimit-headers-from-rate-limits-rather-than-quotas

+1,034 −302

probelabs / Visor: architecture succeeded Feb 19, 2026 in 51s

✅ Check Passed (Warnings Found)

architecture check passed. Found 3 warnings, but fail_if condition was not met.

Details

📊 Summary

Total Issues: 3
Warning Issues: 3

🔍 Failure Condition Results

Passed Conditions

global_fail_if: Condition passed

Issues by Category

Architecture (3)

⚠️ gateway/server.go:1612 - The limitHeaderFactory is initialized in NewGateway and then re-initialized here in initSystem. The accompanying comment // !!! review bootstrap process and NewGateway(); this process is broken :/ indicates a fragile or broken bootstrap process. This redundant initialization points to an underlying architectural issue in configuration loading and component initialization, making the system harder to reason about and maintain.
⚠️ internal/rate/limiter/limiter_leaky_bucket.go:35 - The function is named LeakyBucket, but a comment added in this PR states // implementation of this function is wrong // 1. it's not a leaky bucket. A function's name should accurately reflect its behavior. A misleading name makes the code difficult to understand and maintain, and can lead to incorrect usage.
⚠️ gateway/session_manager.go:357 - The newRateLimitChecker function contains a complex switch statement that handles multiple distinct rate-limiting strategies based on different configuration flags. While the introduction of the rate.Checker interface is a good abstraction, this function concentrates a lot of conditional logic in one place, which reduces modularity and makes the function difficult to test in isolation.

Powered by Visor from Probelabs

💡 TIP: You can chat with Visor using /visor ask <your question>

Annotations

Check warning on line 1613 in gateway/server.go

probelabs / Visor: architecture

architecture Issue

The `limitHeaderFactory` is initialized in `NewGateway` and then re-initialized here in `initSystem`. The accompanying comment `// !!! review bootstrap process and NewGateway(); this process is broken :/` indicates a fragile or broken bootstrap process. This redundant initialization points to an underlying architectural issue in configuration loading and component initialization, making the system harder to reason about and maintain.

Raw output


            Refactor the gateway's bootstrap process to ensure components like `limitHeaderFactory` are initialized only once with the final, correct configuration. This may involve consolidating configuration loading logic or changing the sequence of initialization steps to avoid redundant or conflicting setups.

Check warning on line 37 in internal/rate/limiter/limiter_leaky_bucket.go

probelabs / Visor: architecture

architecture Issue

The function is named `LeakyBucket`, but a comment added in this PR states `// implementation of this function is wrong // 1. it's not a leaky bucket`. A function's name should accurately reflect its behavior. A misleading name makes the code difficult to understand and maintain, and can lead to incorrect usage.

Raw output


            Rename the function to accurately describe its algorithm (e.g., `ThrottlingLimiter` or similar, based on its actual behavior). If the implementation is indeed incorrect or only for development builds (as hinted by `// 2. it's hidden behind compilation flag "dev"`), consider refactoring or removing it to avoid confusion.

Check warning on line 357 in gateway/session_manager.go

probelabs / Visor: architecture

architecture Issue

The `newRateLimitChecker` function contains a complex `switch` statement that handles multiple distinct rate-limiting strategies based on different configuration flags. While the introduction of the `rate.Checker` interface is a good abstraction, this function concentrates a lot of conditional logic in one place, which reduces modularity and makes the function difficult to test in isolation.

Raw output


            Refactor this logic by creating separate implementations of the `rate.Checker` interface for each strategy (e.g., `sentinelChecker`, `redisRollingChecker`, `drlChecker`). A simpler factory function could then select the appropriate checker implementation based on the configuration. This would align better with the Open/Closed Principle and make the system easier to extend and test.

View more details on probelabs