From aa82fe198f66f4e2a41647fd0845834acf365e0e Mon Sep 17 00:00:00 2001 From: beorn7 Date: Tue, 22 Aug 2023 21:51:56 +0200 Subject: [PATCH] tsdb: Fix histogram validation So far, `ValidateHistogram` would not detect if the count did not include the count in the zero bucket. This commit fixes the problem and updates all the tests that have been undetected offenders so far. Note that this problem would only ever create false negatives, so we never falsely rejected to store a histogram because of it. On the other hand, `ValidateFloatHistogram` has been to strict with the count being at least as large as the sum of the counts in all the buckets. Float precision issues could create false positives here, see products of PromQL evaluations, it's actually quite hard to put an upper limit no the floating point imprecision. Users could produce the weirdest expressions, maxing out float precision problems. Therefore, this commit simply removes that particular check from `ValidateFloatHistogram`. Signed-off-by: beorn7 --- promql/engine_test.go | 26 +++++++------- storage/remote/read_handler_test.go | 2 +- tsdb/block_test.go | 4 +-- tsdb/compact_test.go | 2 +- tsdb/db_test.go | 4 +-- tsdb/head_append.go | 11 +++--- tsdb/head_test.go | 53 +++++++++++++++++------------ tsdb/tsdbutil/histogram.go | 4 +-- web/federate_test.go | 3 +- 9 files changed, 59 insertions(+), 50 deletions(-) diff --git a/promql/engine_test.go b/promql/engine_test.go index 0df969375..154a45514 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -3182,7 +3182,7 @@ func TestNativeHistogramRate(t *testing.T) { Schema: 1, ZeroThreshold: 0.001, ZeroCount: 1. / 15., - Count: 8. / 15., + Count: 9. / 15., Sum: 1.226666666666667, PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}, {Offset: 1, Length: 2}}, PositiveBuckets: []float64{1. / 15., 1. / 15., 1. / 15., 1. / 15.}, @@ -3223,7 +3223,7 @@ func TestNativeFloatHistogramRate(t *testing.T) { Schema: 1, ZeroThreshold: 0.001, ZeroCount: 1. / 15., - Count: 8. / 15., + Count: 9. / 15., Sum: 1.226666666666667, PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}, {Offset: 1, Length: 2}}, PositiveBuckets: []float64{1. / 15., 1. / 15., 1. / 15., 1. / 15.}, @@ -3996,7 +3996,7 @@ func TestNativeHistogram_Sum_Count_Add_AvgOperator(t *testing.T) { { CounterResetHint: histogram.GaugeType, Schema: 0, - Count: 21, + Count: 25, Sum: 1234.5, ZeroThreshold: 0.001, ZeroCount: 4, @@ -4014,7 +4014,7 @@ func TestNativeHistogram_Sum_Count_Add_AvgOperator(t *testing.T) { { CounterResetHint: histogram.GaugeType, Schema: 0, - Count: 36, + Count: 41, Sum: 2345.6, ZeroThreshold: 0.001, ZeroCount: 5, @@ -4034,7 +4034,7 @@ func TestNativeHistogram_Sum_Count_Add_AvgOperator(t *testing.T) { { CounterResetHint: histogram.GaugeType, Schema: 0, - Count: 36, + Count: 41, Sum: 1111.1, ZeroThreshold: 0.001, ZeroCount: 5, @@ -4061,7 +4061,7 @@ func TestNativeHistogram_Sum_Count_Add_AvgOperator(t *testing.T) { Schema: 0, ZeroThreshold: 0.001, ZeroCount: 14, - Count: 93, + Count: 107, Sum: 4691.2, PositiveSpans: []histogram.Span{ {Offset: 0, Length: 7}, @@ -4078,7 +4078,7 @@ func TestNativeHistogram_Sum_Count_Add_AvgOperator(t *testing.T) { Schema: 0, ZeroThreshold: 0.001, ZeroCount: 3.5, - Count: 23.25, + Count: 26.75, Sum: 1172.8, PositiveSpans: []histogram.Span{ {Offset: 0, Length: 7}, @@ -4189,7 +4189,7 @@ func TestNativeHistogram_SubOperator(t *testing.T) { histograms: []histogram.Histogram{ { Schema: 0, - Count: 36, + Count: 41, Sum: 2345.6, ZeroThreshold: 0.001, ZeroCount: 5, @@ -4224,7 +4224,7 @@ func TestNativeHistogram_SubOperator(t *testing.T) { }, expected: histogram.FloatHistogram{ Schema: 0, - Count: 25, + Count: 30, Sum: 1111.1, ZeroThreshold: 0.001, ZeroCount: 2, @@ -4245,7 +4245,7 @@ func TestNativeHistogram_SubOperator(t *testing.T) { histograms: []histogram.Histogram{ { Schema: 0, - Count: 36, + Count: 41, Sum: 2345.6, ZeroThreshold: 0.001, ZeroCount: 5, @@ -4280,7 +4280,7 @@ func TestNativeHistogram_SubOperator(t *testing.T) { }, expected: histogram.FloatHistogram{ Schema: 0, - Count: 25, + Count: 30, Sum: 1111.1, ZeroThreshold: 0.001, ZeroCount: 2, @@ -4315,7 +4315,7 @@ func TestNativeHistogram_SubOperator(t *testing.T) { }, { Schema: 0, - Count: 36, + Count: 41, Sum: 2345.6, ZeroThreshold: 0.001, ZeroCount: 5, @@ -4335,7 +4335,7 @@ func TestNativeHistogram_SubOperator(t *testing.T) { }, expected: histogram.FloatHistogram{ Schema: 0, - Count: -25, + Count: -30, Sum: -1111.1, ZeroThreshold: 0.001, ZeroCount: -2, diff --git a/storage/remote/read_handler_test.go b/storage/remote/read_handler_test.go index 3d9182640..7e8618615 100644 --- a/storage/remote/read_handler_test.go +++ b/storage/remote/read_handler_test.go @@ -421,7 +421,7 @@ func TestStreamReadEndpoint(t *testing.T) { { Type: prompb.Chunk_FLOAT_HISTOGRAM, MaxTimeMs: 7140000, - Data: []byte("\x00x\x00\xff?PbM\xd2\xf1\xa9\xfc\x8c\xa4\x94e$\xa2@$\x00\x00\x00\x00\x00\x00@\x00\x00\x00\x00\x00\x00\x00@2ffffff?\xf0\x00\x00\x00\x00\x00\x00@\x00\x00\x00\x00\x00\x00\x00?\xf0\x00\x00\x00\x00\x00\x00?\xf0\x00\x00\x00\x00\x00\x00?\xf0\x00\x00\x00\x00\x00\x00@\x00\x00\x00\x00\x00\x00\x00?\xf0\x00\x00\x00\x00\x00\x00?\xf0\x00\x00\x00\x00\x00\x00\xf8\xea`\xd6%\xec\a\xa4?\x84\xbf\xff\xb0\x1e\x12\xff\xfe\x12\xff\xfe\x12\xff\xfe\xc0xK\xff\xf8K\xff\xe95\x85\xec\xd2\x7f\xff\xff\xff\xff\xff\xf6\x03\xd6\x17\xb0\x1e\xc0{\x01\xeb\v\xd8\x0f`6\x91\xfd\xed\a\xaf\\\xff\xff\xff\xff\xff\xff\xeb\v\xda\x0fX^\xb0\xbda{A\xeb\v\xd6\x16\x82l\v\x8a\xcc\xcc\xcc\xcc\xccʹ\x1e\xc0\xbd\xa0\xf6\x83\xda\x0f`^\xd0{A\xa1\x932fffffg`\\\xec\v\xd8\x17\xb0.v\x05\xec\vA5\t\xfa\x87\xef:\x84\xf9\x99\xd4'̵\x8d\xde\xe0{\xb2\x9f\xff\xff\xff\xff\xff\xf5\t\xfb\x81\xea\x13\xf5\t\xfa\x84\xfd\xc0\xf5\t\xfa\x84\xf4&н\xb9\xedUUUUU]\xc0\xf6\x85\xee\a\xb8\x1e\xe0{B\xf7\x03\xdc\r\x193\xb333333\xda\x17;B\xf6\x85\xed\v\x9d\xa1{BЛ\x03\xfb2\xe4\xcc\xcc\xcc\xcc\xcc\xe7`~fv\a\xe6S\x91ݕ\xaa\xaa\xaa\xaa\xaa\xab\xb0?\x1d\x81\xfd\x81\xfd\x81\xf8\xec\x0f\xec\x0f\xa1'\xb7<\xff\xff\xff\xff\xff\xff\x19\xc61\x9cb\x8c\x8e\xbd{\xff\xff\xff\xff\xff\xff8\xces\x8c\xe6\x84\xd6'\xc1Y\x99\x99\x99\x99\x99\x8e\xb1>1\x8e\xb1>1j#\xefx8d\xcc\xcc\xcc\xcc\xcc\xda\xc4\xfd\xe0\xf5\x89\xfa\xc4\xfdb~\xf0z\xc4\xfdbz\x04\xdc\x17\a\xaa\xaa\xaa\xaa\xaa\xabx=\xc1{\xc1\xef\a\xbc\x1e\xe0\xbd\xe0\xf7\x83A\x93\x1c\xff\xff\xff\xff\xff\xffp\\\xee\v\xdc\x17\xb8.w\x05\xee\v@\x9bC\xf0Z\xaa\xaa\xaa\xaa\xaa\xa7h~fv\x87\xe6P\xe4al\xcc\xcc\xcc\xcc\xcc\xed\x0f\xc7h\x7fh\x7fh~;C\xfbC\xe8\x12sə\x99\x99\x99\x99\xa38\xc63\x8cPd`\xb5UUUUUN3\x9c\xe39\xa0M\x82|3\xff\xff\xff\xff\xff\xf8\xec\x13\xe3\x18\xec\x13\xe3\x14y\f\x1e\xaa\xaa\xaa\xaa\xaa\xad\x82|;\x04\xfd\x82~\xc1>\x1d\x82~\xc1=\x02G\x1c\x99\x99\x99\x99\x99\x9a\x18\xe1\x86\x18\xe1\x85\x06C\x05ffffff8c\x8e8c\x8d\x02O\v\xaa\xaa\xaa\xaa\xaa\xaa\x19\xe1\x86\x19\xe1\x85\x0eC\xa3\x8f\xf1UUUUUY\xe1\x9ey\xe1\x9et\t\x1c\x01j\xaa\xaa\xaa\xaa\xab\fp\xc3\fp\u0083!\x80{33333#\x868\xe3\x868\xd0&\x91\xff\xc0\x12fffffp\xe9\x1f\xfc0ä\x7f\xf0\xc2\xd6G\xdf\x00p\x1d\xaa\xaa\xaa\xaa\xaa\xae\x91\xff\xf0\a\xa4\x7f\xfaG\xff\xa4\x7f\xfc\x01\xe9\x1f\xfe\x91\xff\xa0M\xe1p\x04\xff\xff\xff\xff\xff\xff\x00{\xc2\xf8\x03\xe0\x0f\x80=\xe1|\x01\xf0\x06\x83&\x01uUUUUU\xde\x17;\xc2\xf7\x85\xef\v\x9d\xe1{\xc2\xd0&\xe0\xfc\x0fY\x99\x99\x99\x99\x99;\x83\xf33\xb8?2\x87#\x00I\x99\x99\x99\x99\x99\xee\x0f\xc7p\x7fp\x7fp~;\x83\xfb\x83\xe8\x12p\x0f\xaa\xaa\xaa\xaa\xaa\xacg\x18\xc6q\x8a\f\x8c\x01?\xff\xff\xff\xff\xff8\xces\x8c\xe6\x816\x89\xf0\x1d\xaa\xaa\xaa\xaa\xaa\xacv\x89\xf1\x8cv\x89\xf1\x8a<\x86\x01l\xcc\xcc\xcc\xcc\xcc\xda'ôO\xda'\xed\x13\xe1\xda'\xed\x13\xd0$p\x04\x99\x99\x99\x99\x99\x9c1\xc3\f1\xc3\n\f\x86\x0f\xb5UUUUU\x8e\x18\xe3\x8e\x18\xe3@\x93\xc0\x13\xff\xff\xff\xff\xff\xf0\xcf\f0\xcf\f(r\x18\a\xd5UUUUVxg\x9exg\x9d\x02G\x00I\x99\x99\x99\x99\x99\xc3\x1c0\xc3\x1c0\xa0\xc8`:\xcc\xcc\xcc\xcc\xcc\xc8\xe1\x8e8\xe1\x8e4\t\xb0_\xc0.\xaa\xaa\xaa\xaa\xaa\xb0\xec\x17\xf0\xc3\x0e\xc1\x7f\f)\xf2\f\x01?\xff\xff\xff\xff\xff\xb0_\xc1\xd8/\xf6\v\xfd\x82\xfe\x0e\xc1\x7f\xb0_\xa0Hp=\xaa\xaa\xaa\xaa\xaa\xac\x18p`\xc1\x87\x06\n\f\x83\x00I\x99\x99\x99\x99\x99Ã\x0e\x1c80\xe1\xa0H\xf0\x0ffffffd\x18\xf0`\xc1\x8f\x06\n\x1c\x83\x00Z\xaa\xaa\xaa\xaa\xaaǃ\x1e|\xf83\xe7\xa0Hp\x03\xd5UUUUT\x18p`\xc1\x87\x06\n\f\x83\x00g\xff\xff\xff\xff\xffÃ\x0e\x1c80\xe1\xa0H\xf0\x02\xd5UUUUT\x18\xf0`\xc1\x8f\x06\n\x1c\x83\x00\xdb33333G\x83\x1e \xf8\x83\xe0\x17\xc4\x1f\x10h\x03&\x00I\x99\x99\x99\x99\x99\xe0\x17<\x02\xf8\x05\xf0\v\x9e\x01|\x02\xd0\x02o\x0f\xc07UUUUUS\xbc?3;\xc3\xf3(\a#\x00g\xff\xff\xff\xff\xff\xef\x0f\xc7x\x7fx\x7fx~;\xc3\xfb\xc3\xe8\x01'\x00-UUUUUFq\x8cg\x18\xa0\f\x8c\x0f\xec\xcc\xcc\xcc\xcc\xcd8\xces\x8c\xe6\x80\x13p\x9f\x00$\xcc\xcc\xcc\xcc\xcc\xc7p\x9f\x18\xc7p\x9f\x18\xa0<\x86\x00ڪ\xaa\xaa\xaa\xaa\xdc'øO\xdc'\xee\x13\xe1\xdc'\xee\x13\xd0\x02G\x00'\xff\xff\xff\xff\xff\xc3\x1c0\xc3\x1c0\xa0\f\x86\x01\xba\xaa\xaa\xaa\xaa\xaa\x8e\x18\xe3\x8e\x18\xe3@\t<\x01\xac\xcc\xcc\xcc\xcc\xcd\f\xf0\xc3\f\xf0\u0080r\x18\x01&fffffxg\x9exg\x9d\x00$p\x1f\xd5UUUUT1\xc3\f1\xc3\n\x00\xc8`\x04\xff\xff\xff\xff\xff\xf8\xe1\x8e8\xe1\x8e4\x00\x9bE\xfc\x01\xb5UUUUU\x0e\xd1\x7f\f0\xed\x17\xf0\u0081\xf2\f\x03l\xcc\xcc\xcc\xccʹ_\xc1\xda/\xf6\x8b\xfd\xa2\xfe\x0e\xd1\x7f\xb4_\xa0\x04\x87\x00$\xcc\xcc\xcc\xcc\xcc\xc1\x87\x06\f\x18p`\xa0\f\x83\x00mUUUUUC\x83\x0e\x1c80\xe1\xa0\x04\x8f\x00'\xff\xff\xff\xff\xff\xc1\x8f\x06\f\x18\xf0`\xa0\x1c\x83\a\xfdUUUUUG\x83\x1e|\xf83\xe7\xa0\x04\x87\x00mUUUUUA\x87\x06\f\x18p`\xa0\f\x83\x00$\xcc\xcc\xcc\xcc\xccÃ\x0e\x1c80\xe1\xa0\x04\x8f\x01\xfb33333A\x8f\x06\f\x18\xf0`\xa0\x1c\x83\x00-UUUUUG\x83\x1e|\3703\347\264\031\3770\340\007\252\252\252\252\252\2500\340\301\203\016\014\027\021&\014\001\237\377\377\377\377\377\016\0148p\340\303\206\340.\343\300\013UUUUUPc\301\203\006<\030)0`\033fffffh\360c\307\217\006\367\t\360\002L\314\314\314\314\314w\t\361\214w\t\361\212\t\206\000\332\252\252\252\252\252\334'\303\270O\334'\356\023\341\334'\356\023\320\374p\002\177\377\377\377\377\3741\303\0141\303\n\t\206\001\272\252\252\252\252\252\216\030\343\216\030\343Gs\300\032\314\314\314\314\314\320\317\0140\317\014(&\030\001&fffffxg\236xg\235\013\307\001\375UUUUUC\0340\303\0340\247\230`\004\377\377\377\377\377\370\341\2168\341\2164\027\264_\300\033UUUUUP\355\027\360\303\016\321\177\014(f\014\003l\314\314\314\314\315\264_\301\332/\366\213\375\242\376\016\321\177\264_\240\370p\002L\314\314\314\314\314\030p`\301\207\006\n9\203\000mUUUUUC\203\016\03480\341\240\270\360\002\177\377\377\377\377\374\030\360`\301\217\006\n\031\203\007\375UUUUUG\203\036\201\360p\001?\377\377\377\377\374\014\034\014\014\014\034\014\013Y\177\366\006\000]UUUUU\203\201\203\203\203\201\203\203@.\036\000\35333333\201\207\201\201\201\207\201\201@f\006\000$\314\314\314\314\314\207\201\207\207\207\201\207\207@\336\016\000}UUUUU\201\203\201\201\201\203\201\201@&\006\000'\377\377\377\377\377\203\201\203\203\203\201\203\203@n>\001\355UUUUU\201\217\201\201\201\217\201\201@&\006\000[33333\217\201\217\217\217\201\217\217"), }, }, }, diff --git a/tsdb/block_test.go b/tsdb/block_test.go index e9dc1a9d0..e0d928aca 100644 --- a/tsdb/block_test.go +++ b/tsdb/block_test.go @@ -621,7 +621,7 @@ func genSeries(totalSeries, labelCount int, mint, maxt int64) []storage.Series { func genHistogramSeries(totalSeries, labelCount int, mint, maxt, step int64, floatHistogram bool) []storage.Series { return genSeriesFromSampleGenerator(totalSeries, labelCount, mint, maxt, step, func(ts int64) tsdbutil.Sample { h := &histogram.Histogram{ - Count: 5 + uint64(ts*4), + Count: 7 + uint64(ts*5), ZeroCount: 2 + uint64(ts), ZeroThreshold: 0.001, Sum: 18.4 * rand.Float64(), @@ -660,7 +660,7 @@ func genHistogramAndFloatSeries(totalSeries, labelCount int, mint, maxt, step in s = sample{t: ts, f: rand.Float64()} } else { h := &histogram.Histogram{ - Count: 5 + uint64(ts*4), + Count: 7 + uint64(ts*5), ZeroCount: 2 + uint64(ts), ZeroThreshold: 0.001, Sum: 18.4 * rand.Float64(), diff --git a/tsdb/compact_test.go b/tsdb/compact_test.go index d20918268..512a6ecfb 100644 --- a/tsdb/compact_test.go +++ b/tsdb/compact_test.go @@ -1364,7 +1364,7 @@ func TestHeadCompactionWithHistograms(t *testing.T) { exp1, exp2, exp3, exp4 []tsdbutil.Sample ) h := &histogram.Histogram{ - Count: 11, + Count: 15, ZeroCount: 4, ZeroThreshold: 0.001, Sum: 35.5, diff --git a/tsdb/db_test.go b/tsdb/db_test.go index 5f36e4333..e8b6d7c3b 100644 --- a/tsdb/db_test.go +++ b/tsdb/db_test.go @@ -6133,7 +6133,7 @@ func testHistogramAppendAndQueryHelper(t *testing.T, floatHistogram bool) { } baseH := &histogram.Histogram{ - Count: 11, + Count: 15, ZeroCount: 4, ZeroThreshold: 0.001, Sum: 35.5, @@ -6506,7 +6506,7 @@ func TestNativeHistogramFlag(t *testing.T) { require.NoError(t, db.Close()) }) h := &histogram.Histogram{ - Count: 6, + Count: 10, ZeroCount: 4, ZeroThreshold: 0.001, Sum: 35.5, diff --git a/tsdb/head_append.go b/tsdb/head_append.go index f06aacba4..8c548fcd9 100644 --- a/tsdb/head_append.go +++ b/tsdb/head_append.go @@ -659,7 +659,7 @@ func ValidateHistogram(h *histogram.Histogram) error { return errors.Wrap(err, "positive side") } - if c := nCount + pCount; c > h.Count { + if c := nCount + pCount + h.ZeroCount; c > h.Count { return errors.Wrap( storage.ErrHistogramCountNotBigEnough, fmt.Sprintf("%d observations found in buckets, but the Count field is %d", c, h.Count), @@ -686,12 +686,9 @@ func ValidateFloatHistogram(h *histogram.FloatHistogram) error { return errors.Wrap(err, "positive side") } - if c := nCount + pCount; c > h.Count { - return errors.Wrap( - storage.ErrHistogramCountNotBigEnough, - fmt.Sprintf("%f observations found in buckets, but the Count field is %f", c, h.Count), - ) - } + // We do not check for h.Count being at least as large as the sum of the + // counts in the buckets because floating point precision issues can + // create false positives here. return nil } diff --git a/tsdb/head_test.go b/tsdb/head_test.go index 9b49aca03..c7fea3f9a 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -4710,42 +4710,42 @@ func TestReplayAfterMmapReplayError(t *testing.T) { func TestHistogramValidation(t *testing.T) { tests := map[string]struct { - h *histogram.Histogram - errMsg string - errMsgFloat string // To be considered for float histogram only if it is non-empty. + h *histogram.Histogram + errMsg string + skipFloat bool }{ "valid histogram": { h: tsdbutil.GenerateTestHistograms(1)[0], }, - "rejects histogram who has too few negative buckets": { + "rejects histogram that has too few negative buckets": { h: &histogram.Histogram{ NegativeSpans: []histogram.Span{{Offset: 0, Length: 1}}, NegativeBuckets: []int64{}, }, errMsg: `negative side: spans need 1 buckets, have 0 buckets`, }, - "rejects histogram who has too few positive buckets": { + "rejects histogram that has too few positive buckets": { h: &histogram.Histogram{ PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}}, PositiveBuckets: []int64{}, }, errMsg: `positive side: spans need 1 buckets, have 0 buckets`, }, - "rejects histogram who has too many negative buckets": { + "rejects histogram that has too many negative buckets": { h: &histogram.Histogram{ NegativeSpans: []histogram.Span{{Offset: 0, Length: 1}}, NegativeBuckets: []int64{1, 2}, }, errMsg: `negative side: spans need 1 buckets, have 2 buckets`, }, - "rejects histogram who has too many positive buckets": { + "rejects histogram that has too many positive buckets": { h: &histogram.Histogram{ PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}}, PositiveBuckets: []int64{1, 2}, }, errMsg: `positive side: spans need 1 buckets, have 2 buckets`, }, - "rejects a histogram which has a negative span with a negative offset": { + "rejects a histogram that has a negative span with a negative offset": { h: &histogram.Histogram{ NegativeSpans: []histogram.Span{{Offset: -1, Length: 1}, {Offset: -1, Length: 1}}, NegativeBuckets: []int64{1, 2}, @@ -4759,21 +4759,21 @@ func TestHistogramValidation(t *testing.T) { }, errMsg: `positive side: span number 2 with offset -1`, }, - "rejects a histogram which has a negative bucket with a negative count": { + "rejects a histogram that has a negative bucket with a negative count": { h: &histogram.Histogram{ NegativeSpans: []histogram.Span{{Offset: -1, Length: 1}}, NegativeBuckets: []int64{-1}, }, errMsg: `negative side: bucket number 1 has observation count of -1`, }, - "rejects a histogram which has a positive bucket with a negative count": { + "rejects a histogram that has a positive bucket with a negative count": { h: &histogram.Histogram{ PositiveSpans: []histogram.Span{{Offset: -1, Length: 1}}, PositiveBuckets: []int64{-1}, }, errMsg: `positive side: bucket number 1 has observation count of -1`, }, - "rejects a histogram which which has a lower count than count in buckets": { + "rejects a histogram that has a lower count than count in buckets": { h: &histogram.Histogram{ Count: 0, NegativeSpans: []histogram.Span{{Offset: -1, Length: 1}}, @@ -4781,25 +4781,36 @@ func TestHistogramValidation(t *testing.T) { NegativeBuckets: []int64{1}, PositiveBuckets: []int64{1}, }, - errMsg: `2 observations found in buckets, but the Count field is 0`, - errMsgFloat: `2.000000 observations found in buckets, but the Count field is 0.000000`, + errMsg: `2 observations found in buckets, but the Count field is 0`, + skipFloat: true, + }, + "rejects a histogram that doesn't count the zero bucket in its count": { + h: &histogram.Histogram{ + Count: 2, + ZeroCount: 1, + NegativeSpans: []histogram.Span{{Offset: -1, Length: 1}}, + PositiveSpans: []histogram.Span{{Offset: -1, Length: 1}}, + NegativeBuckets: []int64{1}, + PositiveBuckets: []int64{1}, + }, + errMsg: `3 observations found in buckets, but the Count field is 2`, + skipFloat: true, }, } for testName, tc := range tests { t.Run(testName, func(t *testing.T) { - switch err := ValidateHistogram(tc.h); { - case tc.errMsg != "": + if err := ValidateHistogram(tc.h); tc.errMsg != "" { require.ErrorContains(t, err, tc.errMsg) - default: + } else { require.NoError(t, err) } - switch err := ValidateFloatHistogram(tc.h.ToFloat()); { - case tc.errMsgFloat != "": - require.ErrorContains(t, err, tc.errMsgFloat) - case tc.errMsg != "": + if tc.skipFloat { + return + } + if err := ValidateFloatHistogram(tc.h.ToFloat()); tc.errMsg != "" { require.ErrorContains(t, err, tc.errMsg) - default: + } else { require.NoError(t, err) } }) diff --git a/tsdb/tsdbutil/histogram.go b/tsdb/tsdbutil/histogram.go index 2145034e1..8270da686 100644 --- a/tsdb/tsdbutil/histogram.go +++ b/tsdb/tsdbutil/histogram.go @@ -33,7 +33,7 @@ func GenerateTestHistograms(n int) (r []*histogram.Histogram) { // GenerateTestHistogram but it is up to the user to set any known counter reset hint. func GenerateTestHistogram(i int) *histogram.Histogram { return &histogram.Histogram{ - Count: 10 + uint64(i*8), + Count: 12 + uint64(i*9), ZeroCount: 2 + uint64(i), ZeroThreshold: 0.001, Sum: 18.4 * float64(i+1), @@ -78,7 +78,7 @@ func GenerateTestFloatHistograms(n int) (r []*histogram.FloatHistogram) { // GenerateTestFloatHistogram but it is up to the user to set any known counter reset hint. func GenerateTestFloatHistogram(i int) *histogram.FloatHistogram { return &histogram.FloatHistogram{ - Count: 10 + float64(i*8), + Count: 12 + float64(i*9), ZeroCount: 2 + float64(i), ZeroThreshold: 0.001, Sum: 18.4 * float64(i+1), diff --git a/web/federate_test.go b/web/federate_test.go index 2d3542ddc..30db0d640 100644 --- a/web/federate_test.go +++ b/web/federate_test.go @@ -306,7 +306,7 @@ func TestFederationWithNativeHistograms(t *testing.T) { db := storage.DB hist := &histogram.Histogram{ - Count: 10, + Count: 12, ZeroCount: 2, ZeroThreshold: 0.001, Sum: 39.4, @@ -359,6 +359,7 @@ func TestFederationWithNativeHistograms(t *testing.T) { }) default: hist.ZeroCount++ + hist.Count++ _, err = app.AppendHistogram(0, l, 100*60*1000, hist.Copy(), nil) expVec = append(expVec, promql.Sample{ T: 100 * 60 * 1000,