Make traces produced by telemetry combine all the trace parts

Make traces produced by telemetry combine all the trace parts

reland of: https://codereview.chromium.org/1980773002/

BUG=catapult:#2309

Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/3fddff09b9dbe88cfec501a5b41899fdc7331abe

[rnephew (Reviews Here), 2016-05-24 17:17:02]

rnephew@chromium.org changed reviewers:
+ charliea@chromium.org, nednguyen@google.com

[rnephew (Reviews Here), 2016-05-24 17:17:02]

Tested locally and tracing.tracing_with_debug_overhead is working.

[nednguyen, 2016-05-24 17:22:26]

On 2016/05/24 17:17:02, rnephew (Reviews Here) wrote:
> Tested locally and tracing.tracing_with_debug_overhead is working.

What was the fix?

[rnephew (Reviews Here), 2016-05-24 17:25:15]

On 2016/05/24 17:22:26, nednguyen wrote:
> On 2016/05/24 17:17:02, rnephew (Reviews Here) wrote:
> > Tested locally and tracing.tracing_with_debug_overhead is working.
> 
> What was the fix?

Charlie's work the past 2 days with:
https://codereview.chromium.org/2002193002/
https://codereview.chromium.org/2007533002/
https://codereview.chromium.org/2006703003/

[rnephew (Reviews Here), 2016-05-24 17:36:26]

I have enabled chrome tracing in the tbmv2 smoke test.

[nednguyen, 2016-05-24 17:38:45]

The CQ bit was checked by nednguyen@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-24 17:38:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2004103002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2004103002/40001

[commit-bot: I haz the power, 2016-05-24 18:08:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-24 18:08:01]

Dry run: This issue passed the CQ dry run.

[charliea (OOO until 10-5), 2016-05-24 18:28:39]

On 2016/05/24 17:25:15, rnephew (Reviews Here) wrote:
> On 2016/05/24 17:22:26, nednguyen wrote:
> > On 2016/05/24 17:17:02, rnephew (Reviews Here) wrote:
> > > Tested locally and tracing.tracing_with_debug_overhead is working.
> > 
> > What was the fix?
> 
> Charlie's work the past 2 days with:
> https://codereview.chromium.org/2002193002/
> https://codereview.chromium.org/2007533002/
> https://codereview.chromium.org/2006703003/

Specifically, we needed TELEMETRY to be specified as the clock domain for the
trace.

Trace viewer was trying to import two traces:

  1) The Telemetry trace, with clock domain UNKNOWN_CHROME_LEGACY and sync
marker 'abc'
  2) The Chrome trace, with clock domain UNKNOWN_CHROME_LEGACY and sync marker
'abc'

The bug here was that Telemetry's clock domain *isn't* UNKNOWN_CHROME_LEGACY,
but was just showing up as such because we hadn't added its real clock domain to
the trace metadata yet. https://codereview.chromium.org/2006703003/ did that,
which caused this CL to stop causing the error "A clock domain cannot sync with
itself."

[charliea (OOO until 10-5), 2016-05-24 18:29:16]

On 2016/05/24 18:28:39, charliea wrote:
> On 2016/05/24 17:25:15, rnephew (Reviews Here) wrote:
> > On 2016/05/24 17:22:26, nednguyen wrote:
> > > On 2016/05/24 17:17:02, rnephew (Reviews Here) wrote:
> > > > Tested locally and tracing.tracing_with_debug_overhead is working.
> > > 
> > > What was the fix?
> > 
> > Charlie's work the past 2 days with:
> > https://codereview.chromium.org/2002193002/
> > https://codereview.chromium.org/2007533002/
> > https://codereview.chromium.org/2006703003/
> 
> Specifically, we needed TELEMETRY to be specified as the clock domain for the
> trace.
> 
> Trace viewer was trying to import two traces:
> 
>   1) The Telemetry trace, with clock domain UNKNOWN_CHROME_LEGACY and sync
> marker 'abc'
>   2) The Chrome trace, with clock domain UNKNOWN_CHROME_LEGACY and sync marker
> 'abc'
> 
> The bug here was that Telemetry's clock domain *isn't* UNKNOWN_CHROME_LEGACY,
> but was just showing up as such because we hadn't added its real clock domain
to
> the trace metadata yet. https://codereview.chromium.org/2006703003/ did that,
> which caused this CL to stop causing the error "A clock domain cannot sync
with
> itself."

