From 66020b419cd1626df4784871bd6c513fdda99d25 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Thu, 22 Apr 2021 11:18:25 +0200 Subject: [PATCH] NGAlert: Consolidate on standard errors package (#33249) * NGAlert: Don't use pkg/errors Signed-off-by: Arve Knudsen * Update pkg/services/ngalert/notifier/alertmanager.go Co-authored-by: Will Browne * Fix logging Signed-off-by: Arve Knudsen Co-authored-by: Will Browne --- pkg/services/ngalert/notifier/alertmanager.go | 30 ++++++++++------- pkg/services/ngalert/notifier/alerts.go | 18 +++++----- .../ngalert/notifier/channels/pagerduty.go | 12 +++---- .../ngalert/notifier/channels/slack.go | 7 ++-- pkg/services/ngalert/notifier/config.go | 4 +-- pkg/services/ngalert/notifier/silences.go | 33 ++++++++++--------- 6 files changed, 56 insertions(+), 48 deletions(-) diff --git a/pkg/services/ngalert/notifier/alertmanager.go b/pkg/services/ngalert/notifier/alertmanager.go index 985495ff1cc..b3165a80f21 100644 --- a/pkg/services/ngalert/notifier/alertmanager.go +++ b/pkg/services/ngalert/notifier/alertmanager.go @@ -10,9 +10,10 @@ import ( "sync" "time" + "errors" + gokit_log "github.com/go-kit/kit/log" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" - "github.com/pkg/errors" "github.com/prometheus/alertmanager/dispatch" "github.com/prometheus/alertmanager/inhibit" "github.com/prometheus/alertmanager/nflog" @@ -115,19 +116,19 @@ func (am *Alertmanager) Init() (err error) { nflog.WithSnapshot(filepath.Join(am.WorkingDirPath(), "notifications")), ) if err != nil { - return errors.Wrap(err, "unable to initialize the notification log component of alerting") + return fmt.Errorf("unable to initialize the notification log component of alerting: %w", err) } am.silences, err = silence.New(silence.Options{ SnapshotFile: filepath.Join(am.WorkingDirPath(), "silences"), Retention: retentionNotificationsAndSilences, }) if err != nil { - return errors.Wrap(err, "unable to initialize the silencing component of alerting") + return fmt.Errorf("unable to initialize the silencing component of alerting: %w", err) } am.alerts, err = NewAlertProvider(am.marker) if err != nil { - return errors.Wrap(err, "unable to initialize the alert provider component of alerting") + return fmt.Errorf("unable to initialize the alert provider component of alerting: %w", err) } return nil @@ -136,7 +137,7 @@ func (am *Alertmanager) Init() (err error) { func (am *Alertmanager) Run(ctx context.Context) error { // Make sure dispatcher starts. We can tolerate future reload failures. if err := am.SyncAndApplyConfigFromDatabase(); err != nil { - am.logger.Error(errors.Wrap(err, "unable to sync configuration").Error()) + am.logger.Error("unable to sync configuration", "err", err) } for { @@ -146,7 +147,7 @@ func (am *Alertmanager) Run(ctx context.Context) error { return nil case <-time.After(pollInterval): if err := am.SyncAndApplyConfigFromDatabase(); err != nil { - am.logger.Error(errors.Wrap(err, "unable to sync configuration").Error()) + am.logger.Error("unable to sync configuration", "err", err) } } } @@ -171,7 +172,7 @@ func (am *Alertmanager) StopAndWait() { func (am *Alertmanager) SaveAndApplyConfig(cfg *apimodels.PostableUserConfig) error { rawConfig, err := json.Marshal(&cfg) if err != nil { - return errors.Wrap(err, "failed to serialize to the Alertmanager configuration") + return fmt.Errorf("failed to serialize to the Alertmanager configuration: %w", err) } am.reloadConfigMtx.Lock() @@ -183,10 +184,13 @@ func (am *Alertmanager) SaveAndApplyConfig(cfg *apimodels.PostableUserConfig) er } if err := am.Store.SaveAlertmanagerConfiguration(cmd); err != nil { - return errors.Wrap(err, "failed to save Alertmanager configuration") + return fmt.Errorf("failed to save Alertmanager configuration: %w", err) + } + if err := am.applyConfig(cfg); err != nil { + return fmt.Errorf("unable to reload configuration: %w", err) } - return errors.Wrap(am.applyConfig(cfg), "unable to reload configuration") + return nil } // SyncAndApplyConfigFromDatabase picks the latest config from database and restarts @@ -202,7 +206,7 @@ func (am *Alertmanager) SyncAndApplyConfigFromDatabase() error { if errors.Is(err, store.ErrNoAlertmanagerConfiguration) { q.Result = &ngmodels.AlertConfiguration{AlertmanagerConfiguration: alertmanagerDefaultConfiguration} } else { - return errors.Wrap(err, "unable to get Alertmanager configuration from the database") + return fmt.Errorf("unable to get Alertmanager configuration from the database: %w", err) } } @@ -211,7 +215,11 @@ func (am *Alertmanager) SyncAndApplyConfigFromDatabase() error { return err } - return errors.Wrap(am.applyConfig(cfg), "unable to reload configuration") + if err := am.applyConfig(cfg); err != nil { + return fmt.Errorf("unable to reload configuration: %w", err) + } + + return nil } // ApplyConfig applies a new configuration by re-initializing all components using the configuration provided. diff --git a/pkg/services/ngalert/notifier/alerts.go b/pkg/services/ngalert/notifier/alerts.go index 7d2e8f348ca..8f0d515d5e5 100644 --- a/pkg/services/ngalert/notifier/alerts.go +++ b/pkg/services/ngalert/notifier/alerts.go @@ -1,12 +1,12 @@ package notifier import ( + "fmt" "regexp" "sort" "time" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" - "github.com/pkg/errors" v2 "github.com/prometheus/alertmanager/api/v2" "github.com/prometheus/alertmanager/dispatch" "github.com/prometheus/alertmanager/pkg/labels" @@ -15,9 +15,9 @@ import ( ) var ( - ErrGetAlertsInternal = errors.New("unable to retrieve alerts(s) due to an internal error") - ErrGetAlertsBadPayload = errors.New("unable to retrieve alerts") - ErrGetAlertGroupsBadPayload = errors.New("unable to retrieve alerts groups") + ErrGetAlertsInternal = fmt.Errorf("unable to retrieve alerts(s) due to an internal error") + ErrGetAlertsBadPayload = fmt.Errorf("unable to retrieve alerts") + ErrGetAlertGroupsBadPayload = fmt.Errorf("unable to retrieve alerts groups") ) func (am *Alertmanager) GetAlerts(active, silenced, inhibited bool, filter []string, receivers string) (apimodels.GettableAlerts, error) { @@ -30,13 +30,13 @@ func (am *Alertmanager) GetAlerts(active, silenced, inhibited bool, filter []str matchers, err := parseFilter(filter) if err != nil { am.logger.Error("failed to parse matchers", "err", err) - return nil, errors.Wrap(ErrGetAlertsBadPayload, err.Error()) + return nil, fmt.Errorf("%s: %w", err.Error(), ErrGetAlertsBadPayload) } receiverFilter, err := parseReceivers(receivers) if err != nil { am.logger.Error("failed to parse receiver regex", "err", err) - return nil, errors.Wrap(ErrGetAlertsBadPayload, err.Error()) + return nil, fmt.Errorf("%s: %w", err.Error(), ErrGetAlertsBadPayload) } alerts := am.alerts.GetPending() @@ -73,7 +73,7 @@ func (am *Alertmanager) GetAlerts(active, silenced, inhibited bool, filter []str if err != nil { am.logger.Error("failed to iterate through the alerts", "err", err) - return nil, errors.Wrap(ErrGetAlertsInternal, err.Error()) + return nil, fmt.Errorf("%s: %w", err.Error(), ErrGetAlertsInternal) } sort.Slice(res, func(i, j int) bool { return *res[i].Fingerprint < *res[j].Fingerprint @@ -86,13 +86,13 @@ func (am *Alertmanager) GetAlertGroups(active, silenced, inhibited bool, filter matchers, err := parseFilter(filter) if err != nil { am.logger.Error("msg", "failed to parse matchers", "err", err) - return nil, errors.Wrap(ErrGetAlertGroupsBadPayload, err.Error()) + return nil, fmt.Errorf("%s: %w", err.Error(), ErrGetAlertGroupsBadPayload) } receiverFilter, err := parseReceivers(receivers) if err != nil { am.logger.Error("msg", "failed to compile receiver regex", "err", err) - return nil, errors.Wrap(ErrGetAlertGroupsBadPayload, err.Error()) + return nil, fmt.Errorf("%s: %w", err.Error(), ErrGetAlertGroupsBadPayload) } rf := func(receiverFilter *regexp.Regexp) func(r *dispatch.Route) bool { diff --git a/pkg/services/ngalert/notifier/channels/pagerduty.go b/pkg/services/ngalert/notifier/channels/pagerduty.go index aad74fc478b..4e6d5f8dfb0 100644 --- a/pkg/services/ngalert/notifier/channels/pagerduty.go +++ b/pkg/services/ngalert/notifier/channels/pagerduty.go @@ -3,11 +3,11 @@ package channels import ( "context" "encoding/json" + "fmt" "net/url" "os" gokit_log "github.com/go-kit/kit/log" - "github.com/pkg/errors" "github.com/prometheus/alertmanager/notify" "github.com/prometheus/alertmanager/template" "github.com/prometheus/alertmanager/types" @@ -97,12 +97,12 @@ func (pn *PagerdutyNotifier) Notify(ctx context.Context, as ...*types.Alert) (bo msg, eventType, err := pn.buildPagerdutyMessage(ctx, alerts, as) if err != nil { - return false, errors.Wrap(err, "build pagerduty message") + return false, fmt.Errorf("build pagerduty message: %w", err) } body, err := json.Marshal(msg) if err != nil { - return false, errors.Wrap(err, "marshal json") + return false, fmt.Errorf("marshal json: %w", err) } pn.log.Info("Notifying Pagerduty", "event_type", eventType) @@ -115,7 +115,7 @@ func (pn *PagerdutyNotifier) Notify(ctx context.Context, as ...*types.Alert) (bo }, } if err := bus.DispatchCtx(ctx, cmd); err != nil { - return false, errors.Wrap(err, "send notification to Pagerduty") + return false, fmt.Errorf("send notification to Pagerduty: %w", err) } return true, nil @@ -140,7 +140,7 @@ func (pn *PagerdutyNotifier) buildPagerdutyMessage(ctx context.Context, alerts m for k, v := range pn.CustomDetails { detail, err := pn.tmpl.ExecuteTextString(v, data) if err != nil { - return nil, "", errors.Wrapf(err, "%q: failed to template %q", k, v) + return nil, "", fmt.Errorf("%q: failed to template %q: %w", k, v, err) } details[k] = detail } @@ -172,7 +172,7 @@ func (pn *PagerdutyNotifier) buildPagerdutyMessage(ctx context.Context, alerts m } if tmplErr != nil { - return nil, "", errors.Wrap(tmplErr, "failed to template PagerDuty message") + return nil, "", fmt.Errorf("failed to template PagerDuty message: %w", tmplErr) } return msg, eventType, nil diff --git a/pkg/services/ngalert/notifier/channels/slack.go b/pkg/services/ngalert/notifier/channels/slack.go index 1900b0be82f..ad7a5ec7ceb 100644 --- a/pkg/services/ngalert/notifier/channels/slack.go +++ b/pkg/services/ngalert/notifier/channels/slack.go @@ -11,7 +11,6 @@ import ( "time" gokit_log "github.com/go-kit/kit/log" - "github.com/pkg/errors" "github.com/prometheus/alertmanager/config" "github.com/prometheus/alertmanager/notify" "github.com/prometheus/alertmanager/template" @@ -136,12 +135,12 @@ type attachment struct { func (sn *SlackNotifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { msg, err := sn.buildSlackMessage(ctx, as) if err != nil { - return false, errors.Wrap(err, "build slack message") + return false, fmt.Errorf("build slack message: %w", err) } b, err := json.Marshal(msg) if err != nil { - return false, errors.Wrap(err, "marshal json") + return false, fmt.Errorf("marshal json: %w", err) } cmd := &models.SendWebhookSync{ @@ -227,7 +226,7 @@ func (sn *SlackNotifier) buildSlackMessage(ctx context.Context, as []*types.Aler } if tmplErr != nil { - tmplErr = errors.Wrap(tmplErr, "failed to template Slack message") + tmplErr = fmt.Errorf("failed to template Slack message: %w", tmplErr) } return req, tmplErr diff --git a/pkg/services/ngalert/notifier/config.go b/pkg/services/ngalert/notifier/config.go index 13ee1fd9f30..32d68dc6fe8 100644 --- a/pkg/services/ngalert/notifier/config.go +++ b/pkg/services/ngalert/notifier/config.go @@ -9,8 +9,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" api "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" - - "github.com/pkg/errors" ) func PersistTemplates(cfg *api.PostableUserConfig, path string) ([]string, bool, error) { @@ -79,7 +77,7 @@ func Load(rawConfig []byte) (*api.PostableUserConfig, error) { cfg := &api.PostableUserConfig{} if err := json.Unmarshal(rawConfig, cfg); err != nil { - return nil, errors.Wrap(err, "unable to parse Alertmanager configuration") + return nil, fmt.Errorf("unable to parse Alertmanager configuration: %w", err) } return cfg, nil diff --git a/pkg/services/ngalert/notifier/silences.go b/pkg/services/ngalert/notifier/silences.go index 72e646c3d5c..27a9cd76dbf 100644 --- a/pkg/services/ngalert/notifier/silences.go +++ b/pkg/services/ngalert/notifier/silences.go @@ -1,20 +1,20 @@ package notifier import ( + "errors" "fmt" "time" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" - "github.com/pkg/errors" v2 "github.com/prometheus/alertmanager/api/v2" "github.com/prometheus/alertmanager/silence" ) var ( - ErrGetSilencesInternal = errors.New("unable to retrieve silence(s) due to an internal error") - ErrDeleteSilenceInternal = errors.New("unable to delete silence due to an internal error") - ErrCreateSilenceBadPayload = errors.New("unable to create silence") - ErrListSilencesBadPayload = errors.New("unable to list silences") + ErrGetSilencesInternal = fmt.Errorf("unable to retrieve silence(s) due to an internal error") + ErrDeleteSilenceInternal = fmt.Errorf("unable to delete silence due to an internal error") + ErrCreateSilenceBadPayload = fmt.Errorf("unable to create silence") + ErrListSilencesBadPayload = fmt.Errorf("unable to list silences") ErrSilenceNotFound = silence.ErrNotFound ) @@ -23,13 +23,13 @@ func (am *Alertmanager) ListSilences(filter []string) (apimodels.GettableSilence matchers, err := parseFilter(filter) if err != nil { am.logger.Error("failed to parse matchers", "err", err) - return nil, errors.Wrap(ErrListSilencesBadPayload, err.Error()) + return nil, fmt.Errorf("%s: %w", ErrListSilencesBadPayload.Error(), err) } psils, _, err := am.silences.Query() if err != nil { am.logger.Error(ErrGetSilencesInternal.Error(), "err", err) - return nil, errors.Wrap(ErrGetSilencesInternal, err.Error()) + return nil, fmt.Errorf("%s: %w", ErrGetSilencesInternal.Error(), err) } sils := apimodels.GettableSilences{} @@ -40,7 +40,8 @@ func (am *Alertmanager) ListSilences(filter []string) (apimodels.GettableSilence silence, err := v2.GettableSilenceFromProto(ps) if err != nil { am.logger.Error("unmarshaling from protobuf failed", "err", err) - return apimodels.GettableSilences{}, errors.Wrap(ErrGetSilencesInternal, fmt.Sprintf("failed to convert internal silence to API silence: %v", err.Error())) + return apimodels.GettableSilences{}, fmt.Errorf("%s: failed to convert internal silence to API silence: %w", + ErrGetSilencesInternal.Error(), err) } sils = append(sils, &silence) } @@ -54,7 +55,7 @@ func (am *Alertmanager) ListSilences(filter []string) (apimodels.GettableSilence func (am *Alertmanager) GetSilence(silenceID string) (apimodels.GettableSilence, error) { sils, _, err := am.silences.Query(silence.QIDs(silenceID)) if err != nil { - return apimodels.GettableSilence{}, errors.Wrap(ErrGetSilencesInternal, err.Error()) + return apimodels.GettableSilence{}, fmt.Errorf("%s: %w", ErrGetSilencesInternal.Error(), err) } if len(sils) == 0 { @@ -65,7 +66,8 @@ func (am *Alertmanager) GetSilence(silenceID string) (apimodels.GettableSilence, sil, err := v2.GettableSilenceFromProto(sils[0]) if err != nil { am.logger.Error("unmarshaling from protobuf failed", "err", err) - return apimodels.GettableSilence{}, errors.Wrap(ErrGetSilencesInternal, fmt.Sprintf("failed to convert internal silence to API silence: %v", err.Error())) + return apimodels.GettableSilence{}, fmt.Errorf("%s: failed to convert internal silence to API silence: %w", + ErrGetSilencesInternal.Error(), err) } return sil, nil @@ -76,19 +78,20 @@ func (am *Alertmanager) CreateSilence(ps *apimodels.PostableSilence) (string, er sil, err := v2.PostableSilenceToProto(ps) if err != nil { am.logger.Error("marshaling to protobuf failed", "err", err) - return "", errors.Wrap(ErrCreateSilenceBadPayload, fmt.Sprintf("failed to convert API silence to internal silence: %v", err.Error())) + return "", fmt.Errorf("%s: failed to convert API silence to internal silence: %w", + ErrCreateSilenceBadPayload.Error(), err) } if sil.StartsAt.After(sil.EndsAt) || sil.StartsAt.Equal(sil.EndsAt) { msg := "start time must be before end time" am.logger.Error(msg, "err", "starts_at", sil.StartsAt, "ends_at", sil.EndsAt) - return "", errors.Wrap(ErrCreateSilenceBadPayload, msg) + return "", fmt.Errorf("%s: %w", msg, ErrCreateSilenceBadPayload) } if sil.EndsAt.Before(time.Now()) { msg := "end time can't be in the past" am.logger.Error(msg, "ends_at", sil.EndsAt) - return "", errors.Wrap(ErrCreateSilenceBadPayload, msg) + return "", fmt.Errorf("%s: %w", msg, ErrCreateSilenceBadPayload) } silenceID, err := am.silences.Set(sil) @@ -97,7 +100,7 @@ func (am *Alertmanager) CreateSilence(ps *apimodels.PostableSilence) (string, er if errors.Is(err, silence.ErrNotFound) { return "", ErrSilenceNotFound } - return "", errors.Wrap(ErrCreateSilenceBadPayload, fmt.Sprintf("unable to save silence: %v", err.Error())) + return "", fmt.Errorf("unable to save silence: %s: %w", err.Error(), ErrCreateSilenceBadPayload) } return silenceID, nil @@ -109,7 +112,7 @@ func (am *Alertmanager) DeleteSilence(silenceID string) error { if errors.Is(err, silence.ErrNotFound) { return ErrSilenceNotFound } - return errors.Wrap(ErrDeleteSilenceInternal, err.Error()) + return fmt.Errorf("%s: %w", err.Error(), ErrDeleteSilenceInternal) } return nil