From 0c4671e31fd77ced27effa5ba116ab0f2804c0c7 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Fri, 27 Jan 2023 16:26:22 -0500 Subject: [PATCH] Alerting: Update historian to ignore transitions from Normal Paused and Updated (#62267) --- pkg/services/ngalert/state/historian/core.go | 12 +++++- .../ngalert/state/historian/core_test.go | 40 +++++++++---------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/pkg/services/ngalert/state/historian/core.go b/pkg/services/ngalert/state/historian/core.go index d8d49868413..d59181cf317 100644 --- a/pkg/services/ngalert/state/historian/core.go +++ b/pkg/services/ngalert/state/historian/core.go @@ -4,6 +4,7 @@ import ( "strings" "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/ngalert/eval" "github.com/grafana/grafana/pkg/services/ngalert/models" @@ -12,8 +13,17 @@ import ( ) func shouldRecord(transition state.StateTransition) bool { + if !transition.Changed() { + return false + } + // Do not log not transitioned states normal states if it was marked as stale - if !transition.Changed() || transition.StateReason == models.StateReasonMissingSeries && transition.PreviousState == eval.Normal && transition.State.State == eval.Normal { + if transition.StateReason == models.StateReasonMissingSeries && transition.PreviousState == eval.Normal && transition.State.State == eval.Normal { + return false + } + // Do not log transition from Normal (Paused|Updated) to Normal + if transition.State.State == eval.Normal && transition.StateReason == "" && + transition.PreviousState == eval.Normal && (transition.PreviousStateReason == models.StateReasonPaused || transition.PreviousStateReason == models.StateReasonUpdated) { return false } return true diff --git a/pkg/services/ngalert/state/historian/core_test.go b/pkg/services/ngalert/state/historian/core_test.go index 0eae1303640..b2f3f522d3c 100644 --- a/pkg/services/ngalert/state/historian/core_test.go +++ b/pkg/services/ngalert/state/historian/core_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/grafana/grafana/pkg/services/ngalert/eval" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/state" @@ -44,10 +45,14 @@ func TestShouldRecord(t *testing.T) { knownReasons := []string{ "", models.StateReasonMissingSeries, + models.StateReasonPaused, + models.StateReasonUpdated, + models.StateReasonRuleDeleted, eval.Error.String(), eval.NoData.String(), } + // all combinations does not reflect the real transitions that could happen, which is a subset. allCombinations := make([]Transition, 0, len(allStates)*len(allStates)*len(knownReasons)*len(knownReasons)) for _, from := range allStates { for _, reasonFrom := range knownReasons { @@ -60,30 +65,23 @@ func TestShouldRecord(t *testing.T) { } negativeTransitions := map[Transition]struct{}{ - noTransition(eval.Normal, ""): {}, - noTransition(eval.Normal, eval.Error.String()): {}, - noTransition(eval.Normal, eval.NoData.String()): {}, - noTransition(eval.Normal, models.StateReasonMissingSeries): {}, - noTransition(eval.Alerting, ""): {}, - noTransition(eval.Alerting, eval.Error.String()): {}, - noTransition(eval.Alerting, eval.NoData.String()): {}, - noTransition(eval.Alerting, models.StateReasonMissingSeries): {}, - noTransition(eval.Pending, ""): {}, - noTransition(eval.Pending, eval.Error.String()): {}, - noTransition(eval.Pending, eval.NoData.String()): {}, - noTransition(eval.Pending, models.StateReasonMissingSeries): {}, - noTransition(eval.NoData, ""): {}, - noTransition(eval.NoData, eval.Error.String()): {}, - noTransition(eval.NoData, eval.NoData.String()): {}, - noTransition(eval.NoData, models.StateReasonMissingSeries): {}, - noTransition(eval.Error, ""): {}, - noTransition(eval.Error, eval.Error.String()): {}, - noTransition(eval.Error, eval.NoData.String()): {}, - noTransition(eval.Error, models.StateReasonMissingSeries): {}, - transition(eval.Normal, "", eval.Normal, models.StateReasonMissingSeries): {}, transition(eval.Normal, eval.Error.String(), eval.Normal, models.StateReasonMissingSeries): {}, transition(eval.Normal, eval.NoData.String(), eval.Normal, models.StateReasonMissingSeries): {}, + + transition(eval.Normal, models.StateReasonPaused, eval.Normal, ""): {}, + transition(eval.Normal, models.StateReasonUpdated, eval.Normal, ""): {}, + + // these transitions are actually not possible + transition(eval.Normal, models.StateReasonRuleDeleted, eval.Normal, models.StateReasonMissingSeries): {}, + transition(eval.Normal, models.StateReasonPaused, eval.Normal, models.StateReasonMissingSeries): {}, + transition(eval.Normal, models.StateReasonUpdated, eval.Normal, models.StateReasonMissingSeries): {}, + } + // add all transitions from reason X(Y) to X(Y) as negative. + for _, s := range allStates { + for _, reason := range knownReasons { + negativeTransitions[noTransition(s, reason)] = struct{}{} + } } for _, tc := range allCombinations {