Skip to content

Commit bb43e55

Browse files
alexluongclaude
andauthored
feat(alert): default/disable semantics for consecutive-failure & exhausted-retries alerts (#964)
* feat(alert): add Settings + enable gates for consecutive/exhausted alerts Introduce alert.Settings (the resolved, operational alert config) plus two monitor gates: WithConsecutiveFailureEnabled and WithExhaustedRetriesEnabled. Both default to true, so behavior is unchanged until a caller opts out. When consecutive-failure alerting is gated off the monitor neither tracks failures nor auto-disables; when exhausted-retries is gated off it never emits, even with retries enabled. Extracts the consecutive-failure path into a helper to keep the replay/ordering semantics identical on the enabled path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(config): resolve alert config to alert.Settings with unset/empty/value rule AlertConfig.ConsecutiveFailureCount and ExhaustedRetriesWindowSeconds become *string so the parse layer can tell three states apart: unset uses the default (100 / 3600), an empty string disables that alert dimension, and any other value must parse to a non-negative integer. AlertConfig.ToConfig resolves the raw values into the operational alert.Settings (domain-owned, so nothing downstream imports config). Validate rejects malformed values at startup. builder wires the resolved gates into the monitor and only builds the exhausted-retries suppression window when enabled with a positive window (0 = alert on every exhaustion). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(config): document alert default/disable behavior in config reference Update the Alerts section of the self-hosting config reference for the new unset/empty/value rule: ALERT_CONSECUTIVE_FAILURE_COUNT defaults to 100 (empty disables), and document the previously-undocumented ALERT_EXHAUSTED_RETRIES_WINDOW_SECONDS (default 3600, empty disables, 0 = no suppression). Also correct stale entries: drop the removed ALERT_CALLBACK_URL, fix the ALERT_AUTO_DISABLE_DESTINATION default (false, not true), and fix the YAML example key (alert, not alerts). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(openapi): describe alert behavior in ManagedConfig Document the unset/empty/value behavior for ALERT_CONSECUTIVE_FAILURE_COUNT, ALERT_EXHAUSTED_RETRIES_WINDOW_SECONDS and ALERT_AUTO_DISABLE_DESTINATION in the ManagedConfig schema. Descriptions only — the properties are already typed as string. SDKs are regenerated from this schema at release time. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(config): make empty-string alert disable work on the env-var surface The *string representation had two problems found by manual QA: 1. caarlos0/env ignores a present-but-empty env var, so `ALERT_..._COUNT=` resolved to the default instead of disabling — the empty=off rule only worked via YAML, not env vars (the primary surface for the cloud product). 2. caarlos0/env crashes ("expected a pointer to a Struct") on any non-nil *string it walks, so setting these in a YAML config file would crash startup (env.Parse runs after the YAML load). Replace *string with an OptionalString value type that implements both TextUnmarshaler (bound by caarlos0/env as a scalar — no crash) and yaml.Unmarshaler (so `key: ""` expresses the empty/off state). The one case caarlos0/env cannot surface — a present-but-empty env var — is handled explicitly via OSInterface.LookupEnv, which also gives env precedence over YAML. Net: unset -> default, empty -> disabled, value -> value, identically on both env and YAML, with env > yaml. Adds a full parse-path test covering the matrix and precedence. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(e2e): use OptionalString for ConsecutiveFailureCount in regression test Missed call site when migrating AlertConfig fields to OptionalString; the raw int assignment broke the cmd/e2e build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 41c95f9 commit bb43e55

15 files changed

Lines changed: 726 additions & 112 deletions

File tree

cmd/e2e/configs/basic.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func Basic(t *testing.T, opts BasicOpts) config.Config {
8787
c.LogBatchSize = 100
8888
c.DeploymentID = opts.DeploymentID
8989
c.Alert.AutoDisableDestination = true
90-
c.Alert.ConsecutiveFailureCount = 20
90+
c.Alert.ConsecutiveFailureCount = config.NewOptionalString("20")
9191

9292
// Setup cleanup
9393
t.Cleanup(func() {

cmd/e2e/regressions_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func TestE2E_Regression_AutoDisableWithoutCallbackURL(t *testing.T) {
9191
LogStorage: configs.LogStorageTypePostgres,
9292
})
9393
cfg.Alert.AutoDisableDestination = true
94-
cfg.Alert.ConsecutiveFailureCount = 20
94+
cfg.Alert.ConsecutiveFailureCount = config.NewOptionalString("20")
9595

9696
require.NoError(t, cfg.Validate(config.Flags{}))
9797
configs.ApplyMigrations(t, &cfg)

docs/apis/openapi.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3094,10 +3094,25 @@ components:
30943094
properties:
30953095
ALERT_AUTO_DISABLE_DESTINATION:
30963096
type: string
3097+
description: >-
3098+
If "true", automatically disables a destination once
3099+
ALERT_CONSECUTIVE_FAILURE_COUNT is reached. Has no effect when
3100+
consecutive-failure alerting is disabled.
30973101
ALERT_CONSECUTIVE_FAILURE_COUNT:
30983102
type: string
3103+
description: >-
3104+
Consecutive delivery failures before alerting on a destination (and
3105+
disabling it when ALERT_AUTO_DISABLE_DESTINATION is "true"). Omit for
3106+
the default of 100; set to an empty string to disable
3107+
consecutive-failure alerting entirely.
30993108
ALERT_EXHAUSTED_RETRIES_WINDOW_SECONDS:
31003109
type: string
3110+
description: >-
3111+
Suppression window in seconds for exhausted_retries alerts: the first
3112+
exhaustion per destination alerts and subsequent ones within the
3113+
window are suppressed ("0" = no suppression). Omit for the default of
3114+
3600; set to an empty string to disable exhausted_retries alerting
3115+
entirely.
31013116
DELIVERY_TIMEOUT_SECONDS:
31023117
type: string
31033118
DESTINATIONS_AWS_KINESIS_METADATA_IN_PAYLOAD:

docs/content/self-hosting/configuration.mdoc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,9 @@ Choose one for event log persistence:
9898

9999
| Variable | Default | Description |
100100
|----------|---------|-------------|
101-
| `ALERT_CALLBACK_URL` | | URL to POST to when an alert fires |
102-
| `ALERT_CONSECUTIVE_FAILURE_COUNT` | `20` | Consecutive failures before alert triggers |
103-
| `ALERT_AUTO_DISABLE_DESTINATION` | `true` | Auto-disable destination when failure count reaches 100% |
101+
| `ALERT_CONSECUTIVE_FAILURE_COUNT` | `100` | Consecutive delivery failures before alerting on a destination (and disabling it when `ALERT_AUTO_DISABLE_DESTINATION` is `true`). Leave unset for the default of `100`; set to an empty string to disable consecutive-failure alerting entirely. |
102+
| `ALERT_AUTO_DISABLE_DESTINATION` | `false` | Auto-disable a destination once `ALERT_CONSECUTIVE_FAILURE_COUNT` is reached. Has no effect when consecutive-failure alerting is disabled. |
103+
| `ALERT_EXHAUSTED_RETRIES_WINDOW_SECONDS` | `3600` | Suppression window (seconds) for `exhausted_retries` alerts: the first exhaustion per destination alerts and subsequent ones within the window are suppressed (`0` = no suppression, alert on every exhaustion). Leave unset for the default of `3600`; set to an empty string to disable `exhausted_retries` alerting entirely. |
104104

105105
## Destinations
106106

@@ -159,7 +159,7 @@ portal:
159159
organization_name: "Acme Corp"
160160
accent_color: "#6122E7"
161161

162-
alerts:
163-
callback_url: "https://yourapp.com/api/outpost-alerts"
162+
alert:
164163
consecutive_failure_count: 50
164+
exhausted_retries_window_seconds: 3600
165165
```

internal/alert/monitor.go

Lines changed: 141 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,23 @@ func WithExhaustedRetriesIdempotence(idemp idempotence.Idempotence) AlertOption
9090
}
9191
}
9292

93+
// WithConsecutiveFailureEnabled toggles consecutive-failure alerting. When set
94+
// to false the monitor never tracks or alerts on consecutive failures (and
95+
// therefore never auto-disables). Defaults to true.
96+
func WithConsecutiveFailureEnabled(enabled bool) AlertOption {
97+
return func(m *alertMonitor) {
98+
m.consecutiveFailureEnabled = enabled
99+
}
100+
}
101+
102+
// WithExhaustedRetriesEnabled toggles exhausted_retries alerting. When set to
103+
// false the monitor never emits exhausted_retries alerts. Defaults to true.
104+
func WithExhaustedRetriesEnabled(enabled bool) AlertOption {
105+
return func(m *alertMonitor) {
106+
m.exhaustedRetriesEnabled = enabled
107+
}
108+
}
109+
93110
// DeliveryAttempt represents a single delivery attempt
94111
type DeliveryAttempt struct {
95112
Event *models.Event
@@ -109,6 +126,9 @@ type alertMonitor struct {
109126
alertThresholds []int
110127
retryMaxLimit int
111128
exhaustedRetryIdemp idempotence.Idempotence
129+
130+
consecutiveFailureEnabled bool
131+
exhaustedRetriesEnabled bool
112132
}
113133

114134
// NewAlertMonitor creates a new alert monitor. Emitter and retryMaxLimit are
@@ -119,10 +139,12 @@ func NewAlertMonitor(logger *logging.Logger, redisClient redis.Cmdable, emitter
119139
panic("alert: NewAlertMonitor requires a non-nil emitter")
120140
}
121141
alertMonitor := &alertMonitor{
122-
logger: logger,
123-
emitter: emitter,
124-
retryMaxLimit: retryMaxLimit,
125-
alertThresholds: []int{50, 70, 90, 100}, // default thresholds
142+
logger: logger,
143+
emitter: emitter,
144+
retryMaxLimit: retryMaxLimit,
145+
alertThresholds: []int{50, 70, 90, 100}, // default thresholds
146+
consecutiveFailureEnabled: true,
147+
exhaustedRetriesEnabled: true,
126148
}
127149

128150
for _, opt := range opts {
@@ -142,93 +164,32 @@ func NewAlertMonitor(logger *logging.Logger, redisClient redis.Cmdable, emitter
142164

143165
func (m *alertMonitor) HandleAttempt(ctx context.Context, attempt DeliveryAttempt) error {
144166
if attempt.Attempt.Status == models.AttemptStatusSuccess {
167+
// Nothing is tracked when consecutive-failure alerting is disabled, so
168+
// there is no count to reset.
169+
if !m.consecutiveFailureEnabled {
170+
return nil
171+
}
145172
return m.store.ResetConsecutiveFailureCount(ctx, attempt.Destination.TenantID, attempt.Destination.ID)
146173
}
147174

148-
// Consecutive failure tracking
149-
res, err := m.store.IncrementConsecutiveFailureCount(ctx, attempt.Destination.TenantID, attempt.Destination.ID, attempt.Attempt.ID)
150-
if err != nil {
151-
return fmt.Errorf("failed to get alert state: %w", err)
152-
}
153-
154-
// Replayed attempt (MQ redelivery, producer re-publish) that already
155-
// completed a full evaluation — skip re-emitting alerts. Attempts that
156-
// were counted but never marked evaluated (eval failed mid-way and the
157-
// message was nacked) fall through and re-run as recovery.
158-
if !res.NewlyCounted && res.AlreadyEvaluated {
159-
m.logger.Ctx(ctx).Debug("skipping replayed attempt: already evaluated",
160-
zap.String("attempt_id", attempt.Attempt.ID),
161-
zap.String("tenant_id", attempt.Destination.TenantID),
162-
zap.String("destination_id", attempt.Destination.ID),
163-
)
164-
return nil
165-
}
166-
167-
count := res.Count
168-
level, shouldAlert := m.evaluator.ShouldAlert(count)
169-
if shouldAlert {
170-
// At 100% threshold, disable the destination and emit disabled alert.
171-
// Both operations are idempotent on replay: DisableDestination is a no-op
172-
// if already disabled, and consumers deduplicate events by ID.
173-
if level == 100 && m.disabler != nil {
174-
if err := m.disabler.DisableDestination(ctx, attempt.Destination.TenantID, attempt.Destination.ID); err != nil {
175-
return fmt.Errorf("failed to disable destination: %w", err)
176-
}
177-
178-
now := time.Now()
179-
attempt.Destination.DisabledAt = &now
180-
181-
m.logger.Ctx(ctx).Audit("destination disabled",
182-
zap.String("attempt_id", attempt.Attempt.ID),
183-
zap.String("event_id", attempt.Event.ID),
184-
zap.String("tenant_id", attempt.Destination.TenantID),
185-
zap.String("destination_id", attempt.Destination.ID),
186-
zap.String("destination_type", attempt.Destination.Type),
187-
)
188-
189-
disabledData := DestinationDisabledData{
190-
TenantID: attempt.Destination.TenantID,
191-
Destination: attempt.Destination,
192-
DisabledAt: now,
193-
Reason: "consecutive_failure",
194-
Event: attempt.Event,
195-
Attempt: attempt.Attempt,
196-
}
197-
if err := m.emitter.Emit(ctx, opevents.TopicAlertDestinationDisabled, attempt.Destination.TenantID, disabledData); err != nil {
198-
return fmt.Errorf("failed to emit destination disabled alert: %w", err)
199-
}
200-
}
201-
202-
// Emit consecutive failure alert
203-
cfData := ConsecutiveFailureData{
204-
TenantID: attempt.Destination.TenantID,
205-
Event: attempt.Event,
206-
Attempt: attempt.Attempt,
207-
Destination: attempt.Destination,
208-
ConsecutiveFailures: ConsecutiveFailures{
209-
Current: count,
210-
Max: m.autoDisableFailureCount,
211-
Threshold: level,
212-
},
175+
if m.consecutiveFailureEnabled {
176+
// A replayed attempt that already completed evaluation skips the rest of
177+
// the pipeline (exhausted-retries check and the evaluated mark), matching
178+
// the original single-pass behavior.
179+
done, err := m.handleConsecutiveFailure(ctx, attempt)
180+
if err != nil {
181+
return err
213182
}
214-
if err := m.emitter.Emit(ctx, opevents.TopicAlertConsecutiveFailure, attempt.Destination.TenantID, cfData); err != nil {
215-
return fmt.Errorf("failed to emit consecutive failure alert: %w", err)
183+
if done {
184+
return nil
216185
}
217-
218-
m.logger.Ctx(ctx).Audit("alert sent",
219-
zap.String("topic", opevents.TopicAlertConsecutiveFailure),
220-
zap.String("attempt_id", attempt.Attempt.ID),
221-
zap.String("event_id", attempt.Event.ID),
222-
zap.String("tenant_id", attempt.Destination.TenantID),
223-
zap.String("destination_id", attempt.Destination.ID),
224-
zap.String("destination_type", attempt.Destination.Type),
225-
)
226186
}
227187

228188
// Exhausted retries check (independent of consecutive failure thresholds).
229189
// Attempt is 1-indexed: with retryMaxLimit=10, attempt 11 is the final one.
230-
// Skip if retryMaxLimit=0 (retries disabled — no exhausted state to report).
231-
if m.retryMaxLimit > 0 && attempt.Event.EligibleForRetry && attempt.Attempt.AttemptNumber > m.retryMaxLimit {
190+
// Skip if retryMaxLimit=0 (retries disabled — no exhausted state to report)
191+
// or if exhausted-retries alerting is disabled.
192+
if m.exhaustedRetriesEnabled && m.retryMaxLimit > 0 && attempt.Event.EligibleForRetry && attempt.Attempt.AttemptNumber > m.retryMaxLimit {
232193
erData := ExhaustedRetriesData{
233194
TenantID: attempt.Destination.TenantID,
234195
Event: attempt.Event,
@@ -263,17 +224,110 @@ func (m *alertMonitor) HandleAttempt(ctx context.Context, attempt DeliveryAttemp
263224
}
264225
}
265226

266-
// Mark the attempt fully evaluated so replays skip re-emitting alerts.
227+
// Mark the attempt fully evaluated so replays skip re-emitting alerts. Only
228+
// relevant when consecutive-failure tracking ran — it is the consumer of the
229+
// evaluated mark (the replay short-circuit above).
267230
// Non-fatal: on failure the attempt simply re-evaluates on replay, which
268231
// matches the previous behavior (emit/disable are idempotent-by-design).
269-
if err := m.store.MarkAttemptEvaluated(ctx, attempt.Destination.TenantID, attempt.Destination.ID, attempt.Attempt.ID); err != nil {
270-
m.logger.Ctx(ctx).Warn("failed to mark attempt evaluated",
271-
zap.Error(err),
232+
if m.consecutiveFailureEnabled {
233+
if err := m.store.MarkAttemptEvaluated(ctx, attempt.Destination.TenantID, attempt.Destination.ID, attempt.Attempt.ID); err != nil {
234+
m.logger.Ctx(ctx).Warn("failed to mark attempt evaluated",
235+
zap.Error(err),
236+
zap.String("attempt_id", attempt.Attempt.ID),
237+
zap.String("tenant_id", attempt.Destination.TenantID),
238+
zap.String("destination_id", attempt.Destination.ID),
239+
)
240+
}
241+
}
242+
243+
return nil
244+
}
245+
246+
// handleConsecutiveFailure runs consecutive-failure tracking, alerting and
247+
// auto-disable for a failed attempt. It returns done=true when the attempt is a
248+
// replay that already completed evaluation, signalling the caller to stop
249+
// processing (skip the exhausted-retries check and the evaluated mark).
250+
func (m *alertMonitor) handleConsecutiveFailure(ctx context.Context, attempt DeliveryAttempt) (done bool, err error) {
251+
res, err := m.store.IncrementConsecutiveFailureCount(ctx, attempt.Destination.TenantID, attempt.Destination.ID, attempt.Attempt.ID)
252+
if err != nil {
253+
return false, fmt.Errorf("failed to get alert state: %w", err)
254+
}
255+
256+
// Replayed attempt (MQ redelivery, producer re-publish) that already
257+
// completed a full evaluation — skip re-emitting alerts. Attempts that
258+
// were counted but never marked evaluated (eval failed mid-way and the
259+
// message was nacked) fall through and re-run as recovery.
260+
if !res.NewlyCounted && res.AlreadyEvaluated {
261+
m.logger.Ctx(ctx).Debug("skipping replayed attempt: already evaluated",
272262
zap.String("attempt_id", attempt.Attempt.ID),
273263
zap.String("tenant_id", attempt.Destination.TenantID),
274264
zap.String("destination_id", attempt.Destination.ID),
275265
)
266+
return true, nil
276267
}
277268

278-
return nil
269+
count := res.Count
270+
level, shouldAlert := m.evaluator.ShouldAlert(count)
271+
if !shouldAlert {
272+
return false, nil
273+
}
274+
275+
// At 100% threshold, disable the destination and emit disabled alert.
276+
// Both operations are idempotent on replay: DisableDestination is a no-op
277+
// if already disabled, and consumers deduplicate events by ID.
278+
if level == 100 && m.disabler != nil {
279+
if err := m.disabler.DisableDestination(ctx, attempt.Destination.TenantID, attempt.Destination.ID); err != nil {
280+
return false, fmt.Errorf("failed to disable destination: %w", err)
281+
}
282+
283+
now := time.Now()
284+
attempt.Destination.DisabledAt = &now
285+
286+
m.logger.Ctx(ctx).Audit("destination disabled",
287+
zap.String("attempt_id", attempt.Attempt.ID),
288+
zap.String("event_id", attempt.Event.ID),
289+
zap.String("tenant_id", attempt.Destination.TenantID),
290+
zap.String("destination_id", attempt.Destination.ID),
291+
zap.String("destination_type", attempt.Destination.Type),
292+
)
293+
294+
disabledData := DestinationDisabledData{
295+
TenantID: attempt.Destination.TenantID,
296+
Destination: attempt.Destination,
297+
DisabledAt: now,
298+
Reason: "consecutive_failure",
299+
Event: attempt.Event,
300+
Attempt: attempt.Attempt,
301+
}
302+
if err := m.emitter.Emit(ctx, opevents.TopicAlertDestinationDisabled, attempt.Destination.TenantID, disabledData); err != nil {
303+
return false, fmt.Errorf("failed to emit destination disabled alert: %w", err)
304+
}
305+
}
306+
307+
// Emit consecutive failure alert
308+
cfData := ConsecutiveFailureData{
309+
TenantID: attempt.Destination.TenantID,
310+
Event: attempt.Event,
311+
Attempt: attempt.Attempt,
312+
Destination: attempt.Destination,
313+
ConsecutiveFailures: ConsecutiveFailures{
314+
Current: count,
315+
Max: m.autoDisableFailureCount,
316+
Threshold: level,
317+
},
318+
}
319+
if err := m.emitter.Emit(ctx, opevents.TopicAlertConsecutiveFailure, attempt.Destination.TenantID, cfData); err != nil {
320+
return false, fmt.Errorf("failed to emit consecutive failure alert: %w", err)
321+
}
322+
323+
m.logger.Ctx(ctx).Audit("alert sent",
324+
zap.String("topic", opevents.TopicAlertConsecutiveFailure),
325+
zap.String("attempt_id", attempt.Attempt.ID),
326+
zap.String("event_id", attempt.Event.ID),
327+
zap.String("tenant_id", attempt.Destination.TenantID),
328+
zap.String("destination_id", attempt.Destination.ID),
329+
zap.String("destination_type", attempt.Destination.Type),
330+
)
331+
332+
return false, nil
279333
}

0 commit comments

Comments
 (0)