[process] Improve sampling of cpu.pct #1928
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Improvements: - Non-blocking call to `cpu_percent` after the first sample - Retrieves an averaged value since the last call (instead of an averaged value over the fraction of a second defined by `cpu_check_interval`) To use the non-blocking call we keep Process instances through multiple check runs. Also updated a test on the check, on which we stop using real PIDs because the test was previously relying on the of `find_pids` to return a list of identical PIDs, whereas `find_pids` actually returns a set of PIDs.
As we discussed, I think that's the right thing to do. |
olivielpeau
force-pushed
the
olivielpeau/better-process-cpu-sample
branch
2 times, most recently
from
29f6d66 to
2b19032
Compare
|
Nice improvement. Looks good to merge to me ! 🚢 |
The check was never setting `last_ad_cache_ts` so the cache was being refreshed at every run of the check. This fixes the issue and updates the ad cache by instance so that every instance refreshes the cache (and not only the first one).
Implement suggestions on the PR: 1. the first sample of `cpu_percent` is not sent at all instead of being retrieved in a blocking call. This allows the first run of the check to be fast even when a lot of processes are monitored 2. as a consequence we drop the `cpu_check_interval` parameter as it has become useless Also, initialize self.process_cache with a defaultdict for simplicity.
olivielpeau
force-pushed
the
olivielpeau/better-process-cpu-sample
branch
from
2b19032 to
1193212
Compare
|
Thanks again for the thorough review ;) I've rebased the PR and changed the Exception type tested in |
|
The CI is failing on unrelated tests, merging. |
olivielpeau
added a commit
that referenced
this pull request
Sep 18, 2015
…sample [process] Improve sampling of cpu.pct
|
👍 |
olivielpeau
added a commit
that referenced
this pull request
Sep 23, 2015
PR #1928 improved the sampling of `cpu.pct` but its value is not accurate for short-lived processes (it was even less accurate before that PR but it's still worth mentioning).
olivielpeau
added a commit
that referenced
this pull request
Sep 23, 2015
PR #1928 improved the sampling of `cpu.pct` but its value is not accurate for short-lived processes (it was even less accurate before that PR but it's still worth mentioning). [skip ci]
urosgruber
pushed a commit
to urosgruber/dd-agent
that referenced
this pull request
Dec 23, 2015
PR DataDog#1928 improved the sampling of `cpu.pct` but its value is not accurate for short-lived processes (it was even less accurate before that PR but it's still worth mentioning). [skip ci]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #1660
Improvements:
cpu_percentafter the first samplecpu_check_interval)The first sample of
cpu.pcton every process still uses a blocking call so that the check can retrieve a value as soon as it finds new processes. Setting thecpu_check_intervalto0removes that first blocking call.Also fixed the handling of the cache of AccessDenied PIDs (it was previously refreshed at every run of every instance).