diff --git a/pkg/services/ngalert/image/mock.go b/pkg/services/ngalert/image/mock.go deleted file mode 100644 index fd806ea45ab..00000000000 --- a/pkg/services/ngalert/image/mock.go +++ /dev/null @@ -1,51 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: github.com/grafana/grafana/pkg/services/ngalert/image (interfaces: ImageService) - -// Package image is a generated GoMock package. -package image - -import ( - context "context" - reflect "reflect" - - gomock "github.com/golang/mock/gomock" - models "github.com/grafana/grafana/pkg/services/ngalert/models" -) - -// MockImageService is a mock of ImageService interface. -type MockImageService struct { - ctrl *gomock.Controller - recorder *MockImageServiceMockRecorder -} - -// MockImageServiceMockRecorder is the mock recorder for MockImageService. -type MockImageServiceMockRecorder struct { - mock *MockImageService -} - -// NewMockImageService creates a new mock instance. -func NewMockImageService(ctrl *gomock.Controller) *MockImageService { - mock := &MockImageService{ctrl: ctrl} - mock.recorder = &MockImageServiceMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockImageService) EXPECT() *MockImageServiceMockRecorder { - return m.recorder -} - -// NewImage mocks base method. -func (m *MockImageService) NewImage(arg0 context.Context, arg1 *models.AlertRule) (*models.Image, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "NewImage", arg0, arg1) - ret0, _ := ret[0].(*models.Image) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// NewImage indicates an expected call of NewImage. -func (mr *MockImageServiceMockRecorder) NewImage(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewImage", reflect.TypeOf((*MockImageService)(nil).NewImage), arg0, arg1) -} diff --git a/pkg/services/ngalert/image/service.go b/pkg/services/ngalert/image/service.go index f3382d338c4..5051051c2de 100644 --- a/pkg/services/ngalert/image/service.go +++ b/pkg/services/ngalert/image/service.go @@ -25,16 +25,6 @@ const ( screenshotTimeout = 10 * time.Second ) -var ( - // ErrNoDashboard is returned when the alert rule does not have a Dashboard UID - // in its annotations or the dashboard does not exist. - ErrNoDashboard = errors.New("no dashboard") - - // ErrNoPanel is returned when the alert rule does not have a PanelID in its - // annotations. - ErrNoPanel = errors.New("no panel") -) - // DeleteExpiredService is a service to delete expired images. type DeleteExpiredService struct { store store.ImageAdminStore @@ -48,7 +38,6 @@ func ProvideDeleteExpiredService(store *store.DBstore) *DeleteExpiredService { return &DeleteExpiredService{store: store} } -//go:generate mockgen -destination=mock.go -package=image github.com/grafana/grafana/pkg/services/ngalert/image ImageService type ImageService interface { // NewImage returns a new image for the alert instance. NewImage(ctx context.Context, r *models.AlertRule) (*models.Image, error) @@ -120,16 +109,16 @@ func NewScreenshotImageServiceFromCfg(cfg *setting.Cfg, db *store.DBstore, ds da // // The alert rule must be associated with a dashboard panel for a screenshot to be // taken. If the alert rule does not have a Dashboard UID in its annotations, -// or the dashboard does not exist, an ErrNoDashboard error is returned. If the +// or the dashboard does not exist, an models.ErrNoDashboard error is returned. If the // alert rule has a Dashboard UID and the dashboard exists, but does not have a -// Panel ID in its annotations then an ErrNoPanel error is returned. +// Panel ID in its annotations then an models.ErrNoPanel error is returned. func (s *ScreenshotImageService) NewImage(ctx context.Context, r *models.AlertRule) (*models.Image, error) { if r.DashboardUID == nil || *r.DashboardUID == "" { - return nil, ErrNoDashboard + return nil, models.ErrNoDashboard } if r.PanelID == nil || *r.PanelID == 0 { - return nil, ErrNoPanel + return nil, models.ErrNoPanel } opts := screenshot.ScreenshotOptions{ @@ -167,7 +156,7 @@ func (s *ScreenshotImageService) NewImage(ctx context.Context, r *models.AlertRu screenshot, err := s.limiter.Do(ctx, opts, s.screenshots.Take) if err != nil { if errors.Is(err, dashboards.ErrDashboardNotFound) { - return nil, ErrNoDashboard + return nil, models.ErrNoDashboard } return nil, err } @@ -204,17 +193,3 @@ func (s *ScreenshotImageService) NewImage(ctx context.Context, r *models.AlertRu return &image, nil } - -// NotAvailableImageService is a service that returns ErrScreenshotsUnavailable. -type NotAvailableImageService struct{} - -func (s *NotAvailableImageService) NewImage(_ context.Context, _ *models.AlertRule) (*models.Image, error) { - return nil, screenshot.ErrScreenshotsUnavailable -} - -// NoopImageService is a no-op image service. -type NoopImageService struct{} - -func (s *NoopImageService) NewImage(_ context.Context, _ *models.AlertRule) (*models.Image, error) { - return &models.Image{}, nil -} diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index 0b56ec2f21d..ee13175dfb7 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -26,6 +26,13 @@ var ( ErrAlertRuleFailedValidation = errors.New("invalid alert rule") ErrAlertRuleUniqueConstraintViolation = errors.New("a conflicting alert rule is found: rule title under the same organisation and folder should be unique") ErrQuotaReached = errors.New("quota has been exceeded") + // ErrNoDashboard is returned when the alert rule does not have a Dashboard UID + // in its annotations or the dashboard does not exist. + ErrNoDashboard = errors.New("no dashboard") + + // ErrNoPanel is returned when the alert rule does not have a PanelID in its + // annotations. + ErrNoPanel = errors.New("no panel") ) // swagger:enum NoDataState diff --git a/pkg/services/ngalert/schedule/schedule_unit_test.go b/pkg/services/ngalert/schedule/schedule_unit_test.go index d5706111ce9..bad68ab113a 100644 --- a/pkg/services/ngalert/schedule/schedule_unit_test.go +++ b/pkg/services/ngalert/schedule/schedule_unit_test.go @@ -23,7 +23,6 @@ import ( "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/eval" - "github.com/grafana/grafana/pkg/services/ngalert/image" "github.com/grafana/grafana/pkg/services/ngalert/metrics" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/state" @@ -62,7 +61,7 @@ func TestProcessTicks(t *testing.T) { Metrics: testMetrics.GetSchedulerMetrics(), AlertSender: notifier, } - st := state.NewManager(testMetrics.GetStateMetrics(), nil, nil, &image.NoopImageService{}, mockedClock, &state.FakeHistorian{}) + st := state.NewManager(testMetrics.GetStateMetrics(), nil, nil, &state.NoopImageService{}, mockedClock, &state.FakeHistorian{}) appUrl := &url.URL{ Scheme: "http", @@ -682,7 +681,7 @@ func setupScheduler(t *testing.T, rs *fakeRulesStore, is *state.FakeInstanceStor AlertSender: senderMock, } - st := state.NewManager(m.GetStateMetrics(), nil, is, &image.NoopImageService{}, mockedClock, &state.FakeHistorian{}) + st := state.NewManager(m.GetStateMetrics(), nil, is, &state.NoopImageService{}, mockedClock, &state.FakeHistorian{}) return NewScheduler(schedCfg, appUrl, st) } diff --git a/pkg/services/ngalert/state/image_mock.go b/pkg/services/ngalert/state/image_mock.go new file mode 100644 index 00000000000..62913c5c05c --- /dev/null +++ b/pkg/services/ngalert/state/image_mock.go @@ -0,0 +1,51 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/grafana/grafana/pkg/services/ngalert/state (interfaces: ImageCapturer) + +// Package state is a generated GoMock package. +package state + +import ( + context "context" + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + models "github.com/grafana/grafana/pkg/services/ngalert/models" +) + +// MockImageCapturer is a mock of ImageCapturer interface. +type MockImageCapturer struct { + ctrl *gomock.Controller + recorder *MockImageCapturerMockRecorder +} + +// MockImageCapturerMockRecorder is the mock recorder for MockImageCapturer. +type MockImageCapturerMockRecorder struct { + mock *MockImageCapturer +} + +// NewMockImageCapturer creates a new mock instance. +func NewMockImageCapturer(ctrl *gomock.Controller) *MockImageCapturer { + mock := &MockImageCapturer{ctrl: ctrl} + mock.recorder = &MockImageCapturerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockImageCapturer) EXPECT() *MockImageCapturerMockRecorder { + return m.recorder +} + +// NewImage mocks base method. +func (m *MockImageCapturer) NewImage(arg0 context.Context, arg1 *models.AlertRule) (*models.Image, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NewImage", arg0, arg1) + ret0, _ := ret[0].(*models.Image) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// NewImage indicates an expected call of NewImage. +func (mr *MockImageCapturerMockRecorder) NewImage(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewImage", reflect.TypeOf((*MockImageCapturer)(nil).NewImage), arg0, arg1) +} diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go index 4459083e81f..f8087b590be 100644 --- a/pkg/services/ngalert/state/manager.go +++ b/pkg/services/ngalert/state/manager.go @@ -10,7 +10,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/ngalert/eval" - "github.com/grafana/grafana/pkg/services/ngalert/image" "github.com/grafana/grafana/pkg/services/ngalert/metrics" ngModels "github.com/grafana/grafana/pkg/services/ngalert/models" ) @@ -35,19 +34,19 @@ type Manager struct { ResendDelay time.Duration instanceStore InstanceStore - imageService image.ImageService + images ImageCapturer historian Historian externalURL *url.URL } -func NewManager(metrics *metrics.State, externalURL *url.URL, instanceStore InstanceStore, imageService image.ImageService, clock clock.Clock, historian Historian) *Manager { +func NewManager(metrics *metrics.State, externalURL *url.URL, instanceStore InstanceStore, images ImageCapturer, clock clock.Clock, historian Historian) *Manager { return &Manager{ cache: newCache(), ResendDelay: ResendDelay, // TODO: make this configurable log: log.New("ngalert.state.manager"), metrics: metrics, instanceStore: instanceStore, - imageService: imageService, + images: images, historian: historian, clock: clock, externalURL: externalURL, @@ -244,7 +243,7 @@ func (st *Manager) setNextState(ctx context.Context, alertRule *ngModels.AlertRu currentState.Resolved = oldState == eval.Alerting && currentState.State == eval.Normal if shouldTakeImage(currentState.State, oldState, currentState.Image, currentState.Resolved) { - image, err := takeImage(ctx, st.imageService, alertRule) + image, err := takeImage(ctx, st.images, alertRule) if err != nil { logger.Warn("Failed to take an image", "dashboard", alertRule.DashboardUID, @@ -371,7 +370,7 @@ func (st *Manager) staleResultsHandler(ctx context.Context, logger log.Logger, r // If there is no resolved image for this rule then take one if resolvedImage == nil { - image, err := takeImage(ctx, st.imageService, r) + image, err := takeImage(ctx, st.images, r) if err != nil { logger.Warn("Failed to take an image", "dashboard", r.DashboardUID, diff --git a/pkg/services/ngalert/state/manager_test.go b/pkg/services/ngalert/state/manager_test.go index 7905d291e4d..06974b44089 100644 --- a/pkg/services/ngalert/state/manager_test.go +++ b/pkg/services/ngalert/state/manager_test.go @@ -22,7 +22,6 @@ import ( "github.com/grafana/grafana/pkg/services/annotations/annotationstest" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/ngalert/eval" - "github.com/grafana/grafana/pkg/services/ngalert/image" "github.com/grafana/grafana/pkg/services/ngalert/metrics" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/state" @@ -103,7 +102,7 @@ func TestWarmStateCache(t *testing.T) { Labels: labels, } _ = dbstore.SaveAlertInstances(ctx, instance2) - st := state.NewManager(testMetrics.GetStateMetrics(), nil, dbstore, &image.NoopImageService{}, clock.NewMock(), &state.FakeHistorian{}) + st := state.NewManager(testMetrics.GetStateMetrics(), nil, dbstore, &state.NoopImageService{}, clock.NewMock(), &state.FakeHistorian{}) st.Warm(ctx, dbstore) t.Run("instance cache has expected entries", func(t *testing.T) { @@ -127,7 +126,7 @@ func TestDashboardAnnotations(t *testing.T) { fakeAnnoRepo := annotationstest.NewFakeAnnotationsRepo() hist := historian.NewAnnotationHistorian(fakeAnnoRepo, &dashboards.FakeDashboardService{}) - st := state.NewManager(testMetrics.GetStateMetrics(), nil, dbstore, &image.NoopImageService{}, clock.New(), hist) + st := state.NewManager(testMetrics.GetStateMetrics(), nil, dbstore, &state.NoopImageService{}, clock.New(), hist) const mainOrgID int64 = 1 @@ -2108,7 +2107,7 @@ func TestProcessEvalResults(t *testing.T) { for _, tc := range testCases { fakeAnnoRepo := annotationstest.NewFakeAnnotationsRepo() hist := historian.NewAnnotationHistorian(fakeAnnoRepo, &dashboards.FakeDashboardService{}) - st := state.NewManager(testMetrics.GetStateMetrics(), nil, &state.FakeInstanceStore{}, &image.NotAvailableImageService{}, clock.New(), hist) + st := state.NewManager(testMetrics.GetStateMetrics(), nil, &state.FakeInstanceStore{}, &state.NotAvailableImageService{}, clock.New(), hist) t.Run(tc.desc, func(t *testing.T) { for _, res := range tc.evalResults { _ = st.ProcessEvalResults(context.Background(), evaluationTime, tc.alertRule, res, data.Labels{ @@ -2135,7 +2134,7 @@ 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, &image.NotAvailableImageService{}, clk, &state.FakeHistorian{}) + st := state.NewManager(testMetrics.GetStateMetrics(), nil, instanceStore, &state.NotAvailableImageService{}, clk, &state.FakeHistorian{}) rule := models.AlertRuleGen()() var results = eval.GenerateResults(rand.Intn(4)+1, eval.ResultGen(eval.WithEvaluatedAt(clk.Now()))) @@ -2264,7 +2263,7 @@ func TestStaleResultsHandler(t *testing.T) { for _, tc := range testCases { ctx := context.Background() - st := state.NewManager(testMetrics.GetStateMetrics(), nil, dbstore, &image.NoopImageService{}, clock.New(), &state.FakeHistorian{}) + st := state.NewManager(testMetrics.GetStateMetrics(), nil, dbstore, &state.NoopImageService{}, clock.New(), &state.FakeHistorian{}) st.Warm(ctx, dbstore) existingStatesForRule := st.GetStatesForRuleUID(rule.OrgID, rule.UID) @@ -2326,7 +2325,7 @@ func TestStaleResults(t *testing.T) { clk := clock.NewMock() clk.Set(time.Now()) - st := state.NewManager(testMetrics.GetStateMetrics(), nil, dbstore, &image.NoopImageService{}, clk, &state.FakeHistorian{}) + st := state.NewManager(testMetrics.GetStateMetrics(), nil, dbstore, &state.NoopImageService{}, clk, &state.FakeHistorian{}) orgID := rand.Int63() rule := tests.CreateTestAlertRule(t, ctx, dbstore, 10, orgID) diff --git a/pkg/services/ngalert/state/persist.go b/pkg/services/ngalert/state/persist.go index 7dacf955b93..78bcd181daf 100644 --- a/pkg/services/ngalert/state/persist.go +++ b/pkg/services/ngalert/state/persist.go @@ -25,3 +25,10 @@ type Historian interface { // RecordStates writes a number of state transitions for a given rule to state history. RecordStates(ctx context.Context, rule *models.AlertRule, states []StateTransition) } + +// ImageCapturer captures images. +// +//go:generate mockgen -destination=image_mock.go -package=state github.com/grafana/grafana/pkg/services/ngalert/state ImageCapturer +type ImageCapturer interface { + NewImage(ctx context.Context, r *models.AlertRule) (*models.Image, error) +} diff --git a/pkg/services/ngalert/state/state.go b/pkg/services/ngalert/state/state.go index 7651af8060d..839dc014c2c 100644 --- a/pkg/services/ngalert/state/state.go +++ b/pkg/services/ngalert/state/state.go @@ -12,7 +12,6 @@ import ( "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/services/ngalert/eval" - "github.com/grafana/grafana/pkg/services/ngalert/image" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/screenshot" ) @@ -334,12 +333,12 @@ func shouldTakeImage(state, previousState eval.State, previousImage *models.Imag // takeImage takes an image for the alert rule. It returns nil if screenshots are disabled or // the rule is not associated with a dashboard panel. -func takeImage(ctx context.Context, s image.ImageService, r *models.AlertRule) (*models.Image, error) { +func takeImage(ctx context.Context, s ImageCapturer, r *models.AlertRule) (*models.Image, error) { img, err := s.NewImage(ctx, r) if err != nil { if errors.Is(err, screenshot.ErrScreenshotsUnavailable) || - errors.Is(err, image.ErrNoDashboard) || - errors.Is(err, image.ErrNoPanel) { + errors.Is(err, models.ErrNoDashboard) || + errors.Is(err, models.ErrNoPanel) { return nil, nil } return nil, err diff --git a/pkg/services/ngalert/state/state_test.go b/pkg/services/ngalert/state/state_test.go index b95f1bc2520..b86037d4bc7 100644 --- a/pkg/services/ngalert/state/state_test.go +++ b/pkg/services/ngalert/state/state_test.go @@ -10,7 +10,6 @@ import ( "github.com/golang/mock/gomock" "github.com/grafana/grafana/pkg/services/ngalert/eval" - "github.com/grafana/grafana/pkg/services/ngalert/image" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/screenshot" "github.com/stretchr/testify/assert" @@ -358,9 +357,9 @@ func TestTakeImage(t *testing.T) { ctx := context.Background() r := ngmodels.AlertRule{} - s := image.NewMockImageService(ctrl) + s := NewMockImageCapturer(ctrl) - s.EXPECT().NewImage(ctx, &r).Return(nil, image.ErrNoDashboard) + s.EXPECT().NewImage(ctx, &r).Return(nil, ngmodels.ErrNoDashboard) image, err := takeImage(ctx, s, &r) assert.NoError(t, err) assert.Nil(t, image) @@ -372,9 +371,9 @@ func TestTakeImage(t *testing.T) { ctx := context.Background() r := ngmodels.AlertRule{DashboardUID: ptr.String("foo")} - s := image.NewMockImageService(ctrl) + s := NewMockImageCapturer(ctrl) - s.EXPECT().NewImage(ctx, &r).Return(nil, image.ErrNoPanel) + s.EXPECT().NewImage(ctx, &r).Return(nil, ngmodels.ErrNoPanel) image, err := takeImage(ctx, s, &r) assert.NoError(t, err) assert.Nil(t, image) @@ -386,7 +385,7 @@ func TestTakeImage(t *testing.T) { ctx := context.Background() r := ngmodels.AlertRule{DashboardUID: ptr.String("foo"), PanelID: ptr.Int64(1)} - s := image.NewMockImageService(ctrl) + s := NewMockImageCapturer(ctrl) s.EXPECT().NewImage(ctx, &r).Return(nil, screenshot.ErrScreenshotsUnavailable) image, err := takeImage(ctx, s, &r) @@ -400,7 +399,7 @@ func TestTakeImage(t *testing.T) { ctx := context.Background() r := ngmodels.AlertRule{DashboardUID: ptr.String("foo"), PanelID: ptr.Int64(1)} - s := image.NewMockImageService(ctrl) + s := NewMockImageCapturer(ctrl) s.EXPECT().NewImage(ctx, &r).Return(nil, errors.New("unknown error")) image, err := takeImage(ctx, s, &r) @@ -414,7 +413,7 @@ func TestTakeImage(t *testing.T) { ctx := context.Background() r := ngmodels.AlertRule{DashboardUID: ptr.String("foo"), PanelID: ptr.Int64(1)} - s := image.NewMockImageService(ctrl) + s := NewMockImageCapturer(ctrl) s.EXPECT().NewImage(ctx, &r).Return(&ngmodels.Image{Path: "foo.png"}, nil) image, err := takeImage(ctx, s, &r) diff --git a/pkg/services/ngalert/state/testing.go b/pkg/services/ngalert/state/testing.go index c003ba446ff..80c2a565e45 100644 --- a/pkg/services/ngalert/state/testing.go +++ b/pkg/services/ngalert/state/testing.go @@ -5,6 +5,7 @@ import ( "sync" "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/services/screenshot" ) var _ InstanceStore = &FakeInstanceStore{} @@ -50,3 +51,17 @@ type FakeHistorian struct{} func (f *FakeHistorian) RecordStates(ctx context.Context, rule *models.AlertRule, states []StateTransition) { } + +// NotAvailableImageService is a service that returns ErrScreenshotsUnavailable. +type NotAvailableImageService struct{} + +func (s *NotAvailableImageService) NewImage(_ context.Context, _ *models.AlertRule) (*models.Image, error) { + return nil, screenshot.ErrScreenshotsUnavailable +} + +// NoopImageService is a no-op image service. +type NoopImageService struct{} + +func (s *NoopImageService) NewImage(_ context.Context, _ *models.AlertRule) (*models.Image, error) { + return &models.Image{}, nil +}