Isolate webhook delivery per destination and skip futile retries

[LarsLaskowski]

Pull Request

📖 Description

Fix (robustness/performance), no breaking change.

Two failure modes in internal/alert/notify.go combined to make webhook delivery degrade far worse than necessary when a single endpoint went bad:

1. Non-retryable responses burned the full retry budget. deliver treated every non-2xx alike, so a permanent 400/404/410 — a revoked webhook URL, a payload the endpoint rejects — was re-POSTed up to notify_max_retries more times with exponential backoff, even though an identical request can never be accepted.
2. A single serial worker drained the shared queue. That wasted budget (and any merely slow endpoint) head-of-line-blocked every other webhook and every queued event, for up to (retries+1) × timeout + backoffs per event.

This PR addresses both halves of the suggested fix in the issue:

• Per-destination isolation. Each webhook now gets its own bounded queue (defaultNotifyQueueSize = 256, unchanged per destination), its own worker goroutine, and its own rate-limit state (webhookWorker). A webhook pointing at a dead host can now only delay — and only ever drop — its own events. The severity filter (min_level) moves into Notify, where it is applied inline while fanning the event out; it is pure and cheap, so it costs the collector nothing. Because each worker owns its lastSent map exclusively, the "no lock needed" property is preserved.
• Error classification. post now returns a typed *statusError, and retryable decides whether another attempt is worth making: transport failures (DNS, refused connections, timeouts) and 5xx stay retryable, as do 408 Request Timeout and 429 Too Many Requests, which explicitly invite a retry. Every other 4xx ends the attempt immediately with a distinct alert notification rejected, not retrying log line. 3xx (which the client would normally have followed) is left retryable, i.e. unchanged behavior.

Ordering within a webhook, the queue-full drop valve, the cleared-bypasses-rate-limit rule, and the "failed delivery doesn't stamp the rate limiter" fix from #62 are all preserved.

🎫 Issues

Closes #63

👩‍💻 Reviewer Notes

• Worth a close look: the rate limiter's key no longer includes the URL (rateLimitKey is now metric\x00resource) because the state lives on the per-webhook worker instead of one shared map. TestNotifier_RateLimitStateIsPerWebhook pins that two webhooks no longer share a limiter.
• The severity filter moving from dispatch (worker side) to Notify (caller side) means a filtered-out event never occupies queue space — a small side benefit, but it does mean Notify now does slightly more work on the collector's goroutine. eventReaches is a pure comparison of two levels, so this is a few nanoseconds per (event × webhook).
• No smoke test on hardware is needed: this is pure Go networking logic with no /proc, /sys, or Pi-specific code paths. It is fully exercised by httptest servers.

📑 Test Plan

Added to internal/alert/notify_test.go, following docs/TESTS.md conventions (Test<Subject>_<Scenario>, httptest fixtures, no real /proc//sys access):

• TestNotifier_DoesNotRetryPermanentClientError — a 404 endpoint with notify_max_retries: 3 must be hit exactly once. Verified failing against the pre-fix implementation (got 4).
• TestNotifier_SlowWebhookDoesNotBlockOthers — one webhook hangs until released, a second must still receive its delivery. Verified failing against the pre-fix implementation (timed out waiting for delivery).
• TestNotifier_RetriesRetryableClientError — 429 must still consume the full retry budget (1 + 2 retries), guarding against over-broad 4xx suppression.
• TestNotifier_RateLimitStateIsPerWebhook — the same event reaches both destinations despite a 60 s notify_min_interval_seconds.
• TestRetryable — table test over the classification: transport error, 400/404/410/403-wrapped, 408, 429, 500, 503, 304.

Existing notifier tests (payload, template, content type, min-level filtering, retry-then-give-up, rate limiting, cleared bypass, #62 regression) pass unchanged.

Full suite locally:

• go build ./... — clean
• go vet ./... — clean
• go test ./... -race -cover — all packages pass; internal/alert coverage 93.1%
• golangci-lint run — 0 issues

✅ Checklist

General

• I have added/updated tests for my changes (go test ./... -race -cover passes locally).
• go vet ./... and golangci-lint run are clean.
• I have tested my changes.
• I have read the CONTRIBUTING documentation and followed the project's code style guidelines.
• I have updated ARCHITECTURE.md if this changes a documented design decision.

REST API / configuration / packaging

Not applicable — no REST API, configuration, or packaging change. Existing notify_max_retries, notify_retry_backoff_seconds, and notify_min_interval_seconds keep their meaning; the retry budget is simply no longer spent on responses that cannot change.

⏭ Next Steps

None required. A possible follow-up (out of scope here, and not requested by the issue) would be honoring a Retry-After header on 429/503 instead of the fixed exponential backoff.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
93.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud