馃悰 remove deprecated workqueue metrics with invalid names #437
Conversation
|
Welcome @abursavich! |
k8s-ci-robot
added
do-not-merge/invalid-commit-message
k8s-ci-robot
added
size/L
abursavich
changed the title
abursavich
changed the title
abursavich
changed the title
abursavich
changed the title
pkg/metrics/workqueue.go
Outdated
| ConstLabels: prometheus.Labels{"name": name}, | ||
| }) | ||
| if err := Registry.Register(depth); err != nil { | ||
| log.Error(err, "failed to register "+metricName+" metric", "name", name) |
There was a problem hiding this comment.
error messages should never be variable -- that's what the key-value pairs are for
There was a problem hiding this comment.
i'm happy to change them if you'd like, but the messages are actually constant expressions (metricName is a local constant in each of the methods)
|
Thanks for the review, @DirectXMan12. PTAL. |
|
/lgtm |
k8s-ci-robot
added
lgtm
|
The test failure was fixed with #466 ... I'll push again to retrigger Travis. |
|
After rebasing and picking up 17e0976, Github really doesn't want me to push https://github.com/kubernetes-sigs/controller-runtime/blob/master/.github/main.workflow to my repo.
|
k8s-ci-robot
removed
the
lgtm
|
I cherrypicked the race fix without the Github Action change, but, uhhh... I don't know how to get around this Github issue. If this PR goes through, I might try deleting my repo and re-forking post-Action commit. |
|
That's a bizarre message. Are you not using a normal git client? Keep me updated on that issue -- if it's causing you issues, it might cause others issues, so we may need to figure out a way around it or back it out. |
Migration work was already done to move controller-runtime from k8s v1.13 to v1.14, however this introduced certain issues. Unfortunately, workqueue_queue_duration_seconds and workqueue_work_duration_seconds were included in v0.1.10, but were recorded with microsecond values. Moving to k8s v1.14 corrects those values to seconds. Primarily this change removes the metrics that were deprecated in k8s v1.14, but were never previously included in v0.1 of controller-runtime. The one metric that was deprecated and is retained is longest_running_processor_microseconds, as it was included in v0.1.10.
|
It let me push it. I've rebased up to master, so this should work now. Will merge once tests pass. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abursavich, DirectXMan12 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Migration work was already done to move controller-runtime from k8s v1.13 to v1.14, however this introduced certain issues.
Unfortunately, workqueue_queue_duration_seconds and workqueue_work_duration_seconds were included in v0.1.10, but were recorded with microsecond values. Moving to k8s v1.14 corrects those values to seconds.
Primarily this change removes the metrics that were deprecated in k8s v1.14, but were never previously included in v0.1 of controller-runtime and thereby fixes #436.
The one metric that was deprecated and retained is longest_running_processor_microseconds, as it was included in v0.1.10.