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

[Disk] Collect disk latency metrics for non-Windows OS #1018

Merged
merged 6 commits into from
Jan 18, 2018

Conversation

dixonscottr
Copy link
Contributor

@dixonscottr dixonscottr commented Jan 16, 2018

What does this PR do?

Allows the check to pull disk latency metrics (system.disk.read_time_pct, system.disk.write_time_pct) from non-windows OS. Also updates collect_latency_metrics() function to not fail in case the fields aren't available to pull these metrics, which is expected for some operating systems.

Motivation

Customer reached out wanting to be able to collect these disk latency metrics and previously the check only collected for Windows hosts

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

Versioning

  • Bumped the check version in manifest.json
  • Bumped the check version in datadog_checks/{integration}/__init__.py
  • Updated CHANGELOG.md. Please use Unreleased as the date in the title
    for the new section.

Additional Notes

Anything else we should know when reviewing?

hush-hush
hush-hush previously approved these changes Jan 18, 2018
Copy link
Member

@hush-hush hush-hush left a comment

Choose a reason for hiding this comment

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

Tiny comment, feel free to merge after addressing it.

except AttributeError:
# Some OS don't return read_time/write_time fields
# http://psutil.readthedocs.io/en/latest/#psutil.disk_io_counters
self.log.debug("Latency metrics not collected for {0}".format(disk_name))
Copy link
Member

Choose a reason for hiding this comment

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

We should log why it's not being collected. Like: `Latency metrics not available for {0}" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @hush-hush. I added the error when logging to be more clear why it's not collected. let me know what you think

@hush-hush hush-hush self-assigned this Jan 18, 2018
@hush-hush hush-hush added this to the 5.22 milestone Jan 18, 2018
@hush-hush hush-hush merged commit 45ee7be into master Jan 18, 2018
@hush-hush hush-hush deleted the scottdixon/disk_latency branch January 18, 2018 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants