Verified Commit f06fd459 authored by Max Woolf's avatar Max Woolf Committed by GitLab
Browse files

Add Limiter#peek read-without-increment API

parent 4674c6a9
Loading
Loading
Loading
Loading
+49 −0
Original line number Diff line number Diff line
@@ -29,6 +29,17 @@ module Labkit
        Result.new(matched: false, error: true, action: :allow)
      end

      # Read-without-increment counterpart to {#check}. Same matching and Result
      # shape; the underlying Redis counter is not mutated and the TTL is not
      # extended. A missing Redis key is treated as count=0 (matched, not exceeded).
      def peek(identifier)
        peek_rules(identifier)
      rescue StandardError => e
        report_error_metrics
        log_error(e, identifier)
        Result.new(matched: false, error: true, action: :allow)
      end

      private

      def check_rules(identifier)
@@ -44,6 +55,16 @@ module Labkit
        Result.new(matched: false, action: :allow)
      end

      def peek_rules(identifier)
        @rules.each do |rule|
          next unless rule_matches?(rule, identifier)

          return peek_rule(rule, identifier)
        end

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

      def rule_matches?(rule, identifier)
        rule.match.all? { |key, value| identifier[key] == value }
      end
@@ -54,6 +75,19 @@ module Labkit
        resolved_period = Integer(resolve_value(rule.period))

        count, ttl = incr_with_ttl(redis_key, resolved_period)
        build_result(rule, resolved_limit, resolved_period, count, ttl)
      end

      def peek_rule(rule, identifier)
        redis_key = build_redis_key(rule, identifier)
        resolved_limit = Integer(resolve_value(rule.limit))
        resolved_period = Integer(resolve_value(rule.period))

        count, ttl = read_with_ttl(redis_key)
        build_result(rule, resolved_limit, resolved_period, count, ttl)
      end

      def build_result(rule, resolved_limit, resolved_period, count, ttl)
        exceeded = count > resolved_limit
        action = exceeded ? rule.action : :allow
        info = Result::Info.new(
@@ -108,6 +142,21 @@ module Labkit
        end
      end

      # Pipelined GET + TTL. No EXPIRE: peek must not extend the window.
      # A missing key (GET => nil, TTL => -2) is reported as count=0; the
      # build_result fallback then derives reset_at from the rule period
      # since there is no Redis-side window to read.
      def read_with_ttl(redis_key)
        @redis.with do |conn|
          raw_count, ttl = conn.pipelined do |pipe|
            pipe.get(redis_key)
            pipe.ttl(redis_key)
          end
          count = raw_count.nil? ? 0 : Integer(raw_count)
          [count, ttl]
        end
      end

      def log_error(error, identifier)
        @logger.warn(
          message: "rate_limit_error",
+19 −0
Original line number Diff line number Diff line
@@ -38,6 +38,25 @@ module Labkit
        @evaluator.check(id)
      end

      # Read the current rate-limit state without incrementing the counter.
      # Mirrors {#check} except the underlying counter is not mutated and the
      # TTL is not extended. Useful for "have we already throttled this caller?"
      # checks where the caller has another path that does the actual increment
      # (typical pattern: peek to gate a side-effect, then call #check on the
      # path that should count).
      #
      # When the underlying Redis key does not exist yet, the result reports
      # count=0, exceeded=false, and remaining=resolved_limit; matched? is
      # still true because the rule applied. On Redis error the result fails
      # open identically to {#check}.
      #
      # @param identifier [Identifier, Hash] caller attributes for this request
      # @return [Result]
      def peek(identifier)
        id = identifier.is_a?(Identifier) ? identifier : Identifier.new(identifier)
        @evaluator.peek(id)
      end

      private

      def validate_name!(name)
+169 −0
Original line number Diff line number Diff line
@@ -380,6 +380,175 @@ RSpec.describe Labkit::RateLimit::Evaluator do
    end
  end

  describe "#peek" do
    let(:peek_rule) { make_rule(name: "peek_rule", limit: 5, period: 60) }

    before do
      allow(pipe).to receive(:get)
      allow(pipe).to receive(:ttl)
    end

    describe "Redis interaction" do
      it "calls GET and TTL pipelined and never INCR or EXPIRE" do
        expected_key = "labkit:rl:rack_request:peek_rule:user:42"
        allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return(["3", 30])

        expect(pipe).to receive(:get).with(expected_key)
        expect(pipe).to receive(:ttl).with(expected_key)
        expect(pipe).not_to receive(:incr)
        expect(raw_redis).not_to receive(:expire)

        evaluator(rules: [peek_rule]).peek(identifier)
      end
    end

    describe "missing Redis key" do
      before do
        allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return([nil, -2])
      end

      it "reports count=0 with matched=true and exceeded=false" do
        result = evaluator(rules: [peek_rule]).peek(identifier)

        expect(result.matched?).to be(true)
        expect(result.exceeded?).to be(false)
        expect(result.info.count).to eq(0)
        expect(result.info.remaining).to eq(5)
      end

      it "falls back to rule period for reset_at when ttl is -2 (key missing)" do
        freeze_time = Time.now.utc
        allow(Time).to receive(:now).and_return(freeze_time)

        result = evaluator(rules: [peek_rule]).peek(identifier)

        expect(result.info.reset_at).to eq(freeze_time.utc + 60)
      end
    end

    describe "existing Redis key" do
      it "returns count under threshold as not exceeded" do
        allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return(["3", 30])

        result = evaluator(rules: [peek_rule]).peek(identifier)

        expect(result.exceeded?).to be(false)
        expect(result.info.count).to eq(3)
        expect(result.info.remaining).to eq(2)
      end

      it "returns count at the threshold as not exceeded (boundary)" do
        allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return(["5", 30])

        result = evaluator(rules: [peek_rule]).peek(identifier)

        expect(result.exceeded?).to be(false)
        expect(result.info.remaining).to eq(0)
      end

      it "returns count above the threshold as exceeded with the rule's action" do
        blocking_rule = make_rule(name: "blocking", limit: 5, action: :block)
        allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return(["8", 30])

        result = evaluator(rules: [blocking_rule]).peek(identifier)

        expect(result.exceeded?).to be(true)
        expect(result.action).to eq(:block)
        expect(result.info.count).to eq(8)
        expect(result.info.remaining).to eq(0)
      end

      it "uses the pipelined ttl for reset_at when ttl > 0" do
        allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return(["3", 45])
        freeze_time = Time.now.utc
        allow(Time).to receive(:now).and_return(freeze_time)

        result = evaluator(rules: [peek_rule]).peek(identifier)

        expect(result.info.reset_at).to eq(freeze_time.utc + 45)
      end
    end

    describe "non-matching rule" do
      it "does not touch Redis and returns matched=false" do
        non_matching = make_rule(match: { user: 999 })
        expect(raw_redis).not_to receive(:pipelined)

        result = evaluator(rules: [non_matching]).peek(identifier)

        expect(result.matched?).to be(false)
        expect(result.action).to eq(:allow)
      end
    end

    describe "error path" do
      it "returns an error Result and fails open when Redis raises" do
        allow(raw_redis).to receive(:pipelined).and_raise(RuntimeError, "connection refused")
        logger = instance_double(Labkit::Logging::JsonLogger)
        expect(logger).to receive(:warn).with(
          hash_including(message: "rate_limit_error", error: "RuntimeError")
        )

        result = described_class.new(
          name: "rack_request", rules: [peek_rule], redis: redis, logger: logger
        ).peek(identifier)

        expect(result.error?).to be(true)
        expect(result.matched?).to be(false)
        expect(result.exceeded?).to be(false)
      end
    end

    describe "callable limit and period resolution" do
      it "resolves the lambda at peek time" do
        call_count = 0
        limit_lambda = lambda do
          call_count += 1
          5
        end
        callable_rule = make_rule(name: "callable", limit: limit_lambda)
        allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return(["3", 30])

        evaluator(rules: [callable_rule]).peek(identifier)

        expect(call_count).to eq(1)
      end
    end

    describe "metrics emission", :with_metrics_config do
      let(:metrics) { Labkit::RateLimit::Metrics }

      it "does not increment calls_total when matched (peek is observational)" do
        allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return(["3", 30])
        evaluator(rules: [peek_rule]).peek(identifier)

        expect(metrics.calls_total.get(rate_limiter: "rack_request", rule: "peek_rule", action: "allow")).to eq(0.0)
      end

      it "does not increment calls_total when no rule matches" do
        non_matching = make_rule(match: { user: 999 })
        evaluator(rules: [non_matching]).peek(identifier)

        expect(metrics.calls_total.get(rate_limiter: "rack_request", rule: "unmatched", action: "allow")).to eq(0.0)
      end

      it "does not set the limit or period gauges on a matched peek" do
        allow(raw_redis).to receive(:pipelined).and_yield(pipe).and_return(["1", 55])
        evaluator(rules: [peek_rule]).peek(identifier)

        expect(metrics.limit_gauge.get(rate_limiter: "rack_request", rule: "peek_rule")).to eq(0.0)
        expect(metrics.period_gauge.get(rate_limiter: "rack_request", rule: "peek_rule")).to eq(0.0)
      end

      it "increments errors_total on Redis failure (shared with check)" do
        allow(raw_redis).to receive(:pipelined).and_raise(RuntimeError, "down")
        evaluator(rules: [peek_rule]).peek(identifier)

        expect(metrics.errors_total.get(rate_limiter: "rack_request")).to eq(1.0)
      end
    end
  end

  describe "Scenario Q: no per-request logging in the success path" do
    it "does not call the logger when a rule matches and Redis is healthy" do
      rule = make_rule(name: "r", limit: 10, period: 60)
+42 −0
Original line number Diff line number Diff line
@@ -109,6 +109,48 @@ RSpec.describe Labkit::RateLimit::Limiter do
    end
  end

  describe "#peek" do
    let(:peek_pipe) { instance_double(Redis, get: nil, ttl: nil) }

    before do
      allow(raw_redis).to receive(:pipelined).and_yield(peek_pipe).and_return([nil, -2])
    end

    it "delegates to the evaluator's peek path" do
      lim = limiter
      expect(lim.instance_variable_get(:@evaluator)).to receive(:peek).with(instance_of(Labkit::RateLimit::Identifier))
      lim.peek({ user: 42 })
    end

    it "accepts an Identifier instance directly without re-wrapping" do
      id = Labkit::RateLimit::Identifier.new(user: 42)
      expect(Labkit::RateLimit::Identifier).not_to receive(:new)
      limiter.peek(id)
    end

    it "returns a fully-populated Result without calling INCR or EXPIRE" do
      expect(raw_redis).not_to receive(:expire)
      expect(peek_pipe).not_to receive(:incr)
      freeze_time = Time.now.utc
      allow(Time).to receive(:now).and_return(freeze_time)
      r = rule

      result = limiter(rules: [r]).peek({ user: 42 })

      expect(result).to eq(Labkit::RateLimit::Result.new(
        matched: true,
        exceeded: false,
        action: :allow,
        rule: r,
        error: false,
        info: Labkit::RateLimit::Result::Info.new(
          resolved_limit: 100, resolved_period: 60,
          count: 0, remaining: 100, reset_at: freeze_time + 60
        )
      ))
    end
  end

  describe "Scenario A: evaluator is reused across checks" do
    it "returns the same evaluator object on repeated calls" do
      lim = limiter