Skip to content
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

Consider eliminating MustRegister, moving global registry to promauto. #715

Open
beorn7 opened this issue Feb 13, 2020 · 2 comments
Open
Milestone

Comments

@beorn7
Copy link
Member

beorn7 commented Feb 13, 2020

With the additions to the promauto package in #713 , the need for MustRegister is diminished: Whoever doesn't care about panicking, can and should use promauto constructors, no matter if they want the local or the global registry. Whoever wants to play safe, won't use MustRegister anyway. With the notable exception of custom collectors. One might argue that those are rare enough to get registered with the normal Register call and subsequent error handling (even if it is just a panic(err)). This would have the advantage of making the Registerer interface leaner.

Another consideration is that users of the global registry have gravitated towards promauto for a while now. So perhaps it's better to have the global registry in promauto, perhaps with a Register top-level function there for use with custom collectors.

This would pave the road for a registry package, which was discussed before, so that registry concerns would be all in that package, making the prometheus package much cleaner

@beorn7 beorn7 added this to the v2 milestone Feb 13, 2020
@beorn7 beorn7 self-assigned this Feb 13, 2020
@beorn7 beorn7 changed the title Consider eliminate MustRegister, move global registry to promauto. Consider eliminating MustRegister, moving global registry to promauto. Feb 13, 2020
@bwplotka
Copy link
Member

Hm, I like the promauto in a way that making this part auto-registered and panicking and removing panicking part from prometheus seems like a clean and reasonable separation. 👍

@beorn7
Copy link
Member Author

beorn7 commented Jun 2, 2021

I mostly assigned this to myself as a “default” because I used to be the maintainer of this repo. Therefore, I'm un-assigning this from myself no. New maintainers @bwplotka & @kakkoyun, please deal with this as you see fit.

@beorn7 beorn7 removed their assignment Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants