wip: support native histograms along classic histograms in panels #1121
Conversation
krajorama
force-pushed
the
krajo/native-histogram-mixin
branch
from
394982a
to
b6041de
Compare
This branch will have force pushes. Native histograms are https://grafana.com/docs/mimir/latest/send/native-histograms/ Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
krajorama
force-pushed
the
krajo/native-histogram-mixin
branch
from
b6041de
to
91c5fc3
Compare
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Must put metric query template into parenthesis otherwise we just multiply one term of the query. Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Easier to add on to existing selector this way Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
| nativeClassicHistogramQuantile(percentile, metric, selector, sum_by=[], rate_interval='$__rate_interval'):: | ||
| local classicSumBy = if std.length(sum_by) > 0 then ' by (%(lbls)s) ' % { lbls: std.join(',', ['le'] + sum_by) } else ' by (le) '; | ||
| local nativeSumBy = if std.length(sum_by) > 0 then ' by (%(lbls)s) ' % { lbls: std.join(',', sum_by) } else ' '; | ||
| 'histogram_quantile(%(percentile)s, sum%(nativeSumBy)s(rate(%(metric)s{%(selector)s}[%(rateInterval)s]))) or histogram_quantile(%(percentile)s, sum%(classicSumBy)s(rate(%(metric)s_bucket{%(selector)s}[%(rateInterval)s])))' % { |
There was a problem hiding this comment.
for reviewers: e.g.:
(histogram_quantile(0.99, sum (rate(cortex_request_duration_seconds{}[$__rate_interval]))) or
histogram_quantile(0.99, sum by (le) (rate(cortex_request_duration_seconds_bucket{}[$__rate_interval]))))
There was a problem hiding this comment.
I think this can be improved.
Imagine you have a fairly long rate_interval (the problem exists for all intervals, but the longer, the more serious it gets), e.g. multiple days or so. While migrating to native histograms, you ingest classic and native histograms in parallel. Very soon, the first leg of the query above will yield a result, but it will be based just on a few samples of native histograms, or the last few minutes of the multi-day range.
So what you want is to use the leg of the query that has a complete coverage of the rate_interval (and in doubt prefer the native histogram). I'll try to come up with a way to do that in the next comment.
There was a problem hiding this comment.
Maybe this could work:
(
histogram_quantile(
0.99,
sum(rate(cortex_request_duration_seconds[1w])) and group(cortex_request_duration_seconds offset 1w)
)
or
histogram_quantile(
0.99,
sum by (le) (rate(cortex_request_duration_seconds_bucket[1w]))
and
group by (le) (cortex_request_duration_seconds_bucket)
)
or
histogram_quantile(0.99, sum(rate(cortex_request_duration_seconds[1w])))
)
This assumes the migration is already going from classic to native histograms. First, the query checks if there has been a native histogram 1w ago. If so, it goes for native histograms. Second, it checks if there is a classic histogram now, and uses them. As the third and final option, it uses native histograms (which is relevant for a new metric that never had classic histograms).
Of course, the stupid PromQL engine will calculate all three legs, even if only one will ever be used.
| // The classicNativeHistogramSumRate function is used to calculate the histogram sum of rate from native histograms or classic histograms. | ||
| // Metric name should be provided without _sum suffix. | ||
| nativeClassicHistogramSumRate(metric, selector, rate_interval='$__rate_interval'):: | ||
| 'histogram_sum(rate(%(metric)s{%(selector)s}[%(rateInterval)s])) or rate(%(metric)s_sum{%(selector)s}[%(rateInterval)s])' % { |
There was a problem hiding this comment.
for reviewers e.g., in context of a sum:
sum(
histogram_sum(rate(cortex_request_duration_seconds{}[$__rate_interval])) or
rate(cortex_request_duration_seconds_sum{}[$__rate_interval])
)
|
Superseded by #1150 |
This branch will have force pushes.
Related to grafana/mimir#7154
Native histograms are
https://grafana.com/docs/mimir/latest/send/native-histograms/