From 5c2dee19d5b1bd9e3493e01c64699d7b1592267b Mon Sep 17 00:00:00 2001 From: Shirley <4163034+fridgepoet@users.noreply.github.com> Date: Sun, 8 May 2022 09:27:03 +0200 Subject: [PATCH] CloudWatch: Refactor tests, remove unused parameters (#48815) --- pkg/tsdb/cloudwatch/query_row_response.go | 6 +- pkg/tsdb/cloudwatch/request_parser.go | 4 +- pkg/tsdb/cloudwatch/request_parser_test.go | 10 ++- pkg/tsdb/cloudwatch/response_parser.go | 2 +- pkg/tsdb/cloudwatch/response_parser_test.go | 72 ++++++++++--------- ...uts.json => multiple-outputs-query-a.json} | 29 +------- .../test-data/multiple-outputs-query-b.json | 47 ++++++++++++ 7 files changed, 93 insertions(+), 77 deletions(-) rename pkg/tsdb/cloudwatch/test-data/{multiple-outputs.json => multiple-outputs-query-a.json} (70%) create mode 100644 pkg/tsdb/cloudwatch/test-data/multiple-outputs-query-b.json diff --git a/pkg/tsdb/cloudwatch/query_row_response.go b/pkg/tsdb/cloudwatch/query_row_response.go index 3a471273fa7..d261d90d75f 100644 --- a/pkg/tsdb/cloudwatch/query_row_response.go +++ b/pkg/tsdb/cloudwatch/query_row_response.go @@ -4,9 +4,7 @@ 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 @@ -14,15 +12,13 @@ type queryRowResponse struct { 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 index f16b63608a4..e31ad779738 100644 --- 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 index d30d47a28de..65d3ee9edc9 100644 --- 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 index 2d58551b6af..7022e588edf 100644 --- 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 index 1e591c42339..4198877c11c 100644 --- 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/pkg/tsdb/cloudwatch/test-data/multiple-outputs.json b/pkg/tsdb/cloudwatch/test-data/multiple-outputs-query-a.json similarity index 70% rename from pkg/tsdb/cloudwatch/test-data/multiple-outputs.json rename to pkg/tsdb/cloudwatch/test-data/multiple-outputs-query-a.json index d0e9edcfe30..0b025a80128 100644 --- a/pkg/tsdb/cloudwatch/test-data/multiple-outputs.json +++ b/pkg/tsdb/cloudwatch/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 new file mode 100644 index 00000000000..3c4d7cae1c1 --- /dev/null +++ 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 + } +]