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

[psutil] Upgrade to 4.4.1 #2957

Merged
merged 4 commits into from
Oct 26, 2016
Merged

Conversation

olivielpeau
Copy link
Member

What does this PR do?

Upgrades psutil to the latest version (4.4.1), and related changes on dd-agent (see details in commit messages):

  • [disk] Use in_use percentage provided by psutil
  • [win32][system] Use psutil to sample cpu.interrupt: tested on my local windows VM: psutil and WMI send the same values for cpu.interrupt

Motivation

The upgrade was requested by a customer so that we ship a version that includes this fix: giampaolo/psutil#761
More generally it's good to keep an up-to-date version.

Testing Guidelines

Went through the changelog of psutil and through the occurences of psutil in dd-agent to find things that could be affected. Not sure I've spotted everything but I'll closely monitor the deploy of the updated Agent on our staging infrastructure (especially for the gunicorn check that could potentially be affected).

Additional Notes

Related PR on omnibus-software: DataDog/omnibus-software#82
psutil changelog: https://github.com/giampaolo/psutil/blob/master/HISTORY.rst

Previously, psutil didn't take into account root reserved space to
compute the `disk.in_use` percentage. PR#1784 implemented a workaround
on the disk check so that it would send values that are consistent with
`df`'s output.

psutil v4.3.0 fixed this issue. Now that we're using psutil 4.4.1,
let's revert our workaround.

Reference:
https://github.com/giampaolo/psutil/blob/master/HISTORY.rst#430
Allows to get rid of WMI in the cpu check. `interrupt` has been added
to psutil 4.1.0
@olivielpeau olivielpeau added this to the 5.10.0 milestone Oct 25, 2016
@olivielpeau olivielpeau force-pushed the olivielpeau/upgrade-psutil-4-4-1 branch from 1dceba0 to 5d56ed3 Compare October 25, 2016 23:12
- remove mock of `open` (useless since psutil upgrade)
- add `Threads` to mocked `status` file (otherwise psutil crashes), and
test related metric
@olivielpeau olivielpeau force-pushed the olivielpeau/upgrade-psutil-4-4-1 branch from 5d56ed3 to 8bc67cd Compare October 25, 2016 23:16
Copy link

@remh remh left a comment

Choose a reason for hiding this comment

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

Looks good beside the open question

self.counter('system.cpu.system')
self.counter('system.cpu.interrupt')
Copy link

Choose a reason for hiding this comment

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

why ?
is that a breaking change ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Basically an old-style-check counters is the equivalent of a rates of a new-style check. It's still stored as a gauge in the backend, so there's no backwards-compatibility issue.

We compute a rate here because psutil.cpu_times returns for interrupt the same kind of value as for the other cpu values, i.e. the total time the cpu(s) spent in the interrupt state since startup, so to get a percentage from this we need to compute the rate divided by the number of cpus and multiply that by 100.

@olivielpeau olivielpeau merged commit 89832f2 into master Oct 26, 2016
@olivielpeau olivielpeau deleted the olivielpeau/upgrade-psutil-4-4-1 branch October 26, 2016 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows: psutil.boot_time() wraps to 0 after 49 days
2 participants