Skip to content

Conversation

@kkk777-7
Copy link
Member

@kkk777-7 kkk777-7 commented Nov 30, 2025

What this PR does / why we need it:
Support global ratelimit shadow mode.

Which issue(s) this PR fixes:

Fixes #6786 (need follow-up pr)

Release Notes: Yes

@kkk777-7 kkk777-7 requested a review from a team as a code owner November 30, 2025 16:02
@codecov
Copy link

codecov bot commented Nov 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.37%. Comparing base (aa3ad43) to head (fe552bf).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7630      +/-   ##
==========================================
+ Coverage   72.36%   72.37%   +0.01%     
==========================================
  Files         233      233              
  Lines       34343    34352       +9     
==========================================
+ Hits        24851    24862      +11     
+ Misses       7712     7710       -2     
  Partials     1780     1780              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: kkk777-7 <[email protected]>
@kkk777-7
Copy link
Member Author

kkk777-7 commented Dec 2, 2025

@zirain @zhaohuabing
I’d appreciate it if could focus your review on the global rate limit in this PR.

@zirain zirain changed the title feat: ratelimit shadow mode feat: global ratelimit shadow mode Dec 2, 2025
zirain
zirain previously approved these changes Dec 2, 2025
@zirain
Copy link
Member

zirain commented Dec 2, 2025

/retest

zhaohuabing
zhaohuabing previously approved these changes Dec 2, 2025
@arkodg
Copy link
Contributor

arkodg commented Dec 5, 2025

hey @kkk777-7 the API looks good ( per rule shadowing), can you confirm a few things, what do the various metrics looks like when this is set

  • ratelimit service metrics
  • ratelimit filter metrics

@kkk777-7 kkk777-7 dismissed stale reviews from zhaohuabing and zirain via fe552bf December 7, 2025 10:25
@kkk777-7
Copy link
Member Author

kkk777-7 commented Dec 7, 2025

Hi @arkodg, Please let me know if there's anything else I should check.

The results of sending 6 requests are as follows (limit: 3 per hour)

ratelimit service metrics

ratelimit_service_total_requests{grpc_method="ShouldRateLimit"} 6

ratelimit_service_rate_limit_total_hits{domain="gateway-conformance-infra/same-namespace/http",key1="httproute/gateway-conformance-infra/shadow-mode-ratelimit/rule/0/match/0/*_httproute/gateway-conformance-infra/shadow-mode-ratelimit/rule/0/match/0/*",key2="rule-0-method_rule-0-method"} 6

ratelimit_service_rate_limit_within_limit{domain="gateway-conformance-infra/same-namespace/http",key1="httproute/gateway-conformance-infra/shadow-mode-ratelimit/rule/0/match/0/*_httproute/gateway-conformance-infra/shadow-mode-ratelimit/rule/0/match/0/*",key2="rule-0-method_rule-0-method"} 3

ratelimit_service_rate_limit_over_limit{domain="gateway-conformance-infra/same-namespace/http",key1="httproute/gateway-conformance-infra/shadow-mode-ratelimit/rule/0/match/0/*_httproute/gateway-conformance-infra/shadow-mode-ratelimit/rule/0/match/0/*",key2="rule-0-method_rule-0-method"} 3

ratelimit_service_rate_limit_shadow_mode{domain="gateway-conformance-infra/same-namespace/http",key1="httproute/gateway-conformance-infra/shadow-mode-ratelimit/rule/0/match/0/*_httproute/gateway-conformance-infra/shadow-mode-ratelimit/rule/0/match/0/*",key2="rule-0-method_rule-0-method"} 3

ratelimit filter metrics

cluster.httproute/gateway-conformance-infra/shadow-mode-ratelimit/rule/0.ratelimit.ok: 6

comfirmed only ratelimit.ok.
other metrics didn't appear. (e.g. error, over_limit, failure_mode_allowed)
https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/rate_limit_filter#statistics

@kkk777-7
Copy link
Member Author

kkk777-7 commented Dec 7, 2025

/retest

@arkodg
Copy link
Contributor

arkodg commented Dec 8, 2025

thanks @kkk777-7, this means, for this feature, the envoy filter metrics dont represent shadowing, but envoy service metrics do, is this the intended goal ?
cc @nacx

@kkk777-7
Copy link
Member Author

kkk777-7 commented Dec 8, 2025

thanks @kkk777-7, this means, for this feature, the envoy filter metrics dont represent shadowing, but envoy service metrics do, is this the intended goal ?

Yes, that’s my understanding at the moment.
It seems that, in the current Envoy implementation, envoy ratelimit filter doesn't include metrics related to shadow mode.

https://github.com/envoyproxy/envoy/blob/main/source/extensions/filters/common/ratelimit/ratelimit_impl.cc
https://github.com/envoyproxy/envoy/blob/main/source/extensions/filters/http/ratelimit/ratelimit.cc#L213-L302

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add shadow_mode field to RateLimitRule

4 participants