From b1ddbaa395c856eaea2683b0c44bd09908146645 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Fri, 7 Aug 2020 11:48:40 +0200 Subject: [PATCH] CloudWatch: Fix sorting of metrics results (#26835) * CloudWatch: Fix sorting of metrics results --- pkg/tsdb/cloudwatch/response_parser.go | 53 +++++++++++---------- pkg/tsdb/cloudwatch/response_parser_test.go | 30 +++++++----- 2 files changed, 46 insertions(+), 37 deletions(-) diff --git a/pkg/tsdb/cloudwatch/response_parser.go b/pkg/tsdb/cloudwatch/response_parser.go index 95be4e45795..c05308302d1 100644 --- a/pkg/tsdb/cloudwatch/response_parser.go +++ b/pkg/tsdb/cloudwatch/response_parser.go @@ -13,7 +13,9 @@ import ( ) func (e *cloudWatchExecutor) parseResponse(metricDataOutputs []*cloudwatch.GetMetricDataOutput, queries map[string]*cloudWatchQuery) ([]*cloudwatchResponse, error) { - mdr := make(map[string]map[string]*cloudwatch.MetricDataResult) + // Map from result ID -> label -> result + mdrs := make(map[string]map[string]*cloudwatch.MetricDataResult) + labels := map[string][]string{} for _, mdo := range metricDataOutputs { requestExceededMaxLimit := false for _, message := range mdo.Messages { @@ -23,36 +25,42 @@ func (e *cloudWatchExecutor) parseResponse(metricDataOutputs []*cloudwatch.GetMe } for _, r := range mdo.MetricDataResults { - if _, exists := mdr[*r.Id]; !exists { - mdr[*r.Id] = make(map[string]*cloudwatch.MetricDataResult) - mdr[*r.Id][*r.Label] = r - } else if _, exists := mdr[*r.Id][*r.Label]; !exists { - mdr[*r.Id][*r.Label] = r + id := *r.Id + label := *r.Label + if _, exists := mdrs[id]; !exists { + mdrs[id] = make(map[string]*cloudwatch.MetricDataResult) + mdrs[id][label] = r + labels[id] = append(labels[id], label) + } else if _, exists := mdrs[id][label]; !exists { + mdrs[id][label] = r + labels[id] = append(labels[id], label) } else { - mdr[*r.Id][*r.Label].Timestamps = append(mdr[*r.Id][*r.Label].Timestamps, r.Timestamps...) - mdr[*r.Id][*r.Label].Values = append(mdr[*r.Id][*r.Label].Values, r.Values...) + mdr := mdrs[id][label] + mdr.Timestamps = append(mdr.Timestamps, r.Timestamps...) + mdr.Values = append(mdr.Values, r.Values...) if *r.StatusCode == "Complete" { - mdr[*r.Id][*r.Label].StatusCode = r.StatusCode + mdr.StatusCode = r.StatusCode } } - queries[*r.Id].RequestExceededMaxLimit = requestExceededMaxLimit + queries[id].RequestExceededMaxLimit = requestExceededMaxLimit } } cloudWatchResponses := make([]*cloudwatchResponse, 0) - for id, lr := range mdr { - series, partialData, err := parseGetMetricDataTimeSeries(lr, queries[id]) + for id, lr := range mdrs { + query := queries[id] + series, partialData, err := parseGetMetricDataTimeSeries(lr, labels[id], query) if err != nil { return nil, err } response := &cloudwatchResponse{ series: series, - Period: queries[id].Period, - Expression: queries[id].UsedExpression, - RefId: queries[id].RefId, - Id: queries[id].Id, - RequestExceededMaxLimit: queries[id].RequestExceededMaxLimit, + Period: query.Period, + Expression: query.UsedExpression, + RefId: query.RefId, + Id: query.Id, + RequestExceededMaxLimit: query.RequestExceededMaxLimit, PartialData: partialData, } cloudWatchResponses = append(cloudWatchResponses, response) @@ -61,16 +69,11 @@ func (e *cloudWatchExecutor) parseResponse(metricDataOutputs []*cloudwatch.GetMe return cloudWatchResponses, nil } -func parseGetMetricDataTimeSeries(metricDataResults map[string]*cloudwatch.MetricDataResult, query *cloudWatchQuery) (*tsdb.TimeSeriesSlice, bool, error) { - metricDataResultLabels := make([]string, 0) - for k := range metricDataResults { - metricDataResultLabels = append(metricDataResultLabels, k) - } - sort.Strings(metricDataResultLabels) - +func parseGetMetricDataTimeSeries(metricDataResults map[string]*cloudwatch.MetricDataResult, labels []string, + query *cloudWatchQuery) (*tsdb.TimeSeriesSlice, bool, error) { partialData := false result := tsdb.TimeSeriesSlice{} - for _, label := range metricDataResultLabels { + for _, label := range labels { metricDataResult := metricDataResults[label] if *metricDataResult.StatusCode != "Complete" { partialData = true diff --git a/pkg/tsdb/cloudwatch/response_parser_test.go b/pkg/tsdb/cloudwatch/response_parser_test.go index c50b872c568..5d8f1471889 100644 --- a/pkg/tsdb/cloudwatch/response_parser_test.go +++ b/pkg/tsdb/cloudwatch/response_parser_test.go @@ -14,7 +14,8 @@ func TestCloudWatchResponseParser(t *testing.T) { Convey("TestCloudWatchResponseParser", t, func() { Convey("can expand dimension value using exact match", func() { timestamp := time.Unix(0, 0) - resp := map[string]*cloudwatch.MetricDataResult{ + labels := []string{"lb1", "lb2"} + mdrs := map[string]*cloudwatch.MetricDataResult{ "lb1": { Id: aws.String("id1"), Label: aws.String("lb1"), @@ -60,7 +61,7 @@ func TestCloudWatchResponseParser(t *testing.T) { Period: 60, Alias: "{{LoadBalancer}} Expanded", } - series, partialData, err := parseGetMetricDataTimeSeries(resp, query) + series, partialData, err := parseGetMetricDataTimeSeries(mdrs, labels, query) timeSeries := (*series)[0] So(err, ShouldBeNil) @@ -75,7 +76,8 @@ func TestCloudWatchResponseParser(t *testing.T) { Convey("can expand dimension value using substring", func() { timestamp := time.Unix(0, 0) - resp := map[string]*cloudwatch.MetricDataResult{ + labels := []string{"lb1 Sum", "lb2 Average"} + mdrs := map[string]*cloudwatch.MetricDataResult{ "lb1 Sum": { Id: aws.String("id1"), Label: aws.String("lb1 Sum"), @@ -121,7 +123,7 @@ func TestCloudWatchResponseParser(t *testing.T) { Period: 60, Alias: "{{LoadBalancer}} Expanded", } - series, partialData, err := parseGetMetricDataTimeSeries(resp, query) + series, partialData, err := parseGetMetricDataTimeSeries(mdrs, labels, query) timeSeries := (*series)[0] So(err, ShouldBeNil) So(partialData, ShouldBeFalse) @@ -135,7 +137,8 @@ func TestCloudWatchResponseParser(t *testing.T) { Convey("can expand dimension value using wildcard", func() { timestamp := time.Unix(0, 0) - resp := map[string]*cloudwatch.MetricDataResult{ + labels := []string{"lb3", "lb4"} + mdrs := map[string]*cloudwatch.MetricDataResult{ "lb3": { Id: aws.String("lb3"), Label: aws.String("lb3"), @@ -181,7 +184,7 @@ func TestCloudWatchResponseParser(t *testing.T) { Period: 60, Alias: "{{LoadBalancer}} Expanded", } - series, partialData, err := parseGetMetricDataTimeSeries(resp, query) + series, partialData, err := parseGetMetricDataTimeSeries(mdrs, labels, query) So(err, ShouldBeNil) So(partialData, ShouldBeFalse) @@ -191,7 +194,8 @@ func TestCloudWatchResponseParser(t *testing.T) { Convey("can expand dimension value when no values are returned and a multi-valued template variable is used", func() { timestamp := time.Unix(0, 0) - resp := map[string]*cloudwatch.MetricDataResult{ + labels := []string{"lb3"} + mdrs := map[string]*cloudwatch.MetricDataResult{ "lb3": { Id: aws.String("lb3"), Label: aws.String("lb3"), @@ -217,7 +221,7 @@ func TestCloudWatchResponseParser(t *testing.T) { Period: 60, Alias: "{{LoadBalancer}} Expanded", } - series, partialData, err := parseGetMetricDataTimeSeries(resp, query) + series, partialData, err := parseGetMetricDataTimeSeries(mdrs, labels, query) So(err, ShouldBeNil) So(partialData, ShouldBeFalse) @@ -228,7 +232,8 @@ func TestCloudWatchResponseParser(t *testing.T) { Convey("can expand dimension value when no values are returned and a multi-valued template variable and two single-valued dimensions are used", func() { timestamp := time.Unix(0, 0) - resp := map[string]*cloudwatch.MetricDataResult{ + labels := []string{"lb3"} + mdrs := map[string]*cloudwatch.MetricDataResult{ "lb3": { Id: aws.String("lb3"), Label: aws.String("lb3"), @@ -256,7 +261,7 @@ func TestCloudWatchResponseParser(t *testing.T) { Period: 60, Alias: "{{LoadBalancer}} Expanded {{InstanceType}} - {{Resource}}", } - series, partialData, err := parseGetMetricDataTimeSeries(resp, query) + series, partialData, err := parseGetMetricDataTimeSeries(mdrs, labels, query) So(err, ShouldBeNil) So(partialData, ShouldBeFalse) @@ -267,7 +272,8 @@ func TestCloudWatchResponseParser(t *testing.T) { Convey("can parse cloudwatch response", func() { timestamp := time.Unix(0, 0) - resp := map[string]*cloudwatch.MetricDataResult{ + labels := []string{"lb"} + mdrs := map[string]*cloudwatch.MetricDataResult{ "lb": { Id: aws.String("id1"), Label: aws.String("lb"), @@ -298,7 +304,7 @@ func TestCloudWatchResponseParser(t *testing.T) { Period: 60, Alias: "{{namespace}}_{{metric}}_{{stat}}", } - series, partialData, err := parseGetMetricDataTimeSeries(resp, query) + series, partialData, err := parseGetMetricDataTimeSeries(mdrs, labels, query) timeSeries := (*series)[0] So(err, ShouldBeNil)