Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ignoreNullValues for AWS CloudWatch Scaler #5635

Merged
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
53ea3fd
add errorWhenMetricValueEmpty
robpickerill Mar 28, 2024
4174e93
fix golangci-lint
robpickerill Mar 28, 2024
8e41311
improve error message for empty results
robpickerill Mar 29, 2024
b0d4e84
add error when empty metric values to changelog
robpickerill Mar 29, 2024
7a5f617
rename errorWhenMetricValuesEmpty -> errorWhenNullValues
robpickerill Mar 31, 2024
96a2254
use getParameterFromConfigV2 to read config for errorWhenNullValues
robpickerill Mar 31, 2024
476b4e5
Merge branch 'main' into cloudwatch-error-when-empty-metrics
robpickerill Mar 31, 2024
0b61f97
add e2e for error state for cw, and improve e2e for min values for cw
robpickerill Apr 3, 2024
c6dc524
remove erroneous print statement
robpickerill Apr 4, 2024
0dc9b77
remove unused vars
robpickerill Apr 4, 2024
1f70736
rename errorWhenMetricValuesEmpty -> ignoreNullValues
robpickerill Apr 11, 2024
185db21
move towards shared package for e2e aws
robpickerill Apr 13, 2024
9f5bbf5
minMetricValue optionality based on ignoreNullValues
robpickerill Apr 14, 2024
97f47a0
fail fast
robpickerill Apr 26, 2024
9d0ce60
Update tests/scalers/aws/aws_cloudwatch_ignore_null_values_false/aws_…
robpickerill Apr 26, 2024
a8959f2
Apply suggestions from code review
robpickerill Apr 26, 2024
702817f
fail fast
robpickerill Apr 26, 2024
2862e9c
fix broken new line
robpickerill Apr 26, 2024
f390bc4
Merge branch 'main' into cloudwatch-error-when-empty-metrics
robpickerill Apr 26, 2024
c684676
fix broken new lines
robpickerill Apr 26, 2024
5aceb72
assert no scaling changes in e2e, and set false for required in minMe…
robpickerill May 4, 2024
96b4097
Merge branch 'main' into cloudwatch-error-when-empty-metrics
robpickerill May 20, 2024
9374c26
fix ci checks
robpickerill May 21, 2024
1898924
Update tests/scalers/aws/aws_cloudwatch_min_metric_value/aws_cloudwat…
robpickerill May 21, 2024
a97992b
Merge branch 'main' into cloudwatch-error-when-empty-metrics
robpickerill May 28, 2024
943f929
Merge branch 'kedacore:main' into cloudwatch-error-when-empty-metrics
robpickerill May 31, 2024
ddd9f63
Merge branch 'main' into cloudwatch-error-when-empty-metrics
robpickerill Jun 8, 2024
2e76a9b
fix merge conflicts
robpickerill Jun 8, 2024
43a2c3f
fix e2e package names
robpickerill Jun 10, 2024
a241593
Merge branch 'main' into cloudwatch-error-when-empty-metrics
robpickerill Aug 16, 2024
8e436c3
Merge branch 'main' into cloudwatch-error-when-empty-metrics
robpickerill Aug 20, 2024
d55be16
Merge branch 'main' into cloudwatch-error-when-empty-metrics
robpickerill Aug 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 ignoreNullValues ([#5352](https://github.com/kedacore/keda/issues/5352))
- **GCP Stackdriver Scaler**: Add missing parameters 'rate' and 'count' for GCP Stackdriver Scaler alignment ([#5633](https://github.com/kedacore/keda/issues/5633))
- **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))
Expand Down
34 changes: 28 additions & 6 deletions pkg/scalers/aws_cloudwatch_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package scalers
import (
"context"
"fmt"
"reflect"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -42,6 +43,7 @@ type awsCloudwatchMetadata struct {
targetMetricValue float64
activationTargetMetricValue float64
minMetricValue float64
ignoreNullValues bool

metricCollectionTime int64
metricStat string
Expand Down Expand Up @@ -115,7 +117,6 @@ func getFloatMetadataValue(metadata map[string]string, key string, required bool

func createCloudwatchClient(ctx context.Context, metadata *awsCloudwatchMetadata) (*cloudwatch.Client, error) {
cfg, err := awsutils.GetAwsConfig(ctx, metadata.awsRegion, metadata.awsAuthorization)

if err != nil {
return nil, err
}
Expand Down Expand Up @@ -189,6 +190,12 @@ func parseAwsCloudwatchMetadata(config *scalersconfig.ScalerConfig) (*awsCloudwa
}
meta.minMetricValue = minMetricValue

ignoreNullValues, err := getParameterFromConfigV2(config, "ignoreNullValues", reflect.TypeOf(true), IsOptional(true), UseMetadata(true), WithDefaultVal(true))
robpickerill marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
meta.ignoreNullValues, _ = ignoreNullValues.(bool)

meta.metricStat = defaultMetricStat
if val, ok := config.TriggerMetadata["metricStat"]; ok && val != "" {
meta.metricStat = val
Expand All @@ -213,8 +220,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)
Expand Down Expand Up @@ -284,6 +291,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))
Expand All @@ -292,7 +307,6 @@ func computeQueryWindow(current time.Time, metricPeriodSec, metricEndTimeOffsetS

func (s *awsCloudwatchScaler) GetMetricsAndActivity(ctx context.Context, metricName string) ([]external_metrics.ExternalMetricValue, bool, error) {
metricValue, err := s.GetCloudwatchMetrics(ctx)

if err != nil {
s.logger.Error(err, "Error getting metric value")
return []external_metrics.ExternalMetricValue{}, false, err
Expand Down Expand Up @@ -375,20 +389,28 @@ func (s *awsCloudwatchScaler) GetCloudwatchMetrics(ctx context.Context) (float64
}

output, err := s.cwClient.GetMetricData(ctx, &input)

if err != nil {
s.logger.Error(err, "Failed to get output")
return -1, err
}

s.logger.V(1).Info("Received Metric Data", "data", output)

// If no metric data results or the first result has no values, and ignoreNullValues is false,
// the scaler should return an error to prevent any further scaling actions.
if len(output.MetricDataResults) > 0 && len(output.MetricDataResults[0].Values) == 0 && !s.metadata.ignoreNullValues {
emptyMetricsErrMsg := "empty metric data received, ignoreNullValues is false, returning error"
s.logger.Error(nil, emptyMetricsErrMsg)
return -1, fmt.Errorf(emptyMetricsErrMsg)
}

var metricValue float64

if len(output.MetricDataResults) > 0 && len(output.MetricDataResults[0].Values) > 0 {
metricValue = output.MetricDataResults[0].Values[0]
} else {
s.logger.Info("empty metric data received, returning minMetricValue")
metricValue = s.metadata.minMetricValue
}

return metricValue, nil
}
Loading