From b94f32f6fa3b619498dd6e1dec9677ab9d30e7de Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Sat, 11 Nov 2023 22:24:47 +0800 Subject: [PATCH 1/4] automatically reduce resolution rather than fail scrape Signed-off-by: Ziqi Zhao --- scrape/target.go | 16 ++++++++++++---- scrape/target_test.go | 5 +++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/scrape/target.go b/scrape/target.go index 8cc8597a4..ed4c598b7 100644 --- a/scrape/target.go +++ b/scrape/target.go @@ -366,13 +366,21 @@ type bucketLimitAppender struct { func (app *bucketLimitAppender) AppendHistogram(ref storage.SeriesRef, lset labels.Labels, t int64, h *histogram.Histogram, fh *histogram.FloatHistogram) (storage.SeriesRef, error) { if h != nil { - if len(h.PositiveBuckets)+len(h.NegativeBuckets) > app.limit { - return 0, errBucketLimit + for len(h.PositiveBuckets)+len(h.NegativeBuckets) > app.limit { + lastSum := len(h.PositiveBuckets) + len(h.NegativeBuckets) + h = h.ReduceResolution(h.Schema - 1) + if len(h.PositiveBuckets)+len(h.NegativeBuckets) == lastSum { + return 0, errBucketLimit + } } } if fh != nil { - if len(fh.PositiveBuckets)+len(fh.NegativeBuckets) > app.limit { - return 0, errBucketLimit + for len(fh.PositiveBuckets)+len(fh.NegativeBuckets) > app.limit { + lastSum := len(fh.PositiveBuckets) + len(fh.NegativeBuckets) + fh = fh.ReduceResolution(fh.Schema - 1) + if len(fh.PositiveBuckets)+len(fh.NegativeBuckets) == lastSum { + return 0, errBucketLimit + } } } ref, err := app.Appender.AppendHistogram(ref, lset, t, h, fh) diff --git a/scrape/target_test.go b/scrape/target_test.go index 4f0c840cd..f676b85c0 100644 --- a/scrape/target_test.go +++ b/scrape/target_test.go @@ -517,6 +517,11 @@ func TestBucketLimitAppender(t *testing.T) { limit: 3, expectError: true, }, + { + h: example, + limit: 4, + expectError: false, + }, { h: example, limit: 10, From 10ebeb0a629e7c4880347be18ae4e2d1850bdf2b Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Thu, 16 Nov 2023 13:00:11 +0800 Subject: [PATCH 2/4] rename lastSum -> lastCount && enrich test case with expectBucketCount and expectSchema Signed-off-by: Ziqi Zhao --- scrape/target.go | 8 +++---- scrape/target_test.go | 50 ++++++++++++++++++++++++++++--------------- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/scrape/target.go b/scrape/target.go index ed4c598b7..72b91a41e 100644 --- a/scrape/target.go +++ b/scrape/target.go @@ -367,18 +367,18 @@ type bucketLimitAppender struct { func (app *bucketLimitAppender) AppendHistogram(ref storage.SeriesRef, lset labels.Labels, t int64, h *histogram.Histogram, fh *histogram.FloatHistogram) (storage.SeriesRef, error) { if h != nil { for len(h.PositiveBuckets)+len(h.NegativeBuckets) > app.limit { - lastSum := len(h.PositiveBuckets) + len(h.NegativeBuckets) + lastCount := len(h.PositiveBuckets) + len(h.NegativeBuckets) h = h.ReduceResolution(h.Schema - 1) - if len(h.PositiveBuckets)+len(h.NegativeBuckets) == lastSum { + if len(h.PositiveBuckets)+len(h.NegativeBuckets) == lastCount { return 0, errBucketLimit } } } if fh != nil { for len(fh.PositiveBuckets)+len(fh.NegativeBuckets) > app.limit { - lastSum := len(fh.PositiveBuckets) + len(fh.NegativeBuckets) + lastCount := len(fh.PositiveBuckets) + len(fh.NegativeBuckets) fh = fh.ReduceResolution(fh.Schema - 1) - if len(fh.PositiveBuckets)+len(fh.NegativeBuckets) == lastSum { + if len(fh.PositiveBuckets)+len(fh.NegativeBuckets) == lastCount { return 0, errBucketLimit } } diff --git a/scrape/target_test.go b/scrape/target_test.go index f676b85c0..604cfa995 100644 --- a/scrape/target_test.go +++ b/scrape/target_test.go @@ -508,9 +508,11 @@ func TestBucketLimitAppender(t *testing.T) { } cases := []struct { - h histogram.Histogram - limit int - expectError bool + h histogram.Histogram + limit int + expectError bool + expectBucketCount int + expectSchema int32 }{ { h: example, @@ -518,14 +520,18 @@ func TestBucketLimitAppender(t *testing.T) { expectError: true, }, { - h: example, - limit: 4, - expectError: false, + h: example, + limit: 4, + expectError: false, + expectBucketCount: 4, + expectSchema: -1, }, { - h: example, - limit: 10, - expectError: false, + h: example, + limit: 10, + expectError: false, + expectBucketCount: 6, + expectSchema: 0, }, } @@ -536,18 +542,28 @@ func TestBucketLimitAppender(t *testing.T) { t.Run(fmt.Sprintf("floatHistogram=%t", floatHisto), func(t *testing.T) { app := &bucketLimitAppender{Appender: resApp, limit: c.limit} ts := int64(10 * time.Minute / time.Millisecond) - h := c.h lbls := labels.FromStrings("__name__", "sparse_histogram_series") var err error if floatHisto { - _, err = app.AppendHistogram(0, lbls, ts, nil, h.Copy().ToFloat()) + fh := c.h.Copy().ToFloat() + _, err = app.AppendHistogram(0, lbls, ts, nil, fh) + if c.expectError { + require.Error(t, err) + } else { + require.Equal(t, c.expectSchema, fh.Schema) + require.Equal(t, c.expectBucketCount, len(fh.NegativeBuckets)+len(fh.PositiveBuckets)) + require.NoError(t, err) + } } else { - _, err = app.AppendHistogram(0, lbls, ts, h.Copy(), nil) - } - if c.expectError { - require.Error(t, err) - } else { - require.NoError(t, err) + h := c.h.Copy() + _, err = app.AppendHistogram(0, lbls, ts, h, nil) + if c.expectError { + require.Error(t, err) + } else { + require.Equal(t, c.expectSchema, h.Schema) + require.Equal(t, c.expectBucketCount, len(h.NegativeBuckets)+len(h.PositiveBuckets)) + require.NoError(t, err) + } } require.NoError(t, app.Commit()) }) From 8fe9250f7d236bcb5a2b21e7357abad2e5dcb31a Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Tue, 21 Nov 2023 16:56:56 +0800 Subject: [PATCH 3/4] optimize the logic of break the loop of reducing resolution Signed-off-by: Ziqi Zhao --- scrape/scrape_test.go | 23 +++++++++++++++++++++++ scrape/target.go | 10 ++++------ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index a2e0d00c6..93e163101 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -1866,6 +1866,29 @@ func TestScrapeLoop_HistogramBucketLimit(t *testing.T) { msg, err = MetricFamilyToProtobuf(histogramMetricFamily) require.NoError(t, err) + now = time.Now() + total, added, seriesAdded, err = sl.append(app, msg, "application/vnd.google.protobuf", now) + require.NoError(t, err) + require.Equal(t, 3, total) + require.Equal(t, 3, added) + require.Equal(t, 3, seriesAdded) + + err = sl.metrics.targetScrapeNativeHistogramBucketLimit.Write(&metric) + require.NoError(t, err) + metricValue = metric.GetCounter().GetValue() + require.Equal(t, beforeMetricValue, metricValue) + beforeMetricValue = metricValue + + nativeHistogram.WithLabelValues("L").Observe(100000.0) // in different bucket since > 10*1.1 + + gathered, err = registry.Gather() + require.NoError(t, err) + require.NotEmpty(t, gathered) + + histogramMetricFamily = gathered[0] + msg, err = MetricFamilyToProtobuf(histogramMetricFamily) + require.NoError(t, err) + now = time.Now() total, added, seriesAdded, err = sl.append(app, msg, "application/vnd.google.protobuf", now) if !errors.Is(err, errBucketLimit) { diff --git a/scrape/target.go b/scrape/target.go index 72b91a41e..62f662693 100644 --- a/scrape/target.go +++ b/scrape/target.go @@ -367,20 +367,18 @@ type bucketLimitAppender struct { func (app *bucketLimitAppender) AppendHistogram(ref storage.SeriesRef, lset labels.Labels, t int64, h *histogram.Histogram, fh *histogram.FloatHistogram) (storage.SeriesRef, error) { if h != nil { for len(h.PositiveBuckets)+len(h.NegativeBuckets) > app.limit { - lastCount := len(h.PositiveBuckets) + len(h.NegativeBuckets) - h = h.ReduceResolution(h.Schema - 1) - if len(h.PositiveBuckets)+len(h.NegativeBuckets) == lastCount { + if h.Schema == -4 { return 0, errBucketLimit } + h = h.ReduceResolution(h.Schema - 1) } } if fh != nil { for len(fh.PositiveBuckets)+len(fh.NegativeBuckets) > app.limit { - lastCount := len(fh.PositiveBuckets) + len(fh.NegativeBuckets) - fh = fh.ReduceResolution(fh.Schema - 1) - if len(fh.PositiveBuckets)+len(fh.NegativeBuckets) == lastCount { + if fh.Schema == -4 { return 0, errBucketLimit } + fh = fh.ReduceResolution(fh.Schema - 1) } } ref, err := app.Appender.AppendHistogram(ref, lset, t, h, fh) From 19ecc5dd94247793409ffe35d5714ba20eb7f7dd Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Sun, 26 Nov 2023 22:17:37 +0800 Subject: [PATCH 4/4] add test case for bigGap Signed-off-by: Ziqi Zhao --- scrape/target_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/scrape/target_test.go b/scrape/target_test.go index 604cfa995..6631f328c 100644 --- a/scrape/target_test.go +++ b/scrape/target_test.go @@ -507,6 +507,19 @@ func TestBucketLimitAppender(t *testing.T) { NegativeBuckets: []int64{3, 0, 0}, } + bigGap := histogram.Histogram{ + Schema: 0, + Count: 21, + Sum: 33, + ZeroThreshold: 0.001, + ZeroCount: 3, + PositiveSpans: []histogram.Span{ + {Offset: 1, Length: 1}, // in (1, 2] + {Offset: 2, Length: 1}, // in (8, 16] + }, + PositiveBuckets: []int64{1, 0}, // 1, 1 + } + cases := []struct { h histogram.Histogram limit int @@ -533,6 +546,13 @@ func TestBucketLimitAppender(t *testing.T) { expectBucketCount: 6, expectSchema: 0, }, + { + h: bigGap, + limit: 1, + expectError: false, + expectBucketCount: 1, + expectSchema: -2, + }, } resApp := &collectResultAppender{}