Skip to content

Comments

Add a collector for NFS client statistics.#360

Merged
discordianfish merged 1 commit intoprometheus:masterfrom
EdSchouten:nfs
Dec 11, 2016
Merged

Add a collector for NFS client statistics.#360
discordianfish merged 1 commit intoprometheus:masterfrom
EdSchouten:nfs

Conversation

@EdSchouten
Copy link
Contributor

This change adds a new collector called "nfs" that parses the contents
of /proc/net/rpc/nfs and turns it into metrics. It can be used to
inspect the number of operations per type, but also to keep an eye on an
extraneous number of retransmissions, which may indicate connectivity
issues.

I've picked the name "nfs", as most operating systems use "nfs" for the
client component and "nfsd" as the server component. If we want to add
stats for the NFS server as well, we'd better call such a collector
"nfsd".

@mdlayher mdlayher mentioned this pull request Dec 7, 2016
Copy link
Member

@discordianfish discordianfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This change adds a new collector called "nfs" that parses the contents
of /proc/net/rpc/nfs and turns it into metrics. It can be used to
inspect the number of operations per type, but also to keep an eye on an
extraneous number of retransmissions, which may indicate connectivity
issues.

I've picked the name "nfs", as most operating systems use "nfs" for the
client component and "nfsd" as the server component. If we want to add
stats for the NFS server as well, we'd better call such a collector
"nfsd".
@EdSchouten
Copy link
Contributor Author

EdSchouten commented Dec 9, 2016

Nice! Thanks for the approval.

I did notice there was still a tiny bug left in the code that we already fixed in our private branch of the node exporter, namely a missing return nil in case the proc file doesn't exist. I've just updated the commit to include this fix.

@discordianfish discordianfish merged commit ff5b901 into prometheus:master Dec 11, 2016
@SuperQ SuperQ mentioned this pull request Jan 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants