Alerting: Log alert warnings for obsolete notifiers when extracting alerts and remove spammy error (#28162)

* Lower level of notification translation messages

* API: Log alert warnings when saving dashboard

* Remove spammy error

* Rename function parameter

* Apply suggestions from code review

Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>

* Apply suggestions from code review

* Fix test

* Change alertValidator return type

* Small fix

* Rename symbol

* Revert "Rename symbol"

This reverts commit 40b619b21a.

* Revert "Small fix"

This reverts commit 2df8319d1d.

* Revert "Change alertValidator return type"

This reverts commit ad933863e3.

* Revert "Fix test"

This reverts commit f728ece2db.

* Revert "Apply suggestions from code review"

This reverts commit f35c5f52af.

* Revert "Apply suggestions from code review"

This reverts commit 7f95800c5f.

* Revert "Rename function parameter"

This reverts commit 95d3e75b00.

* Revert "API: Log alert warnings when saving dashboard"

This reverts commit 1ac5c3f281.

* Conditionally log translation failures

* Fix issue causing test to fail

* Fix test

* Log instead of propagating translations failures due to database errors

Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>

pkg/models/alert_notifications.go

@@ -15,6 +15,7 @@ var (
 	ErrAlertNotificationStateVersionConflict    = errors.New("alert notification state update version conflict")
 	ErrAlertNotificationStateAlreadyExist       = errors.New("alert notification state already exists")
 	ErrAlertNotificationFailedGenerateUniqueUid = errors.New("Failed to generate unique alert notification uid")
+	ErrAlertNotificationFailedTranslateUniqueID = errors.New("Failed to translate Notification Id to Uid")
 	ErrAlertNotificationWithSameNameExists      = errors.New("alert notification with same name already exists")
 	ErrAlertNotificationWithSameUIDExists       = errors.New("alert notification with same uid already exists")
 )

pkg/services/alerting/extractor.go

@@ -74,7 +74,7 @@ func copyJSON(in json.Marshaler) (*simplejson.Json, error) {
 	return simplejson.NewJson(rawJSON)
 }
 
-func (e *DashAlertExtractor) getAlertFromPanels(jsonWithPanels *simplejson.Json, validateAlertFunc func(*models.Alert) bool) ([]*models.Alert, error) {
+func (e *DashAlertExtractor) getAlertFromPanels(jsonWithPanels *simplejson.Json, validateAlertFunc func(*models.Alert) bool, logTranslationFailures bool) ([]*models.Alert, error) {
 	alerts := make([]*models.Alert, 0)
 
 	for _, panelObj := range jsonWithPanels.Get("panels").MustArray() {
@@ -84,7 +84,7 @@ func (e *DashAlertExtractor) getAlertFromPanels(jsonWithPanels *simplejson.Json,
 		// check if the panel is collapsed
 		if collapsed && collapsedJSON.MustBool() {
 			// extract alerts from sub panels for collapsed panels
-			alertSlice, err := e.getAlertFromPanels(panel, validateAlertFunc)
+			alertSlice, err := e.getAlertFromPanels(panel, validateAlertFunc, logTranslationFailures)
 			if err != nil {
 				return nil, err
 			}
@@ -188,7 +188,7 @@ func (e *DashAlertExtractor) getAlertFromPanels(jsonWithPanels *simplejson.Json,
 		alert.Settings = jsonAlert
 
 		// validate
-		_, err = NewRuleFromDBAlert(alert)
+		_, err = NewRuleFromDBAlert(alert, logTranslationFailures)
 		if err != nil {
 			return nil, err
 		}
@@ -209,10 +209,10 @@ func validateAlertRule(alert *models.Alert) bool {
 
 // GetAlerts extracts alerts from the dashboard json and does full validation on the alert json data.
 func (e *DashAlertExtractor) GetAlerts() ([]*models.Alert, error) {
-	return e.extractAlerts(validateAlertRule)
+	return e.extractAlerts(validateAlertRule, true)
 }
 
-func (e *DashAlertExtractor) extractAlerts(validateFunc func(alert *models.Alert) bool) ([]*models.Alert, error) {
+func (e *DashAlertExtractor) extractAlerts(validateFunc func(alert *models.Alert) bool, logTranslationFailures bool) ([]*models.Alert, error) {
 	dashboardJSON, err := copyJSON(e.Dash.Data)
 	if err != nil {
 		return nil, err
@@ -226,7 +226,7 @@ func (e *DashAlertExtractor) extractAlerts(validateFunc func(alert *models.Alert
 	if len(rows) > 0 {
 		for _, rowObj := range rows {
 			row := simplejson.NewFromAny(rowObj)
-			a, err := e.getAlertFromPanels(row, validateFunc)
+			a, err := e.getAlertFromPanels(row, validateFunc, logTranslationFailures)
 			if err != nil {
 				return nil, err
 			}
@@ -234,7 +234,7 @@ func (e *DashAlertExtractor) extractAlerts(validateFunc func(alert *models.Alert
 			alerts = append(alerts, a...)
 		}
 	} else {
-		a, err := e.getAlertFromPanels(dashboardJSON, validateFunc)
+		a, err := e.getAlertFromPanels(dashboardJSON, validateFunc, logTranslationFailures)
 		if err != nil {
 			return nil, err
 		}
@@ -249,6 +249,6 @@ func (e *DashAlertExtractor) extractAlerts(validateFunc func(alert *models.Alert
 // ValidateAlerts validates alerts in the dashboard json but does not require a valid dashboard id
 // in the first validation pass.
 func (e *DashAlertExtractor) ValidateAlerts() error {
-	_, err := e.extractAlerts(func(alert *models.Alert) bool { return alert.OrgId != 0 && alert.PanelId != 0 })
+	_, err := e.extractAlerts(func(alert *models.Alert) bool { return alert.OrgId != 0 && alert.PanelId != 0 }, false)
 	return err
 }

pkg/services/alerting/reader.go

@@ -36,7 +36,7 @@ func (arr *defaultRuleReader) fetch() []*Rule {
 
 	res := make([]*Rule, 0)
 	for _, ruleDef := range cmd.Result {
-		if model, err := NewRuleFromDBAlert(ruleDef); err != nil {
+		if model, err := NewRuleFromDBAlert(ruleDef, false); err != nil {
 			arr.log.Error("Could not build alert model for rule", "ruleId", ruleDef.Id, "error", err)
 		} else {
 			res = append(res, model)

pkg/services/alerting/rule.go

@@ -113,7 +113,7 @@ func getTimeDurationStringToSeconds(str string) (int64, error) {
 
 // NewRuleFromDBAlert maps a db version of
 // alert to an in-memory version.
-func NewRuleFromDBAlert(ruleDef *models.Alert) (*Rule, error) {
+func NewRuleFromDBAlert(ruleDef *models.Alert, logTranslationFailures bool) (*Rule, error) {
 	model := &Rule{}
 	model.ID = ruleDef.Id
 	model.OrgID = ruleDef.OrgId
@@ -140,7 +140,13 @@ func NewRuleFromDBAlert(ruleDef *models.Alert) (*Rule, error) {
 		if id, err := jsonModel.Get("id").Int64(); err == nil {
 			uid, err := translateNotificationIDToUID(id, ruleDef.OrgId)
 			if err != nil {
-				logger.Error("Unable to translate notification id to uid", "error", err.Error(), "dashboardId", model.DashboardID, "alertId", model.ID, "panelId", model.PanelID, "notificationId", id)
+				if !errors.Is(err, models.ErrAlertNotificationFailedTranslateUniqueID) {
+					logger.Error("Failed to tranlate notification id to uid", "error", err.Error(), "dashboardId", model.DashboardID, "alert", model.Name, "panelId", model.PanelID, "notificationId", id)
+				}
+
+				if logTranslationFailures {
+					logger.Warn("Unable to translate notification id to uid", "dashboardId", model.DashboardID, "alert", model.Name, "panelId", model.PanelID, "notificationId", id)
+				}
 			} else {
 				model.Notifications = append(model.Notifications, uid)
 			}
@@ -176,7 +182,6 @@ func NewRuleFromDBAlert(ruleDef *models.Alert) (*Rule, error) {
 func translateNotificationIDToUID(id int64, orgID int64) (string, error) {
 	notificationUID, err := getAlertNotificationUIDByIDAndOrgID(id, orgID)
 	if err != nil {
-		logger.Debug("Failed to translate Notification Id to Uid", "orgID", orgID, "Id", id)
 		return "", err
 	}
 

pkg/services/alerting/rule_test.go

@@ -100,7 +100,7 @@ func TestAlertRuleModel(t *testing.T) {
 					Settings: alertJSON,
 				}
 
-				alertRule, err := NewRuleFromDBAlert(alert)
+				alertRule, err := NewRuleFromDBAlert(alert, false)
 				So(err, ShouldBeNil)
 
 				So(len(alertRule.Conditions), ShouldEqual, 1)
@@ -142,7 +142,7 @@ func TestAlertRuleModel(t *testing.T) {
 				Settings: alertJSON,
 			}
 
-			alertRule, err := NewRuleFromDBAlert(alert)
+			alertRule, err := NewRuleFromDBAlert(alert, false)
 			Convey("swallows the error", func() {
 				So(err, ShouldBeNil)
 				So(alertRule.Notifications, ShouldNotContain, "999")
@@ -175,7 +175,7 @@ func TestAlertRuleModel(t *testing.T) {
 				Settings: alertJSON,
 			}
 
-			alertRule, err := NewRuleFromDBAlert(alert)
+			alertRule, err := NewRuleFromDBAlert(alert, false)
 			So(err, ShouldBeNil)
 			So(alertRule.Frequency, ShouldEqual, 60)
 		})
@@ -213,7 +213,7 @@ func TestAlertRuleModel(t *testing.T) {
 				Settings: alertJSON,
 			}
 
-			_, err := NewRuleFromDBAlert(alert)
+			_, err := NewRuleFromDBAlert(alert, false)
 			So(err, ShouldNotBeNil)
 			So(err.Error(), ShouldEqual, "alert validation error: Neither id nor uid is specified in 'notifications' block, type assertion to string failed AlertId: 1 PanelId: 1 DashboardId: 1")
 		})

pkg/services/alerting/test_rule.go

@@ -35,7 +35,7 @@ func handleAlertTestCommand(cmd *AlertTestCommand) error {
 
 	for _, alert := range alerts {
 		if alert.PanelId == cmd.PanelID {
-			rule, err := NewRuleFromDBAlert(alert)
+			rule, err := NewRuleFromDBAlert(alert, true)
 			if err != nil {
 				return err
 			}

pkg/services/sqlstore/alert_notification.go

@@ -185,7 +185,7 @@ func getAlertNotificationUidInternal(query *models.GetAlertNotificationUidQuery,
 	}
 
 	if len(results) == 0 {
-		return fmt.Errorf("Alert notification [ Id: %v, OrgId: %v ] not found", query.Id, query.OrgId)
+		return models.ErrAlertNotificationFailedTranslateUniqueID
 	}
 
 	query.Result = results[0]

pkg/services/sqlstore/alert_notification_test.go

@@ -4,6 +4,7 @@ package sqlstore
 
 import (
 	"context"
+	"errors"
 	"regexp"
 	"testing"
 	"time"
@@ -384,7 +385,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 				err := ss.GetAlertNotificationUidWithId(query)
 				So(query.Result, ShouldEqual, "")
 				So(err, ShouldNotBeNil)
-				So(err.Error(), ShouldEqual, "Alert notification [ Id: -1, OrgId: 100 ] not found")
+				So(errors.Is(err, models.ErrAlertNotificationFailedTranslateUniqueID), ShouldBeTrue)
 
 				cacheKey := newAlertNotificationUidCacheKey(query.OrgId, query.Id)
 				result, found := ss.CacheService.Get(cacheKey)