alerting: make alert extractor backwards compatible

closes #10270

pkg/services/alerting/extractor.go

@@ -69,95 +69,120 @@ func copyJson(in *simplejson.Json) (*simplejson.Json, error) {
 	return simplejson.NewJson(rawJson)
 }
 
-func (e *DashAlertExtractor) GetAlerts() ([]*m.Alert, error) {
-	e.log.Debug("GetAlerts")
+func (e *DashAlertExtractor) GetAlertFromPanels(jsonWithPanels *simplejson.Json) ([]*m.Alert, error) {
+	alerts := make([]*m.Alert, 0)
 
-	dashboardJson, err := copyJson(e.Dash.Data)
-	if err != nil {
-		return nil, err
-	}
+	for _, panelObj := range jsonWithPanels.Get("panels").MustArray() {
+		panel := simplejson.NewFromAny(panelObj)
+		jsonAlert, hasAlert := panel.CheckGet("alert")
 
-	alerts := make([]*m.Alert, 0)
-	for _, rowObj := range dashboardJson.Get("rows").MustArray() {
-		row := simplejson.NewFromAny(rowObj)
+		if !hasAlert {
+			continue
+		}
 
-		for _, panelObj := range row.Get("panels").MustArray() {
-			panel := simplejson.NewFromAny(panelObj)
-			jsonAlert, hasAlert := panel.CheckGet("alert")
+		panelId, err := panel.Get("id").Int64()
+		if err != nil {
+			return nil, fmt.Errorf("panel id is required. err %v", err)
+		}
 
-			if !hasAlert {
-				continue
-			}
+		// backward compatibility check, can be removed later
+		enabled, hasEnabled := jsonAlert.CheckGet("enabled")
+		if hasEnabled && enabled.MustBool() == false {
+			continue
+		}
 
-			panelId, err := panel.Get("id").Int64()
-			if err != nil {
-				return nil, fmt.Errorf("panel id is required. err %v", err)
-			}
+		frequency, err := getTimeDurationStringToSeconds(jsonAlert.Get("frequency").MustString())
+		if err != nil {
+			return nil, ValidationError{Reason: "Could not parse frequency"}
+		}
+
+		alert := &m.Alert{
+			DashboardId: e.Dash.Id,
+			OrgId:       e.OrgId,
+			PanelId:     panelId,
+			Id:          jsonAlert.Get("id").MustInt64(),
+			Name:        jsonAlert.Get("name").MustString(),
+			Handler:     jsonAlert.Get("handler").MustInt64(),
+			Message:     jsonAlert.Get("message").MustString(),
+			Frequency:   frequency,
+		}
+
+		for _, condition := range jsonAlert.Get("conditions").MustArray() {
+			jsonCondition := simplejson.NewFromAny(condition)
+
+			jsonQuery := jsonCondition.Get("query")
+			queryRefId := jsonQuery.Get("params").MustArray()[0].(string)
+			panelQuery := findPanelQueryByRefId(panel, queryRefId)
 
-			// backward compatibility check, can be removed later
-			enabled, hasEnabled := jsonAlert.CheckGet("enabled")
-			if hasEnabled && enabled.MustBool() == false {
-				continue
+			if panelQuery == nil {
+				reason := fmt.Sprintf("Alert on PanelId: %v refers to query(%s) that cannot be found", alert.PanelId, queryRefId)
+				return nil, ValidationError{Reason: reason}
 			}
 
-			frequency, err := getTimeDurationStringToSeconds(jsonAlert.Get("frequency").MustString())
-			if err != nil {
-				return nil, ValidationError{Reason: "Could not parse frequency"}
+			dsName := ""
+			if panelQuery.Get("datasource").MustString() != "" {
+				dsName = panelQuery.Get("datasource").MustString()
+			} else if panel.Get("datasource").MustString() != "" {
+				dsName = panel.Get("datasource").MustString()
 			}
 
-			alert := &m.Alert{
-				DashboardId: e.Dash.Id,
-				OrgId:       e.OrgId,
-				PanelId:     panelId,
-				Id:          jsonAlert.Get("id").MustInt64(),
-				Name:        jsonAlert.Get("name").MustString(),
-				Handler:     jsonAlert.Get("handler").MustInt64(),
-				Message:     jsonAlert.Get("message").MustString(),
-				Frequency:   frequency,
+			if datasource, err := e.lookupDatasourceId(dsName); err != nil {
+				return nil, err
+			} else {
+				jsonQuery.SetPath([]string{"datasourceId"}, datasource.Id)
 			}
 
-			for _, condition := range jsonAlert.Get("conditions").MustArray() {
-				jsonCondition := simplejson.NewFromAny(condition)
+			if interval, err := panel.Get("interval").String(); err == nil {
+				panelQuery.Set("interval", interval)
+			}
 
-				jsonQuery := jsonCondition.Get("query")
-				queryRefId := jsonQuery.Get("params").MustArray()[0].(string)
-				panelQuery := findPanelQueryByRefId(panel, queryRefId)
+			jsonQuery.Set("model", panelQuery.Interface())
+		}
 
-				if panelQuery == nil {
-					reason := fmt.Sprintf("Alert on PanelId: %v refers to query(%s) that cannot be found", alert.PanelId, queryRefId)
-					return nil, ValidationError{Reason: reason}
-				}
+		alert.Settings = jsonAlert
 
-				dsName := ""
-				if panelQuery.Get("datasource").MustString() != "" {
-					dsName = panelQuery.Get("datasource").MustString()
-				} else if panel.Get("datasource").MustString() != "" {
-					dsName = panel.Get("datasource").MustString()
-				}
+		// validate
+		_, err = NewRuleFromDBAlert(alert)
+		if err == nil && alert.ValidToSave() {
+			alerts = append(alerts, alert)
+		} else {
+			return nil, err
+		}
+	}
 
-				if datasource, err := e.lookupDatasourceId(dsName); err != nil {
-					return nil, err
-				} else {
-					jsonQuery.SetPath([]string{"datasourceId"}, datasource.Id)
-				}
+	return alerts, nil
+}
 
-				if interval, err := panel.Get("interval").String(); err == nil {
-					panelQuery.Set("interval", interval)
-				}
+func (e *DashAlertExtractor) GetAlerts() ([]*m.Alert, error) {
+	e.log.Debug("GetAlerts")
 
-				jsonQuery.Set("model", panelQuery.Interface())
-			}
+	dashboardJson, err := copyJson(e.Dash.Data)
+	if err != nil {
+		return nil, err
+	}
 
-			alert.Settings = jsonAlert
+	alerts := make([]*m.Alert, 0)
 
-			// validate
-			_, err = NewRuleFromDBAlert(alert)
-			if err == nil && alert.ValidToSave() {
-				alerts = append(alerts, alert)
-			} else {
+	// We extract alerts from rows to be backwards compatible
+	// with the old dashboard json model.
+	rows := dashboardJson.Get("rows").MustArray()
+	if len(rows) > 0 {
+		for _, rowObj := range rows {
+			row := simplejson.NewFromAny(rowObj)
+			a, err := e.GetAlertFromPanels(row)
+			if err != nil {
 				return nil, err
 			}
+
+			alerts = append(alerts, a...)
+		}
+	} else {
+		a, err := e.GetAlertFromPanels(dashboardJson)
+		if err != nil {
+			return nil, err
 		}
+
+		alerts = append(alerts, a...)
 	}
 
 	e.log.Debug("Extracted alerts from dashboard", "alertCount", len(alerts))

pkg/services/alerting/extractor_test.go

@@ -155,11 +155,31 @@ func TestAlertRuleExtraction(t *testing.T) {
 			})
 		})
 
+		Convey("Parse alerts from dashboard without rows", func() {
+			json, err := ioutil.ReadFile("./test-data/v5-dashboard.json")
+			So(err, ShouldBeNil)
+
+			dashJson, err := simplejson.NewJson(json)
+			So(err, ShouldBeNil)
+			dash := m.NewDashboardFromJson(dashJson)
+			extractor := NewDashAlertExtractor(dash, 1)
+
+			alerts, err := extractor.GetAlerts()
+
+			Convey("Get rules without error", func() {
+				So(err, ShouldBeNil)
+			})
+
+			Convey("Should have 2 alert rule", func() {
+				So(len(alerts), ShouldEqual, 2)
+			})
+		})
+
 		Convey("Parse and validate dashboard containing influxdb alert", func() {
-			json2, err := ioutil.ReadFile("./test-data/influxdb-alert.json")
+			json, err := ioutil.ReadFile("./test-data/influxdb-alert.json")
 			So(err, ShouldBeNil)
 
-			dashJson, err := simplejson.NewJson(json2)
+			dashJson, err := simplejson.NewJson(json)
 			So(err, ShouldBeNil)
 			dash := m.NewDashboardFromJson(dashJson)
 			extractor := NewDashAlertExtractor(dash, 1)

pkg/services/alerting/test-data/v5-dashboard.json

@@ -0,0 +1,60 @@
+{
+    "id": 57,
+    "title": "Graphite 4",
+    "originalTitle": "Graphite 4",
+    "tags": ["graphite"],
+      "panels": [
+      {
+        "title": "Active desktop users",
+        "editable": true,
+        "type": "graph",
+        "id": 3,
+        "targets": [
+        {
+          "refId": "A",
+          "target": "aliasByNode(statsd.fakesite.counters.session_start.desktop.count, 4)"
+        }
+        ],
+        "datasource": null,
+        "alert": {
+          "name": "name1",
+          "message": "desc1",
+          "handler": 1,
+          "frequency": "60s",
+          "conditions": [
+          {
+            "type": "query",
+            "query": {"params": ["A", "5m", "now"]},
+            "reducer": {"type": "avg", "params": []},
+            "evaluator": {"type": ">", "params": [100]}
+          }
+          ]
+        }
+      },
+      {
+        "title": "Active mobile users",
+        "id": 4,
+        "targets": [
+          {"refId": "A", "target": ""},
+          {"refId": "B", "target": "aliasByNode(statsd.fakesite.counters.session_start.mobile.count, 4)"}
+        ],
+        "datasource": "graphite2",
+        "alert": {
+          "name": "name2",
+          "message": "desc2",
+          "handler": 0,
+          "frequency": "60s",
+          "severity": "warning",
+          "conditions": [
+          {
+            "type": "query",
+            "query":  {"params": ["B", "5m", "now"]},
+            "reducer": {"type": "avg", "params": []},
+            "evaluator": {"type": ">", "params": [100]}
+          }
+          ]
+        }
+
+    }
+  ]
+  }