Replace some FreeBSD collectors with pure Go versions #385
Conversation
The code may also work for other BSDs, but I don't have access to those for testing.
The code may also work for other BSDs, but I don't have access to those for testing.
|
Very nice. |
|
Oh that's awesome! I've tested this on darwin by using those collectors for all BSDs (see https://github.com/discordianfish/node_exporter/tree/used-dominikh-all-bsd is anyone wants to try it). @matthiasr You happen to have some BSDs around and can quickly test my branch? |
This removes some error handling, which should be fine. If the calls fail, we will get the zeroes, which is a safe enough fallback. Additionally, if the first sysctl (page_size) succeeded it is unlikely that other ones will fail.
| if err != nil { | ||
| return nil, err | ||
| } | ||
| load := *(*loadavg)(unsafe.Pointer((&b[0]))) |
There was a problem hiding this comment.
Minor, unsafe.Pointer((&b[0])) can be simplified to unsafe.Pointer(&b[0]).
| } | ||
|
|
||
| var ro float64 | ||
| if (fs.Flags & MNT_RDONLY) != 0 { |
There was a problem hiding this comment.
This is equivalent to if fs.Flags&MNT_RDONLY != 0 {, which is simpler and equally readable I think, since gofmt removes the spaces around & to help visualize the order of operations.
There was a problem hiding this comment.
The suggested alternative looks like a blob of ink to me, precisely because of how gofmt formats it. Parentheses also help visualize the order of operations, but render much more readable in the case of bitmasks.
There was a problem hiding this comment.
Yes, keeping it like it is is readable IMO
|
I've pushed another commit, rewriting the BSD meminfo collector to be cgo-free. |
|
Now also for the CPU collector. |
|
LGTM! If you want to work on other collectors, better create a new PR so we can get this reviewed quicker. |
|
@discordianfish I'm pretty much done; the initial two collectors and the two I pushed yesterday. There remain two collectors that use cgo but are not trivial to rewrite. |
Reuse the Go-only implementation already in place for FreeBSD (prometheus#385) on Darwin, DragonflyBSD, NetBSD and OpenBSD. Tested on all affected platforms. Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Reuse the Go-only implementation already in place for FreeBSD (#385) on Darwin, DragonflyBSD, NetBSD and OpenBSD. Tested on all affected platforms. Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Reuse the Go-only implementation already in place for FreeBSD (prometheus#385) on Darwin, DragonflyBSD, NetBSD and OpenBSD. Tested on all affected platforms. Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Reuse the Go-only implementation already in place for FreeBSD (prometheus#385) on Darwin, DragonflyBSD, NetBSD and OpenBSD. Tested on all affected platforms. Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Synchronize common files from prometheus/prometheus
This PR replaces the loadavg and filesystem collectors for FreeBSD with pure Go versions, using syscalls and sysctl directly.
It is possible that these may also work for other BSDs without changes, but I don't have any machines to test that on.
I use
unsafein two places, once for endianness reasons (loadavg) and once to avoid some memory allocations (file systems). I'm happy to remove either of those.