Initiate the script that takes a snapshot of CPU usage

Go to issue 2066413002 for latest updates


BUG=

[Ziqi Xiong, 2016-06-01 23:50:43]

zxiong@google.com changed reviewers:
+ robertocn@google.com

[Ziqi Xiong, 2016-06-01 23:50:43]

First attempt.

[RobertoCN, 2016-06-03 18:01:41]

robertocn@chromium.org changed reviewers:
+ robertocn@chromium.org

[RobertoCN, 2016-06-03 18:01:43]

This approach looks good.

I made some styling suggestions to the code.

https://codereview.chromium.org/2031883002/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/auto_bisect/bisector.py (right):

https://codereview.chromium.org/2031883002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/auto_bisect/bisector.py:677: logs['Debug Info with
CPU Snapshot'] = lines
nit: alignment. I would break just after `logs[` to use implied line
continuation rather than the backslash.

https://codereview.chromium.org/2031883002/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/auto_bisect/resources/cpu_usage.py (right):

https://codereview.chromium.org/2031883002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/auto_bisect/resources/cpu_usage.py:57:
top_processes = processes[WINDOWS_START_LINE_NUM: \
Backslash not necessary to break the line here because of the open brackets.

https://codereview.chromium.org/2031883002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/auto_bisect/resources/cpu_usage.py:58:
WINDOWS_START_LINE_NUM+n]
spaces around the + operator, align with the previous line.

https://codereview.chromium.org/2031883002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/auto_bisect/resources/cpu_usage.py:61:
top_processes = processes[LINUX_START_LINE_NUM: \
same as before

https://codereview.chromium.org/2031883002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/auto_bisect/resources/cpu_usage.py:69: parser =
argparse.ArgumentParser(description=\
Maybe it would be easier to read to break the line after opening parenthesis,
backslash not necessary in this case.

https://codereview.chromium.org/2031883002/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/auto_bisect/revision_state.py (right):

https://codereview.chromium.org/2031883002/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/auto_bisect/revision_state.py:495:
["python","resources/cpu_usage.py"],
instead of "resources/cpu_usage.py" use api.resource('cpu_usage.py') this lets
the recipe engine resolve the path however it sees fit.

Also, use single quotes wherever possible for consistency.

[RobertoCN, 2016-06-07 21:44:28]

robertocn@chromium.org changed reviewers:
+ prasadv@chromium.org, sullivan@chromium.org

[RobertoCN, 2016-06-07 21:44:29]

Some more comments. If you have addressed previous comments, reply to them to
mark them as Done.

https://codereview.chromium.org/2031883002/diff/160001/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/auto_bisect/revision_state.py (right):

https://codereview.chromium.org/2031883002/diff/160001/scripts/slave/recipe_m...
scripts/slave/recipe_modules/auto_bisect/revision_state.py:495:
['python',api.resource('cpu_usage.py')],
Alignment. I suggest
... = api.m.step(
    'Take a snapshot of CPU',
    ['python', api.resource('...')],
    stdout=...)

Also space after the comma that goes before api.resource

https://codereview.chromium.org/2031883002/diff/160001/scripts/slave/recipe_m...
scripts/slave/recipe_modules/auto_bisect/revision_state.py:499:
api.m.step.active_result.presentation.logs['CPU snapshot'] = process_list
You need to make sure that process_list is a list of lines so that you can
assign it to result.presentation.logs[...]

[prasadv, 2016-06-07 22:00:49]

https://codereview.chromium.org/2031883002/diff/160001/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/auto_bisect/bisector.py (right):

https://codereview.chromium.org/2031883002/diff/160001/scripts/slave/recipe_m...
scripts/slave/recipe_modules/auto_bisect/bisector.py:669: def
print_result_debug_info_with_cpu_snapshot(self):
can we rename this method name shorter? "print_cpu_snapshot"?

https://codereview.chromium.org/2031883002/diff/160001/scripts/slave/recipe_m...
scripts/slave/recipe_modules/auto_bisect/bisector.py:670: """Prints extra debug
info with CPU snapshot at the
Function docstring should be of single line summary. And the detail description
can be mentioned in a newline.

https://engdoc.corp.google.com/eng/doc/devguide/py/styleguide.shtml?cl=head#s...

https://codereview.chromium.org/2031883002/diff/160001/scripts/slave/recipe_m...
scripts/slave/recipe_modules/auto_bisect/bisector.py:677: 'Debug Info with CPU
Snapshot'] = lines
Need 4 space indentation. Or otherwise declare 
step_name = 'Debug Info with CPU Snapshot'
self.api.m.step(step_name, [])
self.api.m.step.active_result.presentation.logs[step_name] = lines

[prasadv, 2016-06-07 22:05:22]

https://codereview.chromium.org/2031883002/diff/160001/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/auto_bisect/resources/cpu_usage.py (right):

https://codereview.chromium.org/2031883002/diff/160001/scripts/slave/recipe_m...
scripts/slave/recipe_modules/auto_bisect/resources/cpu_usage.py:23: """Parses a
line from top output
According to python style guide we follow 2 space alignments.
https://engdoc.corp.google.com/eng/doc/devguide/py/styleguide.shtml?cl=head

[sullivan, 2016-06-07 22:15:34]

sullivan@chromium.org changed reviewers:
+ dtu@chromium.org, nednguyen@google.com

[sullivan, 2016-06-07 22:15:34]

There were some comments from Ned and Nat on the design doc on putting this into
telemetry as a lightweight tracing agent. I think that is a really good idea
because
1) We can get the data anywhere telemetry is run, from perfbots to bisect, to
locally
2) I think that should also be able to integrate with the telemetry code to
automatically upload traces to cloud storage, so that they could easily be
linked and shared.
Let's discuss at the automation & monitoring meeting tomorrow.

Also, please link the design doc in the CL description:
https://docs.google.com/document/d/1fYKiJn270MY-1Wz1cPbrKHGwlDuo2vDauWRPb50z3...

[Ziqi Xiong, 2016-06-07 22:24:12]

Description was changed from

==========
Initiate the script that takes a snapshot of CPU usage
Initiate script that prints out top processes running in the background

BUG=
==========

to

==========
design doc:
https://docs.google.com/document/d/1fYKiJn270MY-1Wz1cPbrKHGwlDuo2vDauWRPb50z3...
Initiate the script that takes a snapshot of CPU usage
Initiate script that prints out top processes running in the background

BUG=
==========

[Ziqi Xiong, 2016-06-09 17:58:39]

Description was changed from

==========
design doc:
https://docs.google.com/document/d/1fYKiJn270MY-1Wz1cPbrKHGwlDuo2vDauWRPb50z3...
Initiate the script that takes a snapshot of CPU usage
Initiate script that prints out top processes running in the background

BUG=
==========

to

==========
design doc:
https://docs.google.com/document/d/1fYKiJn270MY-1Wz1cPbrKHGwlDuo2vDauWRPb50z3...
incorporate CPU usage information into the results of bisect jobs to help
sheriffs on duty to decide the validity of bisect scores.


BUG=
==========

[Ziqi Xiong, 2016-06-23 21:08:19]

Description was changed from

==========
design doc:
https://docs.google.com/document/d/1fYKiJn270MY-1Wz1cPbrKHGwlDuo2vDauWRPb50z3...
incorporate CPU usage information into the results of bisect jobs to help
sheriffs on duty to decide the validity of bisect scores.


BUG=
==========

to

==========
Go to issue 2066413002 for latest updates


BUG=
==========