Add support for stats from /proc/net/snmp as well.#85
Add support for stats from /proc/net/snmp as well.#85beorn7 merged 1 commit intoprometheus:masterfrom
Conversation
collector/netstat.go
Outdated
There was a problem hiding this comment.
Maybe distinguish the error messages?
There was a problem hiding this comment.
Yes, thought of that, but then the problem comes up at line 68 as well. Maybe I'll just change line 68 then to 'couldn't distinguish stats'. Good call though, I'll change it.
There was a problem hiding this comment.
Forget what I said about line 68. I think the invalid value message applies to the merged stats just fine. I think it's a minor nitpick, but it could simply be "stats", but then we should start renaming the variables to just 'stats' and copying both maps to it.
|
@jtakkala Oh sorry, I never looked at this again because there was no ping on the PR after the latest updates. Taking a look now. |
collector/netstat.go
Outdated
There was a problem hiding this comment.
This'd be snmpStats in common Go style (local variables are never capitalized at the beginning).
|
👍 besides last nits. Then squash in the end please :) |
jtakkala
force-pushed
the
snmpStats
branch
5 times, most recently
from
2be4b46 to
45f46fa
Compare
|
@juliusv fixed all nits. Good to merge now? |
|
All of Julius's points are addressed, as it seems. Since he is on vacation, I'll rubberstamp this... |
Add support for stats from /proc/net/snmp as well.
make clear that the netstat collector also reads /proc/net/snmp (prometheus#85) and not just /proc/net/netstat to avoid confusions like prometheus#2179
net_dev.go: fix typo
4 participants
This is the naive implementation for #59. I thought of a few other approaches, mainly looping over a list of files and doing the merge of maps in getStats(). Any other approach would be longer though and I'm not sure of its benefit given the limited and well defined set of inputs (/proc/net/netstat, /proc/net/snmp). I'm happy to hear other opinions if you think this can be refactored a bit more. @matthiasr