From e1ce9ceac14cf9a3c95a462cb36566a3b3754a55 Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Thu, 12 Jun 2025 13:27:08 +0200 Subject: [PATCH] Alerting: Simplify alert rule unique constraint violation errors (#106608) Alerting: Simplify alert rule storage unique constraint violation errors --- pkg/services/ngalert/models/errors.go | 30 +++---------------- pkg/services/ngalert/store/alert_rule.go | 16 ++-------- pkg/services/ngalert/store/alert_rule_test.go | 2 +- 3 files changed, 7 insertions(+), 41 deletions(-) diff --git a/pkg/services/ngalert/models/errors.go b/pkg/services/ngalert/models/errors.go index ea2a1b047ab..ceb0eb0571c 100644 --- a/pkg/services/ngalert/models/errors.go +++ b/pkg/services/ngalert/models/errors.go @@ -7,37 +7,15 @@ import ( ) var ( - errAlertRuleConflictMsg = "conflicting alert rule found [rule_uid: '{{ .Public.RuleUID }}', title: '{{ .Public.Title }}', namespace_uid: '{{ .Public.NamespaceUID }}']: {{ .Public.Error }}" - errAlertRuleConflictMsgVerbose = "alert rule [rule_uid: '{{ .Public.New.RuleUID }}', title: '{{ .Public.New.Title }}', namespace_uid: '{{ .Public.New.NamespaceUID }}'] conflicts with existing [rule_uid: '{{ .Public.Existing.RuleUID }}', title: '{{ .Public.Existing.Title }}', namespace_uid: '{{ .Public.Existing.NamespaceUID }}']: {{ .Public.Error }}" - ErrAlertRuleConflictBase = errutil.Conflict("alerting.alert-rule.conflict"). - MustTemplate(errAlertRuleConflictMsg, errutil.WithPublic(errAlertRuleConflictMsg)) - ErrAlertRuleConflictBaseVerbose = errutil.Conflict("alerting.alert-rule.conflict"). - MustTemplate(errAlertRuleConflictMsgVerbose, errutil.WithPublic(errAlertRuleConflictMsgVerbose)) + errAlertRuleConflictMsg = "Failed to save alert rule '{{ .Public.RuleUID }}' in organization {{ .Public.OrgID }} due to conflict: {{ .Public.Error }}" + ErrAlertRuleConflictBase = errutil.Conflict("alerting.alert-rule.conflict").MustTemplate(errAlertRuleConflictMsg, errutil.WithPublic(errAlertRuleConflictMsg)) ErrAlertRuleGroupNotFound = errutil.NotFound("alerting.alert-rule.notFound") ErrInvalidRelativeTimeRangeBase = errutil.BadRequest("alerting.alert-rule.invalidRelativeTime").MustTemplate("Invalid alert rule query {{ .Public.RefID }}: invalid relative time range [From: {{ .Public.From }}, To: {{ .Public.To }}]") ErrConditionNotExistBase = errutil.BadRequest("alerting.alert-rule.conditionNotExist").MustTemplate("Condition {{ .Public.Given }} does not exist, must be one of {{ .Public.Existing }}") ) -func ErrAlertRuleConflict(rule AlertRule, underlying error) error { - return ErrAlertRuleConflictBase.Build(errutil.TemplateData{Public: map[string]any{"RuleUID": rule.UID, "Title": rule.Title, "NamespaceUID": rule.NamespaceUID, "Error": underlying.Error()}, Error: underlying}) -} - -func ErrAlertRuleConflictVerbose(existingPartialRule, rule AlertRule, underlying error) error { - return ErrAlertRuleConflictBaseVerbose.Build(errutil.TemplateData{Public: map[string]any{ - "New": map[string]any{ - "RuleUID": rule.UID, - "Title": rule.Title, - "NamespaceUID": rule.NamespaceUID, - "RuleGroup": rule.RuleGroup, - }, - "Existing": map[string]any{ - "RuleUID": existingPartialRule.UID, - "Title": existingPartialRule.Title, - "NamespaceUID": existingPartialRule.NamespaceUID, - "RuleGroup": existingPartialRule.RuleGroup, - }, - "Error": underlying.Error(), - }, Error: underlying}) +func ErrAlertRuleConflict(ruleUID string, orgID int64, err error) error { + return ErrAlertRuleConflictBase.Build(errutil.TemplateData{Public: map[string]any{"RuleUID": ruleUID, "OrgID": orgID, "Error": err.Error()}, Error: err}) } func ErrInvalidRelativeTimeRange(refID string, rtr RelativeTimeRange) error { diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index a436225eec6..c535bbe4379 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -373,7 +373,7 @@ func (st DBstore) InsertAlertRules(ctx context.Context, user *ngmodels.UserUID, for i := range newRules { if _, err := sess.Insert(&newRules[i]); err != nil { if st.SQLStore.GetDialect().IsUniqueConstraintViolation(err) { - return ruleConstraintViolationToErr(rules[i], err) + return ngmodels.ErrAlertRuleConflict(newRules[i].UID, newRules[i].OrgID, err) } return fmt.Errorf("failed to create new rules: %w", err) } @@ -431,7 +431,7 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, user *ngmodels.UserUID, if updated, err := sess.ID(r.Existing.ID).AllCols().Omit("rule_guid").Update(converted); err != nil || updated == 0 { if err != nil { if st.SQLStore.GetDialect().IsUniqueConstraintViolation(err) { - return ruleConstraintViolationToErr(r.New, err) + return ngmodels.ErrAlertRuleConflict(r.New.UID, r.New.OrgID, err) } return fmt.Errorf("failed to update rule [%s] %s: %w", r.New.UID, r.New.Title, err) } @@ -1180,18 +1180,6 @@ func (st DBstore) RenameTimeIntervalInNotificationSettings( return result, nil, st.UpdateAlertRules(ctx, &ngmodels.AlertingUserUID, updates) } -func ruleConstraintViolationToErr(rule ngmodels.AlertRule, err error) error { - msg := err.Error() - if strings.Contains(msg, "UQE_alert_rule_org_id_uid") || strings.Contains(msg, "alert_rule.org_id, alert_rule.uid") { - // return verbose conflicting alert rule error response - // see: https://github.com/grafana/grafana/issues/89755 - existingPartialAlertRule := ngmodels.AlertRule{UID: rule.UID} - return ngmodels.ErrAlertRuleConflictVerbose(existingPartialAlertRule, rule, errors.New("rule UID under the same organisation should be unique")) - } else { - return ngmodels.ErrAlertRuleConflict(rule, err) - } -} - // GetNamespacesByRuleUID returns a map of rule UIDs to their namespace UID. func (st DBstore) GetNamespacesByRuleUID(ctx context.Context, orgID int64, uids ...string) (map[string]string, error) { result := make(map[string]string) diff --git a/pkg/services/ngalert/store/alert_rule_test.go b/pkg/services/ngalert/store/alert_rule_test.go index e75d90940ac..ba6dd1ebbde 100644 --- a/pkg/services/ngalert/store/alert_rule_test.go +++ b/pkg/services/ngalert/store/alert_rule_test.go @@ -839,7 +839,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) { cp.Title = "unique-test-title" _, err = store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{*cp}) require.ErrorIs(t, err, models.ErrAlertRuleConflictBase) - require.ErrorContains(t, err, "rule UID under the same organisation should be unique") + require.ErrorContains(t, err, fmt.Sprintf("Failed to save alert rule '%s' in organization %d due to conflict", cp.UID, cp.OrgID)) }) t.Run("should emit event when rules are inserted", func(t *testing.T) {