From 537f1fb857cdefabd0bef1d97f87a7966ead12e6 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Tue, 30 Jul 2024 18:07:13 -0400 Subject: [PATCH] Alerting: Fix persisting result fingerprint that is used by recovery threshold (#91224) * fix persister to save result fingerprint * revert change * fmt --- pkg/services/ngalert/state/cache.go | 8 +- pkg/services/ngalert/state/persister_sync.go | 1 + .../ngalert/state/persister_sync_test.go | 78 +++++++++++++++++++ 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/pkg/services/ngalert/state/cache.go b/pkg/services/ngalert/state/cache.go index 99f0dcb885f..28a5391f55d 100644 --- a/pkg/services/ngalert/state/cache.go +++ b/pkg/services/ngalert/state/cache.go @@ -91,10 +91,10 @@ func (c *cache) getOrCreate(ctx context.Context, log log.Logger, alertRule *ngMo states = &ruleStates{states: make(map[data.Fingerprint]*State)} c.states[stateCandidate.OrgID][stateCandidate.AlertRuleUID] = states } - return states.getOrAdd(stateCandidate) + return states.getOrAdd(stateCandidate, log) } -func (rs *ruleStates) getOrAdd(stateCandidate State) *State { +func (rs *ruleStates) getOrAdd(stateCandidate State, log log.Logger) *State { state, ok := rs.states[stateCandidate.CacheID] // Check if the state with this ID already exists. if !ok { @@ -115,6 +115,10 @@ func (rs *ruleStates) getOrAdd(stateCandidate State) *State { } state.Annotations = stateCandidate.Annotations state.Values = stateCandidate.Values + if state.ResultFingerprint != stateCandidate.ResultFingerprint { + log.Info("Result fingerprint has changed", "oldFingerprint", state.ResultFingerprint, "newFingerprint", stateCandidate.ResultFingerprint, "cacheID", state.CacheID, "stateLabels", state.Labels.String()) + state.ResultFingerprint = stateCandidate.ResultFingerprint + } rs.states[stateCandidate.CacheID] = state return state } diff --git a/pkg/services/ngalert/state/persister_sync.go b/pkg/services/ngalert/state/persister_sync.go index fe34d9f9fe5..1aee0bed09e 100644 --- a/pkg/services/ngalert/state/persister_sync.go +++ b/pkg/services/ngalert/state/persister_sync.go @@ -104,6 +104,7 @@ func (a *SyncStatePersister) saveAlertStates(ctx context.Context, states ...Stat CurrentStateEnd: s.EndsAt, ResolvedAt: s.ResolvedAt, LastSentAt: s.LastSentAt, + ResultFingerprint: s.ResultFingerprint.String(), } err = a.store.SaveAlertInstance(ctx, instance) diff --git a/pkg/services/ngalert/state/persister_sync_test.go b/pkg/services/ngalert/state/persister_sync_test.go index 58d63c9cc72..390e97a4137 100644 --- a/pkg/services/ngalert/state/persister_sync_test.go +++ b/pkg/services/ngalert/state/persister_sync_test.go @@ -2,9 +2,13 @@ package state import ( "context" + "errors" "fmt" + "math/rand" "testing" + "time" + "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/component-base/tracing" @@ -100,4 +104,78 @@ func TestSyncPersister_saveAlertStates(t *testing.T) { assert.Containsf(t, savedKeys, key, "state %s (%s) was not saved but should be", tr.State.State, tr.StateReason) } }) + + t.Run("should save expected fields", func(t *testing.T) { + trace := tracing.NewNoopTracerProvider().Tracer("test") + _, span := trace.Start(context.Background(), "") + st := &FakeInstanceStore{} + syncStatePersister := NewSyncStatePersisiter(&logtest.Fake{}, ManagerCfg{ + InstanceStore: st, + MaxStateSaveConcurrency: 1, + }) + + state := &State{ + OrgID: rand.Int63(), + AlertRuleUID: util.GenerateShortUID(), + CacheID: data.Fingerprint(rand.Int63()), + State: eval.Alerting, + StateReason: "TEST", + ResultFingerprint: data.Fingerprint(rand.Int63()), + LatestResult: &Evaluation{ + EvaluationTime: time.Now().Add(1 * time.Minute), + EvaluationState: eval.Alerting, + Values: map[string]float64{ + "A": 1.0, + "B": 2.0, + }, + Condition: "A", + }, + Error: errors.New("test"), + Image: &ngmodels.Image{ + ID: rand.Int63(), + Token: util.GenerateShortUID(), + Path: util.GenerateShortUID(), + URL: util.GenerateShortUID(), + CreatedAt: time.Now().Add(2 * time.Minute), + ExpiresAt: time.Now().Add(3 * time.Minute), + }, + Annotations: ngmodels.GenerateAlertLabels(4, "annotations_"), + Labels: ngmodels.GenerateAlertLabels(4, "labels_"), + Values: map[string]float64{ + "A1": 11.0, + "B1": 12.0, + }, + StartsAt: time.Now().Add(4 * time.Minute), + EndsAt: time.Now().Add(5 * time.Minute), + ResolvedAt: util.Pointer(time.Now().Add(6 * time.Minute)), + LastSentAt: util.Pointer(time.Now().Add(7 * time.Minute)), + LastEvaluationString: util.GenerateShortUID(), + LastEvaluationTime: time.Now().Add(8 * time.Minute), + EvaluationDuration: time.Duration(rand.Intn(100)+1) * time.Second, + } + + transition := StateTransition{ + State: state, + PreviousState: eval.Normal, + PreviousStateReason: util.GenerateShortUID(), + } + + syncStatePersister.Sync(context.Background(), span, []StateTransition{transition}) + + require.Len(t, st.RecordedOps(), 1) + saved := st.RecordedOps()[0].(ngmodels.AlertInstance) + + expectedAlertInstanceKey, err := state.GetAlertInstanceKey() + require.NoError(t, err) + assert.Equal(t, expectedAlertInstanceKey, saved.AlertInstanceKey) + assert.Equal(t, ngmodels.InstanceLabels(state.Labels), saved.Labels) + assert.EqualValues(t, ngmodels.InstanceStateType(state.State.String()), saved.CurrentState) + assert.Equal(t, state.StateReason, saved.CurrentReason) + assert.Equal(t, state.StartsAt, saved.CurrentStateSince) + assert.Equal(t, state.EndsAt, saved.CurrentStateEnd) + assert.Equal(t, state.LastEvaluationTime, saved.LastEvalTime) + assert.Equal(t, state.LastSentAt, saved.LastSentAt) + assert.EqualValues(t, state.ResolvedAt, saved.ResolvedAt) + assert.Equal(t, state.ResultFingerprint.String(), saved.ResultFingerprint) + }) }