ratelimit: per-descriptor hits_addend for route config #37972
Conversation
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
| @@ -263,12 +264,80 @@ QueryParameterValueMatchAction::buildQueryParameterMatcherVector( | |||
| return ret; | |||
| } | |||
|
|
|||
| absl::Status resolveHitsAddendSource(const envoy::config::route::v3::RateLimit::HitsAddend& config, | |||
There was a problem hiding this comment.
this and getHitsAddendViaProvider are simply moved from the ratelimit_config.cc below, and their logic hasn't changed
|
/assign @wbpcode |
|
ok formatter having exceptions makes mobile angry |
|
so i disable this for envoy mobile build using ENVOY_STATIC_EXTENSION_REGISTRATION - at the end of the day, it's useless on mobile, so it should be good |
|
@phlax fyi this will be a blocker for the next v1.33 since otherwise the new API would be half-backed |
| ] + select({ | ||
| "//bazel:disable_static_extension_registration": [], | ||
| "//conditions:default": [ | ||
| # See the comments in the header file. | ||
| "//source/common/formatter:formatter_extension_lib", | ||
| "//source/common/formatter:substitution_formatter_lib", | ||
| ], | ||
| }), |
There was a problem hiding this comment.
This actually make thing too complex. basically, you only need to link the substitution_formatter_lib here which is exception free.
The main lib will link the formatter_extension_lib as extensions.
There was a problem hiding this comment.
Basically, I won't recommend to add new feature to the route's rate_limits because we should use the typed_per_filter_config for filters' configuration.
So, if there is no strong requirement, I will suggest you to use typed_per_filter_config rather than enhancing the route's rate_limits.
|
/wait |
|
ok, i will take a look at the EG side on how i can mitigate - let's see |
|
ok managed to make it work without this change, thank you for the comment @wbpcode |
Commit Message: ratelimit: per-descriptor hits_addend for route config
Additional Description:
This is a follow up on #37684. Previously, the newly introduced
per-descriptor hits_addend API was only supported by typed_per_filter_config
and its support in the "legacy" core router ratelimit configuration was left TODO.
Since the RateLimitPolicy is coupled with route(r), this impl must be inside
the core, which makes it inappropriate to use formatter in Envoy mobile due to
its use of exceptions. However, even if it didn't have exception uses, it would be
still useless as the mobile cannot use rate limit filter at the end of the day.
So, this patch uses the macro to exclude the entire logic and dependency for
Envoy mobile, which can be relaxed after removing exceptions from formatters.
Risk Level: low
Testing: unit test
Docs Changes: n/a (already done in #37684)
Release Notes: n/a (already done in #37684)
Platform Specific Features: n/a