-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Cleanup iostats field naming #4900
Conversation
|
||
type: float | ||
|
||
The number of write requests that were issued to the device per second | ||
|
||
|
||
[float] | ||
=== `system.diskio.iostat.read_byte_per_sec` | ||
=== `system.diskio.iostat.read.per_sec.bytes` |
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.
what about read.bytes.per_sec
to be as the previous one?
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 put bytes
at the end as this is the unit.
"busy": extraMetrics.BusyPct, | ||
"read": common.MapStr{ | ||
"request": common.MapStr{ | ||
"merge_per_sec": extraMetrics.ReadRequestMergeCountPerSec, |
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.
Shouldn't be merged_per_sec
as in the description of the fields.yml
?
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.
Changed it in both places to merges_per_sec
Looks good to me. I left a few minor comments. |
Align naming with other metricsets.
5064002
to
89517fe
Compare
Align naming with other metricsets.