Keep each labkit web throttle on one Redis counter

Max Woolfrequested to merge
29362-web-throttle-parity-labkit into master

What does this MR do and why?

Each labkit web throttle split its Rack::Attack counter across two Redis keys. Rack::Attack keeps one counter per person per throttle; "is this a web request?" means web_request? || frontend_request?, and since a labkit rule can only AND conditions, the disjunction was written as two rules (*_web + *_web_frontend). Labkit keys each Redis counter by rule name, so one person's web traffic was divided between two counters and neither filled up.

The split is close to 50/50 in practice (~46% of authenticated web traffic lands on the frontend counter on gprd), roughly the worst case: a person must send close to double the limit before either counter trips. Measured per-day on gprd, labkit blocks only 0.65-0.83× Rack::Attack's volume on throttle_unauthenticated_web and 0.15-0.33× on throttle_authenticated_web - both far below the 0.95 per-throttle rollout gate, and web is the only throttle family where labkit blocks less (the API family runs 1.12-1.33×). At cutover this would mean roughly 117k requests/day that Rack::Attack blocks on throttle_unauthenticated_web sailing through. Full metrics on the issue.

The fix

  • Compute the disjunction once, before the rules run, as a single web_or_frontend fact on ClassifiedRequest. Each web throttle is then exactly one rule with one Redis counter, as Rack::Attack keeps it.
  • The general API rules gain frontend: false, the native form of the API predicates' unconditional !frontend_request? exclusion. Previously the web rules' ordering provided the exclusion, but only while a web throttle setting was on; Rack::Attack excludes frontend traffic from the API throttles regardless of settings.
  • Remove the now-unused sibling-entry (:throttle) indirection in the registry.

The rack shadow's enforce flags are off in production, so this changes shadow measurement only; no user-facing behaviour changes, hence no changelog.

What this does not fix

Requests Rack::Attack counts under two throttles at once (collector + web; frontend traffic on specialized API paths) are still counted under only the first matching labkit rule. Known and accepted for Stage 2b, pinned by tests (including a new request spec here), tracked in gitlab-com/gl-infra/production-engineering#29363 (closed).

References

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist.

Edited by Max Woolf

Merge request reports