NetBSD support for CPU collector#2586
NetBSD support for CPU collector#2586MatthiasPetermann wants to merge 0 commit intoprometheus:masterfrom MatthiasPetermann:master
Conversation
SuperQ
changed the title
|
Hi there! I can imagine that my multiple resubmissions of the pull requests may have caused some confusion. However, from my perspective, the request looks good now - all the checks have passed, and only one approval is pending. I don't want to rush you, especially since I may have already tested your patience. But I would greatly appreciate it if you could give me an approximate idea of when we can expect to merge the patch. Thank you very much for your help and support! |
collector/cpu_netbsd.go
Outdated
| pref_len uint64 | ||
| } | ||
|
|
||
| type values struct { |
There was a problem hiding this comment.
values and prop(s) are a bit too generic for package wide variables that are only used here. Can you give it a bit more specific name? Or do you think they are useful beyond the cpu collector?
There was a problem hiding this comment.
I agree, these names are a bit generic to be package level with no context.
Also, it would be good to document these structs.
// values is something something something.
type values struct {
|
LGTM in general but not sure about these package wide names/variables. Maybe we should actually move these netbsd specific things to a subpackage? But also just prefixing them to make their scope more clear would be sufficient. After that we can merge this soon. |
3 participants
Based on cpu_freebsd.go I extended the CPU collector so that it is also useful for NetBSD. At the moment it supports CPU times (analogous to the FreeBSD collector via sysctl calls divided by mode), as well as CPU core temperatures on an experimental basis. The core temperatures depend at the moment on the call of a binary from the NetBSD base system (envstat). I'm not quite happy with this yet - the overhead to start a process and parse the XML proplist is certainly measurable and then again has influence on the overall load. An alternative would be low level calls / ioctls to the envsys subsystem. But here I have to familiarize myself first. If you think it would be ok to merge the extension in its current form, that would be a welcome addition for NetBSD users already. In case of doubt, the collector can be switched off via command line, and the extension has no influence on the binaries for other systems than NetBSD. I would also appreciate any comments or suggestions.
Signed-off-by: Matthias Petermann mp@petermann-it.de