Fixing race leading to unbounded stats retention

inmem.go

@@ -189,38 +189,49 @@ func (i *InmemSink) Data() []*IntervalMetrics {
 	return intervals
 }
 
-// getInterval returns the current interval to write to
-func (i *InmemSink) getInterval() *IntervalMetrics {
-	intv := time.Now().Truncate(i.interval)
-
-	// Check if the last interval is a match
+func (i *InmemSink) getExistingInterval(intv time.Time) *IntervalMetrics {
 	i.intervalLock.RLock()
+	defer i.intervalLock.RUnlock()
+
 	n := len(i.intervals)
 	if n > 0 && i.intervals[n-1].Interval == intv {
-		i.intervalLock.RUnlock()
 		return i.intervals[n-1]
 	}
-	i.intervalLock.RUnlock()
+	return nil
+}
 
-	// Create a new interval
+func (i *InmemSink) createInterval(intv time.Time) *IntervalMetrics {
 	i.intervalLock.Lock()
 	defer i.intervalLock.Unlock()
 
-	// Create a new interval
+	// Check for an existing interval
+	n := len(i.intervals)
+	if n > 0 && i.intervals[n-1].Interval == intv {
+		return i.intervals[n-1]
+	}
+
+	// Add the current interval
 	current := NewIntervalMetrics(intv)
+	i.intervals = append(i.intervals, current)
+	n++
 
-	// If we're at the max intervals, remove the last interval
-	if n == i.maxIntervals {
-		copy(i.intervals[0:], i.intervals[1:])
-		i.intervals[n-1] = current
-	} else {
-		i.intervals = append(i.intervals, current)
+	// Truncate the intervals if they are too long
+	if n >= i.maxIntervals {
+		copy(i.intervals[0:], i.intervals[n-i.maxIntervals:])
+		i.intervals = i.intervals[:i.maxIntervals]
 	}
-
-	// Return the new interval
 	return current
 }
 
+// getInterval returns the current interval to write to
+func (i *InmemSink) getInterval() *IntervalMetrics {
+	intv := time.Now().Truncate(i.interval)
+	if m := i.getExistingInterval(intv); m != nil {
+		return m
+	}
+	return i.createInterval(intv)
+}
+
 // Flattens the key for formatting, removes spaces
 func (i *InmemSink) flattenKey(parts []string) string {
 	joined := strings.Join(parts, ".")

inmem_test.go

@@ -69,11 +69,11 @@ func TestInmemSink(t *testing.T) {
 
 	intvM.RUnlock()
 
-	for i := 1; i < 5; i++ {
+	for i := 1; i < 10; i++ {
 		time.Sleep(10 * time.Millisecond)
 		inm.SetGauge([]string{"foo", "bar"}, 42)
 		data = inm.Data()
-		if len(data) != i+1 {
+		if len(data) != min(i+1, 5) {
 			t.Fatalf("bad: %v", data)
 		}
 	}
@@ -86,3 +86,10 @@ func TestInmemSink(t *testing.T) {
 		t.Fatalf("bad: %v", data)
 	}
 }
+
+func min(a, b int) int {
+	if a < b {
+		return a
+	}
+	return b
+}

metrics_test.go

@@ -202,45 +202,52 @@ func TestMetrics_EmitRuntimeStats(t *testing.T) {
 		t.Fatalf("bad val: %v", m.vals)
 	}
 
-	if m.keys[2][0] != "runtime" || m.keys[2][1] != "malloc_count" {
+	if m.keys[2][0] != "runtime" || m.keys[2][1] != "sys_bytes" {
 		t.Fatalf("bad key %v", m.keys)
 	}
-	if m.vals[2] <= 100 {
+	if m.vals[2] <= 100000 {
 		t.Fatalf("bad val: %v", m.vals)
 	}
 
-	if m.keys[3][0] != "runtime" || m.keys[3][1] != "free_count" {
+	if m.keys[3][0] != "runtime" || m.keys[3][1] != "malloc_count" {
 		t.Fatalf("bad key %v", m.keys)
 	}
 	if m.vals[3] <= 100 {
 		t.Fatalf("bad val: %v", m.vals)
 	}
 
-	if m.keys[4][0] != "runtime" || m.keys[4][1] != "heap_objects" {
+	if m.keys[4][0] != "runtime" || m.keys[4][1] != "free_count" {
 		t.Fatalf("bad key %v", m.keys)
 	}
-	if m.vals[4] <= 200 {
+	if m.vals[4] <= 100 {
 		t.Fatalf("bad val: %v", m.vals)
 	}
 
-	if m.keys[5][0] != "runtime" || m.keys[5][1] != "total_gc_pause_ns" {
+	if m.keys[5][0] != "runtime" || m.keys[5][1] != "heap_objects" {
 		t.Fatalf("bad key %v", m.keys)
 	}
-	if m.vals[5] <= 100000 {
+	if m.vals[5] <= 200 {
 		t.Fatalf("bad val: %v", m.vals)
 	}
 
-	if m.keys[6][0] != "runtime" || m.keys[6][1] != "total_gc_runs" {
+	if m.keys[6][0] != "runtime" || m.keys[6][1] != "total_gc_pause_ns" {
 		t.Fatalf("bad key %v", m.keys)
 	}
-	if m.vals[6] <= 1 {
+	if m.vals[6] <= 100000 {
 		t.Fatalf("bad val: %v", m.vals)
 	}
 
-	if m.keys[7][0] != "runtime" || m.keys[7][1] != "gc_pause_ns" {
+	if m.keys[7][0] != "runtime" || m.keys[7][1] != "total_gc_runs" {
 		t.Fatalf("bad key %v", m.keys)
 	}
-	if m.vals[7] <= 1000 {
+	if m.vals[7] <= 1 {
+		t.Fatalf("bad val: %v", m.vals)
+	}
+
+	if m.keys[8][0] != "runtime" || m.keys[8][1] != "gc_pause_ns" {
+		t.Fatalf("bad key %v", m.keys)
+	}
+	if m.vals[8] <= 1000 {
 		t.Fatalf("bad val: %v", m.vals)
 	}
 }