Commit 0cf76147 authored by Sam Wiskow's avatar Sam Wiskow
Browse files

chore(rate_limit): merge master into spec-10-regex-matcher

Resolves a conflict in evaluator_spec.rb where master added three
describe blocks (metrics emission, Stage 1c remaining/reset_at,
Scenario Q no per-request logging) at the end of the file alongside
the regex-matcher scenarios from this branch. Both stay; master's
blocks first, regex blocks after.

Adapts the regex blocks to master's pooled-Redis fixture
(`pipe.incr` + `raw_redis.expire` via `raw_redis.pipelined`); the
default before-block stub returns `[1, 55]`, and the regex tests
assert against `pipe`/`raw_redis` to match.

Co-Authored-By: default avatarClaude Opus 4.7 (1M context) <noreply@anthropic.com>
parents 2f4bc36a 4674c6a9
Loading
Loading
Loading
Loading
+3 −3
Original line number Diff line number Diff line
@@ -19,13 +19,13 @@ include:
  # It includes standard checks, gitlab-scanners, validations and release processes
  # common to all projects using this template library.
  # see https://gitlab.com/gitlab-com/gl-infra/common-ci-tasks/-/blob/main/templates/standard.md
  - component: $CI_SERVER_FQDN/gitlab-com/gl-infra/common-ci-tasks/standard-build@v3.12
  - component: $CI_SERVER_FQDN/gitlab-com/gl-infra/common-ci-tasks/standard-build@v3.24

  # Runs rspec tests and rubocop on the project
  # see https://gitlab.com/gitlab-com/gl-infra/common-ci-tasks/-/blob/main/templates/ruby.md
  - component: $CI_SERVER_FQDN/gitlab-com/gl-infra/common-ci-tasks/ruby-build@v3.12
  - component: $CI_SERVER_FQDN/gitlab-com/gl-infra/common-ci-tasks/ruby-build@v3.24

  - component: $CI_SERVER_FQDN/gitlab-com/gl-infra/common-ci-tasks/danger@v3.12
  - component: $CI_SERVER_FQDN/gitlab-com/gl-infra/common-ci-tasks/danger@v3.24

ruby-versions:
  extends: rspec
+1 −1
Original line number Diff line number Diff line
@@ -25,7 +25,7 @@ repos:
  # Documentation available at
  # https://gitlab.com/gitlab-com/gl-infra/common-ci-tasks/-/blob/main/docs/pre-commit.md
  - repo: https://gitlab.com/gitlab-com/gl-infra/common-ci-tasks
    rev: v3.12  # renovate:managed
    rev: v3.24  # renovate:managed

    hooks:
      - id: shellcheck  # Run shellcheck for changed Shell files
+2 −1
Original line number Diff line number Diff line
@@ -6,7 +6,7 @@ module Labkit
  #
  # @example Configuration (e.g. in a Rails initializer)
  #   Labkit::RateLimit.configure do |c|
  #     c.redis  = Redis.current
  #     c.redis  = ConnectionPool.new { Redis.new }  # must respond to .with { |conn| }
  #     c.logger = Labkit::Logging::JsonLogger.new($stdout)
  #   end
  #
@@ -24,6 +24,7 @@ module Labkit
    autoload :Rule, "labkit/rate_limit/rule"
    autoload :Evaluator, "labkit/rate_limit/evaluator"
    autoload :Limiter, "labkit/rate_limit/limiter"
    autoload :Metrics, "labkit/rate_limit/metrics"

    class << self
      def configure
+55 −9
Original line number Diff line number Diff line
@@ -24,8 +24,9 @@ module Labkit
      rescue StandardError => e
        # Intentionally broad: fail-open applies to any unexpected error (network,
        # timeout, OOM) not only Redis protocol errors.
        report_error_metrics
        log_error(e, identifier)
        Result.new(matched: false, error: true)
        Result.new(matched: false, error: true, action: :allow)
      end

      private
