Skip to content

rate-limit - add unit_multiplier#40830

Closed
Pawan-Bishnoi wants to merge 3 commits intoenvoyproxy:mainfrom
Pawan-Bishnoi:add_unit_multiplier_rls
Closed

rate-limit - add unit_multiplier#40830
Pawan-Bishnoi wants to merge 3 commits intoenvoyproxy:mainfrom
Pawan-Bishnoi:add_unit_multiplier_rls

Conversation

@Pawan-Bishnoi
Copy link
Copy Markdown
Contributor

Commit Message: add a unit multiplier field to rls proto
Additional Description: so that we can configure rate limits for n minutes/days etc.
Risk Level: Low
Testing: added
Release Notes: todo
Fixes: #33277

@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto).
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @mattklein123
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #40830 was opened by Pawan-Bishnoi.

see: more, trace.


// Multiplier for the unit of time. For example, if unit is SECOND and unit_multiplier is 30,
// the rate limit will be applied per 30 seconds. If not specified, defaults to 1.
uint32 unit_multiplier = 3 [(validate.rules).uint32 = {gte: 1}];
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pawan-Bishnoi
Copy link
Copy Markdown
Contributor Author

@agrawroh are the test failures related to your recent PR: #40169 ?

Copy link
Copy Markdown
Member

@talnordan talnordan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Note that this pull request seems to cover the scope of #40222, which might be a duplicate issue.

See comments.

Comment thread source/extensions/filters/http/ratelimit/ratelimit_headers.cc Outdated
Signed-off-by: Pawan Bishnoi <pawanbishnoi@outlook.com>
Signed-off-by: Pawan Bishnoi <pawanbishnoi@outlook.com>
Signed-off-by: Pawan Bishnoi <pawanbishnoi@outlook.com>
@Pawan-Bishnoi Pawan-Bishnoi force-pushed the add_unit_multiplier_rls branch from aae79da to 8d1961d Compare August 27, 2025 16:45
@kyessenov
Copy link
Copy Markdown
Contributor

@mattklein123 PTAL for API review.
/wait


// Multiplier for the unit of time. For example, if unit is SECOND and unit_multiplier is 30,
// the rate limit will be applied per 30 seconds. If not specified, defaults to 1.
uint32 unit_multiplier = 4;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not touch v2 directory, it's an archive.

// Multiplier for the unit of time. For example, if unit is SECOND and unit_multiplier is 30,
// the rate limit will be applied per 30 seconds. If not specified, defaults to 1.
// To prevent integer overflow and enforce reasonable limits, the maximum value is 2000.
uint32 unit_multiplier = 3 [(validate.rules).uint32 = {lte: 2000 gte: 1}];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use google.protobuf.Uint32Wrapper if the default value is not 0.

struct RateLimitOverride {
uint32_t requests_per_unit_;
envoy::type::v3::RateLimitUnit unit_;
uint32_t unit_multiplier_ = 1;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's handling unset values correctly, or I'm missing it somewhere?

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 3, 2025

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions Bot added the stale stalebot believes this issue/PR has not been touched recently label Oct 3, 2025
@github-actions
Copy link
Copy Markdown

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions Bot closed this Oct 10, 2025
@Pawan-Bishnoi
Copy link
Copy Markdown
Contributor Author

@kyessenov please help reopen this PR. I am working on review comments. Thank you!

@lboynton
Copy link
Copy Markdown
Contributor

👍 for reopening

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api stale stalebot believes this issue/PR has not been touched recently v2-freeze waiting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support multiples of units for rate limits

5 participants