api: adds cost specifier to RateLimitRule

[mathetake]

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

• http ratelimit: option to reduce budget on stream done  envoy#37548
• global rate limit: supported ratelimits in the typed per filter config envoy#37684

Which issue(s) this PR fixes:
#4756

Release Notes: No (until the implementation is done)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.69%. Comparing base (5427854) to head (0d506e2).
Report is 3 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

[mathetake]

feel like maybe calling it "weight" instead of usage maybe be better? idk

[mathetake]

will add the cel validation this afternoon (in japan)

[mathetake]

maybe simply Usage->HitAddends would be consistent

[mathetake]

@zirain @zhaohuabing ptal - I removed the implementation-hide as envoyproxy/ratelimit#802 got merged

[mathetake]

i will do the implementation in a subsequent PR

[zhaohuabing]

@zirain @zhaohuabing ptal - I removed the implementation-hide as envoyproxy/ratelimit#802 got merged

@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]

LGTM thanks!
Prefer to hide the API in the docs as it's not implemented yet.

[mathetake]

ah ok - will do

[mathetake]

@zhaohuabing hid the APIs!

[zirain]

LGTM, hold for a while as @arkodg may want to take a look.

[zhaohuabing]

LGTM defer to @arkodg

[mathetake]

👍

[mathetake]

yeah i would also prefer costPerRequest over perRequestCost

[missBerg]

Just to add another idea into the mix.... what about:

cost:
  request:
      source: metadata
      metadata:
        namespace:
        key:
  response:
        source: metadata
      metadata:
        namespace:
        key:

then we get cleaner API field names, and we put all the cost configurations together.

[arkodg]

Just to add another idea into the mix.... what about:

cost:
  request:
      source: metadata
      metadata:
        namespace:
        key:
  response:
        source: metadata
      metadata:
        namespace:
        key:

then we get cleaner API field names, and we put all the cost configurations together.

+1 wfm

[arkodg]

LGTM thanks !

[mathetake]

is this flake or what

[zirain]

/retest

[mathetake]

sorry the e2e tests logs are hard to grok ... will take a look

[mathetake]

@zirain passed now

[yuzisun]

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?

[mathetake]

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".

[arkodg]

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