Create debug dialog for data stoppage alerts.

Create debug dialog for data stoppage alerts.

This CL creates a dialog and XHR backend for debugging stoppage alerts and
wires it into the stoppage alert table and the tooltip for the last datapoint on a
chart.

BUG=catapult:#3132

Review-Url: https://codereview.chromium.org/2616153002
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/8295aca699a5a202abab86f88481ee9ef721fad5

[sullivan, 2017-01-09 03:45:49]

sullivan@chromium.org changed reviewers:
+ aiolos@chromium.org, eakuefner@chromium.org, simonhatch@chromium.org

[sullivan, 2017-01-09 03:45:51]

I was perf sheriff last week and there were 2000 data stoppage alerts. In
addition, we had a perfbot sheriffing fixit and one thing that was really hard
was getting bisect parameters right.

Stephen had told me that the milo API can give us details about builds, which I
thought we could plug in to find out the revision range, next build, etc. This
is mostly working.

Demo: https://dev-sullivan-9e3ee3ae-dot-chromeperf.appspot.com/alerts

You can click "Debug" in the stoppage alerts table right-hand column, or on the
tooltip for the last data point in any graph. The one big bug I'm having right
now is that after signing in, clicking the bisect button doesn't work; the debug
dialog closes and you can't see the bisect one. Any ideas?

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/al...
File dashboard/dashboard/alerts.py (right):

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/al...
dashboard/dashboard/alerts.py:184: 'type': 'stoppage_alert',
These types fix a longstanding bug on /group_report: it did not correctly
specify extraColumns for stoppage alerts.

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/co...
File dashboard/dashboard/common/utils.py (right):

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/co...
dashboard/dashboard/common/utils.py:506: return
'https://luci-logdog.appspot.com/v/?s=%s' % s_param
These are in utils because they were factored out of graph_json.py; seems like a
good place to put them if multiple unrelated files need these functions.

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
File dashboard/dashboard/elements/alerts-page.html (right):

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
dashboard/dashboard/elements/alerts-page.html:212: 'label': ''
This adds the column for the debugging button.

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
File dashboard/dashboard/elements/alerts-table.html (left):

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
dashboard/dashboard/elements/alerts-table.html:383: <td
class="test"><label>{{item.test}}</label></td>
The functionality that used these labels was removed years ago.

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
File dashboard/dashboard/elements/bisect-button.html (right):

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
dashboard/dashboard/elements/bisect-button.html:65: },
I expose the default mode on the bisect so that the stoppage alert debug dialog
can kick off a return_code bisect.

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
File dashboard/dashboard/elements/bisect-form.html (right):

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
dashboard/dashboard/elements/bisect-form.html:191: bisectMode: {
Can only expose as a property if it follows polymer's naming conventions.

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
File dashboard/dashboard/elements/chart-container.html (right):

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
dashboard/dashboard/elements/chart-container.html:1989: dataIndex ===
seriesFromJson.data.length - 1;
The last point on the chart shows a debugging button.

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
File dashboard/dashboard/elements/group-report-page.html (right):

https://codereview.chromium.org/2616153002/diff/120001/dashboard/dashboard/el...
dashboard/dashboard/elements/group-report-page.html:470: }
This new code sets extraColumns on the alerts table in /group_report based on
whether the table is a set of performance regressions or data stoppage alerts.
Previously, it always assumed performance regressions.

[sullivan, 2017-01-09 15:02:27]

Please hold off on full review--this is too many CLs at once, breaking it up.

[eakuefner, 2017-01-09 21:49:03]

Please ping this CL when it's ready for review.

[sullivan, 2017-01-10 17:31:47]

The CQ bit was checked by sullivan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 17:31:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 17:39:18]

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

[commit-bot: I haz the power, 2017-01-10 17:39:19]

Dry run: Try jobs failed on following builders:
  Catapult Windows Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Wi...)

[sullivan, 2017-01-10 18:26:27]

Okay, this is ready for review. Demos (will need to log in to see all data):

https://dev-sullivan-3f7703fa-dot-chromeperf.appspot.com/alerts
https://dev-sullivan-3f854bb4-dot-chromeperf.appspot.com/group_report?sid=e08...

On both pages, you'll see the right column of the data stoppage table has debug
buttons. On the second page, you'll also see the debug button in the tooltip for
the last data point in the chart.


For background, I broke this up into multiple CLs. The rest are already
submitted:
https://codereview.chromium.org/2622503004/
https://codereview.chromium.org/2620713002/
https://codereview.chromium.org/2621643002/
https://codereview.chromium.org/2621803002/
https://codereview.chromium.org/2620693002/

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
File dashboard/dashboard/elements/alerts-page.html (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/alerts-page.html:212: 'label': ''
This adds a column for the debug button on the stoppage alert table.

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
File dashboard/dashboard/elements/alerts-table.html (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/alerts-table.html:394: </template>
This special case actually generates the HTML for the button on the alerts
table. The HTML for the other "extra" columns is all just the column value text
in the column table.

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
File dashboard/dashboard/elements/chart-container.html (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/chart-container.html:1989: dataIndex ===
seriesFromJson.data.length - 1;
This tells the tooltip whether it is rendering the last point in the chart, so
it can add a debug button for stoppages.

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
File dashboard/dashboard/elements/chart-tooltip.html (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/chart-tooltip.html:126: </template>
This actually shows the button on the tooltip for the last data point on a
chart.

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
File dashboard/dashboard/elements/group-report-page.html (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/group-report-page.html:440: 'label': ''
This adds the debug button to the alerts table in /group_report.

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
File dashboard/dashboard/elements/stoppage-alert-debug-button.html (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/stoppage-alert-debug-button.html:12: <dom-module
id="stoppage-alert-debug-button">
This polymer element shows the button. It makes an XHR when the button is
clicked to /stoppage_alert_debugging_info, and fills in the UI when that
returns.

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
File dashboard/dashboard/stoppage_alert_debugging_info.py (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
dashboard/dashboard/stoppage_alert_debugging_info.py:18: class
StoppageAlertDebuggingInfoHandler(request_handler.RequestHandler):
This is the url endpoint that fills in the data for the dialog. It uses
milo_service to get the revision of the next build, which makes getting the
revision range to look through and bisect a lot easier. We use milo_service
since it's the infra-supported method, and it has much longer history than other
options.

[sullivan, 2017-01-10 18:28:04]

Description was changed from

==========
Create debug dialog for data stoppage alerts.

* Adds milo service to get build data.
* Factors stdio_uri parsing into utils and creates
  stoppage_alert_debugging module.
==========

to

==========
Create debug dialog for data stoppage alerts.

This CL creates a dialog and XHR backend for debugging stoppage alerts and
wires it into the stoppage alert table and the tooltip for the last datapoint on
a
chart.

BUG=catapult:#3132
==========

[shatch, 2017-01-10 22:07:36]

Thanks for all the explanations throughout, made this super easy to understand
how the pieces fit together. lgtm

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
File dashboard/dashboard/elements/chart-container.html (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/chart-container.html:1989: dataIndex ===
seriesFromJson.data.length - 1;
On 2017/01/10 18:26:27, sullivan wrote:
> This tells the tooltip whether it is rendering the last point in the chart, so
> it can add a debug button for stoppages.

nit: group this with the other this.$.tooltip.foo = blah stuff below?

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
File dashboard/dashboard/stoppage_alert_debugging_info_test.py (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
dashboard/dashboard/stoppage_alert_debugging_info_test.py:68: print results
nit: debug stuff?

[jessimb, 2017-01-10 22:19:17]

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

[jessimb, 2017-01-10 22:19:18]

I tried the debug button from the /alerts page and am a bit confused.

The first stoppage alert (from 2016-11-19) next revision's status is indicated
as succeeded, however when I click on the buildbot status page, there is a list
of red failures. Perhaps this is user error? 

The second stoppage alert (from 2016-11-21) next revision's status is exception,
and when I click on the link I get a 404. The link is:
https://build.chromium.org/p/chromium.perf/builders/Linux%20Perf%20%282%29/bu...

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
File dashboard/dashboard/elements/stoppage-alert-debug-button.html (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/stoppage-alert-debug-button.html:167: if
(response['current_result'][0] == '0') {
What does response['current_result'][0] contain? Why is '0' a success?

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/stoppage-alert-debug-button.html:169: } else if
(response['current_result'][1][0]) {
Similar to above, response['current_result'][1][0] doesn't mean much to me.
Perhaps some comments about how succeeded/failed is determined.

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
File dashboard/dashboard/stoppage_alert_debugging_info.py (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
dashboard/dashboard/stoppage_alert_debugging_info.py:27: rev: Point id of the
last known revision; ignored if rev is specified.
ignored if key is specified

[shatch, 2017-01-10 22:24:21]

On 2017/01/10 22:19:18, jessimb wrote:
> I tried the debug button from the /alerts page and am a bit confused.
> 
> The first stoppage alert (from 2016-11-19) next revision's status is indicated
> as succeeded, however when I click on the buildbot status page, there is a
list
> of red failures. Perhaps this is user error? 

Yeah that threw me off initially, turns out the test is succeeding but
generating NaN's.

> 
> The second stoppage alert (from 2016-11-21) next revision's status is
exception,
> and when I click on the link I get a 404. The link is:
>
https://build.chromium.org/p/chromium.perf/builders/Linux%20Perf%20%282%29/bu...
> 
>
https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
> File dashboard/dashboard/elements/stoppage-alert-debug-button.html (right):
> 
>
https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
> dashboard/dashboard/elements/stoppage-alert-debug-button.html:167: if
> (response['current_result'][0] == '0') {
> What does response['current_result'][0] contain? Why is '0' a success?
> 
>
https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
> dashboard/dashboard/elements/stoppage-alert-debug-button.html:169: } else if
> (response['current_result'][1][0]) {
> Similar to above, response['current_result'][1][0] doesn't mean much to me.
> Perhaps some comments about how succeeded/failed is determined.
> 
>
https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
> File dashboard/dashboard/stoppage_alert_debugging_info.py (right):
> 
>
https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
> dashboard/dashboard/stoppage_alert_debugging_info.py:27: rev: Point id of the
> last known revision; ignored if rev is specified.
> ignored if key is specified

[sullivan, 2017-01-11 13:53:06]

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
File dashboard/dashboard/elements/chart-container.html (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/chart-container.html:1989: dataIndex ===
seriesFromJson.data.length - 1;
On 2017/01/10 22:07:36, shatch wrote:
> On 2017/01/10 18:26:27, sullivan wrote:
> > This tells the tooltip whether it is rendering the last point in the chart,
so
> > it can add a debug button for stoppages.
> 
> nit: group this with the other this.$.tooltip.foo = blah stuff below?

Done.

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
File dashboard/dashboard/elements/stoppage-alert-debug-button.html (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/stoppage-alert-debug-button.html:167: if
(response['current_result'][0] == '0') {
On 2017/01/10 22:19:18, jessimb wrote:
> What does response['current_result'][0] contain? Why is '0' a success?

Thanks, I added a long comment. 0 is a common exit code for success.

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/el...
dashboard/dashboard/elements/stoppage-alert-debug-button.html:169: } else if
(response['current_result'][1][0]) {
On 2017/01/10 22:19:18, jessimb wrote:
> Similar to above, response['current_result'][1][0] doesn't mean much to me.
> Perhaps some comments about how succeeded/failed is determined.

Same, factored this out to a function with a long comment.

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
File dashboard/dashboard/stoppage_alert_debugging_info.py (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
dashboard/dashboard/stoppage_alert_debugging_info.py:27: rev: Point id of the
last known revision; ignored if rev is specified.
On 2017/01/10 22:19:18, jessimb wrote:
> ignored if key is specified

Done.

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
File dashboard/dashboard/stoppage_alert_debugging_info_test.py (right):

https://codereview.chromium.org/2616153002/diff/220001/dashboard/dashboard/st...
dashboard/dashboard/stoppage_alert_debugging_info_test.py:68: print results
On 2017/01/10 22:07:36, shatch wrote:
> nit: debug stuff?

Whoops, thanks!

[sullivan, 2017-01-11 13:56:41]

Thanks for the thorough reviews, Simon and Jessi!

On 2017/01/10 22:19:18, jessimb wrote:
> I tried the debug button from the /alerts page and am a bit confused.

While I think this makes debugging data stoppage alerts much better, it's
unfortunately still pretty hard.

> The first stoppage alert (from 2016-11-19) next revision's status is indicated
> as succeeded, however when I click on the buildbot status page, there is a
list
> of red failures. Perhaps this is user error? 

As Simon said, the step for the test succeeded (and produced NaNs), but there
were unrelated failures.

> The second stoppage alert (from 2016-11-21) next revision's status is
exception,
> and when I click on the link I get a 404. The link is:
>
https://build.chromium.org/p/chromium.perf/builders/Linux%20Perf%20%282%29/bu...

Unfortunately, the bot name was changed when we switched to swarming, and when
that happens, almost all the old data is wiped away. So the status page for
pre-swarming bots is gone. The good news is that once the current alerts are
triaged, we shouldn't have more alerts from pre-swarming bots.

[sullivan, 2017-01-11 13:56:47]

The CQ bit was checked by sullivan@chromium.org

[sullivan, 2017-01-11 13:56:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
simonhatch@chromium.org
Link to the patchset: https://codereview.chromium.org/2616153002/#ps240001
(title: "Addressed review comments")

[commit-bot: I haz the power, 2017-01-11 13:56:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-11 14:19:41]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1484143007191100,
"parent_rev": "8b10e7828ad23b4aed04259a870ab11d2f323a23", "commit_rev":
"8295aca699a5a202abab86f88481ee9ef721fad5"}

[commit-bot: I haz the power, 2017-01-11 14:19:44]

Description was changed from

==========
Create debug dialog for data stoppage alerts.

This CL creates a dialog and XHR backend for debugging stoppage alerts and
wires it into the stoppage alert table and the tooltip for the last datapoint on
a
chart.

BUG=catapult:#3132
==========

to

==========
Create debug dialog for data stoppage alerts.

This CL creates a dialog and XHR backend for debugging stoppage alerts and
wires it into the stoppage alert table and the tooltip for the last datapoint on
a
chart.

BUG=catapult:#3132

Review-Url: https://codereview.chromium.org/2616153002
Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2017-01-11 14:19:45]

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