go/opentelemetry-go
1
0
Fork 0
mirror of https://github.com/open-telemetry/opentelemetry-go.git synced 2026-06-03 18:35:08 +02:00
Code Issues Releases Activity

Fix guard of measured value to not record empty (#4452)

Browse Source 
A guard was added in #4446 to prevent non-normal float64 from being
recorded. This was added in the low-level `record` method meaning that
the higher-level `measure` method will still keep a record of the
invalid value measurement, just with a zero-value.

This fixes that issue by moving the guard to the `measure` method.
This commit is contained in:
Tyler Yahn
2023-08-18 06:43:09 -07:00
committed by GitHub
parent 9b47674bc5
commit 16ce491d38
2 changed files with 15 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sdk/metric/internal/aggregate/exponential_histogram.go
+5 -5
View File
@@ -76,11 +76,6 @@ func newExpoHistogramDataPoint[N int64 | float64](maxSize, maxScale int, noMinMa
// record adds a new measurement to the histogram. It will rescale the buckets if needed. // record adds a new measurement to the histogram. It will rescale the buckets if needed.
func (p *expoHistogramDataPoint[N]) record(v N) { func (p *expoHistogramDataPoint[N]) record(v N) {
// Ignore NaN and infinity.
if math.IsInf(float64(v), 0) || math.IsNaN(float64(v)) {
return
}
p.count++ p.count++
if !p.noMinMax { if !p.noMinMax {
@@ -321,6 +316,11 @@ type expoHistogram[N int64 | float64] struct {
} }
func (e *expoHistogram[N]) measure(_ context.Context, value N, attr attribute.Set) { func (e *expoHistogram[N]) measure(_ context.Context, value N, attr attribute.Set) {
// Ignore NaN and infinity.
if math.IsInf(float64(value), 0) || math.IsNaN(float64(value)) {
return
}
e.valuesMu.Lock() e.valuesMu.Lock()
defer e.valuesMu.Unlock() defer e.valuesMu.Unlock()
sdk/metric/internal/aggregate/exponential_histogram_test.go
+10 -8
View File
@@ -45,10 +45,10 @@ func withHandler(t *testing.T) func() {
func TestExpoHistogramDataPointRecord(t *testing.T) { func TestExpoHistogramDataPointRecord(t *testing.T) {
t.Run("float64", testExpoHistogramDataPointRecord[float64]) t.Run("float64", testExpoHistogramDataPointRecord[float64])
t.Run("float64 MinMaxSum", testExpoHistogramDataPointRecordMinMaxSumFloat64) t.Run("float64 MinMaxSum", testExpoHistogramMinMaxSumFloat64)
t.Run("float64-2", testExpoHistogramDataPointRecordFloat64) t.Run("float64-2", testExpoHistogramDataPointRecordFloat64)
t.Run("int64", testExpoHistogramDataPointRecord[int64]) t.Run("int64", testExpoHistogramDataPointRecord[int64])
t.Run("int64 MinMaxSum", testExpoHistogramDataPointRecordMinMaxSumInt64) t.Run("int64 MinMaxSum", testExpoHistogramMinMaxSumInt64)
} }
// TODO: This can be defined in the test after we drop support for go1.19. // TODO: This can be defined in the test after we drop support for go1.19.
@@ -171,7 +171,7 @@ type expoHistogramDataPointRecordMinMaxSumTestCase[N int64 | float64] struct {
expected expectedMinMaxSum[N] expected expectedMinMaxSum[N]
} }
func testExpoHistogramDataPointRecordMinMaxSumInt64(t *testing.T) { func testExpoHistogramMinMaxSumInt64(t *testing.T) {
testCases := []expoHistogramDataPointRecordMinMaxSumTestCase[int64]{ testCases := []expoHistogramDataPointRecordMinMaxSumTestCase[int64]{
{ {
values: []int64{2, 4, 1}, values: []int64{2, 4, 1},
@@ -188,10 +188,11 @@ func testExpoHistogramDataPointRecordMinMaxSumInt64(t *testing.T) {
restore := withHandler(t) restore := withHandler(t)
defer restore() defer restore()
dp := newExpoHistogramDataPoint[int64](4, 20, false, false) h := newExponentialHistogram[int64](4, 20, false, false)
for _, v := range tt.values { for _, v := range tt.values {
dp.record(v) h.measure(context.Background(), v, alice)
} }
dp := h.values[alice]
assert.Equal(t, tt.expected.max, dp.max) assert.Equal(t, tt.expected.max, dp.max)
assert.Equal(t, tt.expected.min, dp.min) assert.Equal(t, tt.expected.min, dp.min)
@@ -200,7 +201,7 @@ func testExpoHistogramDataPointRecordMinMaxSumInt64(t *testing.T) {
} }
} }
func testExpoHistogramDataPointRecordMinMaxSumFloat64(t *testing.T) { func testExpoHistogramMinMaxSumFloat64(t *testing.T) {
testCases := []expoHistogramDataPointRecordMinMaxSumTestCase[float64]{ testCases := []expoHistogramDataPointRecordMinMaxSumTestCase[float64]{
{ {
values: []float64{2, 4, 1}, values: []float64{2, 4, 1},
@@ -229,10 +230,11 @@ func testExpoHistogramDataPointRecordMinMaxSumFloat64(t *testing.T) {
restore := withHandler(t) restore := withHandler(t)
defer restore() defer restore()
dp := newExpoHistogramDataPoint[float64](4, 20, false, false) h := newExponentialHistogram[float64](4, 20, false, false)
for _, v := range tt.values { for _, v := range tt.values {
dp.record(v) h.measure(context.Background(), v, alice)
} }
dp := h.values[alice]
assert.Equal(t, tt.expected.max, dp.max) assert.Equal(t, tt.expected.max, dp.max)
assert.Equal(t, tt.expected.min, dp.min) assert.Equal(t, tt.expected.min, dp.min)