add ignoreNullValues for AWS CloudWatch Scaler

CHANGELOG.md

@@ -67,6 +67,7 @@ Here is an overview of all new **experimental** features:
 - **General**: Add GRPC Healthchecks ([#5590](https://github.com/kedacore/keda/issues/5590))
 - **General**: Add OPENTELEMETRY flag in e2e test YAML ([#5375](https://github.com/kedacore/keda/issues/5375))
 - **General**: Add support for cross tenant/cloud authentication when using Azure Workload Identity for TriggerAuthentication ([#5441](https://github.com/kedacore/keda/issues/5441))
+- **AWS CloudWatch Scaler**: Add support for errorWhenMetricValuesEmpty ([#5352](https://github.com/kedacore/keda/issues/5352))
 - **Metrics API Scaler**: Add support for various formats: json, xml, yaml, prometheus ([#2633](https://github.com/kedacore/keda/issues/2633))
 - **MongoDB Scaler**: Add scheme field support srv record ([#5544](https://github.com/kedacore/keda/issues/5544))
 

pkg/scalers/aws_cloudwatch_scaler.go

@@ -42,6 +42,7 @@ type awsCloudwatchMetadata struct {
 	targetMetricValue           float64
 	activationTargetMetricValue float64
 	minMetricValue              float64
+	errorWhenMetricValuesEmpty  bool
 
 	metricCollectionTime int64
 	metricStat           string
@@ -113,6 +114,22 @@ func getFloatMetadataValue(metadata map[string]string, key string, required bool
 	return defaultValue, nil
 }
 
+func getBoolMetadataValue(metadata map[string]string, key string, required bool, defaultValue bool) (bool, error) {
+	if val, ok := metadata[key]; ok && val != "" {
+		value, err := strconv.ParseBool(val)
+		if err != nil {
+			return false, fmt.Errorf("error parsing %s metadata: %w", key, err)
+		}
+		return value, nil
+	}
+
+	if required {
+		return false, fmt.Errorf("metadata %s not given", key)
+	}
+
+	return defaultValue, nil
+}
+
 func createCloudwatchClient(ctx context.Context, metadata *awsCloudwatchMetadata) (*cloudwatch.Client, error) {
 	cfg, err := awsutils.GetAwsConfig(ctx, metadata.awsRegion, metadata.awsAuthorization)
 
@@ -189,6 +206,12 @@ func parseAwsCloudwatchMetadata(config *scalersconfig.ScalerConfig) (*awsCloudwa
 	}
 	meta.minMetricValue = minMetricValue
 
+	errorWhenMetricValuesEmpty, err := getBoolMetadataValue(config.TriggerMetadata, "errorWhenMetricValuesEmpty", false, false)
+	if err != nil {
+		return nil, err
+	}
+	meta.errorWhenMetricValuesEmpty = errorWhenMetricValuesEmpty
+
 	meta.metricStat = defaultMetricStat
 	if val, ok := config.TriggerMetadata["metricStat"]; ok && val != "" {
 		meta.metricStat = val
@@ -213,8 +236,8 @@ func parseAwsCloudwatchMetadata(config *scalersconfig.ScalerConfig) (*awsCloudwa
 	}
 	meta.metricCollectionTime = metricCollectionTime
 
-	if meta.metricCollectionTime < 0 || meta.metricCollectionTime%meta.metricStatPeriod != 0 {
-		return nil, fmt.Errorf("metricCollectionTime must be greater than 0 and a multiple of metricStatPeriod(%d), %d is given", meta.metricStatPeriod, meta.metricCollectionTime)
+	if err = checkMetricCollectionTime(meta.metricCollectionTime, meta.metricStatPeriod); err != nil {
+		return nil, err
 	}
 
 	metricEndTimeOffset, err := getIntMetadataValue(config.TriggerMetadata, "metricEndTimeOffset", false, defaultMetricEndTimeOffset)
@@ -284,6 +307,14 @@ func checkMetricStatPeriod(period int64) error {
 	return nil
 }
 
+func checkMetricCollectionTime(metricCollectionTime, metricStatPeriod int64) error {
+	if metricCollectionTime < 0 || metricCollectionTime%metricStatPeriod != 0 {
+		return fmt.Errorf("metricCollectionTime must be greater than 0 and a multiple of metricStatPeriod(%d), %d is given", metricStatPeriod, metricCollectionTime)
+	}
+
+	return nil
+}
+
 func computeQueryWindow(current time.Time, metricPeriodSec, metricEndTimeOffsetSec, metricCollectionTimeSec int64) (startTime, endTime time.Time) {
 	endTime = current.Add(time.Second * -1 * time.Duration(metricEndTimeOffsetSec)).Truncate(time.Duration(metricPeriodSec) * time.Second)
 	startTime = endTime.Add(time.Second * -1 * time.Duration(metricCollectionTimeSec))
@@ -383,6 +414,15 @@ func (s *awsCloudwatchScaler) GetCloudwatchMetrics(ctx context.Context) (float64
 
 	s.logger.V(1).Info("Received Metric Data", "data", output)
 	var metricValue float64
+
+	// If no metric data is received and errorWhenMetricValuesEmpty is set to true, return error,
+	// otherwise continue with either the metric value received, or the minMetricValue
+	if len(output.MetricDataResults) == 0 && s.metadata.errorWhenMetricValuesEmpty {
+		emptyMetricsErrMessage := "empty result of metric values received, and errorWhenMetricValuesEmpty is set to true"
+		s.logger.Error(err, emptyMetricsErrMessage)
+		return -1, fmt.Errorf(emptyMetricsErrMessage)
+	}
+
 	if len(output.MetricDataResults) > 0 && len(output.MetricDataResults[0].Values) > 0 {
 		metricValue = output.MetricDataResults[0].Values[0]
 	} else {

pkg/scalers/aws_cloudwatch_scaler_test.go

@@ -3,6 +3,7 @@ package scalers
 import (
 	"context"
 	"errors"
+	"strings"
 	"testing"
 	"time"
 
@@ -356,6 +357,48 @@ var testAWSCloudwatchMetadata = []parseAWSCloudwatchMetadataTestData{
 		"awsRegion":         "eu-west-1"},
 		testAWSAuthentication, true,
 		"unsupported metricUnit"},
+	// test errorWhenMetricValuesEmpty is false
+	{map[string]string{
+		"namespace":                  "AWS/SQS",
+		"dimensionName":              "QueueName",
+		"dimensionValue":             "keda",
+		"metricName":                 "ApproximateNumberOfMessagesVisible",
+		"targetMetricValue":          "2",
+		"minMetricValue":             "0",
+		"metricStatPeriod":           "60",
+		"metricStat":                 "Average",
+		"errorWhenMetricValuesEmpty": "false",
+		"awsRegion":                  "eu-west-1"},
+		testAWSAuthentication, false,
+		"with errorWhenMetricValuesEmpty set to false"},
+	// test errorWhenMetricValuesEmpty is true
+	{map[string]string{
+		"namespace":                  "AWS/SQS",
+		"dimensionName":              "QueueName",
+		"dimensionValue":             "keda",
+		"metricName":                 "ApproximateNumberOfMessagesVisible",
+		"targetMetricValue":          "2",
+		"minMetricValue":             "0",
+		"metricStatPeriod":           "60",
+		"metricStat":                 "Average",
+		"errorWhenMetricValuesEmpty": "true",
+		"awsRegion":                  "eu-west-1"},
+		testAWSAuthentication, false,
+		"with errorWhenMetricValuesEmpty set to true"},
+	// test errorWhenMetricValuesEmpty is incorrect
+	{map[string]string{
+		"namespace":                  "AWS/SQS",
+		"dimensionName":              "QueueName",
+		"dimensionValue":             "keda",
+		"metricName":                 "ApproximateNumberOfMessagesVisible",
+		"targetMetricValue":          "2",
+		"minMetricValue":             "0",
+		"metricStatPeriod":           "60",
+		"metricStat":                 "Average",
+		"errorWhenMetricValuesEmpty": "maybe",
+		"awsRegion":                  "eu-west-1"},
+		testAWSAuthentication, true,
+		"unsupported value for errorWhenMetricValuesEmpty"},
 }
 
 var awsCloudwatchMetricIdentifiers = []awsCloudwatchMetricIdentifier{
@@ -444,6 +487,42 @@ var awsCloudwatchGetMetricTestData = []awsCloudwatchMetadata{
 		awsAuthorization:     awsutils.AuthorizationMetadata{PodIdentityOwner: false},
 		triggerIndex:         0,
 	},
+	// Test for metric with no data
+	{
+		namespace:                  "Custom",
+		metricsName:                testAWSCloudwatchNoValueMetric,
+		dimensionName:              []string{"DIM"},
+		dimensionValue:             []string{"DIM_VALUE"},
+		targetMetricValue:          100,
+		minMetricValue:             0,
+		errorWhenMetricValuesEmpty: false,
+		metricCollectionTime:       60,
+		metricStat:                 "Average",
+		metricUnit:                 "",
+		metricStatPeriod:           60,
+		metricEndTimeOffset:        60,
+		awsRegion:                  "us-west-2",
+		awsAuthorization:           awsutils.AuthorizationMetadata{PodIdentityOwner: false},
+		triggerIndex:               0,
+	},
+	// Test for metric with no data, and the scaler errors when metric data values are empty
+	{
+		namespace:                  "Custom",
+		metricsName:                testAWSCloudwatchNoValueMetric,
+		dimensionName:              []string{"DIM"},
+		dimensionValue:             []string{"DIM_VALUE"},
+		targetMetricValue:          100,
+		minMetricValue:             0,
+		errorWhenMetricValuesEmpty: true,
+		metricCollectionTime:       60,
+		metricStat:                 "Average",
+		metricUnit:                 "",
+		metricStatPeriod:           60,
+		metricEndTimeOffset:        60,
+		awsRegion:                  "us-west-2",
+		awsAuthorization:           awsutils.AuthorizationMetadata{PodIdentityOwner: false},
+		triggerIndex:               0,
+	},
 }
 
 type mockCloudwatch struct {
@@ -507,7 +586,12 @@ func TestAWSCloudwatchScalerGetMetrics(t *testing.T) {
 		case testAWSCloudwatchErrorMetric:
 			assert.Error(t, err, "expect error because of cloudwatch api error")
 		case testAWSCloudwatchNoValueMetric:
-			assert.NoError(t, err, "dont expect error when returning empty metric list from cloudwatch")
+			// if errorWhenMetricValuesEmpty is defined, then an error is expected
+			if meta.errorWhenMetricValuesEmpty {
+				assert.Error(t, err, "expected an error when returning empty metric list from cloudwatch")
+			} else {
+				assert.NoError(t, err, "dont expect error when returning empty metric list from cloudwatch")
+			}
 		default:
 			assert.EqualValues(t, int64(10.0), value[0].Value.Value())
 		}
@@ -556,3 +640,77 @@ func TestComputeQueryWindow(t *testing.T) {
 		assert.Equal(t, testData.expectedEndTime, endTime.UTC().Format(time.RFC3339Nano), "unexpected endTime", "name", testData.name)
 	}
 }
+
+func TestGetBoolMetadataValue(t *testing.T) {
+	testCases := []struct {
+		name          string
+		metadata      map[string]string
+		key           string
+		required      bool
+		defaultValue  bool
+		expectedValue bool
+		expectedError string
+	}{
+		{
+			name:          "valid true value",
+			metadata:      map[string]string{"key": "true"},
+			key:           "key",
+			required:      true,
+			defaultValue:  false,
+			expectedValue: true,
+			expectedError: "",
+		},
+		{
+			name:          "valid false value",
+			metadata:      map[string]string{"key": "false"},
+			key:           "key",
+			required:      true,
+			defaultValue:  true,
+			expectedValue: false,
+			expectedError: "",
+		},
+		{
+			name:          "invalid value",
+			metadata:      map[string]string{"key": "invalid"},
+			key:           "key",
+			required:      true,
+			defaultValue:  false,
+			expectedValue: false,
+			expectedError: "error parsing key metadata: strconv.ParseBool: parsing \"invalid\": invalid syntax",
+		},
+		{
+			name:          "key not present, required",
+			metadata:      map[string]string{},
+			key:           "key",
+			required:      true,
+			defaultValue:  false,
+			expectedValue: false,
+			expectedError: "metadata key not given",
+		},
+		{
+			name:          "key not present, not required",
+			metadata:      map[string]string{},
+			key:           "key",
+			required:      false,
+			defaultValue:  true,
+			expectedValue: true,
+			expectedError: "",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			value, err := getBoolMetadataValue(tc.metadata, tc.key, tc.required, tc.defaultValue)
+
+			if tc.expectedError == "" && err != nil {
+				t.Errorf("unexpected error: %v", err)
+			} else if tc.expectedError != "" && (err == nil || !strings.Contains(err.Error(), tc.expectedError)) {
+				t.Errorf("expected error containing %q, got %v", tc.expectedError, err)
+			}
+
+			if value != tc.expectedValue {
+				t.Errorf("expected value %v, got %v", tc.expectedValue, value)
+			}
+		})
+	}
+}