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 <erik.sundell87@gmail.com>

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:                []*cloudwatch.MetricDataResult{},
-		Metrics:                map[string]*cloudwatch.MetricDataResult{},
 	}
 }
 
 func (q *queryRowResponse) addMetricDataResult(mdr *cloudwatch.MetricDataResult) {
-	label := *mdr.Label
+	if partialData, ok := q.partialDataSet[*mdr.Label]; ok {
-	q.Labels = append(q.Labels, label)
+		partialData.Timestamps = append(partialData.Timestamps, mdr.Timestamps...)
-	q.Metrics[label] = mdr
+		partialData.Values = append(partialData.Values, mdr.Values...)
-	q.StatusCode = *mdr.StatusCode
+		q.StatusCode = *mdr.StatusCode
-}
+		if *mdr.StatusCode != "PartialData" {
-
+			delete(q.partialDataSet, *mdr.Label)
-func (q *queryRowResponse) appendTimeSeries(mdr *cloudwatch.MetricDataResult) {
+		}
-	if _, exists := q.Metrics[*mdr.Label]; !exists {
+		return
-		q.Metrics[*mdr.Label] = &cloudwatch.MetricDataResult{}
 	}
-	metric := q.Metrics[*mdr.Label]
+
-	metric.Timestamps = append(metric.Timestamps, mdr.Timestamps...)
+	q.Metrics = append(q.Metrics, mdr)
-	metric.Values = append(metric.Values, mdr.Values...)
 	q.StatusCode = *mdr.StatusCode
+	if *mdr.StatusCode == "PartialData" {
+		q.partialDataSet[*mdr.Label] = mdr
+	}
 }
 
 func (q *queryRowResponse) addArithmeticError(message *string) {

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)
-				response.addMetricDataResult(r)
-			} else {
-				response.appendTimeSeries(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 {
+	for _, metric := range aggregatedResponse.Metrics {
-		metric := aggregatedResponse.Metrics[label]
+		label := *metric.Label
 
 		deepLink, err := query.buildDeepLink(startTime, endTime)
 		if err != nil {

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: []*cloudwatch.MetricDataResult{
-			Metrics: map[string]*cloudwatch.MetricDataResult{
+				{
-				"lb1": {
 					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: []*cloudwatch.MetricDataResult{
-			Metrics: map[string]*cloudwatch.MetricDataResult{
+				{
-				"lb1 Sum": {
 					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: []*cloudwatch.MetricDataResult{
-			Metrics: map[string]*cloudwatch.MetricDataResult{
+				{
-				"lb3": {
 					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: []*cloudwatch.MetricDataResult{
-			Metrics: map[string]*cloudwatch.MetricDataResult{
+				{
-				"lb3": {
 					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: []*cloudwatch.MetricDataResult{
-			Metrics: map[string]*cloudwatch.MetricDataResult{
+				{
-				"lb3": {
 					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: []*cloudwatch.MetricDataResult{
-			Metrics: map[string]*cloudwatch.MetricDataResult{
+				{
-				"lb3": {
 					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: []*cloudwatch.MetricDataResult{
-			Metrics: map[string]*cloudwatch.MetricDataResult{
+				{
-				"lb": {
 					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: []*cloudwatch.MetricDataResult{
-			Metrics: map[string]*cloudwatch.MetricDataResult{
+				{
-				"some response label": {
+					Label:      aws.String("some response label"),
 					Timestamps: []*time.Time{},
 					Values:     []*float64{aws.Float64(10)},
 					StatusCode: aws.String("Complete"),

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
+  }
+]

public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryEditor.tsx

@@ -140,7 +140,6 @@ export const MetricsQueryEditor = (props: Props) => {
           >
             <Input
               id={`${query.refId}-cloudwatch-metric-query-editor-label`}
-              placeholder="auto"
               onBlur={onRunQuery}
               value={preparedQuery.label ?? ''}
               onChange={(event: ChangeEvent<HTMLInputElement>) =>