[TT-7325] Enable fixed window rate limiter by titpetric · Pull Request #6253 · TykTechnologies/tyk

titpetric · 2024-04-26T07:54:26Z

User description

The PR enables a fixed window rate limiter from behind the dev build tag.

Type

enhancement

Description

Removed the EnableFixedWindowRateLimiter setting from config/development.go as it seems to be restructured.
Introduced EnableFixedWindowRateLimiter in config/rate_limit.go to centralize rate limiting configurations.
Added a string output for when the fixed window rate limiter is enabled, improving debuggability and monitoring.

Changes walkthrough

Relevant files

Configuration changes

development.go `Remove Fixed Window Rate Limiter Setting from Development Config` config/development.go Removed the `EnableFixedWindowRateLimiter` setting.	+0/-3

Enhancement

rate_limit.go `Introduce Fixed Window Rate Limiter Configuration` config/rate_limit.go Added `EnableFixedWindowRateLimiter` setting to enable fixed window rate limiting. Added a string representation for when the fixed window rate limiter is enabled.	+7/-0

✨ PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

github-actions · 2024-04-26T07:55:23Z

PR Description updated to latest commit (1f9f0aa)

github-actions · 2024-04-26T07:55:25Z

API Changes

--- prev.txt	2024-05-16 07:39:52.185815846 +0000
+++ current.txt	2024-05-16 07:39:49.045816523 +0000
@@ -5812,6 +5812,9 @@
     GetOAuthTokensPurgeInterval returns purge interval for lapsed OAuth tokens.
 
 type RateLimit struct {
+	// EnableFixedWindow enables fixed window rate limiting.
+	EnableFixedWindowRateLimiter bool `json:"enable_fixed_window_rate_limiter"`
+
 	// Redis based rate limiter with fixed window. Provides 100% rate limiting accuracy, but require two additional Redis roundtrip for each request.
 	EnableRedisRollingLimiter bool `json:"enable_redis_rolling_limiter"`

github-actions · 2024-04-26T07:55:38Z

PR Review

⏱️ Estimated effort to review [1-5]	2, because the changes are straightforward and involve moving a configuration setting from one file to another while adding a string representation for debugging. The logic is simple and the impact is limited to configuration handling.
🧪 Relevant tests	No
🔍 Possible issues	Missing Condition Handling: The new string representation for the fixed window rate limiter in `config/rate_limit.go` does not handle the case where multiple rate limiters might be enabled simultaneously. This could lead to misleading debug outputs if more than one limiter is active.
🔒 Security concerns	No

Code feedback:

relevant file	config/rate_limit.go
suggestion	Consider handling multiple rate limiters being enabled simultaneously. You could append the status of each rate limiter to the output string rather than returning immediately when one is enabled. This change would provide a more comprehensive debug output. [important]
relevant line	return fmt.Sprintf("Fixed Window Rate Limiter enabled")

✨ Review tool usage guide:

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

github-actions · 2024-04-26T07:55:59Z

PR Code Suggestions

Category	Suggestions
Enhancement	Modify the method to include all applicable rate limiter settings in the output. Instead of returning immediately when `EnableFixedWindowRateLimiter` is true, consider appending this information to a string and continue checking other conditions. This ensures that all relevant rate limiter settings are included in the output, not just the first one that evaluates to true. config/rate_limit.go [45-47] if r.EnableFixedWindowRateLimiter { - return fmt.Sprintf("Fixed Window Rate Limiter enabled") + info += "Fixed Window Rate Limiter enabled; " }
Best practice	Ensure proper spacing in concatenated status messages. Add a space after the semicolon in the string concatenation to ensure proper formatting when multiple rate limiter settings are enabled. config/rate_limit.go [45-47] -info += "Fixed Window Rate Limiter enabled;" +info += "Fixed Window Rate Limiter enabled; "

✨ Improve tool usage guide:

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

github-actions · 2024-04-26T07:56:23Z

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

github-actions · 2024-04-26T07:56:25Z

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

github-actions · 2024-04-26T11:03:15Z

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

sonarqubecloud · 2024-05-16T07:52:58Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

## **User description** The PR enables a fixed window rate limiter from behind the dev build tag. ___ ## **Type** enhancement ___ ## **Description** - Removed the `EnableFixedWindowRateLimiter` setting from `config/development.go` as it seems to be restructured. - Introduced `EnableFixedWindowRateLimiter` in `config/rate_limit.go` to centralize rate limiting configurations. - Added a string output for when the fixed window rate limiter is enabled, improving debuggability and monitoring. ___ ## **Changes walkthrough** <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Configuration changes </strong></td><td><table> <tr> <td> <details> <summary><strong>development.go</strong><dd><code>Remove Fixed Window Rate Limiter Setting from Development Config</code></dd></summary> <hr> config/development.go - Removed the `EnableFixedWindowRateLimiter` setting. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6253/files#diff-d2253d9377e5163d9de068a2df71738383fb97e0b07b64482404a83610cd53b8">+0/-3</a>      </td> </tr> </table></td></tr><tr><td><strong>Enhancement </strong></td><td><table> <tr> <td> <details> <summary><strong>rate_limit.go</strong><dd><code>Introduce Fixed Window Rate Limiter Configuration</code>                </dd></summary> <hr> config/rate_limit.go <li>Added <code>EnableFixedWindowRateLimiter</code> setting to enable fixed window rate <br>limiting.<br> <li> Added a string representation for when the fixed window rate limiter <br>is enabled.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6253/files#diff-375bf116f8d6527c50d7591d7cb01e8f821b22df4a4ca18b4da4c6f0d526f18e">+7/-0</a>      </td> </tr> </table></td></tr></tr></tbody></table> ___ > ✨ **PR-Agent usage**: >Comment `/help` on the PR to get a list of all available PR-Agent tools and their descriptions --------- Co-authored-by: Tit Petric <[email protected]>

github-actions Bot added the enhancement label Apr 26, 2024

github-actions Bot added the Review effort [1-5]: 2 label Apr 26, 2024

titpetric requested a review from a team as a code owner April 26, 2024 11:01

titpetric force-pushed the test/patch-sentinel-key-2 branch from 4d09ac6 to 081f3a9 Compare May 8, 2024 14:39

jeffy-mathew approved these changes May 8, 2024

View reviewed changes

titpetric force-pushed the test/patch-sentinel-key-2 branch from 081f3a9 to 59612cd Compare May 16, 2024 07:10

titpetric enabled auto-merge (squash) May 16, 2024 07:10

Tit Petric added 6 commits May 16, 2024 09:39

Don't count blocked requests during sentinel key lifetime

bfd11de

Get rid of DRL/RRL switch and use the quota mechanism for rate limit

c1c5e55

Re-implement fixed-window from existing RRL

ea9c802

Revert POC changes, enable fixed window rate limiter

999ca5e

Actually enable fixed window rate limiter

068ac79

Fix lint

2d2116e

titpetric force-pushed the test/patch-sentinel-key-2 branch from 59612cd to 2d2116e Compare May 16, 2024 07:39

titpetric merged commit d3e40b8 into master May 16, 2024

titpetric deleted the test/patch-sentinel-key-2 branch May 16, 2024 07:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[TT-7325] Enable fixed window rate limiter#6253

[TT-7325] Enable fixed window rate limiter#6253
titpetric merged 6 commits into
masterfrom
test/patch-sentinel-key-2

titpetric commented Apr 26, 2024 •

edited by github-actions Bot

Loading

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

github-actions Bot commented Apr 26, 2024 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

sonarqubecloud Bot commented May 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

titpetric commented Apr 26, 2024 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Type

Description

Changes walkthrough

Uh oh!

github-actions Bot commented Apr 26, 2024

Uh oh!

github-actions Bot commented Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 26, 2024

PR Review

Uh oh!

github-actions Bot commented Apr 26, 2024

PR Code Suggestions

Uh oh!

github-actions Bot commented Apr 26, 2024

💥 CI tests failed 🙈

git-state

Uh oh!

github-actions Bot commented Apr 26, 2024

💥 CI tests failed 🙈

git-state

Uh oh!

github-actions Bot commented Apr 26, 2024

💥 CI tests failed 🙈

git-state

Uh oh!

sonarqubecloud Bot commented May 16, 2024

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

titpetric commented Apr 26, 2024 •

edited by github-actions Bot

Loading

github-actions Bot commented Apr 26, 2024 •

edited

Loading