-
Notifications
You must be signed in to change notification settings - Fork 165
Support custom prometheus registerer #37
Comments
Happy to take a PR :) |
Looks like this would require a major rewrite or force us to set a global instance which is exactly what I'm trying to avoid. For now my solution is to merge the global gatherer with a custom instance. But nevertheless it would be nice if global state could be avoided. |
This is already possible. Quick example that I threw together, that we should probably document 🙂
Similarly this exists for client metrics. |
Oh, nice, although it looks like it has never been released. Can we expect a new tag in the near future? |
@Bplotka how do you feel about a new release? I think I'd be ok with that, but would like to identify if there are any breaking changes, as I'm not too familiar with all the changes since the last release (it's been 1.5 years). |
Yea, let's review what we have and paste into some changelog maybe. Based on that we can tell if it deserves Can schedule some time for this during next days, but it would be manual process for me as well (: |
It seems like that we should review events which happend last time. A important thing shuold be discussed is "Revising our project README doc.", we can make it easier understand to freshmen. |
@Bplotka I was mainly referring to whether you are in the know of anything major that needs to be worked on. I should be able to reserve some time today to write up a change log and see whether that’s compatible with the latest release. |
@brancz If there will be a new major version may I suggest we take a look at the current static analysis/linter status of the package? Esp
FWIW there are other linter warnings reported by
I can contribute some fixes if we agree this is worthwhile. |
@knweiss @Bplotka I think that if we decide to fix those warnings, we should determine what is suitable check tool for our project. Because variety of check tools often report various warnings. |
I opened #43 to discuss the new release. Let me know what you think, and I encourage everyone to quickly double check my statement about backward compatibility to ensure we're not falsely pushing out a new minor version if it's not actually compatible. |
Closing here as non global registry is already possible. Everything regarding a new release is discussed in #43 |
Can i ask a question about global registerer instance? |
@brancz @sagikazarmark @Bplotka |
I've seen a number of projects that register global metrics (also libraries) that then pollute the global metrics library, the worst about this is that often you don't even know what information you expose. We prefer non-global registries because we want to know explicitly what we instrument (also note that registering things twice can cause panics). Also as an example: minikube in Kubernetes combines all components into a single binary, as this uses a global metrics registry, you get API metrics in the scheduler - very confusing. |
According to your explanations, i summarized some key points about why non-global registry at below.
Except above two points, is there anything missing? @brancz |
plus any article you can find on why globals should be avoided :) otherwise yes that's a good summary |
Yea, agree with @brancz ^^ You know @fengzixu , there is a good way to convince your colleagues:
Basically, you will get panic that will lead you for example to the first You will laugh if you will know how many times I needed to debug similar issues, for example when someone accidentally reused |
Your example for Golang http.DefaultServeMux is very suitable. Thanks for your explanations. I have convince my partners in our team. Thanks. @Bplotka |
@sagikazarmark you can try promgrpc it allows what you need. It simply implements |
@piotrkowalczuk just curious, what's the difference between this project and yours https://github.com/piotrkowalczuk/promgrpc ? (: |
@bwplotka Main differences:
|
How about we merge those things back into this code base and cut a 2.0? :) |
Generally - all of those makes sense to me, let's discuss all differences in separate Issue to not mislead by talking in this one (: |
Currently all metrics are exposed in the global registerer instance. Would be nice if one could provide a custom registerer instead of using a global one.
The text was updated successfully, but these errors were encountered: