rate_limit: don't update metrics when a selector has no metrics: block

[moonchen]

A rate_limit selector configured without a metrics: block should be a metrics no-op. It isn't.

_metrics is value-initialized to 0, but incrementMetric() only suppresses the update when an entry equals the TS_ERROR (−1) sentinel that metric_helper() uses for "not registered":

std::array<int, RATE_LIMITER_METRIC_MAX> _metrics{};   // value-initialized to 0

void incrementMetric(uint metric) {
  if (_metrics[metric] != TS_ERROR) {                  // 0 != -1 → true → does NOT skip
    TSStatIntIncrement(_metrics[metric], 1);           // increments an unregistered ID
  }
}

_metrics is only populated by initializeMetrics(), which parseYaml() calls only when a selector has a metrics: block. Without that block the guard never fires, so every queue/reject/expire/resume (sni_limiter.cc, sni_selector.cc, txn_limiter.cc) calls TSStatIntIncrement() on an unregistered ID. It currently lands on the reserved proxy.process.api.metrics.bad_id slot (id 0), so it's absorbed rather than fatal — but the plugin still shouldn't be emitting anything.

Fix

Default _metrics to TS_ERROR in the constructor — the same "not registered" marker metric_helper() writes per element — so incrementMetric() stays a no-op until metrics are actually registered.

Testing

The equivalent fix was validated under AddressSanitizer on a 10.x branch. Not built against master locally — relying on CI for the compile.

[Copilot]

Pull request overview

This PR fixes a subtle metrics mis-accounting issue in the experimental rate_limit plugin by ensuring the RateLimiter’s metric ID array starts in an explicit “not registered” state, making metric increments a no-op unless metrics are configured and registered.

Changes:

• Initialize RateLimiter::_metrics to TS_ERROR in the default constructor via _metrics.fill(TS_ERROR).
• Add an explanatory comment documenting why TS_ERROR is required to avoid silently incrementing metric id 0 when selectors omit a metrics: block.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.