Skip to content

Commit e1f9ca1

Browse files
committed
fix(systemstatsmonitor): key disk_percent_used by device_name only
disk_percent_used declared only device_name but recorded four labels (device_name, fs_type, mount_option, state). With per-metric label validation, Record now rejects the undeclared labels and the metric is dropped from the export (caught by the metriconly e2e). Record only device_name, preserving the schema NPD has exported since the metric was introduced: the OpenCensus view aggregated by device_name and dropped the other tags, so on the wire this metric has always had a single label. It also maps to the Google-owned compute.googleapis.com/guest/disk/percent_used descriptor, which can reject writes carrying undeclared label keys. Add a regression test pinning the schema.
1 parent bb3180d commit e1f9ca1

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

pkg/systemstatsmonitor/disk_collector.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,13 @@ func (dc *diskCollector) collect() {
330330
klog.Errorf("Failed to record used bytes for %s: %v", deviceName, err)
331331
}
332332
if dc.mPercentUsed != nil {
333-
if err := dc.mPercentUsed.Record(map[string]string{deviceNameLabel: deviceName, fsTypeLabel: fstype, mountOptionLabel: opttypes, stateLabel: "used"}, float64(usageStat.UsedPercent)); err != nil {
333+
// disk_percent_used is keyed only by device_name, matching the
334+
// label schema NPD has exported since the metric was introduced
335+
// (the OpenCensus view aggregated by device_name and dropped the
336+
// other tags). This also keeps writes compatible with the
337+
// Google-owned compute.googleapis.com/guest/disk/percent_used
338+
// descriptor.
339+
if err := dc.mPercentUsed.Record(map[string]string{deviceNameLabel: deviceName}, float64(usageStat.UsedPercent)); err != nil {
334340
klog.Errorf("Failed to record used percent for %s: %v", deviceName, err)
335341
}
336342
}

pkg/systemstatsmonitor/disk_collector_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,44 @@ package systemstatsmonitor
1919
import (
2020
"testing"
2121

22+
sdkmetric "go.opentelemetry.io/otel/sdk/metric"
23+
2224
ssmtypes "k8s.io/node-problem-detector/pkg/systemstatsmonitor/types"
25+
"k8s.io/node-problem-detector/pkg/util/metrics"
26+
otelutil "k8s.io/node-problem-detector/pkg/util/otel"
2327
)
2428

2529
func TestDiskCollector(t *testing.T) {
2630
dc := NewDiskCollectorOrDie(&ssmtypes.DiskStatsConfig{})
2731
dc.collect()
2832
}
33+
34+
// TestDiskCollectorPercentUsedLabels pins the label schema of disk_percent_used
35+
// to device_name only. NPD has exported it that way since the metric was
36+
// introduced (the OpenCensus view aggregated by device_name), and it maps to the
37+
// Google-owned compute.googleapis.com/guest/disk/percent_used descriptor, which
38+
// can reject writes carrying undeclared label keys. Because Record now rejects
39+
// undeclared labels, widening the declared set is the only way collect() could
40+
// emit a wider schema — so the metric must accept device_name and reject more.
41+
func TestDiskCollectorPercentUsedLabels(t *testing.T) {
42+
otelutil.ResetForTesting()
43+
defer otelutil.ResetForTesting()
44+
otelutil.AddMetricReader(sdkmetric.NewManualReader())
45+
otelutil.InitializeMeterProvider()
46+
47+
dc := NewDiskCollectorOrDie(&ssmtypes.DiskStatsConfig{
48+
MetricsConfigs: map[string]ssmtypes.MetricConfig{
49+
string(metrics.DiskPercentUsedID): {DisplayName: string(metrics.DiskPercentUsedID)},
50+
},
51+
})
52+
53+
// collect() records disk_percent_used keyed only by device_name.
54+
if err := dc.mPercentUsed.Record(map[string]string{deviceNameLabel: "sda1"}, 42.0); err != nil {
55+
t.Fatalf("recording disk_percent_used with device_name failed: %v", err)
56+
}
57+
58+
// A wider label set must be rejected, keeping the exported schema stable.
59+
if err := dc.mPercentUsed.Record(map[string]string{deviceNameLabel: "sda1", fsTypeLabel: "ext4"}, 42.0); err == nil {
60+
t.Error("disk_percent_used accepted an fs_type label; its schema must stay device_name-only")
61+
}
62+
}

0 commit comments

Comments
 (0)