ethtool: Sanitize metric names by bdrung · Pull Request #2093 · prometheus/node_exporter

bdrung · 2021-07-21T17:32:51Z

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.

The function SanitizeMetricName will be useful for other exporters. Please let me know where the best place is for it. Maybe https://github.com/prometheus/common or https://github.com/prometheus/client_golang?

Fixes: #2083

collector/ethtool_linux.go

discordianfish

LGTM

SuperQ

LGTM.

Looks like this needs a rebase before merging.

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: prometheus#2083 Signed-off-by: Benjamin Drung <benjamin.drung@ionos.com>

bdrung · 2021-08-16T13:26:22Z

Rebased.

discordianfish · 2021-08-16T13:28:00Z

Thanks!

* [CHANGE] Add path label to rapl collector #2146 * [FEATURE] Add os release collector #2094 * [FEATURE] Add node_network_address collector #2105 * [ENHANCEMENT] ethtool: Expose node_ethtool_info metric #2080 * [ENHANCEMENT] Add collector.ethtool.metrics-include #2117 * [ENHANCEMENT] Add flag to disable guest CPU metrics #2123 * [BUGFIX] ethtool: Sanitize metric names #2093 * [BUGFIX] Fix ethtool collector for multiple interfaces #2126 * [BUGFIX] Fix possible panic on macOS #2133 * [BUGFIX] Collect flag_info and bug_info only for one core #2156 Signed-off-by: Ben Kochie <superq@gmail.com>

* [CHANGE] Add path label to rapl collector #2146 * [FEATURE] Add os release collector #2094 * [FEATURE] Add netdev.address-info collector #2105 * [ENHANCEMENT] ethtool: Expose node_ethtool_info metric #2080 * [ENHANCEMENT] Add collector.ethtool.metrics-include #2117 * [ENHANCEMENT] Add flag to disable guest CPU metrics #2123 * [BUGFIX] ethtool: Sanitize metric names #2093 * [BUGFIX] Fix ethtool collector for multiple interfaces #2126 * [BUGFIX] Fix possible panic on macOS #2133 * [BUGFIX] Collect flag_info and bug_info only for one core #2156 Signed-off-by: Ben Kochie <superq@gmail.com>

NOTE: In order to support globs in the textfile collector path, filenames exposed by `node_textfile_mtime_seconds` now contain the full path name. * [CHANGE] Add path label to rapl collector #2146 * [FEATURE] Add support for monitoring GPUs on Linux #1998 * [FEATURE] Add os release collector #2094 * [FEATURE] Add netdev.address-info collector #2105 * [ENHANCEMENT] Support glob textfile collector directories #1985 * [ENHANCEMENT] ethtool: Expose node_ethtool_info metric #2080 * [ENHANCEMENT] Use include/exclude flags for ethtool filtering #2165 * [ENHANCEMENT] Add flag to disable guest CPU metrics #2123 * [BUGFIX] ethtool: Sanitize metric names #2093 * [BUGFIX] Fix ethtool collector for multiple interfaces #2126 * [BUGFIX] Fix possible panic on macOS #2133 * [BUGFIX] Collect flag_info and bug_info only for one core #2156 Signed-off-by: Ben Kochie <superq@gmail.com>

NOTE: In order to support globs in the textfile collector path, filenames exposed by `node_textfile_mtime_seconds` now contain the full path name. * [CHANGE] Add path label to rapl collector #2146 * [FEATURE] Add support for monitoring GPUs on Linux #1998 * [FEATURE] Add os release collector #2094 * [FEATURE] Add netdev.address-info collector #2105 * [ENHANCEMENT] Support glob textfile collector directories #1985 * [ENHANCEMENT] ethtool: Expose node_ethtool_info metric #2080 * [ENHANCEMENT] Use include/exclude flags for ethtool filtering #2165 * [ENHANCEMENT] Add flag to disable guest CPU metrics #2123 * [ENHANCEMENT] Add threads metrics to processes collector #2164 * [BUGFIX] ethtool: Sanitize metric names #2093 * [BUGFIX] Fix ethtool collector for multiple interfaces #2126 * [BUGFIX] Fix possible panic on macOS #2133 * [BUGFIX] Collect flag_info and bug_info only for one core #2156 Signed-off-by: Ben Kochie <superq@gmail.com>

