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 <benjamin.drung@ionos.com>

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_`)
+	metricNameRegex        = regexp.MustCompile(`_*[^0-9A-Za-z_]+_*`)
-	transmittedRegex       = regexp.MustCompile(`_tx_`)
+	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 := buildEthtoolFQName(metric)
-			metricFQName = receivedRegex.ReplaceAllString(metricFQName, "_received_")
-			metricFQName = transmittedRegex.ReplaceAllString(metricFQName, "_transmitted_")
 
 			// Check to see if this metric exists; if not then create it and store it in c.entries.
 			entry, exists := c.entries[metric]

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(),