[perf][ui] Check v8 hashes to see if single commit
- Add Proxy Handler to fetch v8 requests.
- Only support v8 instance, ignores all others.
- Added overhead, but no other option.
- Set parent trace check
Change-Id: I6211a747bb10ede3519ff288347bfd0daae7ada5
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/1062685
Reviewed-by: Ashwin Verleker <ashwinpv@google.com>
Commit-Queue: Tony Seaward <seawardt@google.com>
diff --git a/perf/go/frontend/BUILD.bazel b/perf/go/frontend/BUILD.bazel
index 31337f5..b16b34d 100644
--- a/perf/go/frontend/BUILD.bazel
+++ b/perf/go/frontend/BUILD.bazel
@@ -3,7 +3,10 @@
go_library(
name = "frontend",
- srcs = ["frontend.go"],
+ srcs = [
+ "frontend.go",
+ "proxy.go",
+ ],
data = glob(["testdata/**"]),
embedsrcs = [
"googleanalytics.html",
diff --git a/perf/go/frontend/frontend.go b/perf/go/frontend/frontend.go
index 4a17fe2..f5ebbd6 100644
--- a/perf/go/frontend/frontend.go
+++ b/perf/go/frontend/frontend.go
@@ -1047,6 +1047,7 @@
router.Post("/_/fe_telemetry", f.feTelemetryHandler)
router.Get("/_/defaults", f.defaultsHandler)
router.Get("/_/revision", f.revisionHandler)
+ router.Get("/_/json/", Proxy_GetHandler)
return router
}
diff --git a/perf/go/frontend/proxy.go b/perf/go/frontend/proxy.go
new file mode 100644
index 0000000..2eeaf0b
--- /dev/null
+++ b/perf/go/frontend/proxy.go
@@ -0,0 +1,57 @@
+package frontend
+
+import (
+ "errors"
+ "io"
+ "net/http"
+
+ "go.skia.org/infra/go/httputils"
+ "go.skia.org/infra/go/sklog"
+)
+
+// Proxy_GetHandler proxies a GET request to the given url.
+//
+// Takes the URL to fetch in the "url" query parameter.
+//
+// It is intended to be used to work around CORS issues, where a browser can't
+// directly contact another server, e.g. googlesource.com.
+func Proxy_GetHandler(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "application/json")
+ url := r.FormValue("url")
+ if url == "" {
+ httputils.ReportError(w, errors.New("Missing 'url' query parameter."), "Missing 'url' query parameter.", http.StatusBadRequest)
+ return
+ }
+
+ // We don't use the httputils.NewTimeoutClient() because we don't want to
+ // follow redirects, we want to proxy them.
+ client := &http.Client{}
+
+ req, err := http.NewRequest("GET", url, nil)
+ if err != nil {
+ httputils.ReportError(w, err, "Failed to create request.", http.StatusInternalServerError)
+ return
+ }
+ // Copy headers from the original request.
+ req.Header = r.Header
+ req.Header.Del("Referer")
+ req.Header.Del("Origin")
+ req.Header.Del("Accept-Encoding") // To avoid getting gzipped content we can't easily handle.
+
+ resp, err := client.Do(req)
+ if err != nil {
+ httputils.ReportError(w, err, "Failed to make request.", http.StatusInternalServerError)
+ return
+ }
+ defer resp.Body.Close()
+
+ // Copy headers from the proxied response.
+ for k, v := range resp.Header {
+ w.Header()[k] = v
+ }
+ w.WriteHeader(resp.StatusCode)
+ if _, err := io.Copy(w, resp.Body); err != nil {
+ // We can't call ReportError here because we've already written the header.
+ sklog.Errorf("Failed to write proxied response: %s", err)
+ }
+}
diff --git a/perf/modules/explore-simple-sk/explore-simple-sk.ts b/perf/modules/explore-simple-sk/explore-simple-sk.ts
index f7b73eb..685f5f2 100644
--- a/perf/modules/explore-simple-sk/explore-simple-sk.ts
+++ b/perf/modules/explore-simple-sk/explore-simple-sk.ts
@@ -489,6 +489,29 @@
private fromParamsKey: string = '';
+ private buildTestPath(params: any) {
+ const parts = [];
+ if (params.test) {
+ parts.push(params.test);
+ }
+ // Only add subtests if they are not the same as story, up to 4.
+ if (params.subtest_1 && params.subtest_1 !== this.story) {
+ parts.push(params.subtest_1);
+ }
+ if (params.subtest_2 && params.subtest_2 !== this.story) {
+ parts.push(params.subtest_2);
+ }
+ if (params.subtest_3 && params.subtest_3 !== this.story) {
+ parts.push(params.subtest_3);
+ }
+ if (params.subtest_4 && params.subtest_4 !== this.story) {
+ parts.push(params.subtest_4);
+ }
+
+ parts.push(this.story);
+ this.testPath = parts.join('/');
+ }
+
private testPath: string = '';
private startCommit: string = '';
@@ -3638,6 +3661,9 @@
case 'show_google_plot':
this._state.show_google_plot = paramValue;
break;
+ case 'disable_filter_parent_traces':
+ this._state.disable_filter_parent_traces = paramValue;
+ break;
default:
break;
}
diff --git a/perf/modules/point-links-sk/point-links-sk.ts b/perf/modules/point-links-sk/point-links-sk.ts
index 55e0ca6..9a48707 100644
--- a/perf/modules/point-links-sk/point-links-sk.ts
+++ b/perf/modules/point-links-sk/point-links-sk.ts
@@ -66,28 +66,48 @@
private fuchsiaBuildLogKey = 'Test stdio';
+ // _pointLinks stores the asynchronously rendered TemplateResult[] for the point links.
+ // This is necessary because `lit-html` cannot directly render an array of Promises.
+ private _pointLinks: TemplateResult[] = [];
+
private static template = (ele: PointLinksSk) =>
html`<div class="point-links" ?hidden=${Object.keys(ele.displayUrls || {}).length === 0}>
<ul class="table">
- ${ele.renderPointLinks()} ${ele.renderRevisionLink()}
+ ${ele._pointLinks} ${ele.renderRevisionLink()}
</ul>
</div>`;
connectedCallback(): void {
super.connectedCallback();
+ // Initial render is handled by the `load` and `reset` functions,
+ // which now call `renderPointLinks` to manage asynchronous updates.
this.render();
}
- renderPointLinks(): TemplateResult[] {
+ /**
+ * Asynchronously renders the point links and updates the `_pointLinks` property.
+ * This function is now `async` to `await` the `isRange` check and uses `Promise.all`
+ * to ensure all individual link rendering promises resolve before updating the DOM.
+ */
+ async renderPointLinks(): Promise<void> {
if (Object.keys(this.displayTexts).length === 0 && Object.keys(this.displayUrls).length === 0) {
- return [];
+ this._pointLinks = [];
+ this.render();
+ return;
}
const keys = Object.keys(this.displayUrls);
- const getHtml = (key: string): TemplateResult => {
+ const getHtml = async (key: string): Promise<TemplateResult> => {
const link = this.displayUrls![key];
// TODO(b/398878559): Strip after 'Git' string until json keys are ready.
const keyText: string = key.split(' Git')[0];
- const linkText = this.displayTexts![key] || 'Link';
+ let linkText = this.displayTexts![key] || 'Link';
+ // This is a specific change for just v8.
+ if (keyText === 'V8') {
+ const isRange = await this.isRange(link);
+ if (!isRange && linkText.includes(' - ')) {
+ linkText = linkText.split(' - ')[1];
+ }
+ }
const htmlUrl = html`<a
href="${link}"
title="${linkText}"
@@ -104,7 +124,9 @@
</md-icon-button>
</li>`;
};
- return keys.map(getHtml);
+ const htmlPromises = keys.map(getHtml);
+ this._pointLinks = await Promise.all(htmlPromises);
+ this.render();
}
renderRevisionLink() {
@@ -133,6 +155,65 @@
navigator.clipboard.writeText(text);
}
+ // isRange returns true if the given link refers to a range of commits, i.e.
+ // more than one commit.
+ //
+ // The ingested data always contains 2 commit hashes, but it is possible that
+ // there is only a single commit between the 2 hashes. We request a JSON
+ // formatted response from googlesource and check the response to see how many
+ // commits exist in the log.
+ private async isRange(link: string): Promise<boolean> {
+ // We can only inspect googlesource links that point to a commit.
+ if (!link.includes('googlesource.com')) {
+ // Not a googlesource commit link we can inspect, assume it's not a single
+ // commit, so we treat it as a range to be safe.
+ return true;
+ }
+
+ const url = new URL(link);
+ url.searchParams.set('format', 'JSON');
+
+ const proxyUrl = `/_/json//?url=${encodeURIComponent(url.toString())}`;
+
+ try {
+ const resp = await fetch(proxyUrl);
+ if (!resp.ok) {
+ // If the proxy fails or the underlying request fails, we can't determine.
+ // Let's log the error and assume it's a range.
+ const text = await resp.text();
+ console.error(
+ `Failed to fetch through proxy for ${url.toString()}: ${resp.status} ${text}`
+ );
+ return true;
+ }
+ const text = await resp.text();
+
+ // Handle googlesource's JSON prefix: )]}'
+ if (!text.startsWith(")]}'")) {
+ // Not the JSON format we expect.
+ return true;
+ }
+ const jsonStr = text.substring(4);
+ // It's possible to get an empty response body.
+ if (!jsonStr.trim()) {
+ return true;
+ }
+ const json = JSON.parse(jsonStr);
+
+ // A range is defined as having more than one commit in the log.
+ if (json && Array.isArray(json.log)) {
+ return json.log.length > 1;
+ }
+ // If we don't have a log array, we can't determine, so assume it's a range.
+ return true;
+ } catch (error) {
+ errorMessage(`Error while determining if link is a range: ${error}`);
+ // On any other error (e.g. network, JSON parsing), we can't be sure.
+ // Returning true is safer as it will display the full link text.
+ return true;
+ }
+ }
+
// load and display the links for the given commit and trace.
public async load(
commit_position: CommitNumber | null,
@@ -166,7 +247,7 @@
// Reuse the existing links
this.displayUrls = existingLink.displayUrls || {};
this.displayTexts = existingLink.displayTexts || {};
- this.render();
+ this.renderPointLinks();
return Promise.resolve(commitLinks);
}
}
@@ -247,6 +328,7 @@
this.displayTexts = displayTexts;
this.displayUrls = displayUrls;
+ this.renderPointLinks();
// Before adding a new commit link, check if it already exists in the array.
// This should not be necessary, but it is a safeguard due to async calls.
@@ -274,7 +356,7 @@
this.commitPosition = null;
this.displayUrls = {};
this.displayTexts = {};
- this.render();
+ this.renderPointLinks();
}
render(): void {
