Skip to content
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

feat(inputs.diskio): Add field io await and util #15950

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jiangxianfu
Copy link

Summary

In practical scenarios, it may be necessary to check the values for disk I/O wait, I/O util, and I/O svctm.
Therefore, I would like to add these three metrics.

@telegraf-tiger
Copy link
Contributor

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

@jiangxianfu
Copy link
Author

jiangxianfu commented Sep 30, 2024

add util/await/svctm #15951

@jiangxianfu
Copy link
Author

done !signed-cla

jiangxianfu added a commit to jiangxianfu/telegraf that referenced this pull request Sep 30, 2024
jiangxianfu added a commit to jiangxianfu/telegraf that referenced this pull request Sep 30, 2024
jiangxianfu added a commit to jiangxianfu/telegraf that referenced this pull request Sep 30, 2024
@jiangxianfu jiangxianfu changed the title add disk io util await svctm Add new field util/await/svctm to diskio Sep 30, 2024
@jiangxianfu jiangxianfu changed the title Add new field util/await/svctm to diskio Add new field for diskio Sep 30, 2024
jiangxianfu added a commit to jiangxianfu/telegraf that referenced this pull request Sep 30, 2024
@jiangxianfu jiangxianfu changed the title Add new field for diskio feat(inputs.diskio) Add field for io await and util Sep 30, 2024
@jiangxianfu jiangxianfu changed the title feat(inputs.diskio) Add field for io await and util feat(inputs.diskio) Add field io await and util Sep 30, 2024
@jiangxianfu jiangxianfu changed the title feat(inputs.diskio) Add field io await and util feat(inputs.diskio): Add field io await and util Sep 30, 2024
@telegraf-tiger telegraf-tiger bot added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Sep 30, 2024
jiangxianfu added a commit to jiangxianfu/telegraf that referenced this pull request Sep 30, 2024
jiangxianfu added a commit to jiangxianfu/telegraf that referenced this pull request Sep 30, 2024
jiangxianfu added a commit to jiangxianfu/telegraf that referenced this pull request Sep 30, 2024
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thanks @jiangxianfu for your contribution! I do have some comments in the code.

plugins/inputs/diskio/README.md Outdated Show resolved Hide resolved
plugins/inputs/diskio/diskio.go Outdated Show resolved Hide resolved
plugins/inputs/diskio/diskio.go Outdated Show resolved Hide resolved
Comment on lines 137 to 148
fieldsG := make(map[string]interface{})
if deltaRWCount > 0 {
fieldsG["io_await"] = deltaRWTime / deltaRWCount
fieldsG["io_svctm"] = deltaIOTime / deltaRWCount
}
itv := float64(collectTime.Sub(d.lastCollectTime).Milliseconds())
if itv > 0 {
fieldsG["io_util"] = 100 * deltaIOTime / itv
}
if len(fieldsG) > 0 {
acc.AddGauge("diskio", fieldsG, tags)
}
Copy link
Member

Choose a reason for hiding this comment

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

Why do you create a new metric instead of simply adding the fields to the existing metric?

Copy link
Author

@jiangxianfu jiangxianfu Oct 1, 2024

Choose a reason for hiding this comment

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

This is because the other metrics are of type "Counter", while the newly added values are of type "Gauge"

Copy link
Member

Choose a reason for hiding this comment

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

Please do not resolve conversations that are still discussed! I'm not fully convinced by that argument as iops_in_progress is also tagged as gauge. If the type is really an issue, you should at least use a different metric name.

plugins/inputs/diskio/diskio_test.go Outdated Show resolved Hide resolved
plugins/inputs/diskio/diskio_test.go Outdated Show resolved Hide resolved
plugins/inputs/diskio/diskio_test.go Outdated Show resolved Hide resolved
plugins/inputs/diskio/diskio_test.go Outdated Show resolved Hide resolved
@srebhan srebhan self-assigned this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants