Update wifi stats to support multiple stations (#977)#980
Update wifi stats to support multiple stations (#977)#980SuperQ merged 1 commit intoprometheus:masterfrom
Conversation
collector/wifi_linux.go
Outdated
There was a problem hiding this comment.
I think hw is nice and short, but not very descriptive. We generally recommend more descriptive names. How about station_address.
There was a problem hiding this comment.
From nl80211.h, it is called
* @NL80211_ATTR_MAC: MAC address (various uses)
On linux iw dev wlan1 station dump command output, it is not explicity named.
I think station_address might be a bit repetitive given the statistics are named node_station_ but any of the following would be fine:
mac
hardware_address
hw_address
or even just address
Would one of these be acceptable?
There was a problem hiding this comment.
True, how about mac_address, to match the datasource.
mdlayher
left a comment
mdlayher
left a comment
There was a problem hiding this comment.
LGTM after changing the label name to mac_address.
Signed-off-by: neiledgar <neil.edgar@btinternet.com>
neiledgar
force-pushed
the
multiple_stations_support
branch
from
b57d682 to
8da695f
Compare
|
Changed hw label to mac_address. I squashed all the changes into one commit. Apologies if I lost your comments. Is the correct workflow to add additional commits rather than squash all review updates into one commit? |
|
@neiledgar Squashing commits is fine. |
|
LGTM |
…etheus#980) Signed-off-by: neiledgar <neil.edgar@btinternet.com>
4 participants
Signed-off-by: neiledgar neil.edgar@btinternet.com