[enhancement]: Incorrect histograms?

[akeamc]

Which feature or improvement would you like to request?

In an attempt to monitor my mail server with Prometheus, I noticed the histograms behave a bit strange. Instead of incrementing the corresponding bucket by 1 on each observation, the bucket is incremented by the observed value:

mail-server/crates/trc/src/atomics/histogram.rs

Lines 42 to 47 in f4efa16

	for (idx, upper_bound) in self.upper_bounds.iter().enumerate() {
	if value < *upper_bound {
	self.buckets.add(idx, value);
	return;
	}
	}

Also, shouldn't the comparison be inclusive, per Prometheus specification?

• cumulative counters for the observation buckets, exposed as <basename>_bucket{le="<upper inclusive bound>"}

Is this really correct?

Is your feature request related to a problem?

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[mdecimus]

The observed value is added but as you can see in the code above the counter is incremented by one.
For example, if the histogram is measuring latencies, the current latency is added to the corresponding bucket and the counter is incremented by one.

[akeamc]

Yes, I'm with you on that. But isn't it still incorrect behavior to add the entire value to the bucket? The Prometheus tutorial specifies a count of observations, not a per-bucket sum.

[mdecimus]

You are correct, this was a bug and it has now been fixed.

[akeamc]

Thanks! Stalwart is a wonderful mail server, by the way.