fix(exporters): keep limiting after the first dropped alert

[AlonLiwsky]

Summary

The per-minute alert limit stops limiting after its first drop, so it drops exactly one alert
per minute and sends every later one. In practice there is no limit.

shouldSendLimitAlert returns count > max && !isNotified — one value carrying two decisions.
The first alert past the limit is dropped and sets isNotified, which makes the condition false
for every later alert in that minute, so they are all sent.

Ticket

None — this repository has no ticket link.

Changes

• admitAlert replaces shouldSendLimitAlert and returns the two decisions separately:
  admitted is false for every alert past the limit, notify is true only for the first one.
• Both call sites drop the alert whenever it is not admitted, and send the AlertLimitReached
  notice only on the first. ReportAlertSuppressed(..., "rate_limit") now fires for every
  dropped alert instead of once per minute, so that metric becomes usable.
• A limit of zero or less means no limit. Validate() already replaces a zero with the default,
  so this only guards an exporter built by hand.
• The window reset no longer swallows the alert that triggers it. The old code returned early
  without counting, so the effective limit was max + 1.
• sendAlertLimitReached no longer sets the notified flag — admitAlert owns that decision —
  and no longer reads count and startTime without the lock.
• New docs/features/alert-rate-limit.md.

Testing

go build ./pkg/exporters/ for GOOS=linux → pass. New tests in
pkg/exporters/alert_limit_test.go: the limiter keeps limiting after its first drop, the window
resets, zero means no limit, and twenty concurrent senders admit exactly the limit while the
notice is decided once. Run on Linux with -race → pass, together with the existing
TestSendRuleAlertRateReached.

AI Review

Local review with armosec-shared-rules:code-review-standards (Opus, high effort) over the
sibling change in the private agent, which carries the identical fix. Verdict: the limiter logic
is correct, the mutex covers the whole decision, and all call sites are consistent. Its one
finding was about the consequence rather than the code — a real limit can starve one alert kind
when consumers share an exporter — which is why ReportAlertSuppressed now counts every drop.

AI-skills: armosec-shared-rules:agent-dispatch-policy

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[matthyx]

Reviewed the logic in pkg/exporters/http_exporter.go independently of the PR description's own AI-review claim.

Correctness: looks right. Traced admitAlert() by hand against the bug it fixes:

• Old code: count > max && !isNotified conflated "drop this alert" and "send the notice" into one bool. The first drop set isNotified, which flipped the condition to false for every later alert in the window — so the limiter dropped exactly one alert per minute and let the rest through.
• New code separates admitted (false for every send past the limit) from notify (true only once, on the first refusal), decided under alertMetrics's mutex for the whole operation. Verified: window reset now counts the triggering alert (fixes the old max+1 effective limit), sendAlertLimitReached no longer double-sets isNotified or reads count/startTime unlocked, and MaxAlertsPerMinute <= 0 correctly short-circuits to "no limit" (matches Validate()'s zero→default behavior for the common path, and still guards a hand-built exporter with a negative value).
• Existing TestSendRuleAlertRateReached still passes under the new semantics (traced by hand: 1st call admitted, 2nd call not admitted + notify). New tests in alert_limit_test.go cover the keep-limiting-after-drop behavior, window reset, zero-means-no-limit, and a 20×20 concurrent-sender case pinning admitted==limit and notified==1 — good coverage for a mutex-guarded counter.
• Confirmed no other call sites reference the old shouldSendLimitAlert name (searched main); the rename is clean.

Blockers before this can merge:

1. DCO check is failing (action_required). The commit 444b0432 has no Signed-off-by trailer. This needs git commit --amend -s (and a force-push) to pass.
2. PR is still a draft — needs to be marked "Ready for review" before it's mergeable regardless of review state.

No code changes requested — once the DCO sign-off is added and the PR is marked ready, this looks good to merge from a correctness standpoint.