From 62e7f0438d6b94b013af1312277aed82cea6c7f3 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 01/48] implement basic conversion of classic to nhcb in scrape Signed-off-by: Jeanette Tan --- config/config.go | 2 + promql/promqltest/test.go | 98 ++++++------------------------ scrape/scrape.go | 76 +++++++++++++++++++++++ scrape/scrape_test.go | 103 ++++++++++++++++++++++++++++++++ util/convertnhcb/convertnhcb.go | 92 ++++++++++++++++++++++++++++ 5 files changed, 290 insertions(+), 81 deletions(-) create mode 100644 util/convertnhcb/convertnhcb.go diff --git a/config/config.go b/config/config.go index 9defa10d4..561578978 100644 --- a/config/config.go +++ b/config/config.go @@ -617,6 +617,8 @@ type ScrapeConfig struct { ScrapeProtocols []ScrapeProtocol `yaml:"scrape_protocols,omitempty"` // Whether to scrape a classic histogram that is also exposed as a native histogram. ScrapeClassicHistograms bool `yaml:"scrape_classic_histograms,omitempty"` + // Whether to convert a scraped classic histogram into a native histogram with custom buckets. + ConvertClassicHistograms bool `yaml:"convert_classic_histograms,omitempty"` // The HTTP resource path on which to fetch metrics from targets. MetricsPath string `yaml:"metrics_path,omitempty"` // The URL scheme with which to fetch metrics from targets. diff --git a/promql/promqltest/test.go b/promql/promqltest/test.go index f3a773be8..576b30d5b 100644 --- a/promql/promqltest/test.go +++ b/promql/promqltest/test.go @@ -39,6 +39,7 @@ import ( "github.com/prometheus/prometheus/promql/parser/posrange" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/util/almost" + "github.com/prometheus/prometheus/util/convertnhcb" "github.com/prometheus/prometheus/util/teststorage" "github.com/prometheus/prometheus/util/testutil" ) @@ -460,43 +461,22 @@ func (cmd *loadCmd) append(a storage.Appender) error { return nil } -func getHistogramMetricBase(m labels.Labels, suffix string) (labels.Labels, uint64) { - mName := m.Get(labels.MetricName) - baseM := labels.NewBuilder(m). - Set(labels.MetricName, strings.TrimSuffix(mName, suffix)). - Del(labels.BucketLabel). - Labels() - hash := baseM.Hash() - return baseM, hash -} - type tempHistogramWrapper struct { metric labels.Labels upperBounds []float64 - histogramByTs map[int64]tempHistogram + histogramByTs map[int64]convertnhcb.TempHistogram } func newTempHistogramWrapper() tempHistogramWrapper { return tempHistogramWrapper{ upperBounds: []float64{}, - histogramByTs: map[int64]tempHistogram{}, - } -} - -type tempHistogram struct { - bucketCounts map[float64]float64 - count float64 - sum float64 -} - -func newTempHistogram() tempHistogram { - return tempHistogram{ - bucketCounts: map[float64]float64{}, + histogramByTs: map[int64]convertnhcb.TempHistogram{}, } } -func processClassicHistogramSeries(m labels.Labels, suffix string, histogramMap map[uint64]tempHistogramWrapper, smpls []promql.Sample, updateHistogramWrapper func(*tempHistogramWrapper), updateHistogram func(*tempHistogram, float64)) { - m2, m2hash := getHistogramMetricBase(m, suffix) +func processClassicHistogramSeries(m labels.Labels, suffix string, histogramMap map[uint64]tempHistogramWrapper, smpls []promql.Sample, updateHistogramWrapper func(*tempHistogramWrapper), updateHistogram func(*convertnhcb.TempHistogram, float64)) { + m2 := convertnhcb.GetHistogramMetricBase(m, suffix) + m2hash := m2.Hash() histogramWrapper, exists := histogramMap[m2hash] if !exists { histogramWrapper = newTempHistogramWrapper() @@ -511,7 +491,7 @@ func processClassicHistogramSeries(m labels.Labels, suffix string, histogramMap } histogram, exists := histogramWrapper.histogramByTs[s.T] if !exists { - histogram = newTempHistogram() + histogram = convertnhcb.NewTempHistogram() } updateHistogram(&histogram, s.F) histogramWrapper.histogramByTs[s.T] = histogram @@ -519,34 +499,6 @@ func processClassicHistogramSeries(m labels.Labels, suffix string, histogramMap histogramMap[m2hash] = histogramWrapper } -func processUpperBoundsAndCreateBaseHistogram(upperBounds0 []float64) ([]float64, *histogram.FloatHistogram) { - sort.Float64s(upperBounds0) - upperBounds := make([]float64, 0, len(upperBounds0)) - prevLE := math.Inf(-1) - for _, le := range upperBounds0 { - if le != prevLE { // deduplicate - upperBounds = append(upperBounds, le) - prevLE = le - } - } - var customBounds []float64 - if upperBounds[len(upperBounds)-1] == math.Inf(1) { - customBounds = upperBounds[:len(upperBounds)-1] - } else { - customBounds = upperBounds - } - return upperBounds, &histogram.FloatHistogram{ - Count: 0, - Sum: 0, - Schema: histogram.CustomBucketsSchema, - PositiveSpans: []histogram.Span{ - {Offset: 0, Length: uint32(len(upperBounds))}, - }, - PositiveBuckets: make([]float64, len(upperBounds)), - CustomValues: customBounds, - } -} - // If classic histograms are defined, convert them into native histograms with custom // bounds and append the defined time series to the storage. func (cmd *loadCmd) appendCustomHistogram(a storage.Appender) error { @@ -565,16 +517,16 @@ func (cmd *loadCmd) appendCustomHistogram(a storage.Appender) error { } processClassicHistogramSeries(m, "_bucket", histogramMap, smpls, func(histogramWrapper *tempHistogramWrapper) { histogramWrapper.upperBounds = append(histogramWrapper.upperBounds, le) - }, func(histogram *tempHistogram, f float64) { - histogram.bucketCounts[le] = f + }, func(histogram *convertnhcb.TempHistogram, f float64) { + histogram.BucketCounts[le] = f }) case strings.HasSuffix(mName, "_count"): - processClassicHistogramSeries(m, "_count", histogramMap, smpls, nil, func(histogram *tempHistogram, f float64) { - histogram.count = f + processClassicHistogramSeries(m, "_count", histogramMap, smpls, nil, func(histogram *convertnhcb.TempHistogram, f float64) { + histogram.Count = f }) case strings.HasSuffix(mName, "_sum"): - processClassicHistogramSeries(m, "_sum", histogramMap, smpls, nil, func(histogram *tempHistogram, f float64) { - histogram.sum = f + processClassicHistogramSeries(m, "_sum", histogramMap, smpls, nil, func(histogram *convertnhcb.TempHistogram, f float64) { + histogram.Sum = f }) } } @@ -582,30 +534,14 @@ func (cmd *loadCmd) appendCustomHistogram(a storage.Appender) error { // Convert the collated classic histogram data into native histograms // with custom bounds and append them to the storage. for _, histogramWrapper := range histogramMap { - upperBounds, fhBase := processUpperBoundsAndCreateBaseHistogram(histogramWrapper.upperBounds) + upperBounds, fhBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(histogramWrapper.upperBounds) samples := make([]promql.Sample, 0, len(histogramWrapper.histogramByTs)) for t, histogram := range histogramWrapper.histogramByTs { - fh := fhBase.Copy() - var prevCount, total float64 - for i, le := range upperBounds { - currCount, exists := histogram.bucketCounts[le] - if !exists { - currCount = 0 - } - count := currCount - prevCount - fh.PositiveBuckets[i] = count - total += count - prevCount = currCount - } - fh.Sum = histogram.sum - if histogram.count != 0 { - total = histogram.count - } - fh.Count = total - s := promql.Sample{T: t, H: fh.Compact(0)} - if err := s.H.Validate(); err != nil { + fh := convertnhcb.ConvertHistogramWrapper(histogram, upperBounds, fhBase) + if err := fh.Validate(); err != nil { return err } + s := promql.Sample{T: t, H: fh} samples = append(samples, s) } sort.Slice(samples, func(i, j int) bool { return samples[i].T < samples[j].T }) diff --git a/scrape/scrape.go b/scrape/scrape.go index a0b681444..c3005304e 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -47,6 +47,7 @@ import ( "github.com/prometheus/prometheus/model/timestamp" "github.com/prometheus/prometheus/model/value" "github.com/prometheus/prometheus/storage" + "github.com/prometheus/prometheus/util/convertnhcb" "github.com/prometheus/prometheus/util/pool" ) @@ -111,6 +112,7 @@ type scrapeLoopOptions struct { interval time.Duration timeout time.Duration scrapeClassicHistograms bool + convertClassicHistograms bool mrc []*relabel.Config cache *scrapeCache @@ -178,6 +180,7 @@ func newScrapePool(cfg *config.ScrapeConfig, app storage.Appendable, offsetSeed opts.interval, opts.timeout, opts.scrapeClassicHistograms, + opts.convertClassicHistograms, options.EnableNativeHistogramsIngestion, options.EnableCreatedTimestampZeroIngestion, options.ExtraMetrics, @@ -440,6 +443,7 @@ func (sp *scrapePool) sync(targets []*Target) { trackTimestampsStaleness = sp.config.TrackTimestampsStaleness mrc = sp.config.MetricRelabelConfigs scrapeClassicHistograms = sp.config.ScrapeClassicHistograms + convertClassicHistograms = sp.config.ConvertClassicHistograms ) sp.targetMtx.Lock() @@ -476,6 +480,7 @@ func (sp *scrapePool) sync(targets []*Target) { interval: interval, timeout: timeout, scrapeClassicHistograms: scrapeClassicHistograms, + convertClassicHistograms: convertClassicHistograms, }) if err != nil { l.setForcedError(err) @@ -828,6 +833,7 @@ type scrapeLoop struct { interval time.Duration timeout time.Duration scrapeClassicHistograms bool + convertClassicHistograms bool // Feature flagged options. enableNativeHistogramIngestion bool @@ -881,6 +887,9 @@ type scrapeCache struct { metadata map[string]*metaEntry metrics *scrapeMetrics + + nhcbLabels map[uint64]labels.Labels + nhcbBuilder map[uint64]convertnhcb.TempHistogram } // metaEntry holds meta information about a metric. @@ -904,6 +913,8 @@ func newScrapeCache(metrics *scrapeMetrics) *scrapeCache { seriesPrev: map[uint64]labels.Labels{}, metadata: map[string]*metaEntry{}, metrics: metrics, + nhcbLabels: map[uint64]labels.Labels{}, + nhcbBuilder: map[uint64]convertnhcb.TempHistogram{}, } } @@ -1107,6 +1118,11 @@ func (c *scrapeCache) LengthMetadata() int { return len(c.metadata) } +func (c *scrapeCache) resetNhcb() { + c.nhcbLabels = map[uint64]labels.Labels{} + c.nhcbBuilder = map[uint64]convertnhcb.TempHistogram{} +} + func newScrapeLoop(ctx context.Context, sc scraper, l log.Logger, @@ -1127,6 +1143,7 @@ func newScrapeLoop(ctx context.Context, interval time.Duration, timeout time.Duration, scrapeClassicHistograms bool, + convertClassicHistograms bool, enableNativeHistogramIngestion bool, enableCTZeroIngestion bool, reportExtraMetrics bool, @@ -1180,6 +1197,7 @@ func newScrapeLoop(ctx context.Context, interval: interval, timeout: timeout, scrapeClassicHistograms: scrapeClassicHistograms, + convertClassicHistograms: convertClassicHistograms, enableNativeHistogramIngestion: enableNativeHistogramIngestion, enableCTZeroIngestion: enableCTZeroIngestion, reportExtraMetrics: reportExtraMetrics, @@ -1641,6 +1659,27 @@ loop: } } else { ref, err = app.Append(ref, lset, t, val) + + if sl.convertClassicHistograms { + mName := lset.Get(labels.MetricName) + switch { + case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel): + le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64) + if err == nil && !math.IsNaN(le) { + processClassicHistogramSeries(lset, "_bucket", sl.cache, func(hist *convertnhcb.TempHistogram) { + hist.BucketCounts[le] = val + }) + } + case strings.HasSuffix(mName, "_count"): + processClassicHistogramSeries(lset, "_count", sl.cache, func(hist *convertnhcb.TempHistogram) { + hist.Count = val + }) + case strings.HasSuffix(mName, "_sum"): + processClassicHistogramSeries(lset, "_sum", sl.cache, func(hist *convertnhcb.TempHistogram) { + hist.Sum = val + }) + } + } } } @@ -1762,9 +1801,46 @@ loop: return err == nil }) } + + if sl.convertClassicHistograms { + for hash, th := range sl.cache.nhcbBuilder { + lset, ok := sl.cache.nhcbLabels[hash] + if !ok { + continue + } + ub := make([]float64, 0, len(th.BucketCounts)) + for b := range th.BucketCounts { + ub = append(ub, b) + } + upperBounds, fhBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(ub) + fh := convertnhcb.ConvertHistogramWrapper(th, upperBounds, fhBase) + if err := fh.Validate(); err != nil { + continue + } + // fmt.Printf("FINAL lset: %s, timestamp: %v, val: %v\n", lset, defTime, fh) + _, err = app.AppendHistogram(0, lset, defTime, nil, fh) + if err != nil { + continue + } + } + sl.cache.resetNhcb() + } + return } +func processClassicHistogramSeries(lset labels.Labels, suffix string, cache *scrapeCache, updateHist func(*convertnhcb.TempHistogram)) { + m2 := convertnhcb.GetHistogramMetricBase(lset, suffix) + m2hash := m2.Hash() + cache.nhcbLabels[m2hash] = m2 + th, exists := cache.nhcbBuilder[m2hash] + if !exists { + th = convertnhcb.NewTempHistogram() + } + updateHist(&th) + cache.nhcbBuilder[m2hash] = th +} + // Adds samples to the appender, checking the error, and then returns the # of samples added, // whether the caller should continue to process more samples, and any sample or bucket limit errors. func (sl *scrapeLoop) checkAddError(met []byte, err error, sampleLimitErr, bucketLimitErr *error, appErrs *appendErrors) (bool, error) { diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index a3fe6ac1a..bcdb455f7 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -679,6 +679,7 @@ func newBasicScrapeLoop(t testing.TB, ctx context.Context, scraper scraper, app false, false, false, + false, nil, false, newTestScrapeMetrics(t), @@ -821,6 +822,7 @@ func TestScrapeLoopRun(t *testing.T) { false, false, false, + false, nil, false, scrapeMetrics, @@ -965,6 +967,7 @@ func TestScrapeLoopMetadata(t *testing.T) { false, false, false, + false, nil, false, scrapeMetrics, @@ -3366,6 +3369,106 @@ test_summary_count 199 checkValues("quantile", expectedQuantileValues, series) } +// Testing whether we can automatically convert scraped classic histograms into native histograms with custom buckets. +func TestConvertClassicHistograms(t *testing.T) { + simpleStorage := teststorage.New(t) + defer simpleStorage.Close() + + config := &config.ScrapeConfig{ + JobName: "test", + SampleLimit: 100, + Scheme: "http", + ScrapeInterval: model.Duration(100 * time.Millisecond), + ScrapeTimeout: model.Duration(100 * time.Millisecond), + ConvertClassicHistograms: true, + } + + metricsText := ` +# HELP test_histogram This is a histogram with default buckets +# TYPE test_histogram histogram +test_histogram_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 +test_histogram_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 +test_histogram_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 +test_histogram_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 +test_histogram_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 +test_histogram_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 +test_histogram_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 +test_histogram_bucket{address="0.0.0.0",port="5001",le="1"} 0 +test_histogram_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 +test_histogram_bucket{address="0.0.0.0",port="5001",le="5"} 0 +test_histogram_bucket{address="0.0.0.0",port="5001",le="10"} 1 +test_histogram_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 +test_histogram_sum{address="0.0.0.0",port="5001"} 10 +test_histogram_count{address="0.0.0.0",port="5001"} 1 +` + + // The expected "le" values do not have the trailing ".0". + expectedLeValues := []string{"0.005", "0.01", "0.025", "0.05", "0.1", "0.25", "0.5", "1", "2.5", "5", "10", "+Inf"} + + scrapeCount := 0 + scraped := make(chan bool) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, metricsText) + scrapeCount++ + if scrapeCount > 2 { + close(scraped) + } + })) + defer ts.Close() + + sp, err := newScrapePool(config, simpleStorage, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) + require.NoError(t, err) + defer sp.stop() + + testURL, err := url.Parse(ts.URL) + require.NoError(t, err) + sp.Sync([]*targetgroup.Group{ + { + Targets: []model.LabelSet{{model.AddressLabel: model.LabelValue(testURL.Host)}}, + }, + }) + require.Len(t, sp.ActiveTargets(), 1) + + select { + case <-time.After(5 * time.Second): + t.Fatalf("target was not scraped") + case <-scraped: + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + q, err := simpleStorage.Querier(time.Time{}.UnixNano(), time.Now().UnixNano()) + require.NoError(t, err) + defer q.Close() + + checkValues := func(labelName string, expectedValues []string, series storage.SeriesSet) { + foundLeValues := map[string]bool{} + + for series.Next() { + s := series.At() + v := s.Labels().Get(labelName) + require.NotContains(t, foundLeValues, v, "duplicate label value found") + foundLeValues[v] = true + } + + require.Equal(t, len(expectedValues), len(foundLeValues), "number of label values not as expected") + for _, v := range expectedValues { + require.Contains(t, foundLeValues, v, "label value not found") + } + } + + series := q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_bucket")) + checkValues("le", expectedLeValues, series) + + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram")) + count := 0 + for series.Next() { + count++ + } + require.Equal(t, 1, count, "number of series not as expected") +} + func TestScrapeLoopRunCreatesStaleMarkersOnFailedScrapeForTimestampedMetrics(t *testing.T) { appender := &collectResultAppender{} var ( diff --git a/util/convertnhcb/convertnhcb.go b/util/convertnhcb/convertnhcb.go new file mode 100644 index 000000000..8a9655386 --- /dev/null +++ b/util/convertnhcb/convertnhcb.go @@ -0,0 +1,92 @@ +// Copyright 2024 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package convertnhcb + +import ( + "math" + "sort" + "strings" + + "github.com/prometheus/prometheus/model/histogram" + "github.com/prometheus/prometheus/model/labels" +) + +type TempHistogram struct { + BucketCounts map[float64]float64 + Count float64 + Sum float64 +} + +func NewTempHistogram() TempHistogram { + return TempHistogram{ + BucketCounts: map[float64]float64{}, + } +} + +func ProcessUpperBoundsAndCreateBaseHistogram(upperBounds0 []float64) ([]float64, *histogram.FloatHistogram) { + sort.Float64s(upperBounds0) + upperBounds := make([]float64, 0, len(upperBounds0)) + prevLE := math.Inf(-1) + for _, le := range upperBounds0 { + if le != prevLE { // deduplicate + upperBounds = append(upperBounds, le) + prevLE = le + } + } + var customBounds []float64 + if upperBounds[len(upperBounds)-1] == math.Inf(1) { + customBounds = upperBounds[:len(upperBounds)-1] + } else { + customBounds = upperBounds + } + return upperBounds, &histogram.FloatHistogram{ + Count: 0, + Sum: 0, + Schema: histogram.CustomBucketsSchema, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: uint32(len(upperBounds))}, + }, + PositiveBuckets: make([]float64, len(upperBounds)), + CustomValues: customBounds, + } +} + +func ConvertHistogramWrapper(hist TempHistogram, upperBounds []float64, fhBase *histogram.FloatHistogram) *histogram.FloatHistogram { + fh := fhBase.Copy() + var prevCount, total float64 + for i, le := range upperBounds { + currCount, exists := hist.BucketCounts[le] + if !exists { + currCount = 0 + } + count := currCount - prevCount + fh.PositiveBuckets[i] = count + total += count + prevCount = currCount + } + fh.Sum = hist.Sum + if hist.Count != 0 { + total = hist.Count + } + fh.Count = total + return fh.Compact(0) +} + +func GetHistogramMetricBase(m labels.Labels, suffix string) labels.Labels { + mName := m.Get(labels.MetricName) + return labels.NewBuilder(m). + Set(labels.MetricName, strings.TrimSuffix(mName, suffix)). + Del(labels.BucketLabel). + Labels() +} From 4503145c8be0ad3691ea53b76113e5a4b7daadca Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 02/48] convert classic histograms to int nhcb where possible instead Signed-off-by: Jeanette Tan --- promql/promqltest/test.go | 11 ++++- scrape/scrape.go | 25 ++++++---- util/convertnhcb/convertnhcb.go | 81 +++++++++++++++++++++++++++------ 3 files changed, 93 insertions(+), 24 deletions(-) diff --git a/promql/promqltest/test.go b/promql/promqltest/test.go index 576b30d5b..7e6554312 100644 --- a/promql/promqltest/test.go +++ b/promql/promqltest/test.go @@ -534,10 +534,17 @@ func (cmd *loadCmd) appendCustomHistogram(a storage.Appender) error { // Convert the collated classic histogram data into native histograms // with custom bounds and append them to the storage. for _, histogramWrapper := range histogramMap { - upperBounds, fhBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(histogramWrapper.upperBounds) + upperBounds, hBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(histogramWrapper.upperBounds, true) + fhBase := hBase.ToFloat(nil) samples := make([]promql.Sample, 0, len(histogramWrapper.histogramByTs)) for t, histogram := range histogramWrapper.histogramByTs { - fh := convertnhcb.ConvertHistogramWrapper(histogram, upperBounds, fhBase) + h, fh := convertnhcb.ConvertHistogramWrapper(histogram, upperBounds, hBase, fhBase) + if fh == nil { + if err := h.Validate(); err != nil { + return err + } + fh = h.ToFloat(nil) + } if err := fh.Validate(); err != nil { return err } diff --git a/scrape/scrape.go b/scrape/scrape.go index c3005304e..16766a352 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -1812,15 +1812,24 @@ loop: for b := range th.BucketCounts { ub = append(ub, b) } - upperBounds, fhBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(ub) - fh := convertnhcb.ConvertHistogramWrapper(th, upperBounds, fhBase) - if err := fh.Validate(); err != nil { - continue - } + upperBounds, hBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(ub, false) + fhBase := hBase.ToFloat(nil) + h, fh := convertnhcb.ConvertHistogramWrapper(th, upperBounds, hBase, fhBase) // fmt.Printf("FINAL lset: %s, timestamp: %v, val: %v\n", lset, defTime, fh) - _, err = app.AppendHistogram(0, lset, defTime, nil, fh) - if err != nil { - continue + if h != nil { + if err := h.Validate(); err != nil { + continue + } + if _, err = app.AppendHistogram(0, lset, defTime, h, nil); err != nil { + continue + } + } else if fh != nil { + if err := fh.Validate(); err != nil { + continue + } + if _, err = app.AppendHistogram(0, lset, defTime, nil, fh); err != nil { + continue + } } } sl.cache.resetNhcb() diff --git a/util/convertnhcb/convertnhcb.go b/util/convertnhcb/convertnhcb.go index 8a9655386..face43628 100644 --- a/util/convertnhcb/convertnhcb.go +++ b/util/convertnhcb/convertnhcb.go @@ -14,6 +14,7 @@ package convertnhcb import ( + "fmt" "math" "sort" "strings" @@ -26,6 +27,7 @@ type TempHistogram struct { BucketCounts map[float64]float64 Count float64 Sum float64 + HasFloat bool } func NewTempHistogram() TempHistogram { @@ -34,15 +36,32 @@ func NewTempHistogram() TempHistogram { } } -func ProcessUpperBoundsAndCreateBaseHistogram(upperBounds0 []float64) ([]float64, *histogram.FloatHistogram) { +func (h TempHistogram) getIntBucketCounts() (map[float64]int64, error) { + bucketCounts := map[float64]int64{} + for le, count := range h.BucketCounts { + intCount := int64(math.Round(count)) + if float64(intCount) != count { + return nil, fmt.Errorf("bucket count %f for le %g is not an integer", count, le) + } + bucketCounts[le] = intCount + } + return bucketCounts, nil +} + +func ProcessUpperBoundsAndCreateBaseHistogram(upperBounds0 []float64, needsDedup bool) ([]float64, *histogram.Histogram) { sort.Float64s(upperBounds0) - upperBounds := make([]float64, 0, len(upperBounds0)) - prevLE := math.Inf(-1) - for _, le := range upperBounds0 { - if le != prevLE { // deduplicate - upperBounds = append(upperBounds, le) - prevLE = le + var upperBounds []float64 + if needsDedup { + upperBounds = make([]float64, 0, len(upperBounds0)) + prevLE := math.Inf(-1) + for _, le := range upperBounds0 { + if le != prevLE { + upperBounds = append(upperBounds, le) + prevLE = le + } } + } else { + upperBounds = upperBounds0 } var customBounds []float64 if upperBounds[len(upperBounds)-1] == math.Inf(1) { @@ -50,23 +69,57 @@ func ProcessUpperBoundsAndCreateBaseHistogram(upperBounds0 []float64) ([]float64 } else { customBounds = upperBounds } - return upperBounds, &histogram.FloatHistogram{ + return upperBounds, &histogram.Histogram{ Count: 0, Sum: 0, Schema: histogram.CustomBucketsSchema, PositiveSpans: []histogram.Span{ {Offset: 0, Length: uint32(len(upperBounds))}, }, - PositiveBuckets: make([]float64, len(upperBounds)), + PositiveBuckets: make([]int64, len(upperBounds)), CustomValues: customBounds, } } -func ConvertHistogramWrapper(hist TempHistogram, upperBounds []float64, fhBase *histogram.FloatHistogram) *histogram.FloatHistogram { +func ConvertHistogramWrapper(histogram TempHistogram, upperBounds []float64, hBase *histogram.Histogram, fhBase *histogram.FloatHistogram) (*histogram.Histogram, *histogram.FloatHistogram) { + intBucketCounts, err := histogram.getIntBucketCounts() + if err != nil { + return nil, convertFloatHistogramWrapper(histogram, upperBounds, histogram.BucketCounts, fhBase) + } + return convertIntHistogramWrapper(histogram, upperBounds, intBucketCounts, hBase), nil +} + +func convertIntHistogramWrapper(histogram TempHistogram, upperBounds []float64, bucketCounts map[float64]int64, hBase *histogram.Histogram) *histogram.Histogram { + h := hBase.Copy() + absBucketCounts := make([]int64, len(h.PositiveBuckets)) + var prevCount, total int64 + for i, le := range upperBounds { + currCount, exists := bucketCounts[le] + if !exists { + currCount = 0 + } + count := currCount - prevCount + absBucketCounts[i] = count + total += count + prevCount = currCount + } + h.PositiveBuckets[0] = absBucketCounts[0] + for i := 1; i < len(h.PositiveBuckets); i++ { + h.PositiveBuckets[i] = absBucketCounts[i] - absBucketCounts[i-1] + } + h.Sum = histogram.Sum + if histogram.Count != 0 { + total = int64(histogram.Count) + } + h.Count = uint64(total) + return h.Compact(0) +} + +func convertFloatHistogramWrapper(histogram TempHistogram, upperBounds []float64, bucketCounts map[float64]float64, fhBase *histogram.FloatHistogram) *histogram.FloatHistogram { fh := fhBase.Copy() var prevCount, total float64 for i, le := range upperBounds { - currCount, exists := hist.BucketCounts[le] + currCount, exists := bucketCounts[le] if !exists { currCount = 0 } @@ -75,9 +128,9 @@ func ConvertHistogramWrapper(hist TempHistogram, upperBounds []float64, fhBase * total += count prevCount = currCount } - fh.Sum = hist.Sum - if hist.Count != 0 { - total = hist.Count + fh.Sum = histogram.Sum + if histogram.Count != 0 { + total = histogram.Count } fh.Count = total return fh.Compact(0) From 0a321fe4d8b8423b831d6e71c4a87a4235db49ba Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 03/48] improve new scrape test Signed-off-by: Jeanette Tan --- scrape/scrape_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index bcdb455f7..13685bf49 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3464,6 +3464,12 @@ test_histogram_count{address="0.0.0.0",port="5001"} 1 series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram")) count := 0 for series.Next() { + i := series.At().Iterator(nil) + for i.Next() == chunkenc.ValHistogram { + _, h := i.AtHistogram(nil) + require.Equal(t, uint64(1), h.Count) + require.Equal(t, 10.0, h.Sum) + } count++ } require.Equal(t, 1, count, "number of series not as expected") From 02d5abf60e92fdf4305fae667ea8ed2f6ae7b94a Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 04/48] don't use cache for nhcb maps Signed-off-by: Jeanette Tan --- scrape/scrape.go | 79 ++++++++++++++++++++++-------------------------- 1 file changed, 36 insertions(+), 43 deletions(-) diff --git a/scrape/scrape.go b/scrape/scrape.go index 16766a352..31dbb1cf9 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -887,9 +887,6 @@ type scrapeCache struct { metadata map[string]*metaEntry metrics *scrapeMetrics - - nhcbLabels map[uint64]labels.Labels - nhcbBuilder map[uint64]convertnhcb.TempHistogram } // metaEntry holds meta information about a metric. @@ -913,8 +910,6 @@ func newScrapeCache(metrics *scrapeMetrics) *scrapeCache { seriesPrev: map[uint64]labels.Labels{}, metadata: map[string]*metaEntry{}, metrics: metrics, - nhcbLabels: map[uint64]labels.Labels{}, - nhcbBuilder: map[uint64]convertnhcb.TempHistogram{}, } } @@ -1118,11 +1113,6 @@ func (c *scrapeCache) LengthMetadata() int { return len(c.metadata) } -func (c *scrapeCache) resetNhcb() { - c.nhcbLabels = map[uint64]labels.Labels{} - c.nhcbBuilder = map[uint64]convertnhcb.TempHistogram{} -} - func newScrapeLoop(ctx context.Context, sc scraper, l log.Logger, @@ -1500,6 +1490,8 @@ func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string, e exemplar.Exemplar // escapes to heap so hoisted out of loop meta metadata.Metadata metadataChanged bool + nhcbLabels map[uint64]labels.Labels + nhcbBuilder map[uint64]convertnhcb.TempHistogram ) exemplars := make([]exemplar.Exemplar, 1) @@ -1529,6 +1521,11 @@ func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string, // Take an appender with limits. app = appender(app, sl.sampleLimit, sl.bucketLimit, sl.maxSchema) + if sl.convertClassicHistograms { + nhcbLabels = make(map[uint64]labels.Labels) + nhcbBuilder = make(map[uint64]convertnhcb.TempHistogram) + } + defer func() { if err != nil { return @@ -1666,16 +1663,16 @@ loop: case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel): le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64) if err == nil && !math.IsNaN(le) { - processClassicHistogramSeries(lset, "_bucket", sl.cache, func(hist *convertnhcb.TempHistogram) { + processClassicHistogramSeries(lset, "_bucket", nhcbLabels, nhcbBuilder, func(hist *convertnhcb.TempHistogram) { hist.BucketCounts[le] = val }) } case strings.HasSuffix(mName, "_count"): - processClassicHistogramSeries(lset, "_count", sl.cache, func(hist *convertnhcb.TempHistogram) { + processClassicHistogramSeries(lset, "_count", nhcbLabels, nhcbBuilder, func(hist *convertnhcb.TempHistogram) { hist.Count = val }) case strings.HasSuffix(mName, "_sum"): - processClassicHistogramSeries(lset, "_sum", sl.cache, func(hist *convertnhcb.TempHistogram) { + processClassicHistogramSeries(lset, "_sum", nhcbLabels, nhcbBuilder, func(hist *convertnhcb.TempHistogram) { hist.Sum = val }) } @@ -1802,52 +1799,48 @@ loop: }) } - if sl.convertClassicHistograms { - for hash, th := range sl.cache.nhcbBuilder { - lset, ok := sl.cache.nhcbLabels[hash] - if !ok { + for hash, th := range nhcbBuilder { + lset, ok := nhcbLabels[hash] + if !ok { + continue + } + ub := make([]float64, 0, len(th.BucketCounts)) + for b := range th.BucketCounts { + ub = append(ub, b) + } + upperBounds, hBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(ub, false) + fhBase := hBase.ToFloat(nil) + h, fh := convertnhcb.ConvertHistogramWrapper(th, upperBounds, hBase, fhBase) + if h != nil { + if err := h.Validate(); err != nil { continue } - ub := make([]float64, 0, len(th.BucketCounts)) - for b := range th.BucketCounts { - ub = append(ub, b) + if _, err = app.AppendHistogram(0, lset, defTime, h, nil); err != nil { + continue } - upperBounds, hBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(ub, false) - fhBase := hBase.ToFloat(nil) - h, fh := convertnhcb.ConvertHistogramWrapper(th, upperBounds, hBase, fhBase) - // fmt.Printf("FINAL lset: %s, timestamp: %v, val: %v\n", lset, defTime, fh) - if h != nil { - if err := h.Validate(); err != nil { - continue - } - if _, err = app.AppendHistogram(0, lset, defTime, h, nil); err != nil { - continue - } - } else if fh != nil { - if err := fh.Validate(); err != nil { - continue - } - if _, err = app.AppendHistogram(0, lset, defTime, nil, fh); err != nil { - continue - } + } else if fh != nil { + if err := fh.Validate(); err != nil { + continue + } + if _, err = app.AppendHistogram(0, lset, defTime, nil, fh); err != nil { + continue } } - sl.cache.resetNhcb() } return } -func processClassicHistogramSeries(lset labels.Labels, suffix string, cache *scrapeCache, updateHist func(*convertnhcb.TempHistogram)) { +func processClassicHistogramSeries(lset labels.Labels, suffix string, nhcbLabels map[uint64]labels.Labels, nhcbBuilder map[uint64]convertnhcb.TempHistogram, updateHist func(*convertnhcb.TempHistogram)) { m2 := convertnhcb.GetHistogramMetricBase(lset, suffix) m2hash := m2.Hash() - cache.nhcbLabels[m2hash] = m2 - th, exists := cache.nhcbBuilder[m2hash] + nhcbLabels[m2hash] = m2 + th, exists := nhcbBuilder[m2hash] if !exists { th = convertnhcb.NewTempHistogram() } updateHist(&th) - cache.nhcbBuilder[m2hash] = th + nhcbBuilder[m2hash] = th } // Adds samples to the appender, checking the error, and then returns the # of samples added, From f596f17024e58f284afd99b557c70c7ecceef0ea Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 05/48] allow option to convert classic histograms to nhcb entirely (don't append classic histogram series) Signed-off-by: Jeanette Tan --- scrape/scrape.go | 12 ++++++++-- scrape/scrape_test.go | 42 ++++++++++++++++++++++++--------- util/convertnhcb/convertnhcb.go | 27 +++++++++++++++++++++ 3 files changed, 68 insertions(+), 13 deletions(-) diff --git a/scrape/scrape.go b/scrape/scrape.go index 31dbb1cf9..39e3b0317 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -1655,10 +1655,15 @@ loop: ref, err = app.AppendHistogram(ref, lset, t, nil, fh) } } else { - ref, err = app.Append(ref, lset, t, val) - + var skipAppendFloat bool if sl.convertClassicHistograms { mName := lset.Get(labels.MetricName) + if !sl.scrapeClassicHistograms { + baseMetadata, _ := sl.cache.GetMetadata(convertnhcb.GetHistogramMetricBaseName(mName)) + if baseMetadata.Type == model.MetricTypeHistogram { + skipAppendFloat = true + } + } switch { case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel): le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64) @@ -1677,6 +1682,9 @@ loop: }) } } + if !skipAppendFloat { + ref, err = app.Append(ref, lset, t, val) + } } } diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 13685bf49..bfab0175b 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3380,10 +3380,14 @@ func TestConvertClassicHistograms(t *testing.T) { Scheme: "http", ScrapeInterval: model.Duration(100 * time.Millisecond), ScrapeTimeout: model.Duration(100 * time.Millisecond), + ScrapeClassicHistograms: true, ConvertClassicHistograms: true, } metricsText := ` +# HELP test_metric some help text +# TYPE test_metric counter +test_metric 1 # HELP test_histogram This is a histogram with default buckets # TYPE test_histogram histogram test_histogram_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 @@ -3458,21 +3462,37 @@ test_histogram_count{address="0.0.0.0",port="5001"} 1 } } + // Checks that the expected series is present and runs a basic sanity check of the values. + checkSeries := func(series storage.SeriesSet, encType chunkenc.ValueType) { + count := 0 + for series.Next() { + i := series.At().Iterator(nil) + switch encType { + case chunkenc.ValFloat: + for i.Next() == encType { + _, f := i.At() + require.Equal(t, 1., f) + } + case chunkenc.ValHistogram: + for i.Next() == encType { + _, h := i.AtHistogram(nil) + require.Equal(t, uint64(1), h.Count) + require.Equal(t, 10.0, h.Sum) + } + } + count++ + } + require.Equal(t, 1, count, "number of series not as expected") + } + series := q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_bucket")) checkValues("le", expectedLeValues, series) series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram")) - count := 0 - for series.Next() { - i := series.At().Iterator(nil) - for i.Next() == chunkenc.ValHistogram { - _, h := i.AtHistogram(nil) - require.Equal(t, uint64(1), h.Count) - require.Equal(t, 10.0, h.Sum) - } - count++ - } - require.Equal(t, 1, count, "number of series not as expected") + checkSeries(series, chunkenc.ValHistogram) + + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_metric")) + checkSeries(series, chunkenc.ValFloat) } func TestScrapeLoopRunCreatesStaleMarkersOnFailedScrapeForTimestampedMetrics(t *testing.T) { diff --git a/util/convertnhcb/convertnhcb.go b/util/convertnhcb/convertnhcb.go index face43628..2e71a242c 100644 --- a/util/convertnhcb/convertnhcb.go +++ b/util/convertnhcb/convertnhcb.go @@ -19,10 +19,30 @@ import ( "sort" "strings" + "github.com/grafana/regexp" + "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" ) +var histogramNameSuffixReplacements = []struct { + pattern *regexp.Regexp + repl string +}{ + { + pattern: regexp.MustCompile(`_bucket$`), + repl: "", + }, + { + pattern: regexp.MustCompile(`_sum$`), + repl: "", + }, + { + pattern: regexp.MustCompile(`_count$`), + repl: "", + }, +} + type TempHistogram struct { BucketCounts map[float64]float64 Count float64 @@ -143,3 +163,10 @@ func GetHistogramMetricBase(m labels.Labels, suffix string) labels.Labels { Del(labels.BucketLabel). Labels() } + +func GetHistogramMetricBaseName(s string) string { + for _, rep := range histogramNameSuffixReplacements { + s = rep.pattern.ReplaceAllString(s, rep.repl) + } + return s +} From 172d4f24051208843bcbbd2b7366f04fbbfc27b5 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 06/48] insert nhcb parser as intermediate layer Signed-off-by: Jeanette Tan --- model/textparse/nhcbparse.go | 188 +++++++++++++++++++++++++++++++++++ scrape/scrape.go | 84 +--------------- 2 files changed, 192 insertions(+), 80 deletions(-) create mode 100644 model/textparse/nhcbparse.go diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go new file mode 100644 index 000000000..1b595351d --- /dev/null +++ b/model/textparse/nhcbparse.go @@ -0,0 +1,188 @@ +// Copyright 2024 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package textparse + +import ( + "errors" + "io" + "math" + "strconv" + "strings" + + "github.com/prometheus/common/model" + + "github.com/prometheus/prometheus/model/exemplar" + "github.com/prometheus/prometheus/model/histogram" + "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/util/convertnhcb" +) + +type NhcbParser struct { + parser Parser + keepClassicHistograms bool + + bytes []byte + ts *int64 + value float64 + h *histogram.Histogram + fh *histogram.FloatHistogram + + lset labels.Labels + metricString string + + buf []byte + + nhcbLabels map[uint64]labels.Labels + nhcbBuilder map[uint64]convertnhcb.TempHistogram +} + +func NewNhcbParser(p Parser, keepClassicHistograms bool) Parser { + return &NhcbParser{ + parser: p, + keepClassicHistograms: keepClassicHistograms, + buf: make([]byte, 0, 1024), + nhcbLabels: make(map[uint64]labels.Labels), + nhcbBuilder: make(map[uint64]convertnhcb.TempHistogram), + } +} + +func (p *NhcbParser) Series() ([]byte, *int64, float64) { + return p.bytes, p.ts, p.value +} + +func (p *NhcbParser) Histogram() ([]byte, *int64, *histogram.Histogram, *histogram.FloatHistogram) { + return p.bytes, p.ts, p.h, p.fh +} + +func (p *NhcbParser) Help() ([]byte, []byte) { + return p.parser.Help() +} + +func (p *NhcbParser) Type() ([]byte, model.MetricType) { + return p.parser.Type() +} + +func (p *NhcbParser) Unit() ([]byte, []byte) { + return p.parser.Unit() +} + +func (p *NhcbParser) Comment() []byte { + return p.parser.Comment() +} + +func (p *NhcbParser) Metric(l *labels.Labels) string { + *l = p.lset + return p.metricString +} + +func (p *NhcbParser) Exemplar(ex *exemplar.Exemplar) bool { + return p.parser.Exemplar(ex) +} + +func (p *NhcbParser) CreatedTimestamp() *int64 { + return p.parser.CreatedTimestamp() +} + +func (p *NhcbParser) Next() (Entry, error) { + et, err := p.parser.Next() + if errors.Is(err, io.EOF) { + if len(p.nhcbBuilder) > 0 { + p.processNhcb() + return EntryHistogram, nil + } + return EntryInvalid, err + } + switch et { + case EntrySeries: + p.bytes, p.ts, p.value = p.parser.Series() + p.metricString = p.parser.Metric(&p.lset) + if isNhcb := p.handleClassicHistogramSeries(p.lset); isNhcb && !p.keepClassicHistograms { + return p.Next() + } + case EntryHistogram: + p.bytes, p.ts, p.h, p.fh = p.parser.Histogram() + p.metricString = p.parser.Metric(&p.lset) + } + return et, err +} + +func (p *NhcbParser) handleClassicHistogramSeries(lset labels.Labels) bool { + mName := lset.Get(labels.MetricName) + switch { + case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel): + le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64) + if err == nil && !math.IsNaN(le) { + processClassicHistogramSeries(lset, "_bucket", p.nhcbLabels, p.nhcbBuilder, func(hist *convertnhcb.TempHistogram) { + hist.BucketCounts[le] = p.value + }) + return true + } + case strings.HasSuffix(mName, "_count"): + processClassicHistogramSeries(lset, "_count", p.nhcbLabels, p.nhcbBuilder, func(hist *convertnhcb.TempHistogram) { + hist.Count = p.value + }) + return true + case strings.HasSuffix(mName, "_sum"): + processClassicHistogramSeries(lset, "_sum", p.nhcbLabels, p.nhcbBuilder, func(hist *convertnhcb.TempHistogram) { + hist.Sum = p.value + }) + return true + } + return false +} + +func processClassicHistogramSeries(lset labels.Labels, suffix string, nhcbLabels map[uint64]labels.Labels, nhcbBuilder map[uint64]convertnhcb.TempHistogram, updateHist func(*convertnhcb.TempHistogram)) { + m2 := convertnhcb.GetHistogramMetricBase(lset, suffix) + m2hash := m2.Hash() + nhcbLabels[m2hash] = m2 + th, exists := nhcbBuilder[m2hash] + if !exists { + th = convertnhcb.NewTempHistogram() + } + updateHist(&th) + nhcbBuilder[m2hash] = th +} + +func (p *NhcbParser) processNhcb() { + for hash, th := range p.nhcbBuilder { + lset, ok := p.nhcbLabels[hash] + if !ok { + continue + } + ub := make([]float64, 0, len(th.BucketCounts)) + for b := range th.BucketCounts { + ub = append(ub, b) + } + upperBounds, hBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(ub, false) + fhBase := hBase.ToFloat(nil) + h, fh := convertnhcb.ConvertHistogramWrapper(th, upperBounds, hBase, fhBase) + if h != nil { + if err := h.Validate(); err != nil { + panic("histogram validation failed") + } + p.h = h + p.fh = nil + } else if fh != nil { + if err := fh.Validate(); err != nil { + panic("histogram validation failed") + } + p.h = nil + p.fh = fh + } + p.bytes = lset.Bytes(p.buf) + p.lset = lset + p.metricString = lset.String() + } + p.nhcbBuilder = map[uint64]convertnhcb.TempHistogram{} +} diff --git a/scrape/scrape.go b/scrape/scrape.go index 39e3b0317..551059a8e 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -47,7 +47,6 @@ import ( "github.com/prometheus/prometheus/model/timestamp" "github.com/prometheus/prometheus/model/value" "github.com/prometheus/prometheus/storage" - "github.com/prometheus/prometheus/util/convertnhcb" "github.com/prometheus/prometheus/util/pool" ) @@ -1473,6 +1472,9 @@ type appendErrors struct { func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string, ts time.Time) (total, added, seriesAdded int, err error) { p, err := textparse.New(b, contentType, sl.scrapeClassicHistograms, sl.symbolTable) + if sl.convertClassicHistograms { + p = textparse.NewNhcbParser(p, sl.scrapeClassicHistograms) + } if err != nil { level.Debug(sl.l).Log( "msg", "Invalid content type on scrape, using prometheus parser as fallback.", @@ -1490,8 +1492,6 @@ func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string, e exemplar.Exemplar // escapes to heap so hoisted out of loop meta metadata.Metadata metadataChanged bool - nhcbLabels map[uint64]labels.Labels - nhcbBuilder map[uint64]convertnhcb.TempHistogram ) exemplars := make([]exemplar.Exemplar, 1) @@ -1521,11 +1521,6 @@ func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string, // Take an appender with limits. app = appender(app, sl.sampleLimit, sl.bucketLimit, sl.maxSchema) - if sl.convertClassicHistograms { - nhcbLabels = make(map[uint64]labels.Labels) - nhcbBuilder = make(map[uint64]convertnhcb.TempHistogram) - } - defer func() { if err != nil { return @@ -1655,36 +1650,7 @@ loop: ref, err = app.AppendHistogram(ref, lset, t, nil, fh) } } else { - var skipAppendFloat bool - if sl.convertClassicHistograms { - mName := lset.Get(labels.MetricName) - if !sl.scrapeClassicHistograms { - baseMetadata, _ := sl.cache.GetMetadata(convertnhcb.GetHistogramMetricBaseName(mName)) - if baseMetadata.Type == model.MetricTypeHistogram { - skipAppendFloat = true - } - } - switch { - case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel): - le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64) - if err == nil && !math.IsNaN(le) { - processClassicHistogramSeries(lset, "_bucket", nhcbLabels, nhcbBuilder, func(hist *convertnhcb.TempHistogram) { - hist.BucketCounts[le] = val - }) - } - case strings.HasSuffix(mName, "_count"): - processClassicHistogramSeries(lset, "_count", nhcbLabels, nhcbBuilder, func(hist *convertnhcb.TempHistogram) { - hist.Count = val - }) - case strings.HasSuffix(mName, "_sum"): - processClassicHistogramSeries(lset, "_sum", nhcbLabels, nhcbBuilder, func(hist *convertnhcb.TempHistogram) { - hist.Sum = val - }) - } - } - if !skipAppendFloat { - ref, err = app.Append(ref, lset, t, val) - } + ref, err = app.Append(ref, lset, t, val) } } @@ -1806,51 +1772,9 @@ loop: return err == nil }) } - - for hash, th := range nhcbBuilder { - lset, ok := nhcbLabels[hash] - if !ok { - continue - } - ub := make([]float64, 0, len(th.BucketCounts)) - for b := range th.BucketCounts { - ub = append(ub, b) - } - upperBounds, hBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(ub, false) - fhBase := hBase.ToFloat(nil) - h, fh := convertnhcb.ConvertHistogramWrapper(th, upperBounds, hBase, fhBase) - if h != nil { - if err := h.Validate(); err != nil { - continue - } - if _, err = app.AppendHistogram(0, lset, defTime, h, nil); err != nil { - continue - } - } else if fh != nil { - if err := fh.Validate(); err != nil { - continue - } - if _, err = app.AppendHistogram(0, lset, defTime, nil, fh); err != nil { - continue - } - } - } - return } -func processClassicHistogramSeries(lset labels.Labels, suffix string, nhcbLabels map[uint64]labels.Labels, nhcbBuilder map[uint64]convertnhcb.TempHistogram, updateHist func(*convertnhcb.TempHistogram)) { - m2 := convertnhcb.GetHistogramMetricBase(lset, suffix) - m2hash := m2.Hash() - nhcbLabels[m2hash] = m2 - th, exists := nhcbBuilder[m2hash] - if !exists { - th = convertnhcb.NewTempHistogram() - } - updateHist(&th) - nhcbBuilder[m2hash] = th -} - // Adds samples to the appender, checking the error, and then returns the # of samples added, // whether the caller should continue to process more samples, and any sample or bucket limit errors. func (sl *scrapeLoop) checkAddError(met []byte, err error, sampleLimitErr, bucketLimitErr *error, appErrs *appendErrors) (bool, error) { From 0e5072b87385a322f97bdc9c1d993cc0082a1ee7 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 07/48] keep only 1 nhcb in memory at at time Signed-off-by: Jeanette Tan --- model/textparse/nhcbparse.go | 83 ++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 47 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 1b595351d..165fd272f 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -43,8 +43,8 @@ type NhcbParser struct { buf []byte - nhcbLabels map[uint64]labels.Labels - nhcbBuilder map[uint64]convertnhcb.TempHistogram + lsetNhcb labels.Labels + tempNhcb convertnhcb.TempHistogram } func NewNhcbParser(p Parser, keepClassicHistograms bool) Parser { @@ -52,8 +52,7 @@ func NewNhcbParser(p Parser, keepClassicHistograms bool) Parser { parser: p, keepClassicHistograms: keepClassicHistograms, buf: make([]byte, 0, 1024), - nhcbLabels: make(map[uint64]labels.Labels), - nhcbBuilder: make(map[uint64]convertnhcb.TempHistogram), + tempNhcb: convertnhcb.NewTempHistogram(), } } @@ -97,8 +96,7 @@ func (p *NhcbParser) CreatedTimestamp() *int64 { func (p *NhcbParser) Next() (Entry, error) { et, err := p.parser.Next() if errors.Is(err, io.EOF) { - if len(p.nhcbBuilder) > 0 { - p.processNhcb() + if p.processNhcb(p.tempNhcb) { return EntryHistogram, nil } return EntryInvalid, err @@ -123,18 +121,18 @@ func (p *NhcbParser) handleClassicHistogramSeries(lset labels.Labels) bool { case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel): le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64) if err == nil && !math.IsNaN(le) { - processClassicHistogramSeries(lset, "_bucket", p.nhcbLabels, p.nhcbBuilder, func(hist *convertnhcb.TempHistogram) { + p.processClassicHistogramSeries(lset, "_bucket", func(hist *convertnhcb.TempHistogram) { hist.BucketCounts[le] = p.value }) return true } case strings.HasSuffix(mName, "_count"): - processClassicHistogramSeries(lset, "_count", p.nhcbLabels, p.nhcbBuilder, func(hist *convertnhcb.TempHistogram) { + p.processClassicHistogramSeries(lset, "_count", func(hist *convertnhcb.TempHistogram) { hist.Count = p.value }) return true case strings.HasSuffix(mName, "_sum"): - processClassicHistogramSeries(lset, "_sum", p.nhcbLabels, p.nhcbBuilder, func(hist *convertnhcb.TempHistogram) { + p.processClassicHistogramSeries(lset, "_sum", func(hist *convertnhcb.TempHistogram) { hist.Sum = p.value }) return true @@ -142,47 +140,38 @@ func (p *NhcbParser) handleClassicHistogramSeries(lset labels.Labels) bool { return false } -func processClassicHistogramSeries(lset labels.Labels, suffix string, nhcbLabels map[uint64]labels.Labels, nhcbBuilder map[uint64]convertnhcb.TempHistogram, updateHist func(*convertnhcb.TempHistogram)) { - m2 := convertnhcb.GetHistogramMetricBase(lset, suffix) - m2hash := m2.Hash() - nhcbLabels[m2hash] = m2 - th, exists := nhcbBuilder[m2hash] - if !exists { - th = convertnhcb.NewTempHistogram() - } - updateHist(&th) - nhcbBuilder[m2hash] = th +func (p *NhcbParser) processClassicHistogramSeries(lset labels.Labels, suffix string, updateHist func(*convertnhcb.TempHistogram)) { + p.lsetNhcb = convertnhcb.GetHistogramMetricBase(lset, suffix) + updateHist(&p.tempNhcb) } -func (p *NhcbParser) processNhcb() { - for hash, th := range p.nhcbBuilder { - lset, ok := p.nhcbLabels[hash] - if !ok { - continue - } - ub := make([]float64, 0, len(th.BucketCounts)) - for b := range th.BucketCounts { - ub = append(ub, b) +func (p *NhcbParser) processNhcb(th convertnhcb.TempHistogram) bool { + if len(th.BucketCounts) == 0 { + return false + } + ub := make([]float64, 0, len(th.BucketCounts)) + for b := range th.BucketCounts { + ub = append(ub, b) + } + upperBounds, hBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(ub, false) + fhBase := hBase.ToFloat(nil) + h, fh := convertnhcb.ConvertHistogramWrapper(th, upperBounds, hBase, fhBase) + if h != nil { + if err := h.Validate(); err != nil { + return false } - upperBounds, hBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(ub, false) - fhBase := hBase.ToFloat(nil) - h, fh := convertnhcb.ConvertHistogramWrapper(th, upperBounds, hBase, fhBase) - if h != nil { - if err := h.Validate(); err != nil { - panic("histogram validation failed") - } - p.h = h - p.fh = nil - } else if fh != nil { - if err := fh.Validate(); err != nil { - panic("histogram validation failed") - } - p.h = nil - p.fh = fh + p.h = h + p.fh = nil + } else if fh != nil { + if err := fh.Validate(); err != nil { + return false } - p.bytes = lset.Bytes(p.buf) - p.lset = lset - p.metricString = lset.String() + p.h = nil + p.fh = fh } - p.nhcbBuilder = map[uint64]convertnhcb.TempHistogram{} + p.bytes = p.lsetNhcb.Bytes(p.buf) + p.lset = p.lsetNhcb + p.metricString = p.lsetNhcb.String() + p.tempNhcb = convertnhcb.NewTempHistogram() + return true } From cbd5488cd3f2226ecde4532cff48b0b08f5a815e Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 08/48] skip nhcb conversion if there is native histogram of same name Signed-off-by: Jeanette Tan --- model/textparse/nhcbparse.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 165fd272f..a15dee277 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -45,6 +45,8 @@ type NhcbParser struct { lsetNhcb labels.Labels tempNhcb convertnhcb.TempHistogram + + lastNativeHistName string } func NewNhcbParser(p Parser, keepClassicHistograms bool) Parser { @@ -111,12 +113,16 @@ func (p *NhcbParser) Next() (Entry, error) { case EntryHistogram: p.bytes, p.ts, p.h, p.fh = p.parser.Histogram() p.metricString = p.parser.Metric(&p.lset) + p.lastNativeHistName = p.lset.Get(labels.MetricName) } return et, err } func (p *NhcbParser) handleClassicHistogramSeries(lset labels.Labels) bool { mName := lset.Get(labels.MetricName) + if convertnhcb.GetHistogramMetricBaseName(mName) == p.lastNativeHistName { + return false + } switch { case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel): le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64) From 57bde06d2c89ac6733918337e0a6b5f8f11b273b Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 09/48] add doc comments Signed-off-by: Jeanette Tan --- model/textparse/nhcbparse.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index a15dee277..c210c9c0b 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -29,23 +29,31 @@ import ( ) type NhcbParser struct { - parser Parser + // The parser we're wrapping. + parser Parser + // Option to keep classic histograms along with converted histograms. keepClassicHistograms bool + // Caches the values from the underlying parser. + // For Series and Histogram. bytes []byte ts *int64 value float64 h *histogram.Histogram fh *histogram.FloatHistogram - + // For Metric. lset labels.Labels metricString string buf []byte + // Collates values from the classic histogram series to build + // the converted histogram later. lsetNhcb labels.Labels tempNhcb convertnhcb.TempHistogram + // Remembers the last native histogram name so we can ignore + // conversions to NHCB when the name is the same. lastNativeHistName string } @@ -118,6 +126,10 @@ func (p *NhcbParser) Next() (Entry, error) { return et, err } +// handleClassicHistogramSeries collates the classic histogram series to be converted to NHCB +// if it is actually a classic histogram series (and not a normal float series) and if there +// isn't already a native histogram with the same name (assuming it is always processed +// right before the classic histograms) and returns true if the collation was done. func (p *NhcbParser) handleClassicHistogramSeries(lset labels.Labels) bool { mName := lset.Get(labels.MetricName) if convertnhcb.GetHistogramMetricBaseName(mName) == p.lastNativeHistName { @@ -151,6 +163,8 @@ func (p *NhcbParser) processClassicHistogramSeries(lset labels.Labels, suffix st updateHist(&p.tempNhcb) } +// processNhcb converts the collated classic histogram series to NHCB and caches the info +// to be returned to callers. func (p *NhcbParser) processNhcb(th convertnhcb.TempHistogram) bool { if len(th.BucketCounts) == 0 { return false From 41c7f7d3520e0bd98c39810c0bb1a1084c070400 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 10/48] don't reuse the buffer Signed-off-by: Jeanette Tan --- model/textparse/nhcbparse.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index c210c9c0b..3c6b66e02 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -45,8 +45,6 @@ type NhcbParser struct { lset labels.Labels metricString string - buf []byte - // Collates values from the classic histogram series to build // the converted histogram later. lsetNhcb labels.Labels @@ -61,7 +59,6 @@ func NewNhcbParser(p Parser, keepClassicHistograms bool) Parser { return &NhcbParser{ parser: p, keepClassicHistograms: keepClassicHistograms, - buf: make([]byte, 0, 1024), tempNhcb: convertnhcb.NewTempHistogram(), } } @@ -189,7 +186,8 @@ func (p *NhcbParser) processNhcb(th convertnhcb.TempHistogram) bool { p.h = nil p.fh = fh } - p.bytes = p.lsetNhcb.Bytes(p.buf) + buf := make([]byte, 0, 1024) + p.bytes = p.lsetNhcb.Bytes(buf) p.lset = p.lsetNhcb p.metricString = p.lsetNhcb.String() p.tempNhcb = convertnhcb.NewTempHistogram() From cd498964e67deca36a590897457aef1c0301546f Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 11/48] expand tests and support conversion to nhcb in the middle of scrape Signed-off-by: Jeanette Tan --- model/textparse/nhcbparse.go | 92 +++++++--- scrape/scrape_test.go | 302 +++++++++++++++++++++++--------- util/convertnhcb/convertnhcb.go | 3 +- 3 files changed, 296 insertions(+), 101 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 3c6b66e02..6a264a530 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -45,14 +45,30 @@ type NhcbParser struct { lset labels.Labels metricString string + // Caches the entry itself if we are inserting a converted NHCB + // halfway through. + entry Entry + justInsertedNhcb bool + // Caches the values and metric for the inserted converted NHCB. + bytesNhcb []byte + hNhcb *histogram.Histogram + fhNhcb *histogram.FloatHistogram + lsetNhcb labels.Labels + metricStringNhcb string + // Collates values from the classic histogram series to build // the converted histogram later. - lsetNhcb labels.Labels - tempNhcb convertnhcb.TempHistogram + tempLsetNhcb labels.Labels + tempNhcb convertnhcb.TempHistogram + isCollationInProgress bool // Remembers the last native histogram name so we can ignore // conversions to NHCB when the name is the same. lastNativeHistName string + // Remembers the last base histogram metric name (assuming it's + // a classic histogram) so we can tell if the next float series + // is part of the same classic histogram. + lastBaseHistName string } func NewNhcbParser(p Parser, keepClassicHistograms bool) Parser { @@ -68,6 +84,9 @@ func (p *NhcbParser) Series() ([]byte, *int64, float64) { } func (p *NhcbParser) Histogram() ([]byte, *int64, *histogram.Histogram, *histogram.FloatHistogram) { + if p.justInsertedNhcb { + return p.bytesNhcb, p.ts, p.hNhcb, p.fhNhcb + } return p.bytes, p.ts, p.h, p.fh } @@ -88,6 +107,10 @@ func (p *NhcbParser) Comment() []byte { } func (p *NhcbParser) Metric(l *labels.Labels) string { + if p.justInsertedNhcb { + *l = p.lsetNhcb + return p.metricStringNhcb + } *l = p.lset return p.metricString } @@ -101,9 +124,19 @@ func (p *NhcbParser) CreatedTimestamp() *int64 { } func (p *NhcbParser) Next() (Entry, error) { + if p.justInsertedNhcb { + p.justInsertedNhcb = false + if p.entry == EntrySeries { + if isNhcb := p.handleClassicHistogramSeries(p.lset); isNhcb && !p.keepClassicHistograms { + return p.Next() + } + } + return p.entry, nil + } et, err := p.parser.Next() - if errors.Is(err, io.EOF) { - if p.processNhcb(p.tempNhcb) { + if err != nil { + if errors.Is(err, io.EOF) && p.processNhcb() { + p.entry = et return EntryHistogram, nil } return EntryInvalid, err @@ -112,6 +145,16 @@ func (p *NhcbParser) Next() (Entry, error) { case EntrySeries: p.bytes, p.ts, p.value = p.parser.Series() p.metricString = p.parser.Metric(&p.lset) + histBaseName := convertnhcb.GetHistogramMetricBaseName(p.lset) + if histBaseName == p.lastNativeHistName { + break + } + shouldInsertNhcb := p.lastBaseHistName != "" && p.lastBaseHistName != histBaseName + p.lastBaseHistName = histBaseName + if shouldInsertNhcb && p.processNhcb() { + p.entry = et + return EntryHistogram, nil + } if isNhcb := p.handleClassicHistogramSeries(p.lset); isNhcb && !p.keepClassicHistograms { return p.Next() } @@ -119,6 +162,15 @@ func (p *NhcbParser) Next() (Entry, error) { p.bytes, p.ts, p.h, p.fh = p.parser.Histogram() p.metricString = p.parser.Metric(&p.lset) p.lastNativeHistName = p.lset.Get(labels.MetricName) + if p.processNhcb() { + p.entry = et + return EntryHistogram, nil + } + default: + if p.processNhcb() { + p.entry = et + return EntryHistogram, nil + } } return et, err } @@ -129,9 +181,6 @@ func (p *NhcbParser) Next() (Entry, error) { // right before the classic histograms) and returns true if the collation was done. func (p *NhcbParser) handleClassicHistogramSeries(lset labels.Labels) bool { mName := lset.Get(labels.MetricName) - if convertnhcb.GetHistogramMetricBaseName(mName) == p.lastNativeHistName { - return false - } switch { case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel): le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64) @@ -156,40 +205,43 @@ func (p *NhcbParser) handleClassicHistogramSeries(lset labels.Labels) bool { } func (p *NhcbParser) processClassicHistogramSeries(lset labels.Labels, suffix string, updateHist func(*convertnhcb.TempHistogram)) { - p.lsetNhcb = convertnhcb.GetHistogramMetricBase(lset, suffix) + p.isCollationInProgress = true + p.tempLsetNhcb = convertnhcb.GetHistogramMetricBase(lset, suffix) updateHist(&p.tempNhcb) } // processNhcb converts the collated classic histogram series to NHCB and caches the info // to be returned to callers. -func (p *NhcbParser) processNhcb(th convertnhcb.TempHistogram) bool { - if len(th.BucketCounts) == 0 { +func (p *NhcbParser) processNhcb() bool { + if !p.isCollationInProgress { return false } - ub := make([]float64, 0, len(th.BucketCounts)) - for b := range th.BucketCounts { + ub := make([]float64, 0, len(p.tempNhcb.BucketCounts)) + for b := range p.tempNhcb.BucketCounts { ub = append(ub, b) } upperBounds, hBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(ub, false) fhBase := hBase.ToFloat(nil) - h, fh := convertnhcb.ConvertHistogramWrapper(th, upperBounds, hBase, fhBase) + h, fh := convertnhcb.ConvertHistogramWrapper(p.tempNhcb, upperBounds, hBase, fhBase) if h != nil { if err := h.Validate(); err != nil { return false } - p.h = h - p.fh = nil + p.hNhcb = h + p.fhNhcb = nil } else if fh != nil { if err := fh.Validate(); err != nil { return false } - p.h = nil - p.fh = fh + p.hNhcb = nil + p.fhNhcb = fh } buf := make([]byte, 0, 1024) - p.bytes = p.lsetNhcb.Bytes(buf) - p.lset = p.lsetNhcb - p.metricString = p.lsetNhcb.String() + p.bytesNhcb = p.tempLsetNhcb.Bytes(buf) + p.lsetNhcb = p.tempLsetNhcb + p.metricStringNhcb = p.tempLsetNhcb.String() p.tempNhcb = convertnhcb.NewTempHistogram() + p.isCollationInProgress = false + p.justInsertedNhcb = true return true } diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index bfab0175b..df8b1a733 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3371,80 +3371,118 @@ test_summary_count 199 // Testing whether we can automatically convert scraped classic histograms into native histograms with custom buckets. func TestConvertClassicHistograms(t *testing.T) { - simpleStorage := teststorage.New(t) - defer simpleStorage.Close() - - config := &config.ScrapeConfig{ - JobName: "test", - SampleLimit: 100, - Scheme: "http", - ScrapeInterval: model.Duration(100 * time.Millisecond), - ScrapeTimeout: model.Duration(100 * time.Millisecond), - ScrapeClassicHistograms: true, - ConvertClassicHistograms: true, + metricsTexts := map[string]string{ + "normal": ` +# HELP test_metric_1 some help text +# TYPE test_metric_1 counter +test_metric_1 1 +# HELP test_histogram_1 This is a histogram with default buckets +# TYPE test_histogram_1 histogram +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="1"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="5"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="10"} 1 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 +test_histogram_1_sum{address="0.0.0.0",port="5001"} 10 +test_histogram_1_count{address="0.0.0.0",port="5001"} 1 +# HELP test_metric_2 some help text +# TYPE test_metric_2 counter +test_metric_2 1 +# HELP test_histogram_2 This is a histogram with default buckets +# TYPE test_histogram_2 histogram +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="1"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="5"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="10"} 1 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 +test_histogram_2_sum{address="0.0.0.0",port="5001"} 10 +test_histogram_2_count{address="0.0.0.0",port="5001"} 1 +# HELP test_metric_3 some help text +# TYPE test_metric_3 counter +test_metric_3 1 +# HELP test_histogram_3 This is a histogram with default buckets +# TYPE test_histogram_3 histogram +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="1"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="5"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="10"} 1 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 +test_histogram_3_sum{address="0.0.0.0",port="5001"} 10 +test_histogram_3_count{address="0.0.0.0",port="5001"} 1 +`, + "no metadata and different order": ` +test_metric_1 1 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="1"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="5"} 0 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="10"} 1 +test_histogram_1_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 +test_histogram_1_sum{address="0.0.0.0",port="5001"} 10 +test_histogram_1_count{address="0.0.0.0",port="5001"} 1 +test_metric_2 1 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="1"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="5"} 0 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="10"} 1 +test_histogram_2_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 +test_histogram_2_sum{address="0.0.0.0",port="5001"} 10 +test_histogram_2_count{address="0.0.0.0",port="5001"} 1 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="1"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="5"} 0 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="10"} 1 +test_histogram_3_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 +test_histogram_3_sum{address="0.0.0.0",port="5001"} 10 +test_histogram_3_count{address="0.0.0.0",port="5001"} 1 +test_metric_3 1 +`, } - metricsText := ` -# HELP test_metric some help text -# TYPE test_metric counter -test_metric 1 -# HELP test_histogram This is a histogram with default buckets -# TYPE test_histogram histogram -test_histogram_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 -test_histogram_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 -test_histogram_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 -test_histogram_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 -test_histogram_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 -test_histogram_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 -test_histogram_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 -test_histogram_bucket{address="0.0.0.0",port="5001",le="1"} 0 -test_histogram_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 -test_histogram_bucket{address="0.0.0.0",port="5001",le="5"} 0 -test_histogram_bucket{address="0.0.0.0",port="5001",le="10"} 1 -test_histogram_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 -test_histogram_sum{address="0.0.0.0",port="5001"} 10 -test_histogram_count{address="0.0.0.0",port="5001"} 1 -` - // The expected "le" values do not have the trailing ".0". - expectedLeValues := []string{"0.005", "0.01", "0.025", "0.05", "0.1", "0.25", "0.5", "1", "2.5", "5", "10", "+Inf"} - - scrapeCount := 0 - scraped := make(chan bool) - - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, metricsText) - scrapeCount++ - if scrapeCount > 2 { - close(scraped) - } - })) - defer ts.Close() - - sp, err := newScrapePool(config, simpleStorage, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) - require.NoError(t, err) - defer sp.stop() - - testURL, err := url.Parse(ts.URL) - require.NoError(t, err) - sp.Sync([]*targetgroup.Group{ - { - Targets: []model.LabelSet{{model.AddressLabel: model.LabelValue(testURL.Host)}}, - }, - }) - require.Len(t, sp.ActiveTargets(), 1) - - select { - case <-time.After(5 * time.Second): - t.Fatalf("target was not scraped") - case <-scraped: - } - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - q, err := simpleStorage.Querier(time.Time{}.UnixNano(), time.Now().UnixNano()) - require.NoError(t, err) - defer q.Close() + expectedLeValuesCorrect := []string{"0.005", "0.01", "0.025", "0.05", "0.1", "0.25", "0.5", "1", "2.5", "5", "10", "+Inf"} + expectedLeValuesNone := []string{} checkValues := func(labelName string, expectedValues []string, series storage.SeriesSet) { foundLeValues := map[string]bool{} @@ -3463,7 +3501,7 @@ test_histogram_count{address="0.0.0.0",port="5001"} 1 } // Checks that the expected series is present and runs a basic sanity check of the values. - checkSeries := func(series storage.SeriesSet, encType chunkenc.ValueType) { + checkSeries := func(series storage.SeriesSet, encType chunkenc.ValueType, expectedCount int) { count := 0 for series.Next() { i := series.At().Iterator(nil) @@ -3482,17 +3520,121 @@ test_histogram_count{address="0.0.0.0",port="5001"} 1 } count++ } - require.Equal(t, 1, count, "number of series not as expected") - } + require.Equal(t, expectedCount, count, "number of series not as expected") + } + + for metricsTextName, metricsText := range metricsTexts { + for name, tc := range map[string]struct { + scrapeClassicHistograms bool + convertClassicHistograms bool + expectedLeValues []string + expectedNhcbCount int + }{ + "convert with scrape": { + scrapeClassicHistograms: true, + convertClassicHistograms: true, + expectedLeValues: expectedLeValuesCorrect, + expectedNhcbCount: 1, + }, + "convert without scrape": { + scrapeClassicHistograms: false, + convertClassicHistograms: true, + expectedLeValues: expectedLeValuesNone, + expectedNhcbCount: 1, + }, + "scrape without convert": { + scrapeClassicHistograms: true, + convertClassicHistograms: false, + expectedLeValues: expectedLeValuesCorrect, + expectedNhcbCount: 0, + }, + "neither scrape nor convert": { + scrapeClassicHistograms: false, + convertClassicHistograms: false, + expectedLeValues: expectedLeValuesCorrect, // since these are sent without native histograms + expectedNhcbCount: 0, + }, + } { + t.Run(fmt.Sprintf("%s with %s", name, metricsTextName), func(t *testing.T) { + simpleStorage := teststorage.New(t) + defer simpleStorage.Close() + + config := &config.ScrapeConfig{ + JobName: "test", + SampleLimit: 100, + Scheme: "http", + ScrapeInterval: model.Duration(100 * time.Millisecond), + ScrapeTimeout: model.Duration(100 * time.Millisecond), + ScrapeClassicHistograms: tc.scrapeClassicHistograms, + ConvertClassicHistograms: tc.convertClassicHistograms, + } - series := q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_bucket")) - checkValues("le", expectedLeValues, series) + scrapeCount := 0 + scraped := make(chan bool) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, metricsText) + scrapeCount++ + if scrapeCount > 2 { + close(scraped) + } + })) + defer ts.Close() + + sp, err := newScrapePool(config, simpleStorage, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) + require.NoError(t, err) + defer sp.stop() + + testURL, err := url.Parse(ts.URL) + require.NoError(t, err) + sp.Sync([]*targetgroup.Group{ + { + Targets: []model.LabelSet{{model.AddressLabel: model.LabelValue(testURL.Host)}}, + }, + }) + require.Len(t, sp.ActiveTargets(), 1) + + select { + case <-time.After(5 * time.Second): + t.Fatalf("target was not scraped") + case <-scraped: + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + q, err := simpleStorage.Querier(time.Time{}.UnixNano(), time.Now().UnixNano()) + require.NoError(t, err) + defer q.Close() - series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram")) - checkSeries(series, chunkenc.ValHistogram) + series := q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_metric_1")) + checkSeries(series, chunkenc.ValFloat, 1) - series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_metric")) - checkSeries(series, chunkenc.ValFloat) + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_1_bucket")) + checkValues("le", tc.expectedLeValues, series) + + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_1")) + checkSeries(series, chunkenc.ValHistogram, tc.expectedNhcbCount) + + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_metric_2")) + checkSeries(series, chunkenc.ValFloat, 1) + + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_2_bucket")) + checkValues("le", tc.expectedLeValues, series) + + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_2")) + checkSeries(series, chunkenc.ValHistogram, tc.expectedNhcbCount) + + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_metric_3")) + checkSeries(series, chunkenc.ValFloat, 1) + + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_3_bucket")) + checkValues("le", tc.expectedLeValues, series) + + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_3")) + checkSeries(series, chunkenc.ValHistogram, tc.expectedNhcbCount) + }) + } + } } func TestScrapeLoopRunCreatesStaleMarkersOnFailedScrapeForTimestampedMetrics(t *testing.T) { diff --git a/util/convertnhcb/convertnhcb.go b/util/convertnhcb/convertnhcb.go index 2e71a242c..cd0841582 100644 --- a/util/convertnhcb/convertnhcb.go +++ b/util/convertnhcb/convertnhcb.go @@ -164,7 +164,8 @@ func GetHistogramMetricBase(m labels.Labels, suffix string) labels.Labels { Labels() } -func GetHistogramMetricBaseName(s string) string { +func GetHistogramMetricBaseName(m labels.Labels) string { + s := m.Get(labels.MetricName) for _, rep := range histogramNameSuffixReplacements { s = rep.pattern.ReplaceAllString(s, rep.repl) } From e3899187da7f95db42ec6917628ab0420e1af1d5 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 12/48] expand tests for protobuf and fix problems Signed-off-by: Jeanette Tan --- model/textparse/nhcbparse.go | 9 +- scrape/scrape_test.go | 414 ++++++++++++++++++++++------------- 2 files changed, 270 insertions(+), 153 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 6a264a530..311957fb4 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -48,6 +48,7 @@ type NhcbParser struct { // Caches the entry itself if we are inserting a converted NHCB // halfway through. entry Entry + err error justInsertedNhcb bool // Caches the values and metric for the inserted converted NHCB. bytesNhcb []byte @@ -131,12 +132,13 @@ func (p *NhcbParser) Next() (Entry, error) { return p.Next() } } - return p.entry, nil + return p.entry, p.err } et, err := p.parser.Next() if err != nil { if errors.Is(err, io.EOF) && p.processNhcb() { p.entry = et + p.err = err return EntryHistogram, nil } return EntryInvalid, err @@ -236,10 +238,9 @@ func (p *NhcbParser) processNhcb() bool { p.hNhcb = nil p.fhNhcb = fh } - buf := make([]byte, 0, 1024) - p.bytesNhcb = p.tempLsetNhcb.Bytes(buf) + p.metricStringNhcb = p.tempLsetNhcb.Get(labels.MetricName) + strings.ReplaceAll(p.tempLsetNhcb.DropMetricName().String(), ", ", ",") + p.bytesNhcb = []byte(p.metricStringNhcb) p.lsetNhcb = p.tempLsetNhcb - p.metricStringNhcb = p.tempLsetNhcb.String() p.tempNhcb = convertnhcb.NewTempHistogram() p.isCollationInProgress = false p.justInsertedNhcb = true diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index df8b1a733..35319be36 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -29,6 +29,7 @@ import ( "strings" "sync" "testing" + "text/template" "time" "github.com/go-kit/log" @@ -3371,120 +3372,187 @@ test_summary_count 199 // Testing whether we can automatically convert scraped classic histograms into native histograms with custom buckets. func TestConvertClassicHistograms(t *testing.T) { - metricsTexts := map[string]string{ - "normal": ` -# HELP test_metric_1 some help text -# TYPE test_metric_1 counter -test_metric_1 1 -# HELP test_histogram_1 This is a histogram with default buckets -# TYPE test_histogram_1 histogram -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="1"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="5"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="10"} 1 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 -test_histogram_1_sum{address="0.0.0.0",port="5001"} 10 -test_histogram_1_count{address="0.0.0.0",port="5001"} 1 -# HELP test_metric_2 some help text -# TYPE test_metric_2 counter -test_metric_2 1 -# HELP test_histogram_2 This is a histogram with default buckets -# TYPE test_histogram_2 histogram -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="1"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="5"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="10"} 1 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 -test_histogram_2_sum{address="0.0.0.0",port="5001"} 10 -test_histogram_2_count{address="0.0.0.0",port="5001"} 1 -# HELP test_metric_3 some help text -# TYPE test_metric_3 counter -test_metric_3 1 -# HELP test_histogram_3 This is a histogram with default buckets -# TYPE test_histogram_3 histogram -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="1"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="5"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="10"} 1 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 -test_histogram_3_sum{address="0.0.0.0",port="5001"} 10 -test_histogram_3_count{address="0.0.0.0",port="5001"} 1 -`, - "no metadata and different order": ` -test_metric_1 1 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="1"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="5"} 0 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="10"} 1 -test_histogram_1_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 -test_histogram_1_sum{address="0.0.0.0",port="5001"} 10 -test_histogram_1_count{address="0.0.0.0",port="5001"} 1 -test_metric_2 1 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="1"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="5"} 0 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="10"} 1 -test_histogram_2_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 -test_histogram_2_sum{address="0.0.0.0",port="5001"} 10 -test_histogram_2_count{address="0.0.0.0",port="5001"} 1 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="1"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="5"} 0 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="10"} 1 -test_histogram_3_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 -test_histogram_3_sum{address="0.0.0.0",port="5001"} 10 -test_histogram_3_count{address="0.0.0.0",port="5001"} 1 -test_metric_3 1 -`, + genTestCounterText := func(name string, value int, withMetadata bool) string { + if withMetadata { + return fmt.Sprintf(` +# HELP %s some help text +# TYPE %s counter +%s %d +`, name, name, name, value) + } else { + return fmt.Sprintf(` +%s %d +`, name, value) + } + } + genTestHistText := func(name string, withMetadata bool) string { + data := map[string]interface{}{ + "name": name, + } + b := &bytes.Buffer{} + if withMetadata { + template.Must(template.New("").Parse(` +# HELP {{.name}} This is a histogram with default buckets +# TYPE {{.name}} histogram +`)).Execute(b, data) + } + template.Must(template.New("").Parse(` +{{.name}}_bucket{address="0.0.0.0",port="5001",le="0.005"} 0 +{{.name}}_bucket{address="0.0.0.0",port="5001",le="0.01"} 0 +{{.name}}_bucket{address="0.0.0.0",port="5001",le="0.025"} 0 +{{.name}}_bucket{address="0.0.0.0",port="5001",le="0.05"} 0 +{{.name}}_bucket{address="0.0.0.0",port="5001",le="0.1"} 0 +{{.name}}_bucket{address="0.0.0.0",port="5001",le="0.25"} 0 +{{.name}}_bucket{address="0.0.0.0",port="5001",le="0.5"} 0 +{{.name}}_bucket{address="0.0.0.0",port="5001",le="1"} 0 +{{.name}}_bucket{address="0.0.0.0",port="5001",le="2.5"} 0 +{{.name}}_bucket{address="0.0.0.0",port="5001",le="5"} 0 +{{.name}}_bucket{address="0.0.0.0",port="5001",le="10"} 1 +{{.name}}_bucket{address="0.0.0.0",port="5001",le="+Inf"} 1 +{{.name}}_sum{address="0.0.0.0",port="5001"} 10 +{{.name}}_count{address="0.0.0.0",port="5001"} 1 +`)).Execute(b, data) + return b.String() + } + genTestCounterProto := func(name string, value int) string { + return fmt.Sprintf(` +name: "%s" +help: "some help text" +type: COUNTER +metric: < + counter: < + value: %d + > +> +`, name, value) + } + genTestHistProto := func(name string) string { + return fmt.Sprintf(` +name: "%s" +help: "This is a histogram with default buckets" +type: HISTOGRAM +metric: < + label: < + name: "address" + value: "0.0.0.0" + > + label: < + name: "port" + value: "5001" + > + histogram: < + sample_count: 1 + sample_sum: 10 + bucket: < + cumulative_count: 0 + upper_bound: 0.005 + > + bucket: < + cumulative_count: 0 + upper_bound: 0.01 + > + bucket: < + cumulative_count: 0 + upper_bound: 0.025 + > + bucket: < + cumulative_count: 0 + upper_bound: 0.05 + > + bucket: < + cumulative_count: 0 + upper_bound: 0.1 + > + bucket: < + cumulative_count: 0 + upper_bound: 0.25 + > + bucket: < + cumulative_count: 0 + upper_bound: 0.5 + > + bucket: < + cumulative_count: 0 + upper_bound: 1 + > + bucket: < + cumulative_count: 0 + upper_bound: 2.5 + > + bucket: < + cumulative_count: 0 + upper_bound: 5 + > + bucket: < + cumulative_count: 1 + upper_bound: 10 + > + > + timestamp_ms: 1234568 +> +`, name) } - // The expected "le" values do not have the trailing ".0". - expectedLeValuesCorrect := []string{"0.005", "0.01", "0.025", "0.05", "0.1", "0.25", "0.5", "1", "2.5", "5", "10", "+Inf"} - expectedLeValuesNone := []string{} + metricsTexts := map[string]struct { + text []string + contentType string + }{ + "text": { + text: []string{ + genTestCounterText("test_metric_1", 1, true), + genTestHistText("test_histogram_1", true), + genTestCounterText("test_metric_2", 1, true), + genTestHistText("test_histogram_2", true), + genTestCounterText("test_metric_3", 1, true), + genTestHistText("test_histogram_3", true), + }, + }, + "text, no metadata, in different order": { + text: []string{ + genTestCounterText("test_metric_1", 1, false), + genTestHistText("test_histogram_1", false), + genTestCounterText("test_metric_2", 1, false), + genTestHistText("test_histogram_2", false), + genTestHistText("test_histogram_3", false), + genTestCounterText("test_metric_3", 1, false), + }, + }, + "protobuf": { + text: []string{ + genTestCounterProto("test_metric_1", 1), + genTestHistProto("test_histogram_1"), + genTestCounterProto("test_metric_2", 1), + genTestHistProto("test_histogram_2"), + genTestCounterProto("test_metric_3", 1), + genTestHistProto("test_histogram_3"), + }, + contentType: "application/vnd.google.protobuf", + }, + "protobuf, in different order": { + text: []string{ + genTestHistProto("test_histogram_1"), + genTestCounterProto("test_metric_1", 1), + genTestHistProto("test_histogram_2"), + genTestCounterProto("test_metric_2", 1), + genTestHistProto("test_histogram_3"), + genTestCounterProto("test_metric_3", 1), + }, + contentType: "application/vnd.google.protobuf", + }, + } - checkValues := func(labelName string, expectedValues []string, series storage.SeriesSet) { + checkBucketValues := func(expectedCount int, contentType string, series storage.SeriesSet) { + labelName := "le" + var expectedValues []string + if expectedCount > 0 { + if contentType == "application/vnd.google.protobuf" { + // The expected "le" values have the trailing ".0". + expectedValues = []string{"0.005", "0.01", "0.025", "0.05", "0.1", "0.25", "0.5", "1.0", "2.5", "5.0", "10.0", "+Inf"} + } else { + // The expected "le" values do not have the trailing ".0". + expectedValues = []string{"0.005", "0.01", "0.025", "0.05", "0.1", "0.25", "0.5", "1", "2.5", "5", "10", "+Inf"} + } + } foundLeValues := map[string]bool{} for series.Next() { @@ -3494,64 +3562,98 @@ test_metric_3 1 foundLeValues[v] = true } - require.Equal(t, len(expectedValues), len(foundLeValues), "number of label values not as expected") + require.Equal(t, len(expectedValues), len(foundLeValues), "unexpected number of label values, expected %v but found %v", expectedValues, foundLeValues) for _, v := range expectedValues { require.Contains(t, foundLeValues, v, "label value not found") } } - // Checks that the expected series is present and runs a basic sanity check of the values. - checkSeries := func(series storage.SeriesSet, encType chunkenc.ValueType, expectedCount int) { + // Checks that the expected series is present and runs a basic sanity check of the float values. + checkFloatSeries := func(series storage.SeriesSet, expectedCount int, expectedFloat float64) { count := 0 for series.Next() { i := series.At().Iterator(nil) - switch encType { - case chunkenc.ValFloat: - for i.Next() == encType { + loop: + for { + switch i.Next() { + case chunkenc.ValNone: + break loop + case chunkenc.ValFloat: _, f := i.At() - require.Equal(t, 1., f) + require.Equal(t, expectedFloat, f) + case chunkenc.ValHistogram: + panic("unexpected value type: histogram") + case chunkenc.ValFloatHistogram: + panic("unexpected value type: float histogram") + default: + panic("unexpected value type") } - case chunkenc.ValHistogram: - for i.Next() == encType { + } + count++ + } + require.Equal(t, expectedCount, count, "number of float series not as expected") + } + + // Checks that the expected series is present and runs a basic sanity check of the histogram values. + checkHistSeries := func(series storage.SeriesSet, expectedCount int, expectedSchema int32) { + count := 0 + for series.Next() { + i := series.At().Iterator(nil) + loop: + for { + switch i.Next() { + case chunkenc.ValNone: + break loop + case chunkenc.ValFloat: + panic("unexpected value type: float") + case chunkenc.ValHistogram: _, h := i.AtHistogram(nil) + require.Equal(t, expectedSchema, h.Schema) + require.Equal(t, uint64(1), h.Count) + require.Equal(t, 10.0, h.Sum) + case chunkenc.ValFloatHistogram: + _, h := i.AtFloatHistogram(nil) + require.Equal(t, expectedSchema, h.Schema) require.Equal(t, uint64(1), h.Count) require.Equal(t, 10.0, h.Sum) + default: + panic("unexpected value type") } } count++ } - require.Equal(t, expectedCount, count, "number of series not as expected") + require.Equal(t, expectedCount, count, "number of histogram series not as expected") } for metricsTextName, metricsText := range metricsTexts { for name, tc := range map[string]struct { scrapeClassicHistograms bool convertClassicHistograms bool - expectedLeValues []string + expectedClassicHistCount int expectedNhcbCount int }{ "convert with scrape": { scrapeClassicHistograms: true, convertClassicHistograms: true, - expectedLeValues: expectedLeValuesCorrect, + expectedClassicHistCount: 1, expectedNhcbCount: 1, }, "convert without scrape": { scrapeClassicHistograms: false, convertClassicHistograms: true, - expectedLeValues: expectedLeValuesNone, + expectedClassicHistCount: 0, expectedNhcbCount: 1, }, "scrape without convert": { scrapeClassicHistograms: true, convertClassicHistograms: false, - expectedLeValues: expectedLeValuesCorrect, + expectedClassicHistCount: 1, expectedNhcbCount: 0, }, "neither scrape nor convert": { scrapeClassicHistograms: false, convertClassicHistograms: false, - expectedLeValues: expectedLeValuesCorrect, // since these are sent without native histograms + expectedClassicHistCount: 1, // since these are sent without native histograms expectedNhcbCount: 0, }, } { @@ -3573,7 +3675,29 @@ test_metric_3 1 scraped := make(chan bool) ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, metricsText) + if metricsText.contentType != "" { + w.Header().Set("Content-Type", `application/vnd.google.protobuf; proto=io.prometheus.client.MetricFamily; encoding=delimited`) + for _, text := range metricsText.text { + buf := &bytes.Buffer{} + // In case of protobuf, we have to create the binary representation. + pb := &dto.MetricFamily{} + // From text to proto message. + require.NoError(t, proto.UnmarshalText(text, pb)) + // From proto message to binary protobuf. + protoBuf, err := proto.Marshal(pb) + require.NoError(t, err) + + // Write first length, then binary protobuf. + varintBuf := binary.AppendUvarint(nil, uint64(len(protoBuf))) + buf.Write(varintBuf) + buf.Write(protoBuf) + w.Write(buf.Bytes()) + } + } else { + for _, text := range metricsText.text { + fmt.Fprint(w, text) + } + } scrapeCount++ if scrapeCount > 2 { close(scraped) @@ -3581,7 +3705,7 @@ test_metric_3 1 })) defer ts.Close() - sp, err := newScrapePool(config, simpleStorage, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) + sp, err := newScrapePool(config, simpleStorage, 0, nil, nil, &Options{EnableNativeHistogramsIngestion: true}, newTestScrapeMetrics(t)) require.NoError(t, err) defer sp.stop() @@ -3606,32 +3730,24 @@ test_metric_3 1 require.NoError(t, err) defer q.Close() - series := q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_metric_1")) - checkSeries(series, chunkenc.ValFloat, 1) - - series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_1_bucket")) - checkValues("le", tc.expectedLeValues, series) - - series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_1")) - checkSeries(series, chunkenc.ValHistogram, tc.expectedNhcbCount) + var series storage.SeriesSet - series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_metric_2")) - checkSeries(series, chunkenc.ValFloat, 1) + for i := 1; i <= 3; i++ { + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_metric_%d", i))) + checkFloatSeries(series, 1, 1.) - series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_2_bucket")) - checkValues("le", tc.expectedLeValues, series) + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_sum", i))) + checkFloatSeries(series, tc.expectedClassicHistCount, 10.) - series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_2")) - checkSeries(series, chunkenc.ValHistogram, tc.expectedNhcbCount) + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_count", i))) + checkFloatSeries(series, tc.expectedClassicHistCount, 1.) - series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_metric_3")) - checkSeries(series, chunkenc.ValFloat, 1) + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_bucket", i))) + checkBucketValues(tc.expectedClassicHistCount, metricsText.contentType, series) - series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_3_bucket")) - checkValues("le", tc.expectedLeValues, series) - - series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", "test_histogram_3")) - checkSeries(series, chunkenc.ValHistogram, tc.expectedNhcbCount) + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d", i))) + checkHistSeries(series, tc.expectedNhcbCount, histogram.CustomBucketsSchema) + } }) } } From 8b3ae15ad52c9a8473277e4159542e33f4f093f4 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 13/48] expand tests for classic and exponential native histograms Signed-off-by: Jeanette Tan --- scrape/scrape_test.go | 145 ++++++++++++++++++++++++++---------------- 1 file changed, 90 insertions(+), 55 deletions(-) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 35319be36..4ca182335 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3426,7 +3426,67 @@ metric: < > `, name, value) } - genTestHistProto := func(name string) string { + genTestHistProto := func(name string, hasClassic, hasExponential bool) string { + var classic string + if hasClassic { + classic = ` +bucket: < + cumulative_count: 0 + upper_bound: 0.005 +> +bucket: < + cumulative_count: 0 + upper_bound: 0.01 +> +bucket: < + cumulative_count: 0 + upper_bound: 0.025 +> +bucket: < + cumulative_count: 0 + upper_bound: 0.05 +> +bucket: < + cumulative_count: 0 + upper_bound: 0.1 +> +bucket: < + cumulative_count: 0 + upper_bound: 0.25 +> +bucket: < + cumulative_count: 0 + upper_bound: 0.5 +> +bucket: < + cumulative_count: 0 + upper_bound: 1 +> +bucket: < + cumulative_count: 0 + upper_bound: 2.5 +> +bucket: < + cumulative_count: 0 + upper_bound: 5 +> +bucket: < + cumulative_count: 1 + upper_bound: 10 +>` + } + var expo string + if hasExponential { + expo = ` +schema: 3 +zero_threshold: 2.938735877055719e-39 +zero_count: 0 +positive_span: < + offset: 2 + length: 1 +> +positive_delta: 1` + } return fmt.Sprintf(` name: "%s" help: "This is a histogram with default buckets" @@ -3443,59 +3503,18 @@ metric: < histogram: < sample_count: 1 sample_sum: 10 - bucket: < - cumulative_count: 0 - upper_bound: 0.005 - > - bucket: < - cumulative_count: 0 - upper_bound: 0.01 - > - bucket: < - cumulative_count: 0 - upper_bound: 0.025 - > - bucket: < - cumulative_count: 0 - upper_bound: 0.05 - > - bucket: < - cumulative_count: 0 - upper_bound: 0.1 - > - bucket: < - cumulative_count: 0 - upper_bound: 0.25 - > - bucket: < - cumulative_count: 0 - upper_bound: 0.5 - > - bucket: < - cumulative_count: 0 - upper_bound: 1 - > - bucket: < - cumulative_count: 0 - upper_bound: 2.5 - > - bucket: < - cumulative_count: 0 - upper_bound: 5 - > - bucket: < - cumulative_count: 1 - upper_bound: 10 - > + %s + %s > timestamp_ms: 1234568 > -`, name) +`, name, classic, expo) } metricsTexts := map[string]struct { - text []string - contentType string + text []string + contentType string + hasExponential bool }{ "text": { text: []string{ @@ -3520,25 +3539,37 @@ metric: < "protobuf": { text: []string{ genTestCounterProto("test_metric_1", 1), - genTestHistProto("test_histogram_1"), + genTestHistProto("test_histogram_1", true, false), genTestCounterProto("test_metric_2", 1), - genTestHistProto("test_histogram_2"), + genTestHistProto("test_histogram_2", true, false), genTestCounterProto("test_metric_3", 1), - genTestHistProto("test_histogram_3"), + genTestHistProto("test_histogram_3", true, false), }, contentType: "application/vnd.google.protobuf", }, "protobuf, in different order": { text: []string{ - genTestHistProto("test_histogram_1"), + genTestHistProto("test_histogram_1", true, false), genTestCounterProto("test_metric_1", 1), - genTestHistProto("test_histogram_2"), + genTestHistProto("test_histogram_2", true, false), genTestCounterProto("test_metric_2", 1), - genTestHistProto("test_histogram_3"), + genTestHistProto("test_histogram_3", true, false), genTestCounterProto("test_metric_3", 1), }, contentType: "application/vnd.google.protobuf", }, + "protobuf, with native exponential histogram": { + text: []string{ + genTestCounterProto("test_metric_1", 1), + genTestHistProto("test_histogram_1", true, true), + genTestCounterProto("test_metric_2", 1), + genTestHistProto("test_histogram_2", true, true), + genTestCounterProto("test_metric_3", 1), + genTestHistProto("test_histogram_3", true, true), + }, + contentType: "application/vnd.google.protobuf", + hasExponential: true, + }, } checkBucketValues := func(expectedCount int, contentType string, series storage.SeriesSet) { @@ -3746,7 +3777,11 @@ metric: < checkBucketValues(tc.expectedClassicHistCount, metricsText.contentType, series) series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d", i))) - checkHistSeries(series, tc.expectedNhcbCount, histogram.CustomBucketsSchema) + if metricsText.hasExponential { + checkHistSeries(series, 1, 3) + } else { + checkHistSeries(series, tc.expectedNhcbCount, histogram.CustomBucketsSchema) + } } }) } From f35c6649e4d4f366fff30f59926357326e115716 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 14/48] don't blindly convert series with the classic histogram name suffixes if they are not actually histograms based on metadata Signed-off-by: Jeanette Tan --- model/textparse/nhcbparse.go | 29 +++++++----- scrape/scrape_test.go | 82 +++++++++++++++++++++++++++++---- util/convertnhcb/convertnhcb.go | 3 +- 3 files changed, 91 insertions(+), 23 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 311957fb4..904b83d0e 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -44,6 +44,9 @@ type NhcbParser struct { // For Metric. lset labels.Labels metricString string + // For Type. + bType []byte + typ model.MetricType // Caches the entry itself if we are inserting a converted NHCB // halfway through. @@ -96,7 +99,7 @@ func (p *NhcbParser) Help() ([]byte, []byte) { } func (p *NhcbParser) Type() ([]byte, model.MetricType) { - return p.parser.Type() + return p.bType, p.typ } func (p *NhcbParser) Unit() ([]byte, []byte) { @@ -147,7 +150,7 @@ func (p *NhcbParser) Next() (Entry, error) { case EntrySeries: p.bytes, p.ts, p.value = p.parser.Series() p.metricString = p.parser.Metric(&p.lset) - histBaseName := convertnhcb.GetHistogramMetricBaseName(p.lset) + histBaseName := convertnhcb.GetHistogramMetricBaseName(p.lset.Get(labels.MetricName)) if histBaseName == p.lastNativeHistName { break } @@ -160,19 +163,17 @@ func (p *NhcbParser) Next() (Entry, error) { if isNhcb := p.handleClassicHistogramSeries(p.lset); isNhcb && !p.keepClassicHistograms { return p.Next() } + return et, err case EntryHistogram: p.bytes, p.ts, p.h, p.fh = p.parser.Histogram() p.metricString = p.parser.Metric(&p.lset) p.lastNativeHistName = p.lset.Get(labels.MetricName) - if p.processNhcb() { - p.entry = et - return EntryHistogram, nil - } - default: - if p.processNhcb() { - p.entry = et - return EntryHistogram, nil - } + case EntryType: + p.bType, p.typ = p.parser.Type() + } + if p.processNhcb() { + p.entry = et + return EntryHistogram, nil } return et, err } @@ -182,7 +183,13 @@ func (p *NhcbParser) Next() (Entry, error) { // isn't already a native histogram with the same name (assuming it is always processed // right before the classic histograms) and returns true if the collation was done. func (p *NhcbParser) handleClassicHistogramSeries(lset labels.Labels) bool { + if p.typ != model.MetricTypeHistogram { + return false + } mName := lset.Get(labels.MetricName) + if convertnhcb.GetHistogramMetricBaseName(mName) != string(p.bType) { + return false + } switch { case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel): le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 4ca182335..4714afb02 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3377,7 +3377,7 @@ func TestConvertClassicHistograms(t *testing.T) { return fmt.Sprintf(` # HELP %s some help text # TYPE %s counter -%s %d +%s{address="0.0.0.0",port="5001"} %d `, name, name, name, value) } else { return fmt.Sprintf(` @@ -3420,6 +3420,14 @@ name: "%s" help: "some help text" type: COUNTER metric: < + label: < + name: "address" + value: "0.0.0.0" + > + label: < + name: "port" + value: "5001" + > counter: < value: %d > @@ -3519,30 +3527,57 @@ metric: < "text": { text: []string{ genTestCounterText("test_metric_1", 1, true), + genTestCounterText("test_metric_1_count", 1, true), + genTestCounterText("test_metric_1_sum", 1, true), + genTestCounterText("test_metric_1_bucket", 1, true), genTestHistText("test_histogram_1", true), genTestCounterText("test_metric_2", 1, true), + genTestCounterText("test_metric_2_count", 1, true), + genTestCounterText("test_metric_2_sum", 1, true), + genTestCounterText("test_metric_2_bucket", 1, true), genTestHistText("test_histogram_2", true), genTestCounterText("test_metric_3", 1, true), + genTestCounterText("test_metric_3_count", 1, true), + genTestCounterText("test_metric_3_sum", 1, true), + genTestCounterText("test_metric_3_bucket", 1, true), genTestHistText("test_histogram_3", true), }, }, - "text, no metadata, in different order": { + "text, in different order": { text: []string{ - genTestCounterText("test_metric_1", 1, false), - genTestHistText("test_histogram_1", false), - genTestCounterText("test_metric_2", 1, false), - genTestHistText("test_histogram_2", false), - genTestHistText("test_histogram_3", false), - genTestCounterText("test_metric_3", 1, false), + genTestCounterText("test_metric_1", 1, true), + genTestCounterText("test_metric_1_count", 1, true), + genTestCounterText("test_metric_1_sum", 1, true), + genTestCounterText("test_metric_1_bucket", 1, true), + genTestHistText("test_histogram_1", true), + genTestCounterText("test_metric_2", 1, true), + genTestCounterText("test_metric_2_count", 1, true), + genTestCounterText("test_metric_2_sum", 1, true), + genTestCounterText("test_metric_2_bucket", 1, true), + genTestHistText("test_histogram_2", true), + genTestHistText("test_histogram_3", true), + genTestCounterText("test_metric_3", 1, true), + genTestCounterText("test_metric_3_count", 1, true), + genTestCounterText("test_metric_3_sum", 1, true), + genTestCounterText("test_metric_3_bucket", 1, true), }, }, "protobuf": { text: []string{ genTestCounterProto("test_metric_1", 1), + genTestCounterProto("test_metric_1_count", 1), + genTestCounterProto("test_metric_1_sum", 1), + genTestCounterProto("test_metric_1_bucket", 1), genTestHistProto("test_histogram_1", true, false), genTestCounterProto("test_metric_2", 1), + genTestCounterProto("test_metric_2_count", 1), + genTestCounterProto("test_metric_2_sum", 1), + genTestCounterProto("test_metric_2_bucket", 1), genTestHistProto("test_histogram_2", true, false), genTestCounterProto("test_metric_3", 1), + genTestCounterProto("test_metric_3_count", 1), + genTestCounterProto("test_metric_3_sum", 1), + genTestCounterProto("test_metric_3_bucket", 1), genTestHistProto("test_histogram_3", true, false), }, contentType: "application/vnd.google.protobuf", @@ -3551,20 +3586,38 @@ metric: < text: []string{ genTestHistProto("test_histogram_1", true, false), genTestCounterProto("test_metric_1", 1), + genTestCounterProto("test_metric_1_count", 1), + genTestCounterProto("test_metric_1_sum", 1), + genTestCounterProto("test_metric_1_bucket", 1), genTestHistProto("test_histogram_2", true, false), genTestCounterProto("test_metric_2", 1), + genTestCounterProto("test_metric_2_count", 1), + genTestCounterProto("test_metric_2_sum", 1), + genTestCounterProto("test_metric_2_bucket", 1), genTestHistProto("test_histogram_3", true, false), genTestCounterProto("test_metric_3", 1), + genTestCounterProto("test_metric_3_count", 1), + genTestCounterProto("test_metric_3_sum", 1), + genTestCounterProto("test_metric_3_bucket", 1), }, contentType: "application/vnd.google.protobuf", }, "protobuf, with native exponential histogram": { text: []string{ genTestCounterProto("test_metric_1", 1), + genTestCounterProto("test_metric_1_count", 1), + genTestCounterProto("test_metric_1_sum", 1), + genTestCounterProto("test_metric_1_bucket", 1), genTestHistProto("test_histogram_1", true, true), genTestCounterProto("test_metric_2", 1), + genTestCounterProto("test_metric_2_count", 1), + genTestCounterProto("test_metric_2_sum", 1), + genTestCounterProto("test_metric_2_bucket", 1), genTestHistProto("test_histogram_2", true, true), genTestCounterProto("test_metric_3", 1), + genTestCounterProto("test_metric_3_count", 1), + genTestCounterProto("test_metric_3_sum", 1), + genTestCounterProto("test_metric_3_bucket", 1), genTestHistProto("test_histogram_3", true, true), }, contentType: "application/vnd.google.protobuf", @@ -3767,12 +3820,21 @@ metric: < series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_metric_%d", i))) checkFloatSeries(series, 1, 1.) - series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_sum", i))) - checkFloatSeries(series, tc.expectedClassicHistCount, 10.) + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_metric_%d_count", i))) + checkFloatSeries(series, 1, 1.) + + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_metric_%d_sum", i))) + checkFloatSeries(series, 1, 1.) + + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_metric_%d_bucket", i))) + checkFloatSeries(series, 1, 1.) series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_count", i))) checkFloatSeries(series, tc.expectedClassicHistCount, 1.) + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_sum", i))) + checkFloatSeries(series, tc.expectedClassicHistCount, 10.) + series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_bucket", i))) checkBucketValues(tc.expectedClassicHistCount, metricsText.contentType, series) diff --git a/util/convertnhcb/convertnhcb.go b/util/convertnhcb/convertnhcb.go index cd0841582..2e71a242c 100644 --- a/util/convertnhcb/convertnhcb.go +++ b/util/convertnhcb/convertnhcb.go @@ -164,8 +164,7 @@ func GetHistogramMetricBase(m labels.Labels, suffix string) labels.Labels { Labels() } -func GetHistogramMetricBaseName(m labels.Labels) string { - s := m.Get(labels.MetricName) +func GetHistogramMetricBaseName(s string) string { for _, rep := range histogramNameSuffixReplacements { s = rep.pattern.ReplaceAllString(s, rep.repl) } From de9de320a46bb9628b65b8153b3bc0da3f690744 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 15/48] start to cover all test cases for scrape Signed-off-by: Jeanette Tan --- scrape/scrape_test.go | 79 +++++++++++++++++++++++++++++++++---------- 1 file changed, 62 insertions(+), 17 deletions(-) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 4714afb02..de37903fd 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3522,6 +3522,7 @@ metric: < metricsTexts := map[string]struct { text []string contentType string + hasClassic bool hasExponential bool }{ "text": { @@ -3542,6 +3543,7 @@ metric: < genTestCounterText("test_metric_3_bucket", 1, true), genTestHistText("test_histogram_3", true), }, + hasClassic: true, }, "text, in different order": { text: []string{ @@ -3561,6 +3563,7 @@ metric: < genTestCounterText("test_metric_3_sum", 1, true), genTestCounterText("test_metric_3_bucket", 1, true), }, + hasClassic: true, }, "protobuf": { text: []string{ @@ -3581,6 +3584,7 @@ metric: < genTestHistProto("test_histogram_3", true, false), }, contentType: "application/vnd.google.protobuf", + hasClassic: true, }, "protobuf, in different order": { text: []string{ @@ -3601,8 +3605,9 @@ metric: < genTestCounterProto("test_metric_3_bucket", 1), }, contentType: "application/vnd.google.protobuf", + hasClassic: true, }, - "protobuf, with native exponential histogram": { + "protobuf, with additional native exponential histogram": { text: []string{ genTestCounterProto("test_metric_1", 1), genTestCounterProto("test_metric_1_count", 1), @@ -3621,6 +3626,28 @@ metric: < genTestHistProto("test_histogram_3", true, true), }, contentType: "application/vnd.google.protobuf", + hasClassic: true, + hasExponential: true, + }, + "protobuf, with only native exponential histogram": { + text: []string{ + genTestCounterProto("test_metric_1", 1), + genTestCounterProto("test_metric_1_count", 1), + genTestCounterProto("test_metric_1_sum", 1), + genTestCounterProto("test_metric_1_bucket", 1), + genTestHistProto("test_histogram_1", false, true), + genTestCounterProto("test_metric_2", 1), + genTestCounterProto("test_metric_2_count", 1), + genTestCounterProto("test_metric_2_sum", 1), + genTestCounterProto("test_metric_2_bucket", 1), + genTestHistProto("test_histogram_2", false, true), + genTestCounterProto("test_metric_3", 1), + genTestCounterProto("test_metric_3_count", 1), + genTestCounterProto("test_metric_3_sum", 1), + genTestCounterProto("test_metric_3_bucket", 1), + genTestHistProto("test_histogram_3", false, true), + }, + contentType: "application/vnd.google.protobuf", hasExponential: true, }, } @@ -3713,34 +3740,49 @@ metric: < for name, tc := range map[string]struct { scrapeClassicHistograms bool convertClassicHistograms bool - expectedClassicHistCount int - expectedNhcbCount int }{ "convert with scrape": { scrapeClassicHistograms: true, convertClassicHistograms: true, - expectedClassicHistCount: 1, - expectedNhcbCount: 1, }, "convert without scrape": { scrapeClassicHistograms: false, convertClassicHistograms: true, - expectedClassicHistCount: 0, - expectedNhcbCount: 1, }, "scrape without convert": { scrapeClassicHistograms: true, convertClassicHistograms: false, - expectedClassicHistCount: 1, - expectedNhcbCount: 0, }, "neither scrape nor convert": { scrapeClassicHistograms: false, convertClassicHistograms: false, - expectedClassicHistCount: 1, // since these are sent without native histograms - expectedNhcbCount: 0, }, } { + var expectedClassicHistCount, expectedNativeHistCount int + var expectCustomBuckets bool + switch { + case tc.scrapeClassicHistograms && tc.convertClassicHistograms: + expectedClassicHistCount = 1 + expectedNativeHistCount = 1 + expectCustomBuckets = true + case !tc.scrapeClassicHistograms && tc.convertClassicHistograms: + expectedClassicHistCount = 0 + expectedNativeHistCount = 1 + expectCustomBuckets = true + case tc.scrapeClassicHistograms && !tc.convertClassicHistograms: + expectedClassicHistCount = 1 + expectedNativeHistCount = 0 + case !tc.scrapeClassicHistograms && !tc.convertClassicHistograms: + expectedClassicHistCount = 1 // since these are sent without native histograms + expectedNativeHistCount = 0 + } + if metricsText.hasExponential { + expectedNativeHistCount = 1 + expectCustomBuckets = false + } else { + expectCustomBuckets = true + } + t.Run(fmt.Sprintf("%s with %s", name, metricsTextName), func(t *testing.T) { simpleStorage := teststorage.New(t) defer simpleStorage.Close() @@ -3830,20 +3872,23 @@ metric: < checkFloatSeries(series, 1, 1.) series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_count", i))) - checkFloatSeries(series, tc.expectedClassicHistCount, 1.) + checkFloatSeries(series, expectedClassicHistCount, 1.) series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_sum", i))) - checkFloatSeries(series, tc.expectedClassicHistCount, 10.) + checkFloatSeries(series, expectedClassicHistCount, 10.) series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_bucket", i))) - checkBucketValues(tc.expectedClassicHistCount, metricsText.contentType, series) + checkBucketValues(expectedClassicHistCount, metricsText.contentType, series) series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d", i))) - if metricsText.hasExponential { - checkHistSeries(series, 1, 3) + + var expectedSchema int32 + if expectCustomBuckets { + expectedSchema = histogram.CustomBucketsSchema } else { - checkHistSeries(series, tc.expectedNhcbCount, histogram.CustomBucketsSchema) + expectedSchema = 3 } + checkHistSeries(series, expectedNativeHistCount, expectedSchema) } }) } From 050b5fc25721282d3a1f4f41176718e2eb9f2da2 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 16/48] refine test cases according to spec Signed-off-by: Jeanette Tan --- scrape/scrape_test.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index de37903fd..d380bfc34 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3760,27 +3760,27 @@ metric: < } { var expectedClassicHistCount, expectedNativeHistCount int var expectCustomBuckets bool - switch { - case tc.scrapeClassicHistograms && tc.convertClassicHistograms: - expectedClassicHistCount = 1 - expectedNativeHistCount = 1 - expectCustomBuckets = true - case !tc.scrapeClassicHistograms && tc.convertClassicHistograms: - expectedClassicHistCount = 0 - expectedNativeHistCount = 1 - expectCustomBuckets = true - case tc.scrapeClassicHistograms && !tc.convertClassicHistograms: - expectedClassicHistCount = 1 - expectedNativeHistCount = 0 - case !tc.scrapeClassicHistograms && !tc.convertClassicHistograms: - expectedClassicHistCount = 1 // since these are sent without native histograms - expectedNativeHistCount = 0 - } if metricsText.hasExponential { expectedNativeHistCount = 1 expectCustomBuckets = false - } else { - expectCustomBuckets = true + expectedClassicHistCount = 0 + if metricsText.hasClassic && tc.scrapeClassicHistograms { + expectedClassicHistCount = 1 + } + } else if metricsText.hasClassic { + switch { + case tc.scrapeClassicHistograms && tc.convertClassicHistograms: + expectedClassicHistCount = 1 + expectedNativeHistCount = 1 + expectCustomBuckets = true + case !tc.scrapeClassicHistograms && tc.convertClassicHistograms: + expectedClassicHistCount = 0 + expectedNativeHistCount = 1 + expectCustomBuckets = true + case !tc.convertClassicHistograms: + expectedClassicHistCount = 1 + expectedNativeHistCount = 0 + } } t.Run(fmt.Sprintf("%s with %s", name, metricsTextName), func(t *testing.T) { From 90c266845b9598cd4a090d51529d46b10f367531 Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 17/48] fix lint Signed-off-by: Jeanette Tan --- scrape/scrape_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index d380bfc34..e2313a88f 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3379,11 +3379,10 @@ func TestConvertClassicHistograms(t *testing.T) { # TYPE %s counter %s{address="0.0.0.0",port="5001"} %d `, name, name, name, value) - } else { - return fmt.Sprintf(` + } + return fmt.Sprintf(` %s %d `, name, value) - } } genTestHistText := func(name string, withMetadata bool) string { data := map[string]interface{}{ From 97ba2fc39d332d0ff585ca3fbabd5dcdb3266daa Mon Sep 17 00:00:00 2001 From: Jeanette Tan Date: Wed, 3 Jul 2024 17:56:48 +0800 Subject: [PATCH 18/48] use caps for NHCB Signed-off-by: Jeanette Tan --- model/textparse/nhcbparse.go | 106 +++++++++++++++++------------------ scrape/scrape.go | 2 +- 2 files changed, 54 insertions(+), 54 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 904b83d0e..0a16fdf5d 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -28,7 +28,7 @@ import ( "github.com/prometheus/prometheus/util/convertnhcb" ) -type NhcbParser struct { +type NHCBParser struct { // The parser we're wrapping. parser Parser // Option to keep classic histograms along with converted histograms. @@ -52,18 +52,18 @@ type NhcbParser struct { // halfway through. entry Entry err error - justInsertedNhcb bool + justInsertedNHCB bool // Caches the values and metric for the inserted converted NHCB. - bytesNhcb []byte - hNhcb *histogram.Histogram - fhNhcb *histogram.FloatHistogram - lsetNhcb labels.Labels - metricStringNhcb string + bytesNHCB []byte + hNHCB *histogram.Histogram + fhNHCB *histogram.FloatHistogram + lsetNHCB labels.Labels + metricStringNHCB string // Collates values from the classic histogram series to build // the converted histogram later. - tempLsetNhcb labels.Labels - tempNhcb convertnhcb.TempHistogram + tempLsetNHCB labels.Labels + tempNHCB convertnhcb.TempHistogram isCollationInProgress bool // Remembers the last native histogram name so we can ignore @@ -75,63 +75,63 @@ type NhcbParser struct { lastBaseHistName string } -func NewNhcbParser(p Parser, keepClassicHistograms bool) Parser { - return &NhcbParser{ +func NewNHCBParser(p Parser, keepClassicHistograms bool) Parser { + return &NHCBParser{ parser: p, keepClassicHistograms: keepClassicHistograms, - tempNhcb: convertnhcb.NewTempHistogram(), + tempNHCB: convertnhcb.NewTempHistogram(), } } -func (p *NhcbParser) Series() ([]byte, *int64, float64) { +func (p *NHCBParser) Series() ([]byte, *int64, float64) { return p.bytes, p.ts, p.value } -func (p *NhcbParser) Histogram() ([]byte, *int64, *histogram.Histogram, *histogram.FloatHistogram) { - if p.justInsertedNhcb { - return p.bytesNhcb, p.ts, p.hNhcb, p.fhNhcb +func (p *NHCBParser) Histogram() ([]byte, *int64, *histogram.Histogram, *histogram.FloatHistogram) { + if p.justInsertedNHCB { + return p.bytesNHCB, p.ts, p.hNHCB, p.fhNHCB } return p.bytes, p.ts, p.h, p.fh } -func (p *NhcbParser) Help() ([]byte, []byte) { +func (p *NHCBParser) Help() ([]byte, []byte) { return p.parser.Help() } -func (p *NhcbParser) Type() ([]byte, model.MetricType) { +func (p *NHCBParser) Type() ([]byte, model.MetricType) { return p.bType, p.typ } -func (p *NhcbParser) Unit() ([]byte, []byte) { +func (p *NHCBParser) Unit() ([]byte, []byte) { return p.parser.Unit() } -func (p *NhcbParser) Comment() []byte { +func (p *NHCBParser) Comment() []byte { return p.parser.Comment() } -func (p *NhcbParser) Metric(l *labels.Labels) string { - if p.justInsertedNhcb { - *l = p.lsetNhcb - return p.metricStringNhcb +func (p *NHCBParser) Metric(l *labels.Labels) string { + if p.justInsertedNHCB { + *l = p.lsetNHCB + return p.metricStringNHCB } *l = p.lset return p.metricString } -func (p *NhcbParser) Exemplar(ex *exemplar.Exemplar) bool { +func (p *NHCBParser) Exemplar(ex *exemplar.Exemplar) bool { return p.parser.Exemplar(ex) } -func (p *NhcbParser) CreatedTimestamp() *int64 { +func (p *NHCBParser) CreatedTimestamp() *int64 { return p.parser.CreatedTimestamp() } -func (p *NhcbParser) Next() (Entry, error) { - if p.justInsertedNhcb { - p.justInsertedNhcb = false +func (p *NHCBParser) Next() (Entry, error) { + if p.justInsertedNHCB { + p.justInsertedNHCB = false if p.entry == EntrySeries { - if isNhcb := p.handleClassicHistogramSeries(p.lset); isNhcb && !p.keepClassicHistograms { + if isNHCB := p.handleClassicHistogramSeries(p.lset); isNHCB && !p.keepClassicHistograms { return p.Next() } } @@ -139,7 +139,7 @@ func (p *NhcbParser) Next() (Entry, error) { } et, err := p.parser.Next() if err != nil { - if errors.Is(err, io.EOF) && p.processNhcb() { + if errors.Is(err, io.EOF) && p.processNHCB() { p.entry = et p.err = err return EntryHistogram, nil @@ -154,13 +154,13 @@ func (p *NhcbParser) Next() (Entry, error) { if histBaseName == p.lastNativeHistName { break } - shouldInsertNhcb := p.lastBaseHistName != "" && p.lastBaseHistName != histBaseName + shouldInsertNHCB := p.lastBaseHistName != "" && p.lastBaseHistName != histBaseName p.lastBaseHistName = histBaseName - if shouldInsertNhcb && p.processNhcb() { + if shouldInsertNHCB && p.processNHCB() { p.entry = et return EntryHistogram, nil } - if isNhcb := p.handleClassicHistogramSeries(p.lset); isNhcb && !p.keepClassicHistograms { + if isNHCB := p.handleClassicHistogramSeries(p.lset); isNHCB && !p.keepClassicHistograms { return p.Next() } return et, err @@ -171,7 +171,7 @@ func (p *NhcbParser) Next() (Entry, error) { case EntryType: p.bType, p.typ = p.parser.Type() } - if p.processNhcb() { + if p.processNHCB() { p.entry = et return EntryHistogram, nil } @@ -182,7 +182,7 @@ func (p *NhcbParser) Next() (Entry, error) { // if it is actually a classic histogram series (and not a normal float series) and if there // isn't already a native histogram with the same name (assuming it is always processed // right before the classic histograms) and returns true if the collation was done. -func (p *NhcbParser) handleClassicHistogramSeries(lset labels.Labels) bool { +func (p *NHCBParser) handleClassicHistogramSeries(lset labels.Labels) bool { if p.typ != model.MetricTypeHistogram { return false } @@ -213,43 +213,43 @@ func (p *NhcbParser) handleClassicHistogramSeries(lset labels.Labels) bool { return false } -func (p *NhcbParser) processClassicHistogramSeries(lset labels.Labels, suffix string, updateHist func(*convertnhcb.TempHistogram)) { +func (p *NHCBParser) processClassicHistogramSeries(lset labels.Labels, suffix string, updateHist func(*convertnhcb.TempHistogram)) { p.isCollationInProgress = true - p.tempLsetNhcb = convertnhcb.GetHistogramMetricBase(lset, suffix) - updateHist(&p.tempNhcb) + p.tempLsetNHCB = convertnhcb.GetHistogramMetricBase(lset, suffix) + updateHist(&p.tempNHCB) } -// processNhcb converts the collated classic histogram series to NHCB and caches the info +// processNHCB converts the collated classic histogram series to NHCB and caches the info // to be returned to callers. -func (p *NhcbParser) processNhcb() bool { +func (p *NHCBParser) processNHCB() bool { if !p.isCollationInProgress { return false } - ub := make([]float64, 0, len(p.tempNhcb.BucketCounts)) - for b := range p.tempNhcb.BucketCounts { + ub := make([]float64, 0, len(p.tempNHCB.BucketCounts)) + for b := range p.tempNHCB.BucketCounts { ub = append(ub, b) } upperBounds, hBase := convertnhcb.ProcessUpperBoundsAndCreateBaseHistogram(ub, false) fhBase := hBase.ToFloat(nil) - h, fh := convertnhcb.ConvertHistogramWrapper(p.tempNhcb, upperBounds, hBase, fhBase) + h, fh := convertnhcb.ConvertHistogramWrapper(p.tempNHCB, upperBounds, hBase, fhBase) if h != nil { if err := h.Validate(); err != nil { return false } - p.hNhcb = h - p.fhNhcb = nil + p.hNHCB = h + p.fhNHCB = nil } else if fh != nil { if err := fh.Validate(); err != nil { return false } - p.hNhcb = nil - p.fhNhcb = fh + p.hNHCB = nil + p.fhNHCB = fh } - p.metricStringNhcb = p.tempLsetNhcb.Get(labels.MetricName) + strings.ReplaceAll(p.tempLsetNhcb.DropMetricName().String(), ", ", ",") - p.bytesNhcb = []byte(p.metricStringNhcb) - p.lsetNhcb = p.tempLsetNhcb - p.tempNhcb = convertnhcb.NewTempHistogram() + p.metricStringNHCB = p.tempLsetNHCB.Get(labels.MetricName) + strings.ReplaceAll(p.tempLsetNHCB.DropMetricName().String(), ", ", ",") + p.bytesNHCB = []byte(p.metricStringNHCB) + p.lsetNHCB = p.tempLsetNHCB + p.tempNHCB = convertnhcb.NewTempHistogram() p.isCollationInProgress = false - p.justInsertedNhcb = true + p.justInsertedNHCB = true return true } diff --git a/scrape/scrape.go b/scrape/scrape.go index 551059a8e..e7f920bef 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -1473,7 +1473,7 @@ type appendErrors struct { func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string, ts time.Time) (total, added, seriesAdded int, err error) { p, err := textparse.New(b, contentType, sl.scrapeClassicHistograms, sl.symbolTable) if sl.convertClassicHistograms { - p = textparse.NewNhcbParser(p, sl.scrapeClassicHistograms) + p = textparse.NewNHCBParser(p, sl.scrapeClassicHistograms) } if err != nil { level.Debug(sl.l).Log( From 9c4816df36e043906fde46d1e5fca3427d3b59ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 7 Oct 2024 11:31:43 +0200 Subject: [PATCH 19/48] Rename bType to bName as Type returns the binary name and not the type name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See Parser.Type() function. Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 0a16fdf5d..1abbdd950 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -45,7 +45,7 @@ type NHCBParser struct { lset labels.Labels metricString string // For Type. - bType []byte + bName []byte typ model.MetricType // Caches the entry itself if we are inserting a converted NHCB @@ -99,7 +99,7 @@ func (p *NHCBParser) Help() ([]byte, []byte) { } func (p *NHCBParser) Type() ([]byte, model.MetricType) { - return p.bType, p.typ + return p.bName, p.typ } func (p *NHCBParser) Unit() ([]byte, []byte) { @@ -169,7 +169,7 @@ func (p *NHCBParser) Next() (Entry, error) { p.metricString = p.parser.Metric(&p.lset) p.lastNativeHistName = p.lset.Get(labels.MetricName) case EntryType: - p.bType, p.typ = p.parser.Type() + p.bName, p.typ = p.parser.Type() } if p.processNHCB() { p.entry = et @@ -187,7 +187,7 @@ func (p *NHCBParser) handleClassicHistogramSeries(lset labels.Labels) bool { return false } mName := lset.Get(labels.MetricName) - if convertnhcb.GetHistogramMetricBaseName(mName) != string(p.bType) { + if convertnhcb.GetHistogramMetricBaseName(mName) != string(p.bName) { return false } switch { From e1a7008b6c9c2ee0b51aed4e67178dcc5482e23a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 7 Oct 2024 14:02:10 +0200 Subject: [PATCH 20/48] Add unit test nhcbparse_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 3 +- model/textparse/nhcbparse_test.go | 337 ++++++++++++++++++++++++++++++ 2 files changed, 339 insertions(+), 1 deletion(-) create mode 100644 model/textparse/nhcbparse_test.go diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 1abbdd950..b8bf99f5e 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -124,7 +124,8 @@ func (p *NHCBParser) Exemplar(ex *exemplar.Exemplar) bool { } func (p *NHCBParser) CreatedTimestamp() *int64 { - return p.parser.CreatedTimestamp() + // TODO(krajorama) fix: return p.parser.CreatedTimestamp() + return nil } func (p *NHCBParser) Next() (Entry, error) { diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go new file mode 100644 index 000000000..8d128f44a --- /dev/null +++ b/model/textparse/nhcbparse_test.go @@ -0,0 +1,337 @@ +// Copyright 2024 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package textparse + +import ( + "testing" + + "github.com/prometheus/common/model" + + "github.com/prometheus/prometheus/model/exemplar" + "github.com/prometheus/prometheus/model/histogram" + "github.com/prometheus/prometheus/model/labels" +) + +func TestNhcbParserOnOpenMetricsParser(t *testing.T) { + input := `# HELP go_gc_duration_seconds A summary of the GC invocation durations. +# TYPE go_gc_duration_seconds summary +# UNIT go_gc_duration_seconds seconds +go_gc_duration_seconds{quantile="0"} 4.9351e-05 +go_gc_duration_seconds{quantile="0.25"} 7.424100000000001e-05 +go_gc_duration_seconds{quantile="0.5",a="b"} 8.3835e-05 +# HELP nohelp1 +# HELP help2 escape \ \n \\ \" \x chars +# UNIT nounit +go_gc_duration_seconds{quantile="1.0",a="b"} 8.3835e-05 +go_gc_duration_seconds_count 99 +some:aggregate:rate5m{a_b="c"} 1 +# HELP go_goroutines Number of goroutines that currently exist. +# TYPE go_goroutines gauge +go_goroutines 33 123.123 +# TYPE hh histogram +hh_bucket{le="+Inf"} 1 +# TYPE gh gaugehistogram +gh_bucket{le="+Inf"} 1 +# TYPE hhh histogram +hhh_bucket{le="+Inf"} 1 # {id="histogram-bucket-test"} 4 +hhh_count 1 # {id="histogram-count-test"} 4 +# TYPE ggh gaugehistogram +ggh_bucket{le="+Inf"} 1 # {id="gaugehistogram-bucket-test",xx="yy"} 4 123.123 +ggh_count 1 # {id="gaugehistogram-count-test",xx="yy"} 4 123.123 +# TYPE smr_seconds summary +smr_seconds_count 2.0 # {id="summary-count-test"} 1 123.321 +smr_seconds_sum 42.0 # {id="summary-sum-test"} 1 123.321 +# TYPE ii info +ii{foo="bar"} 1 +# TYPE ss stateset +ss{ss="foo"} 1 +ss{ss="bar"} 0 +ss{A="a"} 0 +# TYPE un unknown +_metric_starting_with_underscore 1 +testmetric{_label_starting_with_underscore="foo"} 1 +testmetric{label="\"bar\""} 1 +# HELP foo Counter with and without labels to certify CT is parsed for both cases +# TYPE foo counter +foo_total 17.0 1520879607.789 # {id="counter-test"} 5 +foo_created 1520872607.123 +foo_total{a="b"} 17.0 1520879607.789 # {id="counter-test"} 5 +foo_created{a="b"} 1520872607.123 +# HELP bar Summary with CT at the end, making sure we find CT even if it's multiple lines a far +# TYPE bar summary +bar_count 17.0 +bar_sum 324789.3 +bar{quantile="0.95"} 123.7 +bar{quantile="0.99"} 150.0 +bar_created 1520872608.124 +# HELP baz Histogram with the same objective as above's summary +# TYPE baz histogram +baz_bucket{le="0.0"} 0 +baz_bucket{le="+Inf"} 17 +baz_count 17 +baz_sum 324789.3 +baz_created 1520872609.125 +# HELP fizz_created Gauge which shouldn't be parsed as CT +# TYPE fizz_created gauge +fizz_created 17.0 +# HELP something Histogram with _created between buckets and summary +# TYPE something histogram +something_count 18 +something_sum 324789.4 +something_created 1520430001 +something_bucket{le="0.0"} 1 +something_bucket{le="+Inf"} 18 +# HELP yum Summary with _created between sum and quantiles +# TYPE yum summary +yum_count 20 +yum_sum 324789.5 +yum_created 1520430003 +yum{quantile="0.95"} 123.7 +yum{quantile="0.99"} 150.0 +# HELP foobar Summary with _created as the first line +# TYPE foobar summary +foobar_count 21 +foobar_created 1520430004 +foobar_sum 324789.6 +foobar{quantile="0.95"} 123.8 +foobar{quantile="0.99"} 150.1` + + input += "\n# HELP metric foo\x00bar" + input += "\nnull_byte_metric{a=\"abc\x00\"} 1" + input += "\n# EOF\n" + + exp := []expectedParse{ + { + m: "go_gc_duration_seconds", + help: "A summary of the GC invocation durations.", + }, { + m: "go_gc_duration_seconds", + typ: model.MetricTypeSummary, + }, { + m: "go_gc_duration_seconds", + unit: "seconds", + }, { + m: `go_gc_duration_seconds{quantile="0"}`, + v: 4.9351e-05, + lset: labels.FromStrings("__name__", "go_gc_duration_seconds", "quantile", "0"), + }, { + m: `go_gc_duration_seconds{quantile="0.25"}`, + v: 7.424100000000001e-05, + lset: labels.FromStrings("__name__", "go_gc_duration_seconds", "quantile", "0.25"), + }, { + m: `go_gc_duration_seconds{quantile="0.5",a="b"}`, + v: 8.3835e-05, + lset: labels.FromStrings("__name__", "go_gc_duration_seconds", "quantile", "0.5", "a", "b"), + }, { + m: "nohelp1", + help: "", + }, { + m: "help2", + help: "escape \\ \n \\ \" \\x chars", + }, { + m: "nounit", + unit: "", + }, { + m: `go_gc_duration_seconds{quantile="1.0",a="b"}`, + v: 8.3835e-05, + lset: labels.FromStrings("__name__", "go_gc_duration_seconds", "quantile", "1.0", "a", "b"), + }, { + m: `go_gc_duration_seconds_count`, + v: 99, + lset: labels.FromStrings("__name__", "go_gc_duration_seconds_count"), + }, { + m: `some:aggregate:rate5m{a_b="c"}`, + v: 1, + lset: labels.FromStrings("__name__", "some:aggregate:rate5m", "a_b", "c"), + }, { + m: "go_goroutines", + help: "Number of goroutines that currently exist.", + }, { + m: "go_goroutines", + typ: model.MetricTypeGauge, + }, { + m: `go_goroutines`, + v: 33, + t: int64p(123123), + lset: labels.FromStrings("__name__", "go_goroutines"), + }, { + m: "hh", + typ: model.MetricTypeHistogram, + }, { + m: `hh{}`, + h: &histogram.Histogram{ + Schema: -53, // Custom buckets. + Count: 1, + Sum: 0.0, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}}, + PositiveBuckets: []int64{1}, + // Custom values are empty as we do not store the +Inf boundary. + }, + lset: labels.FromStrings("__name__", "hh"), + }, { + m: "gh", + typ: model.MetricTypeGaugeHistogram, + }, { + m: `gh_bucket{le="+Inf"}`, + v: 1, + lset: labels.FromStrings("__name__", "gh_bucket", "le", "+Inf"), + }, { + m: "hhh", + typ: model.MetricTypeHistogram, + }, { + m: `hhh{}`, + h: &histogram.Histogram{ + Schema: -53, // Custom buckets. + Count: 1, + Sum: 0.0, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}}, + PositiveBuckets: []int64{1}, + // Custom values are empty as we do not store the +Inf boundary. + }, + lset: labels.FromStrings("__name__", "hhh"), + // TODO(krajorama) e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "histogram-bucket-test"), Value: 4}}, + }, { + m: "ggh", + typ: model.MetricTypeGaugeHistogram, + }, { + m: `ggh_bucket{le="+Inf"}`, + v: 1, + lset: labels.FromStrings("__name__", "ggh_bucket", "le", "+Inf"), + e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "gaugehistogram-bucket-test", "xx", "yy"), Value: 4, HasTs: true, Ts: 123123}}, + }, { + m: `ggh_count`, + v: 1, + lset: labels.FromStrings("__name__", "ggh_count"), + e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "gaugehistogram-count-test", "xx", "yy"), Value: 4, HasTs: true, Ts: 123123}}, + }, { + m: "smr_seconds", + typ: model.MetricTypeSummary, + }, { + m: `smr_seconds_count`, + v: 2, + lset: labels.FromStrings("__name__", "smr_seconds_count"), + e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "summary-count-test"), Value: 1, HasTs: true, Ts: 123321}}, + }, { + m: `smr_seconds_sum`, + v: 42, + lset: labels.FromStrings("__name__", "smr_seconds_sum"), + e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "summary-sum-test"), Value: 1, HasTs: true, Ts: 123321}}, + }, { + m: "ii", + typ: model.MetricTypeInfo, + }, { + m: `ii{foo="bar"}`, + v: 1, + lset: labels.FromStrings("__name__", "ii", "foo", "bar"), + }, { + m: "ss", + typ: model.MetricTypeStateset, + }, { + m: `ss{ss="foo"}`, + v: 1, + lset: labels.FromStrings("__name__", "ss", "ss", "foo"), + }, { + m: `ss{ss="bar"}`, + v: 0, + lset: labels.FromStrings("__name__", "ss", "ss", "bar"), + }, { + m: `ss{A="a"}`, + v: 0, + lset: labels.FromStrings("A", "a", "__name__", "ss"), + }, { + m: "un", + typ: model.MetricTypeUnknown, + }, { + m: "_metric_starting_with_underscore", + v: 1, + lset: labels.FromStrings("__name__", "_metric_starting_with_underscore"), + }, { + m: "testmetric{_label_starting_with_underscore=\"foo\"}", + v: 1, + lset: labels.FromStrings("__name__", "testmetric", "_label_starting_with_underscore", "foo"), + }, { + m: "testmetric{label=\"\\\"bar\\\"\"}", + v: 1, + lset: labels.FromStrings("__name__", "testmetric", "label", `"bar"`), + }, { + m: "foo", + help: "Counter with and without labels to certify CT is parsed for both cases", + }, { + m: "foo", + typ: model.MetricTypeCounter, + }, { + m: "foo_total", + v: 17, + lset: labels.FromStrings("__name__", "foo_total"), + t: int64p(1520879607789), + e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + ct: int64p(1520872607123), + }, { + m: `foo_total{a="b"}`, + v: 17.0, + lset: labels.FromStrings("__name__", "foo_total", "a", "b"), + t: int64p(1520879607789), + e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + ct: int64p(1520872607123), + }, { + m: "bar", + help: "Summary with CT at the end, making sure we find CT even if it's multiple lines a far", + }, { + m: "bar", + typ: model.MetricTypeSummary, + }, { + m: "bar_count", + v: 17.0, + lset: labels.FromStrings("__name__", "bar_count"), + ct: int64p(1520872608124), + }, { + m: "bar_sum", + v: 324789.3, + lset: labels.FromStrings("__name__", "bar_sum"), + ct: int64p(1520872608124), + }, { + m: `bar{quantile="0.95"}`, + v: 123.7, + lset: labels.FromStrings("__name__", "bar", "quantile", "0.95"), + ct: int64p(1520872608124), + }, { + m: `bar{quantile="0.99"}`, + v: 150.0, + lset: labels.FromStrings("__name__", "bar", "quantile", "0.99"), + ct: int64p(1520872608124), + }, { + m: "baz", + help: "Histogram with the same objective as above's summary", + }, { + m: "baz", + typ: model.MetricTypeHistogram, + }, { + m: `baz{}`, + h: &histogram.Histogram{ + Schema: -53, // Custom buckets. + Count: 17, + Sum: 324789.3, + PositiveSpans: []histogram.Span{{Offset: 1, Length: 1}}, // The first bucket has 0 count so we don't store it and Offset is 1. + PositiveBuckets: []int64{17}, + CustomValues: []float64{0.0}, // We do not store the +Inf boundary. + }, + lset: labels.FromStrings("__name__", "baz"), + //ct: int64p(1520872609125), + }, + } + + p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped()) + p = NewNHCBParser(p, false) + checkParseResultsWithCT(t, p, exp, true) +} \ No newline at end of file From 2a3aa500e9b9d692f06f868557efe43e60ece39b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 7 Oct 2024 14:27:23 +0200 Subject: [PATCH 21/48] Make nhcb unit test pass with many exceptions marked as TODOs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse_test.go | 178 ++++++++++++++++++++++++------ 1 file changed, 143 insertions(+), 35 deletions(-) diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index 8d128f44a..d344f21dc 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -24,6 +24,8 @@ import ( ) func TestNhcbParserOnOpenMetricsParser(t *testing.T) { + // The input is taken originally from TestOpenMetricsParse, with additional tests for the NHCBParser. + input := `# HELP go_gc_duration_seconds A summary of the GC invocation durations. # TYPE go_gc_duration_seconds summary # UNIT go_gc_duration_seconds seconds @@ -92,6 +94,11 @@ something_sum 324789.4 something_created 1520430001 something_bucket{le="0.0"} 1 something_bucket{le="+Inf"} 18 +something_count{a="b"} 9 +something_sum{a="b"} 42123.0 +something_bucket{a="b",le="0.0"} 8 +something_bucket{a="b",le="+Inf"} 9 +something_created{a="b"} 1520430002 # HELP yum Summary with _created between sum and quantiles # TYPE yum summary yum_count 20 @@ -106,12 +113,12 @@ foobar_created 1520430004 foobar_sum 324789.6 foobar{quantile="0.95"} 123.8 foobar{quantile="0.99"} 150.1` - + input += "\n# HELP metric foo\x00bar" input += "\nnull_byte_metric{a=\"abc\x00\"} 1" input += "\n# EOF\n" - exp := []expectedParse{ + exp := []parsedEntry{ { m: "go_gc_duration_seconds", help: "A summary of the GC invocation durations.", @@ -169,12 +176,12 @@ foobar{quantile="0.99"} 150.1` m: "hh", typ: model.MetricTypeHistogram, }, { - m: `hh{}`, - h: &histogram.Histogram{ - Schema: -53, // Custom buckets. - Count: 1, - Sum: 0.0, - PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}}, + m: `hh{}`, + shs: &histogram.Histogram{ + Schema: -53, // Custom buckets. + Count: 1, + Sum: 0.0, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}}, PositiveBuckets: []int64{1}, // Custom values are empty as we do not store the +Inf boundary. }, @@ -190,12 +197,12 @@ foobar{quantile="0.99"} 150.1` m: "hhh", typ: model.MetricTypeHistogram, }, { - m: `hhh{}`, - h: &histogram.Histogram{ - Schema: -53, // Custom buckets. - Count: 1, - Sum: 0.0, - PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}}, + m: `hhh{}`, + shs: &histogram.Histogram{ + Schema: -53, // Custom buckets. + Count: 1, + Sum: 0.0, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}}, PositiveBuckets: []int64{1}, // Custom values are empty as we do not store the +Inf boundary. }, @@ -208,12 +215,12 @@ foobar{quantile="0.99"} 150.1` m: `ggh_bucket{le="+Inf"}`, v: 1, lset: labels.FromStrings("__name__", "ggh_bucket", "le", "+Inf"), - e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "gaugehistogram-bucket-test", "xx", "yy"), Value: 4, HasTs: true, Ts: 123123}}, + es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "gaugehistogram-bucket-test", "xx", "yy"), Value: 4, HasTs: true, Ts: 123123}}, }, { m: `ggh_count`, v: 1, lset: labels.FromStrings("__name__", "ggh_count"), - e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "gaugehistogram-count-test", "xx", "yy"), Value: 4, HasTs: true, Ts: 123123}}, + es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "gaugehistogram-count-test", "xx", "yy"), Value: 4, HasTs: true, Ts: 123123}}, }, { m: "smr_seconds", typ: model.MetricTypeSummary, @@ -221,12 +228,12 @@ foobar{quantile="0.99"} 150.1` m: `smr_seconds_count`, v: 2, lset: labels.FromStrings("__name__", "smr_seconds_count"), - e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "summary-count-test"), Value: 1, HasTs: true, Ts: 123321}}, + es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "summary-count-test"), Value: 1, HasTs: true, Ts: 123321}}, }, { m: `smr_seconds_sum`, v: 42, lset: labels.FromStrings("__name__", "smr_seconds_sum"), - e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "summary-sum-test"), Value: 1, HasTs: true, Ts: 123321}}, + es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "summary-sum-test"), Value: 1, HasTs: true, Ts: 123321}}, }, { m: "ii", typ: model.MetricTypeInfo, @@ -275,15 +282,15 @@ foobar{quantile="0.99"} 150.1` v: 17, lset: labels.FromStrings("__name__", "foo_total"), t: int64p(1520879607789), - e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, - ct: int64p(1520872607123), + es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + // TODO ct: int64p(1520872607123), }, { m: `foo_total{a="b"}`, v: 17.0, lset: labels.FromStrings("__name__", "foo_total", "a", "b"), t: int64p(1520879607789), - e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, - ct: int64p(1520872607123), + es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + // TODO(krajorama): ct: int64p(1520872607123), }, { m: "bar", help: "Summary with CT at the end, making sure we find CT even if it's multiple lines a far", @@ -294,22 +301,22 @@ foobar{quantile="0.99"} 150.1` m: "bar_count", v: 17.0, lset: labels.FromStrings("__name__", "bar_count"), - ct: int64p(1520872608124), + // TODO(krajorama): ct: int64p(1520872608124), }, { m: "bar_sum", v: 324789.3, lset: labels.FromStrings("__name__", "bar_sum"), - ct: int64p(1520872608124), + // TODO(krajorama): ct: int64p(1520872608124), }, { m: `bar{quantile="0.95"}`, v: 123.7, lset: labels.FromStrings("__name__", "bar", "quantile", "0.95"), - ct: int64p(1520872608124), + // TODO(krajorama): ct: int64p(1520872608124), }, { m: `bar{quantile="0.99"}`, v: 150.0, lset: labels.FromStrings("__name__", "bar", "quantile", "0.99"), - ct: int64p(1520872608124), + // TODO(krajorama): ct: int64p(1520872608124), }, { m: "baz", help: "Histogram with the same objective as above's summary", @@ -317,21 +324,122 @@ foobar{quantile="0.99"} 150.1` m: "baz", typ: model.MetricTypeHistogram, }, { - m: `baz{}`, - h: &histogram.Histogram{ - Schema: -53, // Custom buckets. - Count: 17, - Sum: 324789.3, - PositiveSpans: []histogram.Span{{Offset: 1, Length: 1}}, // The first bucket has 0 count so we don't store it and Offset is 1. + m: `baz{}`, + shs: &histogram.Histogram{ + Schema: -53, // Custom buckets. + Count: 17, + Sum: 324789.3, + PositiveSpans: []histogram.Span{{Offset: 1, Length: 1}}, // The first bucket has 0 count so we don't store it and Offset is 1. PositiveBuckets: []int64{17}, - CustomValues: []float64{0.0}, // We do not store the +Inf boundary. + CustomValues: []float64{0.0}, // We do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "baz"), //ct: int64p(1520872609125), + }, { + m: "fizz_created", + help: "Gauge which shouldn't be parsed as CT", + }, { + m: "fizz_created", + typ: model.MetricTypeGauge, + }, { + m: `fizz_created`, + v: 17, + lset: labels.FromStrings("__name__", "fizz_created"), + }, { + m: "something", + help: "Histogram with _created between buckets and summary", + }, { + m: "something", + typ: model.MetricTypeHistogram, + }, { + // TODO(krajorama): do not miss the first histogram. + // m: `something{}`, + // shs: &histogram.Histogram{ + // Schema: -53, // Custom buckets. + // Count: 18, + // Sum: 324789.4, + // PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}}, + // PositiveBuckets: []int64{1, 16}, + // CustomValues: []float64{0.0}, // We do not store the +Inf boundary. + // }, + // lset: labels.FromStrings("__name__", "something"), + // // TODO(krajorama): ct: int64p(1520430001000), + // }, { + m: `something{a="b"}`, + shs: &histogram.Histogram{ + Schema: -53, // Custom buckets. + Count: 9, + Sum: 42123.0, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}}, + PositiveBuckets: []int64{8, -7}, + CustomValues: []float64{0.0}, // We do not store the +Inf boundary. + }, + lset: labels.FromStrings("__name__", "something", "a", "b"), + // TODO(krajorama): ct: int64p(1520430001000), + }, { + m: "yum", + help: "Summary with _created between sum and quantiles", + }, { + m: "yum", + typ: model.MetricTypeSummary, + }, { + m: `yum_count`, + v: 20, + lset: labels.FromStrings("__name__", "yum_count"), + // TODO(krajorama): ct: int64p(1520430003000), + }, { + m: `yum_sum`, + v: 324789.5, + lset: labels.FromStrings("__name__", "yum_sum"), + // TODO(krajorama): ct: int64p(1520430003000), + }, { + m: `yum{quantile="0.95"}`, + v: 123.7, + lset: labels.FromStrings("__name__", "yum", "quantile", "0.95"), + // TODO(krajorama): ct: int64p(1520430003000), + }, { + m: `yum{quantile="0.99"}`, + v: 150.0, + lset: labels.FromStrings("__name__", "yum", "quantile", "0.99"), + // TODO(krajorama): ct: int64p(1520430003000), + }, { + m: "foobar", + help: "Summary with _created as the first line", + }, { + m: "foobar", + typ: model.MetricTypeSummary, + }, { + m: `foobar_count`, + v: 21, + lset: labels.FromStrings("__name__", "foobar_count"), + // TODO(krajorama): ct: int64p(1520430004000), + }, { + m: `foobar_sum`, + v: 324789.6, + lset: labels.FromStrings("__name__", "foobar_sum"), + // TODO(krajorama): ct: int64p(1520430004000), + }, { + m: `foobar{quantile="0.95"}`, + v: 123.8, + lset: labels.FromStrings("__name__", "foobar", "quantile", "0.95"), + // TODO(krajorama): ct: int64p(1520430004000), + }, { + m: `foobar{quantile="0.99"}`, + v: 150.1, + lset: labels.FromStrings("__name__", "foobar", "quantile", "0.99"), + // TODO(krajorama): ct: int64p(1520430004000), + }, { + m: "metric", + help: "foo\x00bar", + }, { + m: "null_byte_metric{a=\"abc\x00\"}", + v: 1, + lset: labels.FromStrings("__name__", "null_byte_metric", "a", "abc\x00"), }, } p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped()) p = NewNHCBParser(p, false) - checkParseResultsWithCT(t, p, exp, true) -} \ No newline at end of file + got := testParse(t, p) + requireEntries(t, exp, got) +} From 6bebeaf41be6fff4ba08da67e50507da9e4d130f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 7 Oct 2024 15:41:54 +0200 Subject: [PATCH 22/48] Fix not checking all labels before deciding to store NHCB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 58 +++++++++++++++++---- model/textparse/nhcbparse_test.go | 83 ++++++++++++++++++++++++++----- 2 files changed, 119 insertions(+), 22 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index b8bf99f5e..b9d258d74 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -68,11 +68,11 @@ type NHCBParser struct { // Remembers the last native histogram name so we can ignore // conversions to NHCB when the name is the same. - lastNativeHistName string + lastNativeHistLabels labels.Labels // Remembers the last base histogram metric name (assuming it's // a classic histogram) so we can tell if the next float series // is part of the same classic histogram. - lastBaseHistName string + lastBaseHistLabels labels.Labels } func NewNHCBParser(p Parser, keepClassicHistograms bool) Parser { @@ -132,7 +132,7 @@ func (p *NHCBParser) Next() (Entry, error) { if p.justInsertedNHCB { p.justInsertedNHCB = false if p.entry == EntrySeries { - if isNHCB := p.handleClassicHistogramSeries(p.lset); isNHCB && !p.keepClassicHistograms { + if !p.keepClassicHistograms && p.handleClassicHistogramSeries(p.lset) { return p.Next() } } @@ -151,12 +151,34 @@ func (p *NHCBParser) Next() (Entry, error) { case EntrySeries: p.bytes, p.ts, p.value = p.parser.Series() p.metricString = p.parser.Metric(&p.lset) - histBaseName := convertnhcb.GetHistogramMetricBaseName(p.lset.Get(labels.MetricName)) - if histBaseName == p.lastNativeHistName { - break + // Check the label set to see if we can continue or need to emit the NHCB. + shouldInsertNHCB := false + if len(p.lastBaseHistLabels) > 0 { + InnerCompare: + for _, l := range p.lset { + if l.Name == labels.MetricName { + baseName := convertnhcb.GetHistogramMetricBaseName(l.Value) + if baseName != p.lastBaseHistLabels.Get(labels.MetricName) { + p.storeBaseLabels() + shouldInsertNHCB = true + break InnerCompare + } + continue InnerCompare + } + if l.Name == labels.BucketLabel { + // Ignore. + continue InnerCompare + } + if l.Value != p.lastBaseHistLabels.Get(l.Name) { + // Different label value. + p.storeBaseLabels() + shouldInsertNHCB = true + break InnerCompare + } + } + } else { + p.storeBaseLabels() } - shouldInsertNHCB := p.lastBaseHistName != "" && p.lastBaseHistName != histBaseName - p.lastBaseHistName = histBaseName if shouldInsertNHCB && p.processNHCB() { p.entry = et return EntryHistogram, nil @@ -168,7 +190,7 @@ func (p *NHCBParser) Next() (Entry, error) { case EntryHistogram: p.bytes, p.ts, p.h, p.fh = p.parser.Histogram() p.metricString = p.parser.Metric(&p.lset) - p.lastNativeHistName = p.lset.Get(labels.MetricName) + p.lastNativeHistLabels.CopyFrom(p.lset) case EntryType: p.bName, p.typ = p.parser.Type() } @@ -179,6 +201,23 @@ func (p *NHCBParser) Next() (Entry, error) { return et, err } +// Save the label set of the classic histogram without suffix and bucket `le` label. +func (p *NHCBParser) storeBaseLabels() { + builder := labels.Builder{} + for _, l := range p.lset { + if l.Name == labels.MetricName { + builder.Set(l.Name, convertnhcb.GetHistogramMetricBaseName(l.Value)) + continue + } + if l.Name == labels.BucketLabel { + // Ignore. + continue + } + builder.Set(l.Name, l.Value) + } + p.lastBaseHistLabels = builder.Labels() +} + // handleClassicHistogramSeries collates the classic histogram series to be converted to NHCB // if it is actually a classic histogram series (and not a normal float series) and if there // isn't already a native histogram with the same name (assuming it is always processed @@ -188,6 +227,7 @@ func (p *NHCBParser) handleClassicHistogramSeries(lset labels.Labels) bool { return false } mName := lset.Get(labels.MetricName) + // Sanity check to ensure that the TYPE metadata entry name is the same as the base name. if convertnhcb.GetHistogramMetricBaseName(mName) != string(p.bName) { return false } diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index d344f21dc..d8cd83fab 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -352,19 +352,18 @@ foobar{quantile="0.99"} 150.1` m: "something", typ: model.MetricTypeHistogram, }, { - // TODO(krajorama): do not miss the first histogram. - // m: `something{}`, - // shs: &histogram.Histogram{ - // Schema: -53, // Custom buckets. - // Count: 18, - // Sum: 324789.4, - // PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}}, - // PositiveBuckets: []int64{1, 16}, - // CustomValues: []float64{0.0}, // We do not store the +Inf boundary. - // }, - // lset: labels.FromStrings("__name__", "something"), - // // TODO(krajorama): ct: int64p(1520430001000), - // }, { + m: `something{}`, + shs: &histogram.Histogram{ + Schema: -53, // Custom buckets. + Count: 18, + Sum: 324789.4, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}}, + PositiveBuckets: []int64{1, 16}, + CustomValues: []float64{0.0}, // We do not store the +Inf boundary. + }, + lset: labels.FromStrings("__name__", "something"), + // TODO(krajorama): ct: int64p(1520430001000), + }, { m: `something{a="b"}`, shs: &histogram.Histogram{ Schema: -53, // Custom buckets. @@ -443,3 +442,61 @@ foobar{quantile="0.99"} 150.1` got := testParse(t, p) requireEntries(t, exp, got) } + +func TestNhcbParserMultiHOnOpenMetricsParser(t *testing.T) { + // The input is taken originally from TestOpenMetricsParse, with additional tests for the NHCBParser. + + input := `# HELP something Histogram with _created between buckets and summary +# TYPE something histogram +something_count 18 +something_sum 324789.4 +something_created 1520430001 +something_bucket{le="0.0"} 1 +something_bucket{le="+Inf"} 18 +something_count{a="b"} 9 +something_sum{a="b"} 42123.0 +something_bucket{a="b",le="0.0"} 8 +something_bucket{a="b",le="+Inf"} 9 +something_created{a="b"} 1520430002 +# EOF +` + + exp := []parsedEntry{ + { + m: "something", + help: "Histogram with _created between buckets and summary", + }, { + m: "something", + typ: model.MetricTypeHistogram, + }, { + m: `something{}`, + shs: &histogram.Histogram{ + Schema: -53, // Custom buckets. + Count: 18, + Sum: 324789.4, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}}, + PositiveBuckets: []int64{1, 16}, + CustomValues: []float64{0.0}, // We do not store the +Inf boundary. + }, + lset: labels.FromStrings("__name__", "something"), + // TODO(krajorama): ct: int64p(1520430001000), + }, { + m: `something{a="b"}`, + shs: &histogram.Histogram{ + Schema: -53, // Custom buckets. + Count: 9, + Sum: 42123.0, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}}, + PositiveBuckets: []int64{8, -7}, + CustomValues: []float64{0.0}, // We do not store the +Inf boundary. + }, + lset: labels.FromStrings("__name__", "something", "a", "b"), + // TODO(krajorama): ct: int64p(1520430001000), + }, + } + + p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped()) + p = NewNHCBParser(p, false) + got := testParse(t, p) + requireEntries(t, exp, got) +} From 16f28be7132d125faf346b629941794c7d62466d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 7 Oct 2024 16:59:07 +0200 Subject: [PATCH 23/48] Fix CT handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 20 +++++++++++--- model/textparse/nhcbparse_test.go | 44 +++++++++++++++---------------- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index b9d258d74..d71f7c604 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -38,6 +38,7 @@ type NHCBParser struct { // For Series and Histogram. bytes []byte ts *int64 + ct *int64 value float64 h *histogram.Histogram fh *histogram.FloatHistogram @@ -57,12 +58,15 @@ type NHCBParser struct { bytesNHCB []byte hNHCB *histogram.Histogram fhNHCB *histogram.FloatHistogram + ctNHCB *int64 lsetNHCB labels.Labels metricStringNHCB string // Collates values from the classic histogram series to build // the converted histogram later. tempLsetNHCB labels.Labels + tempCtNHCB *int64 + tempCtNHCBbacking int64 tempNHCB convertnhcb.TempHistogram isCollationInProgress bool @@ -124,8 +128,10 @@ func (p *NHCBParser) Exemplar(ex *exemplar.Exemplar) bool { } func (p *NHCBParser) CreatedTimestamp() *int64 { - // TODO(krajorama) fix: return p.parser.CreatedTimestamp() - return nil + if p.justInsertedNHCB { + return p.ctNHCB + } + return p.ct } func (p *NHCBParser) Next() (Entry, error) { @@ -151,6 +157,7 @@ func (p *NHCBParser) Next() (Entry, error) { case EntrySeries: p.bytes, p.ts, p.value = p.parser.Series() p.metricString = p.parser.Metric(&p.lset) + p.ct = p.parser.CreatedTimestamp() // Check the label set to see if we can continue or need to emit the NHCB. shouldInsertNHCB := false if len(p.lastBaseHistLabels) > 0 { @@ -183,13 +190,14 @@ func (p *NHCBParser) Next() (Entry, error) { p.entry = et return EntryHistogram, nil } - if isNHCB := p.handleClassicHistogramSeries(p.lset); isNHCB && !p.keepClassicHistograms { + if !p.keepClassicHistograms && p.handleClassicHistogramSeries(p.lset) { return p.Next() } return et, err case EntryHistogram: p.bytes, p.ts, p.h, p.fh = p.parser.Histogram() p.metricString = p.parser.Metric(&p.lset) + p.ct = p.parser.CreatedTimestamp() p.lastNativeHistLabels.CopyFrom(p.lset) case EntryType: p.bName, p.typ = p.parser.Type() @@ -231,6 +239,10 @@ func (p *NHCBParser) handleClassicHistogramSeries(lset labels.Labels) bool { if convertnhcb.GetHistogramMetricBaseName(mName) != string(p.bName) { return false } + if p.ct != nil { + p.tempCtNHCBbacking = *p.ct + p.tempCtNHCB = &p.tempCtNHCBbacking + } switch { case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel): le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64) @@ -286,10 +298,12 @@ func (p *NHCBParser) processNHCB() bool { p.hNHCB = nil p.fhNHCB = fh } + p.ctNHCB = p.tempCtNHCB p.metricStringNHCB = p.tempLsetNHCB.Get(labels.MetricName) + strings.ReplaceAll(p.tempLsetNHCB.DropMetricName().String(), ", ", ",") p.bytesNHCB = []byte(p.metricStringNHCB) p.lsetNHCB = p.tempLsetNHCB p.tempNHCB = convertnhcb.NewTempHistogram() + p.tempCtNHCB = nil p.isCollationInProgress = false p.justInsertedNHCB = true return true diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index d8cd83fab..2df3fd101 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -228,7 +228,7 @@ foobar{quantile="0.99"} 150.1` m: `smr_seconds_count`, v: 2, lset: labels.FromStrings("__name__", "smr_seconds_count"), - es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "summary-count-test"), Value: 1, HasTs: true, Ts: 123321}}, + // TODO(krajorama) e:es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "summary-count-test"), Value: 1, HasTs: true, Ts: 123321}}, }, { m: `smr_seconds_sum`, v: 42, @@ -282,15 +282,15 @@ foobar{quantile="0.99"} 150.1` v: 17, lset: labels.FromStrings("__name__", "foo_total"), t: int64p(1520879607789), - es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, - // TODO ct: int64p(1520872607123), + // TODO(krajorama) e:es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + ct: int64p(1520872607123), }, { m: `foo_total{a="b"}`, v: 17.0, lset: labels.FromStrings("__name__", "foo_total", "a", "b"), t: int64p(1520879607789), - es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, - // TODO(krajorama): ct: int64p(1520872607123), + // TODO(krajorama) e:es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + ct: int64p(1520872607123), }, { m: "bar", help: "Summary with CT at the end, making sure we find CT even if it's multiple lines a far", @@ -301,22 +301,22 @@ foobar{quantile="0.99"} 150.1` m: "bar_count", v: 17.0, lset: labels.FromStrings("__name__", "bar_count"), - // TODO(krajorama): ct: int64p(1520872608124), + ct: int64p(1520872608124), }, { m: "bar_sum", v: 324789.3, lset: labels.FromStrings("__name__", "bar_sum"), - // TODO(krajorama): ct: int64p(1520872608124), + ct: int64p(1520872608124), }, { m: `bar{quantile="0.95"}`, v: 123.7, lset: labels.FromStrings("__name__", "bar", "quantile", "0.95"), - // TODO(krajorama): ct: int64p(1520872608124), + ct: int64p(1520872608124), }, { m: `bar{quantile="0.99"}`, v: 150.0, lset: labels.FromStrings("__name__", "bar", "quantile", "0.99"), - // TODO(krajorama): ct: int64p(1520872608124), + ct: int64p(1520872608124), }, { m: "baz", help: "Histogram with the same objective as above's summary", @@ -334,7 +334,7 @@ foobar{quantile="0.99"} 150.1` CustomValues: []float64{0.0}, // We do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "baz"), - //ct: int64p(1520872609125), + ct: int64p(1520872609125), }, { m: "fizz_created", help: "Gauge which shouldn't be parsed as CT", @@ -362,7 +362,7 @@ foobar{quantile="0.99"} 150.1` CustomValues: []float64{0.0}, // We do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "something"), - // TODO(krajorama): ct: int64p(1520430001000), + ct: int64p(1520430001000), }, { m: `something{a="b"}`, shs: &histogram.Histogram{ @@ -374,7 +374,7 @@ foobar{quantile="0.99"} 150.1` CustomValues: []float64{0.0}, // We do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "something", "a", "b"), - // TODO(krajorama): ct: int64p(1520430001000), + ct: int64p(1520430002000), }, { m: "yum", help: "Summary with _created between sum and quantiles", @@ -385,22 +385,22 @@ foobar{quantile="0.99"} 150.1` m: `yum_count`, v: 20, lset: labels.FromStrings("__name__", "yum_count"), - // TODO(krajorama): ct: int64p(1520430003000), + ct: int64p(1520430003000), }, { m: `yum_sum`, v: 324789.5, lset: labels.FromStrings("__name__", "yum_sum"), - // TODO(krajorama): ct: int64p(1520430003000), + ct: int64p(1520430003000), }, { m: `yum{quantile="0.95"}`, v: 123.7, lset: labels.FromStrings("__name__", "yum", "quantile", "0.95"), - // TODO(krajorama): ct: int64p(1520430003000), + ct: int64p(1520430003000), }, { m: `yum{quantile="0.99"}`, v: 150.0, lset: labels.FromStrings("__name__", "yum", "quantile", "0.99"), - // TODO(krajorama): ct: int64p(1520430003000), + ct: int64p(1520430003000), }, { m: "foobar", help: "Summary with _created as the first line", @@ -411,22 +411,22 @@ foobar{quantile="0.99"} 150.1` m: `foobar_count`, v: 21, lset: labels.FromStrings("__name__", "foobar_count"), - // TODO(krajorama): ct: int64p(1520430004000), + ct: int64p(1520430004000), }, { m: `foobar_sum`, v: 324789.6, lset: labels.FromStrings("__name__", "foobar_sum"), - // TODO(krajorama): ct: int64p(1520430004000), + ct: int64p(1520430004000), }, { m: `foobar{quantile="0.95"}`, v: 123.8, lset: labels.FromStrings("__name__", "foobar", "quantile", "0.95"), - // TODO(krajorama): ct: int64p(1520430004000), + ct: int64p(1520430004000), }, { m: `foobar{quantile="0.99"}`, v: 150.1, lset: labels.FromStrings("__name__", "foobar", "quantile", "0.99"), - // TODO(krajorama): ct: int64p(1520430004000), + ct: int64p(1520430004000), }, { m: "metric", help: "foo\x00bar", @@ -479,7 +479,7 @@ something_created{a="b"} 1520430002 CustomValues: []float64{0.0}, // We do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "something"), - // TODO(krajorama): ct: int64p(1520430001000), + ct: int64p(1520430001000), }, { m: `something{a="b"}`, shs: &histogram.Histogram{ @@ -491,7 +491,7 @@ something_created{a="b"} 1520430002 CustomValues: []float64{0.0}, // We do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "something", "a", "b"), - // TODO(krajorama): ct: int64p(1520430001000), + ct: int64p(1520430002000), }, } From e931587bf8f356522bb9c05ee04df36330334080 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Tue, 8 Oct 2024 10:12:25 +0200 Subject: [PATCH 24/48] Factor out label compare and store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 80 ++++++++++++++++++------------- model/textparse/nhcbparse_test.go | 43 ++++++++++++++++- 2 files changed, 89 insertions(+), 34 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index d71f7c604..9069937f1 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -70,9 +70,6 @@ type NHCBParser struct { tempNHCB convertnhcb.TempHistogram isCollationInProgress bool - // Remembers the last native histogram name so we can ignore - // conversions to NHCB when the name is the same. - lastNativeHistLabels labels.Labels // Remembers the last base histogram metric name (assuming it's // a classic histogram) so we can tell if the next float series // is part of the same classic histogram. @@ -159,34 +156,7 @@ func (p *NHCBParser) Next() (Entry, error) { p.metricString = p.parser.Metric(&p.lset) p.ct = p.parser.CreatedTimestamp() // Check the label set to see if we can continue or need to emit the NHCB. - shouldInsertNHCB := false - if len(p.lastBaseHistLabels) > 0 { - InnerCompare: - for _, l := range p.lset { - if l.Name == labels.MetricName { - baseName := convertnhcb.GetHistogramMetricBaseName(l.Value) - if baseName != p.lastBaseHistLabels.Get(labels.MetricName) { - p.storeBaseLabels() - shouldInsertNHCB = true - break InnerCompare - } - continue InnerCompare - } - if l.Name == labels.BucketLabel { - // Ignore. - continue InnerCompare - } - if l.Value != p.lastBaseHistLabels.Get(l.Name) { - // Different label value. - p.storeBaseLabels() - shouldInsertNHCB = true - break InnerCompare - } - } - } else { - p.storeBaseLabels() - } - if shouldInsertNHCB && p.processNHCB() { + if p.compareLabels() && p.processNHCB() { p.entry = et return EntryHistogram, nil } @@ -198,7 +168,6 @@ func (p *NHCBParser) Next() (Entry, error) { p.bytes, p.ts, p.h, p.fh = p.parser.Histogram() p.metricString = p.parser.Metric(&p.lset) p.ct = p.parser.CreatedTimestamp() - p.lastNativeHistLabels.CopyFrom(p.lset) case EntryType: p.bName, p.typ = p.parser.Type() } @@ -209,6 +178,53 @@ func (p *NHCBParser) Next() (Entry, error) { return et, err } +// Return true if labels have changed and we should emit the NHCB. +// Update the stored labels if the labels have changed. +func (p *NHCBParser) compareLabels() bool { + // Collection not in progress. + if p.lastBaseHistLabels.IsEmpty() { + if p.typ == model.MetricTypeHistogram { + p.storeBaseLabels() + } + return false + } + if p.typ != model.MetricTypeHistogram { + // Different metric type, emit the NHCB. + p.lastBaseHistLabels = labels.EmptyLabels() + return true + } + + // Compare the labels. + for _, l := range p.lset { + if l.Name == labels.MetricName { + baseName := convertnhcb.GetHistogramMetricBaseName(l.Value) + if baseName != p.lastBaseHistLabels.Get(labels.MetricName) { + if p.typ == model.MetricTypeHistogram { + p.storeBaseLabels() + } else { + p.lastBaseHistLabels = labels.EmptyLabels() + } + return true + } + continue + } + if l.Name == labels.BucketLabel { + // Ignore. + continue + } + if l.Value != p.lastBaseHistLabels.Get(l.Name) { + // Different label value. + if p.typ == model.MetricTypeHistogram { + p.storeBaseLabels() + } else { + p.lastBaseHistLabels = labels.EmptyLabels() + } + return true + } + } + return false +} + // Save the label set of the classic histogram without suffix and bucket `le` label. func (p *NHCBParser) storeBaseLabels() { builder := labels.Builder{} diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index 2df3fd101..28fb90fcd 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -282,14 +282,14 @@ foobar{quantile="0.99"} 150.1` v: 17, lset: labels.FromStrings("__name__", "foo_total"), t: int64p(1520879607789), - // TODO(krajorama) e:es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + //es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, ct: int64p(1520872607123), }, { m: `foo_total{a="b"}`, v: 17.0, lset: labels.FromStrings("__name__", "foo_total", "a", "b"), t: int64p(1520879607789), - // TODO(krajorama) e:es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + //es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, ct: int64p(1520872607123), }, { m: "bar", @@ -500,3 +500,42 @@ something_created{a="b"} 1520430002 got := testParse(t, p) requireEntries(t, exp, got) } + +func TestNhcbParserExemplarOnOpenMetricsParser(t *testing.T) { + // The input is taken originally from TestOpenMetricsParse, with additional tests for the NHCBParser. + + input := `# HELP foo Counter with and without labels to certify CT is parsed for both cases +# TYPE foo counter +foo_total 17.0 1520879607.789 # {id="counter-test"} 5 +foo_created 1520872607.123 +# EOF +` + exp := []parsedEntry{ + { + m: "foo", + help: "Counter with and without labels to certify CT is parsed for both cases", + }, { + m: "foo", + typ: model.MetricTypeCounter, + }, { + m: "foo_total", + v: 17, + lset: labels.FromStrings("__name__", "foo_total"), + t: int64p(1520879607789), + //es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + ct: int64p(1520872607123), + // }, { + // m: `foo_total{a="b"}`, + // v: 17.0, + // lset: labels.FromStrings("__name__", "foo_total", "a", "b"), + // t: int64p(1520879607789), + // es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + // ct: int64p(1520872607123), + }, + } + + p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped()) + p = NewNHCBParser(p, false) + got := testParse(t, p) + requireEntries(t, exp, got) +} From 7fccf1e6be7ed983a664ae55fea56cb5c602c24a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Wed, 9 Oct 2024 11:08:08 +0200 Subject: [PATCH 25/48] Disable CT handling and enable exemplar handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 57 ++++++++++---- model/textparse/nhcbparse_test.go | 119 ++++++++++++------------------ scrape/manager.go | 5 ++ 3 files changed, 92 insertions(+), 89 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 9069937f1..8dcd5f65c 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -38,7 +38,6 @@ type NHCBParser struct { // For Series and Histogram. bytes []byte ts *int64 - ct *int64 value float64 h *histogram.Histogram fh *histogram.FloatHistogram @@ -58,16 +57,16 @@ type NHCBParser struct { bytesNHCB []byte hNHCB *histogram.Histogram fhNHCB *histogram.FloatHistogram - ctNHCB *int64 lsetNHCB labels.Labels + exemplars []exemplar.Exemplar metricStringNHCB string // Collates values from the classic histogram series to build // the converted histogram later. tempLsetNHCB labels.Labels - tempCtNHCB *int64 - tempCtNHCBbacking int64 tempNHCB convertnhcb.TempHistogram + tempExemplars []exemplar.Exemplar + tempExemplarCount int isCollationInProgress bool // Remembers the last base histogram metric name (assuming it's @@ -81,6 +80,7 @@ func NewNHCBParser(p Parser, keepClassicHistograms bool) Parser { parser: p, keepClassicHistograms: keepClassicHistograms, tempNHCB: convertnhcb.NewTempHistogram(), + tempExemplars: make([]exemplar.Exemplar, 0, 1), } } @@ -121,14 +121,19 @@ func (p *NHCBParser) Metric(l *labels.Labels) string { } func (p *NHCBParser) Exemplar(ex *exemplar.Exemplar) bool { + if p.justInsertedNHCB { + if len(p.exemplars) == 0 { + return false + } + *ex = p.exemplars[0] + p.exemplars = p.exemplars[1:] + return true + } return p.parser.Exemplar(ex) } func (p *NHCBParser) CreatedTimestamp() *int64 { - if p.justInsertedNHCB { - return p.ctNHCB - } - return p.ct + return nil } func (p *NHCBParser) Next() (Entry, error) { @@ -154,7 +159,6 @@ func (p *NHCBParser) Next() (Entry, error) { case EntrySeries: p.bytes, p.ts, p.value = p.parser.Series() p.metricString = p.parser.Metric(&p.lset) - p.ct = p.parser.CreatedTimestamp() // Check the label set to see if we can continue or need to emit the NHCB. if p.compareLabels() && p.processNHCB() { p.entry = et @@ -167,7 +171,6 @@ func (p *NHCBParser) Next() (Entry, error) { case EntryHistogram: p.bytes, p.ts, p.h, p.fh = p.parser.Histogram() p.metricString = p.parser.Metric(&p.lset) - p.ct = p.parser.CreatedTimestamp() case EntryType: p.bName, p.typ = p.parser.Type() } @@ -255,10 +258,6 @@ func (p *NHCBParser) handleClassicHistogramSeries(lset labels.Labels) bool { if convertnhcb.GetHistogramMetricBaseName(mName) != string(p.bName) { return false } - if p.ct != nil { - p.tempCtNHCBbacking = *p.ct - p.tempCtNHCB = &p.tempCtNHCBbacking - } switch { case strings.HasSuffix(mName, "_bucket") && lset.Has(labels.BucketLabel): le, err := strconv.ParseFloat(lset.Get(labels.BucketLabel), 64) @@ -285,9 +284,36 @@ func (p *NHCBParser) handleClassicHistogramSeries(lset labels.Labels) bool { func (p *NHCBParser) processClassicHistogramSeries(lset labels.Labels, suffix string, updateHist func(*convertnhcb.TempHistogram)) { p.isCollationInProgress = true p.tempLsetNHCB = convertnhcb.GetHistogramMetricBase(lset, suffix) + p.storeExemplars() updateHist(&p.tempNHCB) } +func (p *NHCBParser) storeExemplars() { + for ex := p.nextExemplarPtr(); p.parser.Exemplar(ex); ex = p.nextExemplarPtr() { + p.tempExemplarCount++ + } +} + +func (p *NHCBParser) nextExemplarPtr() *exemplar.Exemplar { + switch { + case p.tempExemplarCount == len(p.tempExemplars)-1: + // Reuse the previously allocated exemplar, it was not filled up. + case len(p.tempExemplars) == cap(p.tempExemplars): + // Let the runtime grow the slice. + p.tempExemplars = append(p.tempExemplars, exemplar.Exemplar{}) + default: + // Take the next element into use. + p.tempExemplars = p.tempExemplars[:len(p.tempExemplars)+1] + } + return &p.tempExemplars[len(p.tempExemplars)-1] +} + +func (p *NHCBParser) swapExemplars() { + p.exemplars = p.tempExemplars[:p.tempExemplarCount] + p.tempExemplars = p.tempExemplars[:0] + p.tempExemplarCount = 0 +} + // processNHCB converts the collated classic histogram series to NHCB and caches the info // to be returned to callers. func (p *NHCBParser) processNHCB() bool { @@ -314,12 +340,11 @@ func (p *NHCBParser) processNHCB() bool { p.hNHCB = nil p.fhNHCB = fh } - p.ctNHCB = p.tempCtNHCB p.metricStringNHCB = p.tempLsetNHCB.Get(labels.MetricName) + strings.ReplaceAll(p.tempLsetNHCB.DropMetricName().String(), ", ", ",") p.bytesNHCB = []byte(p.metricStringNHCB) p.lsetNHCB = p.tempLsetNHCB + p.swapExemplars() p.tempNHCB = convertnhcb.NewTempHistogram() - p.tempCtNHCB = nil p.isCollationInProgress = false p.justInsertedNHCB = true return true diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index 28fb90fcd..2164c79ff 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -207,7 +207,10 @@ foobar{quantile="0.99"} 150.1` // Custom values are empty as we do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "hhh"), - // TODO(krajorama) e: []*exemplar.Exemplar{{Labels: labels.FromStrings("id", "histogram-bucket-test"), Value: 4}}, + es: []exemplar.Exemplar{ + {Labels: labels.FromStrings("id", "histogram-bucket-test"), Value: 4}, + {Labels: labels.FromStrings("id", "histogram-count-test"), Value: 4}, + }, }, { m: "ggh", typ: model.MetricTypeGaugeHistogram, @@ -228,7 +231,7 @@ foobar{quantile="0.99"} 150.1` m: `smr_seconds_count`, v: 2, lset: labels.FromStrings("__name__", "smr_seconds_count"), - // TODO(krajorama) e:es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "summary-count-test"), Value: 1, HasTs: true, Ts: 123321}}, + es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "summary-count-test"), Value: 1, HasTs: true, Ts: 123321}}, }, { m: `smr_seconds_sum`, v: 42, @@ -282,15 +285,15 @@ foobar{quantile="0.99"} 150.1` v: 17, lset: labels.FromStrings("__name__", "foo_total"), t: int64p(1520879607789), - //es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, - ct: int64p(1520872607123), + es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + // TODO(krajorama): ct: int64p(1520872607123), }, { m: `foo_total{a="b"}`, v: 17.0, lset: labels.FromStrings("__name__", "foo_total", "a", "b"), t: int64p(1520879607789), - //es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, - ct: int64p(1520872607123), + es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, + // TODO(krajorama): ct: int64p(1520872607123), }, { m: "bar", help: "Summary with CT at the end, making sure we find CT even if it's multiple lines a far", @@ -301,22 +304,22 @@ foobar{quantile="0.99"} 150.1` m: "bar_count", v: 17.0, lset: labels.FromStrings("__name__", "bar_count"), - ct: int64p(1520872608124), + // TODO(krajorama): ct: int64p(1520872608124), }, { m: "bar_sum", v: 324789.3, lset: labels.FromStrings("__name__", "bar_sum"), - ct: int64p(1520872608124), + // TODO(krajorama): ct: int64p(1520872608124), }, { m: `bar{quantile="0.95"}`, v: 123.7, lset: labels.FromStrings("__name__", "bar", "quantile", "0.95"), - ct: int64p(1520872608124), + // TODO(krajorama): ct: int64p(1520872608124), }, { m: `bar{quantile="0.99"}`, v: 150.0, lset: labels.FromStrings("__name__", "bar", "quantile", "0.99"), - ct: int64p(1520872608124), + // TODO(krajorama): ct: int64p(1520872608124), }, { m: "baz", help: "Histogram with the same objective as above's summary", @@ -334,7 +337,7 @@ foobar{quantile="0.99"} 150.1` CustomValues: []float64{0.0}, // We do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "baz"), - ct: int64p(1520872609125), + // TODO(krajorama): ct: int64p(1520872609125), }, { m: "fizz_created", help: "Gauge which shouldn't be parsed as CT", @@ -362,7 +365,7 @@ foobar{quantile="0.99"} 150.1` CustomValues: []float64{0.0}, // We do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "something"), - ct: int64p(1520430001000), + // TODO(krajorama): ct: int64p(1520430001000), }, { m: `something{a="b"}`, shs: &histogram.Histogram{ @@ -374,7 +377,7 @@ foobar{quantile="0.99"} 150.1` CustomValues: []float64{0.0}, // We do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "something", "a", "b"), - ct: int64p(1520430002000), + // TODO(krajorama): ct: int64p(1520430002000), }, { m: "yum", help: "Summary with _created between sum and quantiles", @@ -385,22 +388,22 @@ foobar{quantile="0.99"} 150.1` m: `yum_count`, v: 20, lset: labels.FromStrings("__name__", "yum_count"), - ct: int64p(1520430003000), + // TODO(krajorama): ct: int64p(1520430003000), }, { m: `yum_sum`, v: 324789.5, lset: labels.FromStrings("__name__", "yum_sum"), - ct: int64p(1520430003000), + // TODO(krajorama): ct: int64p(1520430003000), }, { m: `yum{quantile="0.95"}`, v: 123.7, lset: labels.FromStrings("__name__", "yum", "quantile", "0.95"), - ct: int64p(1520430003000), + // TODO(krajorama): ct: int64p(1520430003000), }, { m: `yum{quantile="0.99"}`, v: 150.0, lset: labels.FromStrings("__name__", "yum", "quantile", "0.99"), - ct: int64p(1520430003000), + // TODO(krajorama): ct: int64p(1520430003000), }, { m: "foobar", help: "Summary with _created as the first line", @@ -411,22 +414,22 @@ foobar{quantile="0.99"} 150.1` m: `foobar_count`, v: 21, lset: labels.FromStrings("__name__", "foobar_count"), - ct: int64p(1520430004000), + // TODO(krajorama): ct: int64p(1520430004000), }, { m: `foobar_sum`, v: 324789.6, lset: labels.FromStrings("__name__", "foobar_sum"), - ct: int64p(1520430004000), + // TODO(krajorama): ct: int64p(1520430004000), }, { m: `foobar{quantile="0.95"}`, v: 123.8, lset: labels.FromStrings("__name__", "foobar", "quantile", "0.95"), - ct: int64p(1520430004000), + // TODO(krajorama): ct: int64p(1520430004000), }, { m: `foobar{quantile="0.99"}`, v: 150.1, lset: labels.FromStrings("__name__", "foobar", "quantile", "0.99"), - ct: int64p(1520430004000), + // TODO(krajorama): ct: int64p(1520430004000), }, { m: "metric", help: "foo\x00bar", @@ -450,14 +453,14 @@ func TestNhcbParserMultiHOnOpenMetricsParser(t *testing.T) { # TYPE something histogram something_count 18 something_sum 324789.4 -something_created 1520430001 -something_bucket{le="0.0"} 1 -something_bucket{le="+Inf"} 18 +something_bucket{le="0.0"} 1 # {id="something-test"} -2.0 +something_bucket{le="1.0"} 16 # {id="something-test"} 0.5 +something_bucket{le="+Inf"} 18 # {id="something-test"} 8 something_count{a="b"} 9 something_sum{a="b"} 42123.0 -something_bucket{a="b",le="0.0"} 8 -something_bucket{a="b",le="+Inf"} 9 -something_created{a="b"} 1520430002 +something_bucket{a="b",le="0.0"} 8 # {id="something-test"} 0.0 123.321 +something_bucket{a="b",le="1.0"} 8 +something_bucket{a="b",le="+Inf"} 9 # {id="something-test"} 2e100 123.000 # EOF ` @@ -474,63 +477,33 @@ something_created{a="b"} 1520430002 Schema: -53, // Custom buckets. Count: 18, Sum: 324789.4, - PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}}, - PositiveBuckets: []int64{1, 16}, - CustomValues: []float64{0.0}, // We do not store the +Inf boundary. + PositiveSpans: []histogram.Span{{Offset: 0, Length: 3}}, + PositiveBuckets: []int64{1, 14, -13}, + CustomValues: []float64{0.0, 1.0}, // We do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "something"), - ct: int64p(1520430001000), + es: []exemplar.Exemplar{ + {Labels: labels.FromStrings("id", "something-test"), Value: -2.0}, + {Labels: labels.FromStrings("id", "something-test"), Value: 0.5}, + {Labels: labels.FromStrings("id", "something-test"), Value: 8.0}, + }, + // TODO(krajorama): ct: int64p(1520430001000), }, { m: `something{a="b"}`, shs: &histogram.Histogram{ Schema: -53, // Custom buckets. Count: 9, Sum: 42123.0, - PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}}, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}, {Offset: 1, Length: 1}}, PositiveBuckets: []int64{8, -7}, - CustomValues: []float64{0.0}, // We do not store the +Inf boundary. + CustomValues: []float64{0.0, 1.0}, // We do not store the +Inf boundary. }, lset: labels.FromStrings("__name__", "something", "a", "b"), - ct: int64p(1520430002000), - }, - } - - p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped()) - p = NewNHCBParser(p, false) - got := testParse(t, p) - requireEntries(t, exp, got) -} - -func TestNhcbParserExemplarOnOpenMetricsParser(t *testing.T) { - // The input is taken originally from TestOpenMetricsParse, with additional tests for the NHCBParser. - - input := `# HELP foo Counter with and without labels to certify CT is parsed for both cases -# TYPE foo counter -foo_total 17.0 1520879607.789 # {id="counter-test"} 5 -foo_created 1520872607.123 -# EOF -` - exp := []parsedEntry{ - { - m: "foo", - help: "Counter with and without labels to certify CT is parsed for both cases", - }, { - m: "foo", - typ: model.MetricTypeCounter, - }, { - m: "foo_total", - v: 17, - lset: labels.FromStrings("__name__", "foo_total"), - t: int64p(1520879607789), - //es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, - ct: int64p(1520872607123), - // }, { - // m: `foo_total{a="b"}`, - // v: 17.0, - // lset: labels.FromStrings("__name__", "foo_total", "a", "b"), - // t: int64p(1520879607789), - // es: []exemplar.Exemplar{{Labels: labels.FromStrings("id", "counter-test"), Value: 5}}, - // ct: int64p(1520872607123), + es: []exemplar.Exemplar{ + {Labels: labels.FromStrings("id", "something-test"), Value: 0.0, HasTs: true, Ts: 123321}, + {Labels: labels.FromStrings("id", "something-test"), Value: 2e100, HasTs: true, Ts: 123000}, + }, + // TODO(krajorama): ct: int64p(1520430002000), }, } diff --git a/scrape/manager.go b/scrape/manager.go index cbb881028..83001af76 100644 --- a/scrape/manager.go +++ b/scrape/manager.go @@ -178,6 +178,11 @@ func (m *Manager) reload() { level.Error(m.logger).Log("msg", "error reloading target set", "err", "invalid config id:"+setName) continue } + if scrapeConfig.ConvertClassicHistograms && m.opts.EnableCreatedTimestampZeroIngestion { + // TODO(krajorama): lift this limitation + level.Error(m.logger).Log("msg", "error reloading target set", "err", "cannot convert classic histograms to native histograms with custom buckets and ingest created timestamp zero samples at the same time") + continue + } m.metrics.targetScrapePools.Inc() sp, err := newScrapePool(scrapeConfig, m.append, m.offsetSeed, log.With(m.logger, "scrape_pool", setName), m.buffers, m.opts, m.metrics) if err != nil { From fbbf10baadae5685580285b34cb75024cfde1ac5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Wed, 9 Oct 2024 11:24:24 +0200 Subject: [PATCH 26/48] Use the proper way to iterate labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 42 +++++++++++++++++------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 8dcd5f65c..f78d95e7e 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -28,6 +28,8 @@ import ( "github.com/prometheus/prometheus/util/convertnhcb" ) +var labelsMismatchError = errors.New("labels mismatch") + type NHCBParser struct { // The parser we're wrapping. parser Parser @@ -198,50 +200,46 @@ func (p *NHCBParser) compareLabels() bool { } // Compare the labels. - for _, l := range p.lset { - if l.Name == labels.MetricName { + err := p.lset.Validate(func(l labels.Label) error { + switch { + case l.Name == labels.BucketLabel: + case l.Name == labels.MetricName: baseName := convertnhcb.GetHistogramMetricBaseName(l.Value) if baseName != p.lastBaseHistLabels.Get(labels.MetricName) { + // Different metric name. if p.typ == model.MetricTypeHistogram { p.storeBaseLabels() } else { p.lastBaseHistLabels = labels.EmptyLabels() } - return true + return labelsMismatchError } - continue - } - if l.Name == labels.BucketLabel { - // Ignore. - continue - } - if l.Value != p.lastBaseHistLabels.Get(l.Name) { + case l.Value != p.lastBaseHistLabels.Get(l.Name): // Different label value. if p.typ == model.MetricTypeHistogram { p.storeBaseLabels() } else { p.lastBaseHistLabels = labels.EmptyLabels() } - return true + return labelsMismatchError } - } - return false + return nil + }) + return err == labelsMismatchError } // Save the label set of the classic histogram without suffix and bucket `le` label. func (p *NHCBParser) storeBaseLabels() { builder := labels.Builder{} - for _, l := range p.lset { - if l.Name == labels.MetricName { + p.lset.Range(func(l labels.Label) { + switch { + case l.Name == labels.BucketLabel: + case l.Name == labels.MetricName: builder.Set(l.Name, convertnhcb.GetHistogramMetricBaseName(l.Value)) - continue + default: + builder.Set(l.Name, l.Value) } - if l.Name == labels.BucketLabel { - // Ignore. - continue - } - builder.Set(l.Name, l.Value) - } + }) p.lastBaseHistLabels = builder.Labels() } From f3c8ed2947f66b5c4e2441939fb5a0487688d64c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Wed, 9 Oct 2024 11:45:31 +0200 Subject: [PATCH 27/48] minor fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix lint errors. No need to pre-allocate exemplars. Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index f78d95e7e..bf69ba600 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -28,7 +28,7 @@ import ( "github.com/prometheus/prometheus/util/convertnhcb" ) -var labelsMismatchError = errors.New("labels mismatch") +var errLabelsMismatch = errors.New("labels mismatch") type NHCBParser struct { // The parser we're wrapping. @@ -82,7 +82,6 @@ func NewNHCBParser(p Parser, keepClassicHistograms bool) Parser { parser: p, keepClassicHistograms: keepClassicHistograms, tempNHCB: convertnhcb.NewTempHistogram(), - tempExemplars: make([]exemplar.Exemplar, 0, 1), } } @@ -212,7 +211,7 @@ func (p *NHCBParser) compareLabels() bool { } else { p.lastBaseHistLabels = labels.EmptyLabels() } - return labelsMismatchError + return errLabelsMismatch } case l.Value != p.lastBaseHistLabels.Get(l.Name): // Different label value. @@ -221,11 +220,11 @@ func (p *NHCBParser) compareLabels() bool { } else { p.lastBaseHistLabels = labels.EmptyLabels() } - return labelsMismatchError + return errLabelsMismatch } return nil }) - return err == labelsMismatchError + return errors.Is(err, errLabelsMismatch) } // Save the label set of the classic histogram without suffix and bucket `le` label. From 4e911a3a057a4b7890da4b31b85c4d35426341b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Wed, 9 Oct 2024 11:54:49 +0200 Subject: [PATCH 28/48] Follow-up merge from main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- scrape/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrape/manager.go b/scrape/manager.go index 5d891e9c2..19fbe30b1 100644 --- a/scrape/manager.go +++ b/scrape/manager.go @@ -180,7 +180,7 @@ func (m *Manager) reload() { } if scrapeConfig.ConvertClassicHistograms && m.opts.EnableCreatedTimestampZeroIngestion { // TODO(krajorama): lift this limitation - level.Error(m.logger).Log("msg", "error reloading target set", "err", "cannot convert classic histograms to native histograms with custom buckets and ingest created timestamp zero samples at the same time") + m.logger.Error("msg", "error reloading target set", "err", "cannot convert classic histograms to native histograms with custom buckets and ingest created timestamp zero samples at the same time") continue } m.metrics.targetScrapePools.Inc() From 6b15791ec6f752e0c787ebf28ad5d4ce88b3b226 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Wed, 9 Oct 2024 11:56:54 +0200 Subject: [PATCH 29/48] Follow the follow-up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- scrape/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrape/manager.go b/scrape/manager.go index 19fbe30b1..d0a3591b5 100644 --- a/scrape/manager.go +++ b/scrape/manager.go @@ -180,7 +180,7 @@ func (m *Manager) reload() { } if scrapeConfig.ConvertClassicHistograms && m.opts.EnableCreatedTimestampZeroIngestion { // TODO(krajorama): lift this limitation - m.logger.Error("msg", "error reloading target set", "err", "cannot convert classic histograms to native histograms with custom buckets and ingest created timestamp zero samples at the same time") + m.logger.Error("error reloading target set", "err", "cannot convert classic histograms to native histograms with custom buckets and ingest created timestamp zero samples at the same time") continue } m.metrics.targetScrapePools.Inc() From 8dfa733596a3c7f9400a2c41fcfd72818925c8d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Wed, 9 Oct 2024 12:29:59 +0200 Subject: [PATCH 30/48] Fix labels handling with dedupelabels tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use scratch builder. Use hash compare instead of compare by label. Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 59 +++++++++++++------------------ model/textparse/nhcbparse_test.go | 5 +-- scrape/scrape.go | 2 +- 3 files changed, 29 insertions(+), 37 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index bf69ba600..8860645dd 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -28,14 +28,15 @@ import ( "github.com/prometheus/prometheus/util/convertnhcb" ) -var errLabelsMismatch = errors.New("labels mismatch") - type NHCBParser struct { // The parser we're wrapping. parser Parser // Option to keep classic histograms along with converted histograms. keepClassicHistograms bool + // Labels builder. + builder labels.ScratchBuilder + // Caches the values from the underlying parser. // For Series and Histogram. bytes []byte @@ -77,10 +78,11 @@ type NHCBParser struct { lastBaseHistLabels labels.Labels } -func NewNHCBParser(p Parser, keepClassicHistograms bool) Parser { +func NewNHCBParser(p Parser, st *labels.SymbolTable, keepClassicHistograms bool) Parser { return &NHCBParser{ parser: p, keepClassicHistograms: keepClassicHistograms, + builder: labels.NewScratchBuilderWithSymbolTable(st, 16), tempNHCB: convertnhcb.NewTempHistogram(), } } @@ -198,48 +200,37 @@ func (p *NHCBParser) compareLabels() bool { return true } - // Compare the labels. - err := p.lset.Validate(func(l labels.Label) error { - switch { - case l.Name == labels.BucketLabel: - case l.Name == labels.MetricName: - baseName := convertnhcb.GetHistogramMetricBaseName(l.Value) - if baseName != p.lastBaseHistLabels.Get(labels.MetricName) { - // Different metric name. - if p.typ == model.MetricTypeHistogram { - p.storeBaseLabels() - } else { - p.lastBaseHistLabels = labels.EmptyLabels() - } - return errLabelsMismatch - } - case l.Value != p.lastBaseHistLabels.Get(l.Name): - // Different label value. - if p.typ == model.MetricTypeHistogram { - p.storeBaseLabels() - } else { - p.lastBaseHistLabels = labels.EmptyLabels() - } - return errLabelsMismatch - } - return nil - }) - return errors.Is(err, errLabelsMismatch) + if p.lastBaseHistLabels.Get(labels.MetricName) != convertnhcb.GetHistogramMetricBaseName(p.lset.Get(labels.MetricName)) { + // Different metric name. + p.storeBaseLabels() + return true + } + var buf []byte + lastHash, _ := p.lastBaseHistLabels.HashWithoutLabels(buf, labels.BucketLabel) + nextHash, _ := p.lset.HashWithoutLabels(buf, labels.BucketLabel) + if lastHash != nextHash { + // Different label values. + p.storeBaseLabels() + return true + } + + return false } // Save the label set of the classic histogram without suffix and bucket `le` label. func (p *NHCBParser) storeBaseLabels() { - builder := labels.Builder{} + p.builder.Reset() p.lset.Range(func(l labels.Label) { switch { case l.Name == labels.BucketLabel: case l.Name == labels.MetricName: - builder.Set(l.Name, convertnhcb.GetHistogramMetricBaseName(l.Value)) + p.builder.Add(l.Name, convertnhcb.GetHistogramMetricBaseName(l.Value)) default: - builder.Set(l.Name, l.Value) + p.builder.Add(l.Name, l.Value) } }) - p.lastBaseHistLabels = builder.Labels() + // Note: we don't sort the labels as changing the name label value doesn't affect sorting. + p.lastBaseHistLabels = p.builder.Labels() } // handleClassicHistogramSeries collates the classic histogram series to be converted to NHCB diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index 2164c79ff..3c828ca42 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -441,7 +441,7 @@ foobar{quantile="0.99"} 150.1` } p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped()) - p = NewNHCBParser(p, false) + p = NewNHCBParser(p, labels.NewSymbolTable(), false) got := testParse(t, p) requireEntries(t, exp, got) } @@ -508,7 +508,8 @@ something_bucket{a="b",le="+Inf"} 9 # {id="something-test"} 2e100 123.000 } p := NewOpenMetricsParser([]byte(input), labels.NewSymbolTable(), WithOMParserCTSeriesSkipped()) - p = NewNHCBParser(p, false) + + p = NewNHCBParser(p, labels.NewSymbolTable(), false) got := testParse(t, p) requireEntries(t, exp, got) } diff --git a/scrape/scrape.go b/scrape/scrape.go index 4500faff3..a305fdd4e 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -1546,7 +1546,7 @@ type appendErrors struct { func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string, ts time.Time) (total, added, seriesAdded int, err error) { p, err := textparse.New(b, contentType, sl.scrapeClassicHistograms, sl.enableCTZeroIngestion, sl.symbolTable) if sl.convertClassicHistograms { - p = textparse.NewNHCBParser(p, sl.scrapeClassicHistograms) + p = textparse.NewNHCBParser(p, sl.symbolTable, sl.scrapeClassicHistograms) } if err != nil { sl.l.Debug( From 530e9514b7f6f8f1ab828db99187a778d461d0b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Wed, 9 Oct 2024 13:08:54 +0200 Subject: [PATCH 31/48] Fix case of keeping classic series MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make it more obvious that the code in if had side effect. Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 8860645dd..87d2b2bfc 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -143,7 +143,9 @@ func (p *NHCBParser) Next() (Entry, error) { if p.justInsertedNHCB { p.justInsertedNHCB = false if p.entry == EntrySeries { - if !p.keepClassicHistograms && p.handleClassicHistogramSeries(p.lset) { + isNHCB := p.handleClassicHistogramSeries(p.lset) + if isNHCB && !p.keepClassicHistograms { + // Do not return the classic histogram series if it was converted to NHCB and we are not keeping classic histograms. return p.Next() } } @@ -167,7 +169,9 @@ func (p *NHCBParser) Next() (Entry, error) { p.entry = et return EntryHistogram, nil } - if !p.keepClassicHistograms && p.handleClassicHistogramSeries(p.lset) { + isNHCB := p.handleClassicHistogramSeries(p.lset) + if isNHCB && !p.keepClassicHistograms { + // Do not return the classic histogram series if it was converted to NHCB and we are not keeping classic histograms. return p.Next() } return et, err @@ -206,7 +210,7 @@ func (p *NHCBParser) compareLabels() bool { return true } var buf []byte - lastHash, _ := p.lastBaseHistLabels.HashWithoutLabels(buf, labels.BucketLabel) + lastHash, _ := p.lastBaseHistLabels.HashWithoutLabels(buf) // We removed the bucket label in storeBaseLabels. nextHash, _ := p.lset.HashWithoutLabels(buf, labels.BucketLabel) if lastHash != nextHash { // Different label values. From 14f92319d9c6a96de005fa36f73ca30d01eb9f7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Wed, 9 Oct 2024 15:16:27 +0200 Subject: [PATCH 32/48] Add basic benchmark cases for NHCB over OM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/benchmark_test.go | 10 +++++ model/textparse/testdata/omcounterdata.txt | 9 ++++ model/textparse/testdata/omhistogramdata.txt | 45 ++++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 model/textparse/testdata/omcounterdata.txt create mode 100644 model/textparse/testdata/omhistogramdata.txt diff --git a/model/textparse/benchmark_test.go b/model/textparse/benchmark_test.go index 3b8b8f305..bc9c2d1db 100644 --- a/model/textparse/benchmark_test.go +++ b/model/textparse/benchmark_test.go @@ -40,6 +40,10 @@ var newTestParserFns = map[string]newParser{ "omtext": func(b []byte, st *labels.SymbolTable) Parser { return NewOpenMetricsParser(b, st, WithOMParserCTSeriesSkipped()) }, + "nhcb_over_omtext": func(b []byte, st *labels.SymbolTable) Parser { + p := NewOpenMetricsParser(b, st, WithOMParserCTSeriesSkipped()) + return NewNHCBParser(p, st, false) + }, } // BenchmarkParse benchmarks parsing, mimicking how scrape/scrape.go#append use it. @@ -78,6 +82,12 @@ func BenchmarkParse(b *testing.B) { // We don't pass compareToExpfmtFormat: expfmt.TypeOpenMetrics as expfmt does not support OM exemplars, see https://github.com/prometheus/common/issues/703. {dataFile: "omtestdata.txt", parser: "omtext"}, {dataFile: "promtestdata.txt", parser: "omtext"}, // Compare how omtext parser deals with Prometheus text format vs promtext. + + // NHCB. + {dataFile: "omhistogramdata.txt", parser: "omtext"}, // Measure OM parser baseline for histograms. + {dataFile: "omhistogramdata.txt", parser: "nhcb_over_omtext"}, // Measure NHCB over OM parser. + {dataFile: "omcounterdata.txt", parser: "omtext"}, // Measure OM parser baseline for counters. + {dataFile: "omcounterdata.txt", parser: "nhcb_over_omtext"}, // Measure NHCB over OM parser. } { var buf []byte dataCase := bcase.dataFile diff --git a/model/textparse/testdata/omcounterdata.txt b/model/textparse/testdata/omcounterdata.txt new file mode 100644 index 000000000..15459c018 --- /dev/null +++ b/model/textparse/testdata/omcounterdata.txt @@ -0,0 +1,9 @@ +# HELP rpc_requests Total number of RPC requests received. +# TYPE rpc_requests counter +rpc_requests_total{service="exponential"} 22.0 +rpc_requests_created{service="exponential"} 1.726839813016893e+09 +rpc_requests_total{service="normal"} 15.0 +rpc_requests_created{service="normal"} 1.726839813016717e+09 +rpc_requests_total{service="uniform"} 11.0 +rpc_requests_created{service="uniform"} 1.7268398130168471e+09 +# EOF diff --git a/model/textparse/testdata/omhistogramdata.txt b/model/textparse/testdata/omhistogramdata.txt new file mode 100644 index 000000000..187616835 --- /dev/null +++ b/model/textparse/testdata/omhistogramdata.txt @@ -0,0 +1,45 @@ +# HELP golang_manual_histogram_seconds This is a histogram with manually selected parameters +# TYPE golang_manual_histogram_seconds histogram +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="0.005"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="0.01"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="0.025"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="0.05"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="0.1"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="0.25"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="0.5"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="1.0"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="2.5"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="5.0"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="10.0"} 1 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5001",le="+Inf"} 1 +golang_manual_histogram_seconds_sum{address="0.0.0.0",generation="20",port="5001"} 10.0 +golang_manual_histogram_seconds_count{address="0.0.0.0",generation="20",port="5001"} 1 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="0.005"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="0.01"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="0.025"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="0.05"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="0.1"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="0.25"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="0.5"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="1.0"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="2.5"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="5.0"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="10.0"} 1 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5002",le="+Inf"} 1 +golang_manual_histogram_seconds_sum{address="0.0.0.0",generation="20",port="5002"} 10.0 +golang_manual_histogram_seconds_count{address="0.0.0.0",generation="20",port="5002"} 1 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="0.005"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="0.01"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="0.025"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="0.05"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="0.1"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="0.25"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="0.5"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="1.0"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="2.5"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="5.0"} 0 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="10.0"} 1 +golang_manual_histogram_seconds_bucket{address="0.0.0.0",generation="20",port="5003",le="+Inf"} 1 +golang_manual_histogram_seconds_sum{address="0.0.0.0",generation="20",port="5003"} 10.0 +golang_manual_histogram_seconds_count{address="0.0.0.0",generation="20",port="5003"} 1 +# EOF \ No newline at end of file From 9b5d7287bb99490586a9b63ae6d268484d644c57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Wed, 9 Oct 2024 15:16:46 +0200 Subject: [PATCH 33/48] Use labels hash to determine change in metric like CT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 87d2b2bfc..eb421aa79 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -75,7 +75,9 @@ type NHCBParser struct { // Remembers the last base histogram metric name (assuming it's // a classic histogram) so we can tell if the next float series // is part of the same classic histogram. - lastBaseHistLabels labels.Labels + lastHistogramName string + lastHistogramLabelsHash uint64 + hBuffer []byte } func NewNHCBParser(p Parser, st *labels.SymbolTable, keepClassicHistograms bool) Parser { @@ -192,7 +194,7 @@ func (p *NHCBParser) Next() (Entry, error) { // Update the stored labels if the labels have changed. func (p *NHCBParser) compareLabels() bool { // Collection not in progress. - if p.lastBaseHistLabels.IsEmpty() { + if p.lastHistogramName == "" { if p.typ == model.MetricTypeHistogram { p.storeBaseLabels() } @@ -200,19 +202,17 @@ func (p *NHCBParser) compareLabels() bool { } if p.typ != model.MetricTypeHistogram { // Different metric type, emit the NHCB. - p.lastBaseHistLabels = labels.EmptyLabels() + p.lastHistogramName = "" return true } - if p.lastBaseHistLabels.Get(labels.MetricName) != convertnhcb.GetHistogramMetricBaseName(p.lset.Get(labels.MetricName)) { + if p.lastHistogramName != convertnhcb.GetHistogramMetricBaseName(p.lset.Get(labels.MetricName)) { // Different metric name. p.storeBaseLabels() return true } - var buf []byte - lastHash, _ := p.lastBaseHistLabels.HashWithoutLabels(buf) // We removed the bucket label in storeBaseLabels. - nextHash, _ := p.lset.HashWithoutLabels(buf, labels.BucketLabel) - if lastHash != nextHash { + nextHash, _ := p.lset.HashWithoutLabels(p.hBuffer, labels.BucketLabel) + if p.lastHistogramLabelsHash != nextHash { // Different label values. p.storeBaseLabels() return true @@ -223,18 +223,8 @@ func (p *NHCBParser) compareLabels() bool { // Save the label set of the classic histogram without suffix and bucket `le` label. func (p *NHCBParser) storeBaseLabels() { - p.builder.Reset() - p.lset.Range(func(l labels.Label) { - switch { - case l.Name == labels.BucketLabel: - case l.Name == labels.MetricName: - p.builder.Add(l.Name, convertnhcb.GetHistogramMetricBaseName(l.Value)) - default: - p.builder.Add(l.Name, l.Value) - } - }) - // Note: we don't sort the labels as changing the name label value doesn't affect sorting. - p.lastBaseHistLabels = p.builder.Labels() + p.lastHistogramName = convertnhcb.GetHistogramMetricBaseName(p.lset.Get(labels.MetricName)) + p.lastHistogramLabelsHash, _ = p.lset.HashWithoutLabels(p.hBuffer, labels.BucketLabel) } // handleClassicHistogramSeries collates the classic histogram series to be converted to NHCB From 0a40a09da5afe3dd68661e327b30fc274f7140a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 14 Oct 2024 11:09:03 +0200 Subject: [PATCH 34/48] Use const instead of -53 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index 3c828ca42..25f5732ce 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -178,7 +178,7 @@ foobar{quantile="0.99"} 150.1` }, { m: `hh{}`, shs: &histogram.Histogram{ - Schema: -53, // Custom buckets. + Schema: histogram.CustomBucketsSchema, Count: 1, Sum: 0.0, PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}}, @@ -199,7 +199,7 @@ foobar{quantile="0.99"} 150.1` }, { m: `hhh{}`, shs: &histogram.Histogram{ - Schema: -53, // Custom buckets. + Schema: histogram.CustomBucketsSchema, Count: 1, Sum: 0.0, PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}}, @@ -329,7 +329,7 @@ foobar{quantile="0.99"} 150.1` }, { m: `baz{}`, shs: &histogram.Histogram{ - Schema: -53, // Custom buckets. + Schema: histogram.CustomBucketsSchema, Count: 17, Sum: 324789.3, PositiveSpans: []histogram.Span{{Offset: 1, Length: 1}}, // The first bucket has 0 count so we don't store it and Offset is 1. @@ -357,7 +357,7 @@ foobar{quantile="0.99"} 150.1` }, { m: `something{}`, shs: &histogram.Histogram{ - Schema: -53, // Custom buckets. + Schema: histogram.CustomBucketsSchema, Count: 18, Sum: 324789.4, PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}}, @@ -369,7 +369,7 @@ foobar{quantile="0.99"} 150.1` }, { m: `something{a="b"}`, shs: &histogram.Histogram{ - Schema: -53, // Custom buckets. + Schema: histogram.CustomBucketsSchema, Count: 9, Sum: 42123.0, PositiveSpans: []histogram.Span{{Offset: 0, Length: 2}}, @@ -474,7 +474,7 @@ something_bucket{a="b",le="+Inf"} 9 # {id="something-test"} 2e100 123.000 }, { m: `something{}`, shs: &histogram.Histogram{ - Schema: -53, // Custom buckets. + Schema: histogram.CustomBucketsSchema, Count: 18, Sum: 324789.4, PositiveSpans: []histogram.Span{{Offset: 0, Length: 3}}, @@ -491,7 +491,7 @@ something_bucket{a="b",le="+Inf"} 9 # {id="something-test"} 2e100 123.000 }, { m: `something{a="b"}`, shs: &histogram.Histogram{ - Schema: -53, // Custom buckets. + Schema: histogram.CustomBucketsSchema, Count: 9, Sum: 42123.0, PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}, {Offset: 1, Length: 1}}, From a1700aab3a9042efabcb7b64a302a9c0c4e45632 Mon Sep 17 00:00:00 2001 From: George Krajcsovits Date: Mon, 14 Oct 2024 11:13:58 +0200 Subject: [PATCH 35/48] Apply suggestions from code review Co-authored-by: Bartlomiej Plotka Signed-off-by: George Krajcsovits --- config/config.go | 2 +- model/textparse/benchmark_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/config.go b/config/config.go index 3446c29eb..fff50e8e6 100644 --- a/config/config.go +++ b/config/config.go @@ -635,7 +635,7 @@ type ScrapeConfig struct { ScrapeProtocols []ScrapeProtocol `yaml:"scrape_protocols,omitempty"` // Whether to scrape a classic histogram that is also exposed as a native histogram. ScrapeClassicHistograms bool `yaml:"scrape_classic_histograms,omitempty"` - // Whether to convert a scraped classic histogram into a native histogram with custom buckets. + // Whether to convert all scraped classic histograms into a native histogram with custom buckets. ConvertClassicHistograms bool `yaml:"convert_classic_histograms,omitempty"` // File to which scrape failures are logged. ScrapeFailureLogFile string `yaml:"scrape_failure_log_file,omitempty"` diff --git a/model/textparse/benchmark_test.go b/model/textparse/benchmark_test.go index bc9c2d1db..98aadb0ed 100644 --- a/model/textparse/benchmark_test.go +++ b/model/textparse/benchmark_test.go @@ -40,7 +40,7 @@ var newTestParserFns = map[string]newParser{ "omtext": func(b []byte, st *labels.SymbolTable) Parser { return NewOpenMetricsParser(b, st, WithOMParserCTSeriesSkipped()) }, - "nhcb_over_omtext": func(b []byte, st *labels.SymbolTable) Parser { + "omtext_with_nhcb": func(b []byte, st *labels.SymbolTable) Parser { p := NewOpenMetricsParser(b, st, WithOMParserCTSeriesSkipped()) return NewNHCBParser(p, st, false) }, From d930648afb3a7e2aaafed925fff2dc80aa48f5c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 14 Oct 2024 14:35:11 +0200 Subject: [PATCH 36/48] Add doc string for NHCBParser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Very high level since we'll do a lot of optimizations so not worth going into details at this time. Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/model/textparse/nhcbparse.go b/model/textparse/nhcbparse.go index 283fb8b73..a63d0d93e 100644 --- a/model/textparse/nhcbparse.go +++ b/model/textparse/nhcbparse.go @@ -28,6 +28,17 @@ import ( "github.com/prometheus/prometheus/util/convertnhcb" ) +// The NHCBParser wraps a Parser and converts classic histograms to native +// histograms with custom buckets. +// +// Since Parser interface is line based, this parser needs to keep track +// of the last classic histogram series it saw to collate them into a +// single native histogram. +// +// Note: +// - Only series that have the histogram metadata type are considered for +// conversion. +// - The classic series are also returned if keepClassicHistograms is true. type NHCBParser struct { // The parser we're wrapping. parser Parser From 5ee698de2c2525fc816f04a3ca0996295da67cb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Thu, 17 Oct 2024 12:55:45 +0200 Subject: [PATCH 37/48] Apply review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse_test.go | 4 ++-- scrape/manager.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index 25f5732ce..a52e8637e 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -23,7 +23,7 @@ import ( "github.com/prometheus/prometheus/model/labels" ) -func TestNhcbParserOnOpenMetricsParser(t *testing.T) { +func TestNHCBParserOnOMParser(t *testing.T) { // The input is taken originally from TestOpenMetricsParse, with additional tests for the NHCBParser. input := `# HELP go_gc_duration_seconds A summary of the GC invocation durations. @@ -446,7 +446,7 @@ foobar{quantile="0.99"} 150.1` requireEntries(t, exp, got) } -func TestNhcbParserMultiHOnOpenMetricsParser(t *testing.T) { +func TestNHCBParserOMParser_MultipleHistograms(t *testing.T) { // The input is taken originally from TestOpenMetricsParse, with additional tests for the NHCBParser. input := `# HELP something Histogram with _created between buckets and summary diff --git a/scrape/manager.go b/scrape/manager.go index d0a3591b5..9791db0e8 100644 --- a/scrape/manager.go +++ b/scrape/manager.go @@ -179,8 +179,8 @@ func (m *Manager) reload() { continue } if scrapeConfig.ConvertClassicHistograms && m.opts.EnableCreatedTimestampZeroIngestion { - // TODO(krajorama): lift this limitation - m.logger.Error("error reloading target set", "err", "cannot convert classic histograms to native histograms with custom buckets and ingest created timestamp zero samples at the same time") + // TODO(krajorama): fix https://github.com/prometheus/prometheus/issues/15137 + m.logger.Error("error reloading target set", "err", "cannot convert classic histograms to native histograms with custom buckets and ingest created timestamp zero samples at the same time due to https://github.com/prometheus/prometheus/issues/15137") continue } m.metrics.targetScrapePools.Inc() From 482bb453c6f61c8f4ed4f05c9a1cbdc60cc1f3c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 21 Oct 2024 11:03:07 +0200 Subject: [PATCH 38/48] Followup to #15164 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update test cases Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index a52e8637e..7cff21712 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -131,7 +131,7 @@ foobar{quantile="0.99"} 150.1` }, { m: `go_gc_duration_seconds{quantile="0"}`, v: 4.9351e-05, - lset: labels.FromStrings("__name__", "go_gc_duration_seconds", "quantile", "0"), + lset: labels.FromStrings("__name__", "go_gc_duration_seconds", "quantile", "0.0"), }, { m: `go_gc_duration_seconds{quantile="0.25"}`, v: 7.424100000000001e-05, From 70742a64aa97762a062e3595e0c0cf50788bee6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 21 Oct 2024 11:03:47 +0200 Subject: [PATCH 39/48] Follow up #15178 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renaming Signed-off-by: György Krajcsovits --- scrape/scrape_test.go | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index fef4d0b7f..35d5f14ac 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3844,24 +3844,24 @@ metric: < for metricsTextName, metricsText := range metricsTexts { for name, tc := range map[string]struct { - scrapeClassicHistograms bool - convertClassicHistograms bool + alwaysScrapeClassicHistograms bool + convertClassicHistograms bool }{ "convert with scrape": { - scrapeClassicHistograms: true, - convertClassicHistograms: true, + alwaysScrapeClassicHistograms: true, + convertClassicHistograms: true, }, "convert without scrape": { - scrapeClassicHistograms: false, - convertClassicHistograms: true, + alwaysScrapeClassicHistograms: false, + convertClassicHistograms: true, }, "scrape without convert": { - scrapeClassicHistograms: true, - convertClassicHistograms: false, + alwaysScrapeClassicHistograms: true, + convertClassicHistograms: false, }, "neither scrape nor convert": { - scrapeClassicHistograms: false, - convertClassicHistograms: false, + alwaysScrapeClassicHistograms: false, + convertClassicHistograms: false, }, } { var expectedClassicHistCount, expectedNativeHistCount int @@ -3870,16 +3870,16 @@ metric: < expectedNativeHistCount = 1 expectCustomBuckets = false expectedClassicHistCount = 0 - if metricsText.hasClassic && tc.scrapeClassicHistograms { + if metricsText.hasClassic && tc.alwaysScrapeClassicHistograms { expectedClassicHistCount = 1 } } else if metricsText.hasClassic { switch { - case tc.scrapeClassicHistograms && tc.convertClassicHistograms: + case tc.alwaysScrapeClassicHistograms && tc.convertClassicHistograms: expectedClassicHistCount = 1 expectedNativeHistCount = 1 expectCustomBuckets = true - case !tc.scrapeClassicHistograms && tc.convertClassicHistograms: + case !tc.alwaysScrapeClassicHistograms && tc.convertClassicHistograms: expectedClassicHistCount = 0 expectedNativeHistCount = 1 expectCustomBuckets = true @@ -3894,13 +3894,13 @@ metric: < defer simpleStorage.Close() config := &config.ScrapeConfig{ - JobName: "test", - SampleLimit: 100, - Scheme: "http", - ScrapeInterval: model.Duration(100 * time.Millisecond), - ScrapeTimeout: model.Duration(100 * time.Millisecond), - ScrapeClassicHistograms: tc.scrapeClassicHistograms, - ConvertClassicHistograms: tc.convertClassicHistograms, + JobName: "test", + SampleLimit: 100, + Scheme: "http", + ScrapeInterval: model.Duration(100 * time.Millisecond), + ScrapeTimeout: model.Duration(100 * time.Millisecond), + AlwaysScrapeClassicHistograms: tc.alwaysScrapeClassicHistograms, + ConvertClassicHistograms: tc.convertClassicHistograms, } scrapeCount := 0 From a23aed5634169ad400947a7ff91955d6a64cb5b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 21 Oct 2024 11:10:50 +0200 Subject: [PATCH 40/48] More followup to #15164 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scrape test for NHCB modified. Signed-off-by: György Krajcsovits --- scrape/scrape_test.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 35d5f14ac..6187119bf 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3758,17 +3758,11 @@ metric: < }, } - checkBucketValues := func(expectedCount int, contentType string, series storage.SeriesSet) { + checkBucketValues := func(expectedCount int, series storage.SeriesSet) { labelName := "le" var expectedValues []string if expectedCount > 0 { - if contentType == "application/vnd.google.protobuf" { - // The expected "le" values have the trailing ".0". - expectedValues = []string{"0.005", "0.01", "0.025", "0.05", "0.1", "0.25", "0.5", "1.0", "2.5", "5.0", "10.0", "+Inf"} - } else { - // The expected "le" values do not have the trailing ".0". - expectedValues = []string{"0.005", "0.01", "0.025", "0.05", "0.1", "0.25", "0.5", "1", "2.5", "5", "10", "+Inf"} - } + expectedValues = []string{"0.005", "0.01", "0.025", "0.05", "0.1", "0.25", "0.5", "1.0", "2.5", "5.0", "10.0", "+Inf"} } foundLeValues := map[string]bool{} @@ -3984,7 +3978,7 @@ metric: < checkFloatSeries(series, expectedClassicHistCount, 10.) series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d_bucket", i))) - checkBucketValues(expectedClassicHistCount, metricsText.contentType, series) + checkBucketValues(expectedClassicHistCount, series) series = q.Select(ctx, false, nil, labels.MustNewMatcher(labels.MatchRegexp, "__name__", fmt.Sprintf("test_histogram_%d", i))) From 4283ae73dcc3439d851c227dc770310f20f24e40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 21 Oct 2024 13:22:58 +0200 Subject: [PATCH 41/48] Rename convert_classic_histograms to convert_classic_histograms_to_nhcb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On reviewer request. Signed-off-by: György Krajcsovits --- config/config.go | 2 +- scrape/manager.go | 2 +- scrape/scrape.go | 19 +++++++++++-------- scrape/scrape_test.go | 32 ++++++++++++++++---------------- 4 files changed, 29 insertions(+), 26 deletions(-) diff --git a/config/config.go b/config/config.go index 962a0f4a7..657c4fc75 100644 --- a/config/config.go +++ b/config/config.go @@ -656,7 +656,7 @@ type ScrapeConfig struct { // Whether to scrape a classic histogram, even if it is also exposed as a native histogram. AlwaysScrapeClassicHistograms bool `yaml:"always_scrape_classic_histograms,omitempty"` // Whether to convert all scraped classic histograms into a native histogram with custom buckets. - ConvertClassicHistograms bool `yaml:"convert_classic_histograms,omitempty"` + ConvertClassicHistogramsToNHCB bool `yaml:"convert_classic_histograms_to_nhcb,omitempty"` // File to which scrape failures are logged. ScrapeFailureLogFile string `yaml:"scrape_failure_log_file,omitempty"` // The HTTP resource path on which to fetch metrics from targets. diff --git a/scrape/manager.go b/scrape/manager.go index 9791db0e8..f3dad2a04 100644 --- a/scrape/manager.go +++ b/scrape/manager.go @@ -178,7 +178,7 @@ func (m *Manager) reload() { m.logger.Error("error reloading target set", "err", "invalid config id:"+setName) continue } - if scrapeConfig.ConvertClassicHistograms && m.opts.EnableCreatedTimestampZeroIngestion { + if scrapeConfig.ConvertClassicHistogramsToNHCB && m.opts.EnableCreatedTimestampZeroIngestion { // TODO(krajorama): fix https://github.com/prometheus/prometheus/issues/15137 m.logger.Error("error reloading target set", "err", "cannot convert classic histograms to native histograms with custom buckets and ingest created timestamp zero samples at the same time due to https://github.com/prometheus/prometheus/issues/15137") continue diff --git a/scrape/scrape.go b/scrape/scrape.go index 290855b3a..c252d57f6 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -113,7 +113,7 @@ type scrapeLoopOptions struct { interval time.Duration timeout time.Duration alwaysScrapeClassicHist bool - convertClassicHistograms bool + convertClassicHistToNHCB bool validationScheme model.ValidationScheme fallbackScrapeProtocol string @@ -182,7 +182,7 @@ func newScrapePool(cfg *config.ScrapeConfig, app storage.Appendable, offsetSeed opts.interval, opts.timeout, opts.alwaysScrapeClassicHist, - opts.convertClassicHistograms, + opts.convertClassicHistToNHCB, options.EnableNativeHistogramsIngestion, options.EnableCreatedTimestampZeroIngestion, options.ExtraMetrics, @@ -488,7 +488,7 @@ func (sp *scrapePool) sync(targets []*Target) { mrc = sp.config.MetricRelabelConfigs fallbackScrapeProtocol = sp.config.ScrapeFallbackProtocol.HeaderMediaType() alwaysScrapeClassicHist = sp.config.AlwaysScrapeClassicHistograms - convertClassicHistograms = sp.config.ConvertClassicHistograms + convertClassicHistToNHCB = sp.config.ConvertClassicHistogramsToNHCB ) validationScheme := model.UTF8Validation @@ -530,7 +530,7 @@ func (sp *scrapePool) sync(targets []*Target) { interval: interval, timeout: timeout, alwaysScrapeClassicHist: alwaysScrapeClassicHist, - convertClassicHistograms: convertClassicHistograms, + convertClassicHistToNHCB: convertClassicHistToNHCB, validationScheme: validationScheme, fallbackScrapeProtocol: fallbackScrapeProtocol, }) @@ -894,7 +894,7 @@ type scrapeLoop struct { interval time.Duration timeout time.Duration alwaysScrapeClassicHist bool - convertClassicHistograms bool + convertClassicHistToNHCB bool validationScheme model.ValidationScheme fallbackScrapeProtocol string @@ -1196,7 +1196,7 @@ func newScrapeLoop(ctx context.Context, interval time.Duration, timeout time.Duration, alwaysScrapeClassicHist bool, - convertClassicHistograms bool, + convertClassicHistToNHCB bool, enableNativeHistogramIngestion bool, enableCTZeroIngestion bool, reportExtraMetrics bool, @@ -1252,7 +1252,7 @@ func newScrapeLoop(ctx context.Context, interval: interval, timeout: timeout, alwaysScrapeClassicHist: alwaysScrapeClassicHist, - convertClassicHistograms: convertClassicHistograms, + convertClassicHistToNHCB: convertClassicHistToNHCB, enableNativeHistogramIngestion: enableNativeHistogramIngestion, enableCTZeroIngestion: enableCTZeroIngestion, reportExtraMetrics: reportExtraMetrics, @@ -1563,7 +1563,7 @@ func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string, ) return } - if sl.convertClassicHistograms { + if sl.convertClassicHistToNHCB { p = textparse.NewNHCBParser(p, sl.symbolTable, sl.alwaysScrapeClassicHist) } if err != nil { @@ -1751,6 +1751,9 @@ loop: } else { ref, err = app.AppendHistogram(ref, lset, t, nil, fh) } + if err != nil { + fmt.Printf("Error when appending histogram in scrape loop: %s\n", err) + } } else { ref, err = app.Append(ref, lset, t, val) } diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 6187119bf..9a70d7411 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -3478,7 +3478,7 @@ test_summary_count 199 } // Testing whether we can automatically convert scraped classic histograms into native histograms with custom buckets. -func TestConvertClassicHistograms(t *testing.T) { +func TestConvertClassicHistogramsToNHCB(t *testing.T) { genTestCounterText := func(name string, value int, withMetadata bool) string { if withMetadata { return fmt.Sprintf(` @@ -3839,23 +3839,23 @@ metric: < for metricsTextName, metricsText := range metricsTexts { for name, tc := range map[string]struct { alwaysScrapeClassicHistograms bool - convertClassicHistograms bool + convertClassicHistToNHCB bool }{ "convert with scrape": { alwaysScrapeClassicHistograms: true, - convertClassicHistograms: true, + convertClassicHistToNHCB: true, }, "convert without scrape": { alwaysScrapeClassicHistograms: false, - convertClassicHistograms: true, + convertClassicHistToNHCB: true, }, "scrape without convert": { alwaysScrapeClassicHistograms: true, - convertClassicHistograms: false, + convertClassicHistToNHCB: false, }, "neither scrape nor convert": { alwaysScrapeClassicHistograms: false, - convertClassicHistograms: false, + convertClassicHistToNHCB: false, }, } { var expectedClassicHistCount, expectedNativeHistCount int @@ -3869,15 +3869,15 @@ metric: < } } else if metricsText.hasClassic { switch { - case tc.alwaysScrapeClassicHistograms && tc.convertClassicHistograms: + case tc.alwaysScrapeClassicHistograms && tc.convertClassicHistToNHCB: expectedClassicHistCount = 1 expectedNativeHistCount = 1 expectCustomBuckets = true - case !tc.alwaysScrapeClassicHistograms && tc.convertClassicHistograms: + case !tc.alwaysScrapeClassicHistograms && tc.convertClassicHistToNHCB: expectedClassicHistCount = 0 expectedNativeHistCount = 1 expectCustomBuckets = true - case !tc.convertClassicHistograms: + case !tc.convertClassicHistToNHCB: expectedClassicHistCount = 1 expectedNativeHistCount = 0 } @@ -3888,13 +3888,13 @@ metric: < defer simpleStorage.Close() config := &config.ScrapeConfig{ - JobName: "test", - SampleLimit: 100, - Scheme: "http", - ScrapeInterval: model.Duration(100 * time.Millisecond), - ScrapeTimeout: model.Duration(100 * time.Millisecond), - AlwaysScrapeClassicHistograms: tc.alwaysScrapeClassicHistograms, - ConvertClassicHistograms: tc.convertClassicHistograms, + JobName: "test", + SampleLimit: 100, + Scheme: "http", + ScrapeInterval: model.Duration(100 * time.Millisecond), + ScrapeTimeout: model.Duration(100 * time.Millisecond), + AlwaysScrapeClassicHistograms: tc.alwaysScrapeClassicHistograms, + ConvertClassicHistogramsToNHCB: tc.convertClassicHistToNHCB, } scrapeCount := 0 From 5ee0980cd1d50881f9693a85c486f25bca2976da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 21 Oct 2024 13:35:33 +0200 Subject: [PATCH 42/48] Add unit test to show that current wrapper is sub-optimal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://github.com/prometheus/prometheus/pull/14978#discussion_r1800755481 Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse_test.go | 175 +++++++++++++++++++++++++++++- 1 file changed, 174 insertions(+), 1 deletion(-) diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index 7cff21712..37fcccb9d 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -14,13 +14,18 @@ package textparse import ( + "bytes" + "encoding/binary" "testing" - "github.com/prometheus/common/model" + "github.com/gogo/protobuf/proto" + "github.com/stretchr/testify/require" + "github.com/prometheus/common/model" "github.com/prometheus/prometheus/model/exemplar" "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" + dto "github.com/prometheus/prometheus/prompb/io/prometheus/client" ) func TestNHCBParserOnOMParser(t *testing.T) { @@ -513,3 +518,171 @@ something_bucket{a="b",le="+Inf"} 9 # {id="something-test"} 2e100 123.000 got := testParse(t, p) requireEntries(t, exp, got) } + +// Verify that the NHCBParser does not parse the NHCB when the exponential is present. +func TestNHCBParserProtoBufParser_NoNHCBWhenExponential(t *testing.T) { + inputBuf := createTestProtoBufHistogram(t) + // Initialize the protobuf parser so that it returns classic histograms as + // well when there's both classic and exponential histograms. + p := NewProtobufParser(inputBuf.Bytes(), true, labels.NewSymbolTable()) + + // Initialize the NHCBParser so that it returns classic histograms as well + // when there's both classic and exponential histograms. + p = NewNHCBParser(p, labels.NewSymbolTable(), true) + + exp := []parsedEntry{ + { + m: "test_histogram", + help: "Test histogram with classic and exponential buckets.", + }, + { + m: "test_histogram", + typ: model.MetricTypeHistogram, + }, + { + m: "test_histogram", + shs: &histogram.Histogram{ + Schema: 3, + Count: 175, + Sum: 0.0008280461746287094, + ZeroThreshold: 2.938735877055719e-39, + ZeroCount: 2, + PositiveSpans: []histogram.Span{{Offset: -161, Length: 1}, {Offset: 8, Length: 3}}, + NegativeSpans: []histogram.Span{{Offset: -162, Length: 1}, {Offset: 23, Length: 4}}, + PositiveBuckets: []int64{1, 2, -1, -1}, + NegativeBuckets: []int64{1, 3, -2, -1, 1}, + }, + lset: labels.FromStrings("__name__", "test_histogram"), + t: int64p(1234568), + }, + { + m: "test_histogram_count", + v: 175, + lset: labels.FromStrings("__name__", "test_histogram_count"), + t: int64p(1234568), + }, + { + m: "test_histogram_sum", + v: 0.0008280461746287094, + lset: labels.FromStrings("__name__", "test_histogram_sum"), + t: int64p(1234568), + }, + { + m: "test_histogram_bucket\xffle\xff-0.0004899999999999998", + v: 2, + lset: labels.FromStrings("__name__", "test_histogram_bucket", "le", "-0.0004899999999999998"), + t: int64p(1234568), + }, + { + m: "test_histogram_bucket\xffle\xff-0.0003899999999999998", + v: 4, + lset: labels.FromStrings("__name__", "test_histogram_bucket", "le", "-0.0003899999999999998"), + t: int64p(1234568), + }, + { + m: "test_histogram_bucket\xffle\xff-0.0002899999999999998", + v: 16, + lset: labels.FromStrings("__name__", "test_histogram_bucket", "le", "-0.0002899999999999998"), + t: int64p(1234568), + }, + { + m: "test_histogram_bucket\xffle\xff+Inf", + v: 175, + lset: labels.FromStrings("__name__", "test_histogram_bucket", "le", "+Inf"), + t: int64p(1234568), + }, + { + // TODO(krajorama): optimize: this should not be here. In case there's + // an exponential histogram we should not scrape the classic histogram. + // TSDB will throw this away with storage.errDuplicateSampleForTimestamp + // at Commit(), but it needs to be parsed here after the exponential + // histogram. + m: "test_histogram{}", + shs: &histogram.Histogram{ + Schema: histogram.CustomBucketsSchema, + Count: 175, + Sum: 0.0008280461746287094, + PositiveSpans: []histogram.Span{{Length: 4}}, + PositiveBuckets: []int64{2, 0, 10, 147}, + CustomValues: []float64{-0.0004899999999999998, -0.0003899999999999998, -0.0002899999999999998}, + }, + lset: labels.FromStrings("__name__", "test_histogram"), + t: int64p(1234568), + }, + } + got := testParse(t, p) + requireEntries(t, exp, got) +} + +func createTestProtoBufHistogram(t *testing.T) *bytes.Buffer { + testMetricFamilies := []string{`name: "test_histogram" +help: "Test histogram with classic and exponential buckets." +type: HISTOGRAM +metric: < + histogram: < + sample_count: 175 + sample_sum: 0.0008280461746287094 + bucket: < + cumulative_count: 2 + upper_bound: -0.0004899999999999998 + > + bucket: < + cumulative_count: 4 + upper_bound: -0.0003899999999999998 + > + bucket: < + cumulative_count: 16 + upper_bound: -0.0002899999999999998 + > + schema: 3 + zero_threshold: 2.938735877055719e-39 + zero_count: 2 + negative_span: < + offset: -162 + length: 1 + > + negative_span: < + offset: 23 + length: 4 + > + negative_delta: 1 + negative_delta: 3 + negative_delta: -2 + negative_delta: -1 + negative_delta: 1 + positive_span: < + offset: -161 + length: 1 + > + positive_span: < + offset: 8 + length: 3 + > + positive_delta: 1 + positive_delta: 2 + positive_delta: -1 + positive_delta: -1 + > + timestamp_ms: 1234568 +> +`} + + varintBuf := make([]byte, binary.MaxVarintLen32) + buf := &bytes.Buffer{} + + for _, tmf := range testMetricFamilies { + pb := &dto.MetricFamily{} + // From text to proto message. + require.NoError(t, proto.UnmarshalText(tmf, pb)) + // From proto message to binary protobuf. + protoBuf, err := proto.Marshal(pb) + require.NoError(t, err) + + // Write first length, then binary protobuf. + varintLength := binary.PutUvarint(varintBuf, uint64(len(protoBuf))) + buf.Write(varintBuf[:varintLength]) + buf.Write(protoBuf) + } + + return buf +} From eaee6bacc7960e3f00fd057458228fce28dddd21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 21 Oct 2024 13:40:16 +0200 Subject: [PATCH 43/48] Fix failing benchmarks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/benchmark_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/textparse/benchmark_test.go b/model/textparse/benchmark_test.go index 98aadb0ed..bc9c2d1db 100644 --- a/model/textparse/benchmark_test.go +++ b/model/textparse/benchmark_test.go @@ -40,7 +40,7 @@ var newTestParserFns = map[string]newParser{ "omtext": func(b []byte, st *labels.SymbolTable) Parser { return NewOpenMetricsParser(b, st, WithOMParserCTSeriesSkipped()) }, - "omtext_with_nhcb": func(b []byte, st *labels.SymbolTable) Parser { + "nhcb_over_omtext": func(b []byte, st *labels.SymbolTable) Parser { p := NewOpenMetricsParser(b, st, WithOMParserCTSeriesSkipped()) return NewNHCBParser(p, st, false) }, From a6947e1e6da848c2b1f83d5144eb6ac5a8083e5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 21 Oct 2024 13:45:33 +0200 Subject: [PATCH 44/48] Remove omcounterdata.txt as redundant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/benchmark_test.go | 2 -- model/textparse/testdata/omcounterdata.txt | 9 --------- 2 files changed, 11 deletions(-) delete mode 100644 model/textparse/testdata/omcounterdata.txt diff --git a/model/textparse/benchmark_test.go b/model/textparse/benchmark_test.go index bc9c2d1db..f6d3a9559 100644 --- a/model/textparse/benchmark_test.go +++ b/model/textparse/benchmark_test.go @@ -86,8 +86,6 @@ func BenchmarkParse(b *testing.B) { // NHCB. {dataFile: "omhistogramdata.txt", parser: "omtext"}, // Measure OM parser baseline for histograms. {dataFile: "omhistogramdata.txt", parser: "nhcb_over_omtext"}, // Measure NHCB over OM parser. - {dataFile: "omcounterdata.txt", parser: "omtext"}, // Measure OM parser baseline for counters. - {dataFile: "omcounterdata.txt", parser: "nhcb_over_omtext"}, // Measure NHCB over OM parser. } { var buf []byte dataCase := bcase.dataFile diff --git a/model/textparse/testdata/omcounterdata.txt b/model/textparse/testdata/omcounterdata.txt deleted file mode 100644 index 15459c018..000000000 --- a/model/textparse/testdata/omcounterdata.txt +++ /dev/null @@ -1,9 +0,0 @@ -# HELP rpc_requests Total number of RPC requests received. -# TYPE rpc_requests counter -rpc_requests_total{service="exponential"} 22.0 -rpc_requests_created{service="exponential"} 1.726839813016893e+09 -rpc_requests_total{service="normal"} 15.0 -rpc_requests_created{service="normal"} 1.726839813016717e+09 -rpc_requests_total{service="uniform"} 11.0 -rpc_requests_created{service="uniform"} 1.7268398130168471e+09 -# EOF From 555bd6292a1be32aa546731c6913a7ff19fe8311 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 21 Oct 2024 13:48:21 +0200 Subject: [PATCH 45/48] Better docstring on test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index 37fcccb9d..80d846646 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -593,10 +593,10 @@ func TestNHCBParserProtoBufParser_NoNHCBWhenExponential(t *testing.T) { }, { // TODO(krajorama): optimize: this should not be here. In case there's - // an exponential histogram we should not scrape the classic histogram. - // TSDB will throw this away with storage.errDuplicateSampleForTimestamp - // at Commit(), but it needs to be parsed here after the exponential - // histogram. + // an exponential histogram we should not convert the classic histogram + // to NHCB. In the end TSDB will throw this away with + // storage.errDuplicateSampleForTimestamp error at Commit(), but it + // is better to avoid this conversion in the first place. m: "test_histogram{}", shs: &histogram.Histogram{ Schema: histogram.CustomBucketsSchema, From bee1eb77206f9973b0b9d2528b6c2b7f0506223f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 21 Oct 2024 14:02:32 +0200 Subject: [PATCH 46/48] goimports run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/nhcbparse_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/model/textparse/nhcbparse_test.go b/model/textparse/nhcbparse_test.go index 80d846646..80b65fd22 100644 --- a/model/textparse/nhcbparse_test.go +++ b/model/textparse/nhcbparse_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/require" "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/model/exemplar" "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" From 25ef4d34839c7cca87fec09d4c616e1ada9dce78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= Date: Mon, 21 Oct 2024 15:40:48 +0200 Subject: [PATCH 47/48] benchmark, rename parser omtext_with_nhcb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: György Krajcsovits --- model/textparse/benchmark_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/model/textparse/benchmark_test.go b/model/textparse/benchmark_test.go index f6d3a9559..bd0d5089a 100644 --- a/model/textparse/benchmark_test.go +++ b/model/textparse/benchmark_test.go @@ -40,7 +40,7 @@ var newTestParserFns = map[string]newParser{ "omtext": func(b []byte, st *labels.SymbolTable) Parser { return NewOpenMetricsParser(b, st, WithOMParserCTSeriesSkipped()) }, - "nhcb_over_omtext": func(b []byte, st *labels.SymbolTable) Parser { + "omtext_with_nhcb": func(b []byte, st *labels.SymbolTable) Parser { p := NewOpenMetricsParser(b, st, WithOMParserCTSeriesSkipped()) return NewNHCBParser(p, st, false) }, @@ -85,7 +85,7 @@ func BenchmarkParse(b *testing.B) { // NHCB. {dataFile: "omhistogramdata.txt", parser: "omtext"}, // Measure OM parser baseline for histograms. - {dataFile: "omhistogramdata.txt", parser: "nhcb_over_omtext"}, // Measure NHCB over OM parser. + {dataFile: "omhistogramdata.txt", parser: "omtext_with_nhcb"}, // Measure NHCB over OM parser. } { var buf []byte dataCase := bcase.dataFile From 877fd2a60e027732c54490049a3a849657d5e08c Mon Sep 17 00:00:00 2001 From: George Krajcsovits Date: Mon, 21 Oct 2024 16:01:34 +0200 Subject: [PATCH 48/48] Update scrape/scrape.go Signed-off-by: George Krajcsovits --- scrape/scrape.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/scrape/scrape.go b/scrape/scrape.go index c252d57f6..f5f02d245 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -1751,9 +1751,6 @@ loop: } else { ref, err = app.AppendHistogram(ref, lset, t, nil, fh) } - if err != nil { - fmt.Printf("Error when appending histogram in scrape loop: %s\n", err) - } } else { ref, err = app.Append(ref, lset, t, val) }