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

More powerful and more customizable MultiError #820

Open
beorn7 opened this issue Dec 2, 2020 · 4 comments
Open

More powerful and more customizable MultiError #820

beorn7 opened this issue Dec 2, 2020 · 4 comments
Milestone

Comments

@beorn7
Copy link
Member

beorn7 commented Dec 2, 2020

The ad-hoc prometheus.MultiError implementation doesn't support the new goodies from the standard errors package. It also formats into a multi-line error string, which is undesired by some users.

Both could be solved by replacing it with something like https://github.com/hashicorp/go-multierror , which allows customizable formatting. However, it would break existing users and therefore has to be a v2 feature.

@beorn7
Copy link
Member Author

beorn7 commented Jan 25, 2021

Also, check what flavors of multi-error other parts of the Prometheus ecosystem are using.

@bwplotka
Copy link
Member

bwplotka commented Jan 25, 2021

I moved our Prometheus multi error to separate module so it's easy to use. The plan would be to use that on Promeheus too (if team agress). Also Thanos etc

It's here: https://github.com/efficientgo/tools/tree/main/core/pkg/merrors

Do you think it would be easier to just focus on one? I used same formatting. We could think of customizing formatting. I think can allow this. WDYT?

@beorn7
Copy link
Member Author

beorn7 commented Jan 25, 2021

To keep things in scope of this issue: We can only change the MultiError in client_golang when we bump the major version. That won't happen tomorrow, so I'd say just do whatever is best for the MultiError in a more general Prometheus (or even wider) context. Once we get to solving this issue here, we'll just check if it it a fit (which I assume it will).

@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