api: adds cost specifier to RateLimitRule#4957
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4957 +/- ##
==========================================
- Coverage 66.74% 66.69% -0.06%
==========================================
Files 209 209
Lines 32264 32264
==========================================
- Hits 21535 21518 -17
- Misses 9432 9445 +13
- Partials 1297 1301 +4 ☔ View full report in Codecov by Sentry. |
|
feel like maybe calling it "weight" instead of usage maybe be better? idk |
|
will add the cel validation this afternoon (in japan) |
|
maybe simply Usage->HitAddends would be consistent |
|
@zirain @zhaohuabing ptal - I removed the implementation-hide as envoyproxy/ratelimit#802 got merged |
|
i will do the implementation in a subsequent PR |
@mathetake the implementation comment is used to hide the unimplemented EG API in the EG docs, we normally hide the API at the API PR and unhide it at the implementation PR. |
zhaohuabing
left a comment
There was a problem hiding this comment.
LGTM thanks!
Prefer to hide the API in the docs as it's not implemented yet.
|
ah ok - will do |
|
@zhaohuabing hid the APIs! |
|
👍 |
c125ff2 to
d1b3de3
Compare
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
a458c84 to
57290ad
Compare
|
is this flake or what |
|
/retest |
|
sorry the e2e tests logs are hard to grok ... will take a look |
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
|
@zirain passed now |
| @@ -91,6 +92,91 @@ type RateLimitRule struct { | |||
| // 429 HTTP status code is sent back to the client when | |||
| // the selected requests have reached the limit. | |||
| Limit RateLimitValue `json:"limit"` | |||
There was a problem hiding this comment.
It seems like it is still required to specify the number of requests limit, how we can handle the case when we only care of the cost not the number of the request limit?
There was a problem hiding this comment.
ok so first of all, this limit is nothing to do with that purpose. And to do the limit purely on the token number (in this context response cost only), you can simply set the cost.response = {from: Number, Number: 0} as per the comment - zero can be used to "only check the budget and if not left anything, then reject".
There was a problem hiding this comment.
also with this new cost field, limit.requests doesnt quite fit anymore, something like limit.total feels better and can be an alias to limit.requests in the future to improve defining intent
This adds the RequestCost field to AIGatewayRoute, which will allows users to do the rate limiting etc based on the calculated "token usage". This is based on the new feature introduced in * envoyproxy/gateway#4957 * envoyproxy/gateway#5035 and because of the feature, the only thing we have to do from AI Gateway side is to set a dynamic metadata as per the comment in the API. --------- Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
What type of PR is this?
This adds a new API into rate limit API.
What this PR does / why we need it:
This is for #4756. Basically, this adds the API to specify the hits_addend for the rate limit rule.
Especially, configuring the hits_addend in the response_path allows us to "reduce" the counter
based on the response content that affects the subsequent requests. This will enable the "token"
based request limits that are required fro AI gateway.
This is based on the Envoy's brand new features called apply_on_stream_done
and per-descriptor level hits_addend configuration introduced in
Which issue(s) this PR fixes:
#4756
Release Notes: No (until the implementation is done)