From 2c1373ea882bb5fef90378d04d9ec2838ac0ea75 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Mon, 16 Mar 2020 12:03:18 -0700 Subject: [PATCH] =?UTF-8?q?Handle=20nil=20pointers=20and=20empty=20arrays?= =?UTF-8?q?=20properly=20in=20Azure=20Monitor=20S=E2=80=A6=20(#680)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pkg/scalers/azure_monitor.go | 10 +++---- pkg/scalers/azure_monitor_test.go | 46 +++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/pkg/scalers/azure_monitor.go b/pkg/scalers/azure_monitor.go index 2753c1ead3d..d1f0f4f965d 100644 --- a/pkg/scalers/azure_monitor.go +++ b/pkg/scalers/azure_monitor.go @@ -118,19 +118,19 @@ func extractValue(azMetricRequest azureExternalMetricRequest, metricResult insig return -1, err } - timeseries := *metricVals[0].Timeseries - if timeseries == nil { + timeseriesPtr := metricVals[0].Timeseries + if timeseriesPtr == nil || len(*timeseriesPtr) == 0 { err := fmt.Errorf("Got metric result for %s/%s and aggregate type %s without timeseries", azMetricRequest.ResourceProviderNamespace, azMetricRequest.MetricName, insights.AggregationType(strings.ToTitle(azMetricRequest.Aggregation))) return -1, err } - data := *timeseries[0].Data - if data == nil { + dataPtr := (*timeseriesPtr)[0].Data + if dataPtr == nil || len(*dataPtr) == 0 { err := fmt.Errorf("Got metric result for %s/%s and aggregate type %s without any metric values", azMetricRequest.ResourceProviderNamespace, azMetricRequest.MetricName, insights.AggregationType(strings.ToTitle(azMetricRequest.Aggregation))) return -1, err } - valuePtr, err := verifyAggregationTypeIsSupported(azMetricRequest.Aggregation, data) + valuePtr, err := verifyAggregationTypeIsSupported(azMetricRequest.Aggregation, *dataPtr) if err != nil { return -1, fmt.Errorf("Unable to get value for metric %s/%s with aggregation %s. No value returned by Azure Monitor", azMetricRequest.ResourceProviderNamespace, azMetricRequest.MetricName, azMetricRequest.Aggregation) } diff --git a/pkg/scalers/azure_monitor_test.go b/pkg/scalers/azure_monitor_test.go index 883be47df33..8c499944d5a 100644 --- a/pkg/scalers/azure_monitor_test.go +++ b/pkg/scalers/azure_monitor_test.go @@ -2,6 +2,8 @@ package scalers import ( "testing" + + "github.com/Azure/azure-sdk-for-go/services/preview/monitor/mgmt/2018-03-01/insights" ) type parseAzMonitorMetadataTestData struct { @@ -62,3 +64,47 @@ func TestAzMonitorParseMetadata(t *testing.T) { } } } + +type testExtractAzMonitorTestData struct { + testName string + isError bool + expectedValue float64 + metricRequest azureExternalMetricRequest + metricResult insights.Response +} + +var testExtractAzMonitordata = []testExtractAzMonitorTestData{ + {"nothing returned", true, -1, azureExternalMetricRequest{}, insights.Response{Value: &[]insights.Metric{}}}, + {"timeseries null", true, -1, azureExternalMetricRequest{}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: nil}}}}, + {"timeseries empty", true, -1, azureExternalMetricRequest{}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{}}}}}, + {"data nil", true, -1, azureExternalMetricRequest{}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: nil}}}}}}, + {"data empty", true, -1, azureExternalMetricRequest{}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{}}}}}}}, + {"Total Aggregation requested", false, 40, azureExternalMetricRequest{Aggregation: "Total"}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{insights.MetricValue{Total: returnFloat64Ptr(40)}}}}}}}}, + {"Average Aggregation requested", false, 41, azureExternalMetricRequest{Aggregation: "Average"}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{insights.MetricValue{Average: returnFloat64Ptr(41)}}}}}}}}, + {"Maximum Aggregation requested", false, 42, azureExternalMetricRequest{Aggregation: "Maximum"}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{insights.MetricValue{Maximum: returnFloat64Ptr(42)}}}}}}}}, + {"Minimum Aggregation requested", false, 43, azureExternalMetricRequest{Aggregation: "Minimum"}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{insights.MetricValue{Minimum: returnFloat64Ptr(43)}}}}}}}}, + {"Count Aggregation requested", false, 44, azureExternalMetricRequest{Aggregation: "Count"}, insights.Response{Value: &[]insights.Metric{insights.Metric{Timeseries: &[]insights.TimeSeriesElement{insights.TimeSeriesElement{Data: &[]insights.MetricValue{insights.MetricValue{Count: returnint64Ptr(44)}}}}}}}}, +} + +func returnFloat64Ptr(x float64) *float64 { + return &x +} + +func returnint64Ptr(x int64) *int64 { + return &x +} + +func TestAzMonitorextractValue(t *testing.T) { + for _, testData := range testExtractAzMonitordata { + value, err := extractValue(testData.metricRequest, testData.metricResult) + if err != nil && !testData.isError { + t.Errorf("Test: %v; Expected success but got error: %v", testData.testName, err) + } + if testData.isError && err == nil { + t.Errorf("Test: %v; Expected error but got success. testData: %v", testData.testName, testData) + } + if err != nil && value != testData.expectedValue { + t.Errorf("Test: %v; Expected value %v but got %v testData: %v", testData.testName, testData.expectedValue, value, testData) + } + } +}