diff --git a/.chloggen/gbbr_fix-otel-namespace.yaml b/.chloggen/gbbr_fix-otel-namespace.yaml new file mode 100755 index 00000000..5333e382 --- /dev/null +++ b/.chloggen/gbbr_fix-otel-namespace.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component (e.g. pkg/quantile) +component: pkg/otlp + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: System (and process.*) metrics were remapped with otel.*, overwriting the original metric and losing it. This change fixes that, keeping the original metric. + +# The PR related to this change +issues: [141] + +# (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: diff --git a/pkg/otlp/metrics/metrics_remapping.go b/pkg/otlp/metrics/metrics_remapping.go index f49f1baa..327fa805 100644 --- a/pkg/otlp/metrics/metrics_remapping.go +++ b/pkg/otlp/metrics/metrics_remapping.go @@ -69,7 +69,12 @@ func remapSystemMetrics(all pmetric.MetricSlice, m pmetric.Metric) { case "system.filesystem.utilization": copyMetric(all, m, "system.disk.in_use", 1) } - // process.* and system.* metrics need to be prepended with the otel.* namespace + if strings.HasPrefix(name, "system.") { + // we keep the original metric for system.* metrics + newm := all.AppendEmpty() + m.CopyTo(newm) + } + // and we prepend all of them with the otel.* namespace namespace m.SetName("otel." + m.Name()) } diff --git a/pkg/otlp/metrics/metrics_remapping_test.go b/pkg/otlp/metrics/metrics_remapping_test.go index 1bef88b7..7c144e56 100644 --- a/pkg/otlp/metrics/metrics_remapping_test.go +++ b/pkg/otlp/metrics/metrics_remapping_test.go @@ -55,22 +55,30 @@ func TestRemapMetrics(t *testing.T) { return m } - dest := pmetric.NewMetricSlice() for _, tt := range []struct { in pmetric.Metric out []pmetric.Metric }{ { - in: metric("system.cpu.load_average.1m", point{f: 1}), - out: []pmetric.Metric{metric("system.load.1", point{f: 1})}, + in: metric("system.cpu.load_average.1m", point{f: 1}), + out: []pmetric.Metric{ + metric("system.load.1", point{f: 1}), + metric("system.cpu.load_average.1m", point{f: 1}), + }, }, { - in: metric("system.cpu.load_average.5m", point{f: 5}), - out: []pmetric.Metric{metric("system.load.5", point{f: 5})}, + in: metric("system.cpu.load_average.5m", point{f: 5}), + out: []pmetric.Metric{ + metric("system.load.5", point{f: 5}), + metric("system.cpu.load_average.5m", point{f: 5}), + }, }, { - in: metric("system.cpu.load_average.15m", point{f: 15}), - out: []pmetric.Metric{metric("system.load.15", point{f: 15})}, + in: metric("system.cpu.load_average.15m", point{f: 15}), + out: []pmetric.Metric{ + metric("system.load.15", point{f: 15}), + metric("system.cpu.load_average.15m", point{f: 15}), + }, }, { in: metric("system.cpu.utilization", @@ -92,11 +100,22 @@ func TestRemapMetrics(t *testing.T) { point{f: 500, attrs: map[string]any{"state": "wait"}}), metric("system.cpu.stolen", point{f: 800, attrs: map[string]any{"state": "steal"}}), + metric("system.cpu.utilization", + point{f: 1, attrs: map[string]any{"state": "idle"}}, + point{f: 2, attrs: map[string]any{"state": "user"}}, + point{f: 3, attrs: map[string]any{"state": "system"}}, + point{f: 5, attrs: map[string]any{"state": "wait"}}, + point{f: 8, attrs: map[string]any{"state": "steal"}}, + point{f: 13, attrs: map[string]any{"state": "other"}}, + ), }, }, { - in: metric("system.cpu.utilization", point{i: 5, attrs: map[string]any{"state": "idle"}}), - out: []pmetric.Metric{metric("system.cpu.idle", point{i: 5, attrs: map[string]any{"state": "idle"}})}, + in: metric("system.cpu.utilization", point{i: 5, attrs: map[string]any{"state": "idle"}}), + out: []pmetric.Metric{ + metric("system.cpu.idle", point{i: 5, attrs: map[string]any{"state": "idle"}}), + metric("system.cpu.utilization", point{i: 5, attrs: map[string]any{"state": "idle"}}), + }, }, { in: metric("system.memory.usage", @@ -121,11 +140,22 @@ func TestRemapMetrics(t *testing.T) { point{f: 2, attrs: map[string]any{"state": "cached"}}, point{f: 3, attrs: map[string]any{"state": "buffered"}}, ), + metric("system.memory.usage", + point{f: divMebibytes * 1, attrs: map[string]any{"state": "free"}}, + point{f: divMebibytes * 2, attrs: map[string]any{"state": "cached"}}, + point{f: divMebibytes * 3, attrs: map[string]any{"state": "buffered"}}, + point{f: divMebibytes * 5, attrs: map[string]any{"state": "steal"}}, + point{f: divMebibytes * 8, attrs: map[string]any{"state": "system"}}, + point{f: divMebibytes * 13, attrs: map[string]any{"state": "user"}}, + ), }, }, { - in: metric("system.memory.usage", point{i: divMebibytes * 5}), - out: []pmetric.Metric{metric("system.mem.total", point{i: 5})}, + in: metric("system.memory.usage", point{i: divMebibytes * 5}), + out: []pmetric.Metric{ + metric("system.mem.total", point{i: 5}), + metric("system.memory.usage", point{i: divMebibytes * 5}), + }, }, { in: metric("system.network.io", @@ -140,6 +170,11 @@ func TestRemapMetrics(t *testing.T) { metric("system.net.bytes_sent", point{f: 2, attrs: map[string]any{"direction": "transmit"}}, ), + metric("system.network.io", + point{f: 1, attrs: map[string]any{"direction": "receive"}}, + point{f: 2, attrs: map[string]any{"direction": "transmit"}}, + point{f: 3, attrs: map[string]any{"state": "buffered"}}, + ), }, }, { @@ -155,11 +190,23 @@ func TestRemapMetrics(t *testing.T) { metric("system.swap.used", point{f: 2, attrs: map[string]any{"state": "used"}}, ), + metric("system.paging.usage", + point{f: divMebibytes * 1, attrs: map[string]any{"state": "free"}}, + point{f: divMebibytes * 2, attrs: map[string]any{"state": "used"}}, + point{f: 3, attrs: map[string]any{"state": "buffered"}}, + ), + }, + }, + { + in: metric("system.filesystem.utilization", point{f: 15}), + out: []pmetric.Metric{ + metric("system.disk.in_use", point{f: 15}), + metric("system.filesystem.utilization", point{f: 15}), }, }, { - in: metric("system.filesystem.utilization", point{f: 15}), - out: []pmetric.Metric{metric("system.disk.in_use", point{f: 15})}, + in: metric("process.metrics", point{f: 15}), + out: []pmetric.Metric{}, }, { in: metric("other.metric", point{f: 15}), @@ -275,16 +322,20 @@ func TestRemapMetrics(t *testing.T) { out: []pmetric.Metric{metric("container.net.rcvd.packets", point{f: 15})}, }, } { - lena := dest.Len() - checkprefix := strings.HasPrefix(tt.in.Name(), "system.") || strings.HasPrefix(tt.in.Name(), "process.") - remapMetrics(dest, tt.in) - if checkprefix { - require.True(t, strings.HasPrefix(tt.in.Name(), "otel."), "system.* and process.* metrics need to be prepended with the otel.* namespace") - } - require.Equal(t, dest.Len()-lena, len(tt.out), "unexpected number of metrics added") - for i, out := range tt.out { - require.Equal(t, out, dest.At(dest.Len()-len(tt.out)+i)) - } + t.Run("", func(t *testing.T) { + dest := pmetric.NewMetricSlice() + name := tt.in.Name() + shouldprepend := strings.HasPrefix(tt.in.Name(), "system.") || strings.HasPrefix(tt.in.Name(), "process.") + remapMetrics(dest, tt.in) + if shouldprepend { + // these metrics need to be prepended with otel.* + require.Equal(t, tt.in.Name(), "otel."+name) + } + require.Equal(t, len(tt.out), dest.Len()) + for i, o := range tt.out { + require.Equal(t, o, dest.At(i)) + } + }) } }