Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2301523007:
Add trace events for some of telemetry internal functions (Closed)
DescriptionAdd trace events for some of telemetry internal functions
This will help understand how telemetry spends its time.
Results of this CL: http://imgur.com/a/h4NSt
This CL also fixes some of the timing in py_trace_event to use
trace_time.time().
BUG=chromium:641934
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/6dd81149598a684d96bdf3cd482e004580ea040a
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address Juan's comments #
Total comments: 3
Patch Set 3 : Add traced meta_class #Patch Set 4 : Add more trace events #Patch Set 5 : rebased #Patch Set 6 : Address Petr's nits #
Total comments: 10
Patch Set 7 : Address Petr's nit #Messages
Total messages: 56 (31 generated)
Description was changed from ========== Add traces for some of telemetry internal functions This will help understand how telemetry spend it times. Results of this CL: http://i.imgur.com/eXKNZFL.png BUG=chromium:641934 ========== to ========== Add traces for some of telemetry internal functions This will help understand how telemetry spend it times. Results of this CL: http://i.imgur.com/eXKNZFL.png This CL also fix some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 ==========
nednguyen@google.com changed reviewers: + charliea@chromium.org, zhenw@chromium.org
The CQ bit was checked by nednguyen@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
perezju@chromium.org changed reviewers: + perezju@chromium.org
Just a few drive by comments. Also, not sure on the exact mechanics of this, but could it be possible to name the process so it says on the trace e.g. "Telemetry" instead of "Process 34746"? https://codereview.chromium.org/2301523007/diff/1/telemetry/telemetry/interna... File telemetry/telemetry/internal/actions/action_runner.py (right): https://codereview.chromium.org/2301523007/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:50: with trace_event.trace('WillRun%s' % str(action_name)): Should this be '%s.WillRunAction' ? nit: no need for the 'str'
https://codereview.chromium.org/2301523007/diff/1/telemetry/telemetry/interna... File telemetry/telemetry/internal/actions/action_runner.py (right): https://codereview.chromium.org/2301523007/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:50: with trace_event.trace('WillRun%s' % str(action_name)): On 2016/09/02 15:41:04, perezju wrote: > Should this be '%s.WillRunAction' ? > > nit: no need for the 'str' Done.
On 2016/09/02 15:44:06, nednguyen wrote: > https://codereview.chromium.org/2301523007/diff/1/telemetry/telemetry/interna... > File telemetry/telemetry/internal/actions/action_runner.py (right): > > https://codereview.chromium.org/2301523007/diff/1/telemetry/telemetry/interna... > telemetry/telemetry/internal/actions/action_runner.py:50: with > trace_event.trace('WillRun%s' % str(action_name)): > On 2016/09/02 15:41:04, perezju wrote: > > Should this be '%s.WillRunAction' ? > > > > nit: no need for the 'str' > > Done. Ben, Petr: how do we populate ""Telemetry (Process 34736)" instead of "Process 34746"?"
On 2016/09/02 15:44:46, nednguyen wrote: > On 2016/09/02 15:44:06, nednguyen wrote: > > > https://codereview.chromium.org/2301523007/diff/1/telemetry/telemetry/interna... > > File telemetry/telemetry/internal/actions/action_runner.py (right): > > > > > https://codereview.chromium.org/2301523007/diff/1/telemetry/telemetry/interna... > > telemetry/telemetry/internal/actions/action_runner.py:50: with > > trace_event.trace('WillRun%s' % str(action_name)): > > On 2016/09/02 15:41:04, perezju wrote: > > > Should this be '%s.WillRunAction' ? > > > > > > nit: no need for the 'str' > > > > Done. > > Ben, Petr: how do we populate ""Telemetry (Process 34736)" instead of "Process > 34746"?" The trace is having problem on Android. This is not ready to land yet.
On 2016/09/02 16:36:09, nednguyen wrote: > On 2016/09/02 15:44:46, nednguyen wrote: > > On 2016/09/02 15:44:06, nednguyen wrote: > > > > > > https://codereview.chromium.org/2301523007/diff/1/telemetry/telemetry/interna... > > > File telemetry/telemetry/internal/actions/action_runner.py (right): > > > > > > > > > https://codereview.chromium.org/2301523007/diff/1/telemetry/telemetry/interna... > > > telemetry/telemetry/internal/actions/action_runner.py:50: with > > > trace_event.trace('WillRun%s' % str(action_name)): > > > On 2016/09/02 15:41:04, perezju wrote: > > > > Should this be '%s.WillRunAction' ? > > > > > > > > nit: no need for the 'str' > > > > > > Done. > > > > Ben, Petr: how do we populate ""Telemetry (Process 34736)" instead of "Process > > 34746"?" > > The trace is having problem on Android. This is not ready to land yet. Filed https://github.com/catapult-project/catapult/issues/2759, I suspect this is a clock sync issue
Description was changed from ========== Add traces for some of telemetry internal functions This will help understand how telemetry spend it times. Results of this CL: http://i.imgur.com/eXKNZFL.png This CL also fix some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 ========== to ========== Add traces for some of telemetry internal functions This will help understand how telemetry spend it times. Results of this CL: http://i.imgur.com/eXKNZFL.png This CL also fix some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ==========
lgtm, but understand that it's blocked on https://github.com/catapult-project/catapult/issues/2600 I have a partially implemented fix, but the fix is actually slightly trickier than it initially looks due to the directory structure
benjhayden@chromium.org changed reviewers: + benjhayden@chromium.org
drive-by question :-) https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... File telemetry/telemetry/internal/story_runner.py (right): https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... telemetry/telemetry/internal/story_runner.py:83: with trace_event.trace('%s.WillRunStory' % state_name, story=story.name): It seems strange that some methods can use the traced() decorator and some require callers to trace them. Would it be possible to use the traced() decorator on the def WillRunStory() to log the story.name?
https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... File telemetry/telemetry/internal/story_runner.py (right): https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... telemetry/telemetry/internal/story_runner.py:83: with trace_event.trace('%s.WillRunStory' % state_name, story=story.name): On 2016/09/02 22:57:15, benjhayden wrote: > It seems strange that some methods can use the traced() decorator and some > require callers to trace them. The rationale is we can have inheritance, so if we just decorate a class method, the trace won't be captured for another class. Things like Shared..State & StoryRunner are subclassable hence I wrap the trace in the caller. traced() decorator are convenient to used & hence is used for ones that don't have such problems. > Would it be possible to use the traced() decorator on the def WillRunStory() to > log the story.name? I did, see the "story=story.name" part ;)
On 2016/09/02 at 23:12:17, nednguyen wrote: > https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... > File telemetry/telemetry/internal/story_runner.py (right): > > https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... > telemetry/telemetry/internal/story_runner.py:83: with trace_event.trace('%s.WillRunStory' % state_name, story=story.name): > On 2016/09/02 22:57:15, benjhayden wrote: > > It seems strange that some methods can use the traced() decorator and some > > require callers to trace them. > > > The rationale is we can have inheritance, so if we just decorate a class method, the trace won't be captured for another class. Things like Shared..State & StoryRunner are subclassable hence I wrap the trace in the caller. Why not use a metaclass? It would take a couple of lines to iterate over all of a class's methods and decorate them, then one line in the base classes to set __metaclass__. > > traced() decorator are convenient to used & hence is used for ones that don't have such problems. > > > Would it be possible to use the traced() decorator on the def WillRunStory() to > > log the story.name? > > I did, see the "story=story.name" part ;) Sorry, my point was to ask if there was any way to do that in the decorator on the definition of the method instead of in the caller. Now that I look at traced(), it looks like it already logs the decorated method's arguments, such as |story| passed to WillRunStory. So you don't need to explicitly log story.name as log as repr(story) includes story.name. Right? :-)
On 2016/09/03 05:00:12, benjhayden wrote: > On 2016/09/02 at 23:12:17, nednguyen wrote: > > > https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... > > File telemetry/telemetry/internal/story_runner.py (right): > > > > > https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... > > telemetry/telemetry/internal/story_runner.py:83: with > trace_event.trace('%s.WillRunStory' % state_name, story=story.name): > > On 2016/09/02 22:57:15, benjhayden wrote: > > > It seems strange that some methods can use the traced() decorator and some > > > require callers to trace them. > > > > > > The rationale is we can have inheritance, so if we just decorate a class > method, the trace won't be captured for another class. Things like Shared..State > & StoryRunner are subclassable hence I wrap the trace in the caller. > > Why not use a metaclass? > It would take a couple of lines to iterate over all of a class's methods and > decorate them, then one line in the base classes to set __metaclass__. This is an interesting thought. I will give it a try. > > > > > traced() decorator are convenient to used & hence is used for ones that don't > have such problems. > > > > > Would it be possible to use the traced() decorator on the def WillRunStory() > to > > > log the story.name? > > > > I did, see the "story=story.name" part ;) > > Sorry, my point was to ask if there was any way to do that in the decorator on > the definition of the method instead of in the caller. > > Now that I look at traced(), it looks like it already logs the decorated > method's arguments, such as |story| passed to WillRunStory. So you don't need to > explicitly log story.name as log as repr(story) includes story.name. Right? :-)
On 2016/09/03 11:40:44, nednguyen wrote: > On 2016/09/03 05:00:12, benjhayden wrote: > > On 2016/09/02 at 23:12:17, nednguyen wrote: > > > > > > https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... > > > File telemetry/telemetry/internal/story_runner.py (right): > > > > > > > > > https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... > > > telemetry/telemetry/internal/story_runner.py:83: with > > trace_event.trace('%s.WillRunStory' % state_name, story=story.name): > > > On 2016/09/02 22:57:15, benjhayden wrote: > > > > It seems strange that some methods can use the traced() decorator and some > > > > require callers to trace them. > > > > > > > > > The rationale is we can have inheritance, so if we just decorate a class > > method, the trace won't be captured for another class. Things like > Shared..State > > & StoryRunner are subclassable hence I wrap the trace in the caller. > > > > Why not use a metaclass? > > It would take a couple of lines to iterate over all of a class's methods and > > decorate them, then one line in the base classes to set __metaclass__. > > This is an interesting thought. I will give it a try. > > > > > > > > > traced() decorator are convenient to used & hence is used for ones that > don't > > have such problems. > > > > > > > Would it be possible to use the traced() decorator on the def > WillRunStory() > > to > > > > log the story.name? > > > > > > I did, see the "story=story.name" part ;) > > > > Sorry, my point was to ask if there was any way to do that in the decorator on > > the definition of the method instead of in the caller. > > > > Now that I look at traced(), it looks like it already logs the decorated > > method's arguments, such as |story| passed to WillRunStory. So you don't need > to > > explicitly log story.name as log as repr(story) includes story.name. Right? > :-) Actually not, some of these subclass are defined by clients, so there is no guaranteed that they will go extra miles to annotate the code for tracing. I still think it's simplest to handle this on the sites to run the callback to guarantee trace coverage.
On 2016/09/03 at 11:50:30, nednguyen wrote: > On 2016/09/03 11:40:44, nednguyen wrote: > > On 2016/09/03 05:00:12, benjhayden wrote: > > > On 2016/09/02 at 23:12:17, nednguyen wrote: > > > > > > > > > https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... > > > > File telemetry/telemetry/internal/story_runner.py (right): > > > > > > > > > > > > > https://codereview.chromium.org/2301523007/diff/20001/telemetry/telemetry/int... > > > > telemetry/telemetry/internal/story_runner.py:83: with > > > trace_event.trace('%s.WillRunStory' % state_name, story=story.name): > > > > On 2016/09/02 22:57:15, benjhayden wrote: > > > > > It seems strange that some methods can use the traced() decorator and some > > > > > require callers to trace them. > > > > > > > > > > > > The rationale is we can have inheritance, so if we just decorate a class > > > method, the trace won't be captured for another class. Things like > > Shared..State > > > & StoryRunner are subclassable hence I wrap the trace in the caller. > > > > > > Why not use a metaclass? > > > It would take a couple of lines to iterate over all of a class's methods and > > > decorate them, then one line in the base classes to set __metaclass__. > > > > This is an interesting thought. I will give it a try. > > > > > > > > > > > > > traced() decorator are convenient to used & hence is used for ones that > > don't > > > have such problems. > > > > > > > > > Would it be possible to use the traced() decorator on the def > > WillRunStory() > > > to > > > > > log the story.name? > > > > > > > > I did, see the "story=story.name" part ;) > > > > > > Sorry, my point was to ask if there was any way to do that in the decorator on > > > the definition of the method instead of in the caller. > > > > > > Now that I look at traced(), it looks like it already logs the decorated > > > method's arguments, such as |story| passed to WillRunStory. So you don't need > > to > > > explicitly log story.name as log as repr(story) includes story.name. Right? > > :-) > > Actually not, some of these subclass are defined by clients, so there is no guaranteed that they will go extra miles to annotate the code for tracing. I still think it's simplest to handle this on the sites to run the callback to guarantee trace coverage. If you use a metaclass on the baseclass, then the subclass does not need any annotation. Subclasses inherit __metaclass__.
lgtm
petrcermak@chromium.org changed reviewers: + petrcermak@chromium.org
LGTM with some description nits: 1) s/Add traces/Add trace events/ 2) s/telemetry spend it times/telemetry spends its time/ 3) s/This CL also fix/This CL also fixes/ Thanks, Petr https://codereview.chromium.org/2301523007/diff/20001/common/py_trace_event/p... File common/py_trace_event/py_trace_event/trace_event_impl/log.py (right): https://codereview.chromium.org/2301523007/diff/20001/common/py_trace_event/p... common/py_trace_event/py_trace_event/trace_event_impl/log.py:13: from py_trace_event import trace_time nit: Is there a reason why py_utils is before py_trace_event? If not, I would opt for alphabetical order.
Description was changed from ========== Add traces for some of telemetry internal functions This will help understand how telemetry spend it times. Results of this CL: http://i.imgur.com/eXKNZFL.png This CL also fix some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ========== to ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spend it times. Results of this CL: http://i.imgur.com/eXKNZFL.png This CL also fix some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ==========
On 2016/09/05 12:50:47, petrcermak wrote: > LGTM with some description nits: > > 1) s/Add traces/Add trace events/ > 2) s/telemetry spend it times/telemetry spends its time/ > 3) s/This CL also fix/This CL also fixes/ > > Thanks, > Petr > > https://codereview.chromium.org/2301523007/diff/20001/common/py_trace_event/p... > File common/py_trace_event/py_trace_event/trace_event_impl/log.py (right): > > https://codereview.chromium.org/2301523007/diff/20001/common/py_trace_event/p... > common/py_trace_event/py_trace_event/trace_event_impl/log.py:13: from > py_trace_event import trace_time > nit: Is there a reason why py_utils is before py_trace_event? If not, I would > opt for alphabetical order. Thanks Ben for your suggestion, we now are able to get a lot more trace events with fewer code: http://imgur.com/a/h4NSt
Description was changed from ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spend it times. Results of this CL: http://i.imgur.com/eXKNZFL.png This CL also fix some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ========== to ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spend it times. Results of this CL: http://imgur.com/a/h4NSt This CL also fix some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ==========
Description was changed from ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spend it times. Results of this CL: http://imgur.com/a/h4NSt This CL also fix some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ========== to ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fix some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ==========
Description was changed from ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fix some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ========== to ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ==========
On 2016/09/06 20:44:50, nednguyen wrote: > On 2016/09/05 12:50:47, petrcermak wrote: > > LGTM with some description nits: > > > > 1) s/Add traces/Add trace events/ > > 2) s/telemetry spend it times/telemetry spends its time/ > > 3) s/This CL also fix/This CL also fixes/ > > > > Thanks, > > Petr > > > > > https://codereview.chromium.org/2301523007/diff/20001/common/py_trace_event/p... > > File common/py_trace_event/py_trace_event/trace_event_impl/log.py (right): > > > > > https://codereview.chromium.org/2301523007/diff/20001/common/py_trace_event/p... > > common/py_trace_event/py_trace_event/trace_event_impl/log.py:13: from > > py_trace_event import trace_time > > nit: Is there a reason why py_utils is before py_trace_event? If not, I would > > opt for alphabetical order. > > Thanks Ben for your suggestion, we now are able to get a lot more trace events > with fewer code: http://imgur.com/a/h4NSt I will leave the issues of showing "telemetry" in the process track view & display the args as follow up work.
https://codereview.chromium.org/2301523007/diff/100001/common/py_trace_event/... File common/py_trace_event/py_trace_event/trace_event_impl/meta_class.py (right): https://codereview.chromium.org/2301523007/diff/100001/common/py_trace_event/... common/py_trace_event/py_trace_event/trace_event_impl/meta_class.py:14: attrs[attr_name] = decorators.traced(attr_value) is this tracing every single method? I think that, at the very least, we should skip methods whose names start with '_'.
I'm a big fan of metaclasses, so LGTM :-) supernit: Missing period after "trace_time.time()" in the patch description. Thanks, Petr https://codereview.chromium.org/2301523007/diff/100001/common/py_trace_event/... File common/py_trace_event/py_trace_event/trace_event_impl/meta_class.py (right): https://codereview.chromium.org/2301523007/diff/100001/common/py_trace_event/... common/py_trace_event/py_trace_event/trace_event_impl/meta_class.py:14: attrs[attr_name] = decorators.traced(attr_value) On 2016/09/07 09:06:51, perezju wrote: > is this tracing every single method? I think that, at the very least, we should > skip methods whose names start with '_'. Unless this hinders performance or causes the resulting traces to be too big (especially the UI), I think it's fine. https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/be... File telemetry/telemetry/benchmark_runner.py (right): https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/be... telemetry/telemetry/benchmark_runner.py:40: nit: there should be 2 blank lines between top-level statements https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/be... telemetry/telemetry/benchmark_runner.py:379: # the log level is set in browser_options nit: s/the/The/ and missing period. https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/in... File telemetry/telemetry/internal/actions/page_action.py (right): https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/in... telemetry/telemetry/internal/actions/page_action.py:11: nit: there should be only 2 blank lines (please remove one) https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/pa... File telemetry/telemetry/page/legacy_page_test.py (right): https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/pa... telemetry/telemetry/page/legacy_page_test.py:14: nit: there should be only 2 blank lines (remove one).
Description was changed from ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time() BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ========== to ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time(). BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ==========
https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/be... File telemetry/telemetry/benchmark_runner.py (right): https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/be... telemetry/telemetry/benchmark_runner.py:40: On 2016/09/07 09:38:24, petrcermak wrote: > nit: there should be 2 blank lines between top-level statements Done. https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/be... telemetry/telemetry/benchmark_runner.py:379: # the log level is set in browser_options On 2016/09/07 09:38:24, petrcermak wrote: > nit: s/the/The/ and missing period. Done. https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/in... File telemetry/telemetry/internal/actions/page_action.py (right): https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/in... telemetry/telemetry/internal/actions/page_action.py:11: On 2016/09/07 09:38:24, petrcermak wrote: > nit: there should be only 2 blank lines (please remove one) Done. https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/pa... File telemetry/telemetry/page/legacy_page_test.py (right): https://codereview.chromium.org/2301523007/diff/100001/telemetry/telemetry/pa... telemetry/telemetry/page/legacy_page_test.py:14: On 2016/09/07 09:38:24, petrcermak wrote: > nit: there should be only 2 blank lines (remove one). Done.
The CQ bit was checked by nednguyen@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time(). BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2759) ========== to ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time(). BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2600) ==========
benjhayden@chromium.org changed reviewers: - benjhayden@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by nednguyen@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from zhenw@chromium.org, charliea@chromium.org, petrcermak@chromium.org Link to the patchset: https://codereview.chromium.org/2301523007/#ps120001 (title: "Address Petr's nit")
Description was changed from ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time(). BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2600) ========== to ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time(). BUG=chromium:641934 ==========
The CQ bit was unchecked by commit-bot@chromium.org
COMMIT=false detected. CQ is abandoning the patch.
The CQ bit was checked by nednguyen@google.com
The CQ bit was unchecked by commit-bot@chromium.org
Description was changed from ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time(). BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2600) ========== to ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time(). BUG=chromium:641934 ==========
COMMIT=false detected. CQ is abandoning the patch.
The CQ bit was checked by nednguyen@google.com
The CQ bit was unchecked by commit-bot@chromium.org
COMMIT=false detected. CQ is abandoning the patch.
Description was changed from ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time(). BUG=chromium:641934 COMMIT=false (blocked on https://github.com/catapult-project/catapult/issues/2600) ========== to ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time(). BUG=chromium:641934 ==========
The CQ bit was checked by nednguyen@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time(). BUG=chromium:641934 ========== to ========== Add trace events for some of telemetry internal functions This will help understand how telemetry spends its time. Results of this CL: http://imgur.com/a/h4NSt This CL also fixes some of the timing in py_trace_event to use trace_time.time(). BUG=chromium:641934 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapu... ==========
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapu... |
