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
Initial XFS collector #568
Conversation
Do you have a link to the documentation on the xfs stats file format? |
vendor/vendor.json
Outdated
"checksumSHA1": "kOWRcAHWFkId0aCIOSOyjzC0Zfc=", | ||
"checksumSHA1": "eiBAd4edewJTOtTwxh/ubJdjd+I=", | ||
"path": "github.com/prometheus/procfs/sysfs", | ||
"revision": "9a96e5d8ebd419449a76f5827bf58d605bd7f7d7", |
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.
Let's use the same revision for all packages coming from prometheus/procfs.
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 must have been an oversight on my part, will fix.
collector/xfs_linux.go
Outdated
metrics := []struct { | ||
name string | ||
desc string | ||
field float64 |
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.
We call this the value
.
collector/xfs_linux.go
Outdated
field float64 | ||
}{ | ||
{ | ||
name: "extent_allocation_extents_allocated_total", |
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.
Our usual naming pattern for counters is namespace_thing_action_total
where action is a noun, like extent_allocations_total
, btree_comparisons_total
, etc. Your current one is consistent as well, I'll leave it to @SuperQ.
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 is why I was wondering about the format documentation. 😄
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.
Updated to explain the naming scheme with references to the wiki.
Any more thoughts? |
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. The reasoning to follow the XFS wiki metric naming sounds fair.
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
There are tons more metrics available, but here's a first pass. I tried something a little different to avoid making a big repetitive mess like I did in mountstats.
Fixes #456 .
/r @discordianfish @SuperQ @grobie