-
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
Added custom time horizon in GCP scaler #5778
Conversation
Signed-off-by: Yaxhveer <yaxhcod@gmail.com>
Signed-off-by: Yaxhveer <yaxhcod@gmail.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.
Looking good 😄
Does it make sense to support this in other stackdriver functions? For example, GetMetrics function has the time window hardcoded too:
keda/pkg/scalers/gcp/gcp_stackdriver_client.go
Lines 222 to 232 in a168022
func (s StackDriverClient) GetMetrics( | |
ctx context.Context, | |
filter string, | |
projectID string, | |
aggregation *monitoringpb.Aggregation, | |
valueIfNull *float64) (float64, error) { | |
// Set the start time to 1 minute ago | |
startTime := time.Now().UTC().Add(time.Minute * -2) | |
// Set the end time to now | |
endTime := time.Now().UTC() |
Signed-off-by: Yaxhveer <yaxhcod@gmail.com>
@JorTurFer I have made the required changes in other stackdrivers too. |
/run-e2e gcp |
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Signed-off-by: Yashveer <101015836+Yaxhveer@users.noreply.github.com>
/run-e2e gcp |
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!
Added the custom time horizon for gcp scalar and updated the test accordingly
Checklist
Fixes #5453
Relates to #