IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Alerting: Add checks for non supported units - disable defaulting to seconds (#32477)

Browse Source 
* Add error handling for unknown units

* Fix test cases

* Add case for empty string

* Changed tests from convey to testify

* Fix lints

* Move regex vars

* Add regex as ng-patterns on alert_tab.html

* Update public/app/features/alerting/partials/alert_tab.html

Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>

* Update public/app/features/alerting/partials/alert_tab.html

Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>

* Make zero and empty string not throw errors

* Updated validation error comments

* Frequency should allow zero or empty strings

* use checkFrequency instead of ng-pattern

checkFrequency is not triggered if ng-pattern is defined.

* Extract getForValue func - add tests

* Fix linting

Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>
This commit is contained in:
Dimitris Sotirakis 2021-04-12 15:53:51 +03:00 committed by GitHub
parent 1ed73ecef8
commit 258578766b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 342 additions and 345 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
pkg/services/alerting/extractor.go
View File

@ -4,7 +4,6 @@ import (
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
"time"
"github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/components/simplejson"
@ -109,12 +108,10 @@ func (e *DashAlertExtractor) getAlertFromPanels(jsonWithPanels *simplejson.Json,
} }
rawFor := jsonAlert.Get("for").MustString() rawFor := jsonAlert.Get("for").MustString()
var forValue time.Duration
if rawFor != "" { forValue, err := getForValue(rawFor)
forValue, err = time.ParseDuration(rawFor) if err != nil {
if err != nil { return nil, err
return nil, ValidationError{Reason: "Could not parse for"}
}
} }
alert := &models.Alert{ alert := &models.Alert{

398
pkg/services/alerting/extractor_test.go
View File

@ -9,293 +9,239 @@ import (
"github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore"
. "github.com/smartystreets/goconvey/convey" "github.com/stretchr/testify/require"
) )
func TestAlertRuleExtraction(t *testing.T) { func TestAlertRuleExtraction(t *testing.T) {
Convey("Parsing alert rules from dashboard json", t, func() { RegisterCondition("query", func(model *simplejson.Json, index int) (Condition, error) {
RegisterCondition("query", func(model *simplejson.Json, index int) (Condition, error) { return &FakeCondition{}, nil
return &FakeCondition{}, nil })
})
// mock data // mock data
defaultDs := &models.DataSource{Id: 12, OrgId: 1, Name: "I am default", IsDefault: true} defaultDs := &models.DataSource{Id: 12, OrgId: 1, Name: "I am default", IsDefault: true}
graphite2Ds := &models.DataSource{Id: 15, OrgId: 1, Name: "graphite2"} graphite2Ds := &models.DataSource{Id: 15, OrgId: 1, Name: "graphite2"}
influxDBDs := &models.DataSource{Id: 16, OrgId: 1, Name: "InfluxDB"} influxDBDs := &models.DataSource{Id: 16, OrgId: 1, Name: "InfluxDB"}
prom := &models.DataSource{Id: 17, OrgId: 1, Name: "Prometheus"} prom := &models.DataSource{Id: 17, OrgId: 1, Name: "Prometheus"}
bus.AddHandler("test", func(query *models.GetDefaultDataSourceQuery) error { bus.AddHandler("test", func(query *models.GetDefaultDataSourceQuery) error {
query.Result = defaultDs
return nil
})
bus.AddHandler("test", func(query *models.GetDataSourceQuery) error {
if query.Name == defaultDs.Name {
query.Result = defaultDs query.Result = defaultDs
return nil }
}) if query.Name == graphite2Ds.Name {
query.Result = graphite2Ds
}
if query.Name == influxDBDs.Name {
query.Result = influxDBDs
}
if query.Name == prom.Name {
query.Result = prom
}
bus.AddHandler("test", func(query *models.GetDataSourceQuery) error { return nil
if query.Name == defaultDs.Name { })
query.Result = defaultDs
}
if query.Name == graphite2Ds.Name {
query.Result = graphite2Ds
}
if query.Name == influxDBDs.Name {
query.Result = influxDBDs
}
if query.Name == prom.Name {
query.Result = prom
}
return nil json, err := ioutil.ReadFile("./testdata/graphite-alert.json")
}) require.Nil(t, err)
json, err := ioutil.ReadFile("./testdata/graphite-alert.json") t.Run("Parsing alert rules from dashboard json", func(t *testing.T) {
So(err, ShouldBeNil) dashJSON, err := simplejson.NewJson(json)
require.Nil(t, err)
Convey("Extractor should not modify the original json", func() { dash := models.NewDashboardFromJson(dashJSON)
dashJSON, err := simplejson.NewJson(json)
So(err, ShouldBeNil)
dash := models.NewDashboardFromJson(dashJSON) getTarget := func(j *simplejson.Json) string {
rowObj := j.Get("rows").MustArray()[0]
row := simplejson.NewFromAny(rowObj)
panelObj := row.Get("panels").MustArray()[0]
panel := simplejson.NewFromAny(panelObj)
conditionObj := panel.Get("alert").Get("conditions").MustArray()[0]
condition := simplejson.NewFromAny(conditionObj)
return condition.Get("query").Get("model").Get("target").MustString()
}
getTarget := func(j *simplejson.Json) string { require.Equal(t, getTarget(dashJSON), "")
rowObj := j.Get("rows").MustArray()[0]
row := simplejson.NewFromAny(rowObj)
panelObj := row.Get("panels").MustArray()[0]
panel := simplejson.NewFromAny(panelObj)
conditionObj := panel.Get("alert").Get("conditions").MustArray()[0]
condition := simplejson.NewFromAny(conditionObj)
return condition.Get("query").Get("model").Get("target").MustString()
}
Convey("Dashboard json rows.panels.alert.query.model.target should be empty", func() { extractor := NewDashAlertExtractor(dash, 1, nil)
So(getTarget(dashJSON), ShouldEqual, "") _, _ = extractor.GetAlerts()
})
extractor := NewDashAlertExtractor(dash, 1, nil) require.Equal(t, getTarget(dashJSON), "")
_, _ = extractor.GetAlerts() })
Convey("Dashboard json should not be updated after extracting rules", func() { t.Run("Parsing and validating dashboard containing graphite alerts", func(t *testing.T) {
So(getTarget(dashJSON), ShouldEqual, "") dashJSON, err := simplejson.NewJson(json)
}) require.Nil(t, err)
})
Convey("Parsing and validating dashboard containing graphite alerts", func() { dash := models.NewDashboardFromJson(dashJSON)
dashJSON, err := simplejson.NewJson(json) extractor := NewDashAlertExtractor(dash, 1, nil)
So(err, ShouldBeNil)
dash := models.NewDashboardFromJson(dashJSON) alerts, err := extractor.GetAlerts()
extractor := NewDashAlertExtractor(dash, 1, nil)
alerts, err := extractor.GetAlerts() require.Nil(t, err)
Convey("Get rules without error", func() { require.Len(t, alerts, 2)
So(err, ShouldBeNil)
})
Convey("all properties have been set", func() { for _, v := range alerts {
So(len(alerts), ShouldEqual, 2) require.EqualValues(t, v.DashboardId, 57)
require.NotEmpty(t, v.Name)
require.NotEmpty(t, v.Message)
for _, v := range alerts { settings := simplejson.NewFromAny(v.Settings)
So(v.DashboardId, ShouldEqual, 57) require.Equal(t, settings.Get("interval").MustString(""), "")
So(v.Name, ShouldNotBeEmpty) }
So(v.Message, ShouldNotBeEmpty)
settings := simplejson.NewFromAny(v.Settings) require.EqualValues(t, alerts[0].Handler, 1)
So(settings.Get("interval").MustString(""), ShouldEqual, "") require.EqualValues(t, alerts[1].Handler, 0)
}
Convey("should extract handler property", func() { require.EqualValues(t, alerts[0].Frequency, 60)
So(alerts[0].Handler, ShouldEqual, 1) require.EqualValues(t, alerts[1].Frequency, 60)
So(alerts[1].Handler, ShouldEqual, 0)
})
Convey("should extract frequency in seconds", func() { require.EqualValues(t, alerts[0].PanelId, 3)
So(alerts[0].Frequency, ShouldEqual, 60) require.EqualValues(t, alerts[1].PanelId, 4)
So(alerts[1].Frequency, ShouldEqual, 60)
})
Convey("should extract panel idc", func() { require.Equal(t, alerts[0].For, time.Minute*2)
So(alerts[0].PanelId, ShouldEqual, 3) require.Equal(t, alerts[1].For, time.Duration(0))
So(alerts[1].PanelId, ShouldEqual, 4)
})
Convey("should extract for param", func() { require.Equal(t, alerts[0].Name, "name1")
So(alerts[0].For, ShouldEqual, time.Minute*2) require.Equal(t, alerts[0].Message, "desc1")
So(alerts[1].For, ShouldEqual, time.Duration(0)) require.Equal(t, alerts[1].Name, "name2")
}) require.Equal(t, alerts[1].Message, "desc2")
Convey("should extract name and desc", func() { condition := simplejson.NewFromAny(alerts[0].Settings.Get("conditions").MustArray()[0])
So(alerts[0].Name, ShouldEqual, "name1") query := condition.Get("query")
So(alerts[0].Message, ShouldEqual, "desc1") require.EqualValues(t, query.Get("datasourceId").MustInt64(), 12)
So(alerts[1].Name, ShouldEqual, "name2")
So(alerts[1].Message, ShouldEqual, "desc2")
})
Convey("should set datasourceId", func() { condition = simplejson.NewFromAny(alerts[0].Settings.Get("conditions").MustArray()[0])
condition := simplejson.NewFromAny(alerts[0].Settings.Get("conditions").MustArray()[0]) model := condition.Get("query").Get("model")
query := condition.Get("query") require.Equal(t, model.Get("target").MustString(), "aliasByNode(statsd.fakesite.counters.session_start.desktop.count, 4)")
So(query.Get("datasourceId").MustInt64(), ShouldEqual, 12) })
})
Convey("should copy query model to condition", func() { t.Run("Panels missing id should return error", func(t *testing.T) {
condition := simplejson.NewFromAny(alerts[0].Settings.Get("conditions").MustArray()[0]) panelWithoutID, err := ioutil.ReadFile("./testdata/panels-missing-id.json")
model := condition.Get("query").Get("model") require.Nil(t, err)
So(model.Get("target").MustString(), ShouldEqual, "aliasByNode(statsd.fakesite.counters.session_start.desktop.count, 4)")
})
})
})
Convey("Panels missing id should return error", func() { dashJSON, err := simplejson.NewJson(panelWithoutID)
panelWithoutID, err := ioutil.ReadFile("./testdata/panels-missing-id.json") require.Nil(t, err)
So(err, ShouldBeNil) dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
dashJSON, err := simplejson.NewJson(panelWithoutID) _, err = extractor.GetAlerts()
So(err, ShouldBeNil)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
_, err = extractor.GetAlerts() require.NotNil(t, err)
})
Convey("panels without Id should return error", func() { t.Run("Panels missing id should return error", func(t *testing.T) {
So(err, ShouldNotBeNil) panelWithIDZero, err := ioutil.ReadFile("./testdata/panel-with-id-0.json")
}) require.Nil(t, err)
})
Convey("Panel with id set to zero should return error", func() { dashJSON, err := simplejson.NewJson(panelWithIDZero)
panelWithIDZero, err := ioutil.ReadFile("./testdata/panel-with-id-0.json") require.Nil(t, err)
So(err, ShouldBeNil) dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
dashJSON, err := simplejson.NewJson(panelWithIDZero) _, err = extractor.GetAlerts()
So(err, ShouldBeNil)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
_, err = extractor.GetAlerts() require.NotNil(t, err)
})
Convey("panel with id 0 should return error", func() { t.Run("Panel does not have datasource configured, use the default datasource", func(t *testing.T) {
So(err, ShouldNotBeNil) panelWithoutSpecifiedDatasource, err := ioutil.ReadFile("./testdata/panel-without-specified-datasource.json")
}) require.Nil(t, err)
})
Convey("Panel does not have datasource configured, use the default datasource", func() { dashJSON, err := simplejson.NewJson(panelWithoutSpecifiedDatasource)
panelWithoutSpecifiedDatasource, err := ioutil.ReadFile("./testdata/panel-without-specified-datasource.json") require.Nil(t, err)
So(err, ShouldBeNil) dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
dashJSON, err := simplejson.NewJson(panelWithoutSpecifiedDatasource) alerts, err := extractor.GetAlerts()
So(err, ShouldBeNil) require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
alerts, err := extractor.GetAlerts() condition := simplejson.NewFromAny(alerts[0].Settings.Get("conditions").MustArray()[0])
query := condition.Get("query")
require.EqualValues(t, query.Get("datasourceId").MustInt64(), 12)
})
Convey("Get rules without error", func() { t.Run("Parse alerts from dashboard without rows", func(t *testing.T) {
So(err, ShouldBeNil) json, err := ioutil.ReadFile("./testdata/v5-dashboard.json")
}) require.Nil(t, err)
Convey("Use default datasource", func() { dashJSON, err := simplejson.NewJson(json)
condition := simplejson.NewFromAny(alerts[0].Settings.Get("conditions").MustArray()[0]) require.Nil(t, err)
query := condition.Get("query") dash := models.NewDashboardFromJson(dashJSON)
So(query.Get("datasourceId").MustInt64(), ShouldEqual, 12) extractor := NewDashAlertExtractor(dash, 1, nil)
})
})
Convey("Parse alerts from dashboard without rows", func() { alerts, err := extractor.GetAlerts()
json, err := ioutil.ReadFile("./testdata/v5-dashboard.json") require.Nil(t, err)
So(err, ShouldBeNil)
dashJSON, err := simplejson.NewJson(json) require.Len(t, alerts, 2)
So(err, ShouldBeNil) })
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
alerts, err := extractor.GetAlerts() t.Run("Alert notifications are in DB", func(t *testing.T) {
sqlstore.InitTestDB(t)
firstNotification := models.CreateAlertNotificationCommand{Uid: "notifier1", OrgId: 1, Name: "1"}
err = sqlstore.CreateAlertNotificationCommand(&firstNotification)
require.Nil(t, err)
secondNotification := models.CreateAlertNotificationCommand{Uid: "notifier2", OrgId: 1, Name: "2"}
err = sqlstore.CreateAlertNotificationCommand(&secondNotification)
require.Nil(t, err)
Convey("Get rules without error", func() { json, err := ioutil.ReadFile("./testdata/influxdb-alert.json")
So(err, ShouldBeNil) require.Nil(t, err)
})
Convey("Should have 2 alert rule", func() { dashJSON, err := simplejson.NewJson(json)
So(len(alerts), ShouldEqual, 2) require.Nil(t, err)
}) dash := models.NewDashboardFromJson(dashJSON)
}) extractor := NewDashAlertExtractor(dash, 1, nil)
Convey("Alert notifications are in DB", func() { alerts, err := extractor.GetAlerts()
sqlstore.InitTestDB(t) require.Nil(t, err)
firstNotification := models.CreateAlertNotificationCommand{Uid: "notifier1", OrgId: 1, Name: "1"}
err = sqlstore.CreateAlertNotificationCommand(&firstNotification)
So(err, ShouldBeNil)
secondNotification := models.CreateAlertNotificationCommand{Uid: "notifier2", OrgId: 1, Name: "2"}
err = sqlstore.CreateAlertNotificationCommand(&secondNotification)
So(err, ShouldBeNil)
Convey("Parse and validate dashboard containing influxdb alert", func() { require.Len(t, alerts, 1)
json, err := ioutil.ReadFile("./testdata/influxdb-alert.json")
So(err, ShouldBeNil)
dashJSON, err := simplejson.NewJson(json) for _, alert := range alerts {
So(err, ShouldBeNil) require.EqualValues(t, alert.DashboardId, 4)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
alerts, err := extractor.GetAlerts() conditions := alert.Settings.Get("conditions").MustArray()
cond := simplejson.NewFromAny(conditions[0])
Convey("Get rules without error", func() { require.Equal(t, cond.Get("query").Get("model").Get("interval").MustString(), ">10s")
So(err, ShouldBeNil) }
}) })
Convey("should be able to read interval", func() { t.Run("Should be able to extract collapsed panels", func(t *testing.T) {
So(len(alerts), ShouldEqual, 1) json, err := ioutil.ReadFile("./testdata/collapsed-panels.json")
require.Nil(t, err)
for _, alert := range alerts { dashJSON, err := simplejson.NewJson(json)
So(alert.DashboardId, ShouldEqual, 4) require.Nil(t, err)
conditions := alert.Settings.Get("conditions").MustArray() dash := models.NewDashboardFromJson(dashJSON)
cond := simplejson.NewFromAny(conditions[0]) extractor := NewDashAlertExtractor(dash, 1, nil)
So(cond.Get("query").Get("model").Get("interval").MustString(), ShouldEqual, ">10s") alerts, err := extractor.GetAlerts()
} require.Nil(t, err)
})
})
Convey("Should be able to extract collapsed panels", func() { require.Len(t, alerts, 4)
json, err := ioutil.ReadFile("./testdata/collapsed-panels.json") })
So(err, ShouldBeNil)
dashJSON, err := simplejson.NewJson(json) t.Run("Parse and validate dashboard without id and containing an alert", func(t *testing.T) {
So(err, ShouldBeNil) json, err := ioutil.ReadFile("./testdata/dash-without-id.json")
require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON) dashJSON, err := simplejson.NewJson(json)
extractor := NewDashAlertExtractor(dash, 1, nil) require.Nil(t, err)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
alerts, err := extractor.GetAlerts() err = extractor.ValidateAlerts()
Convey("Get rules without error", func() { require.Nil(t, err)
So(err, ShouldBeNil)
})
Convey("should be able to extract collapsed alerts", func() { _, err = extractor.GetAlerts()
So(len(alerts), ShouldEqual, 4) require.Equal(t, err.Error(), "alert validation error: Panel id is not correct, alertName=Influxdb, panelId=1")
})
})
Convey("Parse and validate dashboard without id and containing an alert", func() {
json, err := ioutil.ReadFile("./testdata/dash-without-id.json")
So(err, ShouldBeNil)
dashJSON, err := simplejson.NewJson(json)
So(err, ShouldBeNil)
dash := models.NewDashboardFromJson(dashJSON)
extractor := NewDashAlertExtractor(dash, 1, nil)
err = extractor.ValidateAlerts()
Convey("Should validate without error", func() {
So(err, ShouldBeNil)
})
Convey("Should fail on save", func() {
_, err := extractor.GetAlerts()
So(err.Error(), ShouldEqual, "alert validation error: Panel id is not correct, alertName=Influxdb, panelId=1")
})
})
})
}) })
} }

69
pkg/services/alerting/rule.go
View File

@ -3,6 +3,7 @@ package alerting
import ( import (
"errors" "errors"
"fmt" "fmt"
"reflect"
"regexp" "regexp"
"strconv" "strconv"
"time" "time"
@ -12,12 +13,28 @@ import (
"github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/models"
) )
var unitMultiplier = map[string]int{
"s": 1,
"m": 60,
"h": 3600,
"d": 86400,
}
var (
valueFormatRegex = regexp.MustCompile(`^\d+`)
isDigitRegex = regexp.MustCompile(`^[0-9]+$`)
unitFormatRegex = regexp.MustCompile(`[a-z]+`)
)
var ( var (
// ErrFrequencyCannotBeZeroOrLess frequency cannot be below zero // ErrFrequencyCannotBeZeroOrLess frequency cannot be below zero
ErrFrequencyCannotBeZeroOrLess = errors.New(`"evaluate every" cannot be zero or below`) ErrFrequencyCannotBeZeroOrLess = errors.New(`"evaluate every" cannot be zero or below`)
// ErrFrequencyCouldNotBeParsed frequency cannot be parsed // ErrFrequencyCouldNotBeParsed frequency cannot be parsed
ErrFrequencyCouldNotBeParsed = errors.New(`"evaluate every" field could not be parsed`) ErrFrequencyCouldNotBeParsed = errors.New(`"evaluate every" field could not be parsed`)
// ErrWrongUnitFormat wrong unit format
ErrWrongUnitFormat = fmt.Errorf(`time unit not supported. supported units: %s`, reflect.ValueOf(unitMultiplier).MapKeys())
) )
// Rule is the in-memory version of an alert rule. // Rule is the in-memory version of an alert rule.
@ -72,20 +89,18 @@ func (e ValidationError) Error() string {
return fmt.Sprintf("alert validation error: %s", extraInfo) return fmt.Sprintf("alert validation error: %s", extraInfo)
} }
var (
valueFormatRegex = regexp.MustCompile(`^\d+`)
unitFormatRegex = regexp.MustCompile(`\w{1}$`)
)
var unitMultiplier = map[string]int{
"s": 1,
"m": 60,
"h": 3600,
"d": 86400,
}
func getTimeDurationStringToSeconds(str string) (int64, error) { func getTimeDurationStringToSeconds(str string) (int64, error) {
multiplier := 1 // Check if frequency lacks unit
if isDigitRegex.MatchString(str) || str == "" {
return 0, ErrFrequencyCouldNotBeParsed
}
unit := unitFormatRegex.FindAllString(str, 1)[0]
if _, ok := unitMultiplier[unit]; !ok {
return 0, ErrWrongUnitFormat
}
multiplier := unitMultiplier[unit]
matches := valueFormatRegex.FindAllString(str, 1) matches := valueFormatRegex.FindAllString(str, 1)
@ -102,15 +117,31 @@ func getTimeDurationStringToSeconds(str string) (int64, error) {
return 0, ErrFrequencyCannotBeZeroOrLess return 0, ErrFrequencyCannotBeZeroOrLess
} }
unit := unitFormatRegex.FindAllString(str, 1)[0]
if val, ok := unitMultiplier[unit]; ok {
multiplier = val
}
return int64(value * multiplier), nil return int64(value * multiplier), nil
} }
func getForValue(rawFor string) (time.Duration, error) {
var forValue time.Duration
var err error
if rawFor != "" {
if rawFor != "0" {
strings := unitFormatRegex.FindAllString(rawFor, 1)
if strings == nil {
return 0, ValidationError{Reason: fmt.Sprintf("no specified unit, error: %s", ErrWrongUnitFormat.Error())}
}
if _, ok := unitMultiplier[strings[0]]; !ok {
return 0, ValidationError{Reason: fmt.Sprintf("could not parse for field, error: %s", ErrWrongUnitFormat.Error())}
}
}
forValue, err = time.ParseDuration(rawFor)
if err != nil {
return 0, ValidationError{Reason: "Could not parse for field"}
}
}
return forValue, nil
}
// NewRuleFromDBAlert maps a db version of // NewRuleFromDBAlert maps a db version of
// alert to an in-memory version. // alert to an in-memory version.
func NewRuleFromDBAlert(ruleDef *models.Alert, logTranslationFailures bool) (*Rule, error) { func NewRuleFromDBAlert(ruleDef *models.Alert, logTranslationFailures bool) (*Rule, error) {

200
pkg/services/alerting/rule_test.go
View File

@ -1,13 +1,14 @@
package alerting package alerting
import ( import (
"fmt"
"testing" "testing"
"time"
"github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore"
. "github.com/smartystreets/goconvey/convey"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
@ -28,11 +29,12 @@ func TestAlertRuleFrequencyParsing(t *testing.T) {
{input: "10m", result: 600}, {input: "10m", result: 600},
{input: "1h", result: 3600}, {input: "1h", result: 3600},
{input: "1d", result: 86400}, {input: "1d", result: 86400},
{input: "1o", result: 1}, {input: "1o", err: ErrWrongUnitFormat},
{input: "0s", err: ErrFrequencyCannotBeZeroOrLess}, {input: "0s", err: ErrFrequencyCannotBeZeroOrLess},
{input: "0m", err: ErrFrequencyCannotBeZeroOrLess}, {input: "0m", err: ErrFrequencyCannotBeZeroOrLess},
{input: "0h", err: ErrFrequencyCannotBeZeroOrLess}, {input: "0h", err: ErrFrequencyCannotBeZeroOrLess},
{input: "0", err: ErrFrequencyCannotBeZeroOrLess}, {input: "0", err: ErrFrequencyCouldNotBeParsed},
{input: "", err: ErrFrequencyCouldNotBeParsed},
{input: "-1s", err: ErrFrequencyCouldNotBeParsed}, {input: "-1s", err: ErrFrequencyCouldNotBeParsed},
} }
@ -49,28 +51,52 @@ func TestAlertRuleFrequencyParsing(t *testing.T) {
} }
} }
func TestAlertRuleForParsing(t *testing.T) {
tcs := []struct {
input string
err error
result time.Duration
}{
{input: "10s", result: time.Duration(10000000000)},
{input: "10m", result: time.Duration(600000000000)},
{input: "1h", result: time.Duration(3600000000000)},
{input: "1o", err: fmt.Errorf("alert validation error: could not parse for field, error: %s", ErrWrongUnitFormat)},
{input: "1", err: fmt.Errorf("alert validation error: no specified unit, error: %s", ErrWrongUnitFormat)},
{input: "0s", result: time.Duration(0)},
{input: "0m", result: time.Duration(0)},
{input: "0h", result: time.Duration(0)},
{input: "0", result: time.Duration(0)},
{input: "", result: time.Duration(0)},
}
for _, tc := range tcs {
t.Run(tc.input, func(t *testing.T) {
r, err := getForValue(tc.input)
if tc.err == nil {
require.NoError(t, err)
} else {
require.EqualError(t, err, tc.err.Error())
}
assert.Equal(t, tc.result, r)
})
}
}
func TestAlertRuleModel(t *testing.T) { func TestAlertRuleModel(t *testing.T) {
sqlstore.InitTestDB(t) sqlstore.InitTestDB(t)
Convey("Testing alert rule", t, func() { RegisterCondition("test", func(model *simplejson.Json, index int) (Condition, error) {
RegisterCondition("test", func(model *simplejson.Json, index int) (Condition, error) { return &FakeCondition{}, nil
return &FakeCondition{}, nil })
})
Convey("should return err for empty string", func() { firstNotification := models.CreateAlertNotificationCommand{Uid: "notifier1", OrgId: 1, Name: "1"}
_, err := getTimeDurationStringToSeconds("") err := sqlstore.CreateAlertNotificationCommand(&firstNotification)
So(err, ShouldNotBeNil) require.Nil(t, err)
}) secondNotification := models.CreateAlertNotificationCommand{Uid: "notifier2", OrgId: 1, Name: "2"}
err = sqlstore.CreateAlertNotificationCommand(&secondNotification)
require.Nil(t, err)
Convey("can construct alert rule model", func() { t.Run("Testing alert rule with notification id and uid", func(t *testing.T) {
firstNotification := models.CreateAlertNotificationCommand{Uid: "notifier1", OrgId: 1, Name: "1"} json := `
err := sqlstore.CreateAlertNotificationCommand(&firstNotification)
So(err, ShouldBeNil)
secondNotification := models.CreateAlertNotificationCommand{Uid: "notifier2", OrgId: 1, Name: "2"}
err = sqlstore.CreateAlertNotificationCommand(&secondNotification)
So(err, ShouldBeNil)
Convey("with notification id and uid", func() {
json := `
{ {
"name": "name2", "name": "name2",
"description": "desc2", "description": "desc2",
@ -91,33 +117,30 @@ func TestAlertRuleModel(t *testing.T) {
} }
` `
alertJSON, jsonErr := simplejson.NewJson([]byte(json)) alertJSON, jsonErr := simplejson.NewJson([]byte(json))
So(jsonErr, ShouldBeNil) require.Nil(t, jsonErr)
alert := &models.Alert{ alert := &models.Alert{
Id: 1, Id: 1,
OrgId: 1, OrgId: 1,
DashboardId: 1, DashboardId: 1,
PanelId: 1, PanelId: 1,
Settings: alertJSON, Settings: alertJSON,
} }
alertRule, err := NewRuleFromDBAlert(alert, false) alertRule, err := NewRuleFromDBAlert(alert, false)
So(err, ShouldBeNil) require.Nil(t, err)
So(len(alertRule.Conditions), ShouldEqual, 1) require.Len(t, alertRule.Conditions, 1)
So(len(alertRule.Notifications), ShouldEqual, 2) require.Len(t, alertRule.Notifications, 2)
Convey("Can read Id and Uid notifications (translate Id to Uid)", func() { require.Contains(t, alertRule.Notifications, "notifier2")
So(alertRule.Notifications, ShouldContain, "notifier2") require.Contains(t, alertRule.Notifications, "notifier1")
So(alertRule.Notifications, ShouldContain, "notifier1") })
})
})
})
Convey("with non existing notification id", func() { t.Run("Testing alert rule with non existing notification id", func(t *testing.T) {
json := ` json := `
{ {
"name": "name3", "name": "name3",
"description": "desc3", "description": "desc3",
@ -133,28 +156,26 @@ func TestAlertRuleModel(t *testing.T) {
} }
` `
alertJSON, jsonErr := simplejson.NewJson([]byte(json)) alertJSON, jsonErr := simplejson.NewJson([]byte(json))
So(jsonErr, ShouldBeNil) require.Nil(t, jsonErr)
alert := &models.Alert{ alert := &models.Alert{
Id: 1, Id: 1,
OrgId: 1, OrgId: 1,
DashboardId: 1, DashboardId: 1,
PanelId: 1, PanelId: 1,
Settings: alertJSON, Settings: alertJSON,
} }
alertRule, err := NewRuleFromDBAlert(alert, false) alertRule, err := NewRuleFromDBAlert(alert, false)
Convey("swallows the error", func() { require.Nil(t, err)
So(err, ShouldBeNil) require.NotContains(t, alertRule.Notifications, "999")
So(alertRule.Notifications, ShouldNotContain, "999") require.Contains(t, alertRule.Notifications, "notifier2")
So(alertRule.Notifications, ShouldContain, "notifier2") })
})
})
Convey("can construct alert rule model with invalid frequency", func() { t.Run("Testing alert rule which can construct alert rule model with invalid frequency", func(t *testing.T) {
json := ` json := `
{ {
"name": "name2", "name": "name2",
"description": "desc2", "description": "desc2",
@ -165,26 +186,26 @@ func TestAlertRuleModel(t *testing.T) {
"notifications": [] "notifications": []
}` }`
alertJSON, jsonErr := simplejson.NewJson([]byte(json)) alertJSON, jsonErr := simplejson.NewJson([]byte(json))
So(jsonErr, ShouldBeNil) require.Nil(t, jsonErr)
alert := &models.Alert{ alert := &models.Alert{
Id: 1, Id: 1,
OrgId: 1, OrgId: 1,
DashboardId: 1, DashboardId: 1,
PanelId: 1, PanelId: 1,
Frequency: 0, Frequency: 0,
Settings: alertJSON, Settings: alertJSON,
} }
alertRule, err := NewRuleFromDBAlert(alert, false) alertRule, err := NewRuleFromDBAlert(alert, false)
So(err, ShouldBeNil) require.Nil(t, err)
So(alertRule.Frequency, ShouldEqual, 60) require.EqualValues(t, alertRule.Frequency, 60)
}) })
Convey("raise error in case of missing notification id and uid", func() { t.Run("Testing alert rule which will raise error in case of missing notification id and uid", func(t *testing.T) {
json := ` json := `
{ {
"name": "name2", "name": "name2",
"description": "desc2", "description": "desc2",
@ -203,22 +224,21 @@ func TestAlertRuleModel(t *testing.T) {
} }
` `
alertJSON, jsonErr := simplejson.NewJson([]byte(json)) alertJSON, jsonErr := simplejson.NewJson([]byte(json))
So(jsonErr, ShouldBeNil) require.Nil(t, jsonErr)
alert := &models.Alert{ alert := &models.Alert{
Id: 1, Id: 1,
OrgId: 1, OrgId: 1,
DashboardId: 1, DashboardId: 1,
PanelId: 1, PanelId: 1,
Frequency: 0, Frequency: 0,
Settings: alertJSON, Settings: alertJSON,
} }
_, err := NewRuleFromDBAlert(alert, false) _, err := NewRuleFromDBAlert(alert, false)
So(err, ShouldNotBeNil) require.NotNil(t, err)
So(err.Error(), ShouldEqual, "alert validation error: Neither id nor uid is specified in 'notifications' block, type assertion to string failed AlertId: 1 PanelId: 1 DashboardId: 1") require.EqualValues(t, err.Error(), "alert validation error: Neither id nor uid is specified in 'notifications' block, type assertion to string failed AlertId: 1 PanelId: 1 DashboardId: 1")
})
}) })
} }

8
public/app/features/alerting/AlertTabCtrl.ts
View File

@ -264,12 +264,14 @@ export class AlertTabCtrl {
} }
checkFrequency() { checkFrequency() {
if (!this.alert.frequency) { this.frequencyWarning = '';
if (!(this.alert.frequency || '').match(/^\d+([dhms])$/)) {
this.frequencyWarning =
'Invalid frequency, has to be numeric followed by one of the following units: "d, h, m, s"';
return; return;
} }
this.frequencyWarning = '';
try { try {
const frequencySecs = rangeUtil.intervalToSeconds(this.alert.frequency); const frequencySecs = rangeUtil.intervalToSeconds(this.alert.frequency);
if (frequencySecs < this.alertingMinIntervalSecs) { if (frequencySecs < this.alertingMinIntervalSecs) {

1
public/app/features/alerting/partials/alert_tab.html
View File

@ -30,6 +30,7 @@
ng-model="ctrl.alert.for" ng-model="ctrl.alert.for"
spellcheck="false" spellcheck="false"
placeholder="5m" placeholder="5m"
ng-pattern="/(^\d+([dhms])$)|(0)|(^$)/"
/> />
<info-popover mode="right-absolute"> <info-popover mode="right-absolute">
If an alert rule has a configured and the query violates the configured threshold, then it goes from OK If an alert rule has a configured and the query violates the configured threshold, then it goes from OK