-
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
Publish Prometheus metrics about build info #4576
Publish Prometheus metrics about build info #4576
Conversation
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, nice addition - Thanks! Would you mind adding a test for it please, or do we not have tests for metrics yet @JorTurFer?
a99edb9
to
d179e5f
Compare
I am on the process of adding a test, that is why I left on draft. But running e2e tests on my computer is a PITA. Working on it. |
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.
The test can be added here: https://github.com/kedacore/keda/blob/main/tests/sequential/prometheus_metrics/prometheus_metrics_test.go
You can just run a specific test, if you have k8s cluster ready with the KEDA build deployed: https://github.com/kedacore/keda/tree/main/tests#specific-test
1d7bf5c
to
49af364
Compare
Can I change somehow the deploy to use the an image built from my version of the code? Otherwise the test will never pass, since it is running against |
you can run |
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.
I have some additional thoughts :)
I think I am all set now. |
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.
Great job, could you please fix also this. I will, trigger e2e then! 🙏
/run-e2e sequential |
Oh boy, I need to update the assertions. |
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>
Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>
Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>
Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>
Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>
Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>
Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>
Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>
Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>
Signed-off-by: Pedro Tanaka <pedro.stanaka@gmail.com>
a42acbb
to
50ce7c3
Compare
Can one of the maintainers enqueue another e2e run? cc @zroubalik @JorTurFer Thanks! |
/run-e2e sequential |
/run-e2e sequential |
@JorTurFer Any thoughts why |
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.
We should also document the metric:
https://keda.sh/docs/2.11/operate/prometheus/
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
Head branch was pushed to by a user without write access
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 your patience with all the rounds of review :)
Docs PR: kedacore/keda-docs#1142 |
Summary
I am adding a new metric that I missed while doing my monitoring setup for Keda. This new metric will tell which version of Keda is running in a given Kubernetes cluster. This is really useful when managing multiple clusters to do spot check of which version of Keda is running.
Checklist
Fixes #
Relates to #4647