Skip to content

Commit

Permalink
add ignoreNullValues for AWS CloudWatch Scaler (#5635)
Browse files Browse the repository at this point in the history
* add errorWhenMetricValueEmpty

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* fix golangci-lint

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* improve error message for empty results

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* add error when empty metric values to changelog

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* rename errorWhenMetricValuesEmpty -> errorWhenNullValues

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* use getParameterFromConfigV2 to read config for errorWhenNullValues

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* add e2e for error state for cw, and improve e2e for min values for cw

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* remove erroneous print statement

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* remove unused vars

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* rename errorWhenMetricValuesEmpty -> ignoreNullValues

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* move towards shared package for e2e aws

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* minMetricValue optionality based on ignoreNullValues

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* fail fast

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* Update tests/scalers/aws/aws_cloudwatch_ignore_null_values_false/aws_cloudwatch_ignore_null_values_false_test.go

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* Apply suggestions from code review

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* fail fast

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* fix broken new line

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* fix broken new lines

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* assert no scaling changes in e2e, and set false for required in minMetricValue

Signed-off-by: robpickerill <r.pickerill@gmail.com>

* fix ci checks

Signed-off-by: robpickerill <r.pickerill@gmail.com>

* Update tests/scalers/aws/aws_cloudwatch_min_metric_value/aws_cloudwatch_min_metric_value_test.go

fix invalid check

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>

* fix merge conflicts

Signed-off-by: robpickerill <r.pickerill@gmail.com>

* fix e2e package names

Signed-off-by: robpickerill <r.pickerill@gmail.com>

---------

Signed-off-by: Rob Pickerill <r.pickerill@gmail.com>
Signed-off-by: robpickerill <r.pickerill@gmail.com>
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
  • Loading branch information
robpickerill and JorTurFer authored Aug 20, 2024
1 parent 03041f4 commit 0771b35
Show file tree
Hide file tree
Showing 7 changed files with 889 additions and 276 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ Here is an overview of all new **experimental** features:

### Improvements

- TODO ([#XXX](https://github.com/kedacore/keda/issues/XXX))
- **AWS CloudWatch Scaler**: Add support for ignoreNullValues ([#5352](https://github.com/kedacore/keda/issues/5352))
- **GCP Scalers**: Added custom time horizon in GCP scalers ([#5778](https://github.com/kedacore/keda/issues/5778))
- **GitHub Scaler**: Fixed pagination, fetching repository list ([#5738](https://github.com/kedacore/keda/issues/5738))
- **Kafka**: Fix logic to scale to zero on invalid offset even with earliest offsetResetPolicy ([#5689](https://github.com/kedacore/keda/issues/5689))

### Fixes

Expand Down
14 changes: 11 additions & 3 deletions pkg/scalers/aws_cloudwatch_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type awsCloudwatchMetadata struct {
TargetMetricValue float64 `keda:"name=targetMetricValue, order=triggerMetadata"`
ActivationTargetMetricValue float64 `keda:"name=activationTargetMetricValue, order=triggerMetadata, optional"`
MinMetricValue float64 `keda:"name=minMetricValue, order=triggerMetadata"`
IgnoreNullValues bool `keda:"name=ignoreNullValues, order=triggerMetadata, optional, default=true"`

MetricCollectionTime int64 `keda:"name=metricCollectionTime, order=triggerMetadata, optional, default=300"`
MetricStat string `keda:"name=metricStat, order=triggerMetadata, optional, default=Average"`
Expand Down Expand Up @@ -191,7 +192,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 @@ -274,20 +274,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

0 comments on commit 0771b35

Please sign in to comment.