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

Division operator for native histograms #12250

Closed
krajorama opened this issue Apr 12, 2023 · 2 comments
Closed

Division operator for native histograms #12250

krajorama opened this issue Apr 12, 2023 · 2 comments

Comments

@krajorama
Copy link
Member

krajorama commented Apr 12, 2023

Proposal

Allow native histogram to be divided by a float (non zero). Also would make sense to allow the other operations to take place by going through the histogram and applying the operation on all components of the histogram.

Rationale:

When calculating rate/increase of native histograms, the result is component wise (sum, count, ...) operation. To be able to scale the output, we need the division.

Concrete problem

In Grafana Mimir we have query splitting by time. Which means running some queries in parts by splitting on time and then merging the result.

One particular case is:
sum(rate(native_histogram_series{}[3m]))

Here rate = increase / time, so you can rewrite the above to say (roughly for brevity):

sum( (increase over first minute + increase over second minute + increase over third minute) / 180 )

So you get partial increase and then divide by the total time (180s).
The only problem is that currently dividing the native histogram by number results in 0s. But if division worked as expected, we would get the correct results.

@beorn7 beorn7 added this to the Native Histograms milestone Apr 12, 2023
@beorn7
Copy link
Member

beorn7 commented Apr 13, 2023

"Scaling" histograms (via multiplication/division by a scalar) sounds like a reasonable thing to do. However, I vaguely remember that I hit some roadblocks when I wanted to implement it a while ago. But I cannot remember the details. Perhaps that was before we fully embraced the FloatHistogram… But since we now use FloatHistogram exclusively inside PromQL, it might be easy to do. Maybe the concern had to do with "negative" histograms, e.g. what happens if you do -1 * some_histogram. Again, with FloatHistogram not a problem, but it throws off counter reset detection. I guess usually you would only scale gauge histograms (e.g. a rated histogram, in fact, rate is already internally scaling the histogram), and we could issue a warning when creating "negative" counter histograms (cf. #12152). Once we are fine with negative histograms, we could also allow subtraction between histograms (because histogram_a - histogram_b is anyway equivalent to histogram_a + -1 * histogram_b).

@beorn7
Copy link
Member

beorn7 commented Jul 26, 2023

This has been implemented a while ago (including negative histograms).

@beorn7 beorn7 closed this as completed Jul 26, 2023
@prometheus prometheus locked as resolved and limited conversation to collaborators Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants