From ef9e08ffcfa998df4a9452b37190014e0a4255ad Mon Sep 17 00:00:00 2001 From: Shirley <4163034+fridgepoet@users.noreply.github.com> Date: Wed, 18 May 2022 00:16:38 -0700 Subject: [PATCH] CloudWatch: Change aggregateResponse to return slice instead of map (#48805) * Rename tests * Change test names * Change metrics from map to slice * Add test for one output, multiple MetricDataResults * Rename test input file * Use map instead of iterating over the response metrics * Rename variable * move partial data set to query row response * remove not used label field * remove incorrect placeholder Co-authored-by: Erik Sundell --- pkg/tsdb/cloudwatch/query_row_response.go | 33 ++++----- pkg/tsdb/cloudwatch/response_parser.go | 11 +-- pkg/tsdb/cloudwatch/response_parser_test.go | 70 +++++++++++-------- ...e-output-multiple-metric-data-results.json | 40 +++++++++++ .../MetricsQueryEditor/MetricsQueryEditor.tsx | 1 - 5 files changed, 101 insertions(+), 54 deletions(-) create mode 100644 pkg/tsdb/cloudwatch/test-data/single-output-multiple-metric-data-results.json diff --git a/pkg/tsdb/cloudwatch/query_row_response.go b/pkg/tsdb/cloudwatch/query_row_response.go index d261d90d75f..ea360c5fdc6 100644 --- a/pkg/tsdb/cloudwatch/query_row_response.go +++ b/pkg/tsdb/cloudwatch/query_row_response.go @@ -4,16 +4,17 @@ import "github.com/aws/aws-sdk-go/service/cloudwatch" // queryRowResponse represents the GetMetricData response for a query row in the query editor. type queryRowResponse struct { + partialDataSet map[string]*cloudwatch.MetricDataResult ErrorCodes map[string]bool - Labels []string HasArithmeticError bool ArithmeticErrorMessage string - Metrics map[string]*cloudwatch.MetricDataResult + Metrics []*cloudwatch.MetricDataResult StatusCode string } func newQueryRowResponse() queryRowResponse { return queryRowResponse{ + partialDataSet: make(map[string]*cloudwatch.MetricDataResult), ErrorCodes: map[string]bool{ maxMetricsExceeded: false, maxQueryTimeRangeExceeded: false, @@ -21,26 +22,26 @@ func newQueryRowResponse() queryRowResponse { maxMatchingResultsExceeded: false}, HasArithmeticError: false, ArithmeticErrorMessage: "", - Labels: []string{}, - Metrics: map[string]*cloudwatch.MetricDataResult{}, + Metrics: []*cloudwatch.MetricDataResult{}, } } func (q *queryRowResponse) addMetricDataResult(mdr *cloudwatch.MetricDataResult) { - label := *mdr.Label - q.Labels = append(q.Labels, label) - q.Metrics[label] = mdr - q.StatusCode = *mdr.StatusCode -} - -func (q *queryRowResponse) appendTimeSeries(mdr *cloudwatch.MetricDataResult) { - if _, exists := q.Metrics[*mdr.Label]; !exists { - q.Metrics[*mdr.Label] = &cloudwatch.MetricDataResult{} + if partialData, ok := q.partialDataSet[*mdr.Label]; ok { + partialData.Timestamps = append(partialData.Timestamps, mdr.Timestamps...) + partialData.Values = append(partialData.Values, mdr.Values...) + q.StatusCode = *mdr.StatusCode + if *mdr.StatusCode != "PartialData" { + delete(q.partialDataSet, *mdr.Label) + } + return } - metric := q.Metrics[*mdr.Label] - metric.Timestamps = append(metric.Timestamps, mdr.Timestamps...) - metric.Values = append(metric.Values, mdr.Values...) + + q.Metrics = append(q.Metrics, mdr) q.StatusCode = *mdr.StatusCode + if *mdr.StatusCode == "PartialData" { + q.partialDataSet[*mdr.Label] = mdr + } } func (q *queryRowResponse) addArithmeticError(message *string) { diff --git a/pkg/tsdb/cloudwatch/response_parser.go b/pkg/tsdb/cloudwatch/response_parser.go index 7022e588edf..c91b6f03985 100644 --- a/pkg/tsdb/cloudwatch/response_parser.go +++ b/pkg/tsdb/cloudwatch/response_parser.go @@ -62,7 +62,6 @@ func aggregateResponse(getMetricDataOutputs []*cloudwatch.GetMetricDataOutput) m } for _, r := range gmdo.MetricDataResults { id := *r.Id - label := *r.Label response := newQueryRowResponse() if _, exists := responseByID[id]; exists { @@ -75,11 +74,7 @@ func aggregateResponse(getMetricDataOutputs []*cloudwatch.GetMetricDataOutput) m } } - if _, exists := response.Metrics[label]; !exists { - response.addMetricDataResult(r) - } else { - response.appendTimeSeries(r) - } + response.addMetricDataResult(r) for code := range errorCodes { if _, exists := response.ErrorCodes[code]; exists { @@ -120,8 +115,8 @@ func getLabels(cloudwatchLabel string, query *cloudWatchQuery) data.Labels { func buildDataFrames(startTime time.Time, endTime time.Time, aggregatedResponse queryRowResponse, query *cloudWatchQuery, dynamicLabelEnabled bool) (data.Frames, error) { frames := data.Frames{} - for _, label := range aggregatedResponse.Labels { - metric := aggregatedResponse.Metrics[label] + for _, metric := range aggregatedResponse.Metrics { + label := *metric.Label deepLink, err := query.buildDeepLink(startTime, endTime) if err != nil { diff --git a/pkg/tsdb/cloudwatch/response_parser_test.go b/pkg/tsdb/cloudwatch/response_parser_test.go index 4198877c11c..0eb9b126caa 100644 --- a/pkg/tsdb/cloudwatch/response_parser_test.go +++ b/pkg/tsdb/cloudwatch/response_parser_test.go @@ -34,11 +34,12 @@ func TestCloudWatchResponseParser(t *testing.T) { aggregatedResponse := aggregateResponse(getMetricDataOutputs) idA := "a" t.Run("should have two labels", func(t *testing.T) { - assert.Len(t, aggregatedResponse[idA].Labels, 2) assert.Len(t, aggregatedResponse[idA].Metrics, 2) }) t.Run("should have points for label1 taken from both getMetricDataOutputs", func(t *testing.T) { - assert.Len(t, aggregatedResponse[idA].Metrics["label1"].Values, 10) + require.NotNil(t, *aggregatedResponse[idA].Metrics[0].Label) + require.Equal(t, "label1", *aggregatedResponse[idA].Metrics[0].Label) + assert.Len(t, aggregatedResponse[idA].Metrics[0].Values, 10) }) t.Run("should have statuscode 'Complete'", func(t *testing.T) { assert.Equal(t, "Complete", aggregatedResponse[idA].StatusCode) @@ -71,6 +72,24 @@ func TestCloudWatchResponseParser(t *testing.T) { }) }) + t.Run("when aggregating multi-outputs response", func(t *testing.T) { + getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./test-data/single-output-multiple-metric-data-results.json") + require.NoError(t, err) + aggregatedResponse := aggregateResponse(getMetricDataOutputs) + idA := "a" + t.Run("should have one label", func(t *testing.T) { + assert.Len(t, aggregatedResponse[idA].Metrics, 1) + }) + t.Run("should have points for label1 taken from both MetricDataResults", func(t *testing.T) { + require.NotNil(t, *aggregatedResponse[idA].Metrics[0].Label) + require.Equal(t, "label1", *aggregatedResponse[idA].Metrics[0].Label) + assert.Len(t, aggregatedResponse[idA].Metrics[0].Values, 6) + }) + t.Run("should have statuscode 'Complete'", func(t *testing.T) { + assert.Equal(t, "Complete", aggregatedResponse[idA].StatusCode) + }) + }) + t.Run("when aggregating response and error codes are in first GetMetricDataOutput", func(t *testing.T) { getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./test-data/multiple-outputs2.json") require.NoError(t, err) @@ -95,9 +114,8 @@ func TestCloudWatchResponseParser(t *testing.T) { t.Run("Expand dimension value using exact match", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &queryRowResponse{ - Labels: []string{"lb1", "lb2"}, - Metrics: map[string]*cloudwatch.MetricDataResult{ - "lb1": { + Metrics: []*cloudwatch.MetricDataResult{ + { Id: aws.String("id1"), Label: aws.String("lb1"), Timestamps: []*time.Time{ @@ -112,7 +130,7 @@ func TestCloudWatchResponseParser(t *testing.T) { }, StatusCode: aws.String("Complete"), }, - "lb2": { + { Id: aws.String("id2"), Label: aws.String("lb2"), Timestamps: []*time.Time{ @@ -160,9 +178,8 @@ func TestCloudWatchResponseParser(t *testing.T) { t.Run("Expand dimension value using substring", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &queryRowResponse{ - Labels: []string{"lb1 Sum", "lb2 Average"}, - Metrics: map[string]*cloudwatch.MetricDataResult{ - "lb1 Sum": { + Metrics: []*cloudwatch.MetricDataResult{ + { Id: aws.String("id1"), Label: aws.String("lb1 Sum"), Timestamps: []*time.Time{ @@ -177,7 +194,7 @@ func TestCloudWatchResponseParser(t *testing.T) { }, StatusCode: aws.String("Complete"), }, - "lb2 Average": { + { Id: aws.String("id2"), Label: aws.String("lb2 Average"), Timestamps: []*time.Time{ @@ -224,9 +241,8 @@ func TestCloudWatchResponseParser(t *testing.T) { t.Run("Expand dimension value using wildcard", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &queryRowResponse{ - Labels: []string{"lb3", "lb4"}, - Metrics: map[string]*cloudwatch.MetricDataResult{ - "lb3": { + Metrics: []*cloudwatch.MetricDataResult{ + { Id: aws.String("lb3"), Label: aws.String("lb3"), Timestamps: []*time.Time{ @@ -241,7 +257,7 @@ func TestCloudWatchResponseParser(t *testing.T) { }, StatusCode: aws.String("Complete"), }, - "lb4": { + { Id: aws.String("lb4"), Label: aws.String("lb4"), Timestamps: []*time.Time{ @@ -284,9 +300,8 @@ func TestCloudWatchResponseParser(t *testing.T) { t.Run("Expand dimension value when no values are returned and a multi-valued template variable is used", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &queryRowResponse{ - Labels: []string{"lb3"}, - Metrics: map[string]*cloudwatch.MetricDataResult{ - "lb3": { + Metrics: []*cloudwatch.MetricDataResult{ + { Id: aws.String("lb3"), Label: aws.String("lb3"), Timestamps: []*time.Time{ @@ -324,9 +339,8 @@ func TestCloudWatchResponseParser(t *testing.T) { t.Run("Expand dimension value when no values are returned and a multi-valued template variable and two single-valued dimensions are used", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &queryRowResponse{ - Labels: []string{"lb3"}, - Metrics: map[string]*cloudwatch.MetricDataResult{ - "lb3": { + Metrics: []*cloudwatch.MetricDataResult{ + { Id: aws.String("lb3"), Label: aws.String("lb3"), Timestamps: []*time.Time{ @@ -367,9 +381,8 @@ func TestCloudWatchResponseParser(t *testing.T) { t.Run("Should only expand certain fields when using SQL queries", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &queryRowResponse{ - Labels: []string{"lb3"}, - Metrics: map[string]*cloudwatch.MetricDataResult{ - "lb3": { + Metrics: []*cloudwatch.MetricDataResult{ + { Id: aws.String("lb3"), Label: aws.String("lb3"), Timestamps: []*time.Time{ @@ -412,9 +425,8 @@ func TestCloudWatchResponseParser(t *testing.T) { t.Run("Parse cloudwatch response", func(t *testing.T) { timestamp := time.Unix(0, 0) response := &queryRowResponse{ - Labels: []string{"lb"}, - Metrics: map[string]*cloudwatch.MetricDataResult{ - "lb": { + Metrics: []*cloudwatch.MetricDataResult{ + { Id: aws.String("id1"), Label: aws.String("lb"), Timestamps: []*time.Time{ @@ -463,9 +475,9 @@ func TestCloudWatchResponseParser(t *testing.T) { t.Run("buildDataFrames should use response label as frame name when dynamic label is enabled", func(t *testing.T) { response := &queryRowResponse{ - Labels: []string{"some response label"}, - Metrics: map[string]*cloudwatch.MetricDataResult{ - "some response label": { + Metrics: []*cloudwatch.MetricDataResult{ + { + Label: aws.String("some response label"), Timestamps: []*time.Time{}, Values: []*float64{aws.Float64(10)}, StatusCode: aws.String("Complete"), diff --git a/pkg/tsdb/cloudwatch/test-data/single-output-multiple-metric-data-results.json b/pkg/tsdb/cloudwatch/test-data/single-output-multiple-metric-data-results.json new file mode 100644 index 00000000000..a0da22faef6 --- /dev/null +++ b/pkg/tsdb/cloudwatch/test-data/single-output-multiple-metric-data-results.json @@ -0,0 +1,40 @@ +[ + { + "Messages": null, + "MetricDataResults": [ + { + "Id": "a", + "Label": "label1", + "Messages": null, + "StatusCode": "PartialData", + "Timestamps": [ + "2021-01-15T19:44:00Z", + "2021-01-15T19:59:00Z", + "2021-01-15T20:14:00Z", + "2021-01-15T20:29:00Z", + "2021-01-15T20:44:00Z" + ], + "Values": [ + 0.1333395078879982, + 0.244268469636633, + 0.15574387947267768, + 0.14447563659125626, + 0.15519743138527173 + ] + }, + { + "Id": "a", + "Label": "label1", + "Messages": null, + "StatusCode": "Complete", + "Timestamps": [ + "2021-01-15T19:44:00Z" + ], + "Values": [ + 0.1333395078879982 + ] + } + ], + "NextToken": null + } +] diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.tsx index 3b65e28c7ca..6f61af7724f 100644 --- a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.tsx @@ -140,7 +140,6 @@ export const MetricsQueryEditor = (props: Props) => { > ) =>