From e775fba14625c960b66f961f079b51c69263655a Mon Sep 17 00:00:00 2001 From: George Robinson Date: Tue, 12 Oct 2021 15:28:45 +0100 Subject: [PATCH] Alerting: Fix error message in ngalert when notifications cannot be sent to alertmanager (#40158) (#40317) (cherry picked from commit 8318e45452b31f44b4538d85ba5896f0199249eb) --- .../ngalert/notifier/channels/alertmanager.go | 12 +- .../notifier/channels/alertmanager_test.go | 106 +++++++++++++----- 2 files changed, 89 insertions(+), 29 deletions(-) diff --git a/pkg/services/ngalert/notifier/channels/alertmanager.go b/pkg/services/ngalert/notifier/channels/alertmanager.go index d772e5d34d1..6db58ef5abf 100644 --- a/pkg/services/ngalert/notifier/channels/alertmanager.go +++ b/pkg/services/ngalert/notifier/channels/alertmanager.go @@ -79,7 +79,10 @@ func (n *AlertmanagerNotifier) Notify(ctx context.Context, as ...*types.Alert) ( return false, err } - errCnt := 0 + var ( + lastErr error + numErrs int + ) for _, u := range n.urls { if _, err := sendHTTPRequest(ctx, u, httpCfg{ user: n.basicAuthUser, @@ -87,14 +90,15 @@ func (n *AlertmanagerNotifier) Notify(ctx context.Context, as ...*types.Alert) ( body: body, }, n.logger); err != nil { n.logger.Warn("Failed to send to Alertmanager", "error", err, "alertmanager", n.Name, "url", u.String()) - errCnt++ + lastErr = err + numErrs++ } } - if errCnt == len(n.urls) { + if numErrs == len(n.urls) { // All attempts to send alerts have failed n.logger.Warn("All attempts to send to Alertmanager failed", "alertmanager", n.Name) - return false, fmt.Errorf("failed to send alert to Alertmanager") + return false, fmt.Errorf("failed to send alert to Alertmanager: %w", lastErr) } return true, nil diff --git a/pkg/services/ngalert/notifier/channels/alertmanager_test.go b/pkg/services/ngalert/notifier/channels/alertmanager_test.go index 0766c794450..0deb43002b5 100644 --- a/pkg/services/ngalert/notifier/channels/alertmanager_test.go +++ b/pkg/services/ngalert/notifier/channels/alertmanager_test.go @@ -3,6 +3,7 @@ package channels import ( "context" "encoding/json" + "errors" "net/url" "testing" @@ -15,7 +16,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" ) -func TestAlertmanagerNotifier(t *testing.T) { +func TestNewAlertmanagerNotifier(t *testing.T) { tmpl := templateForTests(t) externalURL, err := url.Parse("http://localhost") @@ -23,11 +24,61 @@ func TestAlertmanagerNotifier(t *testing.T) { tmpl.ExternalURL = externalURL cases := []struct { - name string - settings string - alerts []*types.Alert - expInitError string - receiverName string + name string + settings string + alerts []*types.Alert + expectedInitError string + receiverName string + }{ + { + name: "Error in initing: missing URL", + settings: `{}`, + expectedInitError: `failed to validate receiver of type "alertmanager": could not find url property in settings`, + }, { + name: "Error in initing: invalid URL", + settings: `{ + "url": "://alertmanager.com" + }`, + expectedInitError: `failed to validate receiver "Alertmanager" of type "alertmanager": invalid url property in settings: parse "://alertmanager.com/api/v1/alerts": missing protocol scheme`, + receiverName: "Alertmanager", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + settingsJSON, err := simplejson.NewJson([]byte(c.settings)) + require.NoError(t, err) + + m := &NotificationChannelConfig{ + Name: c.receiverName, + Type: "alertmanager", + Settings: settingsJSON, + } + + sn, err := NewAlertmanagerNotifier(m, tmpl) + if c.expectedInitError != "" { + require.Equal(t, c.expectedInitError, err.Error()) + return + } + require.NoError(t, err) + require.NotNil(t, sn) + }) + } +} + +func TestAlertmanagerNotifier_Notify(t *testing.T) { + tmpl := templateForTests(t) + + externalURL, err := url.Parse("http://localhost") + require.NoError(t, err) + tmpl.ExternalURL = externalURL + + cases := []struct { + name string + settings string + alerts []*types.Alert + expectedError string + sendHTTPRequestError error + receiverName string }{ { name: "Default config with one alert", @@ -54,16 +105,21 @@ func TestAlertmanagerNotifier(t *testing.T) { }, }, }, { - name: "Error in initing: missing URL", - settings: `{}`, - expInitError: `failed to validate receiver of type "alertmanager": could not find url property in settings`, - }, { - name: "Error in initing: invalid URL", + name: "Error sending to Alertmanager", settings: `{ - "url": "://alertmanager.com" + "url": "https://alertmanager.com" }`, - expInitError: `failed to validate receiver "Alertmanager" of type "alertmanager": invalid url property in settings: parse "://alertmanager.com/api/v1/alerts": missing protocol scheme`, - receiverName: "Alertmanager", + alerts: []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{"__alert_rule_uid__": "rule uid", "alertname": "alert1", "lbl1": "val1"}, + Annotations: model.LabelSet{"ann1": "annv1"}, + }, + }, + }, + expectedError: "failed to send alert to Alertmanager: expected error", + sendHTTPRequestError: errors.New("expected error"), + receiverName: "Alertmanager", }, } for _, c := range cases { @@ -78,10 +134,6 @@ func TestAlertmanagerNotifier(t *testing.T) { } sn, err := NewAlertmanagerNotifier(m, tmpl) - if c.expInitError != "" { - require.Equal(t, c.expInitError, err.Error()) - return - } require.NoError(t, err) var body []byte @@ -91,19 +143,23 @@ func TestAlertmanagerNotifier(t *testing.T) { }) sendHTTPRequest = func(ctx context.Context, url *url.URL, cfg httpCfg, logger log.Logger) ([]byte, error) { body = cfg.body - return nil, nil + return nil, c.sendHTTPRequestError } ctx := notify.WithGroupKey(context.Background(), "alertname") ctx = notify.WithGroupLabels(ctx, model.LabelSet{"alertname": ""}) ok, err := sn.Notify(ctx, c.alerts...) - require.NoError(t, err) - require.True(t, ok) - expBody, err := json.Marshal(c.alerts) - require.NoError(t, err) - - require.JSONEq(t, string(expBody), string(body)) + if c.sendHTTPRequestError != nil { + require.EqualError(t, err, c.expectedError) + require.False(t, ok) + } else { + require.NoError(t, err) + require.True(t, ok) + expBody, err := json.Marshal(c.alerts) + require.NoError(t, err) + require.JSONEq(t, string(expBody), string(body)) + } }) } }