ratelimits: Fix cardinality explosion in overrideUsageGauge #7548
Conversation
aarongable
requested review from
a team and
pgporada
and removed request for
a team
| for _, entry := range v.Ids { | ||
| limit := v.limit |
There was a problem hiding this comment.
Since v.limit doesn't vary from loop to loop, it would make more sense to do this assignment outside the loop. If you move it all the way to the top of the containing loop (L155), you can also replace some other instances of v.limit with just limit (which I believe is the purpose of the assignment, to create a shorthand).
There was a problem hiding this comment.
The purpose of this assignment is to create a copy-by-value of the limit, so that we can set fields which do vary per-loop (namely the overrideKey) without affecting the v.limit object from the parent loop.
There was a problem hiding this comment.
This was one of those things that didn't need to be fixed but could certainly lead to some unpleasantness if someone were to refactor it at a later date. So we decided to fix it now.
No description provided.