Commit Graph

72 Commits (cfb54fa6a3e4349398875d4d5d48025e36489356)

Author SHA1 Message Date
zenador 191bf9055b
Handle more arithmetic operators for native histograms (#12262)
Handle more arithmetic operators and aggregators for native histograms

This includes operators for multiplication (formerly known as scaling), division, and subtraction. Plus aggregations for average and the avg_over_time function.

Stdvar and stddev will (for now) ignore histograms properly (rather than counting them but adding a 0 for them).

Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
2023-05-16 21:15:20 +02:00
beorn7 5b53aa1108 style: Replace `else if` cascades with `switch`
Wiser coders than myself have come to the conclusion that a `switch`
statement is almost always superior to a statement that includes any
`else if`.

The exceptions that I have found in our codebase are just these two:

* The `if else` is followed by an additional statement before the next
  condition (separated by a `;`).
* The whole thing is within a `for` loop and `break` statements are
  used. In this case, using `switch` would require tagging the `for`
  loop, which probably tips the balance.

Why are `switch` statements more readable?

For one, fewer curly braces. But more importantly, the conditions all
have the same alignment, so the whole thing follows the natural flow
of going down a list of conditions. With `else if`, in contrast, all
conditions but the first are "hidden" behind `} else if `, harder to
spot and (for no good reason) presented differently from the first
condition.

I'm sure the aforemention wise coders can list even more reasons.

In any case, I like it so much that I have found myself recommending
it in code reviews. I would like to make it a habit in our code base,
without making it a hard requirement that we would test on the CI. But
for that, there has to be a role model, so this commit eliminates all
`if else` occurrences, unless it is autogenerated code or fits one of
the exceptions above.

Signed-off-by: beorn7 <beorn@grafana.com>
2023-04-19 17:22:31 +02:00
Trevor Whitney e3513d1dd2
Change nested ifs to a switch
Signed-off-by: Trevor Whitney <trevorjwhitney@gmail.com>
2023-03-14 14:22:20 -06:00
Trevor Whitney b4e324f637
Handle valid cases of mismatched hints when adding
Signed-off-by: Trevor Whitney <trevorjwhitney@gmail.com>
2023-03-14 14:22:14 -06:00
Trevor Whitney dd94ebb87b
promql: set CounterResetHint after rate and sum
Signed-off-by: Trevor Whitney <trevorjwhitney@gmail.com>
2023-03-14 14:21:59 -06:00
Ganesh Vernekar a87e7e9e33
tsdb: Add counter reset hint to histograms and support in WAL
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2023-01-10 17:41:53 +05:30
Marc Tudurí 9474610baf
Support FloatHistogram in TSDB (#11522)
Extends Appender.AppendHistogram function to accept the FloatHistogram. TSDB supports appending, querying, WAL replay, for this new type of histogram.

Signed-off-by: Marc Tudurí <marctc@protonmail.com>
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
Co-authored-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-12-28 14:25:07 +05:30
Björn Rabenstein dccfb9db4e
histogram: Remove code replication via generics (#11361)
* histogram: Simplify iterators

We don't really need currLower and currUpper and can calculate it when
needed (as already done for the floatBucketIterator). The calculation
is cheap, while keeping those extra variables around costs RAM
(potentially a lot with many iterators).

* histogram: Convert Bucket/FloatBucket to one generic type

* histogram: Move some bucket iterator code into generic base iterator

* histogram: Remove cumulative iterator for FloatHistogram

We added it in the past for completeness (Histogram has one), but it
has never been used. Plus, even the cumulative iterator for Histogram
is only there for test reasons.

We can always add it back, and then maybe even using generics.

Signed-off-by: beorn7 <beorn@grafana.com>
2022-10-03 16:45:27 +05:30
beorn7 a7c519930e histograms: Add Compact method to the normal integer Histogram
And use the new method to call to compact Histograms during
parsing. This happens for both `Histogram` and `FloatHistogram`. In
this way, if targets decide to optimize the exposition size by merging
spans with empty buckets in between, we still get a normalized
results. It will also normalize away any valid but weird
representations like empty spans, spans with offset zero, and empty
buckets at the start or end of a span.

The implementation seemed easy at first as it just turns the
`compactBuckets` helper into a generic function (which now got its own
file). However, the integer Histograms have delta buckets instead of
absolute buckets, which had to be treated specially in the generic
`compactBuckets` function. To make sure it works, I have added plenty
of explicit tests for `Histogram` in addition to the `FloatHistogram`
tests.

I have also updated the doc comment for the `Compact` method.

Based on the insights now expressed in the doc comment, compacting
with a maxEmptyBuckets > 0 is rarely useful. Therefore, this commit
also sets the value to 0 in the two cases we were using 3 so far. We
might still want to reconsider, so I don't want to remove the
maxEmptyBuckets parameter right now.

Signed-off-by: beorn7 <beorn@grafana.com>
2022-09-27 13:04:16 +02:00
beorn7 2d233cf95e Histogram: Fix allFloatBucketIterator
If the zero threshold overlaps with the highest negative bucket and/or
the lowest positive bucket, its upper or lower boundary, respectively,
has to be adjusted. In valid histograms, only ever the highest
negative bucket and/or the lowest positive bucket may overlap with the
zero bucket. This is assumed in this code to simplify the checks.

Signed-off-by: beorn7 <beorn@grafana.com>
2022-05-03 16:24:11 +02:00
beorn7 56db51c826 Histgram: Fix Compact for spans of only empty buckets
Signed-off-by: beorn7 <beorn@grafana.com>
2022-04-12 00:37:50 +02:00
beorn7 15583af9bb Histogram: Fix crash when compacting only empty buckets
Signed-off-by: beorn7 <beorn@grafana.com>
2022-04-07 16:24:48 +02:00
beorn7 9fbcf14e5c histogram: Handle changes of the ZeroThreshold and the Schema
Signed-off-by: beorn7 <beorn@grafana.com>
2022-03-17 18:05:31 +01:00
Björn Rabenstein ec80745884
Merge pull request #10075 from prometheus/beorn7/histogram
model: Implement FloatHistogram.Compact
2022-01-05 16:09:39 +01:00
beorn7 3b4d6c3fdb model: Implement FloatHistogram.Compact
Signed-off-by: beorn7 <beorn@grafana.com>
2022-01-05 14:34:03 +01:00
beorn7 e7592fe353 sparsehistogram: Address two TODOs
Signed-off-by: beorn7 <beorn@grafana.com>
2022-01-04 12:48:59 +01:00
beorn7 27f865ec49 histograms: avoid initialization tracking in reverseFloatBucketIterator
Signed-off-by: beorn7 <beorn@grafana.com>
2021-12-15 16:56:33 +01:00
beorn7 a6acdfe346 histograms: Doc comment and naming improvements
Signed-off-by: beorn7 <beorn@grafana.com>
2021-12-15 16:50:37 +01:00
Ganesh Vernekar 4a43349aca
`histogram_quantile` for sparse histograms (#9935)
* MergeFloatBucketIterator for []FloatBucketIterator

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* histogram_quantile for histograms

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix histogram_quantile

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Unit test and enhancements

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Iterators to iterate buckets in reverse and all buckets together including zero bucket

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Consider all buckets for histogram_quantile and fix the implementation

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Remove unneeded code

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix lint

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2021-12-06 19:17:22 +05:30
Björn Rabenstein 0e1b9dd308
Promql: Initial rate implementation for sparse histograms (#9926)
Signed-off-by: beorn7 <beorn@grafana.com>
2021-12-06 18:19:18 +05:30
Björn Rabenstein 7e42acd3b1
tsdb: Rework iterators (#9877)
- Pick At... method via return value of Next/Seek.
- Do not clobber returned buckets.
- Add partial FloatHistogram suppert.

Note that the promql package is now _only_ dealing with
FloatHistograms, following the idea that PromQL only knows float
values.

As a byproduct, I have removed the histogramSeries metric. In my
understanding, series can have both float and histogram samples, so
that metric doesn't make sense anymore.

As another byproduct, I have converged the sampleBuf and the
histogramSampleBuf in memSeries into one. The sample type stored in
the sampleBuf has been extended to also contain histograms even before
this commit.

Signed-off-by: beorn7 <beorn@grafana.com>
2021-11-29 13:24:23 +05:30
beorn7 6a820a646c histogram: Add FloatHistogram
Including a few adjustments for normal Histogram, too, e.g. use
pointer receiver to avoid the large copy on method calls.

Signed-off-by: beorn7 <beorn@grafana.com>
2021-11-23 19:40:49 +01:00