perf: all fetch of specific or all benchmarks
The ?benchmark query param specifies the benchmarks to fetch. "all"
fetches all benchmarks. Omitting it returns the default set.
For golang/go#48803.
Change-Id: I221ac3a702eb0cbb263ef86b2723e5d951662cef
Reviewed-on: https://go-review.googlesource.com/c/build/+/412998
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/perf/app/dashboard.go b/perf/app/dashboard.go
index c917a44..fae9c43 100644
--- a/perf/app/dashboard.go
+++ b/perf/app/dashboard.go
@@ -8,13 +8,16 @@
"context"
"embed"
"encoding/json"
+ "errors"
"fmt"
"log"
"net/http"
+ "regexp"
"sort"
"time"
"github.com/influxdata/influxdb-client-go/v2/api"
+ "github.com/influxdata/influxdb-client-go/v2/api/query"
"golang.org/x/build/internal/influx"
"golang.org/x/build/third_party/bandchart"
)
@@ -55,19 +58,59 @@
High float64
}
-// fetch queries Influx to fill Values. Name and Unit must be set.
-//
-// WARNING: Name and Unit are not sanitized. DO NOT pass user input.
-func (b *BenchmarkJSON) fetch(ctx context.Context, qc api.QueryAPI) error {
- if b.Name == "" {
- return fmt.Errorf("Name must be set")
- }
- if b.Unit == "" {
- return fmt.Errorf("Unit must be set")
+func fluxRecordToValue(rec *query.FluxRecord) (ValueJSON, error) {
+ low, ok := rec.ValueByKey("low").(float64)
+ if !ok {
+ return ValueJSON{}, fmt.Errorf("record %s low value got type %T want float64", rec, rec.ValueByKey("low"))
}
- // TODO(prattmic): Adjust UI to comfortably display more than 7d of
- // data.
+ center, ok := rec.ValueByKey("center").(float64)
+ if !ok {
+ return ValueJSON{}, fmt.Errorf("record %s center value got type %T want float64", rec, rec.ValueByKey("center"))
+ }
+
+ high, ok := rec.ValueByKey("high").(float64)
+ if !ok {
+ return ValueJSON{}, fmt.Errorf("record %s high value got type %T want float64", rec, rec.ValueByKey("high"))
+ }
+
+ commit, ok := rec.ValueByKey("experiment-commit").(string)
+ if !ok {
+ return ValueJSON{}, fmt.Errorf("record %s experiment-commit value got type %T want float64", rec, rec.ValueByKey("experiment-commit"))
+ }
+
+ return ValueJSON{
+ CommitDate: rec.Time(),
+ CommitHash: commit,
+ Low: low - 1,
+ Center: center - 1,
+ High: high - 1,
+ }, nil
+}
+
+// validateRe is an allowlist of characters for a Flux string literal for
+// benchmark names. The string will be quoted, so we must not allow ending the
+// quote sequence.
+var validateRe = regexp.MustCompile(`[a-zA-Z/_:-]+`)
+
+func validateFluxString(s string) error {
+ if !validateRe.MatchString(s) {
+ return fmt.Errorf("malformed value %q", s)
+ }
+ return nil
+}
+
+var errBenchmarkNotFound = errors.New("benchmark not found")
+
+// fetchNamedUnitBenchmark queries Influx for a specific name + unit benchmark.
+func fetchNamedUnitBenchmark(ctx context.Context, qc api.QueryAPI, name, unit string) (*BenchmarkJSON, error) {
+ if err := validateFluxString(name); err != nil {
+ return nil, fmt.Errorf("invalid benchmark name: %w", err)
+ }
+ if err := validateFluxString(unit); err != nil {
+ return nil, fmt.Errorf("invalid benchmark name: %w", err)
+ }
+
query := fmt.Sprintf(`
from(bucket: "perf")
|> range(start: -30d)
@@ -79,50 +122,183 @@
|> filter(fn: (r) => r["goarch"] == "amd64")
|> pivot(columnKey: ["_field"], rowKey: ["_time"], valueColumn: "_value")
|> yield(name: "last")
-`, b.Name, b.Unit)
+`, name, unit)
- ir, err := qc.Query(ctx, query)
+ res, err := qc.Query(ctx, query)
if err != nil {
- return fmt.Errorf("error performing query: %W", err)
+ return nil, fmt.Errorf("error performing query: %W", err)
}
- for ir.Next() {
- rec := ir.Record()
+ b, err := groupBenchmarkResults(res)
+ if err != nil {
+ return nil, err
+ }
+ if len(b) == 0 {
+ return nil, errBenchmarkNotFound
+ }
+ if len(b) > 1 {
+ return nil, fmt.Errorf("query returned too many benchmarks: %+v", b)
+ }
+ return b[0], nil
+}
- low, ok := rec.ValueByKey("low").(float64)
+// fetchDefaultBenchmarks queries Influx for the default benchmark set.
+func fetchDefaultBenchmarks(ctx context.Context, qc api.QueryAPI) ([]*BenchmarkJSON, error) {
+ // Keep benchmarks with the same name grouped together, which is
+ // assumed by the JS.
+ benchmarks := []struct{ name, unit string }{
+ {
+ name: "Tile38WithinCircle100kmRequest",
+ unit: "sec/op",
+ },
+ {
+ name: "Tile38WithinCircle100kmRequest",
+ unit: "p90-latency-sec",
+ },
+ {
+ name: "Tile38WithinCircle100kmRequest",
+ unit: "average-RSS-bytes",
+ },
+ {
+ name: "Tile38WithinCircle100kmRequest",
+ unit: "peak-RSS-bytes",
+ },
+ {
+ name: "GoBuildKubelet",
+ unit: "sec/op",
+ },
+ {
+ name: "GoBuildKubeletLink",
+ unit: "sec/op",
+ },
+ }
+
+ ret := make([]*BenchmarkJSON, 0, len(benchmarks))
+ for _, bench := range benchmarks {
+ b, err := fetchNamedUnitBenchmark(ctx, qc, bench.name, bench.unit)
+ if err != nil {
+ return nil, fmt.Errorf("error fetching benchmark %s/%s: %w", bench.name, bench.unit, err)
+ }
+ ret = append(ret, b)
+ }
+
+ return ret, nil
+}
+
+// fetchNamedBenchmark queries Influx for all benchmark results with the passed
+// name (for all units).
+func fetchNamedBenchmark(ctx context.Context, qc api.QueryAPI, name string) ([]*BenchmarkJSON, error) {
+ if err := validateFluxString(name); err != nil {
+ return nil, fmt.Errorf("invalid benchmark name: %w", err)
+ }
+
+ query := fmt.Sprintf(`
+from(bucket: "perf")
+ |> range(start: -30d)
+ |> filter(fn: (r) => r["_measurement"] == "benchmark-result")
+ |> filter(fn: (r) => r["name"] == "%s")
+ |> filter(fn: (r) => r["branch"] == "master")
+ |> filter(fn: (r) => r["goos"] == "linux")
+ |> filter(fn: (r) => r["goarch"] == "amd64")
+ |> pivot(columnKey: ["_field"], rowKey: ["_time"], valueColumn: "_value")
+ |> yield(name: "last")
+`, name)
+
+ res, err := qc.Query(ctx, query)
+ if err != nil {
+ return nil, fmt.Errorf("error performing query: %W", err)
+ }
+
+ b, err := groupBenchmarkResults(res)
+ if err != nil {
+ return nil, err
+ }
+ if len(b) == 0 {
+ return nil, errBenchmarkNotFound
+ }
+ return b, nil
+}
+
+// fetchAllBenchmarks queries Influx for all benchmark results.
+func fetchAllBenchmarks(ctx context.Context, qc api.QueryAPI) ([]*BenchmarkJSON, error) {
+ const query = `
+from(bucket: "perf")
+ |> range(start: -30d)
+ |> filter(fn: (r) => r["_measurement"] == "benchmark-result")
+ |> filter(fn: (r) => r["branch"] == "master")
+ |> filter(fn: (r) => r["goos"] == "linux")
+ |> filter(fn: (r) => r["goarch"] == "amd64")
+ |> pivot(columnKey: ["_field"], rowKey: ["_time"], valueColumn: "_value")
+ |> yield(name: "last")
+`
+
+ res, err := qc.Query(ctx, query)
+ if err != nil {
+ return nil, fmt.Errorf("error performing query: %W", err)
+ }
+
+ return groupBenchmarkResults(res)
+}
+
+// groupBenchmarkResults groups all benchmark results from the passed query.
+func groupBenchmarkResults(res *api.QueryTableResult) ([]*BenchmarkJSON, error) {
+ type key struct {
+ name string
+ unit string
+ }
+ m := make(map[key]*BenchmarkJSON)
+
+ for res.Next() {
+ rec := res.Record()
+
+ name, ok := rec.ValueByKey("name").(string)
if !ok {
- return fmt.Errorf("record %s low value got type %T want float64", rec, rec.ValueByKey("low"))
+ return nil, fmt.Errorf("record %s name value got type %T want string", rec, rec.ValueByKey("name"))
}
- center, ok := rec.ValueByKey("center").(float64)
+ unit, ok := rec.ValueByKey("unit").(string)
if !ok {
- return fmt.Errorf("record %s center value got type %T want float64", rec, rec.ValueByKey("center"))
+ return nil, fmt.Errorf("record %s unit value got type %T want string", rec, rec.ValueByKey("unit"))
}
- high, ok := rec.ValueByKey("high").(float64)
+ k := key{name, unit}
+ b, ok := m[k]
if !ok {
- return fmt.Errorf("record %s high value got type %T want float64", rec, rec.ValueByKey("high"))
+ b = &BenchmarkJSON{
+ Name: name,
+ Unit: unit,
+ }
+ m[k] = b
}
- commit, ok := rec.ValueByKey("experiment-commit").(string)
- if !ok {
- return fmt.Errorf("record %s experiment-commit value got type %T want float64", rec, rec.ValueByKey("experiment-commit"))
+ v, err := fluxRecordToValue(res.Record())
+ if err != nil {
+ return nil, err
}
- b.Values = append(b.Values, ValueJSON{
- CommitDate: rec.Time(),
- CommitHash: commit,
- Low: low - 1,
- Center: center - 1,
- High: high - 1,
+ b.Values = append(b.Values, v)
+ }
+
+ s := make([]*BenchmarkJSON, 0, len(m))
+ for _, b := range m {
+ s = append(s, b)
+ }
+ // Keep benchmarks with the same name grouped together, which is
+ // assumed by the JS.
+ sort.Slice(s, func(i, j int) bool {
+ if s[i].Name == s[j].Name {
+ return s[i].Unit < s[j].Unit
+ }
+ return s[i].Name < s[j].Name
+ })
+
+ for _, b := range s {
+ sort.Slice(b.Values, func(i, j int) bool {
+ return b.Values[i].CommitDate.Before(b.Values[j].CommitDate)
})
}
- sort.Slice(b.Values, func(i, j int) bool {
- return b.Values[i].CommitDate.Before(b.Values[j].CommitDate)
- })
-
- return nil
+ return s, nil
}
// search handles /dashboard/data.json.
@@ -147,46 +323,24 @@
qc := ifxc.QueryAPI(influx.Org)
- // Keep benchmarks with the same name grouped together, which is
- // assumed by the JS.
- //
- // WARNING: Name and Unit are not sanitized. DO NOT pass user input.
- benchmarks := []BenchmarkJSON{
- {
- Name: "Tile38WithinCircle100kmRequest",
- Unit: "sec/op",
- },
- {
- Name: "Tile38WithinCircle100kmRequest",
- Unit: "p90-latency-sec",
- },
- {
- Name: "Tile38WithinCircle100kmRequest",
- Unit: "average-RSS-bytes",
- },
- {
- Name: "Tile38WithinCircle100kmRequest",
- Unit: "peak-RSS-bytes",
- },
- {
- Name: "GoBuildKubelet",
- Unit: "sec/op",
- },
- {
- Name: "GoBuildKubeletLink",
- Unit: "sec/op",
- },
+ benchmark := r.FormValue("benchmark")
+ var benchmarks []*BenchmarkJSON
+ if benchmark == "" {
+ benchmarks, err = fetchDefaultBenchmarks(ctx, qc)
+ } else if benchmark == "all" {
+ benchmarks, err = fetchAllBenchmarks(ctx, qc)
+ } else {
+ benchmarks, err = fetchNamedBenchmark(ctx, qc, benchmark)
}
-
- for i := range benchmarks {
- b := &benchmarks[i]
- // WARNING: Name and Unit are not sanitized. DO NOT pass user
- // input.
- if err := b.fetch(ctx, qc); err != nil {
- log.Printf("Error fetching benchmark %s/%s: %v", b.Name, b.Unit, err)
- http.Error(w, "Error fetching benchmark", 500)
- return
- }
+ if err == errBenchmarkNotFound {
+ log.Printf("Benchmark not found: %q", benchmark)
+ http.Error(w, "Benchmark not found", 404)
+ return
+ }
+ if err != nil {
+ log.Printf("Error fetching benchmarks: %v", err)
+ http.Error(w, "Error fetching benchmarks", 500)
+ return
}
w.Header().Set("Content-Type", "application/json")
diff --git a/perf/app/dashboard/index.html b/perf/app/dashboard/index.html
index bda57b7..d83e081 100644
--- a/perf/app/dashboard/index.html
+++ b/perf/app/dashboard/index.html
@@ -41,26 +41,9 @@
<div id="dashboard"></div>
<script>
-function addContent(name, benchmarks) {
+function addContent(benchmarks) {
let dashboard = document.getElementById("dashboard");
- if (name == "" || name == null || name == undefined) {
- // All benchmarks.
- // TODO(prattmic): Replace with a simpler overview?
- } else {
- // Filter to specified benchmark.
- benchmarks = benchmarks.filter(function(b) {
- return b.Name == name;
- });
- if (benchmarks.length == 0) {
- let title = document.createElement("h2");
- title.classList.add("Dashboard-title");
- title.innerHTML = "Benchmark \"" + name + "\" not found.";
- dashboard.appendChild(title);
- return;
- }
- }
-
let prevName = "";
let grid = null;
for (const b in benchmarks) {
@@ -88,9 +71,27 @@
}
}
+function failure(name) {
+ let dashboard = document.getElementById("dashboard");
+
+ let title = document.createElement("h2");
+ title.classList.add("Dashboard-title");
+ title.innerHTML = "Benchmark \"" + name + "\" not found.";
+ dashboard.appendChild(title);
+}
+
let benchmark = (new URLSearchParams(window.location.search)).get('benchmark');
-fetch('./data.json')
- .then(response => response.json())
+let dataURL = './data.json';
+if (benchmark) {
+ dataURL += '?benchmark=' + benchmark;
+}
+fetch(dataURL)
+ .then(response => {
+ if (!response.ok) {
+ throw new Error("Data fetch failed");
+ }
+ return response.json();
+ })
.then(function(benchmarks) {
// Convert CommitDate to a proper date.
benchmarks.forEach(function(b) {
@@ -99,8 +100,11 @@
});
});
- addContent(benchmark, benchmarks);
- });
+ addContent(benchmarks);
+ })
+ // This will look odd if the error isn't a 404 and benchmark is
+ // undefined, but close enough.
+ .catch(error => failure(benchmark));
</script>
</body>
