Reworks the prometheus metrics to adhere to best practices
https://github.com/kedacore/keda/issues/4854
I reworked the prometheus metric names to follow conventions: added units where needed, and appended total.
Also passed a time.Duration down to the metrics handlers, as the conversion to units needs to happen when construction the metrics.
No tests were present, none added.
No idea where to track deprecation actions (when and how will the deprecated metrics be removed?)
Checklist
- [X] I have verified that my change is according to the deprecations & breaking changes policy
- [ ] Tests have been added
- [X] Changelog has been updated and is aligned with our changelog requirements
- [X] Commits are signed with Developer Certificate of Origin (DCO - learn more)
Fixes #4854
Relates to https://github.com/kedacore/keda-docs/pull/1258
Thank you for your contribution! 🙏 We will review your PR as soon as possible.
While you are waiting, make sure to:
- Add an entry in our changelog in alphabetical order and link related issue
- Update the documentation, if needed
- Add unit & e2e tests for your changes
- GitHub checks are passing
- Is the DCO check failing? Here is how you can fix DCO issues
Learn more about:
updated the e2e tests, didn't add anything for testing for the deprecation notice, seems silly to me to rework.
also made the needed changes in the webhook, as I only spotted those when going through the e2e tests.
not able to run the e2e tests locally, fingers crossed this passes
tests fail, but I don't think it's the part I was fiddling with. Also, the ending newline in the json - no idea what's the idea there, my VSCode removes the newline, and a vim-edit seems to add one, but then it's one too many ...
/run-e2e sequential Update: You can check the progress here
fixed the DCO and the newline - seems to be all green on those. E2E is out of my league ;-)
hey @wonko, would you be available to address the merge conflicts? I can see these files are now unable to merge.
both modified: CHANGELOG.md
both modified: pkg/metricscollector/metricscollectors.go
both modified: pkg/metricscollector/opentelemetry.go
both modified: pkg/metricscollector/prommetrics.go
both modified: pkg/scaling/scale_handler.go
both modified: tests/sequential/opentelemetry_metrics/opentelemetry_metrics_test.go
both modified: tests/sequential/prometheus_metrics/prometheus_metrics_test.go
hey @wonko, do you think you will be able to find some time to resolve the conflicts? It's ok if not, I can help out to cherry-pick your work and follow up in a new PR.
@wozniakjan Been busy with some other stuff lately, but I could pick this up either this week, or this weekend latest.
no pressure @wonko, I'm available at your convenience. I just think this is really solid work and it would be great to get this merged in the upcoming months :)
no pressure @wonko, I'm available at your convenience. I just think this is really solid work and it would be great to get this merged in the upcoming months :)
@wozniakjan I think it's good now ... please have a good look at it, as I did the merge with a lot of interruptions ...
/run-e2e sequential Update: You can check the progress here
hey @wonko, the e2e tests seem to fail with metrics registration error
2024/02/12 22:24:44 maxprocs: Updating GOMAXPROCS=1: determined from CPU quota
panic: a previously registered descriptor with the same fully-qualified name as Desc{fqName: "keda_scaler_errors_total", help: "The total number of errors
encountered for each scaler.", constLabels: {}, variableLabels: {namespace,metric,scaledObject,scaler,triggerIndex,type}} has different label names or a different
help string
goroutine 1 [running]:
github.com/prometheus/client_golang/prometheus.(*Registry).MustRegister(0x172ccd8?, {0xc000084e80?, 0x1, 0xc000c9f5d0?})
/workspace/vendor/github.com/prometheus/client_golang/prometheus/registry.go:405 +0x78
github.com/kedacore/keda/v2/pkg/metricscollector.NewPromMetrics()
/workspace/pkg/metricscollector/prommetrics.go:233 +0x30e
github.com/kedacore/keda/v2/pkg/metricscollector.NewMetricsCollectors(0xd?, 0x1)
/workspace/pkg/metricscollector/metricscollectors.go:79 +0x25
main.main()
/workspace/cmd/operator/main.go:148 +0xb3f
can you please take a look at it? The error location could mean it's related to the changes.
Sure thing, I can investigate tomorrow.