-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add NFS Server metrics collector. #803
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
collector/nfsd_linux.go
Outdated
) | ||
|
||
var ( | ||
nfsdReplyCacheHitsDesc = prometheus.NewDesc( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, but I'd make these fields on the nfsdCollector
type so you don't pollute the package level with variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll move things around.
collector/nfsd_linux.go
Outdated
return nil | ||
} | ||
|
||
// updateNFSdReplyCacheStats collects statistics for a single XFS filesystem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
XFS? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy pasta!
63db1c1
to
460f5ff
Compare
@mdlayher Ok, I moved the desc stuff into the update function, does that seem like the right way to you? |
LGTM, beside the help strings. |
LGTM |
collector/nfsd_linux.go
Outdated
ch <- prometheus.MustNewConstMetric( | ||
prometheus.NewDesc( | ||
prometheus.BuildFQName(namespace, nfsdSubsystem, "reply_cache_hits_total"), | ||
"NFSd Reply Cache client did not receive a reply and decided to re-transmit its request and the reply was cached. (bad).", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with having an explaination here, but it should still follow the form of metric help strings, e.g "Total number of something something".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a copy-n-paste from the blog post. I'll clean it up a bit.
@discordianfish Ok, I've updated all the help text, PTAL. |
* Add NFS Server metrics collector. * Add File Handles metrics. * Add nfsd IO stats. * Add metrics for NFSd threads. * Add metrics for NFSd read ahead cache. * Add NFSd network traffic counters. * Add RPC metrics. * Add V2 requests metrics. * Add NFSv3 metrics. * Add NFSv4 metrics. * Update reply cache comment. * Update help text.
Adds support for various Linux NFS Kernel Server metrics read from
/proc/net/rpc/nfsd
.Closes: #607