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.

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

Merged
merged 13 commits into from
Jun 5, 2023

Conversation

pedro-stanaka
Copy link
Contributor

@pedro-stanaka pedro-stanaka commented May 25, 2023

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

Copy link
Member

@tomkerkhove tomkerkhove left a 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?

@pedro-stanaka
Copy link
Contributor Author

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.

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.

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

pkg/util/build_info.go Outdated Show resolved Hide resolved
@pedro-stanaka
Copy link
Contributor Author

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

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 ghcr.io/kedacore/keda:main

@pedro-stanaka pedro-stanaka marked this pull request as ready for review June 1, 2023 12:59
@pedro-stanaka pedro-stanaka requested a review from a team as a code owner June 1, 2023 12:59
@zroubalik
Copy link
Member

zroubalik commented Jun 1, 2023

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

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 ghcr.io/kedacore/keda:main

you can run make publish && make deploy just change the image/registry in the Makefile. These commands will build and deploy KEDA on your cluster. Then just run this test.

https://github.com/kedacore/keda/blob/main/Makefile#L18-L19

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.

I have some additional thoughts :)

cmd/adapter/main.go Outdated Show resolved Hide resolved
pkg/prommetrics/prommetrics.go Outdated Show resolved Hide resolved
pkg/prommetrics/prommetrics.go Show resolved Hide resolved
@pedro-stanaka
Copy link
Contributor Author

I think I am all set now.

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.

Great job, could you please fix also this. I will, trigger e2e then! 🙏

cmd/adapter/main.go Outdated Show resolved Hide resolved
pkg/prommetrics/prommetrics.go Outdated Show resolved Hide resolved
@zroubalik
Copy link
Member

zroubalik commented Jun 1, 2023

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

@zroubalik zroubalik requested a review from JorTurFer June 1, 2023 16:01
@pedro-stanaka
Copy link
Contributor Author

Oh boy, I need to update the assertions.

pedro-stanaka and others added 11 commits June 2, 2023 17:24
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>
@pedro-stanaka
Copy link
Contributor Author

Can one of the maintainers enqueue another e2e run? cc @zroubalik @JorTurFer

Thanks!

@JorTurFer JorTurFer closed this Jun 2, 2023
@JorTurFer JorTurFer reopened this Jun 2, 2023
@JorTurFer
Copy link
Member

JorTurFer commented Jun 2, 2023

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

@JorTurFer
Copy link
Member

Please ignore this:
image

Sometimes I'm like a monkey...
image

@zroubalik zroubalik enabled auto-merge (squash) June 2, 2023 15:55
@tomkerkhove
Copy link
Member

tomkerkhove commented Jun 5, 2023

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

@tomkerkhove
Copy link
Member

@JorTurFer Any thoughts why e2e tests is still pending?

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.

We should also document the metric:
https://keda.sh/docs/2.11/operate/prometheus/

CHANGELOG.md Outdated Show resolved Hide resolved
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
auto-merge was automatically disabled June 5, 2023 11:39

Head branch was pushed to by a user without write access

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.

LGTM

Thanks for your patience with all the rounds of review :)

@pedro-stanaka
Copy link
Contributor Author

We should also document the metric: https://keda.sh/docs/2.11/operate/prometheus/

Docs PR: kedacore/keda-docs#1142

@zroubalik zroubalik changed the title Publish build info metric from all major components Publish Prometheus metrics about build info Jun 5, 2023
@zroubalik zroubalik merged commit 3cbba26 into kedacore:main Jun 5, 2023
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants