[perf] Enable filtering on flagged clusters.
Should make triaging easier.
Screenshots:
https://screenshot.googleplex.com/eabbPZ85qOZ.png
https://screenshot.googleplex.com/XojuFyZfop7.png
https://screenshot.googleplex.com/hcjhnTcisga.png
Bug: skia:
Change-Id: I878896a31e2d66e2b9fd5d51c0c7a1a9483e29bd
Reviewed-on: https://skia-review.googlesource.com/20145
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
diff --git a/perf/go/cid/cid.go b/perf/go/cid/cid.go
index 7cc031a..0d67a92 100644
--- a/perf/go/cid/cid.go
+++ b/perf/go/cid/cid.go
@@ -46,6 +46,25 @@
return fmt.Sprintf("%s-%06d", safeRe.ReplaceAllLiteralString(c.Source, "_"), c.Offset)
}
+// FromID is the inverse operator to ID().
+func FromID(s string) (*CommitID, error) {
+ parts := strings.Split(s, "-")
+ if len(parts) != 2 {
+ return nil, fmt.Errorf("Invalid ID format: %s", s)
+ }
+ if strings.Contains(parts[0], "_") {
+ return nil, fmt.Errorf("Invalid ID format: %s", s)
+ }
+ i, err := strconv.ParseInt(parts[1], 10, 64)
+ if err != nil {
+ return nil, fmt.Errorf("Invalid ID format: %s", s)
+ }
+ return &CommitID{
+ Offset: int(i),
+ Source: parts[0],
+ }, nil
+}
+
// CommitDetail describes a CommitID.
type CommitDetail struct {
CommitID
diff --git a/perf/go/cid/cid_test.go b/perf/go/cid/cid_test.go
index 315b97a..32683cc 100644
--- a/perf/go/cid/cid_test.go
+++ b/perf/go/cid/cid_test.go
@@ -195,3 +195,54 @@
assert.Contains(t, err.Error(), "Can't parse timestamp")
assert.Equal(t, 4, index)
}
+
+func TestFromID(t *testing.T) {
+ testutils.SmallTest(t)
+ testCases := []struct {
+ value string
+ expected *CommitID
+ err bool
+ message string
+ }{
+ {
+ value: "master-000051",
+ expected: &CommitID{
+ Offset: 51,
+ Source: "master",
+ },
+ err: false,
+ message: "Simple",
+ },
+ {
+ value: "some_trybot-000051",
+ expected: nil,
+ err: true,
+ message: "TryBot should fail",
+ },
+ {
+ value: "master-notanint",
+ expected: nil,
+ err: true,
+ message: "Fail parse int",
+ },
+ {
+ value: "invalid",
+ expected: nil,
+ err: true,
+ message: "no dashes",
+ },
+ {
+ value: "in-val-id",
+ expected: nil,
+ err: true,
+ message: "too many dashes",
+ },
+ }
+
+ for _, tc := range testCases {
+ got, err := FromID(tc.value)
+ assert.Equal(t, tc.err, err != nil, tc.message)
+ assert.Equal(t, tc.expected, got, tc.message)
+ }
+
+}
diff --git a/perf/go/regression/db.go b/perf/go/regression/db.go
index bc18224..15fac6c 100644
--- a/perf/go/regression/db.go
+++ b/perf/go/regression/db.go
@@ -12,6 +12,15 @@
"go.skia.org/infra/perf/go/db"
)
+// Subset is the subset of regressions we are querying for.
+type Subset string
+
+const (
+ ALL_SUBSET Subset = "all" // Include all regressions in a range.
+ FLAGGED_SUBSET Subset = "flagged" // Only include regressions in a range that are alerting.
+ UNTRIAGED_SUBSET Subset = "untriaged" // All untriaged alerting regressions regardless of range.
+)
+
// Store persists Regressions to/from an SQL database.
type Store struct {
}
@@ -74,9 +83,13 @@
}
// Range returns a map from cid.ID()'s to *Regressions that exist in the given time range.
-func (s *Store) Range(begin, end int64) (map[string]*Regressions, error) {
+func (s *Store) Range(begin, end int64, subset Subset) (map[string]*Regressions, error) {
ret := map[string]*Regressions{}
+
rows, err := db.DB.Query("SELECT cid, timestamp, body FROM regression WHERE timestamp >= ? AND timestamp < ? ORDER BY timestamp", begin, end)
+ if subset == UNTRIAGED_SUBSET {
+ rows, err = db.DB.Query("SELECT cid, timestamp, body FROM regression WHERE triaged=false ORDER BY timestamp")
+ }
if err != nil {
return nil, fmt.Errorf("Failed to query from database: %s", err)
}
diff --git a/perf/go/regression/db_test.go b/perf/go/regression/db_test.go
index d2543f1..02fc662 100644
--- a/perf/go/regression/db_test.go
+++ b/perf/go/regression/db_test.go
@@ -205,7 +205,7 @@
// Execute our method.
st := NewStore()
- reg, err := st.Range(1479235651, 1479235999)
+ reg, err := st.Range(1479235651, 1479235999, ALL_SUBSET)
assert.NoError(t, err)
assert.Equal(t, 2, len(reg))
assert.True(t, reg[c1.ID()].Triaged())
diff --git a/perf/go/skiaperf/main.go b/perf/go/skiaperf/main.go
index 7bde76a..5ab75ea 100644
--- a/perf/go/skiaperf/main.go
+++ b/perf/go/skiaperf/main.go
@@ -718,8 +718,9 @@
//
// Begin and End are Unix timestamps in seconds.
type RegressionRangeRequest struct {
- Begin int64 `json:"begin"`
- End int64 `json:"end"`
+ Begin int64 `json:"begin"`
+ End int64 `json:"end"`
+ Subset regression.Subset `json:"subset"`
}
// RegressionRow are all the Regression's for a specific commit. It is used in
@@ -758,25 +759,8 @@
return
}
- // Get a list of commits for the range.
- indexCommits := git.Range(time.Unix(rr.Begin, 0), time.Unix(rr.End, 0))
- ids := make([]*cid.CommitID, 0, len(indexCommits))
- for _, indexCommit := range indexCommits {
- ids = append(ids, &cid.CommitID{
- Source: "master",
- Offset: indexCommit.Index,
- })
- }
-
- // Convert the CommitIDs to CommitDetails.
- cids, err := cidl.Lookup(ids)
- if err != nil {
- httputils.ReportError(w, r, err, "Failed to look up commit details")
- return
- }
-
// Query for Regressions in the range.
- regMap, err := regStore.Range(rr.Begin, rr.End)
+ regMap, err := regStore.Range(rr.Begin, rr.End, rr.Subset)
if err != nil {
httputils.ReportError(w, r, err, "Failed to retrieve clusters.")
return
@@ -793,6 +777,43 @@
headers = util.NewStringSet(headers).Keys()
sort.Sort(sort.StringSlice(headers))
+ // Get a list of commits for the range.
+ var ids []*cid.CommitID
+ if rr.Subset == regression.ALL_SUBSET {
+ indexCommits := git.Range(time.Unix(rr.Begin, 0), time.Unix(rr.End, 0))
+ ids = make([]*cid.CommitID, 0, len(indexCommits))
+ for _, indexCommit := range indexCommits {
+ ids = append(ids, &cid.CommitID{
+ Source: "master",
+ Offset: indexCommit.Index,
+ })
+ }
+ } else {
+ // If rr.Subset == UNTRIAGED_QS or FLAGGED_QS then only get the commits that
+ // exactly line up with the regressions in regMap.
+ ids = make([]*cid.CommitID, 0, len(regMap))
+ keys := []string{}
+ for k, _ := range regMap {
+ keys = append(keys, k)
+ }
+ sort.Sort(sort.StringSlice(keys))
+ for _, key := range keys {
+ c, err := cid.FromID(key)
+ if err != nil {
+ httputils.ReportError(w, r, err, "Got an invalid commit id.")
+ return
+ }
+ ids = append(ids, c)
+ }
+ }
+
+ // Convert the CommitIDs to CommitDetails.
+ cids, err := cidl.Lookup(ids)
+ if err != nil {
+ httputils.ReportError(w, r, err, "Failed to look up commit details")
+ return
+ }
+
// Reverse the order of the cids, so the latest
// commit shows up first in the UI display.
revCids := make([]*cid.CommitDetail, len(cids), len(cids))
diff --git a/perf/res/imp/triage-page.html b/perf/res/imp/triage-page.html
index 92abb6f..73a3b15 100644
--- a/perf/res/imp/triage-page.html
+++ b/perf/res/imp/triage-page.html
@@ -14,6 +14,7 @@
-->
<link rel="import" href="/res/imp/bower_components/iron-flex-layout/iron-flex-layout-classes.html">
<link rel="import" href="/res/imp/bower_components/paper-spinner/paper-spinner.html">
+<link rel="import" href="/res/imp/bower_components/iron-selector/iron-selector.html">
<link rel="import" href="/res/common/imp/query-summary-sk.html" />
<link rel="import" href="/res/common/imp/details-summary.html" />
@@ -60,11 +61,40 @@
text-decoration: none;
color: black;
}
+
+ .iron-selected {
+ background: #eee;
+ }
+
+ iron-selector div {
+ width: 10em;
+ margin: 0.3em 1em;
+ padding: 0.2em;
+ }
+
+ summary-sk h2 {
+ display: inline;
+ }
</style>
<template>
+ <details-sk open>
+ <summary-sk>
+ <h2 id="filter">Filter</h2>
+ </summary-sk>
+ <iron-selector
+ attr-for-selected="value"
+ selected="{{state.subset}}"
+ fallback-selection="untriaged"
+ on-selected-item-changed="_updateRange"
+ >
+ <div value=all title="Show results for all commits in the time range.">All</div>
+ <div value=flagged title="Show only the commits with alerts in the given time range.">Flagged</div>
+ <div value=untriaged title="Show all the untriaged alerts regardless of the time range.">Untriaged</div>
+ </iron-selector>
+ </details-sk>
<details-sk>
<summary-sk>
- Range
+ <h2 id="range">Range</h2>
</summary-sk>
<day-range-sk id=range on-day-range-change="_rangeChange"></day-range-sk>
</details-sk>
@@ -122,10 +152,13 @@
properties: {
state: {
type: Object,
- value: function() { return {
- begin: now - 4*24*60*60,
- end: now,
- }; },
+ value: function() {
+ return {
+ begin: now - 4*24*60*60,
+ end: now,
+ subset: "untriaged",
+ };
+ },
},
_reg: {
type: Object,
@@ -163,11 +196,13 @@
},
_updateRange: function() {
+ this.set('_reg', {});
this.$.range.begin = this.state.begin;
this.$.range.end = this.state.end;
var body = {
begin: this.state.begin,
end: this.state.end,
+ subset: this.state.subset,
};
this.$.spinner.active = true;
this.$.table.classList.toggle("hidden", true);
@@ -230,7 +265,7 @@
},
_display: function(index) {
- if (this.state.end == now && index < sk.perf.radius) {
+ if (this.state.end == now && index < sk.perf.radius && this.state.subset == "all") {
return "";
} else {
return "∅";