NOTE: In order to support globs in the textfile collector path, filenames exposed by `node_textfile_mtime_seconds` now contain the full path name. * [CHANGE] Add path label to rapl collector #2146 * [CHANGE] Exclude filesystems under /run/credentials #2157 * [FEATURE] Add support for monitoring GPUs on Linux #1998 * [FEATURE] Add Darwin thermal collector #2032 * [FEATURE] Add os release collector #2094 * [FEATURE] Add netdev.address-info collector #2105 * [ENHANCEMENT] Support glob textfile collector directories #1985 * [ENHANCEMENT] ethtool: Expose node_ethtool_info metric #2080 * [ENHANCEMENT] Use include/exclude flags for ethtool filtering #2165 * [ENHANCEMENT] Add flag to disable guest CPU metrics #2123 * [ENHANCEMENT] Add DMI collector #2131 * [ENHANCEMENT] Add threads metrics to processes collector #2164 * [ENHANCMMENT] Reduce timer GC delays in the Linux filesystem collector #2169 * [BUGFIX] ethtool: Sanitize metric names #2093 * [BUGFIX] Fix ethtool collector for multiple interfaces #2126 * [BUGFIX] Fix possible panic on macOS #2133 * [BUGFIX] Collect flag_info and bug_info only for one core #2156 Signed-off-by: Ben Kochie <superq@gmail.com>

NOTE: In order to support globs in the textfile collector path, filenames exposed by `node_textfile_mtime_seconds` now contain the full path name. * [CHANGE] Add path label to rapl collector #2146 * [CHANGE] Exclude filesystems under /run/credentials #2157 * [FEATURE] Add darwin powersupply collector #1777 * [FEATURE] Add support for monitoring GPUs on Linux #1998 * [FEATURE] Add Darwin thermal collector #2032 * [FEATURE] Add os release collector #2094 * [FEATURE] Add netdev.address-info collector #2105 * [ENHANCEMENT] Support glob textfile collector directories #1985 * [ENHANCEMENT] ethtool: Expose node_ethtool_info metric #2080 * [ENHANCEMENT] Use include/exclude flags for ethtool filtering #2165 * [ENHANCEMENT] Add flag to disable guest CPU metrics #2123 * [ENHANCEMENT] Add DMI collector #2131 * [ENHANCEMENT] Add threads metrics to processes collector #2164 * [ENHANCMMENT] Reduce timer GC delays in the Linux filesystem collector #2169 * [BUGFIX] ethtool: Sanitize metric names #2093 * [BUGFIX] Fix ethtool collector for multiple interfaces #2126 * [BUGFIX] Fix possible panic on macOS #2133 * [BUGFIX] Collect flag_info and bug_info only for one core #2156 Signed-off-by: Ben Kochie <superq@gmail.com>

NOTE: In order to support globs in the textfile collector path, filenames exposed by `node_textfile_mtime_seconds` now contain the full path name. * [CHANGE] Add path label to rapl collector #2146 * [CHANGE] Exclude filesystems under /run/credentials #2157 * [FEATURE] Add lnstat collector for metrics from /proc/net/stat/ #1771 * [FEATURE] Add darwin powersupply collector #1777 * [FEATURE] Add support for monitoring GPUs on Linux #1998 * [FEATURE] Add Darwin thermal collector #2032 * [FEATURE] Add os release collector #2094 * [FEATURE] Add netdev.address-info collector #2105 * [ENHANCEMENT] Support glob textfile collector directories #1985 * [ENHANCEMENT] ethtool: Expose node_ethtool_info metric #2080 * [ENHANCEMENT] Use include/exclude flags for ethtool filtering #2165 * [ENHANCEMENT] Add flag to disable guest CPU metrics #2123 * [ENHANCEMENT] Add DMI collector #2131 * [ENHANCEMENT] Add threads metrics to processes collector #2164 * [ENHANCMMENT] Reduce timer GC delays in the Linux filesystem collector #2169 * [BUGFIX] ethtool: Sanitize metric names #2093 * [BUGFIX] Fix ethtool collector for multiple interfaces #2126 * [BUGFIX] Fix possible panic on macOS #2133 * [BUGFIX] Collect flag_info and bug_info only for one core #2156 Signed-off-by: Ben Kochie <superq@gmail.com>

NOTE: In order to support globs in the textfile collector path, filenames exposed by `node_textfile_mtime_seconds` now contain the full path name. * [CHANGE] Add path label to rapl collector prometheus#2146 * [CHANGE] Exclude filesystems under /run/credentials prometheus#2157 * [FEATURE] Add lnstat collector for metrics from /proc/net/stat/ prometheus#1771 * [FEATURE] Add darwin powersupply collector prometheus#1777 * [FEATURE] Add support for monitoring GPUs on Linux prometheus#1998 * [FEATURE] Add Darwin thermal collector prometheus#2032 * [FEATURE] Add os release collector prometheus#2094 * [FEATURE] Add netdev.address-info collector prometheus#2105 * [ENHANCEMENT] Support glob textfile collector directories prometheus#1985 * [ENHANCEMENT] ethtool: Expose node_ethtool_info metric prometheus#2080 * [ENHANCEMENT] Use include/exclude flags for ethtool filtering prometheus#2165 * [ENHANCEMENT] Add flag to disable guest CPU metrics prometheus#2123 * [ENHANCEMENT] Add DMI collector prometheus#2131 * [ENHANCEMENT] Add threads metrics to processes collector prometheus#2164 * [ENHANCMMENT] Reduce timer GC delays in the Linux filesystem collector prometheus#2169 * [BUGFIX] ethtool: Sanitize metric names prometheus#2093 * [BUGFIX] Fix ethtool collector for multiple interfaces prometheus#2126 * [BUGFIX] Fix possible panic on macOS prometheus#2133 * [BUGFIX] Collect flag_info and bug_info only for one core prometheus#2156 Signed-off-by: Ben Kochie <superq@gmail.com>

dswarbrick reviewed Jul 21, 2021

View reviewed changes

collector/ethtool_linux.go Outdated Show resolved Hide resolved

bdrung force-pushed the fix-metric-names branch 2 times, most recently from 0b14086 to cb4b1bb Compare July 21, 2021 18:37

dswarbrick reviewed Jul 21, 2021

View reviewed changes

collector/ethtool_linux.go Outdated Show resolved Hide resolved

discordianfish approved these changes Jul 22, 2021

View reviewed changes

bdrung mentioned this pull request Jul 22, 2021

ethtool collector produces too many metrics (hundreds per device) #2096

Closed

bdrung force-pushed the fix-metric-names branch 3 times, most recently from 863d2fb to bc37167 Compare July 27, 2021 15:36

discordianfish requested a review from SuperQ July 27, 2021 16:13

SuperQ approved these changes Aug 16, 2021

View reviewed changes

bdrung force-pushed the fix-metric-names branch from bc37167 to f13217d Compare August 16, 2021 13:25

discordianfish merged commit 6ac6ea2 into prometheus:master Aug 16, 2021

bdrung deleted the fix-metric-names branch August 16, 2021 13:34

SuperQ mentioned this pull request Oct 11, 2021

Release 1.3.0 #2168

Merged

ventifus mentioned this pull request Oct 28, 2021

ethtool error: "panic: "node_ethtool_ err" is not a valid metric name" #2185

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

ethtool: Sanitize metric names#2093

ethtool: Sanitize metric names#2093
discordianfish merged 1 commit intoprometheus:masterfrom
bdrung:fix-metric-names

bdrung commented Jul 21, 2021

Uh oh!

Uh oh!

Uh oh!

discordianfish left a comment

Uh oh!

SuperQ left a comment

Uh oh!

bdrung commented Aug 16, 2021

Uh oh!

discordianfish commented Aug 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

Conversation

bdrung commented Jul 21, 2021

Uh oh!

Uh oh!

Uh oh!

discordianfish left a comment

Choose a reason for hiding this comment

Uh oh!

SuperQ left a comment

Choose a reason for hiding this comment

Uh oh!

bdrung commented Aug 16, 2021

Uh oh!

discordianfish commented Aug 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants