Fix cgroup path computation #399
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
In testing scenarios, the process path is not necessarily `/proc/<pid>/cgroup`, but `fixtures/<pid>/cgroup` or similar. Signed-off-by: Nathan Monfils <nmo@escaux.com>
azertyfun
added a commit
to azertyfun/process-exporter
that referenced
this pull request
Jul 20, 2021
This simply exposes the procps method to get a process' cgroups. If the cgroup is not available, we simply return an empty array as this can commonly happen on older kernels using the v1 cgroups (found at `/proc/self/cgroups` instead of `/proc/self/cgroup`). This commit still needs to be updated with the version of procfs which implements prometheus/procfs#399, as otherwise the tests will fail due to unsupported fixtures.
discordianfish
approved these changes
Jul 21, 2021
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.
In testing scenarios, the process path is not necessarily
/proc/<pid>/cgroup, butfixtures/<pid>/cgroupor similar.I have encountered this issue while trying to extend process-exporter to support cgroups, which worked fine in practiced but failed the tests because it attempted to read
/proc/<pid>/cgroupinstead of../fixtures/<pid>/cgroup.-> The fix is trivial:
p.path("cgroup")already exists and is the more flexible alternative for cases where the proc directory isn't/proc/. This makes behavior consistent with the rest of the library as far as I can tell.I haven't found anywhere else that needs fixing by grepping on
Sprintf.*PID.Tangentially related but I couldn't figure out how to test my changes to process-exporter with my fork of procps without changing all
imports to point to my fork and updating my fork's master to rename all importsprometheus/procfstoazertyfun/procfs. What am I doing wrong? Surely importing my fork should be easier?