Reworks the prometheus metrics to adhere to best practices

[wozniakjan]

This is continuing work from #5174. I have rebased on main, resolved conflicts and working on fixing the e2e bug as described in #5174 (comment).

Original text from @wonko

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?)

There are two parts to this PR

• original work from #5174 squashed and rebased on latest main - 6e5f45c
• changes from this PR to ensure no original metric gets broken - 31dd060, d2ee08f, c60020b, 25783ee

Checklist

• I have verified that my change is according to the deprecations & breaking changes policy
• Changelog has been updated and is aligned with our changelog requirements
• A PR is opened to update the documentation on (repo) - keda-docs#1374
• Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #4854
Closes #5174
Relates to kedacore/keda-docs#1374

[wozniakjan]

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

[wozniakjan]

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

[wozniakjan]

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

[wozniakjan]

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

[wozniakjan]

• ced62ff contains original work from Reworks the prometheus metrics to adhere to best practices #5174
• e8d3c37 fixes the duplicate metric registration for keda_scaler_error_total
• 30e16b9 adds back scaled_job_errors with proper deprecation message

[wozniakjan]

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

[wonko]

thx for picking this up @wozniakjan

[wozniakjan]

hmm, now the e2e test failure is odd

 75     opentelemetry_metrics_test.go:720:
 76             Error Trace:    /__w/keda/keda/tests/sequential/opentelemetry_metrics/opentelemetry_metrics_test.go:720
 77                                         /__w/keda/keda/tests/sequential/opentelemetry_metrics/opentelemetry_metrics_test.go:458
 78             Error:          Should be true
 79             Test:           TestOpenTelemetryMetrics
 80             Messages:       keda_internal_scale_loop_latency not available

but I can observe both keda_internal_scale_loop_latency as well as keda_internal_scale_loop_latency_seconds

$ curl localhost:8080/metrics 2>/dev/null | grep keda_internal_scale_loop_latency
# HELP keda_internal_scale_loop_latency DEPRECATED - will be removed in 2.16: use 'keda_internal_scale_loop_latency_seconds' instead.
# TYPE keda_internal_scale_loop_latency gauge
keda_internal_scale_loop_latency{namespace="default",resource="php-apache-deployment-2",type="scaledobject"} 1
# HELP keda_internal_scale_loop_latency_seconds Total deviation (in seconds) between the expected execution time and the actual execution time for the scaling loop.
# TYPE keda_internal_scale_loop_latency_seconds gauge
keda_internal_scale_loop_latency_seconds{namespace="default",resource="php-apache-deployment-2",type="scaledobject"} 0.001352704

can't even get this to reproduce locally, tests just pass on every run

IIMAGE_REGISTRY=docker.io MAGE_REPO=wozniakjan E2E_TEST_REGEX=sequential ENABLE_OPENTELEMETRY=true make publish deploy e2e-test-local

...
Executing tests/sequential/opentelemetry_metrics/opentelemetry_metrics_test.go, attempt "one"
Execution of tests/sequential/opentelemetry_metrics/opentelemetry_metrics_test.go, attempt "one" has passed
...

[wozniakjan]

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

[wozniakjan]

tests finally pass, ptal @kedacore/keda-core-contributors

[tomkerkhove]

@wozniakjan It looks like kedacore/keda-docs#1258 is not owned by you, but linked. Is this one still up to date or do we need another one?

[wozniakjan]

It looks like kedacore/keda-docs#1258 is not owned by you, but linked. Is this one still up to date or do we need another one?

correct, kedacore/keda-docs#1258 was implemented by @wonko just like the majority of this PR originates in @wonko's PR #5174

There are a couple of minor suggestions in kedacore/keda-docs#1258, perhaps I can open a new docs PR that reflects reviewer suggestions.

We'll need a new issue for this; if I understand correctly we are just renaming them and keep the old ones.

correct, the old ones remained, just in the description KEDA will inform users these are deprecated. The new ones should be in alignment with prometheus conventions.

[wozniakjan]

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

[zroubalik]

There are a couple of minor suggestions in kedacore/keda-docs#1258, perhaps I can open a new docs PR that reflects reviewer suggestions.

if these suggestions are the only additions we can merge them in the existing PR

[zroubalik]

There are a couple of minor suggestions in kedacore/keda-docs#1258, perhaps I can open a new docs PR that reflects reviewer suggestions.

if these suggestions are the only additions we can merge them in the existing PR

oh, I can see the PR is opened against 2.13, we should move that to 2.14. @wozniakjan could you please open a new doc PR then?

[wozniakjan]

hey @zroubalik, with the docs PR kedacore/keda-docs#1374 approved, is there anything else I should do here or is this PR good to merge?