From 914443c816e5abe355b4481edf28d44cd7b0fd53 Mon Sep 17 00:00:00 2001 From: Kyle Brandt Date: Wed, 28 Apr 2021 11:42:19 -0400 Subject: [PATCH] Alerting: Fix state cache id duplication (#33480) --- .../ngalert/models/instance_labels.go | 9 ++++ pkg/services/ngalert/schedule/schedule.go | 6 ++- pkg/services/ngalert/state/cache.go | 12 ++++- pkg/services/ngalert/state/manager.go | 2 +- pkg/services/ngalert/tests/manager_test.go | 52 +++++++++---------- pkg/services/ngalert/tests/schedule_test.go | 4 +- 6 files changed, 53 insertions(+), 32 deletions(-) diff --git a/pkg/services/ngalert/models/instance_labels.go b/pkg/services/ngalert/models/instance_labels.go index e4b819a64c8..d8bde220fd6 100644 --- a/pkg/services/ngalert/models/instance_labels.go +++ b/pkg/services/ngalert/models/instance_labels.go @@ -38,6 +38,15 @@ func (il *InstanceLabels) ToDB() ([]byte, error) { return []byte{}, fmt.Errorf("database serialization of alerting ng Instance labels is not implemented") } +func (il *InstanceLabels) StringKey() (string, error) { + tl := labelsToTupleLabels(*il) + b, err := json.Marshal(tl) + if err != nil { + return "", fmt.Errorf("can not gereate key due to failure to encode labels: %w", err) + } + return string(b), nil +} + // StringAndHash returns a the json representation of the labels as tuples // sorted by key. It also returns the a hash of that representation. func (il *InstanceLabels) StringAndHash() (string, string, error) { diff --git a/pkg/services/ngalert/schedule/schedule.go b/pkg/services/ngalert/schedule/schedule.go index e5102efffc1..d8d0c7beca6 100644 --- a/pkg/services/ngalert/schedule/schedule.go +++ b/pkg/services/ngalert/schedule/schedule.go @@ -355,10 +355,14 @@ func (sch *schedule) WarmStateCache(st *state.Manager) { } for _, entry := range cmd.Result { lbs := map[string]string(entry.Labels) + cacheId, err := entry.Labels.StringKey() + if err != nil { + sch.log.Error("error getting cacheId for entry", "msg", err.Error()) + } stateForEntry := &state.State{ AlertRuleUID: entry.DefinitionUID, OrgID: entry.DefinitionOrgID, - CacheId: fmt.Sprintf("%s %s", entry.DefinitionUID, lbs), + CacheId: cacheId, Labels: lbs, State: translateInstanceState(entry.CurrentState), Results: []state.Evaluation{}, diff --git a/pkg/services/ngalert/state/cache.go b/pkg/services/ngalert/state/cache.go index 78bd75b29d0..516636b1d60 100644 --- a/pkg/services/ngalert/state/cache.go +++ b/pkg/services/ngalert/state/cache.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/ngalert/eval" ngModels "github.com/grafana/grafana/pkg/services/ngalert/models" prometheusModel "github.com/prometheus/common/model" @@ -14,11 +15,13 @@ import ( type cache struct { states map[string]*State mtxStates sync.RWMutex + log log.Logger } -func newCache() *cache { +func newCache(logger log.Logger) *cache { return &cache{ states: make(map[string]*State), + log: logger, } } @@ -32,7 +35,12 @@ func (c *cache) getOrCreate(alertRule *ngModels.AlertRule, result eval.Result) * lbs[ngModels.NamespaceUIDLabel] = alertRule.NamespaceUID lbs[prometheusModel.AlertNameLabel] = alertRule.Title - id := fmt.Sprintf("%s", map[string]string(lbs)) + il := ngModels.InstanceLabels(lbs) + id, err := il.StringKey() + if err != nil { + c.log.Error("error getting cacheId for entry", "msg", err.Error()) + } + if state, ok := c.states[id]; ok { return state } diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go index 5804aa84cf5..614835533dc 100644 --- a/pkg/services/ngalert/state/manager.go +++ b/pkg/services/ngalert/state/manager.go @@ -17,7 +17,7 @@ type Manager struct { func NewManager(logger log.Logger) *Manager { manager := &Manager{ - cache: newCache(), + cache: newCache(logger), quit: make(chan struct{}), Log: logger, } diff --git a/pkg/services/ngalert/tests/manager_test.go b/pkg/services/ngalert/tests/manager_test.go index 8147ab49c26..e9dc6d286d4 100644 --- a/pkg/services/ngalert/tests/manager_test.go +++ b/pkg/services/ngalert/tests/manager_test.go @@ -51,10 +51,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid alertname:test_title instance_label:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid alertname:test_title instance_label:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid"],["alertname","test_title"],["instance_label","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid", @@ -103,10 +103,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid alertname:test_title instance_label_1:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid"],["alertname","test_title"],["instance_label_1","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid alertname:test_title instance_label_1:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid"],["alertname","test_title"],["instance_label_1","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid", @@ -125,10 +125,10 @@ func TestProcessEvalResults(t *testing.T) { EvaluationDuration: evaluationDuration, Annotations: map[string]string{"annotation": "test"}, }, - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid alertname:test_title instance_label_2:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid"],["alertname","test_title"],["instance_label_2","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid alertname:test_title instance_label_2:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid"],["alertname","test_title"],["instance_label_2","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid", @@ -181,10 +181,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_1 alertname:test_title instance_label:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_1"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid_1", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_1 alertname:test_title instance_label:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_1"],["alertname","test_title"],["instance_label","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid_1", @@ -239,10 +239,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid_2", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid_2", @@ -308,10 +308,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid_2", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid_2", @@ -373,10 +373,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid_2", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid_2", @@ -434,10 +434,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid_2", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid_2", @@ -495,10 +495,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid_2", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid_2", @@ -556,10 +556,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid_2", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid_2", @@ -618,10 +618,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid_2", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid_2", @@ -680,10 +680,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid_2", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid_2", @@ -742,10 +742,10 @@ func TestProcessEvalResults(t *testing.T) { }, }, expectedStates: map[string]*state.State{ - "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]": { + `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`: { AlertRuleUID: "test_alert_rule_uid_2", OrgID: 1, - CacheId: "map[__alert_rule_namespace_uid__:test_namespace_uid __alert_rule_uid__:test_alert_rule_uid_2 alertname:test_title instance_label:test label:test]", + CacheId: `[["__alert_rule_namespace_uid__","test_namespace_uid"],["__alert_rule_uid__","test_alert_rule_uid_2"],["alertname","test_title"],["instance_label","test"],["label","test"]]`, Labels: data.Labels{ "__alert_rule_namespace_uid__": "test_namespace_uid", "__alert_rule_uid__": "test_alert_rule_uid_2", diff --git a/pkg/services/ngalert/tests/schedule_test.go b/pkg/services/ngalert/tests/schedule_test.go index fad9928df74..f605a310d28 100644 --- a/pkg/services/ngalert/tests/schedule_test.go +++ b/pkg/services/ngalert/tests/schedule_test.go @@ -37,7 +37,7 @@ func TestWarmStateCache(t *testing.T) { { AlertRuleUID: "test_uid", OrgID: 123, - CacheId: "test_uid map[test1:testValue1]", + CacheId: `[["test1","testValue1"]]`, Labels: data.Labels{"test1": "testValue1"}, State: eval.Normal, Results: []state.Evaluation{ @@ -49,7 +49,7 @@ func TestWarmStateCache(t *testing.T) { }, { AlertRuleUID: "test_uid", OrgID: 123, - CacheId: "test_uid map[test2:testValue2]", + CacheId: `[["test2","testValue2"]]`, Labels: data.Labels{"test2": "testValue2"}, State: eval.Alerting, Results: []state.Evaluation{