Alerting: Don't record annotations for mapped NoData transitions, when NoData is mapped to OK (#77164)

* Exclude mapped nodata transitions when nodata mapped to OK

* Fix processEvalResults test

* Don't check NoDataState when filtering transition

* Add comment to explain purpose of separate function

---------

Co-authored-by: William Wernert <william.wernert@grafana.com>

pkg/services/ngalert/state/historian/annotation.go

@@ -173,7 +173,7 @@ func (h *AnnotationBackend) Query(ctx context.Context, query ngmodels.HistoryQue
 func buildAnnotations(rule history_model.RuleMeta, states []state.StateTransition, logger log.Logger) []annotations.Item {
 	items := make([]annotations.Item, 0, len(states))
 	for _, state := range states {
-		if !shouldRecord(state) {
+		if !shouldRecordAnnotation(state) {
 			continue
 		}
 		logger.Debug("Alert state changed creating annotation", "newState", state.Formatted(), "oldState", state.PreviousFormatted())

pkg/services/ngalert/state/historian/core.go

@@ -34,6 +34,24 @@ func shouldRecord(transition state.StateTransition) bool {
 	return true
 }
 
+// shouldRecordAnnotation returns true if an annotation should be created for a given state transition.
+// This is stricter than shouldRecord to avoid cluttering panels with state transitions.
+func shouldRecordAnnotation(t state.StateTransition) bool {
+	if !shouldRecord(t) {
+		return false
+	}
+
+	// Do not record transitions between Normal and Normal (NoData)
+	if t.State.State == eval.Normal && t.PreviousState == eval.Normal {
+		if (t.State.StateReason == "" && t.PreviousStateReason == models.StateReasonNoData) ||
+			(t.State.StateReason == models.StateReasonNoData && t.PreviousStateReason == "") {
+			return false
+		}
+	}
+
+	return true
+}
+
 func removePrivateLabels(labels data.Labels) data.Labels {
 	result := make(data.Labels)
 	for k, v := range labels {

pkg/services/ngalert/state/historian/core_test.go

@@ -97,6 +97,58 @@ func TestShouldRecord(t *testing.T) {
 	}
 }
 
+func TestShouldRecordAnnotation(t *testing.T) {
+	transition := func(from eval.State, fromReason string, to eval.State, toReason string) state.StateTransition {
+		return state.StateTransition{
+			PreviousState:       from,
+			PreviousStateReason: fromReason,
+			State:               &state.State{State: to, StateReason: toReason},
+		}
+	}
+
+	t.Run("transitions between Normal and Normal(NoData) not recorded", func(t *testing.T) {
+		forward := transition(eval.Normal, "", eval.Normal, models.StateReasonNoData)
+		backward := transition(eval.Normal, models.StateReasonNoData, eval.Normal, "")
+
+		require.False(t, shouldRecordAnnotation(forward), "Normal -> Normal(NoData) should be false")
+		require.False(t, shouldRecordAnnotation(backward), "Normal(NoData) -> Normal should be false")
+	})
+
+	t.Run("other Normal transitions involving NoData still recorded", func(t *testing.T) {
+		pauseForward := transition(eval.Normal, models.StateReasonNoData, eval.Normal, models.StateReasonPaused)
+		pauseBackward := transition(eval.Normal, models.StateReasonPaused, eval.Normal, models.StateReasonNoData)
+		errorForward := transition(eval.Normal, models.StateReasonNoData, eval.Normal, models.StateReasonError)
+		errorBackward := transition(eval.Normal, models.StateReasonError, eval.Normal, models.StateReasonNoData)
+		missingSeriesBackward := transition(eval.Normal, models.StateReasonMissingSeries, eval.Normal, models.StateReasonNoData)
+
+		require.True(t, shouldRecordAnnotation(pauseForward), "Normal(NoData) -> Normal(Paused) should be true")
+		require.True(t, shouldRecordAnnotation(pauseBackward), "Normal(Paused) -> Normal(NoData) should be true")
+		require.True(t, shouldRecordAnnotation(errorForward), "Normal(NoData) -> Normal(Error) should be true")
+		require.True(t, shouldRecordAnnotation(errorBackward), "Normal(Error) -> Normal(NoData) should be true")
+		require.True(t, shouldRecordAnnotation(missingSeriesBackward), "Normal(MissingSeries) -> Normal(NoData) should be true")
+	})
+
+	t.Run("respects filters in shouldRecord()", func(t *testing.T) {
+		missingSeries := transition(eval.Normal, "", eval.Normal, models.StateReasonMissingSeries)
+		unpause := transition(eval.Normal, models.StateReasonPaused, eval.Normal, "")
+		afterUpdate := transition(eval.Normal, models.StateReasonUpdated, eval.Normal, "")
+
+		require.False(t, shouldRecordAnnotation(missingSeries), "Normal -> Normal(MissingSeries) should be false")
+		require.False(t, shouldRecordAnnotation(unpause), "Normal(Paused) -> Normal should be false")
+		require.False(t, shouldRecordAnnotation(afterUpdate), "Normal(Updated) -> Normal should be false")
+
+		// Smoke test a few basic ones, exhaustive tests for shouldRecord() already exist elsewhere.
+		basicPending := transition(eval.Normal, "", eval.Pending, "")
+		basicAlerting := transition(eval.Pending, "", eval.Alerting, "")
+		basicResolve := transition(eval.Alerting, "", eval.Normal, "")
+		basicError := transition(eval.Normal, "", eval.Error, "")
+		require.True(t, shouldRecordAnnotation(basicPending), "Normal -> Pending should be true")
+		require.True(t, shouldRecordAnnotation(basicAlerting), "Pending -> Alerting should be true")
+		require.True(t, shouldRecordAnnotation(basicResolve), "Alerting -> Normal should be true")
+		require.True(t, shouldRecordAnnotation(basicError), "Normal -> Error should be true")
+	})
+}
+
 func TestRemovePrivateLabels(t *testing.T) {
 	type testCase struct {
 		name string

pkg/services/ngalert/state/manager_test.go

@@ -852,7 +852,7 @@ func TestProcessEvalResults(t *testing.T) {
 					newResult(eval.WithState(eval.NoData), eval.WithLabels(labels1)), // TODO fix it because NoData does not have same labels
 				},
 			},
-			expectedAnnotations: 1,
+			expectedAnnotations: 0,
 			expectedStates: []*state.State{
 				{
 					Labels:      labels["system + rule + labels1"],