-
Notifications
You must be signed in to change notification settings - Fork 1.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
Please expose wrappingCollector
publicly
#1060
Comments
Hi, thanks for proposing this. It sounds what you try to achieve is similar to our attempt here: https://github.com/prometheus/client_golang/blob/cache/prometheus/cache/cache.go Do you mind providing us with more details? It might be useful to upstream that special collector if this will be useful for others!
Does it mean you want to change the |
Sorry, I think I wasn't clear about presenting my use case and it got confused with what I'm asking for. I don't want to cache metrics, and I don't want to change labels over time. I'm just using a cache (of some application-specific objects, not metrics) as an example of where we have nested tl;dr: If Detailed use case information We have a general-purpose "cache some value and refresh it every X interval" type, The problem arises when a program has multiple As If we did have a public c := prometheus.Wrap(cache, labels)
// ...
prometheus.MustRegister(c)
defer prometheus.Unregister(c) Or, again independently of registration details, |
I have another possible use case for this. We have a process worker pool controller. We'd like to use But that function needs to be wrapped with additional So we'd end up with something like this: for w := range workers {
c := prometheus.Wrap(
collectors.NewProcessCollector(...),
prometheus.Labels{"worker_id", w.ID},
)
prometheus.MustRegister(c)
} Not sure if this is valid, but that's what we're in need of. |
Nevermind, this turned out to be easier to do than I thought.
|
Hello 👋 Looks like there was no activity on this issue for the last 3 months. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 |
I'm still interested in this. We could probably improve our user-facing API a little after #1103, which would make it easier for the cache to maintain its own |
Closing for now as promised, let us know if you need this to be reopened! 🤗 |
Bad bot, why did you close the ticket even after I commented? I am still interested in this. |
We have a number of custom collectors that wrap other collectors - e.g. some cached value is periodically refreshed, where we want to collect metrics about the last refresh time, and also metrics about the value itself.
Currently for the outer collector to add a label or metric to the inner collector, we either need to reimplement
wrappingCollector
ourselves, or we need to also apply the labels to the outer collector when it's registered using e.g.WrapRegistererWith
, in turn leading to some ugly patterns likerepeated for every collector with different labels.
With a public
wrappingCollector
we could avoid this by either wrapping it at the layer between the outer and inner collector.The text was updated successfully, but these errors were encountered: