-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add build info metric #1226
Add build info metric #1226
Conversation
9e93c58
to
255cd5d
Compare
pkg/version/version.go
Outdated
@@ -18,6 +18,9 @@ import ( | |||
"os" | |||
"path/filepath" | |||
"runtime" | |||
|
|||
"github.com/prometheus/client_golang/prometheus" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's not add these imports to the version
pkg
pkg/version/version.go
Outdated
@@ -62,3 +65,18 @@ func GetVersion() Version { | |||
Platform: fmt.Sprintf("%s/%s", runtime.GOOS, runtime.GOARCH), | |||
} | |||
} | |||
|
|||
// RegisterBuildInfo registers the build info metric. | |||
func RegisterBuildInfo(r prometheus.Registerer) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can move this method to main.go instead. In main.go
, we can call version.GetVersion()
, get the version struct and use its fields to instantiate the Gauge vector in main.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thanks! I will update it.
Thanks so much for this PR @yeya24 . Provided my comments |
Thanks for the PR, I already started the kube-state-metrics alpha 2.0 release, so might need to wait a bit longer until that is cut. Hope that's okay. :) |
I just updated the pr, please take a look again @tariq1890 Hi @lilic, no problem. Thanks for letting me know 👍 |
Thank you for the PR. This new metrics need to be added to docs also? I'm not sure. |
Yes, you are totally right. Thank you! Let me update the pr. |
@yeya24 I changed the base to release-2.0, do you mind also adding the docs to the README like we have for other ksm own metrics and we can merge afterwards, thanks! :) |
func registerBuildInfo(r prometheus.Registerer) { | ||
v := version.GetVersion() | ||
promauto.With(r).NewGaugeFunc(prometheus.GaugeOpts{ | ||
Name: "kube_state_metrics_build_info", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need it to be "kube_state_metrics_build_info"
, what about just build_info
? We have the job label to specify that it pertains to kube-state-metrics right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the full name kube_state_metrics_build_info
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yeya24 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Sorry for the late, I just updated the doc. Please take a look. |
Currently right in the middle of cutting the next release, but we can merge straight after that. Bad timing, sorry :) |
Co-authored-by: Lili Cosic <cosiclili@gmail.com>
README.md: Update section on container images
Fix pod-metric missing reasons
fix typo in node-metrics.md
Fix some typos
…lpha.1 and 2.0.0-alpha.2
…792) Signed-off-by: Denys Havrysh <denys.gavrysh@gmail.com>
Currently, Get, List and Watch are required. List and Watch are provided by the ClusterRole related to collectors. The STS collector can be disabled breaking the current AutoSharding role permissions. Moreover, the resourceNames depends of the RELEASE_NAME as defined in deployment.yaml so we need to reflect this or we won't have the correct permission to read our STS. Signed-off-by: Mathis Raguin <mathis@cri.epita.fr>
…850) * added resource config example as its mentioned in the readme Signed-off-by: André Bauer <monotek23@gmail.com> * added newline Signed-off-by: André Bauer <monotek23@gmail.com> * updated chart version Signed-off-by: André Bauer <monotek23@gmail.com> * removed blank line Signed-off-by: André Bauer <monotek23@gmail.com> * updated version Signed-off-by: André Bauer <monotek23@gmail.com>
Signed-off-by: Juan Pablo Jaramillo Pineda <juan@pablox.io>
… (#23380) * Added kubeconfig arg and secret Signed-off-by: Mike Ritter <mike.ritter@target.com> * Updated secret name Signed-off-by: Mike Ritter <mike.ritter@target.com> * Updated secret name Signed-off-by: Mike Ritter <mike.ritter@target.com> * Removed trailing spaces Signed-off-by: Mike Ritter <mike.ritter@target.com> * Removed XXX as default secret Signed-off-by: Mike Ritter <mike.ritter@target.com>
…er Nov 13 2020 Signed-off-by: Scott Rigby <scott@r6by.com>
Signed-off-by: Scott Rigby <scott@r6by.com>
…). Requires configuring a gh-pages branch before merge to master Signed-off-by: Scott Rigby <scott@r6by.com>
Signed-off-by: Manuel Rüger <manuel@rueg.eu> Co-authored-by: Scott Rigby <scott@r6by.com>
Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>
internal/store: fix high cyclo complexity
…d from. Chart commit history original source: https://github.com/helm/charts/tree/master/stable/kube-state-metrics For context about chart reloation, see "[stable] relocate charts to new repos" https://github.com/helm/charts/issues/21103 Signed-off-by: Scott Rigby <scott@r6by.com>
- kubernetes#1237 (comment) - kubernetes#1237 (comment) Signed-off-by: Scott Rigby <scott@r6by.com>
Note we may want to add GPG signing in a follow-up PR. Signed-off-by: Scott Rigby <scott@r6by.com>
Host stable/kube-state-metrics helm chart
Latest version of chart-releaser action is [v1.1.0](https://github.com/helm/chart-releaser-action/releases/tag/v1.1.0) Signed-off-by: Torsten Walter <torsten.walter@syncier.com>
The test-unit Makefile rule had a dependency on the clean and build rules. However, these rules aren't needed to run kube-state-metrics unit tests. The original rule can be problematic when running the unit tests in an environment without docker installed. Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>
Makefile: remove test-unit dependencies
helm chart: Fix GitHub action
Signed-off-by: Scott Rigby <scott@r6by.com>
Signed-off-by: Scott Rigby <scott@r6by.com>
…e-template Chart releaser name template and release
Signed-off-by: Ben Ye <yb532204897@gmail.com>
465f098
to
0e319bd
Compare
Keywords which can automatically close issues and at(@) or hashtag(#) mentions are not allowed in commit messages. The list of commits with invalid commit messages:
Instructions 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/test-infra repository. I understand the commands that are listed here. |
Will close a new pr against master. Sorry for the late. |
Signed-off-by: Ben Ye yb532204897@gmail.com
What this PR does / why we need it:
This pr adds the
kube_state_metrics_build_info
metric as required by #1132Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #1132