[pinpoint] Add trace links.

[pinpoint] Add trace links.

BUG=catapult:#3887

[dtu, 2017-09-26 22:45:46]

dtu@chromium.org changed reviewers:
+ perezju@chromium.org, simonhatch@chromium.org, sullivan@chromium.org

[sullivan, 2017-09-27 01:44:32]

lgtm

\o/

[perezju, 2017-09-27 08:33:07]

lgtm

[shatch, 2017-09-27 11:55:50]

https://codereview.chromium.org/3014663002/diff/1/dashboard/dashboard/pinpoin...
File dashboard/dashboard/pinpoint/models/quest/read_value.py (right):

https://codereview.chromium.org/3014663002/diff/1/dashboard/dashboard/pinpoin...
dashboard/dashboard/pinpoint/models/quest/read_value.py:51: if not
self._trace_url:
Getting a failure here with no attribute _trace_url on old data. Should include
some migration in this cl.

https://codereview.chromium.org/3014663002/diff/1/dashboard/dashboard/pinpoin...
dashboard/dashboard/pinpoint/models/quest/read_value.py:77: if self._trace:
The bug linked (technically the one duped and closed) is for surfacing all the
trace links for perf try jobs, but this looks like it'll only surface a single
trace link?

[perezju, 2017-09-27 12:24:06]

https://codereview.chromium.org/3014663002/diff/1/dashboard/dashboard/pinpoin...
File dashboard/dashboard/pinpoint/models/quest/read_value.py (right):

https://codereview.chromium.org/3014663002/diff/1/dashboard/dashboard/pinpoin...
dashboard/dashboard/pinpoint/models/quest/read_value.py:77: if self._trace:
On 2017/09/27 11:55:50, shatch wrote:
> The bug linked (technically the one duped and closed) is for surfacing all the
> trace links for perf try jobs, but this looks like it'll only surface a single
> trace link?

chartjson (I think) only keeps a link to one of the traces. :(

And, in most cases, since we're using --pageset-repeat 1 this should be OK (I
think).

And this shouldn't be an issue when we move to proper histograms.

[shatch, 2017-09-27 14:48:44]

On 2017/09/27 12:24:06, perezju wrote:
>
https://codereview.chromium.org/3014663002/diff/1/dashboard/dashboard/pinpoin...
> File dashboard/dashboard/pinpoint/models/quest/read_value.py (right):
> 
>
https://codereview.chromium.org/3014663002/diff/1/dashboard/dashboard/pinpoin...
> dashboard/dashboard/pinpoint/models/quest/read_value.py:77: if self._trace:
> On 2017/09/27 11:55:50, shatch wrote:
> > The bug linked (technically the one duped and closed) is for surfacing all
the
> > trace links for perf try jobs, but this looks like it'll only surface a
single
> > trace link?
> 
> chartjson (I think) only keeps a link to one of the traces. :(
> 
> And, in most cases, since we're using --pageset-repeat 1 this should be OK (I
> think).
> 

I thought there's a trace produced for every story for every repeat? So this
approach would work fine for bisect but presumably a perf try job could run
without a story filter.

> And this shouldn't be an issue when we move to proper histograms.

[perezju, 2017-09-27 15:38:13]

On 2017/09/27 14:48:44, shatch wrote:
> On 2017/09/27 12:24:06, perezju wrote:
> >
>
https://codereview.chromium.org/3014663002/diff/1/dashboard/dashboard/pinpoin...
> > File dashboard/dashboard/pinpoint/models/quest/read_value.py (right):
> > 
> >
>
https://codereview.chromium.org/3014663002/diff/1/dashboard/dashboard/pinpoin...
> > dashboard/dashboard/pinpoint/models/quest/read_value.py:77: if self._trace:
> > On 2017/09/27 11:55:50, shatch wrote:
> > > The bug linked (technically the one duped and closed) is for surfacing all
> the
> > > trace links for perf try jobs, but this looks like it'll only surface a
> single
> > > trace link?
> > 
> > chartjson (I think) only keeps a link to one of the traces. :(
> > 
> > And, in most cases, since we're using --pageset-repeat 1 this should be OK
(I
> > think).
> > 
> 
> I thought there's a trace produced for every story for every repeat?

Yes, there is one trace for every story repeat, but only one of them survives to
the chartjson file due to:
https://github.com/catapult-project/catapult/blob/1b6b78dad5017a6279f9f02a553...

See e.g.:
https://luci-logdog.appspot.com/v/?s=chrome%2Fbb%2Fchromium.perf%2FAndroid_Ne...

There is only one trace per story, despite each story running 3 times.

This is not an issue in histograms json (AFAIK).

> So this
> approach would work fine for bisect but presumably a perf try job could run
> without a story filter.

Yes, but note this chartjson reader only gets either:

- values for a particular story (confusingly named "trace" here due to legacy
chartjson-reasons)
- or the summary values (i.e. averages over all stories for the selected
metric).

In the later case I think this code gets no traces at all.

Aside, it breaks my head trying to remember this mapping, probably should be
added to some docstring here:

chart -> metric (e.g.
"memory:chrome:browser_process:reported_by_os:proportional_resident_size_avg")
tir_label -> story_group (e.g. "load_news")
trace -> story (e.g. "load:news:cnn") or "summary"

Also I known they don't always match exactly for all benchmarks, but maybe we
could use the "new" names in the code here? (follow up CL?)