non-owner lgtm

[nednguyen, 2016-05-24 19:13:52]

Can you update the title of the CL. "Changes needed to runBattor tests on
telemetry" is not very descriptive.

I think the core change here is to trace_value, which you make sure that the
trace produced by telemetry combined all the trace parts instead of just chrome
trace.

https://codereview.chromium.org/2004103002/diff/40001/telemetry/telemetry/tim...
File telemetry/telemetry/timeline/trace_data.py (right):

https://codereview.chromium.org/2004103002/diff/40001/telemetry/telemetry/tim...
telemetry/telemetry/timeline/trace_data.py:198: if isinstance(events,
basestring):
We no longer need this change since you can just use SetTraceFor directly

[rnephew (Reviews Here), 2016-05-24 19:26:47]

Description was changed from

==========
Reland [BattOr][Telemetry] Changes needed to run BattOr tests on telemetry.

reland of: https://codereview.chromium.org/1980773002/

BUG=catapult:#2309
==========

to

==========
Make traces produced by telemetry combine all the trace parts

reland of: https://codereview.chromium.org/1980773002/

BUG=catapult:#2309
==========

[rnephew (Reviews Here), 2016-05-24 19:26:57]

https://codereview.chromium.org/2004103002/diff/40001/telemetry/telemetry/tim...
File telemetry/telemetry/timeline/trace_data.py (right):

https://codereview.chromium.org/2004103002/diff/40001/telemetry/telemetry/tim...
telemetry/telemetry/timeline/trace_data.py:198: if isinstance(events,
basestring):
On 2016/05/24 19:13:52, nednguyen wrote:
> We no longer need this change since you can just use SetTraceFor directly

Done.

[nednguyen, 2016-05-24 19:29:06]

lg2me but please be careful with rebase, otherwise you may end up undo Petr's CL

https://codereview.chromium.org/2004103002/diff/60001/telemetry/telemetry/int...
File telemetry/telemetry/internal/backends/chrome_inspector/tracing_backend.py
(right):

https://codereview.chromium.org/2004103002/diff/60001/telemetry/telemetry/int...
telemetry/telemetry/internal/backends/chrome_inspector/tracing_backend.py:90:
support_modern_devtools_tracing_start_api=False):
Looks like you need to rebase. This is changes from Petr's CL

https://codereview.chromium.org/2004103002/diff/60001/telemetry/telemetry/tim...
File telemetry/telemetry/timeline/tracing_config.py (right):

https://codereview.chromium.org/2004103002/diff/60001/telemetry/telemetry/tim...
telemetry/telemetry/timeline/tracing_config.py:205: def
requires_modern_devtools_tracing_start_api(self):
Same here

[rnephew (Reviews Here), 2016-05-24 19:43:38]

Rebased.

[nednguyen, 2016-05-24 19:47:40]

lgtm

[rnephew (Reviews Here), 2016-05-24 19:49:19]

The CQ bit was checked by rnephew@chromium.org

[rnephew (Reviews Here), 2016-05-24 19:49:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
charliea@chromium.org
Link to the patchset: https://codereview.chromium.org/2004103002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-24 19:49:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2004103002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2004103002/80001

[commit-bot: I haz the power, 2016-05-24 20:53:15]

Description was changed from

==========
Make traces produced by telemetry combine all the trace parts

reland of: https://codereview.chromium.org/1980773002/

BUG=catapult:#2309
==========

to

==========
Make traces produced by telemetry combine all the trace parts

reland of: https://codereview.chromium.org/1980773002/

BUG=catapult:#2309

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-05-24 20:53:16]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...