From 3b6a8775bb07d95e5aee3b6c7093f41369b06e25 Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Mon, 8 Jul 2024 12:30:23 -0500 Subject: [PATCH] Alerting: Fix stale values associated with states that have gone to NoData, unify values calculation (#89807) * Unify values * Fix with latest changes on main * Fix up NaN test * Keep refIDs with -1 as value * Test that refIDs are preserved on Normal to Error transition * Alerting to err test too * Add a blurb to docs about this behavior --- .../create-grafana-managed-rule.md | 2 + .../ngalert/api/api_prometheus_test.go | 11 +- pkg/services/ngalert/api/testing.go | 2 +- pkg/services/ngalert/eval/testing.go | 6 + pkg/services/ngalert/state/cache.go | 11 -- pkg/services/ngalert/state/cache_test.go | 28 ----- pkg/services/ngalert/state/manager.go | 3 +- .../ngalert/state/manager_private_test.go | 53 +++++--- pkg/services/ngalert/state/manager_test.go | 117 +++++++++++++++++- pkg/services/ngalert/state/state.go | 44 +++++-- pkg/services/ngalert/state/state_test.go | 14 +-- 11 files changed, 209 insertions(+), 82 deletions(-) diff --git a/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md b/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md index c2b6cf84e48..9ab472e172a 100644 --- a/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md +++ b/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md @@ -269,6 +269,8 @@ You can also configure the alert instance state when its evaluation returns an e | Normal | Sets alert instance state to `Normal`. | | Keep Last State | Maintains the alert instance in its last state. Useful for mitigating temporary issues, refer to [Keep last state](ref:keep-last-state). | +When you configure the No data or Error behavior to `Alerting` or `Normal`, Grafana will attempt to keep a stable set of fields under notification `Values`. If your query returns no data or an error, Grafana re-uses the latest known set of fields in `Values`, but will use `-1` in place of the measured value. + ## Create alerts from panels Create alerts from any panel type. This means you can reuse the queries in the panel and create alerts based on them. diff --git a/pkg/services/ngalert/api/api_prometheus_test.go b/pkg/services/ngalert/api/api_prometheus_test.go index a7fd8016472..478253e5646 100644 --- a/pkg/services/ngalert/api/api_prometheus_test.go +++ b/pkg/services/ngalert/api/api_prometheus_test.go @@ -48,7 +48,7 @@ func Test_FormatValues(t *testing.T) { name: "with no value, it renders the evaluation string", alertState: &state.State{ LastEvaluationString: "[ var='A' metric='vector(10) + time() % 50' labels={} value=1.1 ]", - LatestResult: &state.Evaluation{Condition: "A", Values: map[string]*float64{}}, + LatestResult: &state.Evaluation{Condition: "A", Values: map[string]float64{}}, }, expected: "[ var='A' metric='vector(10) + time() % 50' labels={} value=1.1 ]", }, @@ -56,7 +56,7 @@ func Test_FormatValues(t *testing.T) { name: "with one value, it renders the single value", alertState: &state.State{ LastEvaluationString: "[ var='A' metric='vector(10) + time() % 50' labels={} value=1.1 ]", - LatestResult: &state.Evaluation{Condition: "A", Values: map[string]*float64{"A": &val1}}, + LatestResult: &state.Evaluation{Condition: "A", Values: map[string]float64{"A": val1}}, }, expected: "1.1e+00", }, @@ -64,7 +64,7 @@ func Test_FormatValues(t *testing.T) { name: "with two values, it renders the value based on their refID and position", alertState: &state.State{ LastEvaluationString: "[ var='B0' metric='vector(10) + time() % 50' labels={} value=1.1 ], [ var='B1' metric='vector(10) + time() % 50' labels={} value=1.4 ]", - LatestResult: &state.Evaluation{Condition: "B", Values: map[string]*float64{"B0": &val1, "B1": &val2}}, + LatestResult: &state.Evaluation{Condition: "B", Values: map[string]float64{"B0": val1, "B1": val2}}, }, expected: "B0: 1.1e+00, B1: 1.4e+00", }, @@ -72,7 +72,7 @@ func Test_FormatValues(t *testing.T) { name: "with a high number of values, it renders the value based on their refID and position using a natural order", alertState: &state.State{ LastEvaluationString: "[ var='B0' metric='vector(10) + time() % 50' labels={} value=1.1 ], [ var='B1' metric='vector(10) + time() % 50' labels={} value=1.4 ]", - LatestResult: &state.Evaluation{Condition: "B", Values: map[string]*float64{"B0": &val1, "B1": &val2, "B2": &val1, "B10": &val2, "B11": &val1}}, + LatestResult: &state.Evaluation{Condition: "B", Values: map[string]float64{"B0": val1, "B1": val2, "B2": val1, "B10": val2, "B11": val1}}, }, expected: "B0: 1.1e+00, B10: 1.4e+00, B11: 1.1e+00, B1: 1.4e+00, B2: 1.1e+00", }, @@ -240,11 +240,10 @@ func TestRouteGetAlertStatuses(t *testing.T) { func withAlertingState() forEachState { return func(s *state.State) *state.State { s.State = eval.Alerting - value := float64(1.1) s.LatestResult = &state.Evaluation{ EvaluationState: eval.Alerting, EvaluationTime: timeNow(), - Values: map[string]*float64{"B": &value}, + Values: map[string]float64{"B": float64(1.1)}, Condition: "B", } return s diff --git a/pkg/services/ngalert/api/testing.go b/pkg/services/ngalert/api/testing.go index 3a84fbd6952..8b85525de78 100644 --- a/pkg/services/ngalert/api/testing.go +++ b/pkg/services/ngalert/api/testing.go @@ -86,7 +86,7 @@ func (f *fakeAlertInstanceManager) GenerateAlertInstances(orgID int64, alertRule LatestResult: &state.Evaluation{ EvaluationTime: evaluationTime.Add(1 * time.Minute), EvaluationState: eval.Normal, - Values: make(map[string]*float64), + Values: make(map[string]float64), }, LastEvaluationTime: evaluationTime.Add(1 * time.Minute), EvaluationDuration: evaluationDuration, diff --git a/pkg/services/ngalert/eval/testing.go b/pkg/services/ngalert/eval/testing.go index b06d071011b..d3549f3e2dd 100644 --- a/pkg/services/ngalert/eval/testing.go +++ b/pkg/services/ngalert/eval/testing.go @@ -80,6 +80,12 @@ func WithLabels(labels data.Labels) ResultMutator { } } +func WithValues(values map[string]NumberValueCapture) ResultMutator { + return func(r *Result) { + r.Values = values + } +} + type FakeLoadedMetricsReader struct { fingerprints map[data.Fingerprint]struct{} } diff --git a/pkg/services/ngalert/state/cache.go b/pkg/services/ngalert/state/cache.go index d6fca25632c..4799e6ab127 100644 --- a/pkg/services/ngalert/state/cache.go +++ b/pkg/services/ngalert/state/cache.go @@ -3,7 +3,6 @@ package state import ( "context" "errors" - "math" "net/url" "strings" "sync" @@ -157,15 +156,6 @@ func calculateState(ctx context.Context, log log.Logger, alertRule *ngModels.Ale labels, _ := expand(ctx, log, alertRule.Title, alertRule.Labels, templateData, externalURL, result.EvaluatedAt) annotations, _ := expand(ctx, log, alertRule.Title, alertRule.Annotations, templateData, externalURL, result.EvaluatedAt) - values := make(map[string]float64) - for refID, v := range result.Values { - if v.Value != nil { - values[refID] = *v.Value - } else { - values[refID] = math.NaN() - } - } - lbs := make(data.Labels, len(extraLabels)+len(labels)+len(resultLabels)) dupes := make(data.Labels) for key, val := range extraLabels { @@ -210,7 +200,6 @@ func calculateState(ctx context.Context, log log.Logger, alertRule *ngModels.Ale Labels: lbs, Annotations: annotations, EvaluationDuration: result.EvaluationDuration, - Values: values, StartsAt: result.EvaluatedAt, EndsAt: result.EvaluatedAt, ResultFingerprint: result.Instance.Fingerprint(), // remember original result fingerprint diff --git a/pkg/services/ngalert/state/cache_test.go b/pkg/services/ngalert/state/cache_test.go index 23292714226..fc57ced138c 100644 --- a/pkg/services/ngalert/state/cache_test.go +++ b/pkg/services/ngalert/state/cache_test.go @@ -238,34 +238,6 @@ func Test_getOrCreate(t *testing.T) { } }) - t.Run("expected Reduce and Math expression values", func(t *testing.T) { - result := eval.Result{ - Instance: models.GenerateAlertLabels(5, "result-"), - Values: map[string]eval.NumberValueCapture{ - "A": {Var: "A", Value: util.Pointer(1.0)}, - "B": {Var: "B", Value: util.Pointer(2.0)}, - }, - } - rule := generateRule() - - state := c.getOrCreate(context.Background(), l, rule, result, nil, url) - assert.Equal(t, map[string]float64{"A": 1, "B": 2}, state.Values) - }) - - t.Run("expected Classic Condition values", func(t *testing.T) { - result := eval.Result{ - Instance: models.GenerateAlertLabels(5, "result-"), - Values: map[string]eval.NumberValueCapture{ - "B0": {Var: "B", Value: util.Pointer(1.0)}, - "B1": {Var: "B", Value: util.Pointer(2.0)}, - }, - } - rule := generateRule() - - state := c.getOrCreate(context.Background(), l, rule, result, nil, url) - assert.Equal(t, map[string]float64{"B0": 1, "B1": 2}, state.Values) - }) - t.Run("when result labels collide with system labels from LabelsUserCannotSpecify", func(t *testing.T) { result := eval.Result{ Instance: models.GenerateAlertLabels(5, "result-"), diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go index d1ecc17828e..19c61e64e6b 100644 --- a/pkg/services/ngalert/state/manager.go +++ b/pkg/services/ngalert/state/manager.go @@ -405,10 +405,11 @@ func (st *Manager) setNextState(ctx context.Context, alertRule *ngModels.AlertRu currentState.LastEvaluationTime = result.EvaluatedAt currentState.EvaluationDuration = result.EvaluationDuration + currentState.SetNextValues(result) currentState.LatestResult = &Evaluation{ EvaluationTime: result.EvaluatedAt, EvaluationState: result.State, - Values: NewEvaluationValues(result.Values), + Values: currentState.Values, Condition: alertRule.Condition, } currentState.LastEvaluationString = result.EvaluationString diff --git a/pkg/services/ngalert/state/manager_private_test.go b/pkg/services/ngalert/state/manager_private_test.go index ed996bca211..e263d11155d 100644 --- a/pkg/services/ngalert/state/manager_private_test.go +++ b/pkg/services/ngalert/state/manager_private_test.go @@ -23,6 +23,7 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/eval" "github.com/grafana/grafana/pkg/services/ngalert/metrics" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/util" ) // Not for parallel tests. @@ -149,14 +150,18 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { return ngmodels.CopyRule(baseRule, mutators...) } - newEvaluation := func(evalTime time.Time, evalState eval.State) *Evaluation { + newEvaluationWithValues := func(evalTime time.Time, evalState eval.State, values map[string]float64) *Evaluation { return &Evaluation{ EvaluationTime: evalTime, EvaluationState: evalState, - Values: make(map[string]*float64), + Values: values, } } + newEvaluation := func(evalTime time.Time, evalState eval.State) *Evaluation { + return newEvaluationWithValues(evalTime, evalState, make(map[string]float64)) + } + newResult := func(mutators ...eval.ResultMutator) eval.Result { r := eval.Result{ State: eval.Normal, @@ -894,7 +899,11 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { desc: "t1[1:normal] t2[NoData] at t2", results: map[time.Time]eval.Results{ t1: { - newResult(eval.WithState(eval.Normal), eval.WithLabels(labels1)), + newResult( + eval.WithState(eval.Normal), + eval.WithLabels(labels1), + eval.WithValues(map[string]eval.NumberValueCapture{"A": {Var: "A", Value: util.Pointer(1.0)}}), + ), }, t2: { newResult(eval.WithState(eval.NoData), eval.WithLabels(noDataLabels)), @@ -913,6 +922,7 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { EndsAt: t2.Add(ResendDelay * 4), LastEvaluationTime: t2, LastSentAt: &t2, + Values: map[string]float64{}, }, }, }, @@ -925,11 +935,12 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { Labels: labels["system + rule + no-data"], State: eval.Alerting, StateReason: eval.NoData.String(), - LatestResult: newEvaluation(t2, eval.NoData), + LatestResult: newEvaluationWithValues(t2, eval.NoData, map[string]float64{}), StartsAt: t2, EndsAt: t2.Add(ResendDelay * 4), LastEvaluationTime: t2, LastSentAt: &t2, + Values: map[string]float64{}, }, }, }, @@ -942,10 +953,11 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { Labels: labels["system + rule + no-data"], State: eval.Normal, StateReason: eval.NoData.String(), - LatestResult: newEvaluation(t2, eval.NoData), + LatestResult: newEvaluationWithValues(t2, eval.NoData, map[string]float64{}), StartsAt: t2, EndsAt: t2, LastEvaluationTime: t2, + Values: map[string]float64{}, }, }, }, @@ -976,11 +988,12 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { Labels: labels["system + rule + labels1"], State: eval.Alerting, StateReason: eval.NoData.String(), - LatestResult: newEvaluation(t2, eval.NoData), + LatestResult: newEvaluationWithValues(t2, eval.NoData, map[string]float64{"A": float64(-1)}), StartsAt: t2, EndsAt: t2.Add(ResendDelay * 4), LastEvaluationTime: t2, LastSentAt: &t2, + Values: map[string]float64{"A": float64(-1)}, }, }, }, @@ -993,10 +1006,11 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { Labels: labels["system + rule + labels1"], State: eval.Normal, StateReason: eval.NoData.String(), - LatestResult: newEvaluation(t2, eval.NoData), + LatestResult: newEvaluationWithValues(t2, eval.NoData, map[string]float64{"A": float64(-1)}), StartsAt: t1, EndsAt: t1, LastEvaluationTime: t2, + Values: map[string]float64{"A": float64(-1)}, }, }, }, @@ -1009,10 +1023,11 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { Labels: labels["system + rule + labels1"], State: eval.Normal, StateReason: ngmodels.ConcatReasons(eval.NoData.String(), ngmodels.StateReasonKeepLast), - LatestResult: newEvaluation(t2, eval.NoData), + LatestResult: newEvaluationWithValues(t2, eval.NoData, map[string]float64{"A": float64(-1)}), StartsAt: t1, EndsAt: t1, LastEvaluationTime: t2, + Values: map[string]float64{"A": float64(-1)}, }, }, }, @@ -2807,7 +2822,7 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { ruleMutators: []ngmodels.AlertRuleMutator{ngmodels.RuleMuts.WithForNTimes(1)}, results: map[time.Time]eval.Results{ t1: { - newResult(eval.WithState(eval.Alerting), eval.WithLabels(labels1)), + newResult(eval.WithState(eval.Alerting), eval.WithLabels(labels1), eval.WithValues(map[string]eval.NumberValueCapture{"A": {Var: "A", Value: util.Pointer(1.0)}})), }, t2: { newResult(eval.WithError(datasourceError)), @@ -2895,11 +2910,12 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { State: eval.Alerting, StateReason: eval.Error.String(), Error: datasourceError, - LatestResult: newEvaluation(t2, eval.Error), + LatestResult: newEvaluationWithValues(t2, eval.Error, map[string]float64{"A": float64(-1)}), StartsAt: t2, EndsAt: t2.Add(ResendDelay * 4), LastEvaluationTime: t2, LastSentAt: &t2, + Values: map[string]float64{"A": float64(-1)}, }, }, }, @@ -2912,10 +2928,11 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { Labels: labels["system + rule + labels1"], State: eval.Normal, StateReason: eval.Error.String(), - LatestResult: newEvaluation(t2, eval.Error), + LatestResult: newEvaluationWithValues(t2, eval.Error, map[string]float64{"A": float64(-1)}), StartsAt: t2, EndsAt: t2, LastEvaluationTime: t2, + Values: map[string]float64{"A": float64(-1)}, }, }, }, @@ -2928,11 +2945,12 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { Labels: labels["system + rule + labels1"], State: eval.Alerting, StateReason: ngmodels.ConcatReasons(eval.Error.String(), ngmodels.StateReasonKeepLast), - LatestResult: newEvaluation(t2, eval.Error), + LatestResult: newEvaluationWithValues(t2, eval.Error, map[string]float64{"A": float64(-1)}), StartsAt: t2, EndsAt: t2.Add(ResendDelay * 4), LastEvaluationTime: t2, LastSentAt: &t2, + Values: map[string]float64{"A": float64(-1)}, }, }, }, @@ -2943,7 +2961,7 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { desc: "t1[1:normal] t2[QueryError] at t2", results: map[time.Time]eval.Results{ t1: { - newResult(eval.WithState(eval.Normal), eval.WithLabels(labels1)), + newResult(eval.WithState(eval.Normal), eval.WithLabels(labels1), eval.WithValues(map[string]eval.NumberValueCapture{"A": {Var: "A", Value: util.Pointer(1.0)}})), }, t2: { newResult(eval.WithError(datasourceError)), @@ -3032,11 +3050,12 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { State: eval.Alerting, StateReason: eval.Error.String(), Error: datasourceError, - LatestResult: newEvaluation(t2, eval.Error), + LatestResult: newEvaluationWithValues(t2, eval.Error, map[string]float64{"A": float64(-1)}), StartsAt: t2, EndsAt: t2.Add(ResendDelay * 4), LastEvaluationTime: t2, LastSentAt: &t2, + Values: map[string]float64{"A": float64(-1)}, }, }, }, @@ -3049,10 +3068,11 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { Labels: labels["system + rule + labels1"], State: eval.Normal, StateReason: eval.Error.String(), - LatestResult: newEvaluation(t2, eval.Error), + LatestResult: newEvaluationWithValues(t2, eval.Error, map[string]float64{"A": float64(-1)}), StartsAt: t1, EndsAt: t1, LastEvaluationTime: t2, + Values: map[string]float64{"A": float64(-1)}, }, }, }, @@ -3065,10 +3085,11 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) { Labels: labels["system + rule + labels1"], State: eval.Normal, StateReason: ngmodels.ConcatReasons(eval.Error.String(), ngmodels.StateReasonKeepLast), - LatestResult: newEvaluation(t2, eval.Error), + LatestResult: newEvaluationWithValues(t2, eval.Error, map[string]float64{"A": float64(-1)}), StartsAt: t1, EndsAt: t1, LastEvaluationTime: t2, + Values: map[string]float64{"A": float64(-1)}, }, }, }, diff --git a/pkg/services/ngalert/state/manager_test.go b/pkg/services/ngalert/state/manager_test.go index a15a2a5da27..96f58326986 100644 --- a/pkg/services/ngalert/state/manager_test.go +++ b/pkg/services/ngalert/state/manager_test.go @@ -324,14 +324,18 @@ func TestProcessEvalResults(t *testing.T) { ExecErrState: models.ErrorErrState, } - newEvaluation := func(evalTime time.Time, evalState eval.State) *state.Evaluation { + newEvaluationWithValues := func(evalTime time.Time, evalState eval.State, values map[string]float64) *state.Evaluation { return &state.Evaluation{ EvaluationTime: evalTime, EvaluationState: evalState, - Values: make(map[string]*float64), + Values: values, } } + newEvaluation := func(evalTime time.Time, evalState eval.State) *state.Evaluation { + return newEvaluationWithValues(evalTime, evalState, make(map[string]float64)) + } + baseRuleWith := func(mutators ...models.AlertRuleMutator) *models.AlertRule { r := models.CopyRule(baseRule, mutators...) return r @@ -1378,6 +1382,76 @@ func TestProcessEvalResults(t *testing.T) { }, }, }, + { + desc: "expected Reduce and Math expression values", + alertRule: baseRuleWith(), + expectedAnnotations: 1, + evalResults: map[time.Time]eval.Results{ + t1: { + newResult( + eval.WithState(eval.Alerting), + eval.WithLabels(data.Labels{}), + eval.WithValues(map[string]eval.NumberValueCapture{ + "A": {Var: "A", Labels: data.Labels{}, Value: util.Pointer(1.0)}, + "B": {Var: "B", Labels: data.Labels{}, Value: util.Pointer(2.0)}, + })), + }, + }, + expectedStates: []*state.State{ + { + Labels: labels["system + rule"], + ResultFingerprint: data.Labels{}.Fingerprint(), + State: eval.Alerting, + LatestResult: newEvaluationWithValues(t1, eval.Alerting, map[string]float64{ + "A": 1.0, + "B": 2.0, + }), + StartsAt: t1, + EndsAt: t1.Add(state.ResendDelay * 4), + LastEvaluationTime: t1, + LastSentAt: &t1, + Values: map[string]float64{ + "A": 1.0, + "B": 2.0, + }, + }, + }, + }, + { + desc: "expected Classic Condition values", + alertRule: baseRuleWith(), + expectedAnnotations: 1, + evalResults: map[time.Time]eval.Results{ + t1: { + newResult( + eval.WithState(eval.Alerting), + eval.WithLabels(data.Labels{}), + eval.WithValues(map[string]eval.NumberValueCapture{ + "B0": {Var: "B", Labels: data.Labels{}, Value: util.Pointer(1.0)}, + "B1": {Var: "B", Labels: data.Labels{}, Value: util.Pointer(2.0)}, + })), + }, + }, + expectedStates: []*state.State{ + { + Labels: labels["system + rule"], + ResultFingerprint: data.Labels{}.Fingerprint(), + State: eval.Alerting, + LatestResult: newEvaluationWithValues(t1, eval.Alerting, map[string]float64{ + "B0": 1.0, + "B1": 2.0, + }), + StartsAt: t1, + EndsAt: t1.Add(state.ResendDelay * 4), + LastEvaluationTime: t1, + LastSentAt: &t1, + Values: map[string]float64{ + "B0": 1.0, + "B1": 2.0, + }, + }, + }, + }, } for _, tc := range testCases { @@ -1488,6 +1562,43 @@ func TestProcessEvalResults(t *testing.T) { }) } + t.Run("converts values to NaN if not defined", func(t *testing.T) { + // We set up our own special test for this, since we need special comparison logic - NaN != NaN + instanceStore := &state.FakeInstanceStore{} + clk := clock.NewMock() + cfg := state.ManagerCfg{ + Metrics: metrics.NewNGAlert(prometheus.NewPedanticRegistry()).GetStateMetrics(), + ExternalURL: nil, + InstanceStore: instanceStore, + Images: &state.NotAvailableImageService{}, + Clock: clk, + Historian: &state.FakeHistorian{}, + Tracer: tracing.InitializeTracerForTest(), + Log: log.New("ngalert.state.manager"), + MaxStateSaveConcurrency: 1, + } + st := state.NewManager(cfg, state.NewNoopPersister()) + rule := baseRuleWith() + time := t1 + res := eval.Results{newResult( + eval.WithState(eval.Alerting), + eval.WithLabels(data.Labels{}), + eval.WithEvaluatedAt(t1), + eval.WithValues(map[string]eval.NumberValueCapture{ + "A": {Var: "A", Labels: data.Labels{}, Value: nil}, + }), + )} + + _ = st.ProcessEvalResults(context.Background(), time, rule, res, systemLabels, state.NoopSender) + + states := st.GetStatesForRuleUID(rule.OrgID, rule.UID) + require.Len(t, states, 1) + state := states[0] + require.NotNil(t, state.Values) + require.Contains(t, state.Values, "A") + require.Truef(t, math.IsNaN(state.Values["A"]), "expected NaN but got %v", state.Values["A"]) + }) + t.Run("should save state to database", func(t *testing.T) { instanceStore := &state.FakeInstanceStore{} clk := clock.New() @@ -1623,7 +1734,7 @@ func TestStaleResultsHandler(t *testing.T) { LatestResult: &state.Evaluation{ EvaluationTime: evaluationTime, EvaluationState: eval.Normal, - Values: make(map[string]*float64), + Values: make(map[string]float64), Condition: "A", }, StartsAt: evaluationTime, diff --git a/pkg/services/ngalert/state/state.go b/pkg/services/ngalert/state/state.go index 8bdc883f0af..57e7f36649d 100644 --- a/pkg/services/ngalert/state/state.go +++ b/pkg/services/ngalert/state/state.go @@ -163,6 +163,32 @@ func (a *State) AddErrorAnnotations(err error, rule *models.AlertRule) { } } +func (a *State) SetNextValues(result eval.Result) { + const sentinel = float64(-1) + + // We try to provide a reasonable object for Values in the event of nodata/error. + // In order to not break templates that might refer to refIDs, + // we instead fill values with the latest known set of refIDs, but with a sentinel -1 to indicate that the value didn't exist. + if result.State == eval.NoData || result.State == eval.Error { + placeholder := make(map[string]float64, len(a.Values)) + for refID := range a.Values { + placeholder[refID] = sentinel + } + a.Values = placeholder + return + } + + newValues := make(map[string]float64, len(result.Values)) + for k, v := range result.Values { + if v.Value != nil { + newValues[k] = *v.Value + } else { + newValues[k] = math.NaN() + } + } + a.Values = newValues +} + // IsNormalStateWithNoReason returns true if the state is Normal and reason is empty func IsNormalStateWithNoReason(s *State) bool { return s.State == eval.Normal && s.StateReason == "" @@ -206,16 +232,20 @@ type Evaluation struct { // Values contains the RefID and value of reduce and math expressions. // Classic conditions can have different values for the same RefID as they can include multiple conditions. // For these, we use the index of the condition in addition RefID as the key e.g. "A0, A1, A2, etc.". - Values map[string]*float64 + Values map[string]float64 // Condition is the refID specified as the condition in the alerting rule at the time of the evaluation. Condition string } // NewEvaluationValues returns the labels and values for each RefID in the capture. -func NewEvaluationValues(m map[string]eval.NumberValueCapture) map[string]*float64 { - result := make(map[string]*float64, len(m)) +func NewEvaluationValues(m map[string]eval.NumberValueCapture) map[string]float64 { + result := make(map[string]float64, len(m)) for k, v := range m { - result[k] = v.Value + if v.Value != nil { + result[k] = *v.Value + } else { + result[k] = math.NaN() + } } return result } @@ -486,11 +516,7 @@ func (a *State) GetLastEvaluationValuesForCondition() map[string]float64 { for refID, value := range lastResult.Values { if strings.Contains(refID, lastResult.Condition) { - if value != nil { - r[refID] = *value - continue - } - r[refID] = math.NaN() + r[refID] = value } } diff --git a/pkg/services/ngalert/state/state_test.go b/pkg/services/ngalert/state/state_test.go index f079d90e391..152081c9cf1 100644 --- a/pkg/services/ngalert/state/state_test.go +++ b/pkg/services/ngalert/state/state_test.go @@ -528,9 +528,9 @@ func TestGetLastEvaluationValuesForCondition(t *testing.T) { eval := &Evaluation{ EvaluationTime: time.Time{}, EvaluationState: 0, - Values: map[string]*float64{ - "B": util.Pointer(rand.Float64()), - "A": util.Pointer(expected), + Values: map[string]float64{ + "B": rand.Float64(), + "A": expected, }, Condition: "A", } @@ -543,8 +543,8 @@ func TestGetLastEvaluationValuesForCondition(t *testing.T) { eval := &Evaluation{ EvaluationTime: time.Time{}, EvaluationState: 0, - Values: map[string]*float64{ - "C": util.Pointer(rand.Float64()), + Values: map[string]float64{ + "C": rand.Float64(), }, Condition: "A", } @@ -556,8 +556,8 @@ func TestGetLastEvaluationValuesForCondition(t *testing.T) { eval := &Evaluation{ EvaluationTime: time.Time{}, EvaluationState: 0, - Values: map[string]*float64{ - "A": nil, + Values: map[string]float64{ + "A": math.NaN(), }, Condition: "A", }