From 0af750b86fd0d08c74332c21c9609cd772ea011f Mon Sep 17 00:00:00 2001 From: tosuke <13393900+tosuke@users.noreply.github.com> Date: Sun, 6 Oct 2024 21:05:22 +0900 Subject: [PATCH] [receiver/podman] Fix precision for cpu metrics --- .chloggen/fix-podman-cpu-precision.yaml | 27 +++++++++++++++++++ receiver/podmanreceiver/documentation.md | 6 ++--- .../podmanreceiver/generated_package_test.go | 3 +-- .../internal/metadata/generated_metrics.go | 18 ++++++------- .../metadata/generated_metrics_test.go | 12 ++++----- receiver/podmanreceiver/metadata.yaml | 6 ++--- receiver/podmanreceiver/receiver.go | 12 ++++----- .../podmanreceiver/record_metrics_test.go | 12 ++++++--- 8 files changed, 63 insertions(+), 33 deletions(-) create mode 100644 .chloggen/fix-podman-cpu-precision.yaml diff --git a/.chloggen/fix-podman-cpu-precision.yaml b/.chloggen/fix-podman-cpu-precision.yaml new file mode 100644 index 000000000000..517090868035 --- /dev/null +++ b/.chloggen/fix-podman-cpu-precision.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: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: podmanreceiver + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Change cpu metric type to float to preserve precision." + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [35633] + +# (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: [user] diff --git a/receiver/podmanreceiver/documentation.md b/receiver/podmanreceiver/documentation.md index 6e5bb91e9912..7efa815cfcfb 100644 --- a/receiver/podmanreceiver/documentation.md +++ b/receiver/podmanreceiver/documentation.md @@ -46,7 +46,7 @@ Total CPU time consumed per CPU-core. | Unit | Metric Type | Value Type | Aggregation Temporality | Monotonic | | ---- | ----------- | ---------- | ----------------------- | --------- | -| s | Sum | Int | Cumulative | true | +| s | Sum | Double | Cumulative | true | #### Attributes @@ -60,7 +60,7 @@ System CPU usage. | Unit | Metric Type | Value Type | Aggregation Temporality | Monotonic | | ---- | ----------- | ---------- | ----------------------- | --------- | -| s | Sum | Int | Cumulative | true | +| s | Sum | Double | Cumulative | true | ### container.cpu.usage.total @@ -68,7 +68,7 @@ Total CPU time consumed. | Unit | Metric Type | Value Type | Aggregation Temporality | Monotonic | | ---- | ----------- | ---------- | ----------------------- | --------- | -| s | Sum | Int | Cumulative | true | +| s | Sum | Double | Cumulative | true | ### container.memory.percent diff --git a/receiver/podmanreceiver/generated_package_test.go b/receiver/podmanreceiver/generated_package_test.go index cee778eafa8a..832ca1b0edca 100644 --- a/receiver/podmanreceiver/generated_package_test.go +++ b/receiver/podmanreceiver/generated_package_test.go @@ -3,9 +3,8 @@ package podmanreceiver import ( - "testing" - "go.uber.org/goleak" + "testing" ) func TestMain(m *testing.M) { diff --git a/receiver/podmanreceiver/internal/metadata/generated_metrics.go b/receiver/podmanreceiver/internal/metadata/generated_metrics.go index 64f40ff7ca9b..0b3946d33edd 100644 --- a/receiver/podmanreceiver/internal/metadata/generated_metrics.go +++ b/receiver/podmanreceiver/internal/metadata/generated_metrics.go @@ -180,14 +180,14 @@ func (m *metricContainerCPUUsagePercpu) init() { m.data.Sum().DataPoints().EnsureCapacity(m.capacity) } -func (m *metricContainerCPUUsagePercpu) recordDataPoint(start pcommon.Timestamp, ts pcommon.Timestamp, val int64, coreAttributeValue string) { +func (m *metricContainerCPUUsagePercpu) recordDataPoint(start pcommon.Timestamp, ts pcommon.Timestamp, val float64, coreAttributeValue string) { if !m.config.Enabled { return } dp := m.data.Sum().DataPoints().AppendEmpty() dp.SetStartTimestamp(start) dp.SetTimestamp(ts) - dp.SetIntValue(val) + dp.SetDoubleValue(val) dp.Attributes().PutStr("core", coreAttributeValue) } @@ -232,14 +232,14 @@ func (m *metricContainerCPUUsageSystem) init() { m.data.Sum().SetAggregationTemporality(pmetric.AggregationTemporalityCumulative) } -func (m *metricContainerCPUUsageSystem) recordDataPoint(start pcommon.Timestamp, ts pcommon.Timestamp, val int64) { +func (m *metricContainerCPUUsageSystem) recordDataPoint(start pcommon.Timestamp, ts pcommon.Timestamp, val float64) { if !m.config.Enabled { return } dp := m.data.Sum().DataPoints().AppendEmpty() dp.SetStartTimestamp(start) dp.SetTimestamp(ts) - dp.SetIntValue(val) + dp.SetDoubleValue(val) } // updateCapacity saves max length of data point slices that will be used for the slice capacity. @@ -283,14 +283,14 @@ func (m *metricContainerCPUUsageTotal) init() { m.data.Sum().SetAggregationTemporality(pmetric.AggregationTemporalityCumulative) } -func (m *metricContainerCPUUsageTotal) recordDataPoint(start pcommon.Timestamp, ts pcommon.Timestamp, val int64) { +func (m *metricContainerCPUUsageTotal) recordDataPoint(start pcommon.Timestamp, ts pcommon.Timestamp, val float64) { if !m.config.Enabled { return } dp := m.data.Sum().DataPoints().AppendEmpty() dp.SetStartTimestamp(start) dp.SetTimestamp(ts) - dp.SetIntValue(val) + dp.SetDoubleValue(val) } // updateCapacity saves max length of data point slices that will be used for the slice capacity. @@ -783,17 +783,17 @@ func (mb *MetricsBuilder) RecordContainerCPUPercentDataPoint(ts pcommon.Timestam } // RecordContainerCPUUsagePercpuDataPoint adds a data point to container.cpu.usage.percpu metric. -func (mb *MetricsBuilder) RecordContainerCPUUsagePercpuDataPoint(ts pcommon.Timestamp, val int64, coreAttributeValue string) { +func (mb *MetricsBuilder) RecordContainerCPUUsagePercpuDataPoint(ts pcommon.Timestamp, val float64, coreAttributeValue string) { mb.metricContainerCPUUsagePercpu.recordDataPoint(mb.startTime, ts, val, coreAttributeValue) } // RecordContainerCPUUsageSystemDataPoint adds a data point to container.cpu.usage.system metric. -func (mb *MetricsBuilder) RecordContainerCPUUsageSystemDataPoint(ts pcommon.Timestamp, val int64) { +func (mb *MetricsBuilder) RecordContainerCPUUsageSystemDataPoint(ts pcommon.Timestamp, val float64) { mb.metricContainerCPUUsageSystem.recordDataPoint(mb.startTime, ts, val) } // RecordContainerCPUUsageTotalDataPoint adds a data point to container.cpu.usage.total metric. -func (mb *MetricsBuilder) RecordContainerCPUUsageTotalDataPoint(ts pcommon.Timestamp, val int64) { +func (mb *MetricsBuilder) RecordContainerCPUUsageTotalDataPoint(ts pcommon.Timestamp, val float64) { mb.metricContainerCPUUsageTotal.recordDataPoint(mb.startTime, ts, val) } diff --git a/receiver/podmanreceiver/internal/metadata/generated_metrics_test.go b/receiver/podmanreceiver/internal/metadata/generated_metrics_test.go index 77d953ba1c5e..852fd70fd20e 100644 --- a/receiver/podmanreceiver/internal/metadata/generated_metrics_test.go +++ b/receiver/podmanreceiver/internal/metadata/generated_metrics_test.go @@ -191,8 +191,8 @@ func TestMetricsBuilder(t *testing.T) { dp := ms.At(i).Sum().DataPoints().At(0) assert.Equal(t, start, dp.StartTimestamp()) assert.Equal(t, ts, dp.Timestamp()) - assert.Equal(t, pmetric.NumberDataPointValueTypeInt, dp.ValueType()) - assert.Equal(t, int64(1), dp.IntValue()) + assert.Equal(t, pmetric.NumberDataPointValueTypeDouble, dp.ValueType()) + assert.InDelta(t, float64(1), dp.DoubleValue(), 0.01) attrVal, ok := dp.Attributes().Get("core") assert.True(t, ok) assert.EqualValues(t, "core-val", attrVal.Str()) @@ -208,8 +208,8 @@ func TestMetricsBuilder(t *testing.T) { dp := ms.At(i).Sum().DataPoints().At(0) assert.Equal(t, start, dp.StartTimestamp()) assert.Equal(t, ts, dp.Timestamp()) - assert.Equal(t, pmetric.NumberDataPointValueTypeInt, dp.ValueType()) - assert.Equal(t, int64(1), dp.IntValue()) + assert.Equal(t, pmetric.NumberDataPointValueTypeDouble, dp.ValueType()) + assert.InDelta(t, float64(1), dp.DoubleValue(), 0.01) case "container.cpu.usage.total": assert.False(t, validatedMetrics["container.cpu.usage.total"], "Found a duplicate in the metrics slice: container.cpu.usage.total") validatedMetrics["container.cpu.usage.total"] = true @@ -222,8 +222,8 @@ func TestMetricsBuilder(t *testing.T) { dp := ms.At(i).Sum().DataPoints().At(0) assert.Equal(t, start, dp.StartTimestamp()) assert.Equal(t, ts, dp.Timestamp()) - assert.Equal(t, pmetric.NumberDataPointValueTypeInt, dp.ValueType()) - assert.Equal(t, int64(1), dp.IntValue()) + assert.Equal(t, pmetric.NumberDataPointValueTypeDouble, dp.ValueType()) + assert.InDelta(t, float64(1), dp.DoubleValue(), 0.01) case "container.memory.percent": assert.False(t, validatedMetrics["container.memory.percent"], "Found a duplicate in the metrics slice: container.memory.percent") validatedMetrics["container.memory.percent"] = true diff --git a/receiver/podmanreceiver/metadata.yaml b/receiver/podmanreceiver/metadata.yaml index 538f945ba96c..44b90eecf721 100644 --- a/receiver/podmanreceiver/metadata.yaml +++ b/receiver/podmanreceiver/metadata.yaml @@ -39,7 +39,7 @@ metrics: description: "System CPU usage." unit: s sum: - value_type: int + value_type: double monotonic: true aggregation_temporality: cumulative container.cpu.usage.total: @@ -47,7 +47,7 @@ metrics: description: "Total CPU time consumed." unit: s sum: - value_type: int + value_type: double monotonic: true aggregation_temporality: cumulative container.cpu.usage.percpu: @@ -55,7 +55,7 @@ metrics: description: "Total CPU time consumed per CPU-core." unit: s sum: - value_type: int + value_type: double monotonic: true aggregation_temporality: cumulative attributes: diff --git a/receiver/podmanreceiver/receiver.go b/receiver/podmanreceiver/receiver.go index 5df2af974870..a66bc84ee6ab 100644 --- a/receiver/podmanreceiver/receiver.go +++ b/receiver/podmanreceiver/receiver.go @@ -145,12 +145,12 @@ func (r *metricsReceiver) recordContainerStats(now pcommon.Timestamp, container } func (r *metricsReceiver) recordCPUMetrics(now pcommon.Timestamp, stats *containerStats) { - r.mb.RecordContainerCPUUsageSystemDataPoint(now, int64(toSecondsWithNanosecondPrecision(stats.CPUSystemNano))) - r.mb.RecordContainerCPUUsageTotalDataPoint(now, int64(toSecondsWithNanosecondPrecision(stats.CPUNano))) + r.mb.RecordContainerCPUUsageSystemDataPoint(now, toSecondsWithNanosecondPrecision(stats.CPUSystemNano)) + r.mb.RecordContainerCPUUsageTotalDataPoint(now, toSecondsWithNanosecondPrecision(stats.CPUNano)) r.mb.RecordContainerCPUPercentDataPoint(now, stats.CPU) for i, cpu := range stats.PerCPU { - r.mb.RecordContainerCPUUsagePercpuDataPoint(now, int64(toSecondsWithNanosecondPrecision(cpu)), fmt.Sprintf("cpu%d", i)) + r.mb.RecordContainerCPUUsagePercpuDataPoint(now, toSecondsWithNanosecondPrecision(cpu), fmt.Sprintf("cpu%d", i)) } } @@ -170,7 +170,7 @@ func (r *metricsReceiver) recordIOMetrics(now pcommon.Timestamp, stats *containe r.mb.RecordContainerBlockioIoServiceBytesRecursiveWriteDataPoint(now, int64(stats.BlockOutput)) } -// nanoseconds to seconds conversion truncating the fractional part -func toSecondsWithNanosecondPrecision(nanoseconds uint64) uint64 { - return nanoseconds / 1e9 +// nanoseconds to seconds conversion +func toSecondsWithNanosecondPrecision(nanoseconds uint64) float64 { + return float64(nanoseconds) / 1e9 } diff --git a/receiver/podmanreceiver/record_metrics_test.go b/receiver/podmanreceiver/record_metrics_test.go index ac949613a488..739e6fdc6961 100644 --- a/receiver/podmanreceiver/record_metrics_test.go +++ b/receiver/podmanreceiver/record_metrics_test.go @@ -60,15 +60,15 @@ func assertStatsEqualToMetrics(t *testing.T, podmanStats *containerStats, md pme assertMetricEqual(t, m, pmetric.MetricTypeSum, []point{{intVal: podmanStats.BlockInput}}) case "container.cpu.usage.system": - assertMetricEqual(t, m, pmetric.MetricTypeSum, []point{{intVal: toSecondsWithNanosecondPrecision(podmanStats.CPUSystemNano)}}) + assertMetricEqual(t, m, pmetric.MetricTypeSum, []point{{doubleVal: toSecondsWithNanosecondPrecision(podmanStats.CPUSystemNano)}}) case "container.cpu.usage.total": - assertMetricEqual(t, m, pmetric.MetricTypeSum, []point{{intVal: toSecondsWithNanosecondPrecision(podmanStats.CPUNano)}}) + assertMetricEqual(t, m, pmetric.MetricTypeSum, []point{{doubleVal: toSecondsWithNanosecondPrecision(podmanStats.CPUNano)}}) case "container.cpu.percent": assertMetricEqual(t, m, pmetric.MetricTypeGauge, []point{{doubleVal: podmanStats.CPU}}) case "container.cpu.usage.percpu": points := make([]point, len(podmanStats.PerCPU)) for i, v := range podmanStats.PerCPU { - points[i] = point{intVal: toSecondsWithNanosecondPrecision(v), attributes: map[string]string{"core": fmt.Sprintf("cpu%d", i)}} + points[i] = point{doubleVal: toSecondsWithNanosecondPrecision(v), attributes: map[string]string{"core": fmt.Sprintf("cpu%d", i)}} } assertMetricEqual(t, m, pmetric.MetricTypeSum, points) @@ -79,6 +79,8 @@ func assertStatsEqualToMetrics(t *testing.T, podmanStats *containerStats, md pme } func assertMetricEqual(t *testing.T, m pmetric.Metric, dt pmetric.MetricType, pts []point) { + t.Helper() + assert.Equal(t, m.Type(), dt) switch dt { case pmetric.MetricTypeGauge: @@ -99,11 +101,13 @@ func assertMetricEqual(t *testing.T, m pmetric.Metric, dt pmetric.MetricType, pt } func assertPoints(t *testing.T, dpts pmetric.NumberDataPointSlice, pts []point) { + t.Helper() + assert.Len(t, pts, dpts.Len()) for i, expected := range pts { got := dpts.At(i) assert.Equal(t, got.IntValue(), int64(expected.intVal)) - assert.Equal(t, expected.doubleVal, got.DoubleValue()) + assert.InDelta(t, expected.doubleVal, got.DoubleValue(), 0.01) for k, expectedV := range expected.attributes { gotV, exists := got.Attributes().Get(k) assert.True(t, exists)