From 5d6ed6c45fcb01005386bcfd21f8dbb3e1ce2f8c Mon Sep 17 00:00:00 2001 From: Carl Bergquist Date: Wed, 15 Nov 2017 11:22:00 +0100 Subject: [PATCH] prom: add support for default step param (#9866) Alerting for prometheus have been depending on the step parameter from each query. In https://github.com/grafana/grafana/pull/9226 we changed the behavior for step in the frontend which caused problems for alerting. This commit fixes that by introducing a default min interval value so alerting always have something to depend on. closes #9777 --- .../features/datasources/prometheus.md | 1 + pkg/tsdb/influxdb/model_parser.go | 12 +- pkg/tsdb/influxdb/model_parser_test.go | 5 +- pkg/tsdb/influxdb/models.go | 5 +- pkg/tsdb/influxdb/query.go | 29 +---- pkg/tsdb/influxdb/query_test.go | 11 +- pkg/tsdb/interval.go | 76 ++++++++++-- pkg/tsdb/interval_test.go | 11 +- pkg/tsdb/prometheus/prometheus.go | 26 ++-- pkg/tsdb/prometheus/prometheus_test.go | 111 ++++++++++++++++++ .../datasource/prometheus/datasource.ts | 2 + .../prometheus/partials/config.html | 13 ++ .../prometheus/specs/datasource_specs.ts | 2 +- .../specs/metric_find_query_specs.ts | 2 +- 14 files changed, 237 insertions(+), 69 deletions(-) diff --git a/docs/sources/features/datasources/prometheus.md b/docs/sources/features/datasources/prometheus.md index 3748a15195f..15247ba5ebd 100644 --- a/docs/sources/features/datasources/prometheus.md +++ b/docs/sources/features/datasources/prometheus.md @@ -34,6 +34,7 @@ Name | Description *Basic Auth* | Enable basic authentication to the Prometheus data source. *User* | Name of your Prometheus user *Password* | Database user's password +*Scrape interval* | This will be used as a lower limit for the Prometheus step query parameter. Default value is 15s. ## Query editor diff --git a/pkg/tsdb/influxdb/model_parser.go b/pkg/tsdb/influxdb/model_parser.go index f3d87739e5b..deb2f15e3ce 100644 --- a/pkg/tsdb/influxdb/model_parser.go +++ b/pkg/tsdb/influxdb/model_parser.go @@ -2,9 +2,11 @@ package influxdb import ( "strconv" + "time" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/tsdb" ) type InfluxdbQueryParser struct{} @@ -37,13 +39,7 @@ func (qp *InfluxdbQueryParser) Parse(model *simplejson.Json, dsInfo *models.Data return nil, err } - interval := model.Get("interval").MustString("") - if interval == "" && dsInfo.JsonData != nil { - dsInterval := dsInfo.JsonData.Get("timeInterval").MustString("") - if dsInterval != "" { - interval = dsInterval - } - } + parsedInterval, err := tsdb.GetIntervalFrom(dsInfo, model, time.Millisecond*1) return &Query{ Measurement: measurement, @@ -53,7 +49,7 @@ func (qp *InfluxdbQueryParser) Parse(model *simplejson.Json, dsInfo *models.Data Tags: tags, Selects: selects, RawQuery: rawQuery, - Interval: interval, + Interval: parsedInterval, Alias: alias, UseRawQuery: useRawQuery, }, nil diff --git a/pkg/tsdb/influxdb/model_parser_test.go b/pkg/tsdb/influxdb/model_parser_test.go index f7049efb9a7..f8759afd3ba 100644 --- a/pkg/tsdb/influxdb/model_parser_test.go +++ b/pkg/tsdb/influxdb/model_parser_test.go @@ -2,6 +2,7 @@ package influxdb import ( "testing" + "time" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" @@ -115,7 +116,7 @@ func TestInfluxdbQueryParser(t *testing.T) { So(len(res.GroupBy), ShouldEqual, 3) So(len(res.Selects), ShouldEqual, 3) So(len(res.Tags), ShouldEqual, 2) - So(res.Interval, ShouldEqual, ">20s") + So(res.Interval, ShouldEqual, time.Second*20) So(res.Alias, ShouldEqual, "serie alias") }) @@ -174,7 +175,7 @@ func TestInfluxdbQueryParser(t *testing.T) { So(len(res.GroupBy), ShouldEqual, 2) So(len(res.Selects), ShouldEqual, 1) So(len(res.Tags), ShouldEqual, 0) - So(res.Interval, ShouldEqual, ">10s") + So(res.Interval, ShouldEqual, time.Second*10) }) }) } diff --git a/pkg/tsdb/influxdb/models.go b/pkg/tsdb/influxdb/models.go index 44e05608290..82ed72c2a18 100644 --- a/pkg/tsdb/influxdb/models.go +++ b/pkg/tsdb/influxdb/models.go @@ -1,5 +1,7 @@ package influxdb +import "time" + type Query struct { Measurement string Policy string @@ -10,8 +12,7 @@ type Query struct { RawQuery string UseRawQuery bool Alias string - - Interval string + Interval time.Duration } type Tag struct { diff --git a/pkg/tsdb/influxdb/query.go b/pkg/tsdb/influxdb/query.go index 3a796e6db63..499f446e9f0 100644 --- a/pkg/tsdb/influxdb/query.go +++ b/pkg/tsdb/influxdb/query.go @@ -29,10 +29,8 @@ func (query *Query) Build(queryContext *tsdb.TsdbQuery) (string, error) { res += query.renderGroupBy(queryContext) } - interval, err := getDefinedInterval(query, queryContext) - if err != nil { - return "", err - } + calculator := tsdb.NewIntervalCalculator(&tsdb.IntervalOptions{}) + interval := calculator.Calculate(queryContext.TimeRange, query.Interval) res = strings.Replace(res, "$timeFilter", query.renderTimeFilter(queryContext), -1) res = strings.Replace(res, "$interval", interval.Text, -1) @@ -41,29 +39,6 @@ func (query *Query) Build(queryContext *tsdb.TsdbQuery) (string, error) { return res, nil } -func getDefinedInterval(query *Query, queryContext *tsdb.TsdbQuery) (*tsdb.Interval, error) { - defaultInterval := tsdb.CalculateInterval(queryContext.TimeRange) - - if query.Interval == "" { - return &defaultInterval, nil - } - - setInterval := strings.Replace(strings.Replace(query.Interval, "<", "", 1), ">", "", 1) - parsedSetInterval, err := time.ParseDuration(setInterval) - - if err != nil { - return nil, err - } - - if strings.Contains(query.Interval, ">") { - if defaultInterval.Value > parsedSetInterval { - return &defaultInterval, nil - } - } - - return &tsdb.Interval{Value: parsedSetInterval, Text: setInterval}, nil -} - func (query *Query) renderTags() []string { var res []string for i, tag := range query.Tags { diff --git a/pkg/tsdb/influxdb/query_test.go b/pkg/tsdb/influxdb/query_test.go index 5c8dc1eaf69..4a620539a26 100644 --- a/pkg/tsdb/influxdb/query_test.go +++ b/pkg/tsdb/influxdb/query_test.go @@ -2,6 +2,7 @@ package influxdb import ( "testing" + "time" "strings" @@ -38,7 +39,7 @@ func TestInfluxdbQueryBuilder(t *testing.T) { Measurement: "cpu", Policy: "policy", GroupBy: []*QueryPart{groupBy1, groupBy3}, - Interval: "10s", + Interval: time.Second * 10, } rawQuery, err := query.Build(queryContext) @@ -52,7 +53,7 @@ func TestInfluxdbQueryBuilder(t *testing.T) { Measurement: "cpu", GroupBy: []*QueryPart{groupBy1, groupBy2, groupBy3}, Tags: []*Tag{tag1, tag2}, - Interval: "5s", + Interval: time.Second * 5, } rawQuery, err := query.Build(queryContext) @@ -64,7 +65,7 @@ func TestInfluxdbQueryBuilder(t *testing.T) { query := &Query{ Selects: []*Select{{*qp1, *qp2, *mathPartDivideBy100}}, Measurement: "cpu", - Interval: "5s", + Interval: time.Second * 5, } rawQuery, err := query.Build(queryContext) @@ -76,7 +77,7 @@ func TestInfluxdbQueryBuilder(t *testing.T) { query := &Query{ Selects: []*Select{{*qp1, *qp2, *mathPartDivideByIntervalMs}}, Measurement: "cpu", - Interval: "5s", + Interval: time.Second * 5, } rawQuery, err := query.Build(queryContext) @@ -117,7 +118,7 @@ func TestInfluxdbQueryBuilder(t *testing.T) { Measurement: "cpu", Policy: "policy", GroupBy: []*QueryPart{groupBy1, groupBy3}, - Interval: "10s", + Interval: time.Second * 10, RawQuery: "Raw query", UseRawQuery: true, } diff --git a/pkg/tsdb/interval.go b/pkg/tsdb/interval.go index aef6cc4f47b..e26d39f3986 100644 --- a/pkg/tsdb/interval.go +++ b/pkg/tsdb/interval.go @@ -2,14 +2,18 @@ package tsdb import ( "fmt" + "strings" "time" + + "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/models" ) var ( - defaultRes int64 = 1500 - minInterval time.Duration = 1 * time.Millisecond - year time.Duration = time.Hour * 24 * 365 - day time.Duration = time.Hour * 24 * 365 + defaultRes int64 = 1500 + defaultMinInterval time.Duration = 1 * time.Millisecond + year time.Duration = time.Hour * 24 * 365 + day time.Duration = time.Hour * 24 ) type Interval struct { @@ -17,14 +21,68 @@ type Interval struct { Value time.Duration } -func CalculateInterval(timerange *TimeRange) Interval { - interval := time.Duration((timerange.MustGetTo().UnixNano() - timerange.MustGetFrom().UnixNano()) / defaultRes) +type intervalCalculator struct { + minInterval time.Duration +} - if interval < minInterval { - return Interval{Text: formatDuration(minInterval), Value: interval} +type IntervalCalculator interface { + Calculate(timeRange *TimeRange, minInterval time.Duration) Interval +} + +type IntervalOptions struct { + MinInterval time.Duration +} + +func NewIntervalCalculator(opt *IntervalOptions) *intervalCalculator { + if opt == nil { + opt = &IntervalOptions{} } - return Interval{Text: formatDuration(roundInterval(interval)), Value: interval} + calc := &intervalCalculator{} + + if opt.MinInterval == 0 { + calc.minInterval = defaultMinInterval + } else { + calc.minInterval = opt.MinInterval + } + + return calc +} + +func (ic *intervalCalculator) Calculate(timerange *TimeRange, minInterval time.Duration) Interval { + to := timerange.MustGetTo().UnixNano() + from := timerange.MustGetFrom().UnixNano() + interval := time.Duration((to - from) / defaultRes) + + if interval < minInterval { + return Interval{Text: formatDuration(minInterval), Value: minInterval} + } + + rounded := roundInterval(interval) + return Interval{Text: formatDuration(rounded), Value: rounded} +} + +func GetIntervalFrom(dsInfo *models.DataSource, queryModel *simplejson.Json, defaultInterval time.Duration) (time.Duration, error) { + interval := queryModel.Get("interval").MustString("") + + if interval == "" && dsInfo.JsonData != nil { + dsInterval := dsInfo.JsonData.Get("timeInterval").MustString("") + if dsInterval != "" { + interval = dsInterval + } + } + + if interval == "" { + return defaultInterval, nil + } + + interval = strings.Replace(strings.Replace(interval, "<", "", 1), ">", "", 1) + parsedInterval, err := time.ParseDuration(interval) + if err != nil { + return time.Duration(0), err + } + + return parsedInterval, nil } func formatDuration(inter time.Duration) string { diff --git a/pkg/tsdb/interval_test.go b/pkg/tsdb/interval_test.go index 7b243b4e3ba..1e36e5428fe 100644 --- a/pkg/tsdb/interval_test.go +++ b/pkg/tsdb/interval_test.go @@ -14,31 +14,33 @@ func TestInterval(t *testing.T) { HomePath: "../../", }) + calculator := NewIntervalCalculator(&IntervalOptions{}) + Convey("for 5min", func() { tr := NewTimeRange("5m", "now") - interval := CalculateInterval(tr) + interval := calculator.Calculate(tr, time.Millisecond*1) So(interval.Text, ShouldEqual, "200ms") }) Convey("for 15min", func() { tr := NewTimeRange("15m", "now") - interval := CalculateInterval(tr) + interval := calculator.Calculate(tr, time.Millisecond*1) So(interval.Text, ShouldEqual, "500ms") }) Convey("for 30min", func() { tr := NewTimeRange("30m", "now") - interval := CalculateInterval(tr) + interval := calculator.Calculate(tr, time.Millisecond*1) So(interval.Text, ShouldEqual, "1s") }) Convey("for 1h", func() { tr := NewTimeRange("1h", "now") - interval := CalculateInterval(tr) + interval := calculator.Calculate(tr, time.Millisecond*1) So(interval.Text, ShouldEqual, "2s") }) @@ -51,6 +53,7 @@ func TestInterval(t *testing.T) { So(formatDuration(time.Second*61), ShouldEqual, "1m") So(formatDuration(time.Millisecond*30), ShouldEqual, "30ms") So(formatDuration(time.Hour*23), ShouldEqual, "23h") + So(formatDuration(time.Hour*24), ShouldEqual, "1d") So(formatDuration(time.Hour*24*367), ShouldEqual, "1y") }) }) diff --git a/pkg/tsdb/prometheus/prometheus.go b/pkg/tsdb/prometheus/prometheus.go index 33219837281..e798b92c6fe 100644 --- a/pkg/tsdb/prometheus/prometheus.go +++ b/pkg/tsdb/prometheus/prometheus.go @@ -48,14 +48,16 @@ func NewPrometheusExecutor(dsInfo *models.DataSource) (tsdb.TsdbQueryEndpoint, e } var ( - plog log.Logger - legendFormat *regexp.Regexp + plog log.Logger + legendFormat *regexp.Regexp + intervalCalculator tsdb.IntervalCalculator ) func init() { plog = log.New("tsdb.prometheus") tsdb.RegisterTsdbQueryEndpoint("prometheus", NewPrometheusExecutor) legendFormat = regexp.MustCompile(`\{\{\s*(.+?)\s*\}\}`) + intervalCalculator = tsdb.NewIntervalCalculator(&tsdb.IntervalOptions{MinInterval: time.Second * 1}) } func (e *PrometheusExecutor) getClient(dsInfo *models.DataSource) (apiv1.API, error) { @@ -88,7 +90,7 @@ func (e *PrometheusExecutor) Query(ctx context.Context, dsInfo *models.DataSourc return nil, err } - query, err := parseQuery(tsdbQuery.Queries, tsdbQuery) + query, err := parseQuery(dsInfo, tsdbQuery.Queries, tsdbQuery) if err != nil { return nil, err } @@ -138,7 +140,7 @@ func formatLegend(metric model.Metric, query *PrometheusQuery) string { return string(result) } -func parseQuery(queries []*tsdb.Query, queryContext *tsdb.TsdbQuery) (*PrometheusQuery, error) { +func parseQuery(dsInfo *models.DataSource, queries []*tsdb.Query, queryContext *tsdb.TsdbQuery) (*PrometheusQuery, error) { queryModel := queries[0] expr, err := queryModel.Model.Get("expr").String() @@ -146,11 +148,6 @@ func parseQuery(queries []*tsdb.Query, queryContext *tsdb.TsdbQuery) (*Prometheu return nil, err } - step, err := queryModel.Model.Get("step").Int64() - if err != nil { - return nil, err - } - format := queryModel.Model.Get("legendFormat").MustString("") start, err := queryContext.TimeRange.ParseFrom() @@ -163,9 +160,18 @@ func parseQuery(queries []*tsdb.Query, queryContext *tsdb.TsdbQuery) (*Prometheu return nil, err } + dsInterval, err := tsdb.GetIntervalFrom(dsInfo, queryModel.Model, time.Second*15) + if err != nil { + return nil, err + } + + intervalFactor := queryModel.Model.Get("intervalFactor").MustInt64(1) + interval := intervalCalculator.Calculate(queryContext.TimeRange, dsInterval) + step := time.Duration(int64(interval.Value) * intervalFactor) + return &PrometheusQuery{ Expr: expr, - Step: time.Second * time.Duration(step), + Step: step, LegendFormat: format, Start: start, End: end, diff --git a/pkg/tsdb/prometheus/prometheus_test.go b/pkg/tsdb/prometheus/prometheus_test.go index d66ef75e479..c551ab98112 100644 --- a/pkg/tsdb/prometheus/prometheus_test.go +++ b/pkg/tsdb/prometheus/prometheus_test.go @@ -2,13 +2,21 @@ package prometheus import ( "testing" + "time" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/tsdb" + + "github.com/grafana/grafana/pkg/components/simplejson" p "github.com/prometheus/common/model" . "github.com/smartystreets/goconvey/convey" ) func TestPrometheus(t *testing.T) { Convey("Prometheus", t, func() { + dsInfo := &models.DataSource{ + JsonData: simplejson.New(), + } Convey("converting metric name", func() { metric := map[p.LabelName]p.LabelValue{ @@ -36,5 +44,108 @@ func TestPrometheus(t *testing.T) { So(formatLegend(metric, query), ShouldEqual, `http_request_total{app="backend", device="mobile"}`) }) + + Convey("parsing query model with step", func() { + json := `{ + "expr": "go_goroutines", + "format": "time_series", + "refId": "A" + }` + jsonModel, _ := simplejson.NewJson([]byte(json)) + queryContext := &tsdb.TsdbQuery{} + queryModels := []*tsdb.Query{ + {Model: jsonModel}, + } + + Convey("with 48h time range", func() { + queryContext.TimeRange = tsdb.NewTimeRange("12h", "now") + + model, err := parseQuery(dsInfo, queryModels, queryContext) + + So(err, ShouldBeNil) + So(model.Step, ShouldEqual, time.Second*30) + }) + }) + + Convey("parsing query model without step parameter", func() { + json := `{ + "expr": "go_goroutines", + "format": "time_series", + "intervalFactor": 1, + "refId": "A" + }` + jsonModel, _ := simplejson.NewJson([]byte(json)) + queryContext := &tsdb.TsdbQuery{} + queryModels := []*tsdb.Query{ + {Model: jsonModel}, + } + + Convey("with 48h time range", func() { + queryContext.TimeRange = tsdb.NewTimeRange("48h", "now") + + model, err := parseQuery(dsInfo, queryModels, queryContext) + + So(err, ShouldBeNil) + So(model.Step, ShouldEqual, time.Minute*2) + }) + + Convey("with 1h time range", func() { + queryContext.TimeRange = tsdb.NewTimeRange("1h", "now") + + model, err := parseQuery(dsInfo, queryModels, queryContext) + + So(err, ShouldBeNil) + So(model.Step, ShouldEqual, time.Second*15) + }) + }) + + Convey("parsing query model with intervalFactor", func() { + Convey("high intervalFactor", func() { + json := `{ + "expr": "go_goroutines", + "format": "time_series", + "intervalFactor": 10, + "refId": "A" + }` + jsonModel, _ := simplejson.NewJson([]byte(json)) + queryContext := &tsdb.TsdbQuery{} + queryModels := []*tsdb.Query{ + {Model: jsonModel}, + } + + Convey("with 48h time range", func() { + queryContext.TimeRange = tsdb.NewTimeRange("48h", "now") + + model, err := parseQuery(dsInfo, queryModels, queryContext) + + So(err, ShouldBeNil) + So(model.Step, ShouldEqual, time.Minute*20) + }) + }) + + Convey("low intervalFactor", func() { + json := `{ + "expr": "go_goroutines", + "format": "time_series", + "intervalFactor": 1, + "refId": "A" + }` + jsonModel, _ := simplejson.NewJson([]byte(json)) + queryContext := &tsdb.TsdbQuery{} + queryModels := []*tsdb.Query{ + {Model: jsonModel}, + } + + Convey("with 48h time range", func() { + queryContext.TimeRange = tsdb.NewTimeRange("48h", "now") + + model, err := parseQuery(dsInfo, queryModels, queryContext) + + So(err, ShouldBeNil) + So(model.Step, ShouldEqual, time.Minute*2) + }) + }) + }) + }) } diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index c4939b5b2fa..ec295760d49 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -19,6 +19,7 @@ export class PrometheusDatasource { basicAuth: any; withCredentials: any; metricsNameCache: any; + interval: string; /** @ngInject */ constructor(instanceSettings, @@ -34,6 +35,7 @@ export class PrometheusDatasource { this.directUrl = instanceSettings.directUrl; this.basicAuth = instanceSettings.basicAuth; this.withCredentials = instanceSettings.withCredentials; + this.interval = instanceSettings.jsonData.timeInterval || '15s'; } _request(method, url, requestId?) { diff --git a/public/app/plugins/datasource/prometheus/partials/config.html b/public/app/plugins/datasource/prometheus/partials/config.html index 1b817a2186d..3bb43253d4d 100644 --- a/public/app/plugins/datasource/prometheus/partials/config.html +++ b/public/app/plugins/datasource/prometheus/partials/config.html @@ -1,3 +1,16 @@ +
+
+
+ Scrape interval + + + Set this to your global scrape interval defined in your Prometheus config file. This will be used as a lower limit for + the Prometheus step query parameter. + +
+
+
+ diff --git a/public/app/plugins/datasource/prometheus/specs/datasource_specs.ts b/public/app/plugins/datasource/prometheus/specs/datasource_specs.ts index 9d528c735de..a7d18f3f158 100644 --- a/public/app/plugins/datasource/prometheus/specs/datasource_specs.ts +++ b/public/app/plugins/datasource/prometheus/specs/datasource_specs.ts @@ -5,7 +5,7 @@ import {PrometheusDatasource} from '../datasource'; describe('PrometheusDatasource', function() { var ctx = new helpers.ServiceTestContext(); - var instanceSettings = {url: 'proxied', directUrl: 'direct', user: 'test', password: 'mupp' }; + var instanceSettings = {url: 'proxied', directUrl: 'direct', user: 'test', password: 'mupp', jsonData: {}}; beforeEach(angularMocks.module('grafana.core')); beforeEach(angularMocks.module('grafana.services')); diff --git a/public/app/plugins/datasource/prometheus/specs/metric_find_query_specs.ts b/public/app/plugins/datasource/prometheus/specs/metric_find_query_specs.ts index 9186d9c4403..bb051d7328d 100644 --- a/public/app/plugins/datasource/prometheus/specs/metric_find_query_specs.ts +++ b/public/app/plugins/datasource/prometheus/specs/metric_find_query_specs.ts @@ -8,7 +8,7 @@ import PrometheusMetricFindQuery from '../metric_find_query'; describe('PrometheusMetricFindQuery', function() { var ctx = new helpers.ServiceTestContext(); - var instanceSettings = {url: 'proxied', directUrl: 'direct', user: 'test', password: 'mupp' }; + var instanceSettings = {url: 'proxied', directUrl: 'direct', user: 'test', password: 'mupp', jsonData: {}}; beforeEach(angularMocks.module('grafana.core')); beforeEach(angularMocks.module('grafana.services'));