From 6ac6ea2d13decf75b7bb138a84ad7603f5c906c6 Mon Sep 17 00:00:00 2001 From: Benjamin Drung Date: Wed, 21 Jul 2021 20:23:11 +0200 Subject: [PATCH] ethtool: Sanitize metric names OpenMetrics and the Prometheus exposition format require the metric name to consist only of alphanumericals and "_", ":" and they must not start with digits. The metric names from the ethtool stats might contain spaces, brackets, and dots. Converting them directly to metric names will produce invalid metric names. Therefore sanitize the metric names and convert them to lower case. Fixes: https://github.com/prometheus/node_exporter/issues/2083 Signed-off-by: Benjamin Drung --- collector/ethtool_linux.go | 33 ++++++++++++++++++++++++++----- collector/ethtool_linux_test.go | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/collector/ethtool_linux.go b/collector/ethtool_linux.go index 058462ce..f00ed639 100644 --- a/collector/ethtool_linux.go +++ b/collector/ethtool_linux.go @@ -25,6 +25,7 @@ import ( "os" "regexp" "sort" + "strings" "syscall" "github.com/go-kit/log" @@ -39,8 +40,9 @@ import ( var ( ethtoolIgnoredDevices = kingpin.Flag("collector.ethtool.ignored-devices", "Regexp of net devices to ignore for ethtool collector.").Default("^$").String() ethtoolIncludedMetrics = kingpin.Flag("collector.ethtool.metrics-include", "Regexp of ethtool stats to include.").Default(".*").String() - receivedRegex = regexp.MustCompile(`_rx_`) - transmittedRegex = regexp.MustCompile(`_tx_`) + metricNameRegex = regexp.MustCompile(`_*[^0-9A-Za-z_]+_*`) + receivedRegex = regexp.MustCompile(`(^|_)rx(_|$)`) + transmittedRegex = regexp.MustCompile(`(^|_)tx(_|$)`) ) type EthtoolStats interface { @@ -123,6 +125,29 @@ func init() { registerCollector("ethtool", defaultDisabled, NewEthtoolCollector) } +// Sanitize the given metric name by replacing invalid characters by underscores. +// +// OpenMetrics and the Prometheus exposition format require the metric name +// to consist only of alphanumericals and "_", ":" and they must not start +// with digits. Since colons in MetricFamily are reserved to signal that the +// MetricFamily is the result of a calculation or aggregation of a general +// purpose monitoring system, colons will be replaced as well. +// +// Note: If not subsequently prepending a namespace and/or subsystem (e.g., +// with prometheus.BuildFQName), the caller must ensure that the supplied +// metricName does not begin with a digit. +func SanitizeMetricName(metricName string) string { + return metricNameRegex.ReplaceAllString(metricName, "_") +} + +// Generate the fully-qualified metric name for the ethool metric. +func buildEthtoolFQName(metric string) string { + metricName := strings.TrimLeft(strings.ToLower(SanitizeMetricName(metric)), "_") + metricName = receivedRegex.ReplaceAllString(metricName, "${1}received${2}") + metricName = transmittedRegex.ReplaceAllString(metricName, "${1}transmitted${2}") + return prometheus.BuildFQName(namespace, "ethtool", metricName) +} + // NewEthtoolCollector returns a new Collector exposing ethtool stats. func NewEthtoolCollector(logger log.Logger) (Collector, error) { return makeEthtoolCollector(logger) @@ -183,9 +208,7 @@ func (c *ethtoolCollector) Update(ch chan<- prometheus.Metric) error { continue } val := stats[metric] - metricFQName := prometheus.BuildFQName(namespace, "ethtool", metric) - metricFQName = receivedRegex.ReplaceAllString(metricFQName, "_received_") - metricFQName = transmittedRegex.ReplaceAllString(metricFQName, "_transmitted_") + metricFQName := buildEthtoolFQName(metric) // Check to see if this metric exists; if not then create it and store it in c.entries. entry, exists := c.entries[metric] diff --git a/collector/ethtool_linux_test.go b/collector/ethtool_linux_test.go index f2a4e087..8a96c5e2 100644 --- a/collector/ethtool_linux_test.go +++ b/collector/ethtool_linux_test.go @@ -81,6 +81,41 @@ func NewEthtoolTestCollector(logger log.Logger) (Collector, error) { return collector, nil } +func TestSanitizeMetricName(t *testing.T) { + testcases := map[string]string{ + "": "", + "rx_errors": "rx_errors", + "Queue[0] AllocFails": "Queue_0_AllocFails", + "Tx LPI entry count": "Tx_LPI_entry_count", + "port.VF_admin_queue_requests": "port_VF_admin_queue_requests", + "[3]: tx_bytes": "_3_tx_bytes", + } + + for metricName, expected := range testcases { + got := SanitizeMetricName(metricName) + if expected != got { + t.Errorf("Expected '%s' but got '%s'", expected, got) + } + } +} + +func TestBuildEthtoolFQName(t *testing.T) { + testcases := map[string]string{ + "rx_errors": "node_ethtool_received_errors", + "Queue[0] AllocFails": "node_ethtool_queue_0_allocfails", + "Tx LPI entry count": "node_ethtool_transmitted_lpi_entry_count", + "port.VF_admin_queue_requests": "node_ethtool_port_vf_admin_queue_requests", + "[3]: tx_bytes": "node_ethtool_3_transmitted_bytes", + } + + for metric, expected := range testcases { + got := buildEthtoolFQName(metric) + if expected != got { + t.Errorf("Expected '%s' but got '%s'", expected, got) + } + } +} + func TestEthtoolCollector(t *testing.T) { testcases := []string{ prometheus.NewDesc("node_ethtool_align_errors", "Network interface align_errors", []string{"device"}, nil).String(),