DEVOPS-556 : add pagefault stats to process metrics

[ovesh]

page fault stats are not supported in psutil.
This pull request reads the 4 relevant stats (minflt, cminflt, majflt, cmajflt) from /proc/{pid}/stat.

I fixed the flake8 failures. However there are still errors because this pull request doesn't provide 100% coverage. I'm not sure what the policy is.

[olivielpeau]

Thanks @ovesh for this rebased PR! We'll review and test it shortly.

One way to address the coverage error would be to mock the get_pagefault_stats method to make it return None in all the existing tests except in the test you've added.

[ovesh]

One way to address the coverage error would be to mock the get_pagefault_stats method to make it return None in all the existing tests except in the test you've added.

Thanks for the tip. I'm still getting a coverage error, ironically for the one test that I added. Not sure what to do about that. Should I just not call coverage_report(), or maybe mocking some other methods as noops?

[gmmeyer]

First off, thank you very much for your contribution. This is a nice feature and we'd love to have it in here. I've been reviewing it and I think that I know what we can do to fix this. I'm gonna ask you to do two, relatively simple things.

The first is easy: rebase your branch against the upstream master. It should be trivial. I'm only asking because we took out support for python 2.6, and the tests won't ever pass because of that.

The second is that, rather than mocking something else out, it would probably be easier to merge your test in with the test_check. Put your pagefault stats mock in that function. Put the assertions before the coverage_report. Have them share the same run_check as the top of that test, and then just assertMetric with the same config you're using already. I think it would be config['instances'][1], since you'd want to share the same config as the rest of that part of the test.

This way, I think, it'll pass the coverage report without much trouble. After you merge the two together, you can just remove the test_pagefault_stats test.

This will, I hope, work. Because, coverage report grabs which metrics the class has been using from the check. Then, it sees if you ran a check for them in the current function. If you haven't, then it fails. So, you would need to mock out everything else in order to get it to pass. would definitely not be ideal. It would be a lot easier to just merge the two.

After you make these changes, we'll look at the test results and go from there!

[ovesh]

awesome, thanks for the communication!

[gmmeyer]

This has been superseded by #2477. Thank you for your contributions! :)

[ovesh]

Thank you!