From 685c9043a82a11432a0e34ab71c08cc824c30d62 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Wed, 22 Jan 2020 13:43:36 +0100 Subject: [PATCH] CloudWatch: Auto period snap to next higher period (#21659) * Snap to next higher period instead of closest * Adjust period calc --- pkg/tsdb/cloudwatch/request_parser.go | 33 ++----- pkg/tsdb/cloudwatch/request_parser_test.go | 105 ++++++++++++--------- 2 files changed, 72 insertions(+), 66 deletions(-) diff --git a/pkg/tsdb/cloudwatch/request_parser.go b/pkg/tsdb/cloudwatch/request_parser.go index 8bf9807c558..6364cdefe08 100644 --- a/pkg/tsdb/cloudwatch/request_parser.go +++ b/pkg/tsdb/cloudwatch/request_parser.go @@ -68,8 +68,15 @@ func parseRequestQuery(model *simplejson.Json, refId string, startTime time.Time var period int if strings.ToLower(p) == "auto" || p == "" { deltaInSeconds := endTime.Sub(startTime).Seconds() - periods := []int{60, 300, 900, 3600, 21600} - period = closest(periods, int(math.Ceil(deltaInSeconds/2000))) + periods := []int{60, 300, 900, 3600, 21600, 86400} + datapoints := int(math.Ceil(deltaInSeconds / 2000)) + period = periods[len(periods)-1] + for _, value := range periods { + if datapoints <= value { + period = value + break + } + } } else { if regexp.MustCompile(`^\d+$`).Match([]byte(p)) { period, err = strconv.Atoi(p) @@ -158,25 +165,3 @@ func sortDimensions(dimensions map[string][]string) map[string][]string { } return sortedDimensions } - -func closest(array []int, num int) int { - minDiff := array[len(array)-1] - var closest int - if num <= array[0] { - return array[0] - } - - if num >= array[len(array)-1] { - return array[len(array)-1] - } - - for _, value := range array { - var m = int(math.Abs(float64(num - value))) - if m <= minDiff { - minDiff = m - closest = value - } - } - - return closest -} diff --git a/pkg/tsdb/cloudwatch/request_parser_test.go b/pkg/tsdb/cloudwatch/request_parser_test.go index 300c8dd1c5b..cf63d1eb642 100644 --- a/pkg/tsdb/cloudwatch/request_parser_test.go +++ b/pkg/tsdb/cloudwatch/request_parser_test.go @@ -2,6 +2,7 @@ package cloudwatch import ( "testing" + "time" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/tsdb" @@ -127,66 +128,86 @@ func TestRequestParser(t *testing.T) { "period": "auto", }) - Convey("when time range is short", func() { + Convey("when time range is 5 minutes", func() { query.Set("period", "auto") - timeRange := tsdb.NewTimeRange("now-2h", "now-1h") - from, _ := timeRange.ParseFrom() - to, _ := timeRange.ParseTo() + to := time.Now() + from := to.Local().Add(time.Minute * time.Duration(5)) res, err := parseRequestQuery(query, "ref1", from, to) So(err, ShouldBeNil) So(res.Period, ShouldEqual, 60) }) - Convey("when time range is 5y", func() { - timeRange := tsdb.NewTimeRange("now-5y", "now") - from, _ := timeRange.ParseFrom() - to, _ := timeRange.ParseTo() + Convey("when time range is 1 day", func() { + query.Set("period", "auto") + to := time.Now() + from := to.AddDate(0, 0, -1) + + res, err := parseRequestQuery(query, "ref1", from, to) + So(err, ShouldBeNil) + So(res.Period, ShouldEqual, 60) + }) + + Convey("when time range is 2 days", func() { + query.Set("period", "auto") + to := time.Now() + from := to.AddDate(0, 0, -2) + res, err := parseRequestQuery(query, "ref1", from, to) + So(err, ShouldBeNil) + So(res.Period, ShouldEqual, 300) + }) + + Convey("when time range is 7 days", func() { + query.Set("period", "auto") + to := time.Now() + from := to.AddDate(0, 0, -7) + + res, err := parseRequestQuery(query, "ref1", from, to) + So(err, ShouldBeNil) + So(res.Period, ShouldEqual, 900) + }) + + Convey("when time range is 30 days", func() { + query.Set("period", "auto") + to := time.Now() + from := to.AddDate(0, 0, -30) + + res, err := parseRequestQuery(query, "ref1", from, to) + So(err, ShouldBeNil) + So(res.Period, ShouldEqual, 3600) + }) + + Convey("when time range is 90 days", func() { + query.Set("period", "auto") + to := time.Now() + from := to.AddDate(0, 0, -90) res, err := parseRequestQuery(query, "ref1", from, to) So(err, ShouldBeNil) So(res.Period, ShouldEqual, 21600) }) - }) - Convey("closest works as expected", func() { - periods := []int{60, 300, 900, 3600, 21600} - Convey("and input is lower than 60", func() { - So(closest(periods, 6), ShouldEqual, 60) + Convey("when time range is 1 year", func() { + query.Set("period", "auto") + to := time.Now() + from := to.AddDate(-1, 0, 0) + + res, err := parseRequestQuery(query, "ref1", from, to) + So(err, ShouldBeNil) + So(res.Period, ShouldEqual, 21600) }) - Convey("and input is exactly 60", func() { - So(closest(periods, 60), ShouldEqual, 60) - }) + Convey("when time range is 2 years", func() { + query.Set("period", "auto") + to := time.Now() + from := to.AddDate(-2, 0, 0) - Convey("and input is exactly between two steps", func() { - So(closest(periods, 180), ShouldEqual, 300) - }) - - Convey("and input is exactly 2000", func() { - So(closest(periods, 2000), ShouldEqual, 900) - }) - - Convey("and input is exactly 5000", func() { - So(closest(periods, 5000), ShouldEqual, 3600) - }) - - Convey("and input is exactly 50000", func() { - So(closest(periods, 50000), ShouldEqual, 21600) - }) - - Convey("and period isn't shorter than min retension for 15 days", func() { - So(closest(periods, (60*60*24*15)+1/2000), ShouldBeGreaterThanOrEqualTo, 300) - }) - - Convey("and period isn't shorter than min retension for 63 days", func() { - So(closest(periods, (60*60*24*63)+1/2000), ShouldBeGreaterThanOrEqualTo, 3600) - }) - - Convey("and period isn't shorter than min retension for 455 days", func() { - So(closest(periods, (60*60*24*455)+1/2000), ShouldBeGreaterThanOrEqualTo, 21600) + res, err := parseRequestQuery(query, "ref1", from, to) + So(err, ShouldBeNil) + So(res.Period, ShouldEqual, 86400) }) }) + }) }) }