From e73bb34cc04f121931622b37f1c00ace2486fc53 Mon Sep 17 00:00:00 2001 From: Nihal <38865967+wasim-nihal@users.noreply.github.com> Date: Wed, 27 Nov 2024 01:43:31 +0530 Subject: [PATCH] Alerting: Fix Conflicting Alert Rule Response Has Wrong 'rule_uid' (#95013) * change to return the right conflicting alert rule uid. see https://github.com/grafana/grafana/issues/89755 Signed-off-by: wasim-nihal * correcting the code comment Signed-off-by: wasim-nihal * changes to return the conflicting uid for both insert and update operations Signed-off-by: wasim-nihal * changes to return verbose conflicting alert rule response payload Signed-off-by: wasim-nihal * changes to return verbose conflicting alert rule response payload Signed-off-by: wasim-nihal * Update pkg/services/ngalert/store/alert_rule.go Co-authored-by: Matthew Jacobson --------- Signed-off-by: wasim-nihal Co-authored-by: Matthew Jacobson --- pkg/services/ngalert/models/errors.go | 23 ++++++++++++++++++++-- pkg/services/ngalert/store/alert_rule.go | 25 +++++++++++++++++++----- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/pkg/services/ngalert/models/errors.go b/pkg/services/ngalert/models/errors.go index d2a15039cda..338cc7709f2 100644 --- a/pkg/services/ngalert/models/errors.go +++ b/pkg/services/ngalert/models/errors.go @@ -7,9 +7,12 @@ import ( ) var ( - errAlertRuleConflictMsg = "conflicting alert rule found [rule_uid: '{{ .Public.RuleUID }}', title: '{{ .Public.Title }}', namespace_uid: '{{ .Public.NamespaceUID }}']: {{ .Public.Error }}" - ErrAlertRuleConflictBase = errutil.Conflict("alerting.alert-rule.conflict"). + 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)) 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 }}") @@ -19,6 +22,22 @@ 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, + }, + "Existing": map[string]any{ + "RuleUID": existingPartialRule.UID, + "Title": existingPartialRule.Title, + "NamespaceUID": existingPartialRule.NamespaceUID, + }, + "Error": underlying.Error(), + }, Error: underlying}) +} + func ErrInvalidRelativeTimeRange(refID string, rtr RelativeTimeRange) error { return ErrInvalidRelativeTimeRangeBase.Build(errutil.TemplateData{Public: map[string]any{"RefID": refID, "From": rtr.From, "To": rtr.To}}) } diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 637f5420124..bbc64fe04cb 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -216,7 +217,7 @@ func (st DBstore) InsertAlertRules(ctx context.Context, rules []ngmodels.AlertRu for i := range newRules { if _, err := sess.Insert(&newRules[i]); err != nil { if st.SQLStore.GetDialect().IsUniqueConstraintViolation(err) { - return ruleConstraintViolationToErr(rules[i], err) + return ruleConstraintViolationToErr(sess, rules[i], err, st.Logger) } return fmt.Errorf("failed to create new rules: %w", err) } @@ -273,7 +274,7 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, rules []ngmodels.UpdateR if updated, err := sess.ID(r.Existing.ID).AllCols().Update(converted); err != nil || updated == 0 { if err != nil { if st.SQLStore.GetDialect().IsUniqueConstraintViolation(err) { - return ruleConstraintViolationToErr(r.New, err) + return ruleConstraintViolationToErr(sess, r.New, err, st.Logger) } return fmt.Errorf("failed to update rule [%s] %s: %w", r.New.UID, r.New.Title, err) } @@ -1033,12 +1034,26 @@ func (st DBstore) RenameTimeIntervalInNotificationSettings( return result, nil, st.UpdateAlertRules(ctx, updates) } -func ruleConstraintViolationToErr(rule ngmodels.AlertRule, err error) error { +func ruleConstraintViolationToErr(sess *db.Session, rule ngmodels.AlertRule, err error, logger log.Logger) error { msg := err.Error() if strings.Contains(msg, "UQE_alert_rule_org_id_namespace_uid_title") || strings.Contains(msg, "alert_rule.org_id, alert_rule.namespace_uid, alert_rule.title") { - return ngmodels.ErrAlertRuleConflict(rule, ngmodels.ErrAlertRuleUniqueConstraintViolation) + // return verbose conflicting alert rule error response + // see: https://github.com/grafana/grafana/issues/89755 + var fetched_uid string + var existingPartialAlertRule ngmodels.AlertRule + ok, uid_fetch_err := sess.Table("alert_rule").Cols("uid").Where("org_id = ? AND title = ? AND namespace_uid = ?", rule.OrgID, rule.Title, rule.NamespaceUID).Get(&fetched_uid) + if uid_fetch_err != nil { + logger.Error("Error fetching uid from alert_rule table", "reason", uid_fetch_err.Error()) + } + if ok { + existingPartialAlertRule = ngmodels.AlertRule{UID: fetched_uid, Title: rule.Title, NamespaceUID: rule.NamespaceUID} + } + return ngmodels.ErrAlertRuleConflictVerbose(existingPartialAlertRule, rule, ngmodels.ErrAlertRuleUniqueConstraintViolation) } else if strings.Contains(msg, "UQE_alert_rule_org_id_uid") || strings.Contains(msg, "alert_rule.org_id, alert_rule.uid") { - return ngmodels.ErrAlertRuleConflict(rule, errors.New("rule UID under the same organisation should be unique")) + // 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) }