Update native histogram spec #2576
Conversation
|
After some thinking, I came to the conclusion that the special behavior with counter reset between the 1st and 2nd histogram is so niche that I don't want to explicitly call it out in the regular PromQL documentation. I would argue that the now established behavior is the most reasonable one given how counter reset calculation works. |
| These functions SHOULD be applied to either gauge histograms or counter | ||
| histograms as noted above. However, they all work with both flavors, but if at | ||
| least one histogram of an unsuitable flavor is contained in the range vector, a | ||
| warn-level annotation is added to the result. | ||
|
|
||
| All these functions return no result for series that contain a mix of float | ||
| samples and histogram samples within the range. A warn-level annotation is | ||
| added for each output element missing for that reason. | ||
| added for each output element missing for that reason. `idelta()` and `irate()` | ||
| only consider the last two samples in the range for this check. |
There was a problem hiding this comment.
Although the last line, "idelta() and irate() only consider the last two samples in the range for this check," clarifies that only the last two samples are considered, it would be better to convey this information in the initial sentence.
There was a problem hiding this comment.
The problem here is that the initial sentence is the general one that is valid for all these function. The sentence about idelta and irate is the exception to it for these two special functions. Maybe I can munge the last and first sentence together…
There was a problem hiding this comment.
Now it looks good to me!
Thanks for the change.
idelta and irate are implemented now. We also realized that the 1st histogram can be safely ignored if there is a counter reset between the 1st and the 2nd histogram. Therefore, we don't even have to error out on incompatible bucket layout in this case. Signed-off-by: beorn7 <[email protected]>
Signed-off-by: beorn7 <[email protected]>
Signed-off-by: beorn7 <[email protected]>
beorn7
force-pushed
the
beorn7/histogram
branch
from
0d7e9c2 to
fd50ced
Compare
|
With @NeerajGartia21 👍 , I'll merge this now. It's "just documentation". @krajorama if you have any concerns, let me know and I'll address them in a separate PR. |
Add the newly established behavior of rate, irate & friends.
Remove some obsolete TODOs.
@NeerajGartia21 @krajorama