feat: add node_readiness_build_info metric - #406
Conversation
✅ Deploy Preview for node-readiness-controller canceled.
|
|
Hi @rawadhossain. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| Name: "node_readiness_build_info", | ||
| Help: "Build information for the node-readiness-controller binary.", | ||
| }, | ||
| []string{"version"}, |
There was a problem hiding this comment.
Pls add a git_commit label here []string{"version", "git_commit"}
Prometheus standards require the commit hash on build metrics
There was a problem hiding this comment.
I kept the metric scoped to the binary version since the commit is already exposed in the startup log.
There was a problem hiding this comment.
Looks like there's an idiomatic way to report this: prometheus/client_golang#693
There was a problem hiding this comment.
Thanks for pointing this out, the example helped. Updated it.
There was a problem hiding this comment.
The other identifying labels from prometheus example (eg: revision / goversion) are useful and could be extended as labels here, but I'm not blocking this CL on this, you could followup on this after -
https://github.com/prometheus/common/blob/2fb5a7cecbffd4bf7dc88ff22712112e8cef8432/version/info.go#L42
There was a problem hiding this comment.
Thanks. I’ll do follow-up on this if needed to extend going ahead.
9ff9d5e to
9689855
Compare
| Name: "node_readiness_build_info", | ||
| Help: "Build information for the node-readiness-controller binary.", | ||
| }, | ||
| []string{"version"}, |
There was a problem hiding this comment.
Looks like there's an idiomatic way to report this: prometheus/client_golang#693
9689855 to
5925c28
Compare
|
/lgtm |
5925c28 to
648bf7c
Compare
|
@ajaysundark added the tests. Should be good to merge. |
Signed-off-by: Rawad Hossain <rawad.hossain00@gmail.com>
648bf7c to
8665be6
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ajaysundark, rawadhossain The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/ok-to-test |
Description
Adds
node_readiness_build_infometric from observability design docChanges
node_readiness_build_infogauge with aversionlabel.-ldflagsin the Makefile and Dockerfile.Implementation choices
versionlabel, while keeping commit information in the existing startup version log.-ldflagswiring because the existinginternal/infoversion variables were not being populated during builds, which otherwise left the version asunknown.Related to Issue #182
Type of Change
/kind feature
Verification
Checklist
make testpassesmake lintpasses