Skip to content

Commit 02d1587

Browse files
authored
[exporter/elasticsearch]Handle empty histogram buckets (#44023)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Empty histogram buckets should not error out. <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes #44022 <!--Describe what testing was performed and which tests were added.--> #### Testing Tests added. <!--Describe the documentation added.--> #### Documentation N/A <!--Please delete paragraphs that you did not use before submitting.-->
1 parent 8599eb6 commit 02d1587

File tree

3 files changed

+115
-4
lines changed

3 files changed

+115
-4
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
7+
component: exporter/elasticsearch
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Handle empty histogram buckets to not result in an invalid datapoint error.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [44022]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]

exporter/elasticsearchexporter/internal/datapoints/histogram.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,9 @@ func (dp Histogram) Metric() pmetric.Metric {
5353
}
5454

5555
func histogramToValue(dp pmetric.HistogramDataPoint, metric pmetric.Metric) (pcommon.Value, error) {
56-
// Histogram conversion function is from
57-
// https://github.com/elastic/apm-data/blob/3b28495c3cbdc0902983134276eb114231730249/input/otlp/metrics.go#L277
5856
bucketCounts := dp.BucketCounts()
5957
explicitBounds := dp.ExplicitBounds()
60-
if bucketCounts.Len() != explicitBounds.Len()+1 || explicitBounds.Len() == 0 {
58+
if bucketCounts.Len() > 0 && bucketCounts.Len() != explicitBounds.Len()+1 {
6159
return pcommon.Value{}, fmt.Errorf("invalid histogram data point %q", metric.Name())
6260
}
6361

@@ -66,8 +64,20 @@ func histogramToValue(dp pmetric.HistogramDataPoint, metric pmetric.Metric) (pco
6664
counts := m.PutEmptySlice("counts")
6765
values := m.PutEmptySlice("values")
6866

69-
values.EnsureCapacity(bucketCounts.Len())
67+
if explicitBounds.Len() == 0 {
68+
// It is possible for explicit bounds to be nil. In this case create
69+
// a bucket using the count and sum which are required to be present.
70+
// See https://opentelemetry.io/docs/specs/otel/metrics/data-model/#histogram
71+
if dp.Count() > 0 {
72+
counts.AppendEmpty().SetInt(safeUint64ToInt64(dp.Count()))
73+
values.AppendEmpty().SetDouble(dp.Sum() / float64(dp.Count()))
74+
}
75+
76+
return vm, nil
77+
}
78+
7079
counts.EnsureCapacity(bucketCounts.Len())
80+
values.EnsureCapacity(bucketCounts.Len())
7181
for i, count := range bucketCounts.All() {
7282
if count == 0 {
7383
continue
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package datapoints // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/elasticsearchexporter/internal/datapoints"
5+
6+
import (
7+
"testing"
8+
"time"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
"go.opentelemetry.io/collector/pdata/pcommon"
13+
"go.opentelemetry.io/collector/pdata/pmetric"
14+
)
15+
16+
func TestHistogramValue(t *testing.T) {
17+
for _, tc := range []struct {
18+
name string
19+
histogramDP pmetric.HistogramDataPoint
20+
expected func(t *testing.T) pcommon.Value
21+
}{
22+
{
23+
name: "empty",
24+
histogramDP: func() pmetric.HistogramDataPoint {
25+
now := time.Now()
26+
dp := pmetric.NewHistogramDataPoint()
27+
dp.SetStartTimestamp(pcommon.NewTimestampFromTime(now.Add(-1 * time.Hour)))
28+
dp.SetTimestamp(pcommon.NewTimestampFromTime(now))
29+
return dp
30+
}(),
31+
expected: func(t *testing.T) pcommon.Value {
32+
t.Helper()
33+
v := pcommon.NewValueMap()
34+
require.NoError(t, v.FromRaw(map[string]any{
35+
"counts": []any{},
36+
"values": []any{},
37+
}))
38+
return v
39+
},
40+
},
41+
{
42+
name: "required_fields_only",
43+
histogramDP: func() pmetric.HistogramDataPoint {
44+
now := time.Now()
45+
dp := pmetric.NewHistogramDataPoint()
46+
dp.SetStartTimestamp(pcommon.NewTimestampFromTime(now.Add(-1 * time.Hour)))
47+
dp.SetTimestamp(pcommon.NewTimestampFromTime(now))
48+
dp.SetCount(100)
49+
dp.SetSum(1000)
50+
return dp
51+
}(),
52+
expected: func(t *testing.T) pcommon.Value {
53+
t.Helper()
54+
v := pcommon.NewValueMap()
55+
require.NoError(t, v.FromRaw(map[string]any{
56+
"counts": []any{100},
57+
"values": []any{10.0},
58+
}))
59+
return v
60+
},
61+
},
62+
} {
63+
t.Run(tc.name, func(t *testing.T) {
64+
m := pmetric.NewMetric()
65+
m.SetName("test")
66+
tc.histogramDP.MoveTo(m.SetEmptyHistogram().DataPoints().AppendEmpty())
67+
68+
esHist := NewHistogram(m, m.Histogram().DataPoints().At(0))
69+
actual, err := esHist.Value()
70+
require.NoError(t, err)
71+
assert.True(t, tc.expected(t).Equal(actual))
72+
})
73+
}
74+
}

0 commit comments

Comments
 (0)