-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Per connection local rate limiting #15843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
dc475ff
24ba562
476e5d7
e8f8e83
745bf66
840742a
edb7ab8
9300cea
b256ffd
6351e40
5bdf0f9
efc3a07
eca4c02
a79905a
8656cd7
6c77508
bbc11c2
3d850ef
443def6
35f35aa
b3e1702
564571a
8719660
9e43177
4e239f9
9bfcf1a
489d66c
fcf83d8
44e4e45
f38f0a7
0e95698
eb71d7b
972758d
8faa072
1c3ec7a
d1397ac
91c8ca2
533254a
9ff513b
61b464a
2054d3e
e2c2280
3ad96b8
0aec425
79afd6b
676588a
fb2178e
e29dc13
8cb5806
99e941a
9a00865
e4eba7c
5876f6c
ef28b68
0e4b716
725816f
83500a7
2960c69
67eb5e2
875e059
8c7e0c0
98fb985
37d353f
32f82c5
44bbf4d
443fdc9
174431b
166241d
707d3b9
aa4832e
93ca37a
0cb5c40
263a0a8
3df15d2
c2dd672
8119596
7366497
fcf28ca
bcf0348
ac1d176
cfa681a
b5be190
a921fee
33ee0c4
d2f6f8f
7bbec8c
fe8d26e
fdfd990
5ca8184
dfcc7bb
89ed219
8f0f92f
7efd4fa
0082d5a
d202a27
dd5eaf5
8c85cec
199cb5a
0dd325d
f00c9fd
c93f6fe
9b4fef0
eccaba5
ad0242d
8711bd4
9e9dfab
a857dd3
6b0235c
0417587
e6408bb
9f17367
2f603d8
562be5e
add870b
6bf6698
f7aa00d
f712bc8
0dd2f98
454fa95
65fb735
ed858b4
badf219
26e73c4
8e338b2
fbcf203
33bedd1
0566b86
762fb6a
5a91170
633098c
1ab6a0c
44ae575
1741a79
50307b6
41fd116
1ff2c4b
2916b64
27460f6
bc12ea0
64bb450
cdf57ee
bb2e153
0e17903
f921dc5
4e54def
99a6c6c
74fcf4e
8e9880a
eacc127
13577b9
c014687
f75eb68
f5bae8e
a57014f
0e2843c
eb93f69
ded5417
06aa1d8
3146b6a
e5065e7
7eae147
d522352
59bd835
5d3f343
3ce48c8
24be394
b6e3303
188b61f
70ee15b
47380a8
14b1705
f68f942
cda66d3
c452d32
6ace0e0
7bf71a4
47d4acc
4c8af40
b7db037
6966d3e
eda8994
a0afd00
99849a0
1ab83de
420fa93
22e72c9
c67e4ba
aeeae43
b2322d5
1bb38e4
09ff689
1ac2759
4c85898
9a4dd7b
c0944a6
45a3a17
a5d5136
33cf77c
89ba8f7
adddd73
1a995bb
cbc0a36
7b8c058
8522075
6de3b44
293060d
5febd9c
c8e32e2
d64c987
671559b
df340bf
ffe87db
1e44f09
bebdce7
0b047ae
4603b74
9d95544
5861909
7ca878b
39c389f
d44697c
aab4d01
e405cf9
e061e08
d7471ea
14d6198
de06957
a8b8d4c
1d36b8c
ea3fe62
57bd864
fc0415e
52ffa87
7525e6d
d4e077a
2e77edd
323eb26
95d8f9a
318c4dc
b076788
8a53a6d
a1d6427
43648c2
9c76913
c1d49bf
2d12dae
c288dc6
a6b15b0
d994fee
b2b2d67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; | |
| // Local Rate limit :ref:`configuration overview <config_http_filters_local_rate_limit>`. | ||
| // [#extension: envoy.filters.http.local_ratelimit] | ||
|
|
||
| // [#next-free-field: 11] | ||
| // [#next-free-field: 12] | ||
| message LocalRateLimit { | ||
| // The human readable prefix to use when emitting stats. | ||
| string stat_prefix = 1 [(validate.rules).string = {min_len: 1}]; | ||
|
|
@@ -97,4 +97,13 @@ message LocalRateLimit { | |
| // | ||
| // The filter supports a range of 0 - 10 inclusively for stage numbers. | ||
| uint32 stage = 9 [(validate.rules).uint32 = {lte: 10}]; | ||
|
|
||
| // Specifies the scope of the rate limiter's token bucket. | ||
| // If set to false, the token bucket is shared across all worker threads, | ||
| // thus the rate limits are applied per Envoy process. | ||
| // If set to true, a token bucket is allocated for each connection. | ||
| // Thus the rate limits are applied per connection thereby allowing | ||
| // one to rate limit requests on a per connection basis. | ||
| // If unspecified, the default value is false. | ||
| bool local_rate_limit_per_downstream_connection = 11; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible for a proxy to have both per-process and per downstream connection rate limits, or just one or the other? Another potential question is if we should consider some additional rate limit criteria in the future like downstream IP or HTTP Cookie. If we expect additional criteria in the future, we may want to make this an enum field.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's definitely one or the other as it stands currently as I'm not sure the added complexity of dealing with potentially conflicting token bucket quotas between the per process and per connection configurations and the precedence rules we'd have to handle buys us much in the way of functionality. We did indeed consider an enum initially but it didn't seem like there were very many realistic use cases mainly because a lot of the toggles that were based on certain request characteristics such as IP, Cookie etc can be handled today by rate limiting on request descriptors ... |
||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,18 +13,24 @@ namespace Extensions { | |
| namespace HttpFilters { | ||
| namespace LocalRateLimitFilter { | ||
|
|
||
| const std::string& PerConnectionRateLimiter::key() { | ||
| CONSTRUCT_ON_FIRST_USE(std::string, "per_connection_local_rate_limiter"); | ||
| } | ||
|
|
||
| FilterConfig::FilterConfig( | ||
| const envoy::extensions::filters::http::local_ratelimit::v3::LocalRateLimit& config, | ||
| const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, Stats::Scope& scope, | ||
| Runtime::Loader& runtime, const bool per_route) | ||
| : status_(toErrorCode(config.status().code())), | ||
| stats_(generateStats(config.stat_prefix(), scope)), | ||
| fill_interval_(std::chrono::milliseconds( | ||
| PROTOBUF_GET_MS_OR_DEFAULT(config.token_bucket(), fill_interval, 0))), | ||
| max_tokens_(config.token_bucket().max_tokens()), | ||
| tokens_per_fill_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config.token_bucket(), tokens_per_fill, 1)), | ||
| descriptors_(config.descriptors()), | ||
| rate_limit_per_connection_(config.local_rate_limit_per_downstream_connection()), | ||
| rate_limiter_(Filters::Common::LocalRateLimit::LocalRateLimiterImpl( | ||
| std::chrono::milliseconds( | ||
| PROTOBUF_GET_MS_OR_DEFAULT(config.token_bucket(), fill_interval, 0)), | ||
| config.token_bucket().max_tokens(), | ||
| PROTOBUF_GET_WRAPPED_OR_DEFAULT(config.token_bucket(), tokens_per_fill, 1), dispatcher, | ||
| config.descriptors())), | ||
| fill_interval_, max_tokens_, tokens_per_fill_, dispatcher, descriptors_)), | ||
| local_info_(local_info), runtime_(runtime), | ||
| filter_enabled_( | ||
| config.has_filter_enabled() | ||
|
|
@@ -84,7 +90,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, | |
| populateDescriptors(descriptors, headers); | ||
| } | ||
|
|
||
| if (config->requestAllowed(descriptors)) { | ||
| if (requestAllowed(descriptors)) { | ||
| config->stats().ok_.inc(); | ||
| return Http::FilterHeadersStatus::Continue; | ||
| } | ||
|
|
@@ -109,6 +115,34 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, | |
| return Http::FilterHeadersStatus::StopIteration; | ||
| } | ||
|
|
||
| bool Filter::requestAllowed(absl::Span<const RateLimit::LocalDescriptor> request_descriptors) { | ||
| const auto* config = getConfig(); | ||
| return config->rateLimitPerConnection() | ||
| ? getPerConnectionRateLimiter().requestAllowed(request_descriptors) | ||
| : config->requestAllowed(request_descriptors); | ||
| } | ||
|
|
||
| const Filters::Common::LocalRateLimit::LocalRateLimiterImpl& Filter::getPerConnectionRateLimiter() { | ||
| const auto* config = getConfig(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move this closer to where it's used, e.g.: just before
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved it back up a couple of lines to add an assertion. |
||
| ASSERT(config->rateLimitPerConnection()); | ||
|
|
||
| if (!decoder_callbacks_->streamInfo().filterState()->hasData<PerConnectionRateLimiter>( | ||
| PerConnectionRateLimiter::key())) { | ||
| decoder_callbacks_->streamInfo().filterState()->setData( | ||
| PerConnectionRateLimiter::key(), | ||
| std::make_unique<PerConnectionRateLimiter>( | ||
| config->fillInterval(), config->maxTokens(), config->tokensPerFill(), | ||
| decoder_callbacks_->dispatcher(), config->descriptors()), | ||
| StreamInfo::FilterState::StateType::ReadOnly, | ||
| StreamInfo::FilterState::LifeSpan::Connection); | ||
| } | ||
|
|
||
| return decoder_callbacks_->streamInfo() | ||
| .filterState() | ||
| ->getDataReadOnly<PerConnectionRateLimiter>(PerConnectionRateLimiter::key()) | ||
| .value(); | ||
| } | ||
|
|
||
| void Filter::populateDescriptors(std::vector<RateLimit::LocalDescriptor>& descriptors, | ||
| Http::RequestHeaderMap& headers) { | ||
| Router::RouteConstSharedPtr route = decoder_callbacks_->route(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.