Verified Commit 8b004d37 authored by Bob Van Landuyt's avatar Bob Van Landuyt 💬
Browse files

refactor: remove per-request logging from `Limiter#check` 

Logging on every rate-limited check was deliberately removed in
!272 to avoid doubling log volume. The `rate_limit_check` warn
message added in !271 reintroduced per-request logging in the
hot path.

Remove the warn call from `#check` and the associated specs.
Observability for rate limit checks will be added back via:

- Prometheus metrics:
  gitlab-com/gl-infra/production-engineering#28798
- Structured fields in existing per-request logs:
  gitlab-com/gl-infra/production-engineering#28799
parent 8eea3315

lib/labkit/rate_limit/limiter.rb

+3 −16
Original line number Diff line number Diff line
@@ -21,11 +21,10 @@ module Labkit 


      def initialize(name:, rules:, redis: nil, logger: nil)

        @logger = logger || RateLimit.config.logger || Labkit::Logging::JsonLogger.new($stdout)

        validated_name = validate_name!(name)

        @name = validated_name

        @name = validate_name!(name)



        @evaluator = Evaluator.new(

          name: validated_name,

          name: @name,

          rules: prepare_rules(rules),

          redis: redis || RateLimit.config.redis,

          logger: @logger
@@ -36,19 +35,7 @@ module Labkit 
      # @return [Result]

      def check(identifier)

        id = identifier.is_a?(Identifier) ? identifier : Identifier.new(identifier)

        result = @evaluator.check(id)



        if result.exceeded? && result.action == :block

          @logger.warn(

            message: "rate_limit_check",

            name: @name,

            rule_name: result.rule.name,

            exceeded: true,

            severity: "WARN"

          )

        end



        result

        @evaluator.check(id)

      end



      private

spec/labkit/rate_limit/limiter_spec.rb

+0 −25
Original line number Diff line number Diff line
@@ -117,31 +117,6 @@ RSpec.describe Labkit::RateLimit::Limiter do 
    end

  end



  describe "Scenario B: exceeded :block rule emits rate_limit_check WARN" do

    it "logs warn with rule_name and exceeded: true" do

      allow(redis).to receive(:incr).and_return(101)

      lim = limiter

      lim.check({ user: 42 })

      expect(logger).to have_received(:warn).with(

        hash_including(

          message: "rate_limit_check",

          rule_name: "default",

          exceeded: true

        )

      )

    end

  end



  describe "Scenario C: exceeded :log rule does not emit rate_limit_check WARN" do

    it "does not call warn with rate_limit_check message" do

      allow(redis).to receive(:incr).and_return(101)

      r = rule(action: :log)

      lim = limiter(rules: [r])

      lim.check({ user: 42 })

      expect(logger).not_to have_received(:warn).with(hash_including(message: "rate_limit_check"))

    end

  end



  describe "prepare_rules: rule name deduplication and sanitization" do

    context "when in dev/test environment" do

      it "raises ArgumentError for duplicate rule names" do