Track bypass header traffic through labkit metrics

Hardik Galarequested to merge
hgala/29032-application-rate-limiter-bypass-visibility into master

Bypass traffic for GITLAB_THROTTLE_BYPASS_HEADER was invisible to labkit Prometheus metrics and Redis counters - a blind spot during the ongoing labkit rate-limiting migration. This change routes bypass traffic through labkit rule evaluation so it's counted in metrics while remaining never-blocked.

What does this MR do

  • Prepends a synthetic :skip labkit rule ahead of each registered rate limit's real rule. The rule is generated once per key at limiter-build time (not duplicated per registry entry) and named "#{key}_bypass" to distinguish each rate limit's bypass volume in Prometheus/Grafana.
  • LabkitAdapter now copies a bypass_header_set fact onto the labkit identifier when the caller supplies it, allowing the synthetic rule to match.
  • Gitlab::ApplicationRateLimiter#throttled? gained a bypass_header_set: keyword (default false).
  • Gitlab::ApplicationRateLimiter#throttled_request? removes its early return for bypass traffic; it now passes bypass_header_set: through to throttled?, so bypass traffic flows into labkit and is counted via gitlab_labkit_rate_limiter_calls_total{action="skip"} - remaining never blocked.
  • EE rate limits are covered automatically, since EE only extends rule definitions, not limiter-build logic.

Why :skip, not :allow or :log

The original issue proposed an :allow action, which doesn't exist in labkit's Rule class-only :limit, :log, and :skip are supported. This was corrected during discussion with @reprazent, the rate-limiting maintainer. The immediate goal is purely counting bypass traffic, which :skip achieves (increments Prometheus counter, no Redis write, terminates before the real rule runs). Per-limit "would this have been throttled" visibility would need :log rules, which is a planned future iteration, not part of this change.

Known tradeoff (reviewed and accepted)

Because the bypass match now happens inside labkit's rule evaluation (downstream of throttled?'s existing key-registration and settings-lookup checks), bypass-header requests are no longer immune to exceptions from those checks the way the old early-return was. Per Bob's analysis, those checks validate the rate limiter's configuration-a property of the code path shared with non-bypass traffic on the same endpoint-not something bypass traffic is uniquely exposed to. No code change was made for this; it's noted so reviewers don't need to independently reach the same conclusion.

Testing

  • New/updated specs in spec/lib/gitlab/application_rate_limiter/labkit_adapter/supported_rate_limits_spec.rb, spec/lib/gitlab/application_rate_limiter/labkit_adapter_spec.rb, and spec/lib/gitlab/application_rate_limiter_spec.rb
  • EE specs in ee/spec/lib/ee/gitlab/application_rate_limiter/labkit_adapter/supported_rate_limits_spec.rb
  • Coverage includes: bypass traffic remains never-blocked even when the real rule is already over its limit; the real rule's Redis counter is untouched when bypassed; the skip rule is generated per key and ordered ahead of the real rule (verified via both #check and #peek); the public bypass_header_set: kwarg on throttled?; EE-registered keys behave identically.

References

gitlab-com/gl-infra/production-engineering#29032 (closed)

Merge request reports