Skip to content

Commit

Permalink
add errorWhenMetricValueEmpty
Browse files Browse the repository at this point in the history 
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
  • Loading branch information
@robpickerill
robpickerill committed Mar 28, 2024
1 parent f288c01 commit 53ea3fd
Show file tree
Hide file tree
Showing 2 changed files with 190 additions and 1 deletion.
31 changes: 31 additions & 0 deletions pkg/scalers/aws_cloudwatch_scaler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type awsCloudwatchMetadata struct {
targetMetricValue float64
activationTargetMetricValue float64
minMetricValue float64
errorWhenMetricValuesEmpty bool

metricCollectionTime int64
metricStat string
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -383,6 +406,14 @@ 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 recieved, or the minMetricValue
if len(output.MetricDataResults) == 0 && s.metadata.errorWhenMetricValuesEmpty {
s.logger.Error(err, "empty metric data received and errorWhenMetricValuesEmpty is set to true")
return -1, fmt.Errorf("empty metric data received and errorWhenMetricValuesEmpty is set to true")
}

if len(output.MetricDataResults) > 0 && len(output.MetricDataResults[0].Values) > 0 {
metricValue = output.MetricDataResults[0].Values[0]
} else {
Expand Down
160 changes: 159 additions & 1 deletion pkg/scalers/aws_cloudwatch_scaler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package scalers
import (
"context"
"errors"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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)
}
})
}
}

0 comments on commit 53ea3fd

Please sign in to comment.