From c1034f3118e8137d724bdd2f3a1c5a72a9549fcf Mon Sep 17 00:00:00 2001 From: Kyle Brandt Date: Mon, 3 May 2021 07:19:15 -0400 Subject: [PATCH] Alerting: Create instanceStore (#33587) for https://github.com/grafana/alerting-squad/issues/129 --- pkg/services/ngalert/api/api.go | 2 +- pkg/services/ngalert/models/instance.go | 16 ++-- pkg/services/ngalert/ngalert.go | 18 ++--- pkg/services/ngalert/schedule/schedule.go | 21 +++-- pkg/services/ngalert/store/alert_rule.go | 3 - .../ngalert/store/instance_database.go | 23 +++--- .../ngalert/tests/instance_database_test.go | 76 +++++++++---------- pkg/services/ngalert/tests/schedule_test.go | 19 ++--- 8 files changed, 90 insertions(+), 88 deletions(-) diff --git a/pkg/services/ngalert/api/api.go b/pkg/services/ngalert/api/api.go index 107cfcd6771..6eb54a7e8d2 100644 --- a/pkg/services/ngalert/api/api.go +++ b/pkg/services/ngalert/api/api.go @@ -42,8 +42,8 @@ type API struct { RouteRegister routing.RouteRegister DataService *tsdb.Service Schedule schedule.ScheduleService - Store store.Store RuleStore store.RuleStore + InstanceStore store.InstanceStore AlertingStore store.AlertingStore DataProxy *datasourceproxy.DatasourceProxyService Alertmanager Alertmanager diff --git a/pkg/services/ngalert/models/instance.go b/pkg/services/ngalert/models/instance.go index c4956d4644f..8f0c26affee 100644 --- a/pkg/services/ngalert/models/instance.go +++ b/pkg/services/ngalert/models/instance.go @@ -44,8 +44,8 @@ func (i InstanceStateType) IsValid() bool { // SaveAlertInstanceCommand is the query for saving a new alert instance. type SaveAlertInstanceCommand struct { - DefinitionOrgID int64 - DefinitionUID string + RuleOrgID int64 + RuleUID string Labels InstanceLabels State InstanceStateType LastEvalTime time.Time @@ -56,18 +56,18 @@ type SaveAlertInstanceCommand struct { // GetAlertInstanceQuery is the query for retrieving/deleting an alert definition by ID. // nolint:unused type GetAlertInstanceQuery struct { - DefinitionOrgID int64 - DefinitionUID string - Labels InstanceLabels + RuleOrgID int64 + RuleUID string + Labels InstanceLabels Result *AlertInstance } // ListAlertInstancesQuery is the query list alert Instances. type ListAlertInstancesQuery struct { - DefinitionOrgID int64 `json:"-"` - DefinitionUID string - State InstanceStateType + RuleOrgID int64 `json:"-"` + RuleUID string + State InstanceStateType Result []*ListAlertInstancesQueryResult } diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index da14b41ca49..f9423051634 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -65,14 +65,14 @@ func (ng *AlertNG) Init() error { store := store.DBstore{BaseInterval: baseInterval, DefaultIntervalSeconds: defaultIntervalSeconds, SQLStore: ng.SQLStore} schedCfg := schedule.SchedulerCfg{ - C: clock.New(), - BaseInterval: baseInterval, - Logger: ng.Log, - MaxAttempts: maxAttempts, - Evaluator: eval.Evaluator{Cfg: ng.Cfg}, - Store: store, - RuleStore: store, - Notifier: ng.Alertmanager, + C: clock.New(), + BaseInterval: baseInterval, + Logger: ng.Log, + MaxAttempts: maxAttempts, + Evaluator: eval.Evaluator{Cfg: ng.Cfg}, + InstanceStore: store, + RuleStore: store, + Notifier: ng.Alertmanager, } ng.schedule = schedule.NewScheduler(schedCfg, ng.DataService) @@ -83,7 +83,7 @@ func (ng *AlertNG) Init() error { DataService: ng.DataService, Schedule: ng.schedule, DataProxy: ng.DataProxy, - Store: store, + InstanceStore: store, RuleStore: store, AlertingStore: store, Alertmanager: ng.Alertmanager, diff --git a/pkg/services/ngalert/schedule/schedule.go b/pkg/services/ngalert/schedule/schedule.go index 4f4630fedb3..fcb6541b133 100644 --- a/pkg/services/ngalert/schedule/schedule.go +++ b/pkg/services/ngalert/schedule/schedule.go @@ -146,10 +146,10 @@ type schedule struct { evaluator eval.Evaluator - store store.Store - ruleStore store.RuleStore + instanceStore store.InstanceStore + dataService *tsdb.Service notifier Notifier @@ -164,8 +164,8 @@ type SchedulerCfg struct { MaxAttempts int64 StopAppliedFunc func(models.AlertRuleKey) Evaluator eval.Evaluator - Store store.Store RuleStore store.RuleStore + InstanceStore store.InstanceStore Notifier Notifier } @@ -182,8 +182,8 @@ func NewScheduler(cfg SchedulerCfg, dataService *tsdb.Service) *schedule { evalAppliedFunc: cfg.EvalAppliedFunc, stopAppliedFunc: cfg.StopAppliedFunc, evaluator: cfg.Evaluator, - store: cfg.Store, ruleStore: cfg.RuleStore, + instanceStore: cfg.InstanceStore, dataService: dataService, notifier: cfg.Notifier, } @@ -321,15 +321,15 @@ func (sch *schedule) saveAlertStates(states []*state.State) { sch.log.Debug("saving alert states", "count", len(states)) for _, s := range states { cmd := models.SaveAlertInstanceCommand{ - DefinitionOrgID: s.OrgID, - DefinitionUID: s.AlertRuleUID, + RuleOrgID: s.OrgID, + RuleUID: s.AlertRuleUID, Labels: models.InstanceLabels(s.Labels), State: models.InstanceStateType(s.State.String()), LastEvalTime: s.LastEvaluationTime, CurrentStateSince: s.StartsAt, CurrentStateEnd: s.EndsAt, } - err := sch.store.SaveAlertInstance(&cmd) + err := sch.instanceStore.SaveAlertInstance(&cmd) if err != nil { sch.log.Error("failed to save alert state", "uid", s.AlertRuleUID, "orgId", s.OrgID, "labels", s.Labels.String(), "state", s.State.String(), "msg", err.Error()) } @@ -341,8 +341,7 @@ func (sch *schedule) WarmStateCache(st *state.Manager) { st.ResetCache() orgIdsCmd := models.FetchUniqueOrgIdsQuery{} - - if err := sch.store.FetchOrgIds(&orgIdsCmd); err != nil { + if err := sch.instanceStore.FetchOrgIds(&orgIdsCmd); err != nil { sch.log.Error("unable to fetch orgIds", "msg", err.Error()) } @@ -363,9 +362,9 @@ func (sch *schedule) WarmStateCache(st *state.Manager) { // Get Instances cmd := models.ListAlertInstancesQuery{ - DefinitionOrgID: orgIdResult.DefinitionOrgID, + RuleOrgID: orgIdResult.DefinitionOrgID, } - if err := sch.ruleStore.ListAlertInstances(&cmd); err != nil { + if err := sch.instanceStore.ListAlertInstances(&cmd); err != nil { sch.log.Error("unable to fetch previous state", "msg", err.Error()) } diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 1b2b7e47a9c..c9323872e00 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -50,9 +50,6 @@ type RuleStore interface { GetOrgRuleGroups(query *ngmodels.ListOrgRuleGroupsQuery) error UpsertAlertRules([]UpsertRule) error UpdateRuleGroup(UpdateRuleGroupCmd) error - GetAlertInstance(*ngmodels.GetAlertInstanceQuery) error - ListAlertInstances(cmd *ngmodels.ListAlertInstancesQuery) error - SaveAlertInstance(cmd *ngmodels.SaveAlertInstanceCommand) error } func getAlertRuleByUID(sess *sqlstore.DBSession, alertRuleUID string, orgID int64) (*ngmodels.AlertRule, error) { diff --git a/pkg/services/ngalert/store/instance_database.go b/pkg/services/ngalert/store/instance_database.go index 563622bdf51..9b796fd5bc1 100644 --- a/pkg/services/ngalert/store/instance_database.go +++ b/pkg/services/ngalert/store/instance_database.go @@ -9,9 +9,15 @@ import ( "github.com/grafana/grafana/pkg/services/sqlstore" ) +type InstanceStore interface { + GetAlertInstance(cmd *models.GetAlertInstanceQuery) error + ListAlertInstances(cmd *models.ListAlertInstancesQuery) error + SaveAlertInstance(cmd *models.SaveAlertInstanceCommand) error + FetchOrgIds(cmd *models.FetchUniqueOrgIdsQuery) error +} + // GetAlertInstance is a handler for retrieving an alert instance based on OrgId, AlertDefintionID, and // the hash of the labels. -// nolint:unused func (st DBstore) GetAlertInstance(cmd *models.GetAlertInstanceQuery) error { return st.SQLStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { instance := models.AlertInstance{} @@ -28,11 +34,11 @@ func (st DBstore) GetAlertInstance(cmd *models.GetAlertInstanceQuery) error { return err } - params := append(make([]interface{}, 0), cmd.DefinitionOrgID, cmd.DefinitionUID, hash) + params := append(make([]interface{}, 0), cmd.RuleOrgID, cmd.RuleUID, hash) has, err := sess.SQL(s.String(), params...).Get(&instance) if !has { - return fmt.Errorf("instance not found for labels %v (hash: %v), alert definition %v (org %v)", cmd.Labels, hash, cmd.DefinitionUID, cmd.DefinitionOrgID) + return fmt.Errorf("instance not found for labels %v (hash: %v), alert definition %v (org %v)", cmd.Labels, hash, cmd.RuleUID, cmd.RuleOrgID) } if err != nil { return err @@ -57,10 +63,10 @@ func (st DBstore) ListAlertInstances(cmd *models.ListAlertInstancesQuery) error params = append(params, p...) } - addToQuery("SELECT alert_instance.*, alert_definition.title AS def_title FROM alert_instance LEFT JOIN alert_definition ON alert_instance.def_org_id = alert_definition.org_id AND alert_instance.def_uid = alert_definition.uid WHERE def_org_id = ?", cmd.DefinitionOrgID) + addToQuery("SELECT alert_instance.*, alert_definition.title AS def_title FROM alert_instance LEFT JOIN alert_definition ON alert_instance.def_org_id = alert_definition.org_id AND alert_instance.def_uid = alert_definition.uid WHERE def_org_id = ?", cmd.RuleOrgID) - if cmd.DefinitionUID != "" { - addToQuery(` AND def_uid = ?`, cmd.DefinitionUID) + if cmd.RuleUID != "" { + addToQuery(` AND def_uid = ?`, cmd.RuleUID) } if cmd.State != "" { @@ -77,7 +83,6 @@ func (st DBstore) ListAlertInstances(cmd *models.ListAlertInstancesQuery) error } // SaveAlertInstance is a handler for saving a new alert instance. -// nolint:unused func (st DBstore) SaveAlertInstance(cmd *models.SaveAlertInstanceCommand) error { return st.SQLStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { labelTupleJSON, labelsHash, err := cmd.Labels.StringAndHash() @@ -86,8 +91,8 @@ func (st DBstore) SaveAlertInstance(cmd *models.SaveAlertInstanceCommand) error } alertInstance := &models.AlertInstance{ - RuleOrgID: cmd.DefinitionOrgID, - RuleUID: cmd.DefinitionUID, + RuleOrgID: cmd.RuleOrgID, + RuleUID: cmd.RuleUID, Labels: cmd.Labels, LabelsHash: labelsHash, CurrentState: cmd.State, diff --git a/pkg/services/ngalert/tests/instance_database_test.go b/pkg/services/ngalert/tests/instance_database_test.go index eceb2954cfb..79c5532cc5f 100644 --- a/pkg/services/ngalert/tests/instance_database_test.go +++ b/pkg/services/ngalert/tests/instance_database_test.go @@ -42,18 +42,18 @@ func TestAlertInstanceOperations(t *testing.T) { t.Run("can save and read new alert instance", func(t *testing.T) { saveCmd := &models.SaveAlertInstanceCommand{ - DefinitionOrgID: alertRule1.OrgID, - DefinitionUID: alertRule1.UID, - State: models.InstanceStateFiring, - Labels: models.InstanceLabels{"test": "testValue"}, + RuleOrgID: alertRule1.OrgID, + RuleUID: alertRule1.UID, + State: models.InstanceStateFiring, + Labels: models.InstanceLabels{"test": "testValue"}, } err := dbstore.SaveAlertInstance(saveCmd) require.NoError(t, err) getCmd := &models.GetAlertInstanceQuery{ - DefinitionOrgID: saveCmd.DefinitionOrgID, - DefinitionUID: saveCmd.DefinitionUID, - Labels: models.InstanceLabels{"test": "testValue"}, + RuleOrgID: saveCmd.RuleOrgID, + RuleUID: saveCmd.RuleUID, + Labels: models.InstanceLabels{"test": "testValue"}, } err = dbstore.GetAlertInstance(getCmd) @@ -66,17 +66,17 @@ func TestAlertInstanceOperations(t *testing.T) { t.Run("can save and read new alert instance with no labels", func(t *testing.T) { saveCmd := &models.SaveAlertInstanceCommand{ - DefinitionOrgID: alertRule2.OrgID, - DefinitionUID: alertRule2.UID, - State: models.InstanceStateNormal, - Labels: models.InstanceLabels{}, + RuleOrgID: alertRule2.OrgID, + RuleUID: alertRule2.UID, + State: models.InstanceStateNormal, + Labels: models.InstanceLabels{}, } err := dbstore.SaveAlertInstance(saveCmd) require.NoError(t, err) getCmd := &models.GetAlertInstanceQuery{ - DefinitionOrgID: saveCmd.DefinitionOrgID, - DefinitionUID: saveCmd.DefinitionUID, + RuleOrgID: saveCmd.RuleOrgID, + RuleUID: saveCmd.RuleUID, } err = dbstore.GetAlertInstance(getCmd) @@ -89,27 +89,27 @@ func TestAlertInstanceOperations(t *testing.T) { t.Run("can save two instances with same org_id, uid and different labels", func(t *testing.T) { saveCmdOne := &models.SaveAlertInstanceCommand{ - DefinitionOrgID: alertRule3.OrgID, - DefinitionUID: alertRule3.UID, - State: models.InstanceStateFiring, - Labels: models.InstanceLabels{"test": "testValue"}, + RuleOrgID: alertRule3.OrgID, + RuleUID: alertRule3.UID, + State: models.InstanceStateFiring, + Labels: models.InstanceLabels{"test": "testValue"}, } err := dbstore.SaveAlertInstance(saveCmdOne) require.NoError(t, err) saveCmdTwo := &models.SaveAlertInstanceCommand{ - DefinitionOrgID: saveCmdOne.DefinitionOrgID, - DefinitionUID: saveCmdOne.DefinitionUID, - State: models.InstanceStateFiring, - Labels: models.InstanceLabels{"test": "meow"}, + RuleOrgID: saveCmdOne.RuleOrgID, + RuleUID: saveCmdOne.RuleUID, + State: models.InstanceStateFiring, + Labels: models.InstanceLabels{"test": "meow"}, } err = dbstore.SaveAlertInstance(saveCmdTwo) require.NoError(t, err) listQuery := &models.ListAlertInstancesQuery{ - DefinitionOrgID: saveCmdOne.DefinitionOrgID, - DefinitionUID: saveCmdOne.DefinitionUID, + RuleOrgID: saveCmdOne.RuleOrgID, + RuleUID: saveCmdOne.RuleUID, } err = dbstore.ListAlertInstances(listQuery) @@ -120,7 +120,7 @@ func TestAlertInstanceOperations(t *testing.T) { t.Run("can list all added instances in org", func(t *testing.T) { listQuery := &models.ListAlertInstancesQuery{ - DefinitionOrgID: orgID, + RuleOrgID: orgID, } err := dbstore.ListAlertInstances(listQuery) @@ -131,8 +131,8 @@ func TestAlertInstanceOperations(t *testing.T) { t.Run("can list all added instances in org filtered by current state", func(t *testing.T) { listQuery := &models.ListAlertInstancesQuery{ - DefinitionOrgID: orgID, - State: models.InstanceStateNormal, + RuleOrgID: orgID, + State: models.InstanceStateNormal, } err := dbstore.ListAlertInstances(listQuery) @@ -143,27 +143,27 @@ func TestAlertInstanceOperations(t *testing.T) { t.Run("update instance with same org_id, uid and different labels", func(t *testing.T) { saveCmdOne := &models.SaveAlertInstanceCommand{ - DefinitionOrgID: alertRule4.OrgID, - DefinitionUID: alertRule4.UID, - State: models.InstanceStateFiring, - Labels: models.InstanceLabels{"test": "testValue"}, + RuleOrgID: alertRule4.OrgID, + RuleUID: alertRule4.UID, + State: models.InstanceStateFiring, + Labels: models.InstanceLabels{"test": "testValue"}, } err := dbstore.SaveAlertInstance(saveCmdOne) require.NoError(t, err) saveCmdTwo := &models.SaveAlertInstanceCommand{ - DefinitionOrgID: saveCmdOne.DefinitionOrgID, - DefinitionUID: saveCmdOne.DefinitionUID, - State: models.InstanceStateNormal, - Labels: models.InstanceLabels{"test": "testValue"}, + RuleOrgID: saveCmdOne.RuleOrgID, + RuleUID: saveCmdOne.RuleUID, + State: models.InstanceStateNormal, + Labels: models.InstanceLabels{"test": "testValue"}, } err = dbstore.SaveAlertInstance(saveCmdTwo) require.NoError(t, err) listQuery := &models.ListAlertInstancesQuery{ - DefinitionOrgID: alertRule4.OrgID, - DefinitionUID: alertRule4.UID, + RuleOrgID: alertRule4.OrgID, + RuleUID: alertRule4.UID, } err = dbstore.ListAlertInstances(listQuery) @@ -171,8 +171,8 @@ func TestAlertInstanceOperations(t *testing.T) { require.Len(t, listQuery.Result, 1) - require.Equal(t, saveCmdTwo.DefinitionOrgID, listQuery.Result[0].RuleOrgID) - require.Equal(t, saveCmdTwo.DefinitionUID, listQuery.Result[0].RuleDefinitionUID) + require.Equal(t, saveCmdTwo.RuleOrgID, listQuery.Result[0].RuleOrgID) + require.Equal(t, saveCmdTwo.RuleUID, listQuery.Result[0].RuleDefinitionUID) require.Equal(t, saveCmdTwo.Labels, listQuery.Result[0].Labels) require.Equal(t, saveCmdTwo.State, listQuery.Result[0].CurrentState) }) diff --git a/pkg/services/ngalert/tests/schedule_test.go b/pkg/services/ngalert/tests/schedule_test.go index f80e0bca588..16400aca870 100644 --- a/pkg/services/ngalert/tests/schedule_test.go +++ b/pkg/services/ngalert/tests/schedule_test.go @@ -69,8 +69,8 @@ func TestWarmStateCache(t *testing.T) { } saveCmd1 := &models.SaveAlertInstanceCommand{ - DefinitionOrgID: rule.OrgID, - DefinitionUID: rule.UID, + RuleOrgID: rule.OrgID, + RuleUID: rule.UID, Labels: models.InstanceLabels{"test1": "testValue1"}, State: models.InstanceStateNormal, LastEvalTime: evaluationTime, @@ -81,8 +81,8 @@ func TestWarmStateCache(t *testing.T) { _ = dbstore.SaveAlertInstance(saveCmd1) saveCmd2 := &models.SaveAlertInstanceCommand{ - DefinitionOrgID: rule.OrgID, - DefinitionUID: rule.UID, + RuleOrgID: rule.OrgID, + RuleUID: rule.UID, Labels: models.InstanceLabels{"test2": "testValue2"}, State: models.InstanceStateFiring, LastEvalTime: evaluationTime, @@ -97,8 +97,9 @@ func TestWarmStateCache(t *testing.T) { C: clock.NewMock(), BaseInterval: time.Second, Logger: log.New("ngalert cache warming test"), - Store: dbstore, - RuleStore: dbstore, + + RuleStore: dbstore, + InstanceStore: dbstore, } sched := schedule.NewScheduler(schedCfg, nil) st := state.NewManager(schedCfg.Logger, nilMetrics) @@ -143,9 +144,9 @@ func TestAlertingTicker(t *testing.T) { StopAppliedFunc: func(alertDefKey models.AlertRuleKey) { stopAppliedCh <- alertDefKey }, - Store: dbstore, - RuleStore: dbstore, - Logger: log.New("ngalert schedule test"), + RuleStore: dbstore, + InstanceStore: dbstore, + Logger: log.New("ngalert schedule test"), } sched := schedule.NewScheduler(schedCfg, nil)