CloudWatch: Refactor tests, remove unused parameters (grafana#48815)

aiven · May 8, 2022 · 5c2dee1 · 5c2dee1
1 parent 99eaa0f
commit 5c2dee1
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 77 deletions.
diff --git a/pkg/tsdb/cloudwatch/query_row_response.go b/pkg/tsdb/cloudwatch/query_row_response.go
@@ -4,25 +4,21 @@ 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 {
-	ID                     string
 	ErrorCodes             map[string]bool
-	PartialData            bool
 	Labels                 []string
 	HasArithmeticError     bool
 	ArithmeticErrorMessage string
 	Metrics                map[string]*cloudwatch.MetricDataResult
 	StatusCode             string
 }
 
-func newQueryRowResponse(id string) queryRowResponse {
+func newQueryRowResponse() queryRowResponse {
 	return queryRowResponse{
-		ID: id,
 		ErrorCodes: map[string]bool{
 			maxMetricsExceeded:         false,
 			maxQueryTimeRangeExceeded:  false,
 			maxQueryResultsExceeded:    false,
 			maxMatchingResultsExceeded: false},
-		PartialData:            false,
 		HasArithmeticError:     false,
 		ArithmeticErrorMessage: "",
 		Labels:                 []string{},

diff --git a/pkg/tsdb/cloudwatch/request_parser.go b/pkg/tsdb/cloudwatch/request_parser.go
@@ -22,7 +22,7 @@ var validMetricDataID = regexp.MustCompile(`^[a-z][a-zA-Z0-9_]*$`)
 func (e *cloudWatchExecutor) parseQueries(queries []backend.DataQuery, startTime time.Time, endTime time.Time) (map[string][]*cloudWatchQuery, error) {
 	requestQueries := make(map[string][]*cloudWatchQuery)
 
-	migratedQueries, err := migrateLegacyQuery(queries, e.features.IsEnabled(featuremgmt.FlagCloudWatchDynamicLabels), startTime, endTime)
+	migratedQueries, err := migrateLegacyQuery(queries, e.features.IsEnabled(featuremgmt.FlagCloudWatchDynamicLabels))
 	if err != nil {
 		return nil, err
 	}
@@ -54,7 +54,7 @@ func (e *cloudWatchExecutor) parseQueries(queries []backend.DataQuery, startTime
 }
 
 // migrateLegacyQuery is also done in the frontend, so this should only ever be needed for alerting queries
-func migrateLegacyQuery(queries []backend.DataQuery, dynamicLabelsEnabled bool, startTime time.Time, endTime time.Time) ([]*backend.DataQuery, error) {
+func migrateLegacyQuery(queries []backend.DataQuery, dynamicLabelsEnabled bool) ([]*backend.DataQuery, error) {
 	migratedQueries := []*backend.DataQuery{}
 	for _, q := range queries {
 		query := q

diff --git a/pkg/tsdb/cloudwatch/request_parser_test.go b/pkg/tsdb/cloudwatch/request_parser_test.go
@@ -14,8 +14,6 @@ import (
 func TestRequestParser(t *testing.T) {
 	t.Run("Query migration ", func(t *testing.T) {
 		t.Run("legacy statistics field is migrated", func(t *testing.T) {
-			startTime := time.Now()
-			endTime := startTime.Add(2 * time.Hour)
 			oldQuery := &backend.DataQuery{
 				MaxDataPoints: 0,
 				QueryType:     "timeSeriesQuery",
@@ -33,7 +31,7 @@ func TestRequestParser(t *testing.T) {
 				"period": "600",
 				"hide": false
 			  }`)
-			migratedQueries, err := migrateLegacyQuery([]backend.DataQuery{*oldQuery}, false, startTime, endTime)
+			migratedQueries, err := migrateLegacyQuery([]backend.DataQuery{*oldQuery}, false)
 			require.NoError(t, err)
 			assert.Equal(t, 1, len(migratedQueries))
 
@@ -404,7 +402,7 @@ func Test_Test_migrateLegacyQuery(t *testing.T) {
 					"period": "600",
 					"hide": false
 				  }`)},
-			}, true, time.Now(), time.Now())
+			}, true)
 		require.NoError(t, err)
 		require.Equal(t, 1, len(migratedQueries))
 
@@ -461,7 +459,7 @@ func Test_Test_migrateLegacyQuery(t *testing.T) {
 					"hide": false
 				  }`),
 				},
-			}, true, time.Now(), time.Now())
+			}, true)
 		require.NoError(t, err)
 		require.Equal(t, 2, len(migratedQueries))
 
@@ -532,7 +530,7 @@ func Test_Test_migrateLegacyQuery(t *testing.T) {
 					"period": "600",
 					"hide": false
 				  }`, tc.labelJson))},
-					}, tc.dynamicLabelsFeatureToggleEnabled, time.Now(), time.Now())
+					}, tc.dynamicLabelsFeatureToggleEnabled)
 				require.NoError(t, err)
 				require.Equal(t, 1, len(migratedQueries))
 

diff --git a/pkg/tsdb/cloudwatch/response_parser.go b/pkg/tsdb/cloudwatch/response_parser.go
@@ -64,7 +64,7 @@ func aggregateResponse(getMetricDataOutputs []*cloudwatch.GetMetricDataOutput) m
 			id := *r.Id
 			label := *r.Label
 
-			response := newQueryRowResponse(id)
+			response := newQueryRowResponse()
 			if _, exists := responseByID[id]; exists {
 				response = responseByID[id]
 			}

diff --git a/pkg/tsdb/cloudwatch/response_parser_test.go b/pkg/tsdb/cloudwatch/response_parser_test.go
@@ -28,44 +28,46 @@ func loadGetMetricDataOutputsFromFile(filePath string) ([]*cloudwatch.GetMetricD
 func TestCloudWatchResponseParser(t *testing.T) {
 	startTime := time.Now()
 	endTime := startTime.Add(2 * time.Hour)
-	t.Run("when aggregating response", func(t *testing.T) {
-		getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./test-data/multiple-outputs.json")
+	t.Run("when aggregating multi-outputs response", func(t *testing.T) {
+		getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./test-data/multiple-outputs-query-a.json")
 		require.NoError(t, err)
 		aggregatedResponse := aggregateResponse(getMetricDataOutputs)
-		t.Run("response for id a", func(t *testing.T) {
-			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)
-			})
-			t.Run("should have statuscode 'Complete'", func(t *testing.T) {
-				assert.Equal(t, "Complete", aggregatedResponse[idA].StatusCode)
-			})
-			t.Run("should have exceeded request limit", func(t *testing.T) {
-				assert.True(t, aggregatedResponse[idA].ErrorCodes["MaxMetricsExceeded"])
-			})
-			t.Run("should have exceeded query time range", func(t *testing.T) {
-				assert.True(t, aggregatedResponse[idA].ErrorCodes["MaxQueryTimeRangeExceeded"])
-			})
-			t.Run("should have exceeded max query results", func(t *testing.T) {
-				assert.True(t, aggregatedResponse[idA].ErrorCodes["MaxQueryResultsExceeded"])
-			})
-			t.Run("should have exceeded max matching results", func(t *testing.T) {
-				assert.True(t, aggregatedResponse[idA].ErrorCodes["MaxMatchingResultsExceeded"])
-			})
+		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("response for id b", func(t *testing.T) {
-			idB := "b"
-			t.Run("should have statuscode is 'Partial'", func(t *testing.T) {
-				assert.Equal(t, "Partial", aggregatedResponse[idB].StatusCode)
-			})
-			t.Run("should have an arithmetic error and an error message", func(t *testing.T) {
-				assert.True(t, aggregatedResponse[idB].HasArithmeticError)
-				assert.Equal(t, "One or more data-points have been dropped due to non-numeric values (NaN, -Infinite, +Infinite)", aggregatedResponse[idB].ArithmeticErrorMessage)
-			})
+		t.Run("should have points for label1 taken from both getMetricDataOutputs", func(t *testing.T) {
+			assert.Len(t, aggregatedResponse[idA].Metrics["label1"].Values, 10)
+		})
+		t.Run("should have statuscode 'Complete'", func(t *testing.T) {
+			assert.Equal(t, "Complete", aggregatedResponse[idA].StatusCode)
+		})
+		t.Run("should have exceeded request limit", func(t *testing.T) {
+			assert.True(t, aggregatedResponse[idA].ErrorCodes["MaxMetricsExceeded"])
+		})
+		t.Run("should have exceeded query time range", func(t *testing.T) {
+			assert.True(t, aggregatedResponse[idA].ErrorCodes["MaxQueryTimeRangeExceeded"])
+		})
+		t.Run("should have exceeded max query results", func(t *testing.T) {
+			assert.True(t, aggregatedResponse[idA].ErrorCodes["MaxQueryResultsExceeded"])
+		})
+		t.Run("should have exceeded max matching results", func(t *testing.T) {
+			assert.True(t, aggregatedResponse[idA].ErrorCodes["MaxMatchingResultsExceeded"])
+		})
+	})
+
+	t.Run("when aggregating multi-outputs response with PartialData and ArithmeticError", func(t *testing.T) {
+		getMetricDataOutputs, err := loadGetMetricDataOutputsFromFile("./test-data/multiple-outputs-query-b.json")
+		require.NoError(t, err)
+		aggregatedResponse := aggregateResponse(getMetricDataOutputs)
+		idB := "b"
+		t.Run("should have statuscode is 'PartialData'", func(t *testing.T) {
+			assert.Equal(t, "PartialData", aggregatedResponse[idB].StatusCode)
+		})
+		t.Run("should have an arithmetic error and an error message", func(t *testing.T) {
+			assert.True(t, aggregatedResponse[idB].HasArithmeticError)
+			assert.Equal(t, "One or more data-points have been dropped due to non-numeric values (NaN, -Infinite, +Infinite)", aggregatedResponse[idB].ArithmeticErrorMessage)
 		})
 	})
 

diff --git a/...loudwatch/test-data/multiple-outputs.json → ...h/test-data/multiple-outputs-query-a.json b/...loudwatch/test-data/multiple-outputs.json → ...h/test-data/multiple-outputs-query-a.json
@@ -6,7 +6,7 @@
         "Id": "a",
         "Label": "label1",
         "Messages": null,
-        "StatusCode": "Complete",
+        "StatusCode": "PartialData",
         "Timestamps": [
           "2021-01-15T19:44:00Z",
           "2021-01-15T19:59:00Z",
@@ -33,18 +33,6 @@
         "Values": [
           0.1333395078879982
         ]
-      },
-      {
-        "Id": "b",
-        "Label": "label2",
-        "Messages": null,
-        "StatusCode": "Complete",
-        "Timestamps": [
-          "2021-01-15T19:44:00Z"
-        ],
-        "Values": [
-          0.1333395078879982
-        ]
       }
     ],
     "NextToken": null
@@ -77,21 +65,6 @@
           0.14447563659125626,
           0.15519743138527173
         ]
-      },
-      {
-        "Id": "b",
-        "Label": "label2",
-        "Messages": [{
-          "Code": "ArithmeticError",
-          "Value": "One or more data-points have been dropped due to non-numeric values (NaN, -Infinite, +Infinite)"
-        }],
-        "StatusCode": "Partial",
-        "Timestamps": [
-          "2021-01-15T19:44:00Z"
-        ],
-        "Values": [
-          0.1333395078879982
-        ]
       }
     ],
     "NextToken": null

diff --git a/pkg/tsdb/cloudwatch/test-data/multiple-outputs-query-b.json b/pkg/tsdb/cloudwatch/test-data/multiple-outputs-query-b.json
@@ -0,0 +1,47 @@
+[
+  {
+    "Messages": null,
+    "MetricDataResults": [
+      {
+        "Id": "b",
+        "Label": "label2",
+        "Messages": null,
+        "StatusCode": "Complete",
+        "Timestamps": [
+          "2021-01-15T19:44:00Z"
+        ],
+        "Values": [
+          0.1333395078879982
+        ]
+      }
+    ],
+    "NextToken": null
+  },
+  {
+    "Messages": [
+      { "Code": "", "Value": null },
+      { "Code": "MaxMetricsExceeded", "Value": null },
+      { "Code": "MaxQueryTimeRangeExceeded", "Value": null },
+      { "Code": "MaxQueryResultsExceeded", "Value": null },
+      { "Code": "MaxMatchingResultsExceeded", "Value": null }
+    ],
+    "MetricDataResults": [
+      {
+        "Id": "b",
+        "Label": "label2",
+        "Messages": [{
+          "Code": "ArithmeticError",
+          "Value": "One or more data-points have been dropped due to non-numeric values (NaN, -Infinite, +Infinite)"
+        }],
+        "StatusCode": "PartialData",
+        "Timestamps": [
+          "2021-01-15T19:44:00Z"
+        ],
+        "Values": [
+          0.1333395078879982
+        ]
+      }
+    ],
+    "NextToken": null
+  }
+]