From 01f1c550bf01c9ea0991af6d72dd06969b10272b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Farkas?= Date: Mon, 6 Sep 2021 09:24:32 +0200 Subject: [PATCH] old-alert: interval calculation: apply default-min-interval of 1ms (#38835) --- pkg/services/alerting/conditions/query.go | 7 ++- .../conditions/query_interval_test.go | 45 ++++++++++++++----- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/pkg/services/alerting/conditions/query.go b/pkg/services/alerting/conditions/query.go index e7efbae16fb..a51b4874a91 100644 --- a/pkg/services/alerting/conditions/query.go +++ b/pkg/services/alerting/conditions/query.go @@ -110,12 +110,17 @@ func (c *QueryCondition) Eval(context *alerting.EvalContext, requestHandler plug } func calculateInterval(timeRange plugins.DataTimeRange, model *simplejson.Json, dsInfo *models.DataSource) (time.Duration, error) { + // if there is no min-interval specified in the datasource or in the dashboard-panel, + // the value of 1ms is used (this is how it is done in the dashboard-interval-calculation too, + // see https://github.com/grafana/grafana/blob/9a0040c0aeaae8357c650cec2ee644a571dddf3d/packages/grafana-data/src/datetime/rangeutil.ts#L264) + defaultMinInterval := time.Millisecond * 1 + // interval.GetIntervalFrom has two problems (but they do not affect us here): // - it returns the min-interval, so it should be called interval.GetMinIntervalFrom // - it falls back to model.intervalMs. it should not, because that one is the real final // interval-value calculated by the browser. but, in this specific case (old-alert), // that value is not set, so the fallback never happens. - minInterval, err := interval.GetIntervalFrom(dsInfo, model, time.Duration(0)) + minInterval, err := interval.GetIntervalFrom(dsInfo, model, defaultMinInterval) if err != nil { return time.Duration(0), err diff --git a/pkg/services/alerting/conditions/query_interval_test.go b/pkg/services/alerting/conditions/query_interval_test.go index 72b9d3e76dd..7736da437e3 100644 --- a/pkg/services/alerting/conditions/query_interval_test.go +++ b/pkg/services/alerting/conditions/query_interval_test.go @@ -15,12 +15,6 @@ import ( . "github.com/smartystreets/goconvey/convey" ) -// the time-range is 5m (300seconds) for every test-case, -// maxDataPoints is 1500 for every test-case, -// so the interval for this simple case should be 300s/1500 = 200ms, -// but in some cases this is overridden by dashboard-panel-min-interval -// or by datasource-min-interval - func TestQueryInterval(t *testing.T) { Convey("When evaluating query condition, regarding the interval value", t, func() { Convey("Can handle interval-calculation with no panel-min-interval and no datasource-min-interval", func() { @@ -30,6 +24,8 @@ func TestQueryInterval(t *testing.T) { // no datasource-min-interval var dataSourceJson *simplejson.Json = nil + timeRange := "5m" + verifier := func(query plugins.DataSubQuery) { // 5minutes timerange = 300000milliseconds; default-resolution is 1500pixels, // so we should have 300000/1500 = 200milliseconds here @@ -37,7 +33,7 @@ func TestQueryInterval(t *testing.T) { So(query.MaxDataPoints, ShouldEqual, interval.DefaultRes) } - applyScenario(dataSourceJson, queryModel, verifier) + applyScenario(timeRange, dataSourceJson, queryModel, verifier) }) Convey("Can handle interval-calculation with panel-min-interval and no datasource-min-interval", func() { // panel-min-interval in the queryModel @@ -46,12 +42,14 @@ func TestQueryInterval(t *testing.T) { // no datasource-min-interval var dataSourceJson *simplejson.Json = nil + timeRange := "5m" + verifier := func(query plugins.DataSubQuery) { So(query.IntervalMS, ShouldEqual, 123000) So(query.MaxDataPoints, ShouldEqual, interval.DefaultRes) } - applyScenario(dataSourceJson, queryModel, verifier) + applyScenario(timeRange, dataSourceJson, queryModel, verifier) }) Convey("Can handle interval-calculation with no panel-min-interval and datasource-min-interval", func() { // no panel-min-interval in the queryModel @@ -63,12 +61,14 @@ func TestQueryInterval(t *testing.T) { }`)) So(err, ShouldBeNil) + timeRange := "5m" + verifier := func(query plugins.DataSubQuery) { So(query.IntervalMS, ShouldEqual, 71000) So(query.MaxDataPoints, ShouldEqual, interval.DefaultRes) } - applyScenario(dataSourceJson, queryModel, verifier) + applyScenario(timeRange, dataSourceJson, queryModel, verifier) }) Convey("Can handle interval-calculation with both panel-min-interval and datasource-min-interval", func() { // panel-min-interval in the queryModel @@ -80,6 +80,8 @@ func TestQueryInterval(t *testing.T) { }`)) So(err, ShouldBeNil) + timeRange := "5m" + verifier := func(query plugins.DataSubQuery) { // when both panel-min-interval and datasource-min-interval exists, // panel-min-interval is used @@ -87,7 +89,26 @@ func TestQueryInterval(t *testing.T) { So(query.MaxDataPoints, ShouldEqual, interval.DefaultRes) } - applyScenario(dataSourceJson, queryModel, verifier) + applyScenario(timeRange, dataSourceJson, queryModel, verifier) + }) + + Convey("Can handle no min-interval, and very small time-ranges, where the default-min-interval=1ms applies", func() { + // no panel-min-interval in the queryModel + queryModel := `{"target": "aliasByNode(statsd.fakesite.counters.session_start.mobile.count, 4)"}` + + // no datasource-min-interval + var dataSourceJson *simplejson.Json = nil + + timeRange := "1s" + + verifier := func(query plugins.DataSubQuery) { + // no min-interval exists, the default-min-interval will be used, + // and for such a short time-range this will cause the value to be 1millisecond. + So(query.IntervalMS, ShouldEqual, 1) + So(query.MaxDataPoints, ShouldEqual, interval.DefaultRes) + } + + applyScenario(timeRange, dataSourceJson, queryModel, verifier) }) }) } @@ -114,7 +135,7 @@ func (rh fakeIntervalTestReqHandler) HandleRequest(ctx context.Context, dsInfo * } //nolint: staticcheck // plugins.DataResponse deprecated -func applyScenario(dataSourceJsonData *simplejson.Json, queryModel string, verifier func(query plugins.DataSubQuery)) { +func applyScenario(timeRange string, dataSourceJsonData *simplejson.Json, queryModel string, verifier func(query plugins.DataSubQuery)) { Convey("desc", func() { bus.AddHandler("test", func(query *models.GetDataSourceQuery) error { query.Result = &models.DataSource{Id: 1, Type: "graphite", JsonData: dataSourceJsonData} @@ -130,7 +151,7 @@ func applyScenario(dataSourceJsonData *simplejson.Json, queryModel string, verif jsonModel, err := simplejson.NewJson([]byte(`{ "type": "query", "query": { - "params": ["A", "5m", "now"], + "params": ["A", "` + timeRange + `", "now"], "datasourceId": 1, "model": ` + queryModel + ` },