diff --git a/pkg/services/ngalert/state/historian/annotation.go b/pkg/services/ngalert/state/historian/annotation.go index 573a50194be..e4ae3b9f8d3 100644 --- a/pkg/services/ngalert/state/historian/annotation.go +++ b/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()) diff --git a/pkg/services/ngalert/state/historian/core.go b/pkg/services/ngalert/state/historian/core.go index cf6c3e5cc78..bdaba336807 100644 --- a/pkg/services/ngalert/state/historian/core.go +++ b/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 { diff --git a/pkg/services/ngalert/state/historian/core_test.go b/pkg/services/ngalert/state/historian/core_test.go index d88bb192721..857aaf10c6f 100644 --- a/pkg/services/ngalert/state/historian/core_test.go +++ b/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 diff --git a/pkg/services/ngalert/state/manager_test.go b/pkg/services/ngalert/state/manager_test.go index 32858008667..f6f519ad94a 100644 --- a/pkg/services/ngalert/state/manager_test.go +++ b/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"],