feat: per-unit-descriptor-response-headers

[seluard]

Summary

Supersedes #1190, which introduced optional response headers for every configured rate-limit descriptor.

This change:

• adds LIMIT_PER_UNIT_HEADERS_ENABLED
• returns Ratelimit-Limit-<unit> and Ratelimit-Remaining-<unit> for each limited descriptor
• documents the setting in README.md
• preserves the existing standard response headers when LIMIT_RESPONSE_HEADERS_ENABLED is also enabled
• resolves the conflict with current main

Credit

This work is based on the original implementation by @samay-arcana in #1190. Thank you for the initial feature and test coverage.

Testing

go test ./test/service -count=1

[seluard]

Can you review this one @psbrar99 ? Thanks !

[nacx]

Thanks you! I left come comments.

[nacx]

Keying the header purely by time unit (ratelimit-limit-seconds, ratelimit-remaining-seconds) breaks down when a request matches two or more descriptors that share the same unit (for example, a per-IP "10/sec" rule and a per-API-key "5/sec" rule on the same request).
You'd emit two ratelimit-limit-seconds / ratelimit-remaining-seconds entries with different values and no way for the client to tell which is which. The test here (and the original #1190 example) only exercises one SECOND + one MINUTE descriptor, so this case isn't covered. Given the feature's whole point is "show me every descriptor's limits," could we disambiguate (like including the descriptor key or an index in the header name) or at least document this as a known limitation and add a test for the collision case?

[seluard]

I agree.. what do you think if we still follow the same approach, and when we enable this new flag it returns the closer limits per unit? so we can replace  LIMIT_ALL_DESCRIPTORS_HEADERS_ENABLED -> LIMIT_PER_UNIT_HEADERS_ENABLED ?

So avoid duplication. Working on it

[nacx]

The chosen approach LGTM. After a second thought, returning descriptor names, etc., could not be desirable in all cases, as it could leak some internal structure/info that is not desired.

[nacx]

thanks! Overall LGTM

[seluard]

Thanks for the approval @psbrar99 ! ❤️ is there any specific time or schedule to get this merge?