Skip to content

Commit

Permalink
Alerting: Fix error message in ngalert when notifications cannot be s…
Browse files Browse the repository at this point in the history
…ent to alertmanager (#40158)

(cherry picked from commit 8318e45)
  • Loading branch information
grobinson-grafana committed Oct 12, 2021
1 parent fa4eec8 commit 1d94fcf
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 29 deletions.
12 changes: 8 additions & 4 deletions pkg/services/ngalert/notifier/channels/alertmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,22 +79,26 @@ 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,
password: n.basicAuthPassword,
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
Expand Down
106 changes: 81 additions & 25 deletions pkg/services/ngalert/notifier/channels/alertmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package channels
import (
"context"
"encoding/json"
"errors"
"net/url"
"testing"

Expand All @@ -15,19 +16,69 @@ 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")
require.NoError(t, err)
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",
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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))
}
})
}
}

0 comments on commit 1d94fcf

Please sign in to comment.