opserver / Opserver

Stack Exchange's Monitoring System
https://opserver.github.io/Opserver/
MIT License
4.51k stars 827 forks source link

Adding volume utilization (performance, not usage) #201

Closed batzen closed 7 years ago

batzen commented 8 years ago

Currently Opserver only supports gathering volume usage data. I'd like to add volume utilization (read, write, length of queue) data to nodes and the dashboard.

I already got this running locally and would like to ask if you would accept a PR adding this feature.

If you don't mind i would add this to my fork and not a separate PR as i already fixed a bunch of WMI related stuff there.

I currently named it VolumePerformance (volumePerformance-chart, VolumePerformanceUtilization, GetPerformanceUtilization etc.) as the wording VolumeUtilization is already used to get volume usage. I'd prefer to rename VolumeUtilization to VolumeUsage.

What do you think about this? If you don't like it being added to the dashboard we could add an option to disable showing volume performance utilization on the dashboard.

NickCraver commented 8 years ago

Are you talking about the main dashboard, or on the per-node view? On per-node I could potentially see something. We'd need to implement similar for the other providers, though. In Bosun we're covered on the base collectors...on Orion I'd have to look. Do you have a screenshot? That would probably help context more than anything.

I don't see this ever happening on the primary dashboard server list (just so few care about this there...you're the first). As said in the other issue though, I prefer not to have one giant take-it-or-leave-it PR. PRs should be about features, not batches of features in most cases. The harder they are to parse test and discern, the slower they're absorbed (if at all) - so much back and forth slowing it down on revisions.

I appreciate all this WMI work, I'm excited to see some of the improvements - it needs the love.

batzen commented 8 years ago

Are you talking about the main dashboard, or on the per-node view?

About both. But optionally on the dashboard.

I don't see this ever happening on the primary dashboard server list

That's exactly why i would like to add a dashboard configuration option and hide it from there by default.

You can view my changes @ https://github.com/batzen/Opserver It's quite a lot but, in my opinion, still not too much for one PR. As most changes depend on each other, i would prefer one PR for this time and promise to keep future PRs smaller.

dashboard

dashboard_graph

node

NickCraver commented 7 years ago

Taking a look at the PR this week, trying to catch up here.

batzen commented 7 years ago

Just created the PR #239 so it's easier for you to have a look. ;-)

The only changes that are not part of my branch are the changes to style.css and style.min.css because those are generated and cause conflicts every time they are changed...

NickCraver commented 7 years ago

Now merged in - closing this one out to cleanup. Thank you!