From 7877776aa6c688d3667977db54f9e2f04af1d8df Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sat, 2 Nov 2024 10:49:17 +0100 Subject: [PATCH 1/2] fix: use injected now() instead of time.Now() in summary methods Signed-off-by: Ivan Goncharov --- prometheus/summary.go | 7 +++++-- prometheus/summary_test.go | 25 +++++++++++++------------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/prometheus/summary.go b/prometheus/summary.go index 1ab0e47..ac5203c 100644 --- a/prometheus/summary.go +++ b/prometheus/summary.go @@ -243,6 +243,7 @@ func newSummary(desc *Desc, opts SummaryOpts, labelValues ...string) Summary { s := &summary{ desc: desc, + now: opts.now, objectives: opts.Objectives, sortedObjectives: make([]float64, 0, len(opts.Objectives)), @@ -280,6 +281,8 @@ type summary struct { desc *Desc + now func() time.Time + objectives map[float64]float64 sortedObjectives []float64 @@ -307,7 +310,7 @@ func (s *summary) Observe(v float64) { s.bufMtx.Lock() defer s.bufMtx.Unlock() - now := time.Now() + now := s.now() if now.After(s.hotBufExpTime) { s.asyncFlush(now) } @@ -326,7 +329,7 @@ func (s *summary) Write(out *dto.Metric) error { s.bufMtx.Lock() s.mtx.Lock() // Swap bufs even if hotBuf is empty to set new hotBufExpTime. - s.swapBufs(time.Now()) + s.swapBufs(s.now()) s.bufMtx.Unlock() s.flushColdBuf() diff --git a/prometheus/summary_test.go b/prometheus/summary_test.go index 6887930..c3a7a3e 100644 --- a/prometheus/summary_test.go +++ b/prometheus/summary_test.go @@ -373,37 +373,38 @@ func TestSummaryVecConcurrency(t *testing.T) { func TestSummaryDecay(t *testing.T) { if testing.Short() { t.Skip("Skipping test in short mode.") - // More because it depends on timing than because it is particularly long... } + now := time.Now() + sum := NewSummary(SummaryOpts{ Name: "test_summary", Help: "helpless", MaxAge: 100 * time.Millisecond, Objectives: map[float64]float64{0.1: 0.001}, AgeBuckets: 10, + now: func() time.Time { + return now + }, }) m := &dto.Metric{} - i := 0 - tick := time.NewTicker(time.Millisecond) - for range tick.C { - i++ + for i := 1; i <= 1000; i++ { + now = now.Add(time.Millisecond) sum.Observe(float64(i)) if i%10 == 0 { sum.Write(m) - if got, want := *m.Summary.Quantile[0].Value, math.Max(float64(i)/10, float64(i-90)); math.Abs(got-want) > 20 { + got := *m.Summary.Quantile[0].Value + want := math.Max(float64(i)/10, float64(i-90)) + if math.Abs(got-want) > 20 { t.Errorf("%d. got %f, want %f", i, got, want) } m.Reset() } - if i >= 1000 { - break - } } - tick.Stop() - // Wait for MaxAge without observations and make sure quantiles are NaN. - time.Sleep(100 * time.Millisecond) + + // Simulate waiting for MaxAge without observations + now = now.Add(100 * time.Millisecond) sum.Write(m) if got := *m.Summary.Quantile[0].Value; !math.IsNaN(got) { t.Errorf("got %f, want NaN after expiration", got) From 6d099da971b147580e843571a37facd417b2017d Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sun, 3 Nov 2024 14:12:24 +0100 Subject: [PATCH 2/2] add: no skip in short mode for TestSummaryDecay Signed-off-by: Ivan Goncharov --- prometheus/summary_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/prometheus/summary_test.go b/prometheus/summary_test.go index c3a7a3e..4402372 100644 --- a/prometheus/summary_test.go +++ b/prometheus/summary_test.go @@ -371,10 +371,6 @@ func TestSummaryVecConcurrency(t *testing.T) { } func TestSummaryDecay(t *testing.T) { - if testing.Short() { - t.Skip("Skipping test in short mode.") - } - now := time.Now() sum := NewSummary(SummaryOpts{