Skip to content

Commit 2754a4c

Browse files
committed
fix for comments
Signed-off-by: Ziqi Zhao <[email protected]>
1 parent d8c7074 commit 2754a4c

File tree

2 files changed

+21
-28
lines changed

2 files changed

+21
-28
lines changed

prometheus/histogram.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -761,8 +761,9 @@ func (h *histogram) Observe(v float64) {
761761
h.observe(v, h.findBucket(v))
762762
}
763763

764-
// ObserveWithExemplar should not be called in high-frequency settings,
765-
// since it isn't lock-free for native histograms with configured exemplars.
764+
// ObserveWithExemplar should not be called in a high-frequency setting
765+
// for a native histogram with configured exemplars. For this case,
766+
// the implementation isn't lock-free and might suffer from lock contention.
766767
func (h *histogram) ObserveWithExemplar(v float64, e Labels) {
767768
i := h.findBucket(v)
768769
h.observe(v, i)
@@ -843,6 +844,8 @@ func (h *histogram) Write(out *dto.Metric) error {
843844
}}
844845
}
845846

847+
// If exemplars are not configured, the cap will be 0.
848+
// So append is not needed in this case.
846849
if cap(h.nativeExemplars.exemplars) > 0 {
847850
h.nativeExemplars.Lock()
848851
his.Exemplars = append(his.Exemplars, h.nativeExemplars.exemplars...)
@@ -1709,7 +1712,7 @@ func (n *nativeExemplars) addExemplar(e *dto.Exemplar) {
17091712
nIdx = len(n.exemplars)
17101713
}
17111714

1712-
if otIdx != -1 && time.Since(ot) > n.ttl {
1715+
if otIdx != -1 && e.Timestamp.AsTime().Sub(ot) > n.ttl {
17131716
rIdx = otIdx
17141717
} else {
17151718
// In the previous for loop, when calculating the closest pair of exemplars,

prometheus/histogram_test.go

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1314,7 +1314,7 @@ func TestNativeHistogramExemplar(t *testing.T) {
13141314
{
13151315
name: "remove exemplar with oldest timestamp, the removed index is smaller than inserted index",
13161316
addFunc: func(h *histogram) {
1317-
time.Sleep(10 * time.Second)
1317+
h.now = func() time.Time { return time.Now().Add(time.Second * 11) }
13181318
h.ObserveWithExemplar(6, Labels{"id": "1"})
13191319
},
13201320
expectedValues: []float64{0, 4, 6},
@@ -1324,14 +1324,7 @@ func TestNativeHistogramExemplar(t *testing.T) {
13241324
for _, tc := range tcs {
13251325
t.Run(tc.name, func(t *testing.T) {
13261326
tc.addFunc(h)
1327-
if len(h.nativeExemplars.exemplars) != len(tc.expectedValues) {
1328-
t.Errorf("the count of exemplars is not %d", len(tc.expectedValues))
1329-
}
1330-
for i, e := range h.nativeExemplars.exemplars {
1331-
if e.GetValue() != tc.expectedValues[i] {
1332-
t.Errorf("the %dth exemplar value %v is not as expected: %v", i, e.GetValue(), tc.expectedValues[i])
1333-
}
1334-
}
1327+
compareNativeExemplarValues(t, h.nativeExemplars.exemplars, tc.expectedValues)
13351328
})
13361329
}
13371330

@@ -1385,14 +1378,7 @@ func TestNativeHistogramExemplar(t *testing.T) {
13851378
for _, tc := range tcs {
13861379
t.Run(tc.name, func(t *testing.T) {
13871380
tc.addFunc(h)
1388-
if len(h.nativeExemplars.exemplars) != len(tc.expectedValues) {
1389-
t.Errorf("the count of exemplars is not %d", len(tc.expectedValues))
1390-
}
1391-
for i, e := range h.nativeExemplars.exemplars {
1392-
if e.GetValue() != tc.expectedValues[i] {
1393-
t.Errorf("the %dth exemplar value %v is not as expected: %v", i, e.GetValue(), tc.expectedValues[i])
1394-
}
1395-
}
1381+
compareNativeExemplarValues(t, h.nativeExemplars.exemplars, tc.expectedValues)
13961382
})
13971383
}
13981384

@@ -1425,14 +1411,18 @@ func TestNativeHistogramExemplar(t *testing.T) {
14251411
for _, tc := range tcs {
14261412
t.Run(tc.name, func(t *testing.T) {
14271413
tc.addFunc(h)
1428-
if len(h.nativeExemplars.exemplars) != len(tc.expectedValues) {
1429-
t.Errorf("the count of exemplars is not %d", len(tc.expectedValues))
1430-
}
1431-
for i, e := range h.nativeExemplars.exemplars {
1432-
if e.GetValue() != tc.expectedValues[i] {
1433-
t.Errorf("the %dth exemplar value %v is not as expected: %v", i, e.GetValue(), tc.expectedValues[i])
1434-
}
1435-
}
1414+
compareNativeExemplarValues(t, h.nativeExemplars.exemplars, tc.expectedValues)
14361415
})
14371416
}
14381417
}
1418+
1419+
func compareNativeExemplarValues(t *testing.T, exps []*dto.Exemplar, values []float64) {
1420+
if len(exps) != len(values) {
1421+
t.Errorf("the count of exemplars is not %d", len(values))
1422+
}
1423+
for i, e := range exps {
1424+
if e.GetValue() != values[i] {
1425+
t.Errorf("the %dth exemplar value %v is not as expected: %v", i, e.GetValue(), values[i])
1426+
}
1427+
}
1428+
}

0 commit comments

Comments
 (0)