From 2a9c625c9f3c01c4eff7f5c1b11add077a51e2d2 Mon Sep 17 00:00:00 2001 From: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> Date: Thu, 7 Jan 2021 21:33:17 +0100 Subject: [PATCH] Alerting: improve alerting default datasource search when extracting alerts (#29993) * improve alerting search datasource * correct the xorm get usage * adding default datasource unittest --- pkg/models/datasource.go | 6 + pkg/services/alerting/extractor.go | 21 +- pkg/services/alerting/extractor_test.go | 26 ++- .../panel-without-specified-datasource.json | 185 ++++++++++++++++++ pkg/services/sqlstore/datasource.go | 15 ++ pkg/services/sqlstore/datasource_test.go | 50 +++++ .../sqlstore/migrations/datasource_mig.go | 3 + 7 files changed, 290 insertions(+), 16 deletions(-) create mode 100644 pkg/services/alerting/testdata/panel-without-specified-datasource.json diff --git a/pkg/models/datasource.go b/pkg/models/datasource.go index b96f99b078a..59d04cc84b1 100644 --- a/pkg/models/datasource.go +++ b/pkg/models/datasource.go @@ -208,6 +208,12 @@ type GetDataSourcesQuery struct { Result []*DataSource } +type GetDefaultDataSourceQuery struct { + OrgId int64 + User *SignedInUser + Result *DataSource +} + type GetDataSourceByIdQuery struct { Id int64 OrgId int64 diff --git a/pkg/services/alerting/extractor.go b/pkg/services/alerting/extractor.go index 1c5cdb787a0..7159a177633 100644 --- a/pkg/services/alerting/extractor.go +++ b/pkg/services/alerting/extractor.go @@ -32,26 +32,19 @@ func NewDashAlertExtractor(dash *models.Dashboard, orgID int64, user *models.Sig func (e *DashAlertExtractor) lookupDatasourceID(dsName string) (*models.DataSource, error) { if dsName == "" { - query := &models.GetDataSourcesQuery{OrgId: e.OrgID} + query := &models.GetDefaultDataSourceQuery{OrgId: e.OrgID} if err := bus.Dispatch(query); err != nil { return nil, err } - - for _, ds := range query.Result { - if ds.IsDefault { - return ds, nil - } - } - } else { - query := &models.GetDataSourceByNameQuery{Name: dsName, OrgId: e.OrgID} - if err := bus.Dispatch(query); err != nil { - return nil, err - } - return query.Result, nil } - return nil, errors.New("Could not find datasource id for " + dsName) + query := &models.GetDataSourceByNameQuery{Name: dsName, OrgId: e.OrgID} + if err := bus.Dispatch(query); err != nil { + return nil, err + } + + return query.Result, nil } func findPanelQueryByRefID(panel *simplejson.Json, refID string) *simplejson.Json { diff --git a/pkg/services/alerting/extractor_test.go b/pkg/services/alerting/extractor_test.go index 5204e47f68c..881847572b9 100644 --- a/pkg/services/alerting/extractor_test.go +++ b/pkg/services/alerting/extractor_test.go @@ -24,8 +24,8 @@ func TestAlertRuleExtraction(t *testing.T) { influxDBDs := &models.DataSource{Id: 16, OrgId: 1, Name: "InfluxDB"} prom := &models.DataSource{Id: 17, OrgId: 1, Name: "Prometheus"} - bus.AddHandler("test", func(query *models.GetDataSourcesQuery) error { - query.Result = []*models.DataSource{defaultDs, graphite2Ds} + bus.AddHandler("test", func(query *models.GetDefaultDataSourceQuery) error { + query.Result = defaultDs return nil }) @@ -175,6 +175,28 @@ func TestAlertRuleExtraction(t *testing.T) { }) }) + Convey("Panel does not have datasource configured, use the default datasource", func() { + panelWithoutSpecifiedDatasource, err := ioutil.ReadFile("./testdata/panel-without-specified-datasource.json") + So(err, ShouldBeNil) + + dashJSON, err := simplejson.NewJson(panelWithoutSpecifiedDatasource) + So(err, ShouldBeNil) + dash := models.NewDashboardFromJson(dashJSON) + extractor := NewDashAlertExtractor(dash, 1, nil) + + alerts, err := extractor.GetAlerts() + + Convey("Get rules without error", func() { + So(err, ShouldBeNil) + }) + + Convey("Use default datasource", func() { + condition := simplejson.NewFromAny(alerts[0].Settings.Get("conditions").MustArray()[0]) + query := condition.Get("query") + So(query.Get("datasourceId").MustInt64(), ShouldEqual, 12) + }) + }) + Convey("Parse alerts from dashboard without rows", func() { json, err := ioutil.ReadFile("./testdata/v5-dashboard.json") So(err, ShouldBeNil) diff --git a/pkg/services/alerting/testdata/panel-without-specified-datasource.json b/pkg/services/alerting/testdata/panel-without-specified-datasource.json new file mode 100644 index 00000000000..38a49dd770f --- /dev/null +++ b/pkg/services/alerting/testdata/panel-without-specified-datasource.json @@ -0,0 +1,185 @@ +{ + "annotations": { + "list": [ + { + "builtIn": 1, + "datasource": "-- Grafana --", + "enable": true, + "hide": true, + "iconColor": "rgba(0, 211, 255, 1)", + "name": "Annotations & Alerts", + "type": "dashboard" + } + ] + }, + "editable": true, + "gnetId": null, + "graphTooltip": 0, + "id": 436, + "links": [], + "panels": [ + { + "alert": { + "alertRuleTags": {}, + "conditions": [ + { + "evaluator": { + "params": [ + 3 + ], + "type": "gt" + }, + "operator": { + "type": "and" + }, + "query": { + "params": [ + "A", + "15s", + "now" + ] + }, + "reducer": { + "params": [], + "type": "last" + }, + "type": "query" + } + ], + "executionErrorState": "alerting", + "for": "0m", + "frequency": "10s", + "handler": 1, + "message": "A message", + "name": "Ok / Alerting Cycle test default datasource", + "noDataState": "no_data", + "notifications": [] + }, + "aliasColors": {}, + "bars": false, + "dashLength": 10, + "dashes": false, + "datasource": null, + "fieldConfig": { + "defaults": { + "custom": {}, + "links": [] + }, + "overrides": [] + }, + "fill": 1, + "fillGradient": 0, + "gridPos": { + "h": 6, + "w": 12, + "x": 0, + "y": 0 + }, + "hiddenSeries": false, + "id": 2, + "legend": { + "avg": false, + "current": false, + "max": false, + "min": false, + "show": true, + "total": false, + "values": false + }, + "lines": true, + "linewidth": 1, + "nullPointMode": "null", + "options": { + "alertThreshold": true + }, + "percentage": false, + "pluginVersion": "7.4.0-pre", + "pointradius": 2, + "points": false, + "renderer": "flot", + "seriesOverrides": [], + "spaceLength": 10, + "stack": false, + "steppedLine": false, + "targets": [ + { + "pulseWave": { + "offCount": 6, + "offValue": "2", + "onCount": 6, + "onValue": "4", + "timeStep": 10 + }, + "refId": "A", + "scenarioId": "predictable_pulse", + "stringInput": "" + } + ], + "thresholds": [ + { + "colorMode": "critical", + "fill": true, + "line": true, + "op": "gt", + "value": 3 + } + ], + "timeFrom": null, + "timeRegions": [], + "timeShift": null, + "title": "Alerting / Ok", + "tooltip": { + "shared": true, + "sort": 0, + "value_type": "individual" + }, + "type": "graph", + "xaxis": { + "buckets": null, + "mode": "time", + "name": null, + "show": true, + "values": [] + }, + "yaxes": [ + { + "$$hashKey": "object:536", + "format": "short", + "label": null, + "logBase": 1, + "max": null, + "min": null, + "show": true + }, + { + "$$hashKey": "object:537", + "format": "short", + "label": null, + "logBase": 1, + "max": null, + "min": null, + "show": true + } + ], + "yaxis": { + "align": false, + "alignLevel": null + } + } + ], + "schemaVersion": 27, + "style": "dark", + "tags": [], + "templating": { + "list": [] + }, + "time": { + "from": "now-6h", + "to": "now" + }, + "timepicker": {}, + "timezone": "", + "title": "extractor test default datasource", + "uid": "bqQguKaMz", + "version": 7 +} \ No newline at end of file diff --git a/pkg/services/sqlstore/datasource.go b/pkg/services/sqlstore/datasource.go index 93dbdce8234..a46d88529d0 100644 --- a/pkg/services/sqlstore/datasource.go +++ b/pkg/services/sqlstore/datasource.go @@ -24,6 +24,7 @@ func init() { bus.AddHandler("sql", UpdateDataSource) bus.AddHandler("sql", GetDataSourceById) bus.AddHandler("sql", GetDataSourceByName) + bus.AddHandler("sql", GetDefaultDataSource) } func getDataSourceByID(id, orgID int64, engine *xorm.Engine) (*models.DataSource, error) { @@ -77,6 +78,20 @@ func GetDataSources(query *models.GetDataSourcesQuery) error { return sess.Find(&query.Result) } +// GetDefaultDataSource is used to get the default datasource of organization +func GetDefaultDataSource(query *models.GetDefaultDataSourceQuery) error { + datasource := models.DataSource{} + + exists, err := x.Where("org_id=? AND is_default=?", query.OrgId, true).Get(&datasource) + + if !exists { + return models.ErrDataSourceNotFound + } + + query.Result = &datasource + return err +} + func DeleteDataSourceById(cmd *models.DeleteDataSourceByIdCommand) error { return inTransaction(func(sess *DBSession) error { var rawSQL = "DELETE FROM data_source WHERE id=? and org_id=?" diff --git a/pkg/services/sqlstore/datasource_test.go b/pkg/services/sqlstore/datasource_test.go index cd6026f1afb..f319b5fbe54 100644 --- a/pkg/services/sqlstore/datasource_test.go +++ b/pkg/services/sqlstore/datasource_test.go @@ -3,10 +3,12 @@ package sqlstore import ( + "errors" "strconv" "testing" "github.com/grafana/grafana/pkg/models" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -287,3 +289,51 @@ func TestDataAccess(t *testing.T) { }) }) } + +func TestGetDefaultDataSource(t *testing.T) { + InitTestDB(t) + + t.Run("should return error if there is no default datasource", func(t *testing.T) { + cmd := models.AddDataSourceCommand{ + OrgId: 10, + Name: "nisse", + Type: models.DS_GRAPHITE, + Access: models.DS_ACCESS_DIRECT, + Url: "http://test", + } + + err := AddDataSource(&cmd) + require.NoError(t, err) + + query := models.GetDefaultDataSourceQuery{OrgId: 10} + err = GetDefaultDataSource(&query) + require.Error(t, err) + assert.True(t, errors.Is(err, models.ErrDataSourceNotFound)) + }) + + t.Run("should return default datasource if exists", func(t *testing.T) { + cmd := models.AddDataSourceCommand{ + OrgId: 10, + Name: "default datasource", + Type: models.DS_GRAPHITE, + Access: models.DS_ACCESS_DIRECT, + Url: "http://test", + IsDefault: true, + } + + err := AddDataSource(&cmd) + require.NoError(t, err) + + query := models.GetDefaultDataSourceQuery{OrgId: 10} + err = GetDefaultDataSource(&query) + require.NoError(t, err) + assert.Equal(t, "default datasource", query.Result.Name) + }) + + t.Run("should not return default datasource of other organisation", func(t *testing.T) { + query := models.GetDefaultDataSourceQuery{OrgId: 1} + err := GetDefaultDataSource(&query) + require.Error(t, err) + assert.True(t, errors.Is(err, models.ErrDataSourceNotFound)) + }) +} diff --git a/pkg/services/sqlstore/migrations/datasource_mig.go b/pkg/services/sqlstore/migrations/datasource_mig.go index a6470e47d23..124f5cd8dd4 100644 --- a/pkg/services/sqlstore/migrations/datasource_mig.go +++ b/pkg/services/sqlstore/migrations/datasource_mig.go @@ -151,4 +151,7 @@ func addDataSourceMigration(mg *Migrator) { mg.AddMigration("Add unique index datasource_org_id_uid", NewAddIndexMigration(tableV2, &Index{ Cols: []string{"org_id", "uid"}, Type: UniqueIndex, })) + + mg.AddMigration("add unique index datasource_org_id_is_default", NewAddIndexMigration(tableV2, &Index{ + Cols: []string{"org_id", "is_default"}})) }