From 0982234bfc5ab0964e9b4b5b3c77ae9a31ac688e Mon Sep 17 00:00:00 2001 From: IbraAoad Date: Tue, 18 Feb 2025 11:12:25 +0200 Subject: [PATCH 1/6] feat: add SystemdVirtualization type Signed-off-by: IbraAoad --- collector/systemd_linux.go | 37 +++++++++++++++++++-- collector/systemd_linux_test.go | 59 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/collector/systemd_linux.go b/collector/systemd_linux.go index ee2ded8d4a..2ee1835602 100644 --- a/collector/systemd_linux.go +++ b/collector/systemd_linux.go @@ -74,6 +74,7 @@ type systemdCollector struct { socketCurrentConnectionsDesc *prometheus.Desc socketRefusedConnectionsDesc *prometheus.Desc systemdVersionDesc *prometheus.Desc + virtualizationDesc *prometheus.Desc // Use regexps for more flexibility than device_filter.go allows systemdUnitIncludePattern *regexp.Regexp systemdUnitExcludePattern *regexp.Regexp @@ -82,6 +83,10 @@ type systemdCollector struct { var unitStatesName = []string{"active", "activating", "deactivating", "inactive", "failed"} +var getManagerPropertyFunc = func(conn *dbus.Conn, name string) (string, error) { + return conn.GetManagerProperty(name) +} + func init() { registerCollector("systemd", defaultDisabled, NewSystemdCollector) } @@ -132,6 +137,9 @@ func NewSystemdCollector(logger *slog.Logger) (Collector, error) { systemdVersionDesc := prometheus.NewDesc( prometheus.BuildFQName(namespace, subsystem, "version"), "Detected systemd version", []string{"version"}, nil) + virtualizationDesc := prometheus.NewDesc( + prometheus.BuildFQName(namespace, subsystem, "virtualization"), + "Detected virtualization technology", []string{"type"}, nil) if *oldSystemdUnitExclude != "" { if !systemdUnitExcludeSet { @@ -167,6 +175,7 @@ func NewSystemdCollector(logger *slog.Logger) (Collector, error) { socketCurrentConnectionsDesc: socketCurrentConnectionsDesc, socketRefusedConnectionsDesc: socketRefusedConnectionsDesc, systemdVersionDesc: systemdVersionDesc, + virtualizationDesc: virtualizationDesc, systemdUnitIncludePattern: systemdUnitIncludePattern, systemdUnitExcludePattern: systemdUnitExcludePattern, logger: logger, @@ -194,6 +203,14 @@ func (c *systemdCollector) Update(ch chan<- prometheus.Metric) error { systemdVersionFull, ) + systemdVirtualization := c.getSystemdVirtualization(conn) + ch <- prometheus.MustNewConstMetric( + c.virtualizationDesc, + prometheus.GaugeValue, + 1.0, + systemdVirtualization, + ) + allUnits, err := c.getAllUnits(conn) if err != nil { return fmt.Errorf("couldn't get units: %w", err) @@ -421,7 +438,7 @@ func (c *systemdCollector) collectSummaryMetrics(ch chan<- prometheus.Metric, su } func (c *systemdCollector) collectSystemState(conn *dbus.Conn, ch chan<- prometheus.Metric) error { - systemState, err := conn.GetManagerProperty("SystemState") + systemState, err := getManagerPropertyFunc(conn, "SystemState") if err != nil { return fmt.Errorf("couldn't get system state: %w", err) } @@ -490,7 +507,7 @@ func filterUnits(units []unit, includePattern, excludePattern *regexp.Regexp, lo } func (c *systemdCollector) getSystemdVersion(conn *dbus.Conn) (float64, string) { - version, err := conn.GetManagerProperty("Version") + version, err := getManagerPropertyFunc(conn, "Version") if err != nil { c.logger.Debug("Unable to get systemd version property, defaulting to 0") return 0, "" @@ -505,3 +522,19 @@ func (c *systemdCollector) getSystemdVersion(conn *dbus.Conn) (float64, string) } return v, version } + +func (c *systemdCollector) getSystemdVirtualization(conn *dbus.Conn) string { + virt, err := getManagerPropertyFunc(conn, "Virtualization") + if err != nil { + c.logger.Debug("Could not get Virtualization property", "err", err) + return "unknown" + } + + virtStr := strings.Trim(virt, `"`) + if virtStr == "" { + // If no virtualization type is returned, assume it's bare metal. + return "none" + } + + return virtStr +} diff --git a/collector/systemd_linux_test.go b/collector/systemd_linux_test.go index 1c290377e6..92aa848ee4 100644 --- a/collector/systemd_linux_test.go +++ b/collector/systemd_linux_test.go @@ -17,6 +17,7 @@ package collector import ( + "errors" "io" "log/slog" "regexp" @@ -137,3 +138,61 @@ func testSummaryHelper(t *testing.T, state string, actual float64, expected floa t.Errorf("Summary mode didn't count %s jobs correctly. Actual: %f, expected: %f", state, actual, expected) } } + +func Test_systemdCollector_getSystemdVirtualization(t *testing.T) { + type fields struct { + logger *slog.Logger + } + type args struct { + conn *dbus.Conn + } + tests := []struct { + name string + fields fields + args args + mock func(conn *dbus.Conn, name string) (string, error) + want string + }{ + { + name: "Error", + fields: fields{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}, + args: args{conn: &dbus.Conn{}}, + mock: func(conn *dbus.Conn, name string) (string, error) { + return "", errors.New("test error") + }, + want: "unknown", + }, + { + name: "Empty", + fields: fields{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}, + args: args{conn: &dbus.Conn{}}, + mock: func(conn *dbus.Conn, name string) (string, error) { + return `""`, nil + }, + want: "none", + }, + { + name: "Valid", + fields: fields{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}, + args: args{conn: &dbus.Conn{}}, + mock: func(conn *dbus.Conn, name string) (string, error) { + return `"kvm"`, nil + }, + want: "kvm", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + origFunc := getManagerPropertyFunc + defer func() { getManagerPropertyFunc = origFunc }() + getManagerPropertyFunc = tt.mock + + c := &systemdCollector{ + logger: tt.fields.logger, + } + if got := c.getSystemdVirtualization(tt.args.conn); got != tt.want { + t.Errorf("systemdCollector.getSystemdVirtualization() = %v, want %v", got, tt.want) + } + }) + } +} From 592c1c7f2982baa239194facbcf81309affa1e84 Mon Sep 17 00:00:00 2001 From: Ibrahim Awwad Date: Tue, 18 Feb 2025 12:56:53 +0200 Subject: [PATCH 2/6] Update collector/systemd_linux.go Co-authored-by: Ben Kochie Signed-off-by: Ibrahim Awwad Signed-off-by: IbraAoad --- collector/systemd_linux.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/collector/systemd_linux.go b/collector/systemd_linux.go index 2ee1835602..f73828ef3d 100644 --- a/collector/systemd_linux.go +++ b/collector/systemd_linux.go @@ -138,8 +138,8 @@ func NewSystemdCollector(logger *slog.Logger) (Collector, error) { prometheus.BuildFQName(namespace, subsystem, "version"), "Detected systemd version", []string{"version"}, nil) virtualizationDesc := prometheus.NewDesc( - prometheus.BuildFQName(namespace, subsystem, "virtualization"), - "Detected virtualization technology", []string{"type"}, nil) + prometheus.BuildFQName(namespace, subsystem, "virtualization_info"), + "Detected virtualization technology", []string{"virtualization_type"}, nil) if *oldSystemdUnitExclude != "" { if !systemdUnitExcludeSet { From 200c72e5794c49064f7c5e5588dfda5600546ec0 Mon Sep 17 00:00:00 2001 From: Mikel Olasagasti Uranga Date: Wed, 19 Feb 2025 12:34:16 +0100 Subject: [PATCH 3/6] build(deps): remove leftover from #3160 (#3255) Signed-off-by: Mikel Olasagasti Uranga Signed-off-by: IbraAoad --- go.mod | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.mod b/go.mod index 02e01d54de..d8318e1208 100644 --- a/go.mod +++ b/go.mod @@ -59,5 +59,3 @@ require ( google.golang.org/protobuf v1.36.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect ) - -//replace github.com/prometheus/procfs => github.com/rexagod/procfs v0.0.0-20241124020414-857c5b813f1b From 179c5932720203e08047919791b5541c78489067 Mon Sep 17 00:00:00 2001 From: Alexander Soelberg Heidarsson <89837986+alex5517@users.noreply.github.com> Date: Wed, 19 Feb 2025 15:37:09 +0100 Subject: [PATCH 4/6] bugfix: :bug: remove invalid variable from cluster use dashboards Signed-off-by: Alexander Soelberg Heidarsson <89837986+alex5517@users.noreply.github.com> Signed-off-by: IbraAoad --- docs/node-mixin/dashboards/use.libsonnet | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs/node-mixin/dashboards/use.libsonnet b/docs/node-mixin/dashboards/use.libsonnet index f9d9e07cf6..cfdaf41604 100644 --- a/docs/node-mixin/dashboards/use.libsonnet +++ b/docs/node-mixin/dashboards/use.libsonnet @@ -200,9 +200,6 @@ local diskSpaceUtilisation = + dashboard.withVariables([ datasource, $._clusterVariable, - variable.query.withDatasourceFromVariable(datasource) - + variable.query.refresh.onTime() - + variable.query.withSort(asc=true), ]) + dashboard.withPanels( grafana.util.grid.makeGrid([ @@ -331,9 +328,6 @@ local diskSpaceUtilisation = + dashboard.withUid(std.md5('node-multicluster-rsrc-use.json')) + dashboard.withVariables([ datasource, - variable.query.withDatasourceFromVariable(datasource) - + variable.query.refresh.onTime() - + variable.query.withSort(asc=true), ]) + dashboard.withPanels( grafana.util.grid.makeGrid([ From 0575acc601db877509bb317d282af6268c535af4 Mon Sep 17 00:00:00 2001 From: IbraAoad Date: Thu, 20 Feb 2025 16:58:58 +0200 Subject: [PATCH 5/6] refactor(systemdCollector): use dependency injection via Manager interface Signed-off-by: IbraAoad --- collector/systemd_linux.go | 16 ++++---- collector/systemd_linux_test.go | 67 ++++++++++++++++----------------- 2 files changed, 41 insertions(+), 42 deletions(-) diff --git a/collector/systemd_linux.go b/collector/systemd_linux.go index f73828ef3d..40c7843821 100644 --- a/collector/systemd_linux.go +++ b/collector/systemd_linux.go @@ -83,8 +83,8 @@ type systemdCollector struct { var unitStatesName = []string{"active", "activating", "deactivating", "inactive", "failed"} -var getManagerPropertyFunc = func(conn *dbus.Conn, name string) (string, error) { - return conn.GetManagerProperty(name) +type Manager interface { + GetManagerProperty(prop string) (string, error) } func init() { @@ -437,8 +437,8 @@ func (c *systemdCollector) collectSummaryMetrics(ch chan<- prometheus.Metric, su } } -func (c *systemdCollector) collectSystemState(conn *dbus.Conn, ch chan<- prometheus.Metric) error { - systemState, err := getManagerPropertyFunc(conn, "SystemState") +func (c *systemdCollector) collectSystemState(m Manager, ch chan<- prometheus.Metric) error { + systemState, err := m.GetManagerProperty("SystemState") if err != nil { return fmt.Errorf("couldn't get system state: %w", err) } @@ -506,8 +506,8 @@ func filterUnits(units []unit, includePattern, excludePattern *regexp.Regexp, lo return filtered } -func (c *systemdCollector) getSystemdVersion(conn *dbus.Conn) (float64, string) { - version, err := getManagerPropertyFunc(conn, "Version") +func (c *systemdCollector) getSystemdVersion(m Manager) (float64, string) { + version, err := m.GetManagerProperty("Version") if err != nil { c.logger.Debug("Unable to get systemd version property, defaulting to 0") return 0, "" @@ -523,8 +523,8 @@ func (c *systemdCollector) getSystemdVersion(conn *dbus.Conn) (float64, string) return v, version } -func (c *systemdCollector) getSystemdVirtualization(conn *dbus.Conn) string { - virt, err := getManagerPropertyFunc(conn, "Virtualization") +func (c *systemdCollector) getSystemdVirtualization(m Manager) string { + virt, err := m.GetManagerProperty("Virtualization") if err != nil { c.logger.Debug("Could not get Virtualization property", "err", err) return "unknown" diff --git a/collector/systemd_linux_test.go b/collector/systemd_linux_test.go index 92aa848ee4..0ac0bdadb3 100644 --- a/collector/systemd_linux_test.go +++ b/collector/systemd_linux_test.go @@ -139,59 +139,58 @@ func testSummaryHelper(t *testing.T, state string, actual float64, expected floa } } +// fakeManager implements the Manager interface for testing. +type fakeManager struct { + result string + err error +} + +// GetManagerProperty returns the controlled result for tests. +func (f *fakeManager) GetManagerProperty(prop string) (string, error) { + return f.result, f.err +} + func Test_systemdCollector_getSystemdVirtualization(t *testing.T) { - type fields struct { - logger *slog.Logger - } - type args struct { - conn *dbus.Conn + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + c, err := NewSystemdCollector(logger) + if err != nil { + t.Fatal(err) } + sysdCollector := c.(*systemdCollector) + tests := []struct { - name string - fields fields - args args - mock func(conn *dbus.Conn, name string) (string, error) - want string + name string + fake *fakeManager + want string }{ { - name: "Error", - fields: fields{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}, - args: args{conn: &dbus.Conn{}}, - mock: func(conn *dbus.Conn, name string) (string, error) { - return "", errors.New("test error") + name: "Error", + fake: &fakeManager{ + err: errors.New("test error"), }, want: "unknown", }, { - name: "Empty", - fields: fields{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}, - args: args{conn: &dbus.Conn{}}, - mock: func(conn *dbus.Conn, name string) (string, error) { - return `""`, nil + name: "Empty", + fake: &fakeManager{ + result: `""`, }, want: "none", }, { - name: "Valid", - fields: fields{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}, - args: args{conn: &dbus.Conn{}}, - mock: func(conn *dbus.Conn, name string) (string, error) { - return `"kvm"`, nil + name: "Valid", + fake: &fakeManager{ + result: `"kvm"`, }, want: "kvm", }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - origFunc := getManagerPropertyFunc - defer func() { getManagerPropertyFunc = origFunc }() - getManagerPropertyFunc = tt.mock - - c := &systemdCollector{ - logger: tt.fields.logger, - } - if got := c.getSystemdVirtualization(tt.args.conn); got != tt.want { - t.Errorf("systemdCollector.getSystemdVirtualization() = %v, want %v", got, tt.want) + got := sysdCollector.getSystemdVirtualization(tt.fake) + if got != tt.want { + t.Errorf("getSystemdVirtualization() = %v, want %v", got, tt.want) } }) } From fa72170992f5990f130eebdc5fb6e12a24d037b6 Mon Sep 17 00:00:00 2001 From: IbraAoad Date: Thu, 20 Feb 2025 17:26:50 +0200 Subject: [PATCH 6/6] refactor(systemdCollector): drop utests Signed-off-by: IbraAoad --- collector/systemd_linux.go | 16 ++++----- collector/systemd_linux_test.go | 58 --------------------------------- 2 files changed, 6 insertions(+), 68 deletions(-) diff --git a/collector/systemd_linux.go b/collector/systemd_linux.go index 40c7843821..a21cc3cb3f 100644 --- a/collector/systemd_linux.go +++ b/collector/systemd_linux.go @@ -83,10 +83,6 @@ type systemdCollector struct { var unitStatesName = []string{"active", "activating", "deactivating", "inactive", "failed"} -type Manager interface { - GetManagerProperty(prop string) (string, error) -} - func init() { registerCollector("systemd", defaultDisabled, NewSystemdCollector) } @@ -437,8 +433,8 @@ func (c *systemdCollector) collectSummaryMetrics(ch chan<- prometheus.Metric, su } } -func (c *systemdCollector) collectSystemState(m Manager, ch chan<- prometheus.Metric) error { - systemState, err := m.GetManagerProperty("SystemState") +func (c *systemdCollector) collectSystemState(conn *dbus.Conn, ch chan<- prometheus.Metric) error { + systemState, err := conn.GetManagerProperty("SystemState") if err != nil { return fmt.Errorf("couldn't get system state: %w", err) } @@ -506,8 +502,8 @@ func filterUnits(units []unit, includePattern, excludePattern *regexp.Regexp, lo return filtered } -func (c *systemdCollector) getSystemdVersion(m Manager) (float64, string) { - version, err := m.GetManagerProperty("Version") +func (c *systemdCollector) getSystemdVersion(conn *dbus.Conn) (float64, string) { + version, err := conn.GetManagerProperty("Version") if err != nil { c.logger.Debug("Unable to get systemd version property, defaulting to 0") return 0, "" @@ -523,8 +519,8 @@ func (c *systemdCollector) getSystemdVersion(m Manager) (float64, string) { return v, version } -func (c *systemdCollector) getSystemdVirtualization(m Manager) string { - virt, err := m.GetManagerProperty("Virtualization") +func (c *systemdCollector) getSystemdVirtualization(conn *dbus.Conn) string { + virt, err := conn.GetManagerProperty("Virtualization") if err != nil { c.logger.Debug("Could not get Virtualization property", "err", err) return "unknown" diff --git a/collector/systemd_linux_test.go b/collector/systemd_linux_test.go index 0ac0bdadb3..1c290377e6 100644 --- a/collector/systemd_linux_test.go +++ b/collector/systemd_linux_test.go @@ -17,7 +17,6 @@ package collector import ( - "errors" "io" "log/slog" "regexp" @@ -138,60 +137,3 @@ func testSummaryHelper(t *testing.T, state string, actual float64, expected floa t.Errorf("Summary mode didn't count %s jobs correctly. Actual: %f, expected: %f", state, actual, expected) } } - -// fakeManager implements the Manager interface for testing. -type fakeManager struct { - result string - err error -} - -// GetManagerProperty returns the controlled result for tests. -func (f *fakeManager) GetManagerProperty(prop string) (string, error) { - return f.result, f.err -} - -func Test_systemdCollector_getSystemdVirtualization(t *testing.T) { - logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - c, err := NewSystemdCollector(logger) - if err != nil { - t.Fatal(err) - } - sysdCollector := c.(*systemdCollector) - - tests := []struct { - name string - fake *fakeManager - want string - }{ - { - name: "Error", - fake: &fakeManager{ - err: errors.New("test error"), - }, - want: "unknown", - }, - { - name: "Empty", - fake: &fakeManager{ - result: `""`, - }, - want: "none", - }, - { - name: "Valid", - fake: &fakeManager{ - result: `"kvm"`, - }, - want: "kvm", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := sysdCollector.getSystemdVirtualization(tt.fake) - if got != tt.want { - t.Errorf("getSystemdVirtualization() = %v, want %v", got, tt.want) - } - }) - } -}