Skip to content

Commit

Permalink
Merge pull request #1672 from imorph/fix_summary_decay2
Browse files Browse the repository at this point in the history
fix: use injected now() instead of time.Now() in summary methods
  • Loading branch information
ArthurSens authored Nov 3, 2024
2 parents d3f2840 + 6d099da commit 0c73c1c
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 17 deletions.
7 changes: 5 additions & 2 deletions prometheus/summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -280,6 +281,8 @@ type summary struct {

desc *Desc

now func() time.Time

objectives map[float64]float64
sortedObjectives []float64

Expand Down Expand Up @@ -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)
}
Expand All @@ -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()
Expand Down
27 changes: 12 additions & 15 deletions prometheus/summary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,39 +371,36 @@ 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)
Expand Down

0 comments on commit 0c73c1c

Please sign in to comment.