promql: implement integral() function#14440
Conversation
integral functionintegral() function
|
CC @beorn7 because he usually has an opinion on anything math-y (no idea yet if we would want to add this function) :) |
Thanks @juliusv !, just look at this awesome example I just added to the PR desc and examples.md (then try to get back time with existing functions, with a non-contrived promQL ;) -> For those metrics that produce |
|
I have this on my (long) review queue and have a detailed look ASAP. At first glance, this looks like avg_over_time with proper weight, which is something I always wanted. On the other hand, there are a million concerns in my head (not only those raised in the linked issues and discussions). Just as a disclaimer ahead of things that I generally would like something like this and are generally positive, but I have to run this against a number of criteria in detail. |
33fa69e to
ff4aa52
Compare
dff8c4f to
528d281
Compare
|
We have reached the beginning of my (unplugged) vacations. I'll come back to this when I'm back (~2w), but feedback from other domain experts would be very welcome in the meantime. |
9351300 to
6235ca2
Compare
Ref: some previous discussion at prometheus#1335 and https://groups.google.com/g/prometheus-developers/c/3EuF6ffNt_k. \## What Implement `integral(v range-vector, strategy=2 scalar)` function. \## Why There are use cases where `sum_over_time()` or `avg_over_time()` multiplied by the length of the whole interval are cumbersome to use (need to factor the length of the interval and/or sampling period), or plainly N/A (e.g. if you had periods of uneven scraping intervals). Couple relevant use cases (added to examples.md): * Calculate the total cost over past week given the per-node hourly cost: integral(hourly_cost[7d]) / 3600 * Calculate the total energy consumed in Joules over last day, given the instant power consumption in Watts: integral(power_rate[1d]) \## How Formula-wise the implementation is mostly straightforward, but details are important: * implements three strategies to calculate the integral via trapezoidal area aggregation: mid-rule (aka _Riemann_'s, default for strategy=2), left-wise (strategy=0) and right-wise (strategy=1); this is done to let the user choose the best one to use depending on their value "alignment" with the integrated interval. * there's specific _NaN_ handling (mapped to zero) to simplify handling of the above strategies, mid-rule in particular. * result needs to support being wrapped inside another aggregate like `sum_over_time()`, i.e. interval limit handling is very important to avoid double-aggregating these cases. \## Testing Extensive testing cases added to `functions.test` \### Strategy Excerpt from functions.go: ``` // Discrete integral using simple (trapezoidal rule). switch strategy { case 0: // Left-wise rectangle rule. // // metric // ^ // | // | v1 // | *........>. v4 // | |#########: *........>. // | |######## v2 | : // | |######## *........>. | : // | |#########|#########: | : // | |#########|#########: | : // | |#########|###### (NaN)......>| (NaN) // +---------+---------+---------+---------+---------+-------> t // t1 t2 t3 t4 t5 // [<--------------------------->) // // integral(metric)[] @t4: + + + // v1 v2 0 // * * // (t2-t1) (t3-t2) value = prevVal case 1: // Right-wise rectangle rule. // // metric // ^ // | // | v1 // |:<.......* v4 // |: | .<........* // |: | v2 :#########| // |: |<........* :#########| // |: |#########| :#########| // |: |#########| :#########| // |: |#########|<......(NaN) ######|<......(NaN) // +---------+---------+---------+---------+---------+-------> t // t1 t2 t3 t4 t5 // (<--------------------------->] // // integral(metric)[] @t4: + + + // v2 0 v4 // * * // (t2-t1) (t4-t3) value = currVal case 2: // Mid-point rule (default). // With NaN as zero, "neighboring" non-zero values are // aggregated (halved at each interval eval). // // metric // ^ // | // | v1 // | ....>*<.... v4 // | : |####: ....>*<.... // | : |####: v2 :####| : // | : |####:...>*<.... :####| : // | : |####:####|####: :####| : // | : |####:####|####: :####| : // | : |####:####|####:.>(NaN)<.:####| :.>(NaN) // +---------+---------+---------+---------+---------+-------> t // t1 t2 t3 t4 t5 // [<--------------------------->] // // integral(metric)[] @t4: + + + // (v1+v2)/2 (v2+0)/2 (0+v4)/2 // * * * // (t2-t1) (t3-t2) (t4-t3) // if prevVal != 0 || currVal != 0 { value = (currVal + prevVal) / 2 } } ``` Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
* use _left-point_ and _right-point_ to document `strategy=0,1` better idiom (I guess?) also matching well-known _mid-point_ wording * add `integral(absent(metric_name)[1d:5m])` example \m/ * `s/Joules/kWh/` (PR description and `examples.md`), as it's more familiar and likely SEO friendlier ;) Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
…en fix integral(metricbignum, 2) case to give a small non-zero number \o/ Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
…> `[1m:200ms]`, also adjust eval range parameters to only go thru period once (1m to 2m step 10s) Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
…c integral case Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
Signed-off-by: JuanJo Ciarlante <juanjosec@gmail.com>
6235ca2 to
d0143e0
Compare
beorn7
left a comment
There was a problem hiding this comment.
Tiny code comment. More high-level comment will follow soon.
| } | ||
|
|
||
| return aggrOverTime(vals, enh, func(s Series) float64 { | ||
| var sum, c float64 |
There was a problem hiding this comment.
Maybe c → cSum for consistency with value, cValue below.
|
First of all, thanks for the high-quality PR with all the test cases, comprehensive documentation, and thoughtful explanations. I have a bunch of considerations at various levels of detail. Let's start with the most simple one: This needs to be behind the The next one is about UX: The strategy argument is quite a source of confusion, first of all in terms of readability. As every code, PromQL is more often read than written, and a reader seeing Which gets us to the use cases. Let's iterate through the examples you have given above.
I think Similarly, I would ask where the I think the true magic of As hinted at above, I always wanted a "weighted" version of One interesting byproduct of the sub-query approach is that it deals quite well with a series disappearing or springing into existence during the interval, especially if it happens multiple times, mostly because it implicitly takes staleness into account. The behavior of the implementation in this PR is handling this in a possibly problematic way. I would need ASCII art now or a photograph of my whiteboard for a proper explanation but let's try: If the This is just the current state of my stream of consciousness. My semi-educated gut feeling currently tells me we might just be better off with sub-queries as a work around because it expresses pretty clearly what we are doing in terms of existing PromQL primitives. It's a bit clunky but doesn't come with surprises for PromQL experts. The points against this gut feeling are maybe the following: If the use case is more than just niche, we might argue that it should not rely on something like a work-around via sub-queries. What would have even more significance (IMHO) would be if we actually needed "more magic" to handle the case of radically different scrape intervals and series dropping in and out, i.e. something we could not emulate with sub-queries. But for that, the current implementation needed more tweaks in any case. Note that the current implementation of I'll stop her, as said, this is incomplete and in no way conclusive. Looking forward to further thoughts. |
Makes sense, but just note range vector selectors currently don't get populated with any staleness markers (staleness markers are only considered for instant vector selectors, and that's it), so this would have to change in the engine. And then all other functions which currently rely on range vectors not containing staleness markers would have to take them into account. |
|
Yeah, that's kind of what troubles me. I think for rate calculation, it's deliberate to not take staleness markers into account. As said above, that's the beauty of counters. We just check the count of events. We don't care if the series stopped to exist in the middle of the range. (We do care about starting after the beginning and stopping before the end of the range. And for that, we have a heuristic in place. And it's consistent with how it handles both ends. We could utilize a staleness marker for the end, and we could even use it to refine our heuristics to calculate the average sample interval. However, I wouldn't apply those subtle changes right now. I'd rather include those consideration into the pending re-thinking of the whole rate calculation (cf. "xrate debate").) However, for this integral function, we are in a somewhat different position because now gaps in the middle of the range are really important. Also, we do the whole exercise here to accommodate for irregular scrape intervals including gaps. In cases where those changes aren't happening, we are fine with the current Implementation-wise, we could make the staleness marker available to function using ranges if they need it. It's nasty, because so far, we can abstract away the staleness marker very early in the stack, essentially in the TSDB query, and PromQL doesn't have to be aware of it at all. Increasingly often, it feels to me that we need a better model of the start and end and sampling interval of each (contiguous piece of a) series. This could help with an actually improved rate calculation, and would also allow a clean implementation of an integral function. |
I don’t agree with this part. I’m only forced to integrate using ‘sum_over_time’ when I can’t get to the original counter metric. This is typically because I’m downstream of some recording rule, which is doing aggregation and relabelling. This being the case, missing samples are not gaps in the underlying data, but gaps in the recording rule. My assumption is that the rule failed to evaluate in the time allowed for evaluation, hence the gap. Meaning I believe a good assumption for the value where the gap lies would not be a zero rate, but a rate identical to the immediately preceding datapoint in that rate metric. Any thoughts @jjo ? As the original PR author, I’d love to hear your take on this. I think you understand the real world conditions when this function would be used, more than most people. |
|
One more thought. I agree with Bjorn that I’d like to just add one function to start with (I’m fine with midpoint) and see if that is enough. (YAGNI- “you ain’t gonna need it” - for the other two) And my naming preference is for “integral”. A “weighted sum over time” isn’t very meaningful to me personally. I think it’s more than just weighted- weighted implies only giving more weight to the samples that are more spaced out- it doesn’t imply scaling up the result for all samples. |
|
How about monte-carlo integration - shouldnt that be somewhat robust towards missing samples? |
|
As discussed with @MichaHoffmann , Monte-Carlo integration works with a random distribution of samples, representative for the full time series. However, what we are trying to solve here is precisely the problem that the samples in the TSDB are not always representative. If the scrape interval changes within the range (the most common problematic case), we systematically over-sample the part where the scrape interval is shorter. If samples are missing (because of scrape problems), we systematically under-sample that part. One could argue that you already get Monte-Carlo integration if just using |
If I had to decide this now, and if I had to decide alone, I would say I don't see sufficient use of this addition in its current state. But I don't feel I understand this deeply enough to really make the call (or to come up with a better solution). I would love to get other opinions, but so far, very little has materialized. I submitted the dev-summit agenda item in the hope of getting more feedback. (I don't think that the dev-summit is the suitable format to do the deep dive into all the gory details I partially dug up above, but it's a good way of finding out if others had similar thoughts and how much interest there is for anything like this.) However, it didn't get a lot of votes so far and never made it to the top of the agenda so far. So I refer back to what I said above:
So the one message we get from the (lack of) feedback is that there is not a lot of interest in this feature among the active developers. Which enforces my gut feeling that it might not be worth going down the rabbit hole of finding out how to implement this feature in a useful way.
I think such an involved new function should always be introduced behind the experimental functions feature flag. Everything I said above was under the assumption that we would do so. The recurring theme with these "hard" problems (integral, but also "xrate") is that we need a better understanding of the beginning and end of a series. The fundamental point of view would be to solve that first and then come back to the problems at hand. However, as usual, I don't want to block anything that others want by my maybe too fundamental and fatalistic view. But it needs somebody else to champion and shepherd this. |
|
Hi, I don't have anything to contribute to this excellent discussion. I more just wanted to voice my support for this feature. The integral feature has many uses which the primary author notes. I would add in my own use case which is to sum over compute usage to get CPU-hours and GPU-hours for a user on a large scale cluster. The regular sum does not do the right thing nor have the useful units, a proper integral would solve this problem. Thanks for your hard work on this, its fascinating to see all the details involved. I hope to see this adopted soon. |
|
Almost a year after adding this as an agenda item for the dev-summit, it was finally discussed. See notes. Sadly, the outcome I had hoped for didn't happen. Ideally, one or more people really deeply into this topic would have popped up, joining forces with @jjo to get this across the finishing line. As I said above, I don't think my current understanding of this topic is sufficient to accept or reject this feature. But it was also pretty obvious that nobody else at the dev-summit had a deeper understanding than me. As also said above, I don't want to block this, but I would like to see someone with a good understanding of this to champion this before we merge this in whatever form. Another take-away from the dev-summit is IMHO that there is no burning desire to have this feature (as already assumed above). It would have meant something if a bunch of people at the dev-summit had stated that they don't understand the details but they really need a feature like this. (Whereas my own inner state currently is that I have something close to a burning desire, but I feel that the current state of this approach is not there yet to really satisfy my desire.) I currently lack the time to deepen my understanding here. Just by writing this comment, I have to resist the urge to get nerd-sniped by this. After re-reading all of the above, my current gut feeling is that we are in a not-too-bad state because we can use the What I would like to drop here (as I have in the dev-summit): There is currently work ongoing on an improved rate calculation (see this proposal and all the stuff linked from it). At the same time, there is active work on a more "native" treatment of metrics with delta temporality. All of these things touch some common ground, and I expect that we will learn a lot from these efforts, how they are used in real-world scenarios, and what will work and what not. I could see that these learnings will also inform how to approach an integral function. (I have a few more detailed thoughts, but I won't dump them into this already too long comment.) |
|
While this looks useful, some of the use case would also be addressed by permitting |
|
Hello from the bug scrub! Seems to be still under discussion. Might be interesting for @roidelapluie to look at #16965. |

Promql: implement
integral(v range-vector, strategy=2 scalar)function.Refs: some previous discussion at #1335 and https://groups.google.com/g/prometheus-developers/c/3EuF6ffNt_k.
What
Implement
integral(v range-vector, strategy=2 scalar)function.Why
There are use cases where
sum_over_time()oravg_over_time()multiplied by the length of the whole interval are cumbersome to use
(need to factor the length of the interval and/or sampling period),
or plainly N/A (e.g. if you had periods of uneven scraping intervals).
Some relevant use cases (added to examples.md):
Calculate the total cost over past week given the per-node hourly cost:
integral(hourly_cost[7d]) / 3600Calculate the total energy consumed in kWh over last day, given the instant
power consumption in Watts:
integral(power_rate[1d]) / 1000 / 3600For those "boolean" metrics that produce
1s or0s, integrating them overtime will give the time in seconds that the value was
1, for example, toget the amount of time a particular metric has been absent during the past
day (sampled every 5 minutes):
integral(absent(metric_name)[1d:5m])As another case for measuring amount of time, for example a boolean metric
representing a service level agreement (SLA) being met divided by the total
time, will give the SLA ratio:
integral(service_sla_ok[7d]) / (3600 * 24 * 7)How
Formula-wise the implementation is mostly straightforward, but details
are important:
implements three strategies to calculate the integral via trapezoidal
area aggregation: mid-point (aka Riemann's, default for
strategy=2),left-point (
strategy=0) and right-point (strategy=1); this is done tolet the user choose the best one to use depending on their value
"alignment" with the integrated interval.
there's specific NaN handling (mapped to zero) to simplify implementing
of the above strategies, mid-point in particular.
mid-point averaging (i.e.
(prevVal+currVal)/2) is implemented withkahanSumInc()to keep big-vs-small numbers support for neighboringsamples.
result needs to support being wrapped inside another aggregate like
sum_over_time(), i.e. interval limit handling is very important toavoid double-aggregating the edge(s).
Testing
Extensive testing cases added to
functions.test.Strategy
Excerpt from functions.go:
Signed-off-by: JuanJo Ciarlante juanjosec@gmail.com