diff --git a/.chloggen/awsemf_SummaryNanCheck.yaml b/.chloggen/awsemf_SummaryNanCheck.yaml new file mode 100755 index 000000000000..0ae3767761fb --- /dev/null +++ b/.chloggen/awsemf_SummaryNanCheck.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: awsemfexporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Improve NaN value checking for Summary metric types. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [28894] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/exporter/awsemfexporter/datapoint.go b/exporter/awsemfexporter/datapoint.go index 54759b047441..b654a26dc5cb 100644 --- a/exporter/awsemfexporter/datapoint.go +++ b/exporter/awsemfexporter/datapoint.go @@ -351,6 +351,15 @@ func (dps summaryDataPointSlice) IsStaleOrNaN(i int) (bool, pcommon.Map) { if math.IsNaN(metric.Sum()) { return true, metric.Attributes() } + + values := metric.QuantileValues() + for i := 0; i < values.Len(); i++ { + quantile := values.At(i) + if math.IsNaN(quantile.Value()) || math.IsNaN(quantile.Quantile()) { + return true, metric.Attributes() + } + } + return false, metric.Attributes() } diff --git a/exporter/awsemfexporter/datapoint_test.go b/exporter/awsemfexporter/datapoint_test.go index 638a98737f2c..28147cd92e3c 100644 --- a/exporter/awsemfexporter/datapoint_test.go +++ b/exporter/awsemfexporter/datapoint_test.go @@ -223,11 +223,11 @@ func generateTestSummaryMetricWithNaN(name string) pmetric.Metrics { summaryDatapoint.SetCount(uint64(5 * i)) summaryDatapoint.SetSum(math.NaN()) firstQuantile := summaryDatapoint.QuantileValues().AppendEmpty() - firstQuantile.SetQuantile(0.0) - firstQuantile.SetValue(1) + firstQuantile.SetQuantile(math.NaN()) + firstQuantile.SetValue(math.NaN()) secondQuantile := summaryDatapoint.QuantileValues().AppendEmpty() - secondQuantile.SetQuantile(100.0) - secondQuantile.SetValue(5) + secondQuantile.SetQuantile(math.NaN()) + secondQuantile.SetValue(math.NaN()) } return otelMetrics @@ -543,7 +543,7 @@ func TestIsStaleOrNaN_HistogramDataPointSlice(t *testing.T) { setFlagsFunc func(point pmetric.HistogramDataPoint) pmetric.HistogramDataPoint }{ { - name: "Histogram with NaNs", + name: "Histogram with all NaNs", histogramDPS: func() pmetric.HistogramDataPointSlice { histogramDPS := pmetric.NewHistogramDataPointSlice() histogramDP := histogramDPS.AppendEmpty() @@ -556,6 +556,48 @@ func TestIsStaleOrNaN_HistogramDataPointSlice(t *testing.T) { }(), boolAssertFunc: assert.True, }, + { + name: "Histogram with NaN Sum", + histogramDPS: func() pmetric.HistogramDataPointSlice { + histogramDPS := pmetric.NewHistogramDataPointSlice() + histogramDP := histogramDPS.AppendEmpty() + histogramDP.SetCount(uint64(17)) + histogramDP.SetSum(math.NaN()) + histogramDP.SetMin(1234) + histogramDP.SetMax(1234) + histogramDP.Attributes().PutStr("label1", "value1") + return histogramDPS + }(), + boolAssertFunc: assert.True, + }, + { + name: "Histogram with NaN Min", + histogramDPS: func() pmetric.HistogramDataPointSlice { + histogramDPS := pmetric.NewHistogramDataPointSlice() + histogramDP := histogramDPS.AppendEmpty() + histogramDP.SetCount(uint64(17)) + histogramDP.SetSum(123) + histogramDP.SetMin(math.NaN()) + histogramDP.SetMax(123) + histogramDP.Attributes().PutStr("label1", "value1") + return histogramDPS + }(), + boolAssertFunc: assert.True, + }, + { + name: "Histogram with nan Max", + histogramDPS: func() pmetric.HistogramDataPointSlice { + histogramDPS := pmetric.NewHistogramDataPointSlice() + histogramDP := histogramDPS.AppendEmpty() + histogramDP.SetCount(uint64(17)) + histogramDP.SetSum(123) + histogramDP.SetMin(123) + histogramDP.SetMax(math.NaN()) + histogramDP.Attributes().PutStr("label1", "value1") + return histogramDPS + }(), + boolAssertFunc: assert.True, + }, { name: "Histogram with min and max", histogramDPS: func() pmetric.HistogramDataPointSlice { @@ -727,6 +769,62 @@ func TestIsStaleOrNaN_ExponentialHistogramDataPointSlice(t *testing.T) { }(), boolAssertFunc: assert.False, }, + { + name: "Exponential histogram with all possible NaN", + histogramDPS: func() pmetric.ExponentialHistogramDataPointSlice { + histogramDPS := pmetric.NewExponentialHistogramDataPointSlice() + histogramDP := histogramDPS.AppendEmpty() + histogramDP.SetCount(uint64(17)) + histogramDP.SetSum(math.NaN()) + histogramDP.SetMin(math.NaN()) + histogramDP.SetMax(math.NaN()) + histogramDP.Attributes().PutStr("label1", "value1") + return histogramDPS + }(), + boolAssertFunc: assert.True, + }, + { + name: "Exponential histogram with NaN Sum", + histogramDPS: func() pmetric.ExponentialHistogramDataPointSlice { + histogramDPS := pmetric.NewExponentialHistogramDataPointSlice() + histogramDP := histogramDPS.AppendEmpty() + histogramDP.SetCount(uint64(17)) + histogramDP.SetSum(math.NaN()) + histogramDP.SetMin(1245) + histogramDP.SetMax(1234556) + histogramDP.Attributes().PutStr("label1", "value1") + return histogramDPS + }(), + boolAssertFunc: assert.True, + }, + { + name: "Exponential histogram with NaN Min", + histogramDPS: func() pmetric.ExponentialHistogramDataPointSlice { + histogramDPS := pmetric.NewExponentialHistogramDataPointSlice() + histogramDP := histogramDPS.AppendEmpty() + histogramDP.SetCount(uint64(17)) + histogramDP.SetSum(1255) + histogramDP.SetMin(math.NaN()) + histogramDP.SetMax(12545) + histogramDP.Attributes().PutStr("label1", "value1") + return histogramDPS + }(), + boolAssertFunc: assert.True, + }, + { + name: "Exponential histogram with NaN Max", + histogramDPS: func() pmetric.ExponentialHistogramDataPointSlice { + histogramDPS := pmetric.NewExponentialHistogramDataPointSlice() + histogramDP := histogramDPS.AppendEmpty() + histogramDP.SetCount(uint64(17)) + histogramDP.SetSum(512444) + histogramDP.SetMin(123) + histogramDP.SetMax(math.NaN()) + histogramDP.Attributes().PutStr("label1", "value1") + return histogramDPS + }(), + boolAssertFunc: assert.True, + }, { name: "Exponential histogram with NaNs", histogramDPS: func() pmetric.ExponentialHistogramDataPointSlice { @@ -862,49 +960,131 @@ func TestCalculateDeltaDatapoints_SummaryDataPointSlice(t *testing.T) { } func TestIsStaleOrNaN_SummaryDataPointSlice(t *testing.T) { + type qMetricObject struct { + value float64 + quantile float64 + } + type quantileTestObj struct { + sum float64 + count uint64 + qMetrics []qMetricObject + } testCases := []struct { name string - summaryMetricValue map[string]any + summaryMetricValue quantileTestObj expectedBoolAssert assert.BoolAssertionFunc setFlagsFunc func(point pmetric.SummaryDataPoint) pmetric.SummaryDataPoint }{ { - name: "summary with no nan values", - summaryMetricValue: map[string]any{"sum": float64(17.3), "count": uint64(17), "firstQuantile": float64(1), "secondQuantile": float64(5)}, + name: "summary with no nan values", + summaryMetricValue: quantileTestObj{ + sum: 17.3, + count: 17, + qMetrics: []qMetricObject{ + { + value: 1, + quantile: 0.5, + }, + { + value: 5, + quantile: 2.0, + }, + }, + }, expectedBoolAssert: assert.False, }, { - name: "Summary with nan values", - summaryMetricValue: map[string]any{"sum": math.NaN(), "count": uint64(25), "firstQuantile": math.NaN(), "secondQuantile": math.NaN()}, + name: "Summary with nan sum", + summaryMetricValue: quantileTestObj{ + sum: math.NaN(), + count: 17, + qMetrics: []qMetricObject{ + { + value: 1, + quantile: 0.5, + }, + { + value: 5, + quantile: 2.0, + }, + }, + }, expectedBoolAssert: assert.True, }, { - name: "Summary with set flag func", - summaryMetricValue: map[string]any{"sum": math.NaN(), "count": uint64(25), "firstQuantile": math.NaN(), "secondQuantile": math.NaN()}, + name: "Summary with no recorded value flag set to true", + summaryMetricValue: quantileTestObj{ + sum: 1245.65, + count: 17, + qMetrics: []qMetricObject{ + { + value: 1, + quantile: 0.5, + }, + { + value: 5, + quantile: 2.0, + }, + }, + }, expectedBoolAssert: assert.True, setFlagsFunc: func(point pmetric.SummaryDataPoint) pmetric.SummaryDataPoint { point.SetFlags(pmetric.DefaultDataPointFlags.WithNoRecordedValue(true)) return point }, }, + { + name: "Summary with nan quantile value", + summaryMetricValue: quantileTestObj{ + sum: 1245.65, + count: 17, + qMetrics: []qMetricObject{ + { + value: 1, + quantile: 0.5, + }, + { + value: math.NaN(), + quantile: 2.0, + }, + }, + }, + expectedBoolAssert: assert.True, + }, + { + name: "Summary with nan quantile", + summaryMetricValue: quantileTestObj{ + sum: 1245.65, + count: 17, + qMetrics: []qMetricObject{ + { + value: 1, + quantile: 0.5, + }, + { + value: 7.8, + quantile: math.NaN(), + }, + }, + }, + expectedBoolAssert: assert.True, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - // Given the summary datapoints with quantile 0, quantile 100, sum and count summaryDPS := pmetric.NewSummaryDataPointSlice() summaryDP := summaryDPS.AppendEmpty() - summaryDP.SetSum(tc.summaryMetricValue["sum"].(float64)) - summaryDP.SetCount(tc.summaryMetricValue["count"].(uint64)) + summaryDP.SetSum(tc.summaryMetricValue.sum) + summaryDP.SetCount(tc.summaryMetricValue.count) summaryDP.Attributes().PutStr("label1", "value1") - summaryDP.QuantileValues().EnsureCapacity(2) - firstQuantileValue := summaryDP.QuantileValues().AppendEmpty() - firstQuantileValue.SetQuantile(0) - firstQuantileValue.SetValue(tc.summaryMetricValue["firstQuantile"].(float64)) - secondQuantileValue := summaryDP.QuantileValues().AppendEmpty() - secondQuantileValue.SetQuantile(100) - secondQuantileValue.SetValue(tc.summaryMetricValue["secondQuantile"].(float64)) + summaryDP.QuantileValues().EnsureCapacity(len(tc.summaryMetricValue.qMetrics)) + for _, qMetric := range tc.summaryMetricValue.qMetrics { + newQ := summaryDP.QuantileValues().AppendEmpty() + newQ.SetValue(qMetric.value) + newQ.SetQuantile(qMetric.quantile) + } summaryDatapointSlice := summaryDataPointSlice{deltaMetricMetadata{}, summaryDPS} if tc.setFlagsFunc != nil {