From 0b74860f55890d27125b22ff97112b682e4b7f6f Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Mon, 11 Feb 2019 13:27:08 +0100 Subject: [PATCH] azuremonitor: fix auto interval calculation on backend Not needed for alerting (as the query intervalms will always be 0) but needed later when being called from the frontend) --- .../azuremonitor/azuremonitor-datasource.go | 8 +++---- .../azuremonitor-datasource_test.go | 6 ++--- pkg/tsdb/azuremonitor/time-grain.go | 23 +++++++++---------- pkg/tsdb/azuremonitor/time-grain_test.go | 21 +++++++++++++---- pkg/tsdb/azuremonitor/url-builder.go | 6 ++--- pkg/tsdb/azuremonitor/url-builder_test.go | 6 ++--- pkg/tsdb/interval_test.go | 10 ++++---- 7 files changed, 45 insertions(+), 35 deletions(-) diff --git a/pkg/tsdb/azuremonitor/azuremonitor-datasource.go b/pkg/tsdb/azuremonitor/azuremonitor-datasource.go index 079910e1b66..5def94aebf6 100644 --- a/pkg/tsdb/azuremonitor/azuremonitor-datasource.go +++ b/pkg/tsdb/azuremonitor/azuremonitor-datasource.go @@ -89,7 +89,7 @@ func (e *AzureMonitorDatasource) buildQueries(queries []*tsdb.Query, timeRange * urlComponents["metricDefinition"] = fmt.Sprintf("%v", azureMonitorTarget["metricDefinition"]) urlComponents["resourceName"] = fmt.Sprintf("%v", azureMonitorTarget["resourceName"]) - ub := URLBuilder{ + ub := urlBuilder{ ResourceGroup: urlComponents["resourceGroup"], MetricDefinition: urlComponents["metricDefinition"], ResourceName: urlComponents["resourceName"], @@ -100,9 +100,9 @@ func (e *AzureMonitorDatasource) buildQueries(queries []*tsdb.Query, timeRange * timeGrain := fmt.Sprintf("%v", azureMonitorTarget["timeGrain"]) if timeGrain == "auto" { - autoInSeconds := e.findClosestAllowedIntervalMs(query.IntervalMs) / 1000 + autoInterval := e.findClosestAllowedIntervalMS(query.IntervalMs) tg := &TimeGrain{} - timeGrain, err = tg.createISO8601DurationFromInterval(fmt.Sprintf("%vs", autoInSeconds)) + timeGrain, err = tg.createISO8601DurationFromIntervalMS(autoInterval) if err != nil { return nil, err } @@ -288,7 +288,7 @@ func (e *AzureMonitorDatasource) parseResponse(queryRes *tsdb.QueryResult, data // findClosestAllowedIntervalMs is used for the auto time grain setting. // It finds the closest time grain from the list of allowed time grains for Azure Monitor // using the Grafana interval in milliseconds -func (e *AzureMonitorDatasource) findClosestAllowedIntervalMs(intervalMs int64) int64 { +func (e *AzureMonitorDatasource) findClosestAllowedIntervalMS(intervalMs int64) int64 { closest := allowedIntervalsMS[0] for i, allowed := range allowedIntervalsMS { diff --git a/pkg/tsdb/azuremonitor/azuremonitor-datasource_test.go b/pkg/tsdb/azuremonitor/azuremonitor-datasource_test.go index 9aba4eb617b..b8d1d6cc266 100644 --- a/pkg/tsdb/azuremonitor/azuremonitor-datasource_test.go +++ b/pkg/tsdb/azuremonitor/azuremonitor-datasource_test.go @@ -240,13 +240,13 @@ func TestAzureMonitorDatasource(t *testing.T) { "2d": 172800000, } - closest := datasource.findClosestAllowedIntervalMs(intervals["3m"]) + closest := datasource.findClosestAllowedIntervalMS(intervals["3m"]) So(closest, ShouldEqual, intervals["5m"]) - closest = datasource.findClosestAllowedIntervalMs(intervals["10m"]) + closest = datasource.findClosestAllowedIntervalMS(intervals["10m"]) So(closest, ShouldEqual, intervals["15m"]) - closest = datasource.findClosestAllowedIntervalMs(intervals["2d"]) + closest = datasource.findClosestAllowedIntervalMS(intervals["2d"]) So(closest, ShouldEqual, intervals["1d"]) }) }) diff --git a/pkg/tsdb/azuremonitor/time-grain.go b/pkg/tsdb/azuremonitor/time-grain.go index 22da2872bab..e6a15aef64f 100644 --- a/pkg/tsdb/azuremonitor/time-grain.go +++ b/pkg/tsdb/azuremonitor/time-grain.go @@ -4,6 +4,9 @@ import ( "fmt" "strconv" "strings" + "time" + + "github.com/grafana/grafana/pkg/tsdb" ) // TimeGrain handles convertions between @@ -15,28 +18,24 @@ var ( smallTimeUnits = []string{"hour", "minute", "h", "m"} ) -func (tg *TimeGrain) createISO8601DurationFromInterval(interval string) (string, error) { - if strings.Contains(interval, "ms") { +func (tg *TimeGrain) createISO8601DurationFromIntervalMS(interval int64) (string, error) { + formatted := tsdb.FormatDuration(time.Duration(interval) * time.Millisecond) + + if strings.Contains(formatted, "ms") { return "PT1M", nil } - timeValueString := interval[0 : len(interval)-1] + timeValueString := formatted[0 : len(formatted)-1] timeValue, err := strconv.Atoi(timeValueString) if err != nil { return "", fmt.Errorf("Could not parse interval %v to an ISO 8061 duration", interval) } - unit := interval[len(interval)-1:] - - if unit == "s" { - toMinutes := (timeValue * 60) % 60 + unit := formatted[len(formatted)-1:] + if unit == "s" && timeValue < 60 { // mimumum interval is 1m for Azure Monitor - if toMinutes < 1 { - toMinutes = 1 - } - - return tg.createISO8601Duration(toMinutes, "m"), nil + return "PT1M", nil } return tg.createISO8601Duration(timeValue, unit), nil diff --git a/pkg/tsdb/azuremonitor/time-grain_test.go b/pkg/tsdb/azuremonitor/time-grain_test.go index be8d0b10a0c..2df3c92b0ff 100644 --- a/pkg/tsdb/azuremonitor/time-grain_test.go +++ b/pkg/tsdb/azuremonitor/time-grain_test.go @@ -37,10 +37,14 @@ func TestTimeGrain(t *testing.T) { }) }) - Convey("create ISO 8601 Duration from Grafana interval", func() { + Convey("create ISO 8601 Duration from Grafana interval in milliseconds", func() { Convey("and interval is less than a minute", func() { - durationMS, _ := tgc.createISO8601DurationFromInterval("100ms") - durationS, _ := tgc.createISO8601DurationFromInterval("59s") + durationMS, err := tgc.createISO8601DurationFromIntervalMS(100) + So(err, ShouldBeNil) + + durationS, err := tgc.createISO8601DurationFromIntervalMS(59999) + So(err, ShouldBeNil) + Convey("should be rounded up to a minute as is the minimum interval for Azure Monitor", func() { So(durationMS, ShouldEqual, "PT1M") So(durationS, ShouldEqual, "PT1M") @@ -48,8 +52,15 @@ func TestTimeGrain(t *testing.T) { }) Convey("and interval is more than a minute", func() { - durationM, _ := tgc.createISO8601DurationFromInterval("10m") - durationD, _ := tgc.createISO8601DurationFromInterval("2d") + intervals := map[string]int64{ + "10m": 600000, + "2d": 172800000, + } + durationM, err := tgc.createISO8601DurationFromIntervalMS(intervals["10m"]) + So(err, ShouldBeNil) + durationD, err := tgc.createISO8601DurationFromIntervalMS(intervals["2d"]) + So(err, ShouldBeNil) + Convey("should be rounded up to a minute as is the minimum interval for Azure Monitor", func() { So(durationM, ShouldEqual, "PT10M") So(durationD, ShouldEqual, "P2D") diff --git a/pkg/tsdb/azuremonitor/url-builder.go b/pkg/tsdb/azuremonitor/url-builder.go index 1ccbbc2bf81..c252048f517 100644 --- a/pkg/tsdb/azuremonitor/url-builder.go +++ b/pkg/tsdb/azuremonitor/url-builder.go @@ -5,8 +5,8 @@ import ( "strings" ) -// URLBuilder builds the URL for calling the Azure Monitor API -type URLBuilder struct { +// urlBuilder builds the URL for calling the Azure Monitor API +type urlBuilder struct { ResourceGroup string MetricDefinition string ResourceName string @@ -14,7 +14,7 @@ type URLBuilder struct { // Build checks the metric definition property to see which form of the url // should be returned -func (ub *URLBuilder) Build() string { +func (ub *urlBuilder) Build() string { if strings.Count(ub.MetricDefinition, "/") > 1 { rn := strings.Split(ub.ResourceName, "/") diff --git a/pkg/tsdb/azuremonitor/url-builder_test.go b/pkg/tsdb/azuremonitor/url-builder_test.go index baf9b34d7eb..85c4f81bc83 100644 --- a/pkg/tsdb/azuremonitor/url-builder_test.go +++ b/pkg/tsdb/azuremonitor/url-builder_test.go @@ -10,7 +10,7 @@ func TestURLBuilder(t *testing.T) { Convey("AzureMonitor URL Builder", t, func() { Convey("when metric definition is in the short form", func() { - ub := &URLBuilder{ + ub := &urlBuilder{ ResourceGroup: "rg", MetricDefinition: "Microsoft.Compute/virtualMachines", ResourceName: "rn", @@ -21,7 +21,7 @@ func TestURLBuilder(t *testing.T) { }) Convey("when metric definition is Microsoft.Storage/storageAccounts/blobServices", func() { - ub := &URLBuilder{ + ub := &urlBuilder{ ResourceGroup: "rg", MetricDefinition: "Microsoft.Storage/storageAccounts/blobServices", ResourceName: "rn1/default", @@ -32,7 +32,7 @@ func TestURLBuilder(t *testing.T) { }) Convey("when metric definition is Microsoft.Storage/storageAccounts/fileServices", func() { - ub := &URLBuilder{ + ub := &urlBuilder{ ResourceGroup: "rg", MetricDefinition: "Microsoft.Storage/storageAccounts/fileServices", ResourceName: "rn1/default", diff --git a/pkg/tsdb/interval_test.go b/pkg/tsdb/interval_test.go index 941b08dd554..4cd3fcea532 100644 --- a/pkg/tsdb/interval_test.go +++ b/pkg/tsdb/interval_test.go @@ -51,11 +51,11 @@ func TestInterval(t *testing.T) { }) Convey("Format value", func() { - 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") + 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") }) }) }