From 6a703592c0d59bb081f033628c8b8d640bf07d84 Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 26 Oct 2023 05:16:36 +0200 Subject: [PATCH] [receiver/dockerstats] remove deprecated container.cpu.percent and complete the transition to container.cpu.utilization (#27795) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Description:** Following up #24183, this PR removes the deprecated `container.cpu.percent` metric as explained in README's [deprecation section](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/dockerstatsreceiver#transition-to-cpu-utilization-metric-name-aligned-with-opentelemetry-specification). **Link to tracking Issue:** #21807 **Testing:** ``` ❯ go test -race -timeout 300s -parallel 4 --tags="" ./... -count 1 ok github.com/open-telemetry/opentelemetry-collector-contrib/receiver/dockerstatsreceiver 1.480s ok github.com/open-telemetry/opentelemetry-collector-contrib/receiver/dockerstatsreceiver/internal/metadata 1.525s ``` **Documentation:** the documentation has been updated using `mdatagen` --------- Co-authored-by: Pablo Baeyens --- .chloggen/remove-deprecated-cpu-metric.yaml | 29 +++++++++ receiver/dockerstatsreceiver/documentation.md | 8 --- .../internal/metadata/generated_config.go | 4 -- .../metadata/generated_config_test.go | 2 - .../internal/metadata/generated_metrics.go | 60 ------------------- .../metadata/generated_metrics_test.go | 19 ------ .../internal/metadata/testdata/config.yaml | 4 -- receiver/dockerstatsreceiver/metadata.yaml | 13 ---- receiver/dockerstatsreceiver/receiver.go | 1 - 9 files changed, 29 insertions(+), 111 deletions(-) create mode 100755 .chloggen/remove-deprecated-cpu-metric.yaml diff --git a/.chloggen/remove-deprecated-cpu-metric.yaml b/.chloggen/remove-deprecated-cpu-metric.yaml new file mode 100755 index 000000000000..5ff0d4240a4c --- /dev/null +++ b/.chloggen/remove-deprecated-cpu-metric.yaml @@ -0,0 +1,29 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: dockerstatsreceiver + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "cpu.container.percent metric is removed in favor of container.cpu.utilization" + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [21807] + +# (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: | + The metric `container.cpu.percentage` is now removed. `container.cpu.utilization` is enabled by default as a replacement. + For details, see the [docs](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/dockerstatsreceiver#transition-to-cpu-utilization-metric-name-aligned-with-opentelemetry-specification). + +# 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/dockerstatsreceiver/documentation.md b/receiver/dockerstatsreceiver/documentation.md index 7061ebf4b294..c2ce8dbeb4d7 100644 --- a/receiver/dockerstatsreceiver/documentation.md +++ b/receiver/dockerstatsreceiver/documentation.md @@ -296,14 +296,6 @@ Number of sectors transferred to/from disk by the group and descendant groups (O | device_minor | Device minor number for block IO operations. | Any Str | | operation | Type of BlockIO operation. | Any Str | -### container.cpu.percent - -[DEPRECATED] Use `container.cpu.utilization` metric instead. Percent of CPU used by the container. - -| Unit | Metric Type | Value Type | -| ---- | ----------- | ---------- | -| 1 | Gauge | Double | - ### container.cpu.throttling_data.periods Number of periods with throttling active. diff --git a/receiver/dockerstatsreceiver/internal/metadata/generated_config.go b/receiver/dockerstatsreceiver/internal/metadata/generated_config.go index aed596f0ba2f..1494a935d53a 100644 --- a/receiver/dockerstatsreceiver/internal/metadata/generated_config.go +++ b/receiver/dockerstatsreceiver/internal/metadata/generated_config.go @@ -33,7 +33,6 @@ type MetricsConfig struct { ContainerBlockioIoTimeRecursive MetricConfig `mapstructure:"container.blockio.io_time_recursive"` ContainerBlockioIoWaitTimeRecursive MetricConfig `mapstructure:"container.blockio.io_wait_time_recursive"` ContainerBlockioSectorsRecursive MetricConfig `mapstructure:"container.blockio.sectors_recursive"` - ContainerCPUPercent MetricConfig `mapstructure:"container.cpu.percent"` ContainerCPUThrottlingDataPeriods MetricConfig `mapstructure:"container.cpu.throttling_data.periods"` ContainerCPUThrottlingDataThrottledPeriods MetricConfig `mapstructure:"container.cpu.throttling_data.throttled_periods"` ContainerCPUThrottlingDataThrottledTime MetricConfig `mapstructure:"container.cpu.throttling_data.throttled_time"` @@ -120,9 +119,6 @@ func DefaultMetricsConfig() MetricsConfig { ContainerBlockioSectorsRecursive: MetricConfig{ Enabled: false, }, - ContainerCPUPercent: MetricConfig{ - Enabled: false, - }, ContainerCPUThrottlingDataPeriods: MetricConfig{ Enabled: false, }, diff --git a/receiver/dockerstatsreceiver/internal/metadata/generated_config_test.go b/receiver/dockerstatsreceiver/internal/metadata/generated_config_test.go index 5d11b8e18d4a..2c94c9a2c75d 100644 --- a/receiver/dockerstatsreceiver/internal/metadata/generated_config_test.go +++ b/receiver/dockerstatsreceiver/internal/metadata/generated_config_test.go @@ -34,7 +34,6 @@ func TestMetricsBuilderConfig(t *testing.T) { ContainerBlockioIoTimeRecursive: MetricConfig{Enabled: true}, ContainerBlockioIoWaitTimeRecursive: MetricConfig{Enabled: true}, ContainerBlockioSectorsRecursive: MetricConfig{Enabled: true}, - ContainerCPUPercent: MetricConfig{Enabled: true}, ContainerCPUThrottlingDataPeriods: MetricConfig{Enabled: true}, ContainerCPUThrottlingDataThrottledPeriods: MetricConfig{Enabled: true}, ContainerCPUThrottlingDataThrottledTime: MetricConfig{Enabled: true}, @@ -117,7 +116,6 @@ func TestMetricsBuilderConfig(t *testing.T) { ContainerBlockioIoTimeRecursive: MetricConfig{Enabled: false}, ContainerBlockioIoWaitTimeRecursive: MetricConfig{Enabled: false}, ContainerBlockioSectorsRecursive: MetricConfig{Enabled: false}, - ContainerCPUPercent: MetricConfig{Enabled: false}, ContainerCPUThrottlingDataPeriods: MetricConfig{Enabled: false}, ContainerCPUThrottlingDataThrottledPeriods: MetricConfig{Enabled: false}, ContainerCPUThrottlingDataThrottledTime: MetricConfig{Enabled: false}, diff --git a/receiver/dockerstatsreceiver/internal/metadata/generated_metrics.go b/receiver/dockerstatsreceiver/internal/metadata/generated_metrics.go index af45193e0075..8aaef0e4d807 100644 --- a/receiver/dockerstatsreceiver/internal/metadata/generated_metrics.go +++ b/receiver/dockerstatsreceiver/internal/metadata/generated_metrics.go @@ -452,55 +452,6 @@ func newMetricContainerBlockioSectorsRecursive(cfg MetricConfig) metricContainer return m } -type metricContainerCPUPercent struct { - data pmetric.Metric // data buffer for generated metric. - config MetricConfig // metric config provided by user. - capacity int // max observed number of data points added to the metric. -} - -// init fills container.cpu.percent metric with initial data. -func (m *metricContainerCPUPercent) init() { - m.data.SetName("container.cpu.percent") - m.data.SetDescription("[DEPRECATED] Use `container.cpu.utilization` metric instead. Percent of CPU used by the container.") - m.data.SetUnit("1") - m.data.SetEmptyGauge() -} - -func (m *metricContainerCPUPercent) recordDataPoint(start pcommon.Timestamp, ts pcommon.Timestamp, val float64) { - if !m.config.Enabled { - return - } - dp := m.data.Gauge().DataPoints().AppendEmpty() - dp.SetStartTimestamp(start) - dp.SetTimestamp(ts) - dp.SetDoubleValue(val) -} - -// updateCapacity saves max length of data point slices that will be used for the slice capacity. -func (m *metricContainerCPUPercent) updateCapacity() { - if m.data.Gauge().DataPoints().Len() > m.capacity { - m.capacity = m.data.Gauge().DataPoints().Len() - } -} - -// emit appends recorded metric data to a metrics slice and prepares it for recording another set of data points. -func (m *metricContainerCPUPercent) emit(metrics pmetric.MetricSlice) { - if m.config.Enabled && m.data.Gauge().DataPoints().Len() > 0 { - m.updateCapacity() - m.data.MoveTo(metrics.AppendEmpty()) - m.init() - } -} - -func newMetricContainerCPUPercent(cfg MetricConfig) metricContainerCPUPercent { - m := metricContainerCPUPercent{config: cfg} - if cfg.Enabled { - m.data = pmetric.NewMetric() - m.init() - } - return m -} - type metricContainerCPUThrottlingDataPeriods struct { data pmetric.Metric // data buffer for generated metric. config MetricConfig // metric config provided by user. @@ -3487,7 +3438,6 @@ type MetricsBuilder struct { metricContainerBlockioIoTimeRecursive metricContainerBlockioIoTimeRecursive metricContainerBlockioIoWaitTimeRecursive metricContainerBlockioIoWaitTimeRecursive metricContainerBlockioSectorsRecursive metricContainerBlockioSectorsRecursive - metricContainerCPUPercent metricContainerCPUPercent metricContainerCPUThrottlingDataPeriods metricContainerCPUThrottlingDataPeriods metricContainerCPUThrottlingDataThrottledPeriods metricContainerCPUThrottlingDataThrottledPeriods metricContainerCPUThrottlingDataThrottledTime metricContainerCPUThrottlingDataThrottledTime @@ -3559,9 +3509,6 @@ func WithStartTime(startTime pcommon.Timestamp) metricBuilderOption { } func NewMetricsBuilder(mbc MetricsBuilderConfig, settings receiver.CreateSettings, options ...metricBuilderOption) *MetricsBuilder { - if mbc.Metrics.ContainerCPUPercent.enabledSetByUser { - settings.Logger.Warn("[WARNING] `container.cpu.percent` should not be configured: The metric is deprecated and will be removed in v0.89.0. Please use `container.cpu.utilization` instead. See https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/dockerstatsreceiver#transition-to-cpu-utilization-metric-name-aligned-with-opentelemetry-specification for more details.") - } mb := &MetricsBuilder{ config: mbc, startTime: pcommon.NewTimestampFromTime(time.Now()), @@ -3575,7 +3522,6 @@ func NewMetricsBuilder(mbc MetricsBuilderConfig, settings receiver.CreateSetting metricContainerBlockioIoTimeRecursive: newMetricContainerBlockioIoTimeRecursive(mbc.Metrics.ContainerBlockioIoTimeRecursive), metricContainerBlockioIoWaitTimeRecursive: newMetricContainerBlockioIoWaitTimeRecursive(mbc.Metrics.ContainerBlockioIoWaitTimeRecursive), metricContainerBlockioSectorsRecursive: newMetricContainerBlockioSectorsRecursive(mbc.Metrics.ContainerBlockioSectorsRecursive), - metricContainerCPUPercent: newMetricContainerCPUPercent(mbc.Metrics.ContainerCPUPercent), metricContainerCPUThrottlingDataPeriods: newMetricContainerCPUThrottlingDataPeriods(mbc.Metrics.ContainerCPUThrottlingDataPeriods), metricContainerCPUThrottlingDataThrottledPeriods: newMetricContainerCPUThrottlingDataThrottledPeriods(mbc.Metrics.ContainerCPUThrottlingDataThrottledPeriods), metricContainerCPUThrottlingDataThrottledTime: newMetricContainerCPUThrottlingDataThrottledTime(mbc.Metrics.ContainerCPUThrottlingDataThrottledTime), @@ -3704,7 +3650,6 @@ func (mb *MetricsBuilder) EmitForResource(rmo ...ResourceMetricsOption) { mb.metricContainerBlockioIoTimeRecursive.emit(ils.Metrics()) mb.metricContainerBlockioIoWaitTimeRecursive.emit(ils.Metrics()) mb.metricContainerBlockioSectorsRecursive.emit(ils.Metrics()) - mb.metricContainerCPUPercent.emit(ils.Metrics()) mb.metricContainerCPUThrottlingDataPeriods.emit(ils.Metrics()) mb.metricContainerCPUThrottlingDataThrottledPeriods.emit(ils.Metrics()) mb.metricContainerCPUThrottlingDataThrottledTime.emit(ils.Metrics()) @@ -3823,11 +3768,6 @@ func (mb *MetricsBuilder) RecordContainerBlockioSectorsRecursiveDataPoint(ts pco mb.metricContainerBlockioSectorsRecursive.recordDataPoint(mb.startTime, ts, val, deviceMajorAttributeValue, deviceMinorAttributeValue, operationAttributeValue) } -// RecordContainerCPUPercentDataPoint adds a data point to container.cpu.percent metric. -func (mb *MetricsBuilder) RecordContainerCPUPercentDataPoint(ts pcommon.Timestamp, val float64) { - mb.metricContainerCPUPercent.recordDataPoint(mb.startTime, ts, val) -} - // RecordContainerCPUThrottlingDataPeriodsDataPoint adds a data point to container.cpu.throttling_data.periods metric. func (mb *MetricsBuilder) RecordContainerCPUThrottlingDataPeriodsDataPoint(ts pcommon.Timestamp, val int64) { mb.metricContainerCPUThrottlingDataPeriods.recordDataPoint(mb.startTime, ts, val) diff --git a/receiver/dockerstatsreceiver/internal/metadata/generated_metrics_test.go b/receiver/dockerstatsreceiver/internal/metadata/generated_metrics_test.go index 894c7f3003cd..6fe3e0d65ced 100644 --- a/receiver/dockerstatsreceiver/internal/metadata/generated_metrics_test.go +++ b/receiver/dockerstatsreceiver/internal/metadata/generated_metrics_test.go @@ -49,10 +49,6 @@ func TestMetricsBuilder(t *testing.T) { mb := NewMetricsBuilder(loadMetricsBuilderConfig(t, test.name), settings, WithStartTime(start)) expectedWarnings := 0 - if test.configSet == testSetAll || test.configSet == testSetNone { - assert.Equal(t, "[WARNING] `container.cpu.percent` should not be configured: The metric is deprecated and will be removed in v0.89.0. Please use `container.cpu.utilization` instead. See https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/dockerstatsreceiver#transition-to-cpu-utilization-metric-name-aligned-with-opentelemetry-specification for more details.", observedLogs.All()[expectedWarnings].Message) - expectedWarnings++ - } assert.Equal(t, expectedWarnings, observedLogs.Len()) @@ -84,9 +80,6 @@ func TestMetricsBuilder(t *testing.T) { allMetricsCount++ mb.RecordContainerBlockioSectorsRecursiveDataPoint(ts, 1, "device_major-val", "device_minor-val", "operation-val") - allMetricsCount++ - mb.RecordContainerCPUPercentDataPoint(ts, 1) - allMetricsCount++ mb.RecordContainerCPUThrottlingDataPeriodsDataPoint(ts, 1) @@ -488,18 +481,6 @@ func TestMetricsBuilder(t *testing.T) { attrVal, ok = dp.Attributes().Get("operation") assert.True(t, ok) assert.EqualValues(t, "operation-val", attrVal.Str()) - case "container.cpu.percent": - assert.False(t, validatedMetrics["container.cpu.percent"], "Found a duplicate in the metrics slice: container.cpu.percent") - validatedMetrics["container.cpu.percent"] = true - assert.Equal(t, pmetric.MetricTypeGauge, ms.At(i).Type()) - assert.Equal(t, 1, ms.At(i).Gauge().DataPoints().Len()) - assert.Equal(t, "[DEPRECATED] Use `container.cpu.utilization` metric instead. Percent of CPU used by the container.", ms.At(i).Description()) - assert.Equal(t, "1", ms.At(i).Unit()) - dp := ms.At(i).Gauge().DataPoints().At(0) - assert.Equal(t, start, dp.StartTimestamp()) - assert.Equal(t, ts, dp.Timestamp()) - assert.Equal(t, pmetric.NumberDataPointValueTypeDouble, dp.ValueType()) - assert.Equal(t, float64(1), dp.DoubleValue()) case "container.cpu.throttling_data.periods": assert.False(t, validatedMetrics["container.cpu.throttling_data.periods"], "Found a duplicate in the metrics slice: container.cpu.throttling_data.periods") validatedMetrics["container.cpu.throttling_data.periods"] = true diff --git a/receiver/dockerstatsreceiver/internal/metadata/testdata/config.yaml b/receiver/dockerstatsreceiver/internal/metadata/testdata/config.yaml index 4f2c619b47a1..8f9f8a706831 100644 --- a/receiver/dockerstatsreceiver/internal/metadata/testdata/config.yaml +++ b/receiver/dockerstatsreceiver/internal/metadata/testdata/config.yaml @@ -17,8 +17,6 @@ all_set: enabled: true container.blockio.sectors_recursive: enabled: true - container.cpu.percent: - enabled: true container.cpu.throttling_data.periods: enabled: true container.cpu.throttling_data.throttled_periods: @@ -168,8 +166,6 @@ none_set: enabled: false container.blockio.sectors_recursive: enabled: false - container.cpu.percent: - enabled: false container.cpu.throttling_data.periods: enabled: false container.cpu.throttling_data.throttled_periods: diff --git a/receiver/dockerstatsreceiver/metadata.yaml b/receiver/dockerstatsreceiver/metadata.yaml index 815476ca37c5..df6e075f0458 100644 --- a/receiver/dockerstatsreceiver/metadata.yaml +++ b/receiver/dockerstatsreceiver/metadata.yaml @@ -137,19 +137,6 @@ metrics: unit: "1" gauge: value_type: double - container.cpu.percent: - enabled: false - description: "[DEPRECATED] Use `container.cpu.utilization` metric instead. Percent of CPU used by the container." - unit: "1" - warnings: - if_configured: >- - The metric is deprecated and will be removed in v0.89.0. Please use `container.cpu.utilization` instead. See - https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/dockerstatsreceiver#transition-to-cpu-utilization-metric-name-aligned-with-opentelemetry-specification - for more details. - gauge: - value_type: double - - # Memory container.memory.usage.limit: enabled: true diff --git a/receiver/dockerstatsreceiver/receiver.go b/receiver/dockerstatsreceiver/receiver.go index 72116695f73d..847305e54525 100644 --- a/receiver/dockerstatsreceiver/receiver.go +++ b/receiver/dockerstatsreceiver/receiver.go @@ -252,7 +252,6 @@ func (r *receiver) recordCPUMetrics(now pcommon.Timestamp, cpuStats *dtypes.CPUS r.mb.RecordContainerCPUThrottlingDataPeriodsDataPoint(now, int64(cpuStats.ThrottlingData.Periods)) r.mb.RecordContainerCPUThrottlingDataThrottledTimeDataPoint(now, int64(cpuStats.ThrottlingData.ThrottledTime)) r.mb.RecordContainerCPUUtilizationDataPoint(now, calculateCPUPercent(prevStats, cpuStats)) - r.mb.RecordContainerCPUPercentDataPoint(now, calculateCPUPercent(prevStats, cpuStats)) for coreNum, v := range cpuStats.CPUUsage.PercpuUsage { r.mb.RecordContainerCPUUsagePercpuDataPoint(now, int64(v), fmt.Sprintf("cpu%s", strconv.Itoa(coreNum)))