From 4b42cd3c1d5e424764ce9b8a004604560fa1a754 Mon Sep 17 00:00:00 2001 From: Yuriy Tseretyan Date: Wed, 22 Jun 2022 12:18:42 -0400 Subject: [PATCH] Alerting: State manager to use clock (#51219) * manager to use clock, to be able to mock real time --- pkg/services/ngalert/ngalert.go | 2 +- pkg/services/ngalert/schedule/schedule_test.go | 4 ++-- pkg/services/ngalert/schedule/schedule_unit_test.go | 2 +- pkg/services/ngalert/state/manager.go | 11 +++++++---- pkg/services/ngalert/state/manager_private_test.go | 4 +++- pkg/services/ngalert/state/manager_test.go | 7 ++++--- 6 files changed, 18 insertions(+), 12 deletions(-) diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index f842b5bd816..ae953825a4f 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -151,7 +151,7 @@ func (ng *AlertNG) init() error { appUrl = nil } - stateManager := state.NewManager(ng.Log, ng.Metrics.GetStateMetrics(), appUrl, store, store, ng.dashboardService, ng.imageService) + stateManager := state.NewManager(ng.Log, ng.Metrics.GetStateMetrics(), appUrl, store, store, ng.dashboardService, ng.imageService, clock.New()) scheduler := schedule.NewScheduler(schedCfg, ng.ExpressionService, appUrl, stateManager, ng.bus) ng.stateManager = stateManager diff --git a/pkg/services/ngalert/schedule/schedule_test.go b/pkg/services/ngalert/schedule/schedule_test.go index 9e354f2b582..4b40331be7d 100644 --- a/pkg/services/ngalert/schedule/schedule_test.go +++ b/pkg/services/ngalert/schedule/schedule_test.go @@ -108,7 +108,7 @@ func TestWarmStateCache(t *testing.T) { Metrics: testMetrics.GetSchedulerMetrics(), AdminConfigPollInterval: 10 * time.Minute, // do not poll in unit tests. } - st := state.NewManager(schedCfg.Logger, testMetrics.GetStateMetrics(), nil, dbstore, dbstore, &dashboards.FakeDashboardService{}, &image.NoopImageService{}) + st := state.NewManager(schedCfg.Logger, testMetrics.GetStateMetrics(), nil, dbstore, dbstore, &dashboards.FakeDashboardService{}, &image.NoopImageService{}, clock.NewMock()) st.Warm(ctx) t.Run("instance cache has expected entries", func(t *testing.T) { @@ -160,7 +160,7 @@ func TestAlertingTicker(t *testing.T) { disabledOrgID: {}, }, } - st := state.NewManager(schedCfg.Logger, testMetrics.GetStateMetrics(), nil, dbstore, dbstore, &dashboards.FakeDashboardService{}, &image.NoopImageService{}) + st := state.NewManager(schedCfg.Logger, testMetrics.GetStateMetrics(), nil, dbstore, dbstore, &dashboards.FakeDashboardService{}, &image.NoopImageService{}, clock.NewMock()) appUrl := &url.URL{ Scheme: "http", Host: "localhost", diff --git a/pkg/services/ngalert/schedule/schedule_unit_test.go b/pkg/services/ngalert/schedule/schedule_unit_test.go index 02f0ce5afa3..8a9708cb635 100644 --- a/pkg/services/ngalert/schedule/schedule_unit_test.go +++ b/pkg/services/ngalert/schedule/schedule_unit_test.go @@ -958,7 +958,7 @@ func setupScheduler(t *testing.T, rs store.RuleStore, is store.InstanceStore, ac Metrics: m.GetSchedulerMetrics(), AdminConfigPollInterval: 10 * time.Minute, // do not poll in unit tests. } - st := state.NewManager(schedCfg.Logger, m.GetStateMetrics(), nil, rs, is, &dashboards.FakeDashboardService{}, &image.NoopImageService{}) + st := state.NewManager(schedCfg.Logger, m.GetStateMetrics(), nil, rs, is, &dashboards.FakeDashboardService{}, &image.NoopImageService{}, clock.NewMock()) appUrl := &url.URL{ Scheme: "http", Host: "localhost", diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go index a462185c974..740ec44f762 100644 --- a/pkg/services/ngalert/state/manager.go +++ b/pkg/services/ngalert/state/manager.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/benbjohnson/clock" "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/grafana/grafana/pkg/infra/log" @@ -35,6 +36,7 @@ type Manager struct { log log.Logger metrics *metrics.State + clock clock.Clock cache *cache quit chan struct{} ResendDelay time.Duration @@ -47,7 +49,7 @@ type Manager struct { func NewManager(logger log.Logger, metrics *metrics.State, externalURL *url.URL, ruleStore store.RuleStore, instanceStore store.InstanceStore, - dashboardService dashboards.DashboardService, imageService image.ImageService) *Manager { + dashboardService dashboards.DashboardService, imageService image.ImageService, clock clock.Clock) *Manager { manager := &Manager{ cache: newCache(logger, metrics, externalURL), quit: make(chan struct{}), @@ -58,6 +60,7 @@ func NewManager(logger log.Logger, metrics *metrics.State, externalURL *url.URL, instanceStore: instanceStore, dashboardService: dashboardService, imageService: imageService, + clock: clock, } go manager.recordMetrics() return manager @@ -272,14 +275,14 @@ func (st *Manager) recordMetrics() { // TODO: parameterize? // Setting to a reasonable default scrape interval for Prometheus. dur := time.Duration(15) * time.Second - ticker := time.NewTicker(dur) + ticker := st.clock.Ticker(dur) for { select { case <-ticker.C: - st.log.Debug("recording state cache metrics", "now", time.Now()) + st.log.Debug("recording state cache metrics", "now", st.clock.Now()) st.cache.recordMetrics() case <-st.quit: - st.log.Debug("stopping state cache metrics recording", "now", time.Now()) + st.log.Debug("stopping state cache metrics recording", "now", st.clock.Now()) ticker.Stop() return } diff --git a/pkg/services/ngalert/state/manager_private_test.go b/pkg/services/ngalert/state/manager_private_test.go index f20014936dd..fed8a0b58c6 100644 --- a/pkg/services/ngalert/state/manager_private_test.go +++ b/pkg/services/ngalert/state/manager_private_test.go @@ -9,6 +9,8 @@ import ( "github.com/stretchr/testify/require" + "github.com/benbjohnson/clock" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/ngalert/eval" @@ -93,7 +95,7 @@ func Test_maybeNewImage(t *testing.T) { imageService := &CountingImageService{} mgr := NewManager(log.NewNopLogger(), &metrics.State{}, nil, &store.FakeRuleStore{}, &store.FakeInstanceStore{}, - &dashboards.FakeDashboardService{}, imageService) + &dashboards.FakeDashboardService{}, imageService, clock.NewMock()) err := mgr.maybeTakeScreenshot(context.Background(), &ngmodels.AlertRule{}, test.state, test.oldState) require.NoError(t, err) if !test.shouldScreenshot { diff --git a/pkg/services/ngalert/state/manager_test.go b/pkg/services/ngalert/state/manager_test.go index 1a0bbe39aee..b8b9238e23c 100644 --- a/pkg/services/ngalert/state/manager_test.go +++ b/pkg/services/ngalert/state/manager_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/benbjohnson/clock" "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" @@ -35,7 +36,7 @@ func TestDashboardAnnotations(t *testing.T) { ctx := context.Background() _, dbstore := tests.SetupTestEnv(t, 1) - st := state.NewManager(log.New("test_stale_results_handler"), testMetrics.GetStateMetrics(), nil, dbstore, dbstore, &dashboards.FakeDashboardService{}, &image.NoopImageService{}) + st := state.NewManager(log.New("test_stale_results_handler"), testMetrics.GetStateMetrics(), nil, dbstore, dbstore, &dashboards.FakeDashboardService{}, &image.NoopImageService{}, clock.New()) fakeAnnoRepo := store.NewFakeAnnotationsRepo() annotations.SetRepository(fakeAnnoRepo) @@ -1880,7 +1881,7 @@ func TestProcessEvalResults(t *testing.T) { } for _, tc := range testCases { - st := state.NewManager(log.New("test_state_manager"), testMetrics.GetStateMetrics(), nil, nil, &store.FakeInstanceStore{}, &dashboards.FakeDashboardService{}, &image.NotAvailableImageService{}) + st := state.NewManager(log.New("test_state_manager"), testMetrics.GetStateMetrics(), nil, nil, &store.FakeInstanceStore{}, &dashboards.FakeDashboardService{}, &image.NotAvailableImageService{}, clock.New()) t.Run(tc.desc, func(t *testing.T) { fakeAnnoRepo := store.NewFakeAnnotationsRepo() annotations.SetRepository(fakeAnnoRepo) @@ -1998,7 +1999,7 @@ func TestStaleResultsHandler(t *testing.T) { for _, tc := range testCases { ctx := context.Background() - st := state.NewManager(log.New("test_stale_results_handler"), testMetrics.GetStateMetrics(), nil, dbstore, dbstore, &dashboards.FakeDashboardService{}, &image.NoopImageService{}) + st := state.NewManager(log.New("test_stale_results_handler"), testMetrics.GetStateMetrics(), nil, dbstore, dbstore, &dashboards.FakeDashboardService{}, &image.NoopImageService{}, clock.New()) st.Warm(ctx) existingStatesForRule := st.GetStatesForRuleUID(rule.OrgID, rule.UID)