@@ -34,10 +35,13 @@ module Labkit
        @rules.each do |rule|
          next unless rule_matches?(rule, identifier)

          return evaluate_rule(rule, identifier)
          result = evaluate_rule(rule, identifier)
          report_matched_metrics(result)
          return result
        end

        Result.new(matched: false)
        report_unmatched_metrics
        Result.new(matched: false, action: :allow)
      end

      def rule_matches?(rule, identifier)
@@ -49,10 +53,17 @@ module Labkit
        resolved_limit = Integer(resolve_value(rule.limit))
        resolved_period = Integer(resolve_value(rule.period))

        count = incr_with_ttl(redis_key, resolved_period)
        count, ttl = incr_with_ttl(redis_key, resolved_period)
        exceeded = count > resolved_limit
        action = exceeded ? rule.action : :allow
        info = Result::Info.new(
          resolved_limit: resolved_limit, resolved_period: resolved_period,
          count: count,
          remaining: [resolved_limit - count, 0].max,
          reset_at: Time.now.utc + (ttl >= 0 ? ttl : resolved_period)
        )

        Result.new(matched: true, exceeded: exceeded, action: rule.action, rule: rule)
        Result.new(matched: true, exceeded: exceeded, action: action, rule: rule, info: info)
      end

      def build_redis_key(rule, identifier)
@@ -83,11 +94,18 @@ module Labkit
        end
      end

      # Pipelines INCR and TTL so both are fetched in a single round-trip.
      # EXPIRE follows as a separate call only on first write (count == 1).
      # On first write TTL will be -1 (expiry not yet set); callers fall back to period.
      def incr_with_ttl(redis_key, period)
        count = @redis.incr(redis_key)
        # Set expiry only on first write to avoid resetting TTL on each call
        @redis.expire(redis_key, period) if count == 1
        count
        @redis.with do |conn|
          count, ttl = conn.pipelined do |pipe|
            pipe.incr(redis_key)
            pipe.ttl(redis_key)
          end
          conn.expire(redis_key, period) if count == 1
          [count, ttl]
        end
      end

      def log_error(error, identifier)
@@ -98,6 +116,34 @@ module Labkit
          identifier: identifier&.to_h
        )
      end

      def report_matched_metrics(result)
        Metrics.calls_total.increment(
          rate_limiter: @name,
          rule: result.rule.name,
          action: result.action.to_s
        )
        Metrics.limit_gauge.set(
          { rate_limiter: @name, rule: result.rule.name },
          result.info.resolved_limit
        )
        Metrics.period_gauge.set(
          { rate_limiter: @name, rule: result.rule.name },
          result.info.resolved_period
        )
      end

      def report_unmatched_metrics
        Metrics.calls_total.increment(
          rate_limiter: @name,
          rule: "unmatched",
          action: "allow"
        )
      end

      def report_error_metrics
        Metrics.errors_total.increment(rate_limiter: @name)
      end
    end
  end
end
+43 −0
Original line number Diff line number Diff line
# frozen_string_literal: true

module Labkit
  module RateLimit
    module Metrics
      module_function

      def calls_total
        Labkit::Metrics::Client.counter(
          :gitlab_labkit_rate_limiter_calls_total,
          'Total number of successful rate limit checks',
          { rate_limiter: nil, rule: nil, action: nil }
        )
      end

      def errors_total
        Labkit::Metrics::Client.counter(
          :gitlab_labkit_rate_limiter_errors_total,
          'Total number of rate limit check errors',
          { rate_limiter: nil }
        )
      end

      def limit_gauge
        Labkit::Metrics::Client.gauge(
          :gitlab_labkit_rate_limiter_limit,
          'The configured rate limit threshold',
          { rate_limiter: nil, rule: nil },
          :max
        )
      end

      def period_gauge
        Labkit::Metrics::Client.gauge(
          :gitlab_labkit_rate_limiter_period_seconds,
          'The configured rate limit period in seconds',
          { rate_limiter: nil, rule: nil },
          :max
        )
      end
    end
  end
end
Loading