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.

Sign up for GitHub

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

Jump to bottom

feat: /proc/mdstat collection #9101

Merged
merged 57 commits into from
Aug 31, 2021
Merged

feat: /proc/mdstat collection #9101

merged 57 commits into from
Aug 31, 2021

Conversation

johnseekins
Copy link
Contributor

Required for all PRs:

  • Updated associated README.md.
  • Wrote appropriate unit tests.

This adds a new input that collects stats on Linux MD arrays on a system. It is based off https://github.com/prometheus/procfs/blob/master/mdstat.go. (which is apache licensed vs. telegraf's MIT.

telegraf-tiger[bot]
telegraf-tiger bot approved these changes Apr 7, 2021
View reviewed changes
Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

🤝 ✅ CLA has been signed. Thank you!

@telegraf-tiger telegraf-tiger bot added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin new plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Apr 7, 2021
@johnseekins
Copy link
Contributor Author

johnseekins commented Apr 7, 2021 •
edited
Loading

Not really sure how to fix this lint complaint:

Warning: function-result-limit: maximum number of return results per function exceeded; max 3 but got 4 (revive)
Warning: function-result-limit: maximum number of return results per function exceeded; max 3 but got 5 (revive)

There are other input plugins that also violate this limit (e.g. kernel_vmstat has a function with 4 return statements) so I'm struggling to see how this check is valid.

@johnseekins
Copy link
Contributor Author

Sorry about all the commits, but make test doesn't seem to work correctly for me locally, so I'm leaning on circle to see my tests pass.

telegraf-tiger[bot]
telegraf-tiger bot reviewed Apr 7, 2021
View reviewed changes
Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

Looks like new artifacts were built from this PR. Get them here!

Artifact URLs

@telegraf-tiger
Copy link
Contributor

Looks like new artifacts were built from this PR. Get them here!

Artifact URLs

@telegraf-tiger
Copy link
Contributor

Looks like new artifacts were built from this PR. Get them here!

Artifact URLs

@telegraf-tiger
Copy link
Contributor

Looks like new artifacts were built from this PR. Get them here!

Artifact URLs

ssoroka
ssoroka suggested changes May 25, 2021
View reviewed changes
Copy link
Contributor

@ssoroka ssoroka left a comment

Choose a reason for hiding this comment

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

Looks great. I have some comments below

plugins/inputs/mdstat/README.md Outdated Show resolved Hide resolved
plugins/inputs/mdstat/README.md Outdated Show resolved Hide resolved
plugins/inputs/mdstat/mdstat.go Outdated Show resolved Hide resolved
plugins/inputs/mdstat/mdstat.go Show resolved Hide resolved
plugins/inputs/mdstat/mdstat_test.go Outdated Show resolved Hide resolved
@popey
Copy link
Contributor

popey commented May 25, 2021 •
edited
Loading

Thanks for the new plugin! As a user of Linux mdraid arrays, I'm keen to monitor them!

I ran it on a system here with four disks, one which is failed, and I don't see it detecting the failed disk? Perhaps the disk just dropped but hasn't failed?

Here's the /proc/mdstat output

Personalities : [linear] [multipath] [raid0] [raid1] [raid6] [raid5] [raid4] [raid10] 
md0 : active raid5 sdd1[3] sdb1[1] sda1[0]
      5860144128 blocks super 1.2 level 5, 64k chunk, algorithm 2 [4/3] [UUU_]
      bitmap: 8/15 pages [32KB], 65536KB chunk

unused devices: <none>

Here's the output:

2021-05-25T21:45:53Z I! Starting Telegraf 
2021-05-25T21:45:53Z I! Loaded inputs: mdstat
2021-05-25T21:45:53Z I! Loaded aggregators: 
2021-05-25T21:45:53Z I! Loaded processors: 
2021-05-25T21:45:53Z I! Loaded outputs: file
2021-05-25T21:45:53Z I! Tags enabled: host=shirka
2021-05-25T21:45:53Z I! [agent] Config: Interval:10s, Quiet:false, Hostname:"shirka", Flush Interval:10s
mdstat,ActivityState=active,Devices=sda1\,sdb1\,sdd1,Name=md0,host=shirka DisksActive=3i,DisksTotal=4i,BlocksSynced=5860144128i,BlocksSyncedSpeed=0,BlocksSyncedFinishTime=0,DisksFailed=0i,DisksSpare=0i,BlocksTotal=5860144128i,BlocksSyncedPct=0 1621979160000000000

@johnseekins
Copy link
Contributor Author

johnseekins commented May 25, 2021 •
edited
Loading

Thanks for the new plugin! As a user of Linux mdraid arrays, I'm keen to monitor them!

I ran it on a system here with four disks, one which is failed, and I don't see it detecting the failed disk? Perhaps the disk just dropped but hasn't failed?

Here's the /proc/mdstat output

Personalities : [linear] [multipath] [raid0] [raid1] [raid6] [raid5] [raid4] [raid10] 
md0 : active raid5 sdd1[3] sdb1[1] sda1[0]
      5860144128 blocks super 1.2 level 5, 64k chunk, algorithm 2 [4/3] [UUU_]
      bitmap: 8/15 pages [32KB], 65536KB chunk

unused devices: <none>

Here's the output:

2021-05-25T21:45:53Z I! Starting Telegraf 
2021-05-25T21:45:53Z I! Loaded inputs: mdstat
2021-05-25T21:45:53Z I! Loaded aggregators: 
2021-05-25T21:45:53Z I! Loaded processors: 
2021-05-25T21:45:53Z I! Loaded outputs: file
2021-05-25T21:45:53Z I! Tags enabled: host=shirka
2021-05-25T21:45:53Z I! [agent] Config: Interval:10s, Quiet:false, Hostname:"shirka", Flush Interval:10s
mdstat,ActivityState=active,Devices=sda1\,sdb1\,sdd1,Name=md0,host=shirka DisksActive=3i,DisksTotal=4i,BlocksSynced=5860144128i,BlocksSyncedSpeed=0,BlocksSyncedFinishTime=0,DisksFailed=0i,DisksSpare=0i,BlocksTotal=5860144128i,BlocksSyncedPct=0 1621979160000000000

This should be resolved with the most recent commits. A disk marked as _ is actually (according to mdstat's docs) different than an actually failed disk, so we'll be counting those separately.

@johnseekins johnseekins requested a review from ssoroka May 25, 2021 23:18
@telegraf-tiger
Copy link
Contributor

Looks like new artifacts were built from this PR. Get them here!

Artifact URLs

@johnseekins
Copy link
Contributor Author

The same collector was approved and merged into Prometheus' node_exporter: prometheus/procfs#380

@johnseekins
Copy link
Contributor Author

@ssoroka I believe I've addressed all your concerns. Any chance you could take another look at this?

@sspaink sspaink changed the title /proc/mdstat collection feat: /proc/mdstat collection Aug 31, 2021
@sspaink sspaink merged commit 435c2a6 into influxdata:master Aug 31, 2021
@reimda reimda mentioned this pull request Aug 31, 2021
Closed
2 tasks
@sspaink sspaink mentioned this pull request Aug 31, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@telegraf-tiger telegraf-tiger[bot] telegraf-tiger[bot] approved these changes

@ssoroka ssoroka ssoroka approved these changes

Assignees
No one assigned
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin new plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@johnseekins @popey @ssoroka @sspaink