Alerting: Refactor store to not export its own interface for InstanceStore, delete dead dependency injection (#55772)

* Add consumer-side store interface to state manager

* Remove dead dependency

* Delete dead dependency in API struct

* Delete store-layer InstanceStore interface

* Move fake for state's InstanceStore interface to state package

pkg/services/ngalert/api/api.go

@@ -69,7 +69,6 @@ type API struct {
 	TransactionManager   provisioning.TransactionManager
 	ProvenanceStore      provisioning.ProvisioningStore
 	RuleStore            store.RuleStore
-	InstanceStore        store.InstanceStore
 	AlertingStore        AlertingStore
 	AdminConfigStore     store.AdminConfigurationStore
 	DataProxy            *datasourceproxy.DataSourceProxyService

pkg/services/ngalert/ngalert.go

@@ -158,14 +158,13 @@ func (ng *AlertNG) init() error {
 	ng.AlertsRouter = alertsRouter
 
 	schedCfg := schedule.SchedulerCfg{
-		Cfg:           ng.Cfg.UnifiedAlerting,
+		Cfg:         ng.Cfg.UnifiedAlerting,
-		C:             clk,
+		C:           clk,
-		Logger:        ng.Log,
+		Logger:      ng.Log,
-		Evaluator:     eval.NewEvaluator(ng.Cfg, ng.Log, ng.DataSourceCache, ng.ExpressionService),
+		Evaluator:   eval.NewEvaluator(ng.Cfg, ng.Log, ng.DataSourceCache, ng.ExpressionService),
-		InstanceStore: store,
+		RuleStore:   store,
-		RuleStore:     store,
+		Metrics:     ng.Metrics.GetSchedulerMetrics(),
-		Metrics:       ng.Metrics.GetSchedulerMetrics(),
+		AlertSender: alertsRouter,
-		AlertSender:   alertsRouter,
 	}
 
 	stateManager := state.NewManager(ng.Log, ng.Metrics.GetStateMetrics(), appUrl, store, store, ng.dashboardService, ng.imageService, clk, ng.annotationsRepo)
@@ -198,7 +197,6 @@ func (ng *AlertNG) init() error {
 		DataProxy:            ng.DataProxy,
 		QuotaService:         ng.QuotaService,
 		TransactionManager:   store,
-		InstanceStore:        store,
 		RuleStore:            store,
 		AlertingStore:        store,
 		AdminConfigStore:     store,

pkg/services/ngalert/schedule/schedule.go

@@ -16,7 +16,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/ngalert/metrics"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/state"
-	"github.com/grafana/grafana/pkg/services/ngalert/store"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
@@ -112,7 +111,6 @@ type SchedulerCfg struct {
 	StopAppliedFunc func(ngmodels.AlertRuleKey)
 	Evaluator       eval.Evaluator
 	RuleStore       RulesStore
-	InstanceStore   store.InstanceStore
 	Metrics         *metrics.Scheduler
 	AlertSender     AlertsSender
 }

pkg/services/ngalert/schedule/schedule_test.go

@@ -106,12 +106,11 @@ func TestWarmStateCache(t *testing.T) {
 	}
 
 	schedCfg := schedule.SchedulerCfg{
-		Cfg:           cfg,
+		Cfg:       cfg,
-		C:             clock.NewMock(),
+		C:         clock.NewMock(),
-		Logger:        log.New("ngalert cache warming test"),
+		Logger:    log.New("ngalert cache warming test"),
-		RuleStore:     dbstore,
+		RuleStore: dbstore,
-		InstanceStore: dbstore,
+		Metrics:   testMetrics.GetSchedulerMetrics(),
-		Metrics:       testMetrics.GetSchedulerMetrics(),
 	}
 	st := state.NewManager(schedCfg.Logger, testMetrics.GetStateMetrics(), nil, dbstore, dbstore, &dashboards.FakeDashboardService{}, &image.NoopImageService{}, clock.NewMock(), annotationstest.NewFakeAnnotationsRepo())
 	st.Warm(ctx)
@@ -161,11 +160,10 @@ func TestAlertingTicker(t *testing.T) {
 		StopAppliedFunc: func(alertDefKey models.AlertRuleKey) {
 			stopAppliedCh <- alertDefKey
 		},
-		RuleStore:     dbstore,
+		RuleStore:   dbstore,
-		InstanceStore: dbstore,
+		Logger:      log.New("ngalert schedule test"),
-		Logger:        log.New("ngalert schedule test"),
+		Metrics:     testMetrics.GetSchedulerMetrics(),
-		Metrics:       testMetrics.GetSchedulerMetrics(),
+		AlertSender: notifier,
-		AlertSender:   notifier,
 	}
 	st := state.NewManager(schedCfg.Logger, testMetrics.GetStateMetrics(), nil, dbstore, dbstore, &dashboards.FakeDashboardService{}, &image.NoopImageService{}, clock.NewMock(), annotationstest.NewFakeAnnotationsRepo())
 	appUrl := &url.URL{

pkg/services/ngalert/schedule/schedule_unit_test.go

@@ -38,9 +38,9 @@ func TestSchedule_ruleRoutine(t *testing.T) {
 	createSchedule := func(
 		evalAppliedChan chan time.Time,
 		senderMock *AlertsSenderMock,
-	) (*schedule, *store.FakeRuleStore, *store.FakeInstanceStore, prometheus.Gatherer) {
+	) (*schedule, *store.FakeRuleStore, *state.FakeInstanceStore, prometheus.Gatherer) {
 		ruleStore := store.NewFakeRuleStore(t)
-		instanceStore := &store.FakeInstanceStore{}
+		instanceStore := &state.FakeInstanceStore{}
 
 		registry := prometheus.NewPedanticRegistry()
 		sch := setupScheduler(t, ruleStore, instanceStore, registry, senderMock, nil)
@@ -481,7 +481,7 @@ func TestSchedule_DeleteAlertRule(t *testing.T) {
 	})
 }
 
-func setupScheduler(t *testing.T, rs *store.FakeRuleStore, is *store.FakeInstanceStore, registry *prometheus.Registry, senderMock *AlertsSenderMock, evalMock *eval.FakeEvaluator) *schedule {
+func setupScheduler(t *testing.T, rs *store.FakeRuleStore, is *state.FakeInstanceStore, registry *prometheus.Registry, senderMock *AlertsSenderMock, evalMock *eval.FakeEvaluator) *schedule {
 	t.Helper()
 
 	mockedClock := clock.NewMock()
@@ -492,7 +492,7 @@ func setupScheduler(t *testing.T, rs *store.FakeRuleStore, is *store.FakeInstanc
 	}
 
 	if is == nil {
-		is = &store.FakeInstanceStore{}
+		is = &state.FakeInstanceStore{}
 	}
 
 	var evaluator eval.Evaluator = evalMock

pkg/services/ngalert/state/manager.go

@@ -21,6 +21,7 @@ import (
 	"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/screenshot"
 )
 
@@ -42,14 +43,14 @@ type Manager struct {
 	ResendDelay time.Duration
 
 	ruleStore        store.RuleStore
-	instanceStore    store.InstanceStore
+	instanceStore    InstanceStore
 	dashboardService dashboards.DashboardService
 	imageService     image.ImageService
 	AnnotationsRepo  annotations.Repository
 }
 
 func NewManager(logger log.Logger, metrics *metrics.State, externalURL *url.URL,
-	ruleStore store.RuleStore, instanceStore store.InstanceStore,
+	ruleStore store.RuleStore, instanceStore InstanceStore,
 	dashboardService dashboards.DashboardService, imageService image.ImageService, clock clock.Clock, annotationsRepo annotations.Repository) *Manager {
 	manager := &Manager{
 		cache:            newCache(logger, metrics, externalURL),

pkg/services/ngalert/state/manager_private_test.go

@@ -95,7 +95,7 @@ func Test_maybeNewImage(t *testing.T) {
 		t.Run(test.description, func(t *testing.T) {
 			imageService := &CountingImageService{}
 			mgr := NewManager(log.NewNopLogger(), &metrics.State{}, nil,
-				&store.FakeRuleStore{}, &store.FakeInstanceStore{},
+				&store.FakeRuleStore{}, &FakeInstanceStore{},
 				&dashboards.FakeDashboardService{}, imageService, clock.NewMock(), annotationstest.NewFakeAnnotationsRepo())
 			err := mgr.maybeTakeScreenshot(context.Background(), &ngmodels.AlertRule{}, test.state, test.oldState)
 			require.NoError(t, err)

pkg/services/ngalert/state/manager_test.go

@@ -25,7 +25,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/ngalert/metrics"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/state"
-	"github.com/grafana/grafana/pkg/services/ngalert/store"
 	"github.com/grafana/grafana/pkg/services/ngalert/tests"
 )
 
@@ -1981,7 +1980,7 @@ func TestProcessEvalResults(t *testing.T) {
 
 	for _, tc := range testCases {
 		fakeAnnoRepo := annotationstest.NewFakeAnnotationsRepo()
-		st := state.NewManager(log.New("test_state_manager"), testMetrics.GetStateMetrics(), nil, nil, &store.FakeInstanceStore{}, &dashboards.FakeDashboardService{}, &image.NotAvailableImageService{}, clock.New(), fakeAnnoRepo)
+		st := state.NewManager(log.New("test_state_manager"), testMetrics.GetStateMetrics(), nil, nil, &state.FakeInstanceStore{}, &dashboards.FakeDashboardService{}, &image.NotAvailableImageService{}, clock.New(), fakeAnnoRepo)
 		t.Run(tc.desc, func(t *testing.T) {
 			for _, res := range tc.evalResults {
 				_ = st.ProcessEvalResults(context.Background(), evaluationTime, tc.alertRule, res, data.Labels{
@@ -2007,7 +2006,7 @@ func TestProcessEvalResults(t *testing.T) {
 	}
 
 	t.Run("should save state to database", func(t *testing.T) {
-		instanceStore := &store.FakeInstanceStore{}
+		instanceStore := &state.FakeInstanceStore{}
 		clk := clock.New()
 		st := state.NewManager(log.New("test_state_manager"), testMetrics.GetStateMetrics(), nil, nil, instanceStore, &dashboards.FakeDashboardService{}, &image.NotAvailableImageService{}, clk, annotationstest.NewFakeAnnotationsRepo())
 		rule := models.AlertRuleGen()()

pkg/services/ngalert/state/persist.go

@@ -0,0 +1,15 @@
+package state
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/services/ngalert/models"
+)
+
+type InstanceStore interface {
+	FetchOrgIds(ctx context.Context) ([]int64, error)
+	ListAlertInstances(ctx context.Context, cmd *models.ListAlertInstancesQuery) error
+	SaveAlertInstance(ctx context.Context, cmd *models.SaveAlertInstanceCommand) error
+	DeleteAlertInstance(ctx context.Context, orgID int64, ruleUID, labelsHash string) error
+	DeleteAlertInstancesByRule(ctx context.Context, key models.AlertRuleKey) error
+}

pkg/services/ngalert/state/testing.go

@@ -0,0 +1,37 @@
+package state
+
+import (
+	"context"
+	"sync"
+
+	"github.com/grafana/grafana/pkg/services/ngalert/models"
+)
+
+type FakeInstanceStore struct {
+	mtx         sync.Mutex
+	RecordedOps []interface{}
+}
+
+func (f *FakeInstanceStore) ListAlertInstances(_ context.Context, q *models.ListAlertInstancesQuery) error {
+	f.mtx.Lock()
+	defer f.mtx.Unlock()
+	f.RecordedOps = append(f.RecordedOps, *q)
+	return nil
+}
+
+func (f *FakeInstanceStore) SaveAlertInstance(_ context.Context, q *models.SaveAlertInstanceCommand) error {
+	f.mtx.Lock()
+	defer f.mtx.Unlock()
+	f.RecordedOps = append(f.RecordedOps, *q)
+	return nil
+}
+
+func (f *FakeInstanceStore) FetchOrgIds(_ context.Context) ([]int64, error) { return []int64{}, nil }
+
+func (f *FakeInstanceStore) DeleteAlertInstance(_ context.Context, _ int64, _, _ string) error {
+	return nil
+}
+
+func (f *FakeInstanceStore) DeleteAlertInstancesByRule(ctx context.Context, key models.AlertRuleKey) error {
+	return nil
+}

pkg/services/ngalert/store/instance_database.go

@@ -9,15 +9,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 )
 
-type InstanceStore interface {
-	GetAlertInstance(ctx context.Context, cmd *models.GetAlertInstanceQuery) error
-	ListAlertInstances(ctx context.Context, cmd *models.ListAlertInstancesQuery) error
-	SaveAlertInstance(ctx context.Context, cmd *models.SaveAlertInstanceCommand) error
-	FetchOrgIds(ctx context.Context) ([]int64, error)
-	DeleteAlertInstance(ctx context.Context, orgID int64, ruleUID, labelsHash string) error
-	DeleteAlertInstancesByRule(ctx context.Context, key models.AlertRuleKey) error
-}
-
 // GetAlertInstance is a handler for retrieving an alert instance based on OrgId, AlertDefintionID, and
 // the hash of the labels.
 func (st DBstore) GetAlertInstance(ctx context.Context, cmd *models.GetAlertInstanceQuery) error {

pkg/services/ngalert/store/testing.go

@@ -460,38 +460,6 @@ func (f *FakeRuleStore) IncreaseVersionForAllRulesInNamespace(_ context.Context,
 	return result, nil
 }
 
-type FakeInstanceStore struct {
-	mtx         sync.Mutex
-	RecordedOps []interface{}
-}
-
-func (f *FakeInstanceStore) GetAlertInstance(_ context.Context, q *models.GetAlertInstanceQuery) error {
-	f.mtx.Lock()
-	defer f.mtx.Unlock()
-	f.RecordedOps = append(f.RecordedOps, *q)
-	return nil
-}
-func (f *FakeInstanceStore) ListAlertInstances(_ context.Context, q *models.ListAlertInstancesQuery) error {
-	f.mtx.Lock()
-	defer f.mtx.Unlock()
-	f.RecordedOps = append(f.RecordedOps, *q)
-	return nil
-}
-func (f *FakeInstanceStore) SaveAlertInstance(_ context.Context, q *models.SaveAlertInstanceCommand) error {
-	f.mtx.Lock()
-	defer f.mtx.Unlock()
-	f.RecordedOps = append(f.RecordedOps, *q)
-	return nil
-}
-
-func (f *FakeInstanceStore) FetchOrgIds(_ context.Context) ([]int64, error) { return []int64{}, nil }
-func (f *FakeInstanceStore) DeleteAlertInstance(_ context.Context, _ int64, _, _ string) error {
-	return nil
-}
-func (f *FakeInstanceStore) DeleteAlertInstancesByRule(ctx context.Context, key models.AlertRuleKey) error {
-	return nil
-}
-
 func NewFakeAdminConfigStore(t *testing.T) *FakeAdminConfigStore {
 	t.Helper()
 	return &FakeAdminConfigStore{Configs: map[int64]*models.AdminConfiguration{}}