prometheus
feat(metrics/histogram): 🍪 `count()` and `sum()` accessors
fixes #241.
this commit introduces two new public methods to Histogram; sum() and count() return the sum of all observations and the number of observations made, respectively.
:wave: hello! this pull request in particular would be very helpful for our usage of Histograms. i would love if you could take a look at this soon, i believe it's a small and non-controversial addition to the Histogram interface.
I am hesitant exposing the internals of Histogram just for the use-case of testing. Unless we come up with better use-cases, I don't think we should merge here.
@mxinden I'm sympathetic to maintaining a minimal public API, but it's pretty important to us that we can write tests that ensure that our metrics are properly instrumented. For example:
https://github.com/linkerd/linkerd2-proxy/blob/ea5136268d65aa6a570cb612150875f923067042/linkerd/app/outbound/src/http/logical/policy/route/backend/metrics/tests.rs#L224-L237
Would you be amenable to feature-gating these public helpers behind a test-util feature? This is a pretty common approach we've seen in other crates.
Or are there alternative approaches you recommend? Would it be better to expose a module akin to github.com/prometheus/client_golang/prometheus/testutil to make assertions about a registry?
@mxinden I'm sympathetic to maintaining a minimal public API, but it's pretty important to us that we can write tests that ensure that our metrics are properly instrumented. For example:
https://github.com/linkerd/linkerd2-proxy/blob/ea5136268d65aa6a570cb612150875f923067042/linkerd/app/outbound/src/http/logical/policy/route/backend/metrics/tests.rs#L224-L237
Would you be amenable to feature-gating these public helpers behind a
test-utilfeature? This is a pretty common approach we've seen in other crates.Or are there alternative approaches you recommend? Would it be better to expose a module akin to
github.com/prometheus/client_golang/prometheus/testutilto make assertions about a registry?
I'm running into this same class of problems: Histogram does not provide an easy way to check the contents in a test. I have no way to test my code that updates histograms based on the result of some computation.
The way to extract data from histograms doesn't need to be performant, but it does need to exist. It could be a test-util that serializes to a string and then parses the result.
Would you be amenable to feature-gating these public helpers behind a
test-utilfeature? This is a pretty common approach we've seen in other crates.
That sounds reasonable to me. Contribution welcome. Thanks. Please point out very prominently that we might break any of those APIs in a patch release.
i've opened #283 to introduce a test-util feature. i can rebase this, and other proposals that received similar feeback, upon that if/when it has landed.