802420 - Add class for recording system resource usage

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Gregory Szorc [:gps]]

Attachment: Add mozsysteminfo package with resource monitory class, v1

I have implemented a class for monitoring entire system resource usage. I think it's pretty useful and would make a good addition to mozbase.

This could arguably be added to mozinfo instead of creating a whole new package. I'll leave that to the mozbase people to decide.

Future additions to this package could include modules for easily looking at running processes, memory usage, etc. Basically, I'm thinking this will be high-level API for psutil.

The specific use case of this class that I have in mind is to record and analyze resource usage as part of performing builds and running test suites. Currently, we don't record any of this info. I'd like to have data such as average CPU usage during builds easily available. This class will facilitate that.

I'm not sure if I tagged the proper reviewer. Also, I don't care that this requires Python 2.7+. I understand some test runners aren't currently running 2.7. Long term they will run 2.7, so that's what I am targeting.

This is pretty low priority. But, as soon as it lands I will update mach to display data porn ;)

Comment 1

[Jeff Hammel]

So, I'm not really sure what to do with this.  My personal position is that it is already extremely confusing to figure out what version of python we are required to support in mozbase and I don't really want to make those waters even muddier.  Our official policy is "2.5+ except 2.4 for talos (sorta, since we only care about 2.4 for windows talos, so...)" :wlach, what's your opinion here?  As much as I want to get on 2.7, I don't really want to start enabling others writing 2.7 code and then me having to backport it, as has often been the situation in the past 6 months or so.

Also, and more minor, we use markdown syntax vs rst syntax for our text files.  FWIW,I prefer rst.  I do want consistency though.

Comment 2

[Gregory Szorc [:gps]]

If mozbase isn't the correct place for this code, I can certainly check it into m-c under python/ if that seems like a better fit. I'm "offering" it to mozbase because I think it will make a good addition.

Comment 3

[Jeff Hammel]

Comment on attachment 672089 [details] [diff] [review]
Add mozsysteminfo package with resource monitory class, v1

I'm going to take myself down as a reviewer.  I really would like to take this patch, and I do think that mozbase is the appropriate place for it, but the README remains in restructured text vs markdown.  Talking with :wlach, he is fine taking packages that have inconsistent versions.  This has been a nightmare for me in the past and there is no particular place that tells what versions of python we require.  The thought of trying to untangle this right now, TBH, I find quite daunting.  This all makes me very sad.

At the risk of saying too much unrelated, I really wish others could step up and try to coordinate this a bit better.  If we want to be on python 2.7 we should throw some effort at that and not try to reap the benefits ahead of time.  If we want to have different mozbase packages require different versions we should note that in some way that doesn't require reading code or failing e.g. on try were we to try to use this in a test harness that is run on an older version.  I've really tried hard to coordinate the metadata about the project to support others.  I guess I have failed.

Comment 4

[Gregory Szorc [:gps]]

Releng has stated that they will move all the machines to Python 2.7. I have heard Chris Cooper say recently that they are working on it.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Is there an ETA on it or a bug we can follow?

Comment 6

[Gregory Szorc [:gps]]

See the bug tree for bug 724191.

Comment 7

[Jeff Hammel]

https://github.com/mozilla/mozbase/pull/36

Comment 8

[Jeff Hammel]

see also bug 803714

Comment 9

[Gregory Szorc [:gps]]

Attachment: Add mozsystemmonitor package, v2

I cleaned up the patch a bit. It now reports individual CPU times in addition to high-level CPU percent. I also made things behave better if psutil is not available.

You can also view at https://github.com/indygreg/mozbase/tree/bug-802420 if that is your thing.

This is my first contribution to mozbase. I tried to follow the directions at https://wiki.mozilla.org/Auto-tools/Projects/MozBase. No clue how well I did.

Comment 10

[Gregory Szorc [:gps]]

Review ping.

Comment 11

[Jeff Hammel]

Comment on attachment 736599 [details] [diff] [review]
Add mozsystemmonitor package, v2

Review of attachment 736599 [details] [diff] [review]:
-----------------------------------------------------------------

I'm sorry I let this lag so long.  My recommendation is to take this as-is with the setup.py change.  I would like a few follow-ups, or you can roll them into the patch:

* the test should probably have a manifest: https://wiki.mozilla.org/Auto-tools/Projects/Mozbase#Tests
* we now do documentation differently; https://wiki.mozilla.org/Auto-tools/Projects/MozBase/WritingDocs ; so the README should go in the top-level docs directory, https://github.com/mozilla/mozbase/tree/master/docs , and setup.py should be changed to not read this file and use a long description a la https://github.com/mozilla/mozbase/blob/master/mozfile/setup.py#L12

Other than that, great work, and apologies again for the huge lag.  If no one opposes, go for landing w/ at least setup.py fixed.

::: mozsystemmonitor/setup.py
@@ +5,5 @@
> +import os
> +
> +from setuptools import setup
> +
> +PACKAGE_VERSION = '0.1'

Please change this to 0.0 as per https://wiki.mozilla.org/Auto-tools/Projects/Mozbase#Adding_a_New_Mozbase_Package

@@ +9,5 @@
> +PACKAGE_VERSION = '0.1'
> +
> +try:
> +    pwd = os.path.dirname(os.path.abspath(__file__))
> +    description = open(os.path.join(here, 'README.rst')).read()

We don't do this anymore. Sorry, my fault that the patch has rotted.

Comment 12

[Gregory Szorc [:gps]]

https://github.com/mozilla/mozbase/commit/0a1214782a81dc0cde21d001bcfd737c8a860ebb

Will file follow-up for review feedback.