Merge branch 'rate-limit/limiter-peek' into 'master' (ae9140c5) · Commits · GitLab.org / ruby / gems / labkit-ruby

lib/labkit/rate_limit/evaluator.rb

+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",

lib/labkit/rate_limit/limiter.rb

+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)

spec/labkit/rate_limit/evaluator_spec.rb

+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)

spec/labkit/rate_limit/limiter_spec.rb

+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