Add initial wifi collector #413
Conversation
|
Fixes #326 . |
|
Yes, testing is in general not easy with the node-exporter.. Only option I see is to mock what your library returns but even then it's hard to test. I'd say it's okay to merge it without tests though. If you based this PR on #412, the tests should parse. But we can also merge that first and rebase this here. Whatever you prefer. |
|
@discordianfish Yep, my plan was to just rebase once #412 was merged. Keeps things simple. |
|
For some of my personal exporters, I've been using this pattern for testing: https://github.com/mdlayher/unifi_exporter/blob/master/unifiexporter_test.go#L27-L48. Seems to work well. You'd just construct the collector with an interface that can use the system implementation or a mocked one, and then parse the output and see if you get the metrics you expect. |
|
@mdlayher That's from the idea similar to the end-to-end-tests.sh. But mocking the whole collector doesn't really make sense, mocking your package's functions would make more sense I guess. |
|
I can do a |
|
@mdlayher Probably better than having no tests.. Wouldn't know a better way without being able to actually emulate wifi devices. Googling for this brought up though https://github.com/martinpitt/umockdev though. Probably doesn't help but maybe worth having a look? |
|
@discordianfish updated with a flag for end-to-end tests that uses JSON fixture files. Once #412 is merged, does this look good? |
|
@discordianfish @SuperQ all fixed up! Take a look! |
| ), | ||
|
|
||
| StationReceiveBitrateBPS: prometheus.NewDesc( | ||
| prometheus.BuildFQName(Namespace, subsystem, "station_receive_bitrate_bps"), |
There was a problem hiding this comment.
Maybe station_receive_bits_per_second?
| ), | ||
|
|
||
| StationTransmitRetriesTotal: prometheus.NewDesc( | ||
| prometheus.BuildFQName(Namespace, subsystem, "station_transmit_retries_total"), |
There was a problem hiding this comment.
I assume there is no count for all transmit/attempts? Would be nice to also have the successful transmits.
There was a problem hiding this comment.
Linux does expose a "transmitted packets" metric via nl80211, but I believe this would be redundant with node_network_transmit_packets.
There was a problem hiding this comment.
Hrm good point but wouldn't that apply to drops/errs as well? Or are they different from failed metrics here? But yeah, either way it probably makes sense to keep these as they are for now.
There was a problem hiding this comment.
Unfortunately nl80211 isn't super clear about it: http://lxr.free-electrons.com/source/include/uapi/linux/nl80211.h#L2620.
* @NL80211_STA_INFO_TX_FAILED: total failed packets (MPDUs)
* (u32, to this station)
For the time being, at least it'll be useful to see if/when a number of transmission failures and beacon losses start occurring, so a weak connection can be easily identified.
|
See @SuperQ's comment and my question, but looking good in general! |
|
I appear to have paused CircleCI because of a few quick amends and pushes. Oops. https://circleci.com/gh/prometheus/node_exporter/457 Hopefully it kicks off on its own soon. |
* fixed arp table mac address parsing - Added ARPEntry.Flags field related to entry flags - Added ARPEntry.IsComplete() to check if entry is complete - Fixed mac address parsing and error handling - Test improved with incomplete entry information Closes prometheus#413. Signed-off-by: Serhii Zasenko <sergii@zasenko.name> * Arp flag const names cleanup Signed-off-by: Serhii Zasenko <sergii@zasenko.name>
Depends on #412 .
No idea how to test this honestly. I've been developing by copying the binary over to my laptop and poking at it there. Can CI tests be done with a virtual wireless device of some sort?