From ac0571e2e7e22bb452564416de9fad2028464556 Mon Sep 17 00:00:00 2001 From: Dean Chen <862469039@qq.com> Date: Wed, 22 Jul 2026 15:44:58 +0500 Subject: [PATCH 1/3] notify: add receiver and integration as structured log fields on failures When notification delivery fails, the dispatcher error log only exposed receiver/integration inside the free-text err string. Annotate RetryStage failures with ErrorWithIntegration and attach receiver/integration as structured fields on the dispatch failure log so operators can group by them in log aggregators without regex-parsing err. Fixes #5396 Signed-off-by: Dean Chen <862469039@qq.com> --- dispatch/dispatch.go | 6 +++++- notify/notify_test.go | 17 +++++++++++++++-- notify/retry_stage.go | 6 ++++-- notify/util.go | 25 +++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/dispatch/dispatch.go b/dispatch/dispatch.go index 2e3da19970..b59ba92fed 100644 --- a/dispatch/dispatch.go +++ b/dispatch/dispatch.go @@ -578,7 +578,11 @@ func (d *Dispatcher) runAG(ag *aggrGroup) { go ag.run(func(ctx context.Context, alerts ...*alert.Alert) bool { _, _, err := d.stage.Exec(ctx, d.logger, alerts...) if err != nil { - logger := d.logger.With("aggrGroup", ag.GroupKey(), "num_alerts", len(alerts), "err", err) + logger := d.logger.With("aggrGroup", ag.GroupKey(), "num_alerts", len(alerts), "receiver", ag.opts.Receiver, "err", err) + var ie *notify.ErrorWithIntegration + if errors.As(err, &ie) { + logger = logger.With("integration", ie.Integration) + } if errors.Is(ctx.Err(), context.Canceled) { // It is expected for the context to be canceled on // configuration reload or shutdown. In this case, the diff --git a/notify/notify_test.go b/notify/notify_test.go index 1bbb4d8574..d25efb6ac5 100644 --- a/notify/notify_test.go +++ b/notify/notify_test.go @@ -500,6 +500,8 @@ func TestRetryStageWithError(t *testing.T) { fail, retry := true, true sent := []*alert.Alert{} i := Integration{ + name: "slack", + idx: 0, notifier: notifierFunc(func(ctx context.Context, alerts ...*alert.Alert) NotifyVerdict { if fail { fail = false @@ -514,7 +516,7 @@ func TestRetryStageWithError(t *testing.T) { }), rs: sendResolved(false), } - r := NewRetryStage(i, "", NewMetrics(prometheus.NewRegistry(), featurecontrol.NoopFlags{}), eventrecorder.NopRecorder()) + r := NewRetryStage(i, "team-receiver", NewMetrics(prometheus.NewRegistry(), featurecontrol.NoopFlags{}), eventrecorder.NopRecorder()) alerts := []*alert.Alert{ { @@ -541,6 +543,12 @@ func TestRetryStageWithError(t *testing.T) { resctx, _, err = r.Exec(ctx, promslog.NewNopLogger(), alerts...) require.Error(t, err) require.NotNil(t, resctx) + + var ie *ErrorWithIntegration + require.ErrorAs(t, err, &ie) + require.Equal(t, "team-receiver", ie.Receiver) + require.Equal(t, "slack[0]", ie.Integration) + require.Contains(t, err.Error(), "fail to deliver notification") } func TestRetryStageWithErrorCode(t *testing.T) { @@ -596,7 +604,7 @@ func TestRetryStageWithContextCanceled(t *testing.T) { }), rs: sendResolved(false), } - r := NewRetryStage(i, "", NewMetrics(prometheus.NewRegistry(), featurecontrol.NoopFlags{}), eventrecorder.NopRecorder()) + r := NewRetryStage(i, "canceled-receiver", NewMetrics(prometheus.NewRegistry(), featurecontrol.NoopFlags{}), eventrecorder.NopRecorder()) alerts := []*alert.Alert{ { @@ -617,6 +625,11 @@ func TestRetryStageWithContextCanceled(t *testing.T) { require.Error(t, err) require.NotNil(t, resctx) + + var ie *ErrorWithIntegration + require.ErrorAs(t, err, &ie) + require.Equal(t, "canceled-receiver", ie.Receiver) + require.Equal(t, "test[0]", ie.Integration) } func TestRetryStageNoResolved(t *testing.T) { diff --git a/notify/retry_stage.go b/notify/retry_stage.go index 455613c4b2..194516d1c6 100644 --- a/notify/retry_stage.go +++ b/notify/retry_stage.go @@ -144,7 +144,8 @@ func (r RetryStage) exec(ctx context.Context, l *slog.Logger, alerts ...*alert.A } if iErr != nil { - return ctx, nil, iReason, fmt.Errorf("%s/%s: notify retry canceled after %d attempts: %w", r.groupName, r.integration.String(), i, iErr) + return ctx, nil, iReason, NewErrorWithIntegration(r.groupName, r.integration.String(), + fmt.Errorf("%s/%s: notify retry canceled after %d attempts: %w", r.groupName, r.integration.String(), i, iErr)) } return ctx, nil, DefaultReason, nil default: @@ -164,7 +165,8 @@ func (r RetryStage) exec(ctx context.Context, l *slog.Logger, alerts ...*alert.A if err := verdict.Err(); err != nil { r.metrics.numNotificationRequestsFailedTotal.WithLabelValues(r.labelValues...).Inc() if !verdict.ShouldRetry() { - return ctx, alerts, verdict.Reason(), fmt.Errorf("%s/%s: notify retry canceled due to unrecoverable error after %d attempts: %w", r.groupName, r.integration.String(), i, err) + return ctx, alerts, verdict.Reason(), NewErrorWithIntegration(r.groupName, r.integration.String(), + fmt.Errorf("%s/%s: notify retry canceled due to unrecoverable error after %d attempts: %w", r.groupName, r.integration.String(), i, err)) } if ctx.Err() == nil { if iErr == nil || err.Error() != iErr.Error() { diff --git a/notify/util.go b/notify/util.go index ad449513c8..5777246900 100644 --- a/notify/util.go +++ b/notify/util.go @@ -290,6 +290,31 @@ func (r *Retrier) Check(statusCode int, body io.Reader) (bool, error) { return retry, errors.New(s) } +// ErrorWithIntegration annotates a notification failure with the receiver and +// integration that failed, so callers can attach them as structured log fields. +type ErrorWithIntegration struct { + Receiver string + Integration string + Err error +} + +// NewErrorWithIntegration returns an error annotated with receiver and integration. +func NewErrorWithIntegration(receiver, integration string, err error) *ErrorWithIntegration { + return &ErrorWithIntegration{ + Receiver: receiver, + Integration: integration, + Err: err, + } +} + +func (e *ErrorWithIntegration) Error() string { + return e.Err.Error() +} + +func (e *ErrorWithIntegration) Unwrap() error { + return e.Err +} + // Reason is the failure reason. type Reason int From 951abb0a1c7811f098c923b541385dfcb6098547 Mon Sep 17 00:00:00 2001 From: Dean Chen <862469039@qq.com> Date: Tue, 15 Sep 2026 13:29:21 +0500 Subject: [PATCH 2/3] dispatch: use errors.AsType for ErrorWithIntegration golangci-lint modernize flags errors.As here. Signed-off-by: Dean Chen <862469039@qq.com> --- dispatch/dispatch.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dispatch/dispatch.go b/dispatch/dispatch.go index b59ba92fed..48be401fb5 100644 --- a/dispatch/dispatch.go +++ b/dispatch/dispatch.go @@ -579,8 +579,7 @@ func (d *Dispatcher) runAG(ag *aggrGroup) { _, _, err := d.stage.Exec(ctx, d.logger, alerts...) if err != nil { logger := d.logger.With("aggrGroup", ag.GroupKey(), "num_alerts", len(alerts), "receiver", ag.opts.Receiver, "err", err) - var ie *notify.ErrorWithIntegration - if errors.As(err, &ie) { + if ie, ok := errors.AsType[*notify.ErrorWithIntegration](err); ok { logger = logger.With("integration", ie.Integration) } if errors.Is(ctx.Err(), context.Canceled) { From c35cdc45c233ae46bb8027c0542437cfdc5ca65e Mon Sep 17 00:00:00 2001 From: Dean Chen <862469039@qq.com> Date: Mon, 21 Sep 2026 12:04:31 +0500 Subject: [PATCH 3/3] dispatch: keep only receiver on notify failure logs Fanout joins errors, so an integration field on the dispatcher line isn't reliable. Leave integration in the error text; RetryStage already logs it on retries. Signed-off-by: Dean Chen <862469039@qq.com> --- dispatch/dispatch.go | 3 --- notify/notify_test.go | 17 ++--------------- notify/retry_stage.go | 6 ++---- notify/util.go | 25 ------------------------- 4 files changed, 4 insertions(+), 47 deletions(-) diff --git a/dispatch/dispatch.go b/dispatch/dispatch.go index 48be401fb5..e8911f10cc 100644 --- a/dispatch/dispatch.go +++ b/dispatch/dispatch.go @@ -579,9 +579,6 @@ func (d *Dispatcher) runAG(ag *aggrGroup) { _, _, err := d.stage.Exec(ctx, d.logger, alerts...) if err != nil { logger := d.logger.With("aggrGroup", ag.GroupKey(), "num_alerts", len(alerts), "receiver", ag.opts.Receiver, "err", err) - if ie, ok := errors.AsType[*notify.ErrorWithIntegration](err); ok { - logger = logger.With("integration", ie.Integration) - } if errors.Is(ctx.Err(), context.Canceled) { // It is expected for the context to be canceled on // configuration reload or shutdown. In this case, the diff --git a/notify/notify_test.go b/notify/notify_test.go index d25efb6ac5..1bbb4d8574 100644 --- a/notify/notify_test.go +++ b/notify/notify_test.go @@ -500,8 +500,6 @@ func TestRetryStageWithError(t *testing.T) { fail, retry := true, true sent := []*alert.Alert{} i := Integration{ - name: "slack", - idx: 0, notifier: notifierFunc(func(ctx context.Context, alerts ...*alert.Alert) NotifyVerdict { if fail { fail = false @@ -516,7 +514,7 @@ func TestRetryStageWithError(t *testing.T) { }), rs: sendResolved(false), } - r := NewRetryStage(i, "team-receiver", NewMetrics(prometheus.NewRegistry(), featurecontrol.NoopFlags{}), eventrecorder.NopRecorder()) + r := NewRetryStage(i, "", NewMetrics(prometheus.NewRegistry(), featurecontrol.NoopFlags{}), eventrecorder.NopRecorder()) alerts := []*alert.Alert{ { @@ -543,12 +541,6 @@ func TestRetryStageWithError(t *testing.T) { resctx, _, err = r.Exec(ctx, promslog.NewNopLogger(), alerts...) require.Error(t, err) require.NotNil(t, resctx) - - var ie *ErrorWithIntegration - require.ErrorAs(t, err, &ie) - require.Equal(t, "team-receiver", ie.Receiver) - require.Equal(t, "slack[0]", ie.Integration) - require.Contains(t, err.Error(), "fail to deliver notification") } func TestRetryStageWithErrorCode(t *testing.T) { @@ -604,7 +596,7 @@ func TestRetryStageWithContextCanceled(t *testing.T) { }), rs: sendResolved(false), } - r := NewRetryStage(i, "canceled-receiver", NewMetrics(prometheus.NewRegistry(), featurecontrol.NoopFlags{}), eventrecorder.NopRecorder()) + r := NewRetryStage(i, "", NewMetrics(prometheus.NewRegistry(), featurecontrol.NoopFlags{}), eventrecorder.NopRecorder()) alerts := []*alert.Alert{ { @@ -625,11 +617,6 @@ func TestRetryStageWithContextCanceled(t *testing.T) { require.Error(t, err) require.NotNil(t, resctx) - - var ie *ErrorWithIntegration - require.ErrorAs(t, err, &ie) - require.Equal(t, "canceled-receiver", ie.Receiver) - require.Equal(t, "test[0]", ie.Integration) } func TestRetryStageNoResolved(t *testing.T) { diff --git a/notify/retry_stage.go b/notify/retry_stage.go index 194516d1c6..455613c4b2 100644 --- a/notify/retry_stage.go +++ b/notify/retry_stage.go @@ -144,8 +144,7 @@ func (r RetryStage) exec(ctx context.Context, l *slog.Logger, alerts ...*alert.A } if iErr != nil { - return ctx, nil, iReason, NewErrorWithIntegration(r.groupName, r.integration.String(), - fmt.Errorf("%s/%s: notify retry canceled after %d attempts: %w", r.groupName, r.integration.String(), i, iErr)) + return ctx, nil, iReason, fmt.Errorf("%s/%s: notify retry canceled after %d attempts: %w", r.groupName, r.integration.String(), i, iErr) } return ctx, nil, DefaultReason, nil default: @@ -165,8 +164,7 @@ func (r RetryStage) exec(ctx context.Context, l *slog.Logger, alerts ...*alert.A if err := verdict.Err(); err != nil { r.metrics.numNotificationRequestsFailedTotal.WithLabelValues(r.labelValues...).Inc() if !verdict.ShouldRetry() { - return ctx, alerts, verdict.Reason(), NewErrorWithIntegration(r.groupName, r.integration.String(), - fmt.Errorf("%s/%s: notify retry canceled due to unrecoverable error after %d attempts: %w", r.groupName, r.integration.String(), i, err)) + return ctx, alerts, verdict.Reason(), fmt.Errorf("%s/%s: notify retry canceled due to unrecoverable error after %d attempts: %w", r.groupName, r.integration.String(), i, err) } if ctx.Err() == nil { if iErr == nil || err.Error() != iErr.Error() { diff --git a/notify/util.go b/notify/util.go index 5777246900..ad449513c8 100644 --- a/notify/util.go +++ b/notify/util.go @@ -290,31 +290,6 @@ func (r *Retrier) Check(statusCode int, body io.Reader) (bool, error) { return retry, errors.New(s) } -// ErrorWithIntegration annotates a notification failure with the receiver and -// integration that failed, so callers can attach them as structured log fields. -type ErrorWithIntegration struct { - Receiver string - Integration string - Err error -} - -// NewErrorWithIntegration returns an error annotated with receiver and integration. -func NewErrorWithIntegration(receiver, integration string, err error) *ErrorWithIntegration { - return &ErrorWithIntegration{ - Receiver: receiver, - Integration: integration, - Err: err, - } -} - -func (e *ErrorWithIntegration) Error() string { - return e.Err.Error() -} - -func (e *ErrorWithIntegration) Unwrap() error { - return e.Err -} - // Reason is the failure reason. type Reason int