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.

Sign up for GitHub

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

Jump to bottom

Add query parameters in prometheus scaler #4957

Merged
merged 21 commits into from Oct 10, 2023
Merged

Add query parameters in prometheus scaler #4957

merged 21 commits into from Oct 10, 2023

Conversation

HenriquePiccolo
Copy link
Contributor

@HenriquePiccolo HenriquePiccolo commented Sep 8, 2023 •
edited by zroubalik

Signed-off-by: Henrique Piccolo henriquepiccolo@hotmail.com.br

Add query parameters in prometheus scaler

Checklist

Relates to #1218

Fixes #4962

@HenriquePiccolo HenriquePiccolo requested a review from a team as a code owner September 8, 2023 18:36
@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

While you are waiting, make sure to:

Learn more about:

@HenriquePiccolo
Add query parameters in prometheus scaler
f01bec3 
Signed-off-by: Henrique Piccolo <henriquepiccolo@hotmail.com.br>
@HenriquePiccolo
Add query parameters in prometheus scaler
08ac6f4 
Signed-off-by: Henrique Piccolo <henriquepiccolo@hotmail.com.br>
@HenriquePiccolo
Add test scaling queryParameters
4fecfd7 
Signed-off-by: Henrique Piccolo <henriquepiccolo@hotmail.com.br>
@HenriquePiccolo
Add test scaling queryParameters
4557574 
Signed-off-by: Henrique Piccolo <henriquepiccolo@hotmail.com.br>
@HenriquePiccolo
Add test scaling queryParameters
c4272b7 
Signed-off-by: Henrique Piccolo <henriquepiccolo@hotmail.com.br>
@HenriquePiccolo
Add test scaling queryParameters
af8a351 
Signed-off-by: Henrique Piccolo <henriquepiccolo@hotmail.com.br>
JorTurFer
JorTurFer reviewed Sep 8, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@JorTurFer
Copy link
Member

What would you like to achieve adding extra info to the query string?

@tomkerkhove tomkerkhove mentioned this pull request Sep 11, 2023
Merged
1 task
zroubalik
zroubalik reviewed Sep 11, 2023
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add issue that explains the reason for this feature?

@HenriquePiccolo
Copy link
Contributor Author

Here's the issue #4962

JorTurFer
JorTurFer reviewed Sep 11, 2023
View reviewed changes
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be done just with the custom headers? I mean, isn't them enough for setting the tenant and those stuff?

@HenriquePiccolo @JorTurFer
Update CHANGELOG.md
ac402f4 
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Henrique Piccolo <henriquepiccolo@hotmail.com.br>
@HenriquePiccolo
Copy link
Contributor Author

@JorTurFer

Yes, we are aware of the custom headers and we use them to specify wich tenant keda just must reach.

But, just to give you some context, we are using thanos as our long term storage tsdb with a setup of hundreds of tenants, and within that, there are different versions of this solution, so it is there that query parameters are necessary, they are used to specify the version of our solution that we want to query.

JorTurFer
JorTurFer reviewed Sep 14, 2023
View reviewed changes
pkg/scalers/prometheus_scaler_test.go Outdated Show resolved Hide resolved
pkg/scalers/prometheus_scaler.go Outdated Show resolved Hide resolved
zroubalik
zroubalik reviewed Sep 26, 2023
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HenriquePiccolo any update on this please? We are going to cut the release tomorrow.

@HenriquePiccolo
Update queryParameter test
c5e4af2 
Signed-off-by: Henrique Piccolo <henriquepiccolo@hotmail.com.br>
@HenriquePiccolo
Add character validate in queryParameter
739534c 
Signed-off-by: Henrique Piccolo <henriquepiccolo@hotmail.com.br>
@HenriquePiccolo
Copy link
Contributor Author

@zroubalik @JorTurFer

I updated the scaler test to validate prometheusQuery.

@axelsccp
Add keda prometheus scaler query parameters tests
5c4ddd9 
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
@axelsccp
Fix prometheus scaler fmt
398d13d 
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
@axelsccp
Add feature to changelog
c253cdf 
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
@axelsccp
Scape prometheus scaler URL
2fba509 
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
@axelsccp axelsccp force-pushed the add-query-parameter-prometheus branch 2 times, most recently from 106a903 to 398d13d Compare October 9, 2023 19:36
@axelsccp axelsccp force-pushed the add-query-parameter-prometheus branch from dd9bb63 to c253cdf Compare October 9, 2023 19:44
@axelsccp
Merge branch 'main' into add-query-parameter-prometheus
1c21430 
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
@axelsccp
Sort changelog correctly
5e19038 
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
@axelsccp
Trigger pipeline
6ee8949 
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
@axelsccp
Remove whitespace
46c1fd8 
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
@axelsccp
Move change to improvement
0c1a4b5 
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
@axelsccp
Copy link
Contributor

axelsccp commented Oct 9, 2023

Hey @JorTurFer @zroubalik we've attended your sugestions and implemented the required tests and modifications, could you verify, please?

zroubalik
zroubalik reviewed Oct 10, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
pkg/scalers/prometheus_scaler.go Show resolved Hide resolved
@axelsccp axelsccp force-pushed the add-query-parameter-prometheus branch from 5d56c79 to 2fba509 Compare October 10, 2023 13:48
@axelsccp
Change improvement message
9be3bfb 
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
@axelsccp
Fix url query scaping
ef9ab02 
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
@axelsccp
Merge branch 'main' into add-query-parameter-prometheus
fcd2052
@zroubalik
Copy link
Member

zroubalik commented Oct 10, 2023 •
edited by github-actions bot

/run-e2e prometheus
Update: You can check the progress here

@zroubalik zroubalik merged commit 853fc61 into kedacore:main Oct 10, 2023
18 of 19 checks passed
toniiiik pushed a commit to toniiiik/keda that referenced this pull request Jan 15, 2024
@HenriquePiccolo @JorTurFer
@axelsccp @toniiiik
Add query parameters in prometheus scaler (kedacore#4957)
9833eb6 
Signed-off-by: Henrique Piccolo <henriquepiccolo@hotmail.com.br>
Signed-off-by: Axel Paschoa <apaschoabarres@gmail.com>
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Co-authored-by: Axel Paschoa <apaschoabarres@gmail.com>
Signed-off-by: anton.lysina <alysina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zroubalik zroubalik zroubalik left review comments

@axelsccp axelsccp axelsccp left review comments

@JorTurFer JorTurFer JorTurFer left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add queryParameters in prometheus-scaler
4 participants
@HenriquePiccolo @JorTurFer @axelsccp @zroubalik