Alerting: Fix state manager to not keep datasource_uid and ref_id lab…

…els in state after Error (#72216)
grafana · Jul 26, 2023 · 78fc3bc · 78fc3bc
1 parent 259a662
commit 78fc3bc
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 0 deletions.
diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go
@@ -282,6 +282,27 @@ func (st *Manager) setNextState(ctx context.Context, alertRule *ngModels.AlertRu
 	// Add the instance to the log context to help correlate log lines for a state
 	logger = logger.New("instance", result.Instance)
 
+	// if the current state is Error but the result is different, then we need o clean up the extra labels
+	// that were added after the state key was calculated
+	// https://github.com/grafana/grafana/blob/1df4d332c982dc5e394201bb2ef35b442727ce63/pkg/services/ngalert/state/state.go#L298-L311
+	// Usually, it happens in the case of classic conditions when the evalResult does not have labels.
+	//
+	// This is temporary change to make sure that the labels are not persistent in the state after it was in Error state
+	// TODO yuri. Remove it in https://github.com/grafana/grafana/pull/68142
+	if currentState.State == eval.Error && result.State != eval.Error {
+		// This is possible because state was updated after the CacheID was calculated.
+		_, curOk := currentState.Labels["ref_id"]
+		_, resOk := result.Instance["ref_id"]
+		if curOk && !resOk {
+			delete(currentState.Labels, "ref_id")
+		}
+		_, curOk = currentState.Labels["datasource_uid"]
+		_, resOk = result.Instance["datasource_uid"]
+		if curOk && !resOk {
+			delete(currentState.Labels, "datasource_uid")
+		}
+	}
+
 	switch result.State {
 	case eval.Normal:
 		logger.Debug("Setting next state", "handler", "resultNormal")

diff --git a/pkg/services/ngalert/state/manager_test.go b/pkg/services/ngalert/state/manager_test.go
@@ -2240,6 +2240,86 @@ func TestProcessEvalResults(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc: "classic condition, execution Error as Error (alerting -> query error -> alerting)",
+			alertRule: &models.AlertRule{
+				OrgID:        1,
+				Title:        "test_title",
+				UID:          "test_alert_rule_uid",
+				NamespaceUID: "test_namespace_uid",
+				Annotations:  map[string]string{},
+				Labels:       map[string]string{"label": "test"},
+				Data: []models.AlertQuery{
+					{
+						RefID:         "A",
+						DatasourceUID: "test-datasource-uid",
+					},
+				},
+				IntervalSeconds: 10,
+				ExecErrState:    models.ErrorErrState,
+			},
+			expectedAnnotations: 2,
+			evalResults: []eval.Results{
+				{
+					eval.Result{
+						Instance:           data.Labels{},
+						State:              eval.Alerting,
+						EvaluatedAt:        evaluationTime,
+						EvaluationDuration: evaluationDuration,
+					},
+					eval.Result{
+						Instance:           data.Labels{},
+						State:              eval.Error,
+						Error:              expr.MakeQueryError("A", "test-datasource-uid", errors.New("this is an error")),
+						EvaluatedAt:        evaluationTime.Add(10 * time.Second),
+						EvaluationDuration: evaluationDuration,
+					},
+					eval.Result{
+						Instance:           data.Labels{},
+						State:              eval.Alerting,
+						EvaluatedAt:        evaluationTime.Add(20 * time.Second),
+						EvaluationDuration: evaluationDuration,
+					},
+				},
+			},
+			expectedStates: map[string]*state.State{
+				`[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid"],["alertname","test_title"],["label","test"]]`: {
+					AlertRuleUID: "test_alert_rule_uid",
+					OrgID:        1,
+					CacheID:      `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid"],["alertname","test_title"],["label","test"]]`,
+					Labels: data.Labels{
+						"__alert_rule_namespace_uid__": "test_namespace_uid",
+						"__alert_rule_uid__":           "test_alert_rule_uid",
+						"alertname":                    "test_title",
+						"label":                        "test",
+					},
+					Values: make(map[string]float64),
+					State:  eval.Alerting,
+					Results: []state.Evaluation{
+						{
+							EvaluationTime:  evaluationTime,
+							EvaluationState: eval.Alerting,
+							Values:          make(map[string]*float64),
+						},
+						{
+							EvaluationTime:  evaluationTime.Add(10 * time.Second),
+							EvaluationState: eval.Error,
+							Values:          make(map[string]*float64),
+						},
+						{
+							EvaluationTime:  evaluationTime.Add(20 * time.Second),
+							EvaluationState: eval.Alerting,
+							Values:          make(map[string]*float64),
+						},
+					},
+					StartsAt:           evaluationTime.Add(20 * time.Second),
+					EndsAt:             evaluationTime.Add(110 * time.Second),
+					LastEvaluationTime: evaluationTime.Add(20 * time.Second),
+					EvaluationDuration: evaluationDuration,
+					Annotations:        map[string]string{},
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {