-
Notifications
You must be signed in to change notification settings - Fork 1.2k
collectors/version: Allow custom additional labels #1860
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
collectors/version: Allow custom additional labels #1860
Conversation
7c65de7
to
8d75511
Compare
d1379a0
to
76fa0f3
Compare
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.
Thanks! Makes sense, just one suggestion to make it flexible for future options, WDYT?
// NewCollectorWithLabels returns a collector that exports metrics about current | ||
// version information and allows to set additional custom labels. | ||
func NewCollectorWithLabels(program string, labels prometheus.Labels) prometheus.Collector { |
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.
Wanna spend extra 10m and add proper options to this type? Will help in future changes (e.g. filtering some labels etc)
type Option func(*options)
type options struct {
extraConstLabels prometheus.Labels
}
func WithExtraConstLabels(l prometheus.Labels) Option {
return func(o *options) {
o.extraConstLabels = l
}
}
func NewCollector(program string, opts ...Option) prometheus.Collector {
o := option{}
for _, opt := range opts {
opt(&o)
}
//...
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 you want me to rename the func to NewCollectorWithOptions then or should I add the opts arg to NewCollector?
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.
Ah, understood, it's a variadic function now.
43cf50a
to
1439d34
Compare
@bwplotka Thanks for the suggestion, I applied you change. |
Co-Authored-by: bwplotka <bwplotka@gmail.com> Signed-off-by: Manuel Rüger <manuel@rueg.eu>
1439d34
to
a8cbb1f
Compare
We have a use case here, where it would be nice to support custom labels in the build info: kubernetes/kube-state-metrics#2739
This would allow us adding labels to the build info metric exposed by the version collector, e.g. in this case the version of the kubernetes client library we're using.
This also adds some tests for the version collector.