From 9977c7ea4337061dfd71d71a5b735b2a11149ee0 Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Fri, 2 Dec 2022 16:02:07 -0600 Subject: [PATCH] Alerting: Simplify scheduler configuration and remove dependency on Grafana-wide settings (#59735) * Make scheduler not depend directly on grafana-wide settings * Re-add missing interval --- pkg/services/ngalert/metrics/ngalert.go | 4 +-- pkg/services/ngalert/ngalert.go | 18 +++++++----- pkg/services/ngalert/schedule/schedule.go | 29 ++++++++++--------- .../ngalert/schedule/schedule_unit_test.go | 28 ++++++++++-------- 4 files changed, 45 insertions(+), 34 deletions(-) diff --git a/pkg/services/ngalert/metrics/ngalert.go b/pkg/services/ngalert/metrics/ngalert.go index 47d5bd4f095..035ed3a3992 100644 --- a/pkg/services/ngalert/metrics/ngalert.go +++ b/pkg/services/ngalert/metrics/ngalert.go @@ -100,7 +100,7 @@ func (ng *NGAlert) GetMultiOrgAlertmanagerMetrics() *MultiOrgAlertmanager { func NewNGAlert(r prometheus.Registerer) *NGAlert { return &NGAlert{ Registerer: r, - schedulerMetrics: newSchedulerMetrics(r), + schedulerMetrics: NewSchedulerMetrics(r), stateMetrics: newStateMetrics(r), multiOrgAlertmanagerMetrics: newMultiOrgAlertmanagerMetrics(r), apiMetrics: newAPIMetrics(r), @@ -125,7 +125,7 @@ func (moa *MultiOrgAlertmanager) GetOrCreateOrgRegistry(id int64) prometheus.Reg return moa.registries.GetOrCreateOrgRegistry(id) } -func newSchedulerMetrics(r prometheus.Registerer) *Scheduler { +func NewSchedulerMetrics(r prometheus.Registerer) *Scheduler { return &Scheduler{ Registerer: r, BehindSeconds: promauto.With(r).NewGauge(prometheus.GaugeOpts{ diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index 12cc93d6ca2..c1ca4c56ec0 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -184,17 +184,21 @@ func (ng *AlertNG) init() error { evalFactory := eval.NewEvaluatorFactory(ng.Cfg.UnifiedAlerting, ng.DataSourceCache, ng.ExpressionService) schedCfg := schedule.SchedulerCfg{ - Cfg: ng.Cfg.UnifiedAlerting, - C: clk, - EvaluatorFactory: evalFactory, - RuleStore: store, - Metrics: ng.Metrics.GetSchedulerMetrics(), - AlertSender: alertsRouter, + MaxAttempts: ng.Cfg.UnifiedAlerting.MaxAttempts, + C: clk, + BaseInterval: ng.Cfg.UnifiedAlerting.BaseInterval, + MinRuleInterval: ng.Cfg.UnifiedAlerting.MinInterval, + DisableGrafanaFolder: ng.Cfg.UnifiedAlerting.ReservedLabels.IsReservedLabelDisabled(models.FolderTitleLabel), + AppURL: appUrl, + EvaluatorFactory: evalFactory, + RuleStore: store, + Metrics: ng.Metrics.GetSchedulerMetrics(), + AlertSender: alertsRouter, } historian := historian.NewAnnotationHistorian(ng.annotationsRepo, ng.dashboardService) stateManager := state.NewManager(ng.Metrics.GetStateMetrics(), appUrl, store, ng.imageService, clk, historian) - scheduler := schedule.NewScheduler(schedCfg, appUrl, stateManager) + 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 if !ng.Cfg.UnifiedAlerting.ReservedLabels.IsReservedLabelDisabled(models.FolderTitleLabel) { diff --git a/pkg/services/ngalert/schedule/schedule.go b/pkg/services/ngalert/schedule/schedule.go index 5a841653eaf..12c6ea4bb62 100644 --- a/pkg/services/ngalert/schedule/schedule.go +++ b/pkg/services/ngalert/schedule/schedule.go @@ -18,7 +18,6 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/state" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" - "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util/ticker" "github.com/benbjohnson/clock" @@ -98,29 +97,33 @@ type schedule struct { // SchedulerCfg is the scheduler configuration. type SchedulerCfg struct { - Cfg setting.UnifiedAlertingSettings - C clock.Clock - EvaluatorFactory eval.EvaluatorFactory - RuleStore RulesStore - Metrics *metrics.Scheduler - AlertSender AlertsSender + MaxAttempts int64 + BaseInterval time.Duration + C clock.Clock + MinRuleInterval time.Duration + DisableGrafanaFolder bool + AppURL *url.URL + EvaluatorFactory eval.EvaluatorFactory + RuleStore RulesStore + Metrics *metrics.Scheduler + AlertSender AlertsSender } // NewScheduler returns a new schedule. -func NewScheduler(cfg SchedulerCfg, appURL *url.URL, stateManager *state.Manager) *schedule { +func NewScheduler(cfg SchedulerCfg, stateManager *state.Manager) *schedule { sch := schedule{ registry: alertRuleInfoRegistry{alertRuleInfo: make(map[ngmodels.AlertRuleKey]*alertRuleInfo)}, - maxAttempts: cfg.Cfg.MaxAttempts, + maxAttempts: cfg.MaxAttempts, clock: cfg.C, - baseInterval: cfg.Cfg.BaseInterval, + baseInterval: cfg.BaseInterval, log: log.New("ngalert.scheduler"), evaluatorFactory: cfg.EvaluatorFactory, ruleStore: cfg.RuleStore, metrics: cfg.Metrics, - appURL: appURL, - disableGrafanaFolder: cfg.Cfg.ReservedLabels.IsReservedLabelDisabled(ngmodels.FolderTitleLabel), + appURL: cfg.AppURL, + disableGrafanaFolder: cfg.DisableGrafanaFolder, stateManager: stateManager, - minRuleInterval: cfg.Cfg.MinInterval, + minRuleInterval: cfg.MinRuleInterval, schedulableAlertRules: alertRulesRegistry{rules: make(map[ngmodels.AlertRuleKey]*ngmodels.AlertRule)}, alertsSender: cfg.AlertSender, } diff --git a/pkg/services/ngalert/schedule/schedule_unit_test.go b/pkg/services/ngalert/schedule/schedule_unit_test.go index bad68ab113a..00cd257ae0c 100644 --- a/pkg/services/ngalert/schedule/schedule_unit_test.go +++ b/pkg/services/ngalert/schedule/schedule_unit_test.go @@ -54,20 +54,22 @@ func TestProcessTicks(t *testing.T) { notifier := &AlertsSenderMock{} notifier.EXPECT().Send(mock.Anything, mock.Anything).Return() - schedCfg := SchedulerCfg{ - Cfg: cfg, - C: mockedClock, - RuleStore: ruleStore, - Metrics: testMetrics.GetSchedulerMetrics(), - AlertSender: notifier, - } - st := state.NewManager(testMetrics.GetStateMetrics(), nil, nil, &state.NoopImageService{}, mockedClock, &state.FakeHistorian{}) - appUrl := &url.URL{ Scheme: "http", Host: "localhost", } - sched := NewScheduler(schedCfg, appUrl, st) + + schedCfg := SchedulerCfg{ + BaseInterval: cfg.BaseInterval, + C: mockedClock, + AppURL: appUrl, + RuleStore: ruleStore, + Metrics: testMetrics.GetSchedulerMetrics(), + AlertSender: notifier, + } + st := state.NewManager(testMetrics.GetStateMetrics(), nil, nil, &state.NoopImageService{}, mockedClock, &state.FakeHistorian{}) + + sched := NewScheduler(schedCfg, st) evalAppliedCh := make(chan evalAppliedInfo, 1) stopAppliedCh := make(chan models.AlertRuleKey, 1) @@ -673,8 +675,10 @@ func setupScheduler(t *testing.T, rs *fakeRulesStore, is *state.FakeInstanceStor } schedCfg := SchedulerCfg{ - Cfg: cfg, + BaseInterval: cfg.BaseInterval, + MaxAttempts: cfg.MaxAttempts, C: mockedClock, + AppURL: appUrl, EvaluatorFactory: evaluator, RuleStore: rs, Metrics: m.GetSchedulerMetrics(), @@ -682,7 +686,7 @@ func setupScheduler(t *testing.T, rs *fakeRulesStore, is *state.FakeInstanceStor } st := state.NewManager(m.GetStateMetrics(), nil, is, &state.NoopImageService{}, mockedClock, &state.FakeHistorian{}) - return NewScheduler(schedCfg, appUrl, st) + return NewScheduler(schedCfg, st) } func withQueryForState(t *testing.T, evalResult eval.State) models.AlertRuleMutator {