deadtrickster / prometheus.erl

Prometheus.io client in Erlang
MIT License
341 stars 117 forks source link

Weighted histogram #155

Closed x0id closed 10 months ago

x0id commented 1 year ago

The prometheus_histogram:observe_n/3,4,5 adds limited support for the weighted histograms. It accepts the extra argument "Weight" which is limited to integer numbers to fit existing bucket counters data type. It may be, for example, the number of time ticks when the recent value was observed, allowing for better accuracy when measurements are irregular.

In general, the weight could be any number, but this is difficult to implement since the overall Prometheus histogram model is based on the bucket counters concept, where the counters are expected to be non-negative integers. That's why the new function is limited to integer weights only, which is still useful in many applications.

saleyn commented 11 months ago

@deadtrickster , could you approve this one? It is a very useful patch.

deadtrickster commented 10 months ago

surprised by Value*Weight - Value expected to be averaged by caller or observe/n called as soon as Value changes? Weight means exact measurements count? Weights are confusing to me because of neural networks and other math stuff. Could it be Count? Why not to call existing observe on each observation?

x0id commented 10 months ago

surprised by Value*Weight - Value expected to be averaged by caller or observe/n called as soon as Value changes?

It may be averaged, or it may be instant. My case is second one. Anyway, the value is still used to calculate the bucket. Disregarding the weight.

Weight means exact measurements count? Weights are confusing to me because of neural networks and other math stuff. Could it be Count?

I don't care of the naming. It could be named count. But the weight is a generalization of count, sorry for math again :)

Why not to call existing observe on each observation?

Because to keep the intention, I would have to call observe N times for a single observation. Remember, we deal with histogram, not the gauges. The values is mapped into the bucket number (or the bucket interval, strictly saying). And I need it to be counted in the bucket more than one time per observation.

ikavgo commented 10 months ago

In prometheus histograms have counts, not weights.

ok, now it's more clear that you have just single event that comes with value and count. I'm ok merging it iff weights renamed to counts and @doc strings are very clear what value ends up in the bucket - observed or observed * count and why/

x0id commented 10 months ago

I'm ok merging it if weights renamed to counts and @doc strings are very clear what value ends up in the bucket - observed or observed * count and why/

Renamed weights to counts, fixed the doc. Please let me know if anything else should be corrected.

ikavgo commented 10 months ago

Great! I think commits must be signed

x0id commented 10 months ago

Great! I think commits must be signed

Is this your requirement? I've never signed my commits before. I don't even have a GPG/PGP or other signature. Why is this important?

deadtrickster commented 10 months ago

It's a requirement for this repo, yes. I think it was made so on request of somebody wanting to use in their org. Years ago

x0id commented 10 months ago

Commits signed.

deadtrickster commented 10 months ago

❤️