Notifications 5/6: delivery hardening — retry, dead-letter, retention, metrics

[LarsLaskowski]

Fifth stage of the notification feature set: make delivery survive an unreliable endpoint, stop retrying forever, keep the ledger from growing without bound, and make the subsystem observable. Depends on #220–#223.

Full design: notification concept

Context

After #222 and #223 the pipeline sends digests, but a failed send simply leaves records unsent until the next dispatch run. That is too naive in both directions: a permanently misconfigured endpoint retries every few minutes forever and floods the log, while a briefly unavailable endpoint gets no meaningful spacing between attempts. The ledger also grows monotonically, and nothing reports how the subsystem is doing.

Two retry layers exist by design and must not be confused: TransientHttpRetryHandler (#221) retries within a single request for 5xx/408/timeouts, like every other outbound client in the application. This issue adds the cross-run layer on top, which is where "the endpoint has been down for an hour" is handled.

Scope

1. Outbox retry with exponential backoff

When a digest send returns a non-Succeeded result, every record included in that batch is updated:

• Status = Retrying
• AttemptCount = AttemptCount + 1
• NextAttemptAtUtc = now + RetryBaseDelayMinutes × 2^(AttemptCount − 1) → 5, 10, 20, 40, 80 minutes with the defaults
• LastError set to the truncated ExternalOperationResult.Message (max 1024 characters, matching the column)

The delivery query in NotificationDispatchService selects Pending records plus Retrying records whose NextAttemptAtUtc <= now, which is exactly what the (Status, NextAttemptAtUtc) index from #220 supports. Records not yet due are skipped without touching the channel, so a backed-off endpoint is not hammered on every 5-minute tick.

Newly detected events that arrive while an earlier batch is backing off join the next due batch — the digest is rebuilt from whatever is due at send time rather than being frozen per batch.

2. Dead-lettering

When AttemptCount reaches MaxDeliveryAttempts (default 5), the record becomes DeadLettered:

• LastError persisted, NextAttemptAtUtc cleared
• a warning log via the NotificationLoggingExtensions event (EventId 3605) naming channel, kind, subject, and attempt count
• the failure metric is incremented

Dead-lettered records are never retried automatically and are never re-detected, because their (ChannelName, DedupKey) row still exists — a re-notification only happens naturally when the dedup key changes (e.g. a newer recommended version). This is a deliberate trade-off: silence about an event whose delivery permanently failed, in exchange for never entering an infinite retry loop. The dead-letter state is what the history page in #225 surfaces.

3. Retention in ScanCleanupBackgroundService

Extend src/DockerUpdateGuard/Images/ScanCleanupBackgroundService.cs — the one existing service this feature set modifies — with a NotificationRecords cleanup that follows the file's existing shape (materialize with ToListAsync, RemoveRange, single SaveChangesAsync, counts added to the completion log):

Delete records where

• Status ∈ { Sent, Suppressed, DeadLettered } (never Pending/Retrying — those are in flight)
• CreatedAtUtc < cutoff, reusing the existing Scanning:RetainScanRunsDays cutoff — no new retention option
• and the record does not belong to a still-active finding: UpdateFindingId/VulnerabilityFindingId either null or pointing at a finding with IsActive == false

That last clause is the important one. Without it, a CVE that stays active longer than the retention window would lose its ledger row and be re-notified as if it were new. The precedent is already in the file: old container snapshots are only removed when no active finding references them.

Extend ScanCleanupCompleted (Images/ImageHostLoggingExtensions.cs) with the removed-notification-record count.

4. Telemetry

In src/DockerUpdateGuard.Telemetry/TelemetryMetricNames.cs (constants added in #220) and src/DockerUpdateGuard/ApplicationTelemetry.cs:

Instrument	Type	Tags	Meaning
dockerupdateguard.notifications.sent	Counter	channel, kind	Records successfully delivered
dockerupdateguard.notifications.failed	Counter	channel, reason (transient/dead_lettered)	Failed delivery attempts
dockerupdateguard.notifications.pending	Observable gauge	—	Records awaiting delivery (Pending + Retrying)

Add RecordNotificationDelivery(...) alongside the existing RecordScanRun(...), and refresh the pending gauge inside RefreshInventoryMetricsAsync(dbContext, ct) next to the existing _activeUpdateFindings / _activeCveFindings gauges. Add the channel tag name to TelemetryTagNames.

Testing

Extend src/Tests/DockerUpdateGuard.Tests/NotificationDispatchServiceTests.cs:

• failing channel → all batch records become Retrying with AttemptCount = 1 and a NextAttemptAtUtc in the future
• consecutive failures → NextAttemptAtUtc gaps grow 5/10/20/40 minutes with default options
• records whose NextAttemptAtUtc is in the future are skipped: the channel is not called at all when nothing else is due
• reaching MaxDeliveryAttempts → DeadLettered with LastError set and NextAttemptAtUtc cleared; a later run neither retries nor re-detects it
• recovery: failure then success → records Sent with SentAtUtc and a shared DispatchBatchId, LastError retained or cleared per the implemented convention
• a newly detected event joins the next due batch alongside retrying records
• ExternalOperationStatus.NotConfigured is treated as a non-retryable configuration problem (no attempt-count inflation) rather than as a transient failure

Extend src/Tests/DockerUpdateGuard.Tests/ScanCleanupBackgroundServiceTests.cs:

• old Sent/Suppressed/DeadLettered records past the cutoff are removed
• Pending/Retrying records are never removed regardless of age
• a record linked to a still-active finding survives the cutoff
• a record linked to an inactive or deleted finding past the cutoff is removed
• the completion log reports the removed count

Plus an ApplicationTelemetry test asserting counter and gauge behavior in the style of the existing telemetry tests.

Acceptance criteria

• A transient endpoint failure moves records to Retrying with a growing NextAttemptAtUtc; records not yet due are skipped without calling the channel
• Exhausted attempts transition to DeadLettered with LastError persisted, a warning log, and a failure metric; dead-lettered records are never retried or re-detected
• Recovery after failures marks the records Sent with a shared DispatchBatchId
• Final-state records older than the retention window are purged, while Pending/Retrying records and records with a still-active linked finding survive
• A long-lived active CVE is not re-notified after its records would otherwise have been cleaned up
• Sent/failed counters and the pending gauge are exported and tagged with channel and kind
• reihitsu-format ./ clean, build without new analyzer findings, all tests green

Out of scope

• Manual requeue of dead-lettered records (a possible follow-up to Notifications 6/6: notification history page (optional) #225)
• A dedicated retention option for notification records — the existing scan retention window is reused
• Per-channel retry configuration; the retry parameters are global for now