Skip to content

Commit

Permalink
dashboard: allow alerts to be saved for new/provisioned dashboards
Browse files Browse the repository at this point in the history 
This changes the logic for the alert validation in the extractor. Validation
is executed twice, once before attempting to save the alerts and once after
saving the dashboard while saving the alerts to the db. The first validation
will not require that the alert has a dashboard id which allows new dashboards
with alerts to be saved.

Fixes #11247
  • Loading branch information
@daniellee
daniellee committed Mar 28, 2018
1 parent 9e2e6fc commit 68833fa
Show file tree
Hide file tree
Showing 5 changed files with 364 additions and 52 deletions.
17 changes: 5 additions & 12 deletions pkg/services/alerting/commands.go
View file
Expand Up @@ -13,11 +13,7 @@ func init() {
func validateDashboardAlerts(cmd *m.ValidateDashboardAlertsCommand) error {
extractor := NewDashAlertExtractor(cmd.Dashboard, cmd.OrgId)

if _, err := extractor.GetAlerts(); err != nil {
return err
}

return nil
return extractor.ValidateAlerts()
}

func updateDashboardAlerts(cmd *m.UpdateDashboardAlertsCommand) error {
Expand All @@ -29,15 +25,12 @@ func updateDashboardAlerts(cmd *m.UpdateDashboardAlertsCommand) error {

extractor := NewDashAlertExtractor(cmd.Dashboard, cmd.OrgId)

if alerts, err := extractor.GetAlerts(); err != nil {
alerts, err := extractor.GetAlerts()
if err != nil {
return err
} else {
saveAlerts.Alerts = alerts
}

if err := bus.Dispatch(&saveAlerts); err != nil {
return err
}
saveAlerts.Alerts = alerts

return nil
return bus.Dispatch(&saveAlerts)
}
95 changes: 56 additions & 39 deletions pkg/services/alerting/extractor.go
View file
Expand Up @@ -11,76 +11,78 @@ import (
m "github.com/grafana/grafana/pkg/models"
)

// DashAlertExtractor extracts alerts from the dashboard json
type DashAlertExtractor struct {
Dash *m.Dashboard
OrgId int64
OrgID int64
log log.Logger
}

func NewDashAlertExtractor(dash *m.Dashboard, orgId int64) *DashAlertExtractor {
// NewDashAlertExtractor returns a new DashAlertExtractor
func NewDashAlertExtractor(dash *m.Dashboard, orgID int64) *DashAlertExtractor {
return &DashAlertExtractor{
Dash: dash,
OrgId: orgId,
OrgID: orgID,
log: log.New("alerting.extractor"),
}
}

func (e *DashAlertExtractor) lookupDatasourceId(dsName string) (*m.DataSource, error) {
func (e *DashAlertExtractor) lookupDatasourceID(dsName string) (*m.DataSource, error) {
if dsName == "" {
query := &m.GetDataSourcesQuery{OrgId: e.OrgId}
query := &m.GetDataSourcesQuery{OrgId: e.OrgID}
if err := bus.Dispatch(query); err != nil {
return nil, err
} else {
for _, ds := range query.Result {
if ds.IsDefault {
return ds, nil
}
}

for _, ds := range query.Result {
if ds.IsDefault {
return ds, nil
}
}
} else {
query := &m.GetDataSourceByNameQuery{Name: dsName, OrgId: e.OrgId}
query := &m.GetDataSourceByNameQuery{Name: dsName, OrgId: e.OrgID}
if err := bus.Dispatch(query); err != nil {
return nil, err
} else {
return query.Result, nil
}

return query.Result, nil
}

return nil, errors.New("Could not find datasource id for " + dsName)
}

func findPanelQueryByRefId(panel *simplejson.Json, refId string) *simplejson.Json {
func findPanelQueryByRefID(panel *simplejson.Json, refID string) *simplejson.Json {
for _, targetsObj := range panel.Get("targets").MustArray() {
target := simplejson.NewFromAny(targetsObj)

if target.Get("refId").MustString() == refId {
if target.Get("refId").MustString() == refID {
return target
}
}
return nil
}

func copyJson(in *simplejson.Json) (*simplejson.Json, error) {
rawJson, err := in.MarshalJSON()
func copyJSON(in *simplejson.Json) (*simplejson.Json, error) {
rawJSON, err := in.MarshalJSON()
if err != nil {
return nil, err
}

return simplejson.NewJson(rawJson)
return simplejson.NewJson(rawJSON)
}

func (e *DashAlertExtractor) GetAlertFromPanels(jsonWithPanels *simplejson.Json) ([]*m.Alert, error) {
func (e *DashAlertExtractor) getAlertFromPanels(jsonWithPanels *simplejson.Json, validateAlertFunc func(*m.Alert) bool) ([]*m.Alert, error) {
alerts := make([]*m.Alert, 0)

for _, panelObj := range jsonWithPanels.Get("panels").MustArray() {
panel := simplejson.NewFromAny(panelObj)

collapsedJson, collapsed := panel.CheckGet("collapsed")
collapsedJSON, collapsed := panel.CheckGet("collapsed")
// check if the panel is collapsed
if collapsed && collapsedJson.MustBool() {
if collapsed && collapsedJSON.MustBool() {

// extract alerts from sub panels for collapsed panels
als, err := e.GetAlertFromPanels(panel)
als, err := e.getAlertFromPanels(panel, validateAlertFunc)
if err != nil {
return nil, err
}
Expand All @@ -95,7 +97,7 @@ func (e *DashAlertExtractor) GetAlertFromPanels(jsonWithPanels *simplejson.Json)
continue
}

panelId, err := panel.Get("id").Int64()
panelID, err := panel.Get("id").Int64()
if err != nil {
return nil, fmt.Errorf("panel id is required. err %v", err)
}
Expand All @@ -113,8 +115,8 @@ func (e *DashAlertExtractor) GetAlertFromPanels(jsonWithPanels *simplejson.Json)

alert := &m.Alert{
DashboardId: e.Dash.Id,
OrgId: e.OrgId,
PanelId: panelId,
OrgId: e.OrgID,
PanelId: panelID,
Id: jsonAlert.Get("id").MustInt64(),
Name: jsonAlert.Get("name").MustString(),
Handler: jsonAlert.Get("handler").MustInt64(),
Expand All @@ -126,11 +128,11 @@ func (e *DashAlertExtractor) GetAlertFromPanels(jsonWithPanels *simplejson.Json)
jsonCondition := simplejson.NewFromAny(condition)

jsonQuery := jsonCondition.Get("query")
queryRefId := jsonQuery.Get("params").MustArray()[0].(string)
panelQuery := findPanelQueryByRefId(panel, queryRefId)
queryRefID := jsonQuery.Get("params").MustArray()[0].(string)
panelQuery := findPanelQueryByRefID(panel, queryRefID)

if panelQuery == nil {
reason := fmt.Sprintf("Alert on PanelId: %v refers to query(%s) that cannot be found", alert.PanelId, queryRefId)
reason := fmt.Sprintf("Alert on PanelId: %v refers to query(%s) that cannot be found", alert.PanelId, queryRefID)
return nil, ValidationError{Reason: reason}
}

Expand All @@ -141,12 +143,13 @@ func (e *DashAlertExtractor) GetAlertFromPanels(jsonWithPanels *simplejson.Json)
dsName = panel.Get("datasource").MustString()
}

if datasource, err := e.lookupDatasourceId(dsName); err != nil {
datasource, err := e.lookupDatasourceID(dsName)
if err != nil {
return nil, err
} else {
jsonQuery.SetPath([]string{"datasourceId"}, datasource.Id)
}

jsonQuery.SetPath([]string{"datasourceId"}, datasource.Id)

if interval, err := panel.Get("interval").String(); err == nil {
panelQuery.Set("interval", interval)
}
Expand All @@ -162,21 +165,28 @@ func (e *DashAlertExtractor) GetAlertFromPanels(jsonWithPanels *simplejson.Json)
return nil, err
}

if alert.ValidToSave() {
alerts = append(alerts, alert)
} else {
if !validateAlertFunc(alert) {
e.log.Debug("Invalid Alert Data. Dashboard, Org or Panel ID is not correct", "alertName", alert.Name, "panelId", alert.PanelId)
return nil, m.ErrDashboardContainsInvalidAlertData
}

alerts = append(alerts, alert)
}

return alerts, nil
}

func validateAlertRule(alert *m.Alert) bool {
return alert.ValidToSave()
}

// GetAlerts extracts alerts from the dashboard json and does full validation on the alert json data
func (e *DashAlertExtractor) GetAlerts() ([]*m.Alert, error) {
e.log.Debug("GetAlerts")
return e.extractAlerts(validateAlertRule)
}

dashboardJson, err := copyJson(e.Dash.Data)
func (e *DashAlertExtractor) extractAlerts(validateFunc func(alert *m.Alert) bool) ([]*m.Alert, error) {
dashboardJSON, err := copyJSON(e.Dash.Data)
if err != nil {
return nil, err
}
Expand All @@ -185,19 +195,19 @@ func (e *DashAlertExtractor) GetAlerts() ([]*m.Alert, error) {

// We extract alerts from rows to be backwards compatible
// with the old dashboard json model.
rows := dashboardJson.Get("rows").MustArray()
rows := dashboardJSON.Get("rows").MustArray()
if len(rows) > 0 {
for _, rowObj := range rows {
row := simplejson.NewFromAny(rowObj)
a, err := e.GetAlertFromPanels(row)
a, err := e.getAlertFromPanels(row, validateFunc)
if err != nil {
return nil, err
}

alerts = append(alerts, a...)
}
} else {
a, err := e.GetAlertFromPanels(dashboardJson)
a, err := e.getAlertFromPanels(dashboardJSON, validateFunc)
if err != nil {
return nil, err
}
Expand All @@ -208,3 +218,10 @@ func (e *DashAlertExtractor) GetAlerts() ([]*m.Alert, error) {
e.log.Debug("Extracted alerts from dashboard", "alertCount", len(alerts))
return alerts, nil
}

// ValidateAlerts validates alerts in the dashboard json but does not require a valid dashboard id
// in the first validation pass
func (e *DashAlertExtractor) ValidateAlerts() error {
_, err := e.extractAlerts(func(alert *m.Alert) bool { return alert.OrgId != 0 && alert.PanelId != 0 })
return err
}
21 changes: 21 additions & 0 deletions pkg/services/alerting/extractor_test.go
View file
Expand Up @@ -240,5 +240,26 @@ func TestAlertRuleExtraction(t *testing.T) {
So(len(alerts), ShouldEqual, 4)
})
})

Convey("Parse and validate dashboard without id and containing an alert", func() {
json, err := ioutil.ReadFile("./test-data/dash-without-id.json")
So(err, ShouldBeNil)

dashJSON, err := simplejson.NewJson(json)
So(err, ShouldBeNil)
dash := m.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1)

err = extractor.ValidateAlerts()

Convey("Should validate without error", func() {
So(err, ShouldBeNil)
})

Convey("Should fail on save", func() {
_, err := extractor.GetAlerts()
So(err, ShouldEqual, m.ErrDashboardContainsInvalidAlertData)
})
})
})
}

0 comments on commit 68833fa

Please sign in to comment.