From 9051100abacce8003a2174a842f3641dc501aec2 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 17 Oct 2023 09:27:46 +0000 Subject: [PATCH 1/6] Scraping: share buffer pool across all scrapes Previously we had one per scrapePool, and one of those per configured scraping job. Each pool holds a few unused buffers, so sharing one across all scrapePools reduces total heap memory. Signed-off-by: Bryan Boreham --- scrape/manager.go | 5 ++++- scrape/scrape.go | 4 +--- scrape/scrape_test.go | 24 +++++++++++++----------- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/scrape/manager.go b/scrape/manager.go index a0ac38f6ba..3b70e48a13 100644 --- a/scrape/manager.go +++ b/scrape/manager.go @@ -32,6 +32,7 @@ import ( "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/util/osutil" + "github.com/prometheus/prometheus/util/pool" ) // NewManager is the Manager constructor. @@ -57,6 +58,7 @@ func NewManager(o *Options, logger log.Logger, app storage.Appendable, registere graceShut: make(chan struct{}), triggerReload: make(chan struct{}, 1), metrics: sm, + buffers: pool.New(1e3, 100e6, 3, func(sz int) interface{} { return make([]byte, 0, sz) }), } m.metrics.setTargetMetadataCacheGatherer(m) @@ -94,6 +96,7 @@ type Manager struct { scrapeConfigs map[string]*config.ScrapeConfig scrapePools map[string]*scrapePool targetSets map[string][]*targetgroup.Group + buffers *pool.Pool triggerReload chan struct{} @@ -156,7 +159,7 @@ func (m *Manager) reload() { continue } m.metrics.targetScrapePools.Inc() - sp, err := newScrapePool(scrapeConfig, m.append, m.offsetSeed, log.With(m.logger, "scrape_pool", setName), m.opts, m.metrics) + sp, err := newScrapePool(scrapeConfig, m.append, m.offsetSeed, log.With(m.logger, "scrape_pool", setName), m.buffers, m.opts, m.metrics) if err != nil { m.metrics.targetScrapePoolsFailed.Inc() level.Error(m.logger).Log("msg", "error creating new scrape pool", "err", err, "scrape_pool", setName) diff --git a/scrape/scrape.go b/scrape/scrape.go index 983bee8378..f3542935a3 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -117,7 +117,7 @@ const maxAheadTime = 10 * time.Minute // returning an empty label set is interpreted as "drop". type labelsMutator func(labels.Labels) labels.Labels -func newScrapePool(cfg *config.ScrapeConfig, app storage.Appendable, offsetSeed uint64, logger log.Logger, options *Options, metrics *scrapeMetrics) (*scrapePool, error) { +func newScrapePool(cfg *config.ScrapeConfig, app storage.Appendable, offsetSeed uint64, logger log.Logger, buffers *pool.Pool, options *Options, metrics *scrapeMetrics) (*scrapePool, error) { if logger == nil { logger = log.NewNopLogger() } @@ -127,8 +127,6 @@ func newScrapePool(cfg *config.ScrapeConfig, app storage.Appendable, offsetSeed return nil, fmt.Errorf("error creating HTTP client: %w", err) } - buffers := pool.New(1e3, 100e6, 3, func(sz int) interface{} { return make([]byte, 0, sz) }) - ctx, cancel := context.WithCancel(context.Background()) sp := &scrapePool{ cancel: cancel, diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 238e90c204..41aca737f2 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -49,6 +49,7 @@ import ( "github.com/prometheus/prometheus/model/value" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/tsdb/chunkenc" + "github.com/prometheus/prometheus/util/pool" "github.com/prometheus/prometheus/util/teststorage" "github.com/prometheus/prometheus/util/testutil" ) @@ -68,7 +69,7 @@ func TestNewScrapePool(t *testing.T) { var ( app = &nopAppendable{} cfg = &config.ScrapeConfig{} - sp, _ = newScrapePool(cfg, app, 0, nil, &Options{}, newTestScrapeMetrics(t)) + sp, _ = newScrapePool(cfg, app, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) ) if a, ok := sp.appendable.(*nopAppendable); !ok || a != app { @@ -104,7 +105,7 @@ func TestDroppedTargetsList(t *testing.T) { }, }, } - sp, _ = newScrapePool(cfg, app, 0, nil, &Options{}, newTestScrapeMetrics(t)) + sp, _ = newScrapePool(cfg, app, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) expectedLabelSetString = "{__address__=\"127.0.0.1:9090\", __scrape_interval__=\"0s\", __scrape_timeout__=\"0s\", job=\"dropMe\"}" expectedLength = 2 ) @@ -504,7 +505,7 @@ func TestScrapePoolTargetLimit(t *testing.T) { func TestScrapePoolAppender(t *testing.T) { cfg := &config.ScrapeConfig{} app := &nopAppendable{} - sp, _ := newScrapePool(cfg, app, 0, nil, &Options{}, newTestScrapeMetrics(t)) + sp, _ := newScrapePool(cfg, app, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) loop := sp.newLoop(scrapeLoopOptions{ target: &Target{}, @@ -560,7 +561,7 @@ func TestScrapePoolRaces(t *testing.T) { newConfig := func() *config.ScrapeConfig { return &config.ScrapeConfig{ScrapeInterval: interval, ScrapeTimeout: timeout} } - sp, _ := newScrapePool(newConfig(), &nopAppendable{}, 0, nil, &Options{}, newTestScrapeMetrics(t)) + sp, _ := newScrapePool(newConfig(), &nopAppendable{}, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) tgts := []*targetgroup.Group{ { Targets: []model.LabelSet{ @@ -3277,7 +3278,7 @@ func TestReuseScrapeCache(t *testing.T) { ScrapeInterval: model.Duration(5 * time.Second), MetricsPath: "/metrics", } - sp, _ = newScrapePool(cfg, app, 0, nil, &Options{}, newTestScrapeMetrics(t)) + sp, _ = newScrapePool(cfg, app, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) t1 = &Target{ discoveredLabels: labels.FromStrings("labelNew", "nameNew", "labelNew1", "nameNew1", "labelNew2", "nameNew2"), } @@ -3481,8 +3482,9 @@ func TestReuseCacheRace(t *testing.T) { ScrapeInterval: model.Duration(5 * time.Second), MetricsPath: "/metrics", } - sp, _ = newScrapePool(cfg, app, 0, nil, &Options{}, newTestScrapeMetrics(t)) - t1 = &Target{ + buffers = pool.New(1e3, 100e6, 3, func(sz int) interface{} { return make([]byte, 0, sz) }) + sp, _ = newScrapePool(cfg, app, 0, nil, buffers, &Options{}, newTestScrapeMetrics(t)) + t1 = &Target{ discoveredLabels: labels.FromStrings("labelNew", "nameNew"), } ) @@ -3612,7 +3614,7 @@ func TestScrapeReportLimit(t *testing.T) { })) defer ts.Close() - sp, err := newScrapePool(cfg, s, 0, nil, &Options{}, newTestScrapeMetrics(t)) + sp, err := newScrapePool(cfg, s, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) require.NoError(t, err) defer sp.stop() @@ -3786,7 +3788,7 @@ func TestTargetScrapeIntervalAndTimeoutRelabel(t *testing.T) { }, }, } - sp, _ := newScrapePool(config, &nopAppendable{}, 0, nil, &Options{}, newTestScrapeMetrics(t)) + sp, _ := newScrapePool(config, &nopAppendable{}, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) tgts := []*targetgroup.Group{ { Targets: []model.LabelSet{{model.AddressLabel: "127.0.0.1:9090"}}, @@ -3877,7 +3879,7 @@ test_summary_count 199 })) defer ts.Close() - sp, err := newScrapePool(config, simpleStorage, 0, nil, &Options{}, newTestScrapeMetrics(t)) + sp, err := newScrapePool(config, simpleStorage, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) require.NoError(t, err) defer sp.stop() @@ -4029,7 +4031,7 @@ func TestScrapeLoopCompression(t *testing.T) { EnableCompression: tc.enableCompression, } - sp, err := newScrapePool(config, simpleStorage, 0, nil, &Options{}, newTestScrapeMetrics(t)) + sp, err := newScrapePool(config, simpleStorage, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) require.NoError(t, err) defer sp.stop() From f0e1b592ab10ea89a56dcabdbbf91f5d7e91cc40 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 24 Nov 2023 14:38:35 +0000 Subject: [PATCH 2/6] Scraping: use slices.sort for exemplars The sort implementation using Go generics is used everywhere else in Prometheus. Signed-off-by: Bryan Boreham --- model/exemplar/exemplar.go | 15 +++++++++++++++ scrape/scrape.go | 14 ++------------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/model/exemplar/exemplar.go b/model/exemplar/exemplar.go index 2e39cf6892..08f55374ef 100644 --- a/model/exemplar/exemplar.go +++ b/model/exemplar/exemplar.go @@ -48,3 +48,18 @@ func (e Exemplar) Equals(e2 Exemplar) bool { return e.Value == e2.Value } + +// Sort first by timestamp, then value, then labels. +func Compare(a, b Exemplar) int { + if a.Ts < b.Ts { + return -1 + } else if a.Ts > b.Ts { + return 1 + } + if a.Value < b.Value { + return -1 + } else if a.Value > b.Value { + return 1 + } + return labels.Compare(a.Labels, b.Labels) +} diff --git a/scrape/scrape.go b/scrape/scrape.go index 983bee8378..590eac889c 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -24,7 +24,6 @@ import ( "math" "net/http" "reflect" - "sort" "strconv" "strings" "sync" @@ -1610,17 +1609,8 @@ loop: exemplars = append(exemplars, e) e = exemplar.Exemplar{} // Reset for next time round loop. } - sort.Slice(exemplars, func(i, j int) bool { - // Sort first by timestamp, then value, then labels so the checking - // for duplicates / out of order is more efficient during validation. - if exemplars[i].Ts != exemplars[j].Ts { - return exemplars[i].Ts < exemplars[j].Ts - } - if exemplars[i].Value != exemplars[j].Value { - return exemplars[i].Value < exemplars[j].Value - } - return exemplars[i].Labels.Hash() < exemplars[j].Labels.Hash() - }) + // Sort so that checking for duplicates / out of order is more efficient during validation. + slices.SortFunc(exemplars, exemplar.Compare) outOfOrderExemplars := 0 for _, e := range exemplars { _, exemplarErr := app.AppendExemplar(ref, lset, e) From 3e287e01700f48959fcd19f7e8b2de6a16d83726 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 16 Oct 2023 13:47:10 +0000 Subject: [PATCH 3/6] Scraping tests: refactor scrapeLoop creation Pull boilerplate code into a function. Where appropriate we set some config on the returned object. Signed-off-by: Bryan Boreham --- scrape/scrape_test.go | 818 ++++++------------------------------------ 1 file changed, 100 insertions(+), 718 deletions(-) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 5e4f3f30c7..15be3bcfaa 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -640,22 +640,22 @@ func TestScrapePoolScrapeLoopsStarted(t *testing.T) { } } -func TestScrapeLoopStopBeforeRun(t *testing.T) { - scraper := &testScraper{} - - sl := newScrapeLoop(context.Background(), +func newBasicScrapeLoop(t testing.TB, ctx context.Context, scraper scraper, app func(ctx context.Context) storage.Appender, interval time.Duration) *scrapeLoop { + return newScrapeLoop(ctx, scraper, nil, nil, nopMutator, nopMutator, - nil, nil, 0, + app, + nil, + 0, true, false, true, 0, 0, nil, - 1, - 0, + interval, + time.Hour, false, false, false, @@ -663,6 +663,11 @@ func TestScrapeLoopStopBeforeRun(t *testing.T) { false, newTestScrapeMetrics(t), ) +} + +func TestScrapeLoopStopBeforeRun(t *testing.T) { + scraper := &testScraper{} + sl := newBasicScrapeLoop(t, context.Background(), scraper, nil, 1) // The scrape pool synchronizes on stopping scrape loops. However, new scrape // loops are started asynchronously. Thus it's possible, that a loop is stopped @@ -717,28 +722,7 @@ func TestScrapeLoopStop(t *testing.T) { app = func(ctx context.Context) storage.Appender { return appender } ) - sl := newScrapeLoop(context.Background(), - scraper, - nil, nil, - nopMutator, - nopMutator, - app, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 10*time.Millisecond, - time.Hour, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), scraper, app, 10*time.Millisecond) // Terminate loop after 2 scrapes. numScrapes := 0 @@ -857,28 +841,8 @@ func TestScrapeLoopRun(t *testing.T) { } ctx, cancel = context.WithCancel(context.Background()) - sl = newScrapeLoop(ctx, - scraper, - nil, nil, - nopMutator, - nopMutator, - app, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - time.Second, - 100*time.Millisecond, - false, - false, - false, - nil, - false, - scrapeMetrics, - ) + sl = newBasicScrapeLoop(t, ctx, scraper, app, time.Second) + sl.timeout = 100 * time.Millisecond go func() { sl.run(errc) @@ -920,28 +884,7 @@ func TestScrapeLoopForcedErr(t *testing.T) { ) ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - scraper, - nil, nil, - nopMutator, - nopMutator, - app, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - time.Second, - time.Hour, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, scraper, app, time.Second) forcedErr := fmt.Errorf("forced err") sl.setForcedError(forcedErr) @@ -1044,28 +987,7 @@ func simpleTestScrapeLoop(t testing.TB) (context.Context, *scrapeLoop) { t.Cleanup(func() { s.Close() }) ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - &testScraper{}, - nil, nil, - nopMutator, - nopMutator, - s.Appender, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, &testScraper{}, s.Appender, 0) t.Cleanup(func() { cancel() }) return ctx, sl @@ -1106,30 +1028,10 @@ func TestScrapeLoopFailWithInvalidLabelsAfterRelabel(t *testing.T) { Separator: ";", Replacement: "$1", }} - sl := newScrapeLoop(ctx, - &testScraper{}, - nil, nil, - func(l labels.Labels) labels.Labels { - return mutateSampleLabels(l, target, true, relabelConfig) - }, - nopMutator, - s.Appender, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, &testScraper{}, s.Appender, 0) + sl.sampleMutator = func(l labels.Labels) labels.Labels { + return mutateSampleLabels(l, target, true, relabelConfig) + } slApp := sl.appender(ctx) total, added, seriesAdded, err := sl.append(slApp, []byte("test_metric 1\n"), "", time.Time{}) @@ -1190,28 +1092,7 @@ func TestScrapeLoopRunCreatesStaleMarkersOnFailedScrape(t *testing.T) { ) ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - scraper, - nil, nil, - nopMutator, - nopMutator, - app, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 10*time.Millisecond, - time.Hour, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, scraper, app, 10*time.Millisecond) // Succeed once, several failures, then stop. numScrapes := 0 @@ -1257,28 +1138,7 @@ func TestScrapeLoopRunCreatesStaleMarkersOnParseFailure(t *testing.T) { ) ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - scraper, - nil, nil, - nopMutator, - nopMutator, - app, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 10*time.Millisecond, - time.Hour, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, scraper, app, 10*time.Millisecond) // Succeed once, several failures, then stop. scraper.scrapeFunc = func(ctx context.Context, w io.Writer) error { @@ -1327,28 +1187,7 @@ func TestScrapeLoopCache(t *testing.T) { ) ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - scraper, - nil, nil, - nopMutator, - nopMutator, - app, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 10*time.Millisecond, - time.Hour, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, scraper, app, 10*time.Millisecond) numScrapes := 0 @@ -1414,28 +1253,7 @@ func TestScrapeLoopCacheMemoryExhaustionProtection(t *testing.T) { ) ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - scraper, - nil, nil, - nopMutator, - nopMutator, - app, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 10*time.Millisecond, - time.Hour, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, scraper, app, 10*time.Millisecond) numScrapes := 0 @@ -1529,31 +1347,13 @@ func TestScrapeLoopAppend(t *testing.T) { labels: labels.FromStrings(test.discoveryLabels...), } - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - func(l labels.Labels) labels.Labels { - return mutateSampleLabels(l, discoveryLabels, test.honorLabels, nil) - }, - func(l labels.Labels) labels.Labels { - return mutateReportSampleLabels(l, discoveryLabels) - }, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) + sl.sampleMutator = func(l labels.Labels) labels.Labels { + return mutateSampleLabels(l, discoveryLabels, test.honorLabels, nil) + } + sl.reportSampleMutator = func(l labels.Labels) labels.Labels { + return mutateReportSampleLabels(l, discoveryLabels) + } now := time.Now() @@ -1635,14 +1435,10 @@ func TestScrapeLoopAppendForConflictingPrefixedLabels(t *testing.T) { for name, tc := range testcases { t.Run(name, func(t *testing.T) { app := &collectResultAppender{} - sl := newScrapeLoop(context.Background(), nil, nil, nil, - func(l labels.Labels) labels.Labels { - return mutateSampleLabels(l, &Target{labels: labels.FromStrings(tc.targetLabels...)}, false, nil) - }, - nil, - func(ctx context.Context) storage.Appender { return app }, - nil, 0, true, false, true, 0, 0, nil, 0, 0, false, false, false, nil, false, newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) + sl.sampleMutator = func(l labels.Labels) labels.Labels { + return mutateSampleLabels(l, &Target{labels: labels.FromStrings(tc.targetLabels...)}, false, nil) + } slApp := sl.appender(context.Background()) _, _, _, err := sl.append(slApp, []byte(tc.exposedLabels), "", time.Date(2000, 1, 1, 1, 0, 0, 0, time.UTC)) require.NoError(t, err) @@ -1663,28 +1459,7 @@ func TestScrapeLoopAppendForConflictingPrefixedLabels(t *testing.T) { func TestScrapeLoopAppendCacheEntryButErrNotFound(t *testing.T) { // collectResultAppender's AddFast always returns ErrNotFound if we don't give it a next. app := &collectResultAppender{} - - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - nopMutator, - nopMutator, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) fakeRef := storage.SeriesRef(1) expValue := float64(1) @@ -1721,32 +1496,14 @@ func TestScrapeLoopAppendSampleLimit(t *testing.T) { resApp := &collectResultAppender{} app := &limitAppender{Appender: resApp, limit: 1} - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - func(l labels.Labels) labels.Labels { - if l.Has("deleteme") { - return labels.EmptyLabels() - } - return l - }, - nopMutator, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - false, - true, - app.limit, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) + sl.sampleMutator = func(l labels.Labels) labels.Labels { + if l.Has("deleteme") { + return labels.EmptyLabels() + } + return l + } + sl.sampleLimit = app.limit // Get the value of the Counter before performing the append. beforeMetric := dto.Metric{} @@ -1802,32 +1559,14 @@ func TestScrapeLoop_HistogramBucketLimit(t *testing.T) { resApp := &collectResultAppender{} app := &bucketLimitAppender{Appender: resApp, limit: 2} - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - func(l labels.Labels) labels.Labels { - if l.Has("deleteme") { - return labels.EmptyLabels() - } - return l - }, - nopMutator, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - false, - true, - app.limit, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) + sl.sampleMutator = func(l labels.Labels) labels.Labels { + if l.Has("deleteme") { + return labels.EmptyLabels() + } + return l + } + sl.sampleLimit = app.limit metric := dto.Metric{} err := sl.metrics.targetScrapeNativeHistogramBucketLimit.Write(&metric) @@ -1908,28 +1647,7 @@ func TestScrapeLoop_ChangingMetricString(t *testing.T) { defer s.Close() capp := &collectResultAppender{} - - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - nopMutator, - nopMutator, - func(ctx context.Context) storage.Appender { capp.next = s.Appender(ctx); return capp }, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return capp }, 0) now := time.Now() slApp := sl.appender(context.Background()) @@ -1961,27 +1679,7 @@ func TestScrapeLoop_ChangingMetricString(t *testing.T) { func TestScrapeLoopAppendStaleness(t *testing.T) { app := &collectResultAppender{} - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - nopMutator, - nopMutator, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) now := time.Now() slApp := sl.appender(context.Background()) @@ -2016,28 +1714,7 @@ func TestScrapeLoopAppendStaleness(t *testing.T) { func TestScrapeLoopAppendNoStalenessIfTimestamp(t *testing.T) { app := &collectResultAppender{} - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - nopMutator, - nopMutator, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) - + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) now := time.Now() slApp := sl.appender(context.Background()) _, _, _, err := sl.append(slApp, []byte("metric_a 1 1000\n"), "", now) @@ -2061,27 +1738,8 @@ func TestScrapeLoopAppendNoStalenessIfTimestamp(t *testing.T) { func TestScrapeLoopAppendStalenessIfTrackTimestampStaleness(t *testing.T) { app := &collectResultAppender{} - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - nopMutator, - nopMutator, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - true, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) + sl.trackTimestampsStaleness = true now := time.Now() slApp := sl.appender(context.Background()) @@ -2430,31 +2088,14 @@ metric: < labels: labels.FromStrings(test.discoveryLabels...), } - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - func(l labels.Labels) labels.Labels { - return mutateSampleLabels(l, discoveryLabels, false, nil) - }, - func(l labels.Labels) labels.Labels { - return mutateReportSampleLabels(l, discoveryLabels) - }, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - test.scrapeClassicHistograms, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) + sl.sampleMutator = func(l labels.Labels) labels.Labels { + return mutateSampleLabels(l, discoveryLabels, false, nil) + } + sl.reportSampleMutator = func(l labels.Labels) labels.Labels { + return mutateReportSampleLabels(l, discoveryLabels) + } + sl.scrapeClassicHistograms = test.scrapeClassicHistograms now := time.Now() @@ -2521,31 +2162,13 @@ func TestScrapeLoopAppendExemplarSeries(t *testing.T) { app := &collectResultAppender{} - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - func(l labels.Labels) labels.Labels { - return mutateSampleLabels(l, discoveryLabels, false, nil) - }, - func(l labels.Labels) labels.Labels { - return mutateReportSampleLabels(l, discoveryLabels) - }, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) + sl.sampleMutator = func(l labels.Labels) labels.Labels { + return mutateSampleLabels(l, discoveryLabels, false, nil) + } + sl.reportSampleMutator = func(l labels.Labels) labels.Labels { + return mutateReportSampleLabels(l, discoveryLabels) + } now := time.Now() @@ -2580,28 +2203,7 @@ func TestScrapeLoopRunReportsTargetDownOnScrapeError(t *testing.T) { ) ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - scraper, - nil, nil, - nopMutator, - nopMutator, - app, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 10*time.Millisecond, - time.Hour, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, scraper, app, 10*time.Millisecond) scraper.scrapeFunc = func(ctx context.Context, w io.Writer) error { cancel() @@ -2620,28 +2222,7 @@ func TestScrapeLoopRunReportsTargetDownOnInvalidUTF8(t *testing.T) { ) ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - scraper, - nil, nil, - nopMutator, - nopMutator, - app, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 10*time.Millisecond, - time.Hour, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, scraper, app, 10*time.Millisecond) scraper.scrapeFunc = func(ctx context.Context, w io.Writer) error { cancel() @@ -2672,29 +2253,7 @@ func (app *errorAppender) Append(ref storage.SeriesRef, lset labels.Labels, t in func TestScrapeLoopAppendGracefullyIfAmendOrOutOfOrderOrOutOfBounds(t *testing.T) { app := &errorAppender{} - - sl := newScrapeLoop(context.Background(), - nil, - nil, nil, - nopMutator, - nopMutator, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) now := time.Unix(1, 0) slApp := sl.appender(context.Background()) @@ -2717,32 +2276,14 @@ func TestScrapeLoopAppendGracefullyIfAmendOrOutOfOrderOrOutOfBounds(t *testing.T func TestScrapeLoopOutOfBoundsTimeError(t *testing.T) { app := &collectResultAppender{} - sl := newScrapeLoop(context.Background(), - nil, - nil, nil, - nopMutator, - nopMutator, + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return &timeLimitAppender{ Appender: app, maxTime: timestamp.FromTime(time.Now().Add(10 * time.Minute)), } }, - nil, 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), ) now := time.Now().Add(20 * time.Minute) @@ -3014,29 +2555,8 @@ func TestScrapeLoop_RespectTimestamps(t *testing.T) { defer s.Close() app := s.Appender(context.Background()) - capp := &collectResultAppender{next: app} - - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - nopMutator, - nopMutator, - func(ctx context.Context) storage.Appender { return capp }, - nil, 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return capp }, 0) now := time.Now() slApp := sl.appender(context.Background()) @@ -3062,26 +2582,8 @@ func TestScrapeLoop_DiscardTimestamps(t *testing.T) { capp := &collectResultAppender{next: app} - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - nopMutator, - nopMutator, - func(ctx context.Context) storage.Appender { return capp }, - nil, 0, - false, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return capp }, 0) + sl.honorTimestamps = false now := time.Now() slApp := sl.appender(context.Background()) @@ -3104,28 +2606,7 @@ func TestScrapeLoopDiscardDuplicateLabels(t *testing.T) { defer s.Close() ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - &testScraper{}, - nil, nil, - nopMutator, - nopMutator, - s.Appender, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, &testScraper{}, s.Appender, 0) defer cancel() // We add a good and a bad metric to check that both are discarded. @@ -3161,33 +2642,13 @@ func TestScrapeLoopDiscardUnnamedMetrics(t *testing.T) { app := s.Appender(context.Background()) ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(context.Background(), - &testScraper{}, - nil, nil, - func(l labels.Labels) labels.Labels { - if l.Has("drop") { - return labels.FromStrings("no", "name") // This label set will trigger an error. - } - return l - }, - nopMutator, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), &testScraper{}, func(ctx context.Context) storage.Appender { return app }, 0) + sl.sampleMutator = func(l labels.Labels) labels.Labels { + if l.Has("drop") { + return labels.FromStrings("no", "name") // This label set will trigger an error. + } + return l + } defer cancel() slApp := sl.appender(context.Background()) @@ -3433,28 +2894,7 @@ func TestScrapeAddFast(t *testing.T) { defer s.Close() ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - &testScraper{}, - nil, nil, - nopMutator, - nopMutator, - s.Appender, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, &testScraper{}, s.Appender, 0) defer cancel() slApp := sl.appender(ctx) @@ -3523,28 +2963,7 @@ func TestScrapeReportSingleAppender(t *testing.T) { ) ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - scraper, - nil, nil, - nopMutator, - nopMutator, - s.Appender, - nil, - 0, - true, - false, - true, - 0, 0, - nil, - 10*time.Millisecond, - time.Hour, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, scraper, s.Appender, 10*time.Millisecond) numScrapes := 0 @@ -3726,31 +3145,14 @@ func TestScrapeLoopLabelLimit(t *testing.T) { labels: labels.FromStrings(test.discoveryLabels...), } - sl := newScrapeLoop(context.Background(), - nil, nil, nil, - func(l labels.Labels) labels.Labels { - return mutateSampleLabels(l, discoveryLabels, false, nil) - }, - func(l labels.Labels) labels.Labels { - return mutateReportSampleLabels(l, discoveryLabels) - }, - func(ctx context.Context) storage.Appender { return app }, - nil, - 0, - true, - false, - true, - 0, 0, - &test.labelLimits, - 0, - 0, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0) + sl.sampleMutator = func(l labels.Labels) labels.Labels { + return mutateSampleLabels(l, discoveryLabels, false, nil) + } + sl.reportSampleMutator = func(l labels.Labels) labels.Labels { + return mutateReportSampleLabels(l, discoveryLabels) + } + sl.labelLimits = &test.labelLimits slApp := sl.appender(context.Background()) _, _, _, err := sl.append(slApp, []byte(test.scrapeLabels), "", time.Now()) @@ -3936,28 +3338,8 @@ func TestScrapeLoopRunCreatesStaleMarkersOnFailedScrapeForTimestampedMetrics(t * ) ctx, cancel := context.WithCancel(context.Background()) - sl := newScrapeLoop(ctx, - scraper, - nil, nil, - nopMutator, - nopMutator, - app, - nil, - 0, - true, - true, - true, - 0, 0, - nil, - 10*time.Millisecond, - time.Hour, - false, - false, - false, - nil, - false, - newTestScrapeMetrics(t), - ) + sl := newBasicScrapeLoop(t, ctx, scraper, app, 10*time.Millisecond) + sl.trackTimestampsStaleness = true // Succeed once, several failures, then stop. numScrapes := 0 From 2e205ee95c121d8d6da0d8984f0b3bc599acaa2a Mon Sep 17 00:00:00 2001 From: Yury Molodov Date: Fri, 24 Nov 2023 22:44:48 +0100 Subject: [PATCH 4/6] ui: heatmap visualization for histogram buckets (#13096) ui: heatmap visualization for histogram buckets Signed-off-by: Yury Moladau --------- Signed-off-by: Yury Moladau --- .../react-app/src/pages/graph/Graph.test.tsx | 19 +- web/ui/react-app/src/pages/graph/Graph.tsx | 48 +++-- .../src/pages/graph/GraphControls.test.tsx | 13 +- .../src/pages/graph/GraphControls.tsx | 30 ++- .../src/pages/graph/GraphHeatmapHelpers.ts | 56 +++++ .../src/pages/graph/GraphHelpers.test.ts | 1 + .../react-app/src/pages/graph/GraphHelpers.ts | 62 +++--- .../src/pages/graph/GraphTabContent.tsx | 7 +- .../react-app/src/pages/graph/Panel.test.tsx | 10 +- web/ui/react-app/src/pages/graph/Panel.tsx | 30 ++- web/ui/react-app/src/types/index.d.ts | 1 + web/ui/react-app/src/utils/index.ts | 16 +- web/ui/react-app/src/utils/utils.test.ts | 17 +- .../src/vendor/flot/jquery.flot.heatmap.js | 195 ++++++++++++++++++ 14 files changed, 413 insertions(+), 92 deletions(-) create mode 100644 web/ui/react-app/src/pages/graph/GraphHeatmapHelpers.ts create mode 100644 web/ui/react-app/src/vendor/flot/jquery.flot.heatmap.js diff --git a/web/ui/react-app/src/pages/graph/Graph.test.tsx b/web/ui/react-app/src/pages/graph/Graph.test.tsx index 7b226792ce..01625f7c2f 100644 --- a/web/ui/react-app/src/pages/graph/Graph.test.tsx +++ b/web/ui/react-app/src/pages/graph/Graph.test.tsx @@ -4,6 +4,7 @@ import { shallow, mount } from 'enzyme'; import Graph from './Graph'; import ReactResizeDetector from 'react-resize-detector'; import { Legend } from './Legend'; +import { GraphDisplayMode } from './Panel'; describe('Graph', () => { beforeAll(() => { @@ -30,7 +31,7 @@ describe('Graph', () => { endTime: 1572130692, resolution: 28, }, - stacked: false, + displayMode: GraphDisplayMode.Stacked, data: { resultType: 'matrix', result: [ @@ -115,7 +116,7 @@ describe('Graph', () => { graph = mount( { ); }); it('should trigger state update when stacked prop is changed', () => { - graph.setProps({ stacked: false }); + graph.setProps({ displayMode: GraphDisplayMode.Lines }); expect(spyState).toHaveBeenCalledWith( { chartData: { @@ -177,7 +178,7 @@ describe('Graph', () => { const graph = mount( { const graph = shallow( { const graph = mount( { const graph = mount( { const graph = mount( { const graph: any = mount( ; }; exemplars: ExemplarData; - stacked: boolean; + displayMode: GraphDisplayMode; useLocalTime: boolean; showExemplars: boolean; handleTimeRangeSelection: (startTime: number, endTime: number) => void; @@ -69,11 +71,11 @@ class Graph extends PureComponent { }; componentDidUpdate(prevProps: GraphProps): void { - const { data, stacked, useLocalTime, showExemplars } = this.props; + const { data, displayMode, useLocalTime, showExemplars } = this.props; if (prevProps.data !== data) { this.selectedSeriesIndexes = []; this.setState({ chartData: normalizeData(this.props) }, this.plot); - } else if (prevProps.stacked !== stacked) { + } else if (prevProps.displayMode !== displayMode) { this.setState({ chartData: normalizeData(this.props) }, () => { if (this.selectedSeriesIndexes.length === 0) { this.plot(); @@ -143,7 +145,18 @@ class Graph extends PureComponent { } this.destroyPlot(); - this.$chart = $.plot($(this.chartRef.current), data, getOptions(this.props.stacked, this.props.useLocalTime)); + const options = getOptions(this.props.displayMode === GraphDisplayMode.Stacked, this.props.useLocalTime); + const isHeatmap = this.props.displayMode === GraphDisplayMode.Heatmap; + options.series.heatmap = isHeatmap; + + if (options.yaxis && isHeatmap) { + options.yaxis.ticks = () => new Array(data.length + 1).fill(0).map((_el, i) => i); + options.yaxis.tickFormatter = (val) => `${val ? data[val - 1].labels.le : ''}`; + options.yaxis.min = 0; + options.yaxis.max = data.length; + options.series.lines = { show: false }; + } + this.$chart = $.plot($(this.chartRef.current), data, options); }; destroyPlot = (): void => { @@ -165,7 +178,10 @@ class Graph extends PureComponent { const { chartData } = this.state; this.plot( this.selectedSeriesIndexes.length === 1 && this.selectedSeriesIndexes.includes(selectedIndex) - ? [...chartData.series.map(toHoverColor(selectedIndex, this.props.stacked)), ...chartData.exemplars] + ? [ + ...chartData.series.map(toHoverColor(selectedIndex, this.props.displayMode === GraphDisplayMode.Stacked)), + ...chartData.exemplars, + ] : [ ...chartData.series.filter((_, i) => selected.includes(i)), ...chartData.exemplars.filter((exemplar) => { @@ -190,7 +206,7 @@ class Graph extends PureComponent { } this.rafID = requestAnimationFrame(() => { this.plotSetAndDraw([ - ...this.state.chartData.series.map(toHoverColor(index, this.props.stacked)), + ...this.state.chartData.series.map(toHoverColor(index, this.props.displayMode === GraphDisplayMode.Stacked)), ...this.state.chartData.exemplars, ]); }); @@ -251,13 +267,15 @@ class Graph extends PureComponent { ) : null} - + {this.props.displayMode !== GraphDisplayMode.Heatmap && ( + + )} {/* This is to make sure the graph box expands when the selected exemplar info pops up. */}
diff --git a/web/ui/react-app/src/pages/graph/GraphControls.test.tsx b/web/ui/react-app/src/pages/graph/GraphControls.test.tsx index 3d1961714d..b5d843ebea 100755 --- a/web/ui/react-app/src/pages/graph/GraphControls.test.tsx +++ b/web/ui/react-app/src/pages/graph/GraphControls.test.tsx @@ -5,13 +5,15 @@ import { Button, ButtonGroup, Form, InputGroup, InputGroupAddon, Input } from 'r import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faPlus, faMinus, faChartArea, faChartLine } from '@fortawesome/free-solid-svg-icons'; import TimeInput from './TimeInput'; +import { GraphDisplayMode } from './Panel'; const defaultGraphControlProps = { range: 60 * 60 * 24 * 1000, endTime: 1572100217898, useLocalTime: false, resolution: 10, - stacked: false, + displayMode: GraphDisplayMode.Lines, + isHeatmapData: false, showExemplars: false, onChangeRange: (): void => { @@ -29,6 +31,9 @@ const defaultGraphControlProps = { onChangeShowExemplars: (): void => { // Do nothing. }, + onChangeDisplayMode: (): void => { + // Do nothing. + }, }; describe('GraphControls', () => { @@ -163,10 +168,10 @@ describe('GraphControls', () => { }, ].forEach((testCase) => { const results: boolean[] = []; - const onChange = (stacked: boolean): void => { - results.push(stacked); + const onChange = (mode: GraphDisplayMode): void => { + results.push(mode === GraphDisplayMode.Stacked); }; - const controls = shallow(); + const controls = shallow(); const group = controls.find(ButtonGroup); const btn = group.find(Button).filterWhere((btn) => btn.prop('title') === testCase.title); const onClick = btn.prop('onClick'); diff --git a/web/ui/react-app/src/pages/graph/GraphControls.tsx b/web/ui/react-app/src/pages/graph/GraphControls.tsx index 969400bfdd..f71b46af5e 100644 --- a/web/ui/react-app/src/pages/graph/GraphControls.tsx +++ b/web/ui/react-app/src/pages/graph/GraphControls.tsx @@ -2,23 +2,24 @@ import React, { Component } from 'react'; import { Button, ButtonGroup, Form, Input, InputGroup, InputGroupAddon } from 'reactstrap'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; -import { faChartArea, faChartLine, faMinus, faPlus } from '@fortawesome/free-solid-svg-icons'; +import { faChartArea, faChartLine, faMinus, faPlus, faBarChart } from '@fortawesome/free-solid-svg-icons'; import TimeInput from './TimeInput'; import { formatDuration, parseDuration } from '../../utils'; +import { GraphDisplayMode } from './Panel'; interface GraphControlsProps { range: number; endTime: number | null; useLocalTime: boolean; resolution: number | null; - stacked: boolean; + displayMode: GraphDisplayMode; + isHeatmapData: boolean; showExemplars: boolean; - onChangeRange: (range: number) => void; onChangeEndTime: (endTime: number | null) => void; onChangeResolution: (resolution: number | null) => void; - onChangeStacking: (stacked: boolean) => void; onChangeShowExemplars: (show: boolean) => void; + onChangeDisplayMode: (mode: GraphDisplayMode) => void; } class GraphControls extends Component { @@ -153,14 +154,29 @@ class GraphControls extends Component { - + {/* TODO: Consider replacing this button with a select dropdown in the future, + to allow users to choose from multiple histogram series if available. */} + {this.props.isHeatmapData && ( + + )} diff --git a/web/ui/react-app/src/pages/graph/GraphHeatmapHelpers.ts b/web/ui/react-app/src/pages/graph/GraphHeatmapHelpers.ts new file mode 100644 index 0000000000..d564959cb8 --- /dev/null +++ b/web/ui/react-app/src/pages/graph/GraphHeatmapHelpers.ts @@ -0,0 +1,56 @@ +import { GraphProps, GraphSeries } from './Graph'; + +export function isHeatmapData(data: GraphProps['data']) { + if (!data?.result?.length || data?.result?.length < 2) { + return false; + } + const result = data.result; + const firstLabels = Object.keys(result[0].metric).filter((label) => label !== 'le'); + return result.every(({ metric }) => { + const labels = Object.keys(metric).filter((label) => label !== 'le'); + const allLabelsMatch = labels.every((label) => metric[label] === result[0].metric[label]); + return metric.le && labels.length === firstLabels.length && allLabelsMatch; + }); +} + +export function prepareHeatmapData(buckets: GraphSeries[]) { + if (!buckets.every((a) => a.labels.le)) { + return buckets; + } + + const sortedBuckets = buckets.sort((a, b) => promValueToNumber(a.labels.le) - promValueToNumber(b.labels.le)); + const result: GraphSeries[] = []; + + for (let i = 0; i < sortedBuckets.length; i++) { + const values = []; + const { data, labels, color } = sortedBuckets[i]; + + for (const [timestamp, value] of data) { + const prevVal = sortedBuckets[i - 1]?.data.find((v) => v[0] === timestamp)?.[1] || 0; + const newVal = Number(value) - prevVal; + values.push([Number(timestamp), newVal]); + } + + result.push({ + data: values, + labels, + color, + index: i, + }); + } + return result; +} + +export function promValueToNumber(s: string) { + switch (s) { + case 'NaN': + return NaN; + case 'Inf': + case '+Inf': + return Infinity; + case '-Inf': + return -Infinity; + default: + return parseFloat(s); + } +} diff --git a/web/ui/react-app/src/pages/graph/GraphHelpers.test.ts b/web/ui/react-app/src/pages/graph/GraphHelpers.test.ts index 5fb0675a40..206cc59a13 100644 --- a/web/ui/react-app/src/pages/graph/GraphHelpers.test.ts +++ b/web/ui/react-app/src/pages/graph/GraphHelpers.test.ts @@ -212,6 +212,7 @@ describe('GraphHelpers', () => { }, series: { stack: false, + heatmap: false, lines: { lineWidth: 1, steps: false, fill: true }, shadowSize: 0, }, diff --git a/web/ui/react-app/src/pages/graph/GraphHelpers.ts b/web/ui/react-app/src/pages/graph/GraphHelpers.ts index f77383f568..21bb768f52 100644 --- a/web/ui/react-app/src/pages/graph/GraphHelpers.ts +++ b/web/ui/react-app/src/pages/graph/GraphHelpers.ts @@ -1,9 +1,11 @@ import $ from 'jquery'; import { escapeHTML } from '../../utils'; -import { GraphProps, GraphData, GraphSeries, GraphExemplar } from './Graph'; +import { GraphData, GraphExemplar, GraphProps, GraphSeries } from './Graph'; import moment from 'moment-timezone'; import { colorPool } from './ColorPool'; +import { prepareHeatmapData } from './GraphHeatmapHelpers'; +import { GraphDisplayMode } from './Panel'; export const formatValue = (y: number | null): string => { if (y === null) { @@ -145,6 +147,7 @@ export const getOptions = (stacked: boolean, useLocalTime: boolean): jquery.flot }, series: { stack: false, // Stacking is set on a per-series basis because exemplar symbols don't support it. + heatmap: false, lines: { lineWidth: stacked ? 1 : 2, steps: false, @@ -158,7 +161,7 @@ export const getOptions = (stacked: boolean, useLocalTime: boolean): jquery.flot }; }; -export const normalizeData = ({ queryParams, data, exemplars, stacked }: GraphProps): GraphData => { +export const normalizeData = ({ queryParams, data, exemplars, displayMode }: GraphProps): GraphData => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const { startTime, endTime, resolution } = queryParams!; @@ -188,36 +191,37 @@ export const normalizeData = ({ queryParams, data, exemplars, stacked }: GraphPr } const deviation = stdDeviation(sum, values); - return { - series: data.result.map(({ values, histograms, metric }, index) => { - // Insert nulls for all missing steps. - const data = []; - let valuePos = 0; - let histogramPos = 0; + const series = data.result.map(({ values, histograms, metric }, index) => { + // Insert nulls for all missing steps. + const data = []; + let valuePos = 0; + let histogramPos = 0; - for (let t = startTime; t <= endTime; t += resolution) { - // Allow for floating point inaccuracy. - const currentValue = values && values[valuePos]; - const currentHistogram = histograms && histograms[histogramPos]; - if (currentValue && values.length > valuePos && currentValue[0] < t + resolution / 100) { - data.push([currentValue[0] * 1000, parseValue(currentValue[1])]); - valuePos++; - } else if (currentHistogram && histograms.length > histogramPos && currentHistogram[0] < t + resolution / 100) { - data.push([currentHistogram[0] * 1000, parseValue(currentHistogram[1].sum)]); - histogramPos++; - } else { - data.push([t * 1000, null]); - } + for (let t = startTime; t <= endTime; t += resolution) { + // Allow for floating point inaccuracy. + const currentValue = values && values[valuePos]; + const currentHistogram = histograms && histograms[histogramPos]; + if (currentValue && values.length > valuePos && currentValue[0] < t + resolution / 100) { + data.push([currentValue[0] * 1000, parseValue(currentValue[1])]); + valuePos++; + } else if (currentHistogram && histograms.length > histogramPos && currentHistogram[0] < t + resolution / 100) { + data.push([currentHistogram[0] * 1000, parseValue(currentHistogram[1].sum)]); + histogramPos++; + } else { + data.push([t * 1000, null]); } + } + return { + labels: metric !== null ? metric : {}, + color: colorPool[index % colorPool.length], + stack: displayMode === GraphDisplayMode.Stacked, + data, + index, + }; + }); - return { - labels: metric !== null ? metric : {}, - color: colorPool[index % colorPool.length], - stack: stacked, - data, - index, - }; - }), + return { + series: displayMode === GraphDisplayMode.Heatmap ? prepareHeatmapData(series) : series, exemplars: Object.values(buckets).flatMap((bucket) => { if (bucket.length === 1) { return bucket[0]; diff --git a/web/ui/react-app/src/pages/graph/GraphTabContent.tsx b/web/ui/react-app/src/pages/graph/GraphTabContent.tsx index e31c7a280f..a5e7529945 100644 --- a/web/ui/react-app/src/pages/graph/GraphTabContent.tsx +++ b/web/ui/react-app/src/pages/graph/GraphTabContent.tsx @@ -3,12 +3,13 @@ import { Alert } from 'reactstrap'; import Graph from './Graph'; import { QueryParams, ExemplarData } from '../../types/types'; import { isPresent } from '../../utils'; +import { GraphDisplayMode } from './Panel'; interface GraphTabContentProps { // eslint-disable-next-line @typescript-eslint/no-explicit-any data: any; exemplars: ExemplarData; - stacked: boolean; + displayMode: GraphDisplayMode; useLocalTime: boolean; showExemplars: boolean; handleTimeRangeSelection: (startTime: number, endTime: number) => void; @@ -19,7 +20,7 @@ interface GraphTabContentProps { export const GraphTabContent: FC = ({ data, exemplars, - stacked, + displayMode, useLocalTime, lastQueryParams, showExemplars, @@ -41,7 +42,7 @@ export const GraphTabContent: FC = ({ { @@ -84,7 +84,7 @@ describe('Panel', () => { range: 10, endTime: 1572100217898, resolution: 28, - stacked: false, + displayMode: GraphDisplayMode.Lines, showExemplars: true, }; const graphPanel = mount(); @@ -94,8 +94,8 @@ describe('Panel', () => { expect(controls.prop('endTime')).toEqual(options.endTime); expect(controls.prop('range')).toEqual(options.range); expect(controls.prop('resolution')).toEqual(options.resolution); - expect(controls.prop('stacked')).toEqual(options.stacked); - expect(graph.prop('stacked')).toEqual(options.stacked); + expect(controls.prop('displayMode')).toEqual(options.displayMode); + expect(graph.prop('displayMode')).toEqual(options.displayMode); }); describe('when switching between modes', () => { diff --git a/web/ui/react-app/src/pages/graph/Panel.tsx b/web/ui/react-app/src/pages/graph/Panel.tsx index 244a7606e7..501ab67c94 100644 --- a/web/ui/react-app/src/pages/graph/Panel.tsx +++ b/web/ui/react-app/src/pages/graph/Panel.tsx @@ -13,6 +13,7 @@ import QueryStatsView, { QueryStats } from './QueryStatsView'; import { QueryParams, ExemplarData } from '../../types/types'; import { API_PATH } from '../../constants/constants'; import { debounce } from '../../utils'; +import { isHeatmapData } from './GraphHeatmapHelpers'; interface PanelProps { options: PanelOptions; @@ -39,6 +40,7 @@ interface PanelState { error: string | null; stats: QueryStats | null; exprInputValue: string; + isHeatmapData: boolean; } export interface PanelOptions { @@ -47,7 +49,7 @@ export interface PanelOptions { range: number; // Range in milliseconds. endTime: number | null; // Timestamp in milliseconds. resolution: number | null; // Resolution in seconds. - stacked: boolean; + displayMode: GraphDisplayMode; showExemplars: boolean; } @@ -56,13 +58,19 @@ export enum PanelType { Table = 'table', } +export enum GraphDisplayMode { + Lines = 'lines', + Stacked = 'stacked', + Heatmap = 'heatmap', +} + export const PanelDefaultOptions: PanelOptions = { type: PanelType.Table, expr: '', range: 60 * 60 * 1000, endTime: null, resolution: null, - stacked: false, + displayMode: GraphDisplayMode.Lines, showExemplars: false, }; @@ -82,6 +90,7 @@ class Panel extends Component { error: null, stats: null, exprInputValue: props.options.expr, + isHeatmapData: false, }; this.debounceExecuteQuery = debounce(this.executeQuery.bind(this), 250); @@ -184,6 +193,11 @@ class Panel extends Component { } } + const isHeatmap = isHeatmapData(query.data); + if (!isHeatmap) { + this.setOptions({ displayMode: GraphDisplayMode.Lines }); + } + this.setState({ error: null, data: query.data, @@ -200,6 +214,7 @@ class Panel extends Component { resultSeries, }, loading: false, + isHeatmapData: isHeatmap, }); this.abortInFlightFetch = null; } catch (err: unknown) { @@ -252,8 +267,8 @@ class Panel extends Component { this.setOptions({ type: type }); }; - handleChangeStacking = (stacked: boolean): void => { - this.setOptions({ stacked: stacked }); + handleChangeDisplayMode = (mode: GraphDisplayMode): void => { + this.setOptions({ displayMode: mode }); }; handleChangeShowExemplars = (show: boolean): void => { @@ -337,18 +352,19 @@ class Panel extends Component { endTime={options.endTime} useLocalTime={this.props.useLocalTime} resolution={options.resolution} - stacked={options.stacked} + displayMode={options.displayMode} + isHeatmapData={this.state.isHeatmapData} showExemplars={options.showExemplars} onChangeRange={this.handleChangeRange} onChangeEndTime={this.handleChangeEndTime} onChangeResolution={this.handleChangeResolution} - onChangeStacking={this.handleChangeStacking} + onChangeDisplayMode={this.handleChangeDisplayMode} onChangeShowExemplars={this.handleChangeShowExemplars} /> { @@ -196,8 +196,12 @@ export const parseOption = (param: string): Partial => { case 'tab': return { type: decodedValue === '0' ? PanelType.Graph : PanelType.Table }; + case 'display_mode': + const validKey = Object.values(GraphDisplayMode).includes(decodedValue as GraphDisplayMode); + return { displayMode: validKey ? (decodedValue as GraphDisplayMode) : GraphDisplayMode.Lines }; + case 'stacked': - return { stacked: decodedValue === '1' }; + return { displayMode: decodedValue === '1' ? GraphDisplayMode.Stacked : GraphDisplayMode.Lines }; case 'show_exemplars': return { showExemplars: decodedValue === '1' }; @@ -225,12 +229,12 @@ export const formatParam = export const toQueryString = ({ key, options }: PanelMeta): string => { const formatWithKey = formatParam(key); - const { expr, type, stacked, range, endTime, resolution, showExemplars } = options; + const { expr, type, displayMode, range, endTime, resolution, showExemplars } = options; const time = isPresent(endTime) ? formatTime(endTime) : false; const urlParams = [ formatWithKey('expr', expr), formatWithKey('tab', type === PanelType.Graph ? 0 : 1), - formatWithKey('stacked', stacked ? 1 : 0), + formatWithKey('display_mode', displayMode), formatWithKey('show_exemplars', showExemplars ? 1 : 0), formatWithKey('range_input', formatDuration(range)), time ? `${formatWithKey('end_input', time)}&${formatWithKey('moment_input', time)}` : '', @@ -264,7 +268,9 @@ export const getQueryParam = (key: string): string => { }; export const createExpressionLink = (expr: string): string => { - return `../graph?g0.expr=${encodeURIComponent(expr)}&g0.tab=1&g0.stacked=0&g0.show_exemplars=0.g0.range_input=1h.`; + return `../graph?g0.expr=${encodeURIComponent(expr)}&g0.tab=1&g0.display_mode=${ + GraphDisplayMode.Lines + }&g0.show_exemplars=0.g0.range_input=1h.`; }; // eslint-disable-next-line @typescript-eslint/no-explicit-any, diff --git a/web/ui/react-app/src/utils/utils.test.ts b/web/ui/react-app/src/utils/utils.test.ts index 4db3e28a64..4b14465816 100644 --- a/web/ui/react-app/src/utils/utils.test.ts +++ b/web/ui/react-app/src/utils/utils.test.ts @@ -16,7 +16,7 @@ import { decodePanelOptionsFromQueryString, parsePrometheusFloat, } from '.'; -import { PanelType } from '../pages/graph/Panel'; +import { GraphDisplayMode, PanelType } from '../pages/graph/Panel'; describe('Utils', () => { describe('escapeHTML', (): void => { @@ -210,7 +210,7 @@ describe('Utils', () => { expr: 'rate(node_cpu_seconds_total{mode="system"}[1m])', range: 60 * 60 * 1000, resolution: null, - stacked: false, + displayMode: GraphDisplayMode.Lines, type: PanelType.Graph, }, }, @@ -221,13 +221,12 @@ describe('Utils', () => { expr: 'node_filesystem_avail_bytes', range: 60 * 60 * 1000, resolution: null, - stacked: false, + displayMode: GraphDisplayMode.Lines, type: PanelType.Table, }, }, ]; - const query = - '?g0.expr=rate(node_cpu_seconds_total%7Bmode%3D%22system%22%7D%5B1m%5D)&g0.tab=0&g0.stacked=0&g0.show_exemplars=0&g0.range_input=1h&g0.end_input=2019-10-25%2023%3A37%3A00&g0.moment_input=2019-10-25%2023%3A37%3A00&g1.expr=node_filesystem_avail_bytes&g1.tab=1&g1.stacked=0&g1.show_exemplars=0&g1.range_input=1h'; + const query = `?g0.expr=rate(node_cpu_seconds_total%7Bmode%3D%22system%22%7D%5B1m%5D)&g0.tab=0&g0.display_mode=${GraphDisplayMode.Lines}&g0.show_exemplars=0&g0.range_input=1h&g0.end_input=2019-10-25%2023%3A37%3A00&g0.moment_input=2019-10-25%2023%3A37%3A00&g1.expr=node_filesystem_avail_bytes&g1.tab=1&g1.display_mode=${GraphDisplayMode.Lines}&g1.show_exemplars=0&g1.range_input=1h`; describe('decodePanelOptionsFromQueryString', () => { it('returns [] when query is empty', () => { @@ -246,7 +245,7 @@ describe('Utils', () => { expect(parseOption('expr=foo')).toEqual({ expr: 'foo' }); }); it('should parse stacked', () => { - expect(parseOption('stacked=1')).toEqual({ stacked: true }); + expect(parseOption('stacked=1')).toEqual({ displayMode: GraphDisplayMode.Stacked }); }); it('should parse end_input', () => { expect(parseOption('end_input=2019-10-25%2023%3A37')).toEqual({ endTime: moment.utc('2019-10-25 23:37').valueOf() }); @@ -294,14 +293,16 @@ describe('Utils', () => { options: { expr: 'foo', type: PanelType.Graph, - stacked: true, + displayMode: GraphDisplayMode.Stacked, showExemplars: true, range: 0, endTime: null, resolution: 1, }, }) - ).toEqual('g0.expr=foo&g0.tab=0&g0.stacked=1&g0.show_exemplars=1&g0.range_input=0s&g0.step_input=1'); + ).toEqual( + `g0.expr=foo&g0.tab=0&g0.display_mode=${GraphDisplayMode.Stacked}&g0.show_exemplars=1&g0.range_input=0s&g0.step_input=1` + ); }); }); diff --git a/web/ui/react-app/src/vendor/flot/jquery.flot.heatmap.js b/web/ui/react-app/src/vendor/flot/jquery.flot.heatmap.js new file mode 100644 index 0000000000..29d5c81ef7 --- /dev/null +++ b/web/ui/react-app/src/vendor/flot/jquery.flot.heatmap.js @@ -0,0 +1,195 @@ +/* Flot plugin for rendering heatmap charts. + +Inspired by a similar feature in VictoriaMetrics. +See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3384 for more details. + */ + +import moment from 'moment-timezone'; +import {formatValue} from "../../pages/graph/GraphHelpers"; + +const TOOLTIP_ID = 'heatmap-tooltip'; +const GRADIENT_STEPS = 16; + +(function ($) { + let mouseMoveHandler = null; + + function init(plot) { + plot.hooks.draw.push((plot, ctx) => { + const options = plot.getOptions(); + if (!options.series.heatmap) { + return; + } + + const series = plot.getData(); + const fillPalette = generateGradient("#FDF4EB", "#752E12", GRADIENT_STEPS); + const fills = countsToFills(series.flatMap(s => s.data.map(d => d[1])), fillPalette); + series.forEach((s, i) => drawHeatmap(s, plot, ctx, i, fills)); + }); + + plot.hooks.bindEvents.push((plot, eventHolder) => { + const options = plot.getOptions(); + if (!options.series.heatmap || !options.tooltip.show) { + return; + } + + mouseMoveHandler = (e) => { + removeTooltip(); + const {left: xOffset, top: yOffset} = plot.offset(); + const pos = plot.c2p({left: e.pageX - xOffset, top: e.pageY - yOffset}); + const seriesIdx = Math.floor(pos.y); + const series = plot.getData(); + + for (let i = 0; i < series.length; i++) { + if (seriesIdx !== i) { + continue; + } + + const s = series[i]; + const label = s?.labels?.le || "" + const prevLabel = series[i - 1]?.labels?.le || "" + for (let j = 0; j < s.data.length - 1; j++) { + const [xStartVal, yStartVal] = s.data[j]; + const [xEndVal] = s.data[j + 1]; + const isIncluded = pos.x >= xStartVal && pos.x <= xEndVal; + if (yStartVal && isIncluded) { + showTooltip({ + cssClass: options.tooltip.cssClass, + x: e.pageX, + y: e.pageY, + value: formatValue(yStartVal), + dateTime: [xStartVal, xEndVal].map(t => moment(t).format('YYYY-MM-DD HH:mm:ss Z')), + label: `${prevLabel} - ${label}`, + }); + + break; + } + } + } + } + + $(eventHolder).on('mousemove', mouseMoveHandler); + }); + + plot.hooks.shutdown.push((_plot, eventHolder) => { + removeTooltip(); + $(eventHolder).off("mousemove", mouseMoveHandler); + }); + } + + function showTooltip({x, y, cssClass, value, dateTime, label}) { + const tooltip = document.createElement('div'); + tooltip.id = TOOLTIP_ID + tooltip.className = cssClass; + + const timeHtml = `
${dateTime.join('
')}
` + const labelHtml = `
Bucket: ${label || 'value'}
` + const valueHtml = `
Value: ${value}
` + tooltip.innerHTML = `
${timeHtml}
${labelHtml}${valueHtml}
`; + + tooltip.style.position = 'absolute'; + tooltip.style.top = y + 5 + 'px'; + tooltip.style.left = x + 5 + 'px'; + tooltip.style.display = 'none'; + document.body.appendChild(tooltip); + + const totalTipWidth = $(tooltip).outerWidth(); + const totalTipHeight = $(tooltip).outerHeight(); + + if (x > ($(window).width() - totalTipWidth)) { + x -= totalTipWidth; + tooltip.style.left = x + 'px'; + } + + if (y > ($(window).height() - totalTipHeight)) { + y -= totalTipHeight; + tooltip.style.top = y + 'px'; + } + + tooltip.style.display = 'block'; // This will trigger a re-render, allowing fadeIn to work + tooltip.style.opacity = '1'; + } + + function removeTooltip() { + let tooltip = document.getElementById(TOOLTIP_ID); + if (tooltip) { + document.body.removeChild(tooltip); + } + } + + function drawHeatmap(series, plot, ctx, seriesIndex, fills) { + const {data: dataPoints} = series; + const {left: xOffset, top: yOffset} = plot.getPlotOffset(); + const plotHeight = plot.height(); + const xaxis = plot.getXAxes()[0]; + const cellHeight = plotHeight / plot.getData().length; + + ctx.save(); + ctx.translate(xOffset, yOffset); + + for (let i = 0, len = dataPoints.length - 1; i < len; i++) { + const [xStartVal, countStart] = dataPoints[i]; + const [xEndVal] = dataPoints[i + 1]; + + const xStart = xaxis.p2c(xStartVal); + const xEnd = xaxis.p2c(xEndVal); + const cellWidth = xEnd - xStart; + const yStart = plotHeight - (seriesIndex + 1) * cellHeight; + + ctx.fillStyle = fills[countStart]; + ctx.fillRect(xStart + 0.5, yStart + 0.5, cellWidth - 1, cellHeight - 1); + } + + ctx.restore(); + } + + function countsToFills(counts, fillPalette) { + const hideThreshold = 0; + const minCount = Math.min(...counts.filter(count => count > hideThreshold)); + const maxCount = Math.max(...counts); + const range = maxCount - minCount; + const paletteSize = fillPalette.length; + + return counts.reduce((acc, count) => { + const index = count === 0 + ? -1 + : Math.min(paletteSize - 1, Math.floor((paletteSize * (count - minCount)) / range)); + acc[count] = fillPalette[index] || "transparent"; + return acc; + }, {}); + } + + function generateGradient(color1, color2, steps) { + function interpolateColor(startColor, endColor, step) { + let r = startColor[0] + step * (endColor[0] - startColor[0]); + let g = startColor[1] + step * (endColor[1] - startColor[1]); + let b = startColor[2] + step * (endColor[2] - startColor[2]); + + return `rgb(${Math.round(r)}, ${Math.round(g)}, ${Math.round(b)})`; + } + + function hexToRgb(hex) { + const bigint = parseInt(hex.slice(1), 16); + const r = (bigint >> 16) & 255; + const g = (bigint >> 8) & 255; + const b = bigint & 255; + + return [r, g, b]; + } + + return new Array(steps).fill("").map((_el, i) => { + return interpolateColor(hexToRgb(color1), hexToRgb(color2), i / (steps - 1)); + }); + } + + + jQuery.plot.plugins.push({ + init, + options: { + series: { + heatmap: false + } + }, + name: 'heatmap', + version: '1.0' + }); +})(jQuery); From ccfe14d7e71fc1dcf2be0efc1a5550ec9336907e Mon Sep 17 00:00:00 2001 From: zenador Date: Sat, 25 Nov 2023 07:05:38 +0800 Subject: [PATCH 5/6] PromQL: ignore small errors for bucketQuantile (#13153) promql: Improve histogram_quantile calculation for classic buckets Tiny differences between classic buckets are most likely caused by floating point precision issues. With this commit, relative changes below a certain threshold are ignored. This makes the result of histogram_quantile more meaningful, and also avoids triggering the _input to histogram_quantile needed to be fixed for monotonicity_ annotations in unactionable cases. This commit also adds explanation of the new adjustment and of the monotonicity annotation to the documentation of `histogram_quantile`. --------- Signed-off-by: Jeanette Tan --- docs/querying/functions.md | 18 ++ promql/engine_test.go | 2 +- promql/functions.go | 2 +- promql/quantile.go | 93 +++++++--- promql/quantile_test.go | 318 ++++++++++++++++++++++++++++++++ promql/test.go | 17 +- util/annotations/annotations.go | 2 +- 7 files changed, 416 insertions(+), 36 deletions(-) create mode 100644 promql/quantile_test.go diff --git a/docs/querying/functions.md b/docs/querying/functions.md index 6838bb72b6..00afa1d223 100644 --- a/docs/querying/functions.md +++ b/docs/querying/functions.md @@ -323,6 +323,24 @@ a histogram. You can use `histogram_quantile(1, v instant-vector)` to get the estimated maximum value stored in a histogram. +Buckets of classic histograms are cumulative. Therefore, the following should always be the case: + +- The counts in the buckets are monotonically increasing (strictly non-decreasing). +- A lack of observations between the upper limits of two consecutive buckets results in equal counts +in those two buckets. + +However, floating point precision issues (e.g. small discrepancies introduced by computing of buckets +with `sum(rate(...))`) or invalid data might violate these assumptions. In that case, +`histogram_quantile` would be unable to return meaningful results. To mitigate the issue, +`histogram_quantile` assumes that tiny relative differences between consecutive buckets are happening +because of floating point precision errors and ignores them. (The threshold to ignore a difference +between two buckets is a trillionth (1e-12) of the sum of both buckets.) Furthermore, if there are +non-monotonic bucket counts even after this adjustment, they are increased to the value of the +previous buckets to enforce monotonicity. The latter is evidence for an actual issue with the input +data and is therefore flagged with an informational annotation reading `input to histogram_quantile +needed to be fixed for monotonicity`. If you encounter this annotation, you should find and remove +the source of the invalid data. + ## `histogram_stddev()` and `histogram_stdvar()` _Both functions only act on native histograms, which are an experimental diff --git a/promql/engine_test.go b/promql/engine_test.go index 7532a3294e..d6a10455af 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -3699,7 +3699,7 @@ func TestNativeHistogram_HistogramQuantile(t *testing.T) { require.Len(t, vector, 1) require.Nil(t, vector[0].H) - require.True(t, almostEqual(sc.value, vector[0].F)) + require.True(t, almostEqual(sc.value, vector[0].F, defaultEpsilon)) }) } idx++ diff --git a/promql/functions.go b/promql/functions.go index e9a03406a2..07e439cf25 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -1163,7 +1163,7 @@ func funcHistogramQuantile(vals []parser.Value, args parser.Expressions, enh *Ev for _, mb := range enh.signatureToMetricWithBuckets { if len(mb.buckets) > 0 { - res, forcedMonotonicity := bucketQuantile(q, mb.buckets) + res, forcedMonotonicity, _ := bucketQuantile(q, mb.buckets) enh.Out = append(enh.Out, Sample{ Metric: mb.metric, F: res, diff --git a/promql/quantile.go b/promql/quantile.go index f289448682..f62519f5b9 100644 --- a/promql/quantile.go +++ b/promql/quantile.go @@ -23,6 +23,25 @@ import ( "github.com/prometheus/prometheus/model/labels" ) +// smallDeltaTolerance is the threshold for relative deltas between classic +// histogram buckets that will be ignored by the histogram_quantile function +// because they are most likely artifacts of floating point precision issues. +// Testing on 2 sets of real data with bugs arising from small deltas, +// the safe ranges were from: +// - 1e-05 to 1e-15 +// - 1e-06 to 1e-15 +// Anything to the left of that would cause non-query-sharded data to have +// small deltas ignored (unnecessary and we should avoid this), and anything +// to the right of that would cause query-sharded data to not have its small +// deltas ignored (so the problem won't be fixed). +// For context, query sharding triggers these float precision errors in Mimir. +// To illustrate, with a relative deviation of 1e-12, we need to have 1e12 +// observations in the bucket so that the change of one observation is small +// enough to get ignored. With the usual observation rate even of very busy +// services, this will hardly be reached in timeframes that matters for +// monitoring. +const smallDeltaTolerance = 1e-12 + // Helpers to calculate quantiles. // excludedLabels are the labels to exclude from signature calculation for @@ -72,16 +91,19 @@ type metricWithBuckets struct { // // If q>1, +Inf is returned. // -// We also return a bool to indicate if monotonicity needed to be forced. -func bucketQuantile(q float64, buckets buckets) (float64, bool) { +// We also return a bool to indicate if monotonicity needed to be forced, +// and another bool to indicate if small differences between buckets (that +// are likely artifacts of floating point precision issues) have been +// ignored. +func bucketQuantile(q float64, buckets buckets) (float64, bool, bool) { if math.IsNaN(q) { - return math.NaN(), false + return math.NaN(), false, false } if q < 0 { - return math.Inf(-1), false + return math.Inf(-1), false, false } if q > 1 { - return math.Inf(+1), false + return math.Inf(+1), false, false } slices.SortFunc(buckets, func(a, b bucket) int { // We don't expect the bucket boundary to be a NaN. @@ -94,27 +116,27 @@ func bucketQuantile(q float64, buckets buckets) (float64, bool) { return 0 }) if !math.IsInf(buckets[len(buckets)-1].upperBound, +1) { - return math.NaN(), false + return math.NaN(), false, false } buckets = coalesceBuckets(buckets) - forcedMonotonic := ensureMonotonic(buckets) + forcedMonotonic, fixedPrecision := ensureMonotonicAndIgnoreSmallDeltas(buckets, smallDeltaTolerance) if len(buckets) < 2 { - return math.NaN(), false + return math.NaN(), false, false } observations := buckets[len(buckets)-1].count if observations == 0 { - return math.NaN(), false + return math.NaN(), false, false } rank := q * observations b := sort.Search(len(buckets)-1, func(i int) bool { return buckets[i].count >= rank }) if b == len(buckets)-1 { - return buckets[len(buckets)-2].upperBound, forcedMonotonic + return buckets[len(buckets)-2].upperBound, forcedMonotonic, fixedPrecision } if b == 0 && buckets[0].upperBound <= 0 { - return buckets[0].upperBound, forcedMonotonic + return buckets[0].upperBound, forcedMonotonic, fixedPrecision } var ( bucketStart float64 @@ -126,7 +148,7 @@ func bucketQuantile(q float64, buckets buckets) (float64, bool) { count -= buckets[b-1].count rank -= buckets[b-1].count } - return bucketStart + (bucketEnd-bucketStart)*(rank/count), forcedMonotonic + return bucketStart + (bucketEnd-bucketStart)*(rank/count), forcedMonotonic, fixedPrecision } // histogramQuantile calculates the quantile 'q' based on the given histogram. @@ -348,6 +370,7 @@ func coalesceBuckets(buckets buckets) buckets { // - Ingestion via the remote write receiver that Prometheus implements. // - Optimisation of query execution where precision is sacrificed for other // benefits, not by Prometheus but by systems built on top of it. +// - Circumstances where floating point precision errors accumulate. // // Monotonicity is usually guaranteed because if a bucket with upper bound // u1 has count c1, then any bucket with a higher upper bound u > u1 must @@ -357,22 +380,42 @@ func coalesceBuckets(buckets buckets) buckets { // bucket with the φ-quantile count, so breaking the monotonicity // guarantee causes bucketQuantile() to return undefined (nonsense) results. // -// As a somewhat hacky solution, we calculate the "envelope" of the histogram -// buckets, essentially removing any decreases in the count between successive -// buckets. We return a bool to indicate if this monotonicity was forced or not. -func ensureMonotonic(buckets buckets) bool { - forced := false - max := buckets[0].count +// As a somewhat hacky solution, we first silently ignore any numerically +// insignificant (relative delta below the requested tolerance and likely to +// be from floating point precision errors) differences between successive +// buckets regardless of the direction. Then we calculate the "envelope" of +// the histogram buckets, essentially removing any decreases in the count +// between successive buckets. +// +// We return a bool to indicate if this monotonicity was forced or not, and +// another bool to indicate if small deltas were ignored or not. +func ensureMonotonicAndIgnoreSmallDeltas(buckets buckets, tolerance float64) (bool, bool) { + var forcedMonotonic, fixedPrecision bool + prev := buckets[0].count for i := 1; i < len(buckets); i++ { - switch { - case buckets[i].count > max: - max = buckets[i].count - case buckets[i].count < max: - buckets[i].count = max - forced = true + curr := buckets[i].count // Assumed always positive. + if curr == prev { + // No correction needed if the counts are identical between buckets. + continue } + if almostEqual(prev, curr, tolerance) { + // Silently correct numerically insignificant differences from floating + // point precision errors, regardless of direction. + // Do not update the 'prev' value as we are ignoring the difference. + buckets[i].count = prev + fixedPrecision = true + continue + } + if curr < prev { + // Force monotonicity by removing any decreases regardless of magnitude. + // Do not update the 'prev' value as we are ignoring the decrease. + buckets[i].count = prev + forcedMonotonic = true + continue + } + prev = curr } - return forced + return forcedMonotonic, fixedPrecision } // quantile calculates the given quantile of a vector of samples. diff --git a/promql/quantile_test.go b/promql/quantile_test.go new file mode 100644 index 0000000000..84f4c0b06d --- /dev/null +++ b/promql/quantile_test.go @@ -0,0 +1,318 @@ +// Copyright 2023 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 promql + +import ( + "math" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestBucketQuantile_ForcedMonotonicity(t *testing.T) { + eps := 1e-12 + + for name, tc := range map[string]struct { + getInput func() buckets // The buckets can be modified in-place so return a new one each time. + expectedForced bool + expectedFixed bool + expectedValues map[float64]float64 + }{ + "simple - monotonic": { + getInput: func() buckets { + return buckets{ + { + upperBound: 10, + count: 10, + }, { + upperBound: 15, + count: 15, + }, { + upperBound: 20, + count: 15, + }, { + upperBound: 30, + count: 15, + }, { + upperBound: math.Inf(1), + count: 15, + }, + } + }, + expectedForced: false, + expectedFixed: false, + expectedValues: map[float64]float64{ + 1: 15., + 0.99: 14.85, + 0.9: 13.5, + 0.5: 7.5, + }, + }, + "simple - non-monotonic middle": { + getInput: func() buckets { + return buckets{ + { + upperBound: 10, + count: 10, + }, { + upperBound: 15, + count: 15, + }, { + upperBound: 20, + count: 15.00000000001, // Simulate the case there's a small imprecision in float64. + }, { + upperBound: 30, + count: 15, + }, { + upperBound: math.Inf(1), + count: 15, + }, + } + }, + expectedForced: false, + expectedFixed: true, + expectedValues: map[float64]float64{ + 1: 15., + 0.99: 14.85, + 0.9: 13.5, + 0.5: 7.5, + }, + }, + "real example - monotonic": { + getInput: func() buckets { + return buckets{ + { + upperBound: 1, + count: 6454661.3014166197, + }, { + upperBound: 5, + count: 8339611.2001912938, + }, { + upperBound: 10, + count: 14118319.2444762159, + }, { + upperBound: 25, + count: 14130031.5272856522, + }, { + upperBound: 50, + count: 46001270.3030008152, + }, { + upperBound: 64, + count: 46008473.8585563600, + }, { + upperBound: 80, + count: 46008473.8585563600, + }, { + upperBound: 100, + count: 46008473.8585563600, + }, { + upperBound: 250, + count: 46008473.8585563600, + }, { + upperBound: 1000, + count: 46008473.8585563600, + }, { + upperBound: math.Inf(1), + count: 46008473.8585563600, + }, + } + }, + expectedForced: false, + expectedFixed: false, + expectedValues: map[float64]float64{ + 1: 64., + 0.99: 49.64475715376406, + 0.9: 46.39671690938454, + 0.5: 31.96098248992002, + }, + }, + "real example - non-monotonic": { + getInput: func() buckets { + return buckets{ + { + upperBound: 1, + count: 6454661.3014166225, + }, { + upperBound: 5, + count: 8339611.2001912957, + }, { + upperBound: 10, + count: 14118319.2444762159, + }, { + upperBound: 25, + count: 14130031.5272856504, + }, { + upperBound: 50, + count: 46001270.3030008227, + }, { + upperBound: 64, + count: 46008473.8585563824, + }, { + upperBound: 80, + count: 46008473.8585563898, + }, { + upperBound: 100, + count: 46008473.8585563824, + }, { + upperBound: 250, + count: 46008473.8585563824, + }, { + upperBound: 1000, + count: 46008473.8585563898, + }, { + upperBound: math.Inf(1), + count: 46008473.8585563824, + }, + } + }, + expectedForced: false, + expectedFixed: true, + expectedValues: map[float64]float64{ + 1: 64., + 0.99: 49.64475715376406, + 0.9: 46.39671690938454, + 0.5: 31.96098248992002, + }, + }, + "real example 2 - monotonic": { + getInput: func() buckets { + return buckets{ + { + upperBound: 0.005, + count: 9.6, + }, { + upperBound: 0.01, + count: 9.688888889, + }, { + upperBound: 0.025, + count: 9.755555556, + }, { + upperBound: 0.05, + count: 9.844444444, + }, { + upperBound: 0.1, + count: 9.888888889, + }, { + upperBound: 0.25, + count: 9.888888889, + }, { + upperBound: 0.5, + count: 9.888888889, + }, { + upperBound: 1, + count: 9.888888889, + }, { + upperBound: 2.5, + count: 9.888888889, + }, { + upperBound: 5, + count: 9.888888889, + }, { + upperBound: 10, + count: 9.888888889, + }, { + upperBound: 25, + count: 9.888888889, + }, { + upperBound: 50, + count: 9.888888889, + }, { + upperBound: 100, + count: 9.888888889, + }, { + upperBound: math.Inf(1), + count: 9.888888889, + }, + } + }, + expectedForced: false, + expectedFixed: false, + expectedValues: map[float64]float64{ + 1: 0.1, + 0.99: 0.03468750000281261, + 0.9: 0.00463541666671875, + 0.5: 0.0025752314815104174, + }, + }, + "real example 2 - non-monotonic": { + getInput: func() buckets { + return buckets{ + { + upperBound: 0.005, + count: 9.6, + }, { + upperBound: 0.01, + count: 9.688888889, + }, { + upperBound: 0.025, + count: 9.755555556, + }, { + upperBound: 0.05, + count: 9.844444444, + }, { + upperBound: 0.1, + count: 9.888888889, + }, { + upperBound: 0.25, + count: 9.888888889, + }, { + upperBound: 0.5, + count: 9.888888889, + }, { + upperBound: 1, + count: 9.888888889, + }, { + upperBound: 2.5, + count: 9.888888889, + }, { + upperBound: 5, + count: 9.888888889, + }, { + upperBound: 10, + count: 9.888888889001, // Simulate the case there's a small imprecision in float64. + }, { + upperBound: 25, + count: 9.888888889, + }, { + upperBound: 50, + count: 9.888888888999, // Simulate the case there's a small imprecision in float64. + }, { + upperBound: 100, + count: 9.888888889, + }, { + upperBound: math.Inf(1), + count: 9.888888889, + }, + } + }, + expectedForced: false, + expectedFixed: true, + expectedValues: map[float64]float64{ + 1: 0.1, + 0.99: 0.03468750000281261, + 0.9: 0.00463541666671875, + 0.5: 0.0025752314815104174, + }, + }, + } { + t.Run(name, func(t *testing.T) { + for q, v := range tc.expectedValues { + res, forced, fixed := bucketQuantile(q, tc.getInput()) + require.Equal(t, tc.expectedForced, forced) + require.Equal(t, tc.expectedFixed, fixed) + require.InEpsilon(t, v, res, eps) + } + }) + } +} diff --git a/promql/test.go b/promql/test.go index f6a31ee431..7274a8f0d9 100644 --- a/promql/test.go +++ b/promql/test.go @@ -49,7 +49,7 @@ var ( ) const ( - epsilon = 0.000001 // Relative error allowed for sample values. + defaultEpsilon = 0.000001 // Relative error allowed for sample values. ) var testStartTime = time.Unix(0, 0).UTC() @@ -440,7 +440,7 @@ func (ev *evalCmd) compareResult(result parser.Value) error { if (expH == nil) != (v.H == nil) || (expH != nil && !expH.Equals(v.H)) { return fmt.Errorf("expected %v for %s but got %s", HistogramTestExpression(expH), v.Metric, HistogramTestExpression(v.H)) } - if !almostEqual(exp0.Value, v.F) { + if !almostEqual(exp0.Value, v.F, defaultEpsilon) { return fmt.Errorf("expected %v for %s but got %v", exp0.Value, v.Metric, v.F) } @@ -464,7 +464,7 @@ func (ev *evalCmd) compareResult(result parser.Value) error { if exp0.Histogram != nil { return fmt.Errorf("expected Histogram %v but got scalar %s", exp0.Histogram.TestExpression(), val.String()) } - if !almostEqual(exp0.Value, val.V) { + if !almostEqual(exp0.Value, val.V, defaultEpsilon) { return fmt.Errorf("expected Scalar %v but got %v", val.V, exp0.Value) } @@ -663,9 +663,9 @@ func (t *test) clear() { t.context, t.cancelCtx = context.WithCancel(context.Background()) } -// samplesAlmostEqual returns true if the two sample lines only differ by a -// small relative error in their sample value. -func almostEqual(a, b float64) bool { +// almostEqual returns true if a and b differ by less than their sum +// multiplied by epsilon. +func almostEqual(a, b, epsilon float64) bool { // NaN has no equality but for testing we still want to know whether both values // are NaN. if math.IsNaN(a) && math.IsNaN(b) { @@ -677,12 +677,13 @@ func almostEqual(a, b float64) bool { return true } + absSum := math.Abs(a) + math.Abs(b) diff := math.Abs(a - b) - if a == 0 || b == 0 || diff < minNormal { + if a == 0 || b == 0 || absSum < minNormal { return diff < epsilon*minNormal } - return diff/(math.Abs(a)+math.Abs(b)) < epsilon + return diff/math.Min(absSum, math.MaxFloat64) < epsilon } func parseNumber(s string) (float64, error) { diff --git a/util/annotations/annotations.go b/util/annotations/annotations.go index fa4983fc9f..2041d02178 100644 --- a/util/annotations/annotations.go +++ b/util/annotations/annotations.go @@ -108,7 +108,7 @@ var ( MixedClassicNativeHistogramsWarning = fmt.Errorf("%w: vector contains a mix of classic and native histograms for metric name", PromQLWarning) PossibleNonCounterInfo = fmt.Errorf("%w: metric might not be a counter, name does not end in _total/_sum/_count/_bucket:", PromQLInfo) - HistogramQuantileForcedMonotonicityInfo = fmt.Errorf("%w: input to histogram_quantile needed to be fixed for monotonicity (and may give inaccurate results) for metric name", PromQLInfo) + HistogramQuantileForcedMonotonicityInfo = fmt.Errorf("%w: input to histogram_quantile needed to be fixed for monotonicity (see https://prometheus.io/docs/prometheus/latest/querying/functions/#histogram_quantile) for metric name", PromQLInfo) ) type annoErr struct { From ecc37588b00c5ec1d4f2349266a23b01f6c91743 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Mon, 27 Nov 2023 16:40:30 +0100 Subject: [PATCH 6/6] tsdb: seriesHashmap.set by making receiver a pointer (#13193) * Fix tsdb.seriesHashmap.set by making receiver a pointer The method tsdb.seriesHashmap.set currently doesn't set the conflicts field properly, due to the receiver being a non-pointer. Fix by turning the receiver into a pointer, and add a corresponding regression test. Signed-off-by: Arve Knudsen --- tsdb/head.go | 4 ++-- tsdb/head_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/tsdb/head.go b/tsdb/head.go index bf181a4158..25209cd01b 100644 --- a/tsdb/head.go +++ b/tsdb/head.go @@ -1718,7 +1718,7 @@ func (m *seriesHashmap) get(hash uint64, lset labels.Labels) *memSeries { return nil } -func (m seriesHashmap) set(hash uint64, s *memSeries) { +func (m *seriesHashmap) set(hash uint64, s *memSeries) { if existing, found := m.unique[hash]; !found || labels.Equal(existing.lset, s.lset) { m.unique[hash] = s return @@ -1736,7 +1736,7 @@ func (m seriesHashmap) set(hash uint64, s *memSeries) { m.conflicts[hash] = append(l, s) } -func (m seriesHashmap) del(hash uint64, lset labels.Labels) { +func (m *seriesHashmap) del(hash uint64, lset labels.Labels) { var rem []*memSeries unique, found := m.unique[hash] switch { diff --git a/tsdb/head_test.go b/tsdb/head_test.go index f2325039a4..64237e76a7 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -5542,3 +5542,54 @@ func TestHeadCompactionWhileAppendAndCommitExemplar(t *testing.T) { app.Commit() h.Close() } + +func labelsWithHashCollision() (labels.Labels, labels.Labels) { + // These two series have the same XXHash; thanks to https://github.com/pstibrany/labels_hash_collisions + ls1 := labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "l6CQ5y") + ls2 := labels.FromStrings("__name__", "metric", "lbl1", "value", "lbl2", "v7uDlF") + + if ls1.Hash() != ls2.Hash() { + // These ones are the same when using -tags stringlabels + ls1 = labels.FromStrings("__name__", "metric", "lbl", "HFnEaGl") + ls2 = labels.FromStrings("__name__", "metric", "lbl", "RqcXatm") + } + + if ls1.Hash() != ls2.Hash() { + panic("This code needs to be updated: find new labels with colliding hash values.") + } + + return ls1, ls2 +} + +func TestStripeSeries_getOrSet(t *testing.T) { + lbls1, lbls2 := labelsWithHashCollision() + ms1 := memSeries{ + lset: lbls1, + } + ms2 := memSeries{ + lset: lbls2, + } + hash := lbls1.Hash() + s := newStripeSeries(1, noopSeriesLifecycleCallback{}) + + got, created, err := s.getOrSet(hash, lbls1, func() *memSeries { + return &ms1 + }) + require.NoError(t, err) + require.True(t, created) + require.Same(t, &ms1, got) + + // Add a conflicting series + got, created, err = s.getOrSet(hash, lbls2, func() *memSeries { + return &ms2 + }) + require.NoError(t, err) + require.True(t, created) + require.Same(t, &ms2, got) + + // Verify that we can get both of the series despite the hash collision + got = s.getByHash(hash, lbls1) + require.Same(t, &ms1, got) + got = s.getByHash(hash, lbls2) + require.Same(t, &ms2, got) +}