Commit a7a6a159 authored by Ashwin S's avatar Ashwin S
Browse files

Added log for metric emission failures

parent 1de2c725
Loading
Loading
Loading
Loading
+5 −0
Original line number Diff line number Diff line
@@ -412,6 +412,11 @@ and remains the only error metric for `peek`, which emits no `checks_total`
`errors_total` must first decide where `peek` errors go — a dedicated peek
metric, or logs only.

Metric emission itself is best-effort: a failure in the metrics stack never
alters the verdict or breaks fail-open, and is logged at WARN with
`error_type: "rate_limit_metrics_error"` (once per evaluator per process, to
avoid flooding).

## Metrics

`Labkit::RateLimit::Metrics` emits the following Prometheus metrics through
+19 −2
Original line number Diff line number Diff line
@@ -431,12 +431,29 @@ module Labkit
          matched: result.matched?.to_s,
          error: (result.error? || result.degraded?).to_s
        )
      rescue StandardError
        nil
      rescue StandardError => e
        log_metrics_failure(e)
      end

      def report_error_metrics
        Metrics.errors_total.increment(rate_limiter: @name)
      rescue StandardError => e
        log_metrics_failure(e)
      end

      # Logged once per evaluator instance so a persistently broken metrics
      # stack stays visible without flooding the hot path, and never raises
      # (callers rescue precisely to protect the verdict and fail-open).
      def log_metrics_failure(error)
        return if @metrics_failure_logged

        @metrics_failure_logged = true
        @logger.warn(
          name: @name,
          Labkit::Fields::ERROR_TYPE => "rate_limit_metrics_error",
          Labkit::Fields::CLASS_NAME => error.class.to_s,
          Labkit::Fields::ERROR_MESSAGE => error.message
        )
      rescue StandardError
        nil
      end
+16 −0
Original line number Diff line number Diff line
@@ -625,6 +625,9 @@ RSpec.describe Labkit::RateLimit::Evaluator do

    it "logs a nil rule when the error is raised with no rule in flight" do
      logger = instance_double(Labkit::Logging::JsonLogger)
      # The allow absorbs the rate_limit_metrics_error warn from the rescue's
      # own swallowed checks_total failure.
      allow(logger).to receive(:warn)
      expect(logger).to receive(:warn).with(hash_including(rule: nil))

      # The unmatched calls_total emission and report_check_metrics both run
@@ -818,6 +821,19 @@ RSpec.describe Labkit::RateLimit::Evaluator do
      expect(result.error?).to be(false)
    end

    it "logs a metrics failure once per evaluator, not per check" do
      allow(Labkit::RateLimit::Metrics).to receive(:checks_total).and_raise("metrics down")

      logger = instance_double(Labkit::Logging::JsonLogger)
      expect(logger).to receive(:warn)
        .with(hash_including(Labkit::Fields::ERROR_TYPE => "rate_limit_metrics_error"))
        .once

      rule = make_rule(name: "api_rule", limit: 100, period: 60)
      ev = described_class.new(name: "rack_request", rules: [rule], redis: redis, logger: logger)
      2.times { ev.check(identifier) }
    end

    it "fails open without raising when errors_total emission itself fails" do
      allow(Labkit::RateLimit::Metrics).to receive(:errors_total).and_raise("metrics down")
      faulty = Class.new do