-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 Support for Mutli-Queries in Datadog Scaler #3550
Conversation
Signed-off-by: dgibbard-cisco <57677847+dgibbard-cisco@users.noreply.github.com>
Would be good to get @arapulido 's eyes on this too if available, as the main code creator :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
I am not super convinced about this, as it starts implementing things like avg or max outside Datadog.
I would accept the change though if we warn the user clearly, and we don't use defaults that can hide what the scaler is doing.
I have added ways of doing that in the inline review. I will review the docs as well.
Some context here; Datadog doesn't support running avg/max operations across the results of multiple queries (eg; something like |
Signed-off-by: Darren Gibbard <dgibbard@cisco.com>
Pushed some changes so that:
@arapulido I think this covers the desire to not have it default, and/or display an error when not explicitly set? As well as the suggested cleanup :) |
Signed-off-by: Darren Gibbard <dgibbard@cisco.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
/run-e2e datadog* |
Signed-off-by: dgibbard-cisco <57677847+dgibbard-cisco@users.noreply.github.com>
Resolved conflict with #3617 :) |
Signed-off-by: Darren Gibbard <dgibbard@cisco.com>
Signed-off-by: dgibbard-cisco <57677847+dgibbard-cisco@users.noreply.github.com>
@zroubalik - can I trouble you for a re-kick for e2e tests? |
/run-e2e datadog* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks for the contribution! ❤️
Signed-off-by: Darren Gibbard dgibbard@cisco.com
Adds support for comma-separated multi-queries in the Datadog Scaler.
Previously, if a query returned more than one element, we would throw an error. With this change, when Datadog returns multiple
Series
elements, we then apply anaverage
ormax
operation (set by thequeryAggregator
parameter) on the latest points from eachSeries
to aggregate the results to a single return value.This is beneficial to reducing the number of Datadog API Requests being made, which can quickly hit rate limits which impacts scaling.
Checklist
Fixes #3423
Doc PR Link: kedacore/keda-docs#885