Conversation
The limit key usually comes from a request variable, so a quote or backslash in it made the variable invalid JSON.
The local fixed window records its end time when a request looks like the first one in the window, i.e. when the counter equals its cost. A zero-cost request such as ai-rate-limiting's access-phase check matches that on every call while the counter is 0, and the first real charge matches it again, so each of them pushed the recorded end time forward while the counter kept its original TTL. The reset header then reported a later reset than the counter actually had. Only record the end time if none is recorded yet for the window.
Append the window the request was counted in to $rate_limiting_info, after the four existing fields: the window type and size, the decision, what the request added to the counter, when it was evaluated, and the current window's boundaries and count in milliseconds. A sliding window also reports the previous window's count and the weight applied to it, and delayed sync reports the synced count and the unsynced local delta. The limiters only pass out values they already hold, as an extra return value and as extra fields in the quota the delayed syncer already stores, so there is no additional shared dict or Redis access and no decision changes. Values a code path does not have are reported as null.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
$rate_limiting_infoonly carries the key, limit, remaining and reset, which isn't enough to tell from an access log why a request was let through or rejected, or when exactly a window started. This PR appends the window the request was counted in, with millisecond timestamps. The four existing fields are unchanged and still come first.Fixed window:
{"rate_limiting_key":"/apisix/routes/1:1:127.0.0.1","rate_limiting_limit":10,"rate_limiting_remaining":3,"rate_limiting_reset":42,"window_type":"fixed","window_size_ms":60000,"decision":"allowed","cost":1,"evaluated_at_ms":1759212345678,"current_window":{"start_ms":1759212300123,"end_ms":1759212360123,"count":7,"created":false}}Sliding window:
{"rate_limiting_key":"/apisix/routes/1:1:127.0.0.1","rate_limiting_limit":10,"rate_limiting_remaining":3,"rate_limiting_reset":14,"window_type":"sliding","window_size_ms":60000,"decision":"allowed","cost":1,"evaluated_at_ms":1759212345678,"current_window":{"id":29320205,"start_ms":1759212300000,"end_ms":1759212360000,"count":4},"previous_window":{"count":10,"weight":0.238700,"weighted_count":2.387}}With
sync_interval, a"delayed_sync":{"synced_at_ms":…,"synced_count":…,"local_delta":…}object is added afterevaluated_at_ms, andcurrent_window.countbecomes the estimatesynced_count + local_delta + cost.decisionisallowed,rejectedorerror, and anerroronly carrieswindow_type,window_size_msanddecision, so a Redis failure no longer looks like a rejection in the log.countis the counter after this request andcostis what this request added to it, which is needed to reconstruct the decision: a fixed window counts rejected requests too, a sliding window and delayed sync don't.This is observability only. The limiters just pass out values they already have, as an extra return value of
incoming()/commit(), plus three more fields in the quota JSON that the delayed syncer already writes to the shared dict. There is no extra shared dict or Redis call, no Redis script change, and no change to any decision. So a few fields arenullwhere the code path doesn't have the value:current_window.createdwith the Redis policiescurrent_window.countwhen the local fixed window rejects (resty.limit.countdrops the counter on rejection)synced_count, and the sliding window fields) until the first sync after an upgradedelayed_sync.local_deltais the local delta the decision subtracted. Right after a refresh of an expired cached quota, that delta was already flushed intosynced_count, which is the double count #13950 fixes. With #13950 it is 0 there, as it should be.The JSON is built in
core.utils.set_var_rate_limiting_info, which still accepts the old five arguments.Two small fixes found along the way, in their own commits:
X-AI-RateLimit-Resetover-reported. The end time is now only recorded when the window has none yet (addinstead ofset).t/plugin/ai-rate-limiting.tTEST 42 fails without it (reset3instead of ~1.5).Per-request overhead: I ran wrk2 at a fixed 10k req/s against a single worker with one local rule and the access log off, so this is the cost every request pays whether or not the variable is logged. Over 8 interleaved rounds against master, worker CPU per request was 52.1 vs 52.2 µs for the fixed window and 54.6 vs 54.6 µs for the sliding window (medians). The paired per-round differences stay within the ±3 µs run-to-run noise. Benchmarking
set_var_rate_limiting_infoalone puts the added formatting at about 0.7 µs (fixed) and 1.1 µs (sliding) per call with the JIT off, and 0.1 / 0.3 µs with it on. The common cases are formatted in a singlestring.formatover integers for that reason.Tests:
t/plugin/limit-count-rate-limiting-info.tcovers local, redis and redis-cluster × fixed/sliding, delayed sync for both window types, and the degraded error path. It decodes the variable in the log phase and checks the timestamps against each other, so it doesn't depend on wall-clock values. The existing access-log assertions inlimit-count-variable.tandlimit-count5.tare adjusted for the appended fields. The docs gain a section on the variable inlimit-count.md(en/zh).