From 86b5fbbf60cd42965f96cc347341c7464b030653 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Tue, 10 Jan 2023 16:26:15 -0500 Subject: [PATCH] Alerting: Introduce state manager config structure (#61249) --- pkg/services/ngalert/backtesting/engine.go | 10 ++- pkg/services/ngalert/ngalert.go | 10 ++- .../ngalert/schedule/schedule_unit_test.go | 22 ++++++- pkg/services/ngalert/state/manager.go | 23 ++++--- pkg/services/ngalert/state/manager_test.go | 61 +++++++++++++++++-- 5 files changed, 108 insertions(+), 18 deletions(-) diff --git a/pkg/services/ngalert/backtesting/engine.go b/pkg/services/ngalert/backtesting/engine.go index dfcf1d3e220..af2a9012bf0 100644 --- a/pkg/services/ngalert/backtesting/engine.go +++ b/pkg/services/ngalert/backtesting/engine.go @@ -45,7 +45,15 @@ func NewEngine(appUrl *url.URL, evalFactory eval.EvaluatorFactory) *Engine { return &Engine{ evalFactory: evalFactory, createStateManager: func() stateManager { - return state.NewManager(nil, appUrl, nil, &NoopImageService{}, clock.New(), nil) + cfg := state.ManagerCfg{ + Metrics: nil, + ExternalURL: appUrl, + InstanceStore: nil, + Images: &NoopImageService{}, + Clock: clock.New(), + Historian: nil, + } + return state.NewManager(cfg) }, } } diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index f715455f7ef..beb3722bba8 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -209,7 +209,15 @@ func (ng *AlertNG) init() error { if err != nil { return err } - stateManager := state.NewManager(ng.Metrics.GetStateMetrics(), appUrl, store, ng.imageService, clk, history) + cfg := state.ManagerCfg{ + Metrics: ng.Metrics.GetStateMetrics(), + ExternalURL: appUrl, + InstanceStore: store, + Images: ng.imageService, + Clock: clk, + Historian: history, + } + stateManager := state.NewManager(cfg) scheduler := schedule.NewScheduler(schedCfg, stateManager) // if it is required to include folder title to the alerts, we need to subscribe to changes of alert title diff --git a/pkg/services/ngalert/schedule/schedule_unit_test.go b/pkg/services/ngalert/schedule/schedule_unit_test.go index ffb63451f7c..328a1bb33b6 100644 --- a/pkg/services/ngalert/schedule/schedule_unit_test.go +++ b/pkg/services/ngalert/schedule/schedule_unit_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/benbjohnson/clock" + alertingModels "github.com/grafana/alerting/alerting/models" "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" @@ -20,7 +21,6 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" - alertingModels "github.com/grafana/alerting/alerting/models" "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins" @@ -72,7 +72,15 @@ func TestProcessTicks(t *testing.T) { AlertSender: notifier, Tracer: testTracer, } - st := state.NewManager(testMetrics.GetStateMetrics(), nil, nil, &state.NoopImageService{}, mockedClock, &state.FakeHistorian{}) + managerCfg := state.ManagerCfg{ + Metrics: testMetrics.GetStateMetrics(), + ExternalURL: nil, + InstanceStore: nil, + Images: &state.NoopImageService{}, + Clock: mockedClock, + Historian: &state.FakeHistorian{}, + } + st := state.NewManager(managerCfg) sched := NewScheduler(schedCfg, st) @@ -691,8 +699,16 @@ func setupScheduler(t *testing.T, rs *fakeRulesStore, is *state.FakeInstanceStor AlertSender: senderMock, Tracer: testTracer, } + managerCfg := state.ManagerCfg{ + Metrics: m.GetStateMetrics(), + ExternalURL: nil, + InstanceStore: is, + Images: &state.NoopImageService{}, + Clock: mockedClock, + Historian: &state.FakeHistorian{}, + } + st := state.NewManager(managerCfg) - st := state.NewManager(m.GetStateMetrics(), nil, is, &state.NoopImageService{}, mockedClock, &state.FakeHistorian{}) return NewScheduler(schedCfg, st) } diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go index 298ae4b31a9..fd31375de7b 100644 --- a/pkg/services/ngalert/state/manager.go +++ b/pkg/services/ngalert/state/manager.go @@ -39,17 +39,26 @@ type Manager struct { externalURL *url.URL } -func NewManager(metrics *metrics.State, externalURL *url.URL, instanceStore InstanceStore, images ImageCapturer, clock clock.Clock, historian Historian) *Manager { +type ManagerCfg struct { + Metrics *metrics.State + ExternalURL *url.URL + InstanceStore InstanceStore + Images ImageCapturer + Clock clock.Clock + Historian Historian +} + +func NewManager(cfg ManagerCfg) *Manager { return &Manager{ cache: newCache(), ResendDelay: ResendDelay, // TODO: make this configurable log: log.New("ngalert.state.manager"), - metrics: metrics, - instanceStore: instanceStore, - images: images, - historian: historian, - clock: clock, - externalURL: externalURL, + metrics: cfg.Metrics, + instanceStore: cfg.InstanceStore, + images: cfg.Images, + historian: cfg.Historian, + clock: cfg.Clock, + externalURL: cfg.ExternalURL, } } diff --git a/pkg/services/ngalert/state/manager_test.go b/pkg/services/ngalert/state/manager_test.go index bed9abc09f2..79a4dafef3e 100644 --- a/pkg/services/ngalert/state/manager_test.go +++ b/pkg/services/ngalert/state/manager_test.go @@ -187,7 +187,16 @@ func TestWarmStateCache(t *testing.T) { Labels: labels, } _ = dbstore.SaveAlertInstances(ctx, instance1, instance2, instance3, instance4, instance5) - st := state.NewManager(testMetrics.GetStateMetrics(), nil, dbstore, &state.NoopImageService{}, clock.NewMock(), &state.FakeHistorian{}) + + cfg := state.ManagerCfg{ + Metrics: testMetrics.GetStateMetrics(), + ExternalURL: nil, + InstanceStore: dbstore, + Images: &state.NoopImageService{}, + Clock: clock.NewMock(), + Historian: &state.FakeHistorian{}, + } + st := state.NewManager(cfg) st.Warm(ctx, dbstore) t.Run("instance cache has expected entries", func(t *testing.T) { @@ -211,7 +220,15 @@ func TestDashboardAnnotations(t *testing.T) { fakeAnnoRepo := annotationstest.NewFakeAnnotationsRepo() hist := historian.NewAnnotationBackend(fakeAnnoRepo, &dashboards.FakeDashboardService{}) - st := state.NewManager(testMetrics.GetStateMetrics(), nil, dbstore, &state.NoopImageService{}, clock.New(), hist) + cfg := state.ManagerCfg{ + Metrics: testMetrics.GetStateMetrics(), + ExternalURL: nil, + InstanceStore: dbstore, + Images: &state.NoopImageService{}, + Clock: clock.New(), + Historian: hist, + } + st := state.NewManager(cfg) const mainOrgID int64 = 1 @@ -2192,7 +2209,15 @@ func TestProcessEvalResults(t *testing.T) { for _, tc := range testCases { fakeAnnoRepo := annotationstest.NewFakeAnnotationsRepo() hist := historian.NewAnnotationBackend(fakeAnnoRepo, &dashboards.FakeDashboardService{}) - st := state.NewManager(testMetrics.GetStateMetrics(), nil, &state.FakeInstanceStore{}, &state.NotAvailableImageService{}, clock.New(), hist) + cfg := state.ManagerCfg{ + Metrics: testMetrics.GetStateMetrics(), + ExternalURL: nil, + InstanceStore: &state.FakeInstanceStore{}, + Images: &state.NotAvailableImageService{}, + Clock: clock.New(), + Historian: hist, + } + st := state.NewManager(cfg) t.Run(tc.desc, func(t *testing.T) { for _, res := range tc.evalResults { _ = st.ProcessEvalResults(context.Background(), evaluationTime, tc.alertRule, res, data.Labels{ @@ -2219,7 +2244,15 @@ func TestProcessEvalResults(t *testing.T) { t.Run("should save state to database", func(t *testing.T) { instanceStore := &state.FakeInstanceStore{} clk := clock.New() - st := state.NewManager(testMetrics.GetStateMetrics(), nil, instanceStore, &state.NotAvailableImageService{}, clk, &state.FakeHistorian{}) + cfg := state.ManagerCfg{ + Metrics: testMetrics.GetStateMetrics(), + ExternalURL: nil, + InstanceStore: instanceStore, + Images: &state.NotAvailableImageService{}, + Clock: clk, + Historian: &state.FakeHistorian{}, + } + st := state.NewManager(cfg) rule := models.AlertRuleGen()() var results = eval.GenerateResults(rand.Intn(4)+1, eval.ResultGen(eval.WithEvaluatedAt(clk.Now()))) @@ -2348,7 +2381,15 @@ func TestStaleResultsHandler(t *testing.T) { for _, tc := range testCases { ctx := context.Background() - st := state.NewManager(testMetrics.GetStateMetrics(), nil, dbstore, &state.NoopImageService{}, clock.New(), &state.FakeHistorian{}) + cfg := state.ManagerCfg{ + Metrics: testMetrics.GetStateMetrics(), + ExternalURL: nil, + InstanceStore: dbstore, + Images: &state.NoopImageService{}, + Clock: clock.New(), + Historian: &state.FakeHistorian{}, + } + st := state.NewManager(cfg) st.Warm(ctx, dbstore) existingStatesForRule := st.GetStatesForRuleUID(rule.OrgID, rule.UID) @@ -2419,7 +2460,15 @@ func TestStaleResults(t *testing.T) { store := &state.FakeInstanceStore{} - st := state.NewManager(testMetrics.GetStateMetrics(), nil, store, &state.NoopImageService{}, clk, &state.FakeHistorian{}) + cfg := state.ManagerCfg{ + Metrics: testMetrics.GetStateMetrics(), + ExternalURL: nil, + InstanceStore: store, + Images: &state.NoopImageService{}, + Clock: clk, + Historian: &state.FakeHistorian{}, + } + st := state.NewManager(cfg) rule := models.AlertRuleGen(models.WithFor(0))()