darwin powersupply collector by nolith · Pull Request #1777 · prometheus/node_exporter

nolith · 2020-07-03T15:25:45Z

hello 👋

This PR introduces darwin support for the powersupply collector.

I tried to keep the same naming as the Linux collector where possible.

Closes #1766

discordianfish · 2020-07-06T08:09:59Z

I'm on the fence on this one. It's hellalot C code. But I see it being useful. @SuperQ @pgier wdyt?

SuperQ · 2020-07-06T08:51:20Z

I had a chat with @nolith about this. The amount of C code is necessary due to the void* passing.

I think this article explains it better.

discordianfish · 2020-07-07T08:09:20Z

Would it maybe make sense to move this to an external module? I'm not sure we're well equipped to maintain significant portions of C code for a platform not everybody of us uses. But I like and want to feature.. So I'm fine either way.

discordianfish · 2020-07-07T08:10:33Z

You know like a IOKit golang wrapper.

nolith · 2020-07-07T11:11:20Z

Would it maybe make sense to move this to an external module? I'm not sure we're well equipped to maintain significant portions of C code for a platform not everybody of us uses. But I like and want to feature.. So I'm fine either way.

There are a lot of C code lines, but the real C code is not that much in my opinion.

I kept most of the logic in func getPowerSourceList() ([]*C.struct_macos_powersupply, error) and I only wrote the struct and the minimal amount of code dealing with the void** parameters in C.

I can move void releasePowerSupply(struct macos_powersupply *ps) to Go as well to reduce the amount of C code.

Could someone give me an hint on the CI failure? 🙏
I can clearly build the node_exporter with go build but it looks like promu has some trouble finding the IOKit and CoreFoundation framework.

discordianfish · 2020-07-08T11:39:17Z

promu does cross compilation on linux.. no clue how/if this works with IOKit and CoreFoundation.

discordianfish · 2021-08-09T14:43:10Z

@nolith I'd love to see this getting in but I have no experience with these macos libs.. Can you build it without specifying any c flags? promu should work the same

discordianfish · 2021-08-09T14:43:41Z

Or maybe it's just a version mismatch?

SuperQ · 2021-08-09T16:18:26Z

This might just need a rebase now that we have updated our cross-compiler docker image.

nolith · 2021-08-10T05:23:00Z

I'll take a look once I'll be back from PTO.

SuperQ · 2021-10-27T11:57:25Z

Ping @nolith :)

nolith · 2021-10-27T14:39:03Z

@SuperQ I'm sorry for the delay.

I've rebased it, but master is broken, the failures are unrelated, but I fixed it in #2180

collector/powersupplyclass_darwin.go

Signed-off-by: Alessio Caiazza <nolith@abisso.org>

Time reported by MacOS PowerSourceDescriptor is expressed in minutes and not in seconds. Signed-off-by: Alessio Caiazza <nolith@abisso.org>

SuperQ

LGTM

nolith · 2021-10-28T08:16:04Z

Is there anything I can do to move this forward @SuperQ ?

Can we make this part of 1.3.0 #2168 or should we merge this after the release?

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>

* Extract powersupply linux code from collector common file. * Add Darwin powersupply collector. Signed-off-by: Alessio Caiazza <nolith@abisso.org>

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>

* Extract powersupply linux code from collector common file. * Add Darwin powersupply collector. Signed-off-by: Alessio Caiazza <nolith@abisso.org>

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>

nolith force-pushed the osx-battery branch from d330401 to fe11cde Compare October 27, 2021 14:20

nolith mentioned this pull request Oct 27, 2021

[BROKEN MASTER] Fix imports when building on MacOS #2180

Merged

nolith force-pushed the osx-battery branch 2 times, most recently from a58735d to fee6079 Compare October 27, 2021 14:37

SuperQ reviewed Oct 27, 2021

View reviewed changes

collector/powersupplyclass_darwin.go Outdated Show resolved Hide resolved

nolith added 3 commits October 27, 2021 17:03

Extract powersupply linux code

981da4e

Signed-off-by: Alessio Caiazza <nolith@abisso.org>

Darwin powersupply collector

ad48f21

Signed-off-by: Alessio Caiazza <nolith@abisso.org>

Scale minutes to seconds

5e26089

Time reported by MacOS PowerSourceDescriptor is expressed in minutes and not in seconds. Signed-off-by: Alessio Caiazza <nolith@abisso.org>

nolith force-pushed the osx-battery branch from fee6079 to 5e26089 Compare October 27, 2021 15:03

SuperQ requested a review from discordianfish October 27, 2021 15:06

SuperQ approved these changes Oct 27, 2021

View reviewed changes

discordianfish approved these changes Oct 27, 2021

View reviewed changes

nolith marked this pull request as ready for review October 27, 2021 15:40

SuperQ merged commit 6523fdf into prometheus:master Oct 28, 2021

nolith deleted the osx-battery branch October 28, 2021 08:27

SuperQ mentioned this pull request Oct 29, 2021

Release 1.3.0 #2168

Merged

Comments

Conversation

nolith commented Jul 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

discordianfish commented Jul 6, 2020

Uh oh!

SuperQ commented Jul 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

discordianfish commented Jul 7, 2020

Uh oh!

discordianfish commented Jul 7, 2020

Uh oh!

nolith commented Jul 7, 2020

Uh oh!

discordianfish commented Jul 8, 2020

Uh oh!

discordianfish commented Aug 9, 2021

Uh oh!

discordianfish commented Aug 9, 2021

Uh oh!

SuperQ commented Aug 9, 2021

Uh oh!

nolith commented Aug 10, 2021

Uh oh!

SuperQ commented Oct 27, 2021

Uh oh!

nolith commented Oct 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

SuperQ left a comment

Choose a reason for hiding this comment

Uh oh!

nolith commented Oct 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nolith commented Jul 3, 2020 •

edited

Loading

SuperQ commented Jul 6, 2020 •

edited

Loading

nolith commented Oct 27, 2021 •

edited

Loading