From 37b6fc70670fd1b0c609fb9bdcd167cede498049 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Thu, 24 Feb 2022 17:55:37 +0000 Subject: [PATCH] Alerting: Use expanded labels in dashboard annotations (#45726) (#45858) --- pkg/services/ngalert/schedule/testing.go | 6 +++ pkg/services/ngalert/state/manager.go | 34 +++++++++++----- pkg/services/ngalert/state/manager_test.go | 47 ++++++++++++++++++++++ pkg/services/ngalert/tests/util.go | 5 +++ 4 files changed, 81 insertions(+), 11 deletions(-) diff --git a/pkg/services/ngalert/schedule/testing.go b/pkg/services/ngalert/schedule/testing.go index fbae64209f0..cf12c2aa895 100644 --- a/pkg/services/ngalert/schedule/testing.go +++ b/pkg/services/ngalert/schedule/testing.go @@ -438,6 +438,12 @@ func NewFakeAnnotationsRepo() *FakeAnnotationsRepo { } } +func (repo *FakeAnnotationsRepo) Items() []*annotations.Item { + repo.mtx.Lock() + defer repo.mtx.Unlock() + return repo.items +} + func (repo *FakeAnnotationsRepo) Len() int { repo.mtx.Lock() defer repo.mtx.Unlock() diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go index cc6ef325953..c476547518f 100644 --- a/pkg/services/ngalert/state/manager.go +++ b/pkg/services/ngalert/state/manager.go @@ -5,18 +5,19 @@ import ( "fmt" "net/url" "strconv" + "strings" "time" - "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/annotations" - "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/grafana/grafana/pkg/infra/log" - + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/annotations" "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/services/ngalert/store" + "github.com/grafana/grafana/pkg/services/sqlstore" ) var ResendDelay = 30 * time.Second @@ -185,7 +186,7 @@ func (st *Manager) setNextState(ctx context.Context, alertRule *ngModels.AlertRu st.set(currentState) if oldState != currentState.State { - go st.createAlertAnnotation(ctx, currentState.State, alertRule, result, oldState) + go st.createAlertAnnotation(ctx, alertRule, currentState.Labels, result.EvaluatedAt, currentState.State, oldState) } return currentState } @@ -233,18 +234,19 @@ func translateInstanceState(state ngModels.InstanceStateType) eval.State { } } -func (st *Manager) createAlertAnnotation(ctx context.Context, new eval.State, alertRule *ngModels.AlertRule, result eval.Result, oldState eval.State) { - st.log.Debug("alert state changed creating annotation", "alertRuleUID", alertRule.UID, "newState", new.String(), "oldState", oldState.String()) +func (st *Manager) createAlertAnnotation(ctx context.Context, alertRule *ngModels.AlertRule, labels data.Labels, evaluatedAt time.Time, state eval.State, previousState eval.State) { + st.log.Debug("alert state changed creating annotation", "alertRuleUID", alertRule.UID, "newState", state.String(), "oldState", previousState.String()) - annotationText := fmt.Sprintf("%s {%s} - %s", alertRule.Title, result.Instance.String(), new.String()) + labels = removePrivateLabels(labels) + annotationText := fmt.Sprintf("%s {%s} - %s", alertRule.Title, labels.String(), state.String()) item := &annotations.Item{ AlertId: alertRule.ID, OrgId: alertRule.OrgID, - PrevState: oldState.String(), - NewState: new.String(), + PrevState: previousState.String(), + NewState: state.String(), Text: annotationText, - Epoch: result.EvaluatedAt.UnixNano() / int64(time.Millisecond), + Epoch: evaluatedAt.UnixNano() / int64(time.Millisecond), } dashUid, ok := alertRule.Annotations[ngModels.DashboardUIDAnnotation] @@ -302,3 +304,13 @@ func (st *Manager) staleResultsHandler(ctx context.Context, alertRule *ngModels. func isItStale(lastEval time.Time, intervalSeconds int64) bool { return lastEval.Add(2 * time.Duration(intervalSeconds) * time.Second).Before(time.Now()) } + +func removePrivateLabels(labels data.Labels) data.Labels { + result := make(data.Labels) + for k, v := range labels { + if !strings.HasPrefix(k, "__") && !strings.HasSuffix(k, "__") { + result[k] = v + } + } + return result +} diff --git a/pkg/services/ngalert/state/manager_test.go b/pkg/services/ngalert/state/manager_test.go index 70ea4b58500..2e051eee5c7 100644 --- a/pkg/services/ngalert/state/manager_test.go +++ b/pkg/services/ngalert/state/manager_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "sort" "testing" "time" @@ -27,6 +28,52 @@ import ( var testMetrics = metrics.NewNGAlert(prometheus.NewPedanticRegistry()) +func TestDashboardAnnotations(t *testing.T) { + evaluationTime, err := time.Parse("2006-01-02", "2022-01-01") + require.NoError(t, err) + + ctx := context.Background() + _, dbstore := tests.SetupTestEnv(t, 1) + + st := state.NewManager(log.New("test_stale_results_handler"), testMetrics.GetStateMetrics(), nil, dbstore, dbstore) + + fakeAnnoRepo := schedule.NewFakeAnnotationsRepo() + annotations.SetRepository(fakeAnnoRepo) + + const mainOrgID int64 = 1 + + rule := tests.CreateTestAlertRuleWithLabels(t, ctx, dbstore, 600, mainOrgID, map[string]string{ + "test1": "testValue1", + "test2": "{{ $labels.instance_label }}", + }) + + st.Warm(ctx) + _ = st.ProcessEvalResults(ctx, rule, eval.Results{{ + Instance: data.Labels{"instance_label": "testValue2"}, + State: eval.Alerting, + EvaluatedAt: evaluationTime, + }}) + + expected := []string{rule.Title + " {alertname=" + rule.Title + ", instance_label=testValue2, test1=testValue1, test2=testValue2} - Alerting"} + sort.Strings(expected) + require.Eventuallyf(t, func() bool { + var actual []string + for _, next := range fakeAnnoRepo.Items() { + actual = append(actual, next.Text) + } + sort.Strings(actual) + if len(expected) != len(actual) { + return false + } + for i := 0; i < len(expected); i++ { + if expected[i] != actual[i] { + return false + } + } + return true + }, time.Second, 100*time.Millisecond, "unexpected annotations") +} + func TestProcessEvalResults(t *testing.T) { evaluationTime, err := time.Parse("2006-01-02", "2021-03-25") if err != nil { diff --git a/pkg/services/ngalert/tests/util.go b/pkg/services/ngalert/tests/util.go index 3e11cb1c722..3158b39d6c2 100644 --- a/pkg/services/ngalert/tests/util.go +++ b/pkg/services/ngalert/tests/util.go @@ -52,6 +52,10 @@ func SetupTestEnv(t *testing.T, baseInterval time.Duration) (*ngalert.AlertNG, * // CreateTestAlertRule creates a dummy alert definition to be used by the tests. func CreateTestAlertRule(t *testing.T, ctx context.Context, dbstore *store.DBstore, intervalSeconds int64, orgID int64) *models.AlertRule { + return CreateTestAlertRuleWithLabels(t, ctx, dbstore, intervalSeconds, orgID, nil) +} + +func CreateTestAlertRuleWithLabels(t *testing.T, ctx context.Context, dbstore *store.DBstore, intervalSeconds int64, orgID int64, labels map[string]string) *models.AlertRule { ruleGroup := fmt.Sprintf("ruleGroup-%s", util.GenerateShortUID()) err := dbstore.UpdateRuleGroup(ctx, store.UpdateRuleGroupCmd{ OrgID: orgID, @@ -62,6 +66,7 @@ func CreateTestAlertRule(t *testing.T, ctx context.Context, dbstore *store.DBsto Rules: []apimodels.PostableExtendedRuleNode{ { ApiRuleNode: &apimodels.ApiRuleNode{ + Labels: labels, Annotations: map[string]string{"testAnnoKey": "testAnnoValue"}, }, GrafanaManagedAlert: &apimodels.PostableGrafanaRule{