First version of benchmark health report.

dashboard/dashboard/benchmark_health_report.py

Index: dashboard/dashboard/benchmark_health_report.py
diff --git a/dashboard/dashboard/benchmark_health_report.py b/dashboard/dashboard/benchmark_health_report.py
new file mode 100644
index 0000000000000000000000000000000000000000..43089940b9d4a0c1e052aff6ddc142b10a965279
--- /dev/null
+++ b/dashboard/dashboard/benchmark_health_report.py
@@ -0,0 +1,89 @@
+# Copyright 2017 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Provides the web interface for a report on benchmark alert health."""
+
+import datetime
+import json
+
+from dashboard import alerts
+from dashboard import list_tests
+from dashboard import update_test_suites
+from dashboard.common import request_handler
+from dashboard.common import utils
+from dashboard.models import anomaly
+
+
+class BenchmarkHealthReportHandler(request_handler.RequestHandler):
+
+  def get(self):
+    """Renders the UI for the group report page."""
+    self.RenderStaticHtml('benchmark_health_report.html')
+
+  def post(self):
+    """Returns data about the alerts for a benchmark.
+
+    Request parameters:
+      master: If specified, the master to list benchmarks for. If no arguments
+              are specified, benchmarks will be listed for chromium.perf.
+      benchmark: If specified, the name of the benchmark to gather details for.
+      num_days: With benchmark, number of days to get data for.

[eakuefner, 2017/02/17 19:11:05]

It looks like this is true based on the code below, but does this mean number of
days from now backwards?

+      bug: If specified, the id of the bug to gather details for.
+
+    Outputs:
+      JSON for the /group_report page XHR request.
+    """
+    master = self.request.get('master')
+    benchmark = self.request.get('benchmark')
+    response_values = {}
+
+    if benchmark:
+      num_days = int(self.request.get('num_days'))
+      response_values = self._GetResponseValuesForBenchmark(
+          benchmark, num_days, master)
+    else:
+      response_values = self._GetResponseValuesForMaster(master)
+
+    self.response.out.write(json.dumps(response_values))
+
+  def _GetResponseValuesForBenchmark(self, benchmark, num_days, master):
+    values = {}
+
+    # The cached test suite info contains info about monitoring and bots.
+    benchmarks = update_test_suites.FetchCachedTestSuites()
+    sheriff = self._GetSheriffForBenchmark(benchmark, master, benchmarks)
+    if sheriff:
+      query = anomaly.Anomaly.query(anomaly.Anomaly.sheriff == sheriff)
+      query = query.filter(anomaly.Anomaly.is_improvement == False)

[eakuefner, 2017/02/17 19:11:06]

Jessi is trying to do similar stuff in speed releasing (querying for Anomaly
entities by what rotation/what benchmark/etc.). It seems like this being
performant depends on the many indices we have for Anomalies -- is that status
quo where we want to be long term?

[sullivan, 2017/02/17 20:01:02]

A couple of things wrapped up in here:
* We want to actually only show Anomalies for a given master. Specifically there
is tons of ChromeOS data we'd want to exclude for our analyses, and also v8 data
with the same benchmark name but run in a totally different context (outside the
browser).
* We do want to show multiple sheriffs, for things like
system_health.common_desktop or v8 tests that do have them.

Indexes are complicated in data store! In this case, they would be a clear win
because:
* Unlike Row, Anomaly/Alert has few enough entities that there wouldn't be a
huge stored data cost (adding an index to Row is multiple terabytes of
additional stored data)
* The largest cost of a query is the number of entities returned. It's true that
each additional index adds overhead, but Row and Test are the biggest places
where that matters because we generally want to be able to query more faster.
For Anomaly, if we could just grab the data for a benchmark it'd be faster
overall to use an index because fewer entities would be returned.

The problem with using an index here is simply that the Anomaly does not store
the master and benchmark, which are what we want to query on. i was not planning
on adding this, but now that you mention it I think it would speed up this
particular query quite a bit, and also remove the complexity around querying by
sheriffing and filtering. I'll do this in a follow up:
https://github.com/catapult-project/catapult/issues/3238

What type of query is Jessi hitting that does not have indexes?

[eakuefner, 2017/02/17 20:13:48]

There's a doc with some notes here
https://docs.google.com/document/d/1p5Yn3cE5sjnwR18lgH3nAj3KLEdVpirLuXLqkK2nm...
(internal, but could be Chromium in principle; there's nothing sensitive)

Anyway, SGTM to do what you proposed in a followup.

+      query = query.filter(
+          anomaly.Anomaly.timestamp >
+          datetime.datetime.now() - datetime.timedelta(days=num_days))
+      query = query.order(-anomaly.Anomaly.timestamp)
+      anomalies = query.fetch()
+      anomalies = [a for a in anomalies if self._BenchmarkName(a) == benchmark]
+      values['monitored'] = True
+      values['alerts'] = alerts.AnomalyDicts(anomalies)
+    else:
+      values['monitored'] = False
+
+    values['bots'] = benchmarks[benchmark]['mas'][master].keys()
+    return values
+
+  def _GetSheriffForBenchmark(self, benchmark, master, benchmarks):
+    # TODO(sullivan): There can be multiple sheriffs; implement this.
+    if not benchmarks[benchmark]['mon']:
+      return None
+    monitored_test_path = benchmarks[benchmark]['mon'][0]
+    pattern = '%s/*/%s/%s' % (master, benchmark, monitored_test_path)
+    monitored_tests = list_tests.GetTestsMatchingPattern(
+        pattern, list_entities=True)
+    return monitored_tests[0].sheriff
+
+  def _BenchmarkName(self, alert):
+    return utils.TestPath(alert.test).split('/')[2]
+
+  def _GetResponseValuesForMaster(self, master):
+    benchmarks = update_test_suites.FetchCachedTestSuites()
+    benchmarks = [b for b in benchmarks if master in benchmarks[b]['mas']]
+    return {'benchmarks': sorted(benchmarks)}