Dashboard Alert Extractor: Create service for dashboard extractor and remove bus (#45518)

* Create DashAlertService service

* Remove no used dashboard service from plugin's manager that generates dependency cycle in Enterprise

* Remove bus for dashboard permissions

* Remove bus from dashboard extractor service

* Add missing argument

* Fix wire

* Fix lint

* More goimports

* Use datasource service instead sql calls

* Fix integration test
This commit is contained in:
Selene
2022-02-28 09:54:56 +01:00
committed by GitHub
parent 1df040eb23
commit 2c90dcf3c0
29 changed files with 294 additions and 232 deletions

View File

@@ -45,16 +45,17 @@ type AlertEngine struct {
DataService legacydata.RequestHandler
Cfg *setting.Cfg
execQueue chan *Job
ticker *Ticker
scheduler scheduler
evalHandler evalHandler
ruleReader ruleReader
log log.Logger
resultHandler resultHandler
usageStatsService usagestats.Service
tracer tracing.Tracer
sqlStore AlertStore
execQueue chan *Job
ticker *Ticker
scheduler scheduler
evalHandler evalHandler
ruleReader ruleReader
log log.Logger
resultHandler resultHandler
usageStatsService usagestats.Service
tracer tracing.Tracer
sqlStore AlertStore
dashAlertExtractor DashAlertExtractor
}
// IsDisabled returns true if the alerting service is disabled for this instance.
@@ -65,16 +66,18 @@ func (e *AlertEngine) IsDisabled() bool {
// ProvideAlertEngine returns a new AlertEngine.
func ProvideAlertEngine(renderer rendering.Service, bus bus.Bus, requestValidator models.PluginRequestValidator,
dataService legacydata.RequestHandler, usageStatsService usagestats.Service, encryptionService encryption.Internal,
notificationService *notifications.NotificationService, tracer tracing.Tracer, sqlStore AlertStore, cfg *setting.Cfg) *AlertEngine {
notificationService *notifications.NotificationService, tracer tracing.Tracer, sqlStore AlertStore, cfg *setting.Cfg,
dashAlertExtractor DashAlertExtractor) *AlertEngine {
e := &AlertEngine{
Cfg: cfg,
RenderService: renderer,
Bus: bus,
RequestValidator: requestValidator,
DataService: dataService,
usageStatsService: usageStatsService,
tracer: tracer,
sqlStore: sqlStore,
Cfg: cfg,
RenderService: renderer,
Bus: bus,
RequestValidator: requestValidator,
DataService: dataService,
usageStatsService: usageStatsService,
tracer: tracer,
sqlStore: sqlStore,
dashAlertExtractor: dashAlertExtractor,
}
e.ticker = NewTicker(time.Now(), time.Second*0, clock.New(), 1)
e.execQueue = make(chan *Job, 1000)

View File

@@ -24,7 +24,7 @@ func TestEngineTimeouts(t *testing.T) {
usMock := &usagestats.UsageStatsMock{T: t}
tracer, err := tracing.InitializeTracerForTest()
require.NoError(t, err)
engine := ProvideAlertEngine(nil, nil, nil, nil, usMock, ossencryption.ProvideService(), nil, tracer, nil, setting.NewCfg())
engine := ProvideAlertEngine(nil, nil, nil, nil, usMock, ossencryption.ProvideService(), nil, tracer, nil, setting.NewCfg(), nil)
setting.AlertingNotificationTimeout = 30 * time.Second
setting.AlertingMaxAttempts = 3
engine.resultHandler = &FakeResultHandler{}

View File

@@ -102,7 +102,7 @@ func TestEngineProcessJob(t *testing.T) {
require.NoError(t, err)
store := &AlertStoreMock{}
engine := ProvideAlertEngine(nil, bus, nil, nil, usMock, ossencryption.ProvideService(), nil, tracer, store, setting.NewCfg())
engine := ProvideAlertEngine(nil, bus, nil, nil, usMock, ossencryption.ProvideService(), nil, tracer, store, setting.NewCfg(), nil)
setting.AlertingEvaluationTimeout = 30 * time.Second
setting.AlertingNotificationTimeout = 30 * time.Second
setting.AlertingMaxAttempts = 3

View File

@@ -6,31 +6,34 @@ import (
"errors"
"fmt"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/datasources/permissions"
)
// DashAlertExtractor extracts alerts from the dashboard json.
type DashAlertExtractor struct {
User *models.SignedInUser
Dash *models.Dashboard
OrgID int64
log log.Logger
type DashAlertExtractor interface {
GetAlerts(ctx context.Context, dashAlertInfo DashAlertInfo) ([]*models.Alert, error)
ValidateAlerts(ctx context.Context, dashAlertInfo DashAlertInfo) error
}
// NewDashAlertExtractor returns a new DashAlertExtractor.
func NewDashAlertExtractor(dash *models.Dashboard, orgID int64, user *models.SignedInUser) *DashAlertExtractor {
return &DashAlertExtractor{
User: user,
Dash: dash,
OrgID: orgID,
log: log.New("alerting.extractor"),
// DashAlertExtractorService extracts alerts from the dashboard json.
type DashAlertExtractorService struct {
datasourcePermissionsService permissions.DatasourcePermissionsService
datasourceService datasources.DataSourceService
log log.Logger
}
func ProvideDashAlertExtractorService(datasourcePermissionsService permissions.DatasourcePermissionsService, datasourceService datasources.DataSourceService) *DashAlertExtractorService {
return &DashAlertExtractorService{
datasourcePermissionsService: datasourcePermissionsService,
datasourceService: datasourceService,
log: log.New("alerting.extractor"),
}
}
func (e *DashAlertExtractor) lookupQueryDataSource(ctx context.Context, panel *simplejson.Json, panelQuery *simplejson.Json) (*models.DataSource, error) {
func (e *DashAlertExtractorService) lookupQueryDataSource(ctx context.Context, panel *simplejson.Json, panelQuery *simplejson.Json, orgID int64) (*models.DataSource, error) {
dsName := ""
dsUid := ""
@@ -47,15 +50,15 @@ func (e *DashAlertExtractor) lookupQueryDataSource(ctx context.Context, panel *s
}
if dsName == "" && dsUid == "" {
query := &models.GetDefaultDataSourceQuery{OrgId: e.OrgID}
if err := bus.Dispatch(ctx, query); err != nil {
query := &models.GetDefaultDataSourceQuery{OrgId: orgID}
if err := e.datasourceService.GetDefaultDataSource(ctx, query); err != nil {
return nil, err
}
return query.Result, nil
}
query := &models.GetDataSourceQuery{Name: dsName, Uid: dsUid, OrgId: e.OrgID}
if err := bus.Dispatch(ctx, query); err != nil {
query := &models.GetDataSourceQuery{Name: dsName, Uid: dsUid, OrgId: orgID}
if err := e.datasourceService.GetDataSource(ctx, query); err != nil {
return nil, err
}
@@ -101,7 +104,7 @@ func UAEnabled(ctx context.Context) bool {
return enabled
}
func (e *DashAlertExtractor) getAlertFromPanels(ctx context.Context, jsonWithPanels *simplejson.Json, validateAlertFunc func(*models.Alert) bool, logTranslationFailures bool) ([]*models.Alert, error) {
func (e *DashAlertExtractorService) getAlertFromPanels(ctx context.Context, jsonWithPanels *simplejson.Json, validateAlertFunc func(*models.Alert) bool, logTranslationFailures bool, dashAlertInfo DashAlertInfo) ([]*models.Alert, error) {
alerts := make([]*models.Alert, 0)
for _, panelObj := range jsonWithPanels.Get("panels").MustArray() {
@@ -111,7 +114,7 @@ func (e *DashAlertExtractor) getAlertFromPanels(ctx context.Context, jsonWithPan
// check if the panel is collapsed
if collapsed && collapsedJSON.MustBool() {
// extract alerts from sub panels for collapsed panels
alertSlice, err := e.getAlertFromPanels(ctx, panel, validateAlertFunc, logTranslationFailures)
alertSlice, err := e.getAlertFromPanels(ctx, panel, validateAlertFunc, logTranslationFailures, dashAlertInfo)
if err != nil {
return nil, err
}
@@ -143,8 +146,8 @@ func (e *DashAlertExtractor) getAlertFromPanels(ctx context.Context, jsonWithPan
Err: validationErr.Err,
PanelID: panelID,
}
if e.Dash != nil {
ve.DashboardID = e.Dash.Id
if dashAlertInfo.Dash != nil {
ve.DashboardID = dashAlertInfo.Dash.Id
}
return ve
}
@@ -170,8 +173,8 @@ func (e *DashAlertExtractor) getAlertFromPanels(ctx context.Context, jsonWithPan
}
alert := &models.Alert{
DashboardId: e.Dash.Id,
OrgId: e.OrgID,
DashboardId: dashAlertInfo.Dash.Id,
OrgId: dashAlertInfo.OrgID,
PanelId: panelID,
Id: jsonAlert.Get("id").MustInt64(),
Name: jsonAlert.Get("name").MustString(),
@@ -198,24 +201,21 @@ func (e *DashAlertExtractor) getAlertFromPanels(ctx context.Context, jsonWithPan
return nil, ValidationError{Reason: reason}
}
datasource, err := e.lookupQueryDataSource(ctx, panel, panelQuery)
datasource, err := e.lookupQueryDataSource(ctx, panel, panelQuery, dashAlertInfo.OrgID)
if err != nil {
return nil, err
}
dsFilterQuery := models.DatasourcesPermissionFilterQuery{
User: e.User,
User: dashAlertInfo.User,
Datasources: []*models.DataSource{datasource},
}
if err := bus.Dispatch(ctx, &dsFilterQuery); err != nil {
if !errors.Is(err, bus.ErrHandlerNotFound) {
return nil, err
}
} else {
if len(dsFilterQuery.Result) == 0 {
return nil, models.ErrDataSourceAccessDenied
}
if err := e.datasourcePermissionsService.FilterDatasourcesBasedOnQueryPermissions(ctx, &dsFilterQuery); err != nil {
return nil, err
}
if len(dsFilterQuery.Result) == 0 {
return nil, models.ErrDataSourceAccessDenied
}
jsonQuery.SetPath([]string{"datasourceId"}, datasource.Id)
@@ -250,12 +250,12 @@ func validateAlertRule(alert *models.Alert) bool {
}
// GetAlerts extracts alerts from the dashboard json and does full validation on the alert json data.
func (e *DashAlertExtractor) GetAlerts(ctx context.Context) ([]*models.Alert, error) {
return e.extractAlerts(ctx, validateAlertRule, true)
func (e *DashAlertExtractorService) GetAlerts(ctx context.Context, dashAlertInfo DashAlertInfo) ([]*models.Alert, error) {
return e.extractAlerts(ctx, validateAlertRule, true, dashAlertInfo)
}
func (e *DashAlertExtractor) extractAlerts(ctx context.Context, validateFunc func(alert *models.Alert) bool, logTranslationFailures bool) ([]*models.Alert, error) {
dashboardJSON, err := copyJSON(e.Dash.Data)
func (e *DashAlertExtractorService) extractAlerts(ctx context.Context, validateFunc func(alert *models.Alert) bool, logTranslationFailures bool, dashAlertInfo DashAlertInfo) ([]*models.Alert, error) {
dashboardJSON, err := copyJSON(dashAlertInfo.Dash.Data)
if err != nil {
return nil, err
}
@@ -268,7 +268,7 @@ func (e *DashAlertExtractor) extractAlerts(ctx context.Context, validateFunc fun
if len(rows) > 0 {
for _, rowObj := range rows {
row := simplejson.NewFromAny(rowObj)
a, err := e.getAlertFromPanels(ctx, row, validateFunc, logTranslationFailures)
a, err := e.getAlertFromPanels(ctx, row, validateFunc, logTranslationFailures, dashAlertInfo)
if err != nil {
return nil, err
}
@@ -276,7 +276,7 @@ func (e *DashAlertExtractor) extractAlerts(ctx context.Context, validateFunc fun
alerts = append(alerts, a...)
}
} else {
a, err := e.getAlertFromPanels(ctx, dashboardJSON, validateFunc, logTranslationFailures)
a, err := e.getAlertFromPanels(ctx, dashboardJSON, validateFunc, logTranslationFailures, dashAlertInfo)
if err != nil {
return nil, err
}
@@ -290,9 +290,9 @@ func (e *DashAlertExtractor) extractAlerts(ctx context.Context, validateFunc fun
// ValidateAlerts validates alerts in the dashboard json but does not require a valid dashboard id
// in the first validation pass.
func (e *DashAlertExtractor) ValidateAlerts(ctx context.Context) error {
func (e *DashAlertExtractorService) ValidateAlerts(ctx context.Context, dashAlertInfo DashAlertInfo) error {
_, err := e.extractAlerts(ctx, func(alert *models.Alert) bool {
return alert.OrgId != 0 && alert.PanelId != 0
}, false)
}, false, dashAlertInfo)
return err
}

View File

@@ -6,9 +6,10 @@ import (
"testing"
"time"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/datasources/permissions"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/stretchr/testify/require"
)
@@ -21,40 +22,24 @@ func TestAlertRuleExtraction(t *testing.T) {
// mock data
defaultDs := &models.DataSource{Id: 12, OrgId: 1, Name: "I am default", IsDefault: true, Uid: "def-uid"}
graphite2Ds := &models.DataSource{Id: 15, OrgId: 1, Name: "graphite2", Uid: "graphite2-uid"}
influxDBDs := &models.DataSource{Id: 16, OrgId: 1, Name: "InfluxDB", Uid: "InfluxDB-uid"}
prom := &models.DataSource{Id: 17, OrgId: 1, Name: "Prometheus", Uid: "Prometheus-uid"}
bus.AddHandler("test", func(ctx context.Context, query *models.GetDefaultDataSourceQuery) error {
query.Result = defaultDs
return nil
})
bus.AddHandler("test", func(ctx context.Context, query *models.GetDataSourceQuery) error {
if query.Name == defaultDs.Name || query.Uid == defaultDs.Uid {
query.Result = defaultDs
}
if query.Name == graphite2Ds.Name || query.Uid == graphite2Ds.Uid {
query.Result = graphite2Ds
}
if query.Name == influxDBDs.Name || query.Uid == influxDBDs.Uid {
query.Result = influxDBDs
}
if query.Name == prom.Name || query.Uid == prom.Uid {
query.Result = prom
}
return nil
})
json, err := ioutil.ReadFile("./testdata/graphite-alert.json")
require.Nil(t, err)
dsPermissions := permissions.NewMockDatasourcePermissionService()
dsPermissions.DsResult = []*models.DataSource{
{
Id: 1,
},
}
dsService := &fakeDatasourceService{ExpectedDatasource: defaultDs}
extractor := ProvideDashAlertExtractorService(dsPermissions, dsService)
t.Run("Parsing alert rules from dashboard json", func(t *testing.T) {
dashJSON, err := simplejson.NewJson(json)
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
getTarget := func(j *simplejson.Json) string {
rowObj := j.Get("rows").MustArray()[0]
row := simplejson.NewFromAny(rowObj)
@@ -67,8 +52,11 @@ func TestAlertRuleExtraction(t *testing.T) {
require.Equal(t, getTarget(dashJSON), "")
extractor := NewDashAlertExtractor(dash, 1, nil)
_, _ = extractor.GetAlerts(context.Background())
_, _ = extractor.GetAlerts(context.Background(), DashAlertInfo{
User: nil,
Dash: models.NewDashboardFromJson(dashJSON),
OrgID: 1,
})
require.Equal(t, getTarget(dashJSON), "")
})
@@ -77,10 +65,12 @@ func TestAlertRuleExtraction(t *testing.T) {
dashJSON, err := simplejson.NewJson(json)
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
alerts, err := extractor.GetAlerts(context.Background())
dsService.ExpectedDatasource = &models.DataSource{Id: 12}
alerts, err := extractor.GetAlerts(context.Background(), DashAlertInfo{
User: nil,
Dash: models.NewDashboardFromJson(dashJSON),
OrgID: 1,
})
require.Nil(t, err)
@@ -127,10 +117,12 @@ func TestAlertRuleExtraction(t *testing.T) {
dashJSON, err := simplejson.NewJson(panelWithoutID)
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
_, err = extractor.GetAlerts(context.Background())
_, err = extractor.GetAlerts(context.Background(), DashAlertInfo{
User: nil,
Dash: models.NewDashboardFromJson(dashJSON),
OrgID: 1,
})
require.NotNil(t, err)
})
@@ -141,10 +133,12 @@ func TestAlertRuleExtraction(t *testing.T) {
dashJSON, err := simplejson.NewJson(panelWithIDZero)
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
_, err = extractor.GetAlerts(context.Background())
_, err = extractor.GetAlerts(context.Background(), DashAlertInfo{
User: nil,
Dash: models.NewDashboardFromJson(dashJSON),
OrgID: 1,
})
require.NotNil(t, err)
})
@@ -154,10 +148,12 @@ func TestAlertRuleExtraction(t *testing.T) {
require.Nil(t, err)
dashJSON, err := simplejson.NewJson(panelWithQuery)
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
_, err = extractor.GetAlerts(WithUAEnabled(context.Background(), true))
_, err = extractor.GetAlerts(WithUAEnabled(context.Background(), true), DashAlertInfo{
User: nil,
Dash: models.NewDashboardFromJson(dashJSON),
OrgID: 1,
})
require.Equal(t, "alert validation error: Alert on PanelId: 2 refers to query(B) that cannot be found. Legacy alerting queries are not able to be removed at this time in order to preserve the ability to rollback to previous versions of Grafana", err.Error())
})
@@ -167,10 +163,13 @@ func TestAlertRuleExtraction(t *testing.T) {
dashJSON, err := simplejson.NewJson(panelWithoutSpecifiedDatasource)
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
alerts, err := extractor.GetAlerts(context.Background())
dsService.ExpectedDatasource = &models.DataSource{Id: 12}
alerts, err := extractor.GetAlerts(context.Background(), DashAlertInfo{
User: nil,
Dash: models.NewDashboardFromJson(dashJSON),
OrgID: 1,
})
require.Nil(t, err)
condition := simplejson.NewFromAny(alerts[0].Settings.Get("conditions").MustArray()[0])
@@ -184,10 +183,12 @@ func TestAlertRuleExtraction(t *testing.T) {
dashJSON, err := simplejson.NewJson(json)
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
alerts, err := extractor.GetAlerts(context.Background())
alerts, err := extractor.GetAlerts(context.Background(), DashAlertInfo{
User: nil,
Dash: models.NewDashboardFromJson(dashJSON),
OrgID: 1,
})
require.Nil(t, err)
require.Len(t, alerts, 2)
@@ -209,10 +210,12 @@ func TestAlertRuleExtraction(t *testing.T) {
dashJSON, err := simplejson.NewJson(json)
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
alerts, err := extractor.GetAlerts(context.Background())
alerts, err := extractor.GetAlerts(context.Background(), DashAlertInfo{
User: nil,
Dash: models.NewDashboardFromJson(dashJSON),
OrgID: 1,
})
require.Nil(t, err)
require.Len(t, alerts, 1)
@@ -235,9 +238,12 @@ func TestAlertRuleExtraction(t *testing.T) {
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
alerts, err := extractor.GetAlerts(context.Background())
alerts, err := extractor.GetAlerts(context.Background(), DashAlertInfo{
User: nil,
Dash: dash,
OrgID: 1,
})
require.Nil(t, err)
require.Len(t, alerts, 4)
@@ -249,14 +255,17 @@ func TestAlertRuleExtraction(t *testing.T) {
dashJSON, err := simplejson.NewJson(json)
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
err = extractor.ValidateAlerts(context.Background())
dashAlertInfo := DashAlertInfo{
User: nil,
Dash: models.NewDashboardFromJson(dashJSON),
OrgID: 1,
}
err = extractor.ValidateAlerts(context.Background(), dashAlertInfo)
require.Nil(t, err)
_, err = extractor.GetAlerts(context.Background())
_, err = extractor.GetAlerts(context.Background(), dashAlertInfo)
require.Equal(t, err.Error(), "alert validation error: Panel id is not correct, alertName=Influxdb, panelId=1")
})
@@ -266,14 +275,18 @@ func TestAlertRuleExtraction(t *testing.T) {
dashJSON, err := simplejson.NewJson(json)
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
err = extractor.ValidateAlerts(context.Background())
dsService.ExpectedDatasource = graphite2Ds
dashAlertInfo := DashAlertInfo{
User: nil,
Dash: models.NewDashboardFromJson(dashJSON),
OrgID: 1,
}
err = extractor.ValidateAlerts(context.Background(), dashAlertInfo)
require.Nil(t, err)
alerts, err := extractor.GetAlerts(context.Background())
alerts, err := extractor.GetAlerts(context.Background(), dashAlertInfo)
require.Nil(t, err)
condition := simplejson.NewFromAny(alerts[0].Settings.Get("conditions").MustArray()[0])
@@ -281,3 +294,18 @@ func TestAlertRuleExtraction(t *testing.T) {
require.EqualValues(t, 15, query.Get("datasourceId").MustInt64())
})
}
type fakeDatasourceService struct {
ExpectedDatasource *models.DataSource
datasources.DataSourceService
}
func (f *fakeDatasourceService) GetDefaultDataSource(ctx context.Context, query *models.GetDefaultDataSourceQuery) error {
query.Result = f.ExpectedDatasource
return nil
}
func (f *fakeDatasourceService) GetDataSource(ctx context.Context, query *models.GetDataSourceQuery) error {
query.Result = f.ExpectedDatasource
return nil
}

View File

@@ -4,6 +4,7 @@ import (
"sync"
"github.com/grafana/grafana/pkg/components/null"
"github.com/grafana/grafana/pkg/models"
)
// Job holds state about when the alert rule should be evaluated.
@@ -42,3 +43,9 @@ type EvalMatch struct {
Metric string `json:"metric"`
Tags map[string]string `json:"tags"`
}
type DashAlertInfo struct {
User *models.SignedInUser
Dash *models.Dashboard
OrgID int64
}

View File

@@ -11,9 +11,12 @@ import (
// AlertTest makes a test alert.
func (e *AlertEngine) AlertTest(orgID int64, dashboard *simplejson.Json, panelID int64, user *models.SignedInUser) (*EvalContext, error) {
dash := models.NewDashboardFromJson(dashboard)
extractor := NewDashAlertExtractor(dash, orgID, user)
alerts, err := extractor.GetAlerts(context.Background())
dashInfo := DashAlertInfo{
User: user,
Dash: dash,
OrgID: orgID,
}
alerts, err := e.dashAlertExtractor.GetAlerts(context.Background(), dashInfo)
if err != nil {
return nil, err
}