keda icon indicating copy to clipboard operation
keda copied to clipboard

AWS Cloudwatch Scaler, do nothing on empty metric data received

Open robpickerill opened this issue 7 months ago • 10 comments

Proposal

Is there a way to do nothing when an empty metric response is returned from AWS CloudWatch? The current behaviour appears to be to return the minMetricValue, but would it be possible to do nothing in this case? I may of overlooked the option for this though

https://github.com/kedacore/keda/blob/main/pkg/scalers/aws_cloudwatch_scaler.go#L385-L386

Happy to implement this if functionality doesn't exist in the current AWS CloudWatch scaler options.

Use-Case

We are experimenting with the AWS Scaler and custom metrics. The use case here is when the system producing metrics is offline and unable to produce metrics, we would want no scaling actions to take place.

Is this a feature you are interested in implementing yourself?

Yes

Anything else?

No response

robpickerill avatar Jan 05 '24 17:01 robpickerill

Hi @robpickerill could you please draft here the empty response? It is a null or empty array and it is something that is expected from CloudWatch? Also, what is the value that should be reported in this case?

zroubalik avatar Jan 08 '24 20:01 zroubalik

Hi, thanks for the follow up @zroubalik.

It is an empty response, so in this case, there are no metrics being returned from the GetMetricData call. We fall into this branch: https://github.com/kedacore/keda/blob/main/pkg/scalers/aws_cloudwatch_scaler.go#L386, as the MetricDataResults has zero length.

The wider scenario here is that another system is producing custom metrics, by which a ScaledObject is being used to query that metric and scale a separate system, which is a deployment, to the relevant capacity. In the case whereby the system producing the custom metrics is offline, no metrics are being returned from the GetMetricData call. In this scenario, no metrics in the GetMetricData call is normal behaviour from CloudWatch.

What I wanted to guard against in this scenario is further scaling activity. I'd like the deployment to remain at the current scale. From what I've observed, in the case whereby a single AWS CloudWatch scaler exists, the deployment will scale down to the largest value of the minReplicaCount, or the evaluation of the minMetricValue in the scaling policy.

I don't know if this is a viable approach, but one way I could see working from what I understand so far is to have an option on the AWS CloudWatch scaler to handle an empty result as an error. Thus marking that scaler/trigger in an error state. This option would therefore be mutually exclusive to the minMetricValue option.


Edit: if you'd want to inspect the GetMetricData json payload here to understand further, I can share some example responses with you tomorrow.

robpickerill avatar Jan 08 '24 20:01 robpickerill

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Mar 08 '24 21:03 stale[bot]

This issue has been automatically closed due to inactivity.

stale[bot] avatar Mar 16 '24 16:03 stale[bot]

Sorry for the delay, I missed the reply somehow.

Having an option setting that would allow AWS CloudWatch scaler to handle an empty result as an error is okay with me. We just need to be backwards compatible.

zroubalik avatar Mar 27 '24 16:03 zroubalik

Thanks @zroubalik -- just to check, would the backwards compatibility be enough to add a new metadata option for the cloudwatch scaler: errorWhenMetricValuesEmpty, that's optional, and defaults to false to maintain previous behaviour if the user supplies no options when upgrading?

robpickerill avatar Mar 28 '24 13:03 robpickerill

Thanks @zroubalik -- just to check, would the backwards compatibility be enough to add a new metadata option for the cloudwatch scaler: errorWhenMetricValuesEmpty, that's optional, and defaults to false to maintain previous behaviour if the user supplies no options when upgrading?

Yes, that's enough :)

zroubalik avatar Mar 28 '24 14:03 zroubalik

Thanks, you can assign this to me, I'll send over a PR for review soon

robpickerill avatar Mar 28 '24 14:03 robpickerill

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar May 27 '24 15:05 stale[bot]

This issue has been automatically closed due to inactivity.

stale[bot] avatar Jun 07 '24 13:06 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Aug 08 '24 05:08 stale[bot]