[perf] connect request debug trace to tooltip This CL exposes the Request Debug Trace button onto the tooltip. The button opens a dialog that allows a user to start a Pinpoint try job with additional arguments that exposes more tracing data. The user can only change the commit position and the trace arguments. This selection mimics the features available in the legacy perf. Bug: b/391784563 Change-Id: I854875415c2ef056f6f2de22b1a10e197378ae86 Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/966739 Commit-Queue: Leina Sun <sunxiaodi@google.com> Reviewed-by: Tony Seaward <seawardt@google.com>
diff --git a/perf/go/pinpoint/pinpoint.go b/perf/go/pinpoint/pinpoint.go index e652ffd..4ec4a72 100644 --- a/perf/go/pinpoint/pinpoint.go +++ b/perf/go/pinpoint/pinpoint.go
@@ -27,18 +27,21 @@ ) type CreateLegacyTryRequest struct { - Name string `json:"name"` - BaseGitHash string `json:"base_git_hash"` - ExperimentGitHash string `json:"experiment_git_hash"` - BasePatch string `json:"base_patch"` - ExperimentPatch string `json:"experiment_patch"` - Configuration string `json:"configuration"` - Benchmark string `json:"benchmark"` - Story string `json:"story"` - ExtraTestArgs string `json:"extra_test_args"` - Repository string `json:"repository"` - BugId string `json:"bug_id"` - User string `json:"user"` + Name string `json:"name"` + BaseGitHash string `json:"base_git_hash"` + // although "experiment" makes more sense in this context, the legacy Pinpoint API + // explicitly defines the experiment commit as "end_git_hash" and defines + // the experiment patch as "experiment_patch" + EndGitHash string `json:"end_git_hash"` + BasePatch string `json:"base_patch"` + ExperimentPatch string `json:"experiment_patch"` + Configuration string `json:"configuration"` + Benchmark string `json:"benchmark"` + Story string `json:"story"` + ExtraTestArgs string `json:"extra_test_args"` + Repository string `json:"repository"` + BugId string `json:"bug_id"` + User string `json:"user"` } type CreateBisectRequest struct { @@ -149,8 +152,8 @@ if req.BaseGitHash != "" { params.Set("base_git_hash", req.BaseGitHash) } - if req.ExperimentGitHash != "" { - params.Set("experiment_git_hash", req.ExperimentGitHash) + if req.EndGitHash != "" { + params.Set("end_git_hash", req.EndGitHash) } if req.BasePatch != "" { params.Set("base_patch", req.BasePatch)
diff --git a/perf/modules/chart-tooltip-sk/BUILD.bazel b/perf/modules/chart-tooltip-sk/BUILD.bazel index bfe4129..94c8a01 100644 --- a/perf/modules/chart-tooltip-sk/BUILD.bazel +++ b/perf/modules/chart-tooltip-sk/BUILD.bazel
@@ -27,6 +27,7 @@ "//elements-sk/modules/icons/check-icon-sk", "//perf/modules/point-links-sk", "//perf/modules/bisect-dialog-sk", + "//perf/modules/pinpoint-try-job-dialog-sk", ], ts_deps = [ "//:node_modules/lit",
diff --git a/perf/modules/chart-tooltip-sk/chart-tooltip-sk.ts b/perf/modules/chart-tooltip-sk/chart-tooltip-sk.ts index af914af..5f51ebd 100644 --- a/perf/modules/chart-tooltip-sk/chart-tooltip-sk.ts +++ b/perf/modules/chart-tooltip-sk/chart-tooltip-sk.ts
@@ -23,6 +23,7 @@ import '../triage-menu-sk/triage-menu-sk'; import '../user-issue-sk/user-issue-sk'; import '../bisect-dialog-sk/bisect-dialog-sk'; +import '../pinpoint-try-job-dialog-sk/pinpoint-try-job-dialog-sk'; import { UserIssueSk } from '../user-issue-sk/user-issue-sk'; import '../../../elements-sk/modules/icons/close-icon-sk'; import '../../../elements-sk/modules/icons/check-icon-sk'; @@ -30,6 +31,10 @@ import { removeSpecialFunctions } from '../paramtools'; import { PointLinksSk } from '../point-links-sk/point-links-sk'; import { BisectDialogSk, BisectPreloadParams } from '../bisect-dialog-sk/bisect-dialog-sk'; +import { + PinpointTryJobDialogSk, + TryJobPreloadParams, +} from '../pinpoint-try-job-dialog-sk/pinpoint-try-job-dialog-sk'; import { defaultColors } from '../common/plot-builder'; @customElement('commit-info-sk') @@ -130,6 +135,8 @@ private preloadBisectInputs: BisectPreloadParams | null = null; + private preloadTryJobInputs: TryJobPreloadParams | null = null; + _tooltip_fixed: boolean = false; _close_button_action: () => void = () => {}; @@ -159,6 +166,10 @@ // Bisect Dialog. bisectDialog: BisectDialogSk | null = null; + // Request debug trace dialog. This dialog creates a try job on legacy Pinpoint + // TODO(b/391784563): hide request debug trace when no tracing links have surfaced + tryJobDialog: PinpointTryJobDialogSk | null = null; + // The overall html template for outlining the contents needed in // chart-tooltip. // @@ -211,11 +222,15 @@ <button id="bisect" @click=${ele.openBisectDialog} ?hidden=${!ele._tooltip_fixed}> Bisect </button> + <button id="try-job" @click=${ele.openTryJobDialog} ?hidden=${!ele._tooltip_fixed}> + Debug Trace + </button> <user-issue-sk id="tooltip-user-issue-sk" ?hidden=${!ele._tooltip_fixed || ele.anomaly}></user-issue-sk> </div> <bisect-dialog-sk id="bisect-dialog-sk"></bisect-dialog-sk> + <pinpoint-try-job-dialog-sk id="pinpoint-try-job-dialog-sk"></pinpoint-try-job-dialog-sk> <point-links-sk id="tooltip-point-links" .commitPosition=${ele.commit_position} @@ -350,6 +365,7 @@ upgradeProperty(this, 'bug_host_url'); upgradeProperty(this, 'bug_id'); upgradeProperty(this, 'preloadBisectInputs'); + upgradeProperty(this, 'preloadTryJobInputs'); this._render(); this.commitRangeSk = this.querySelector('#tooltip-commit-range-link'); @@ -357,6 +373,7 @@ this.triageMenu = this.querySelector('#triage-menu'); this.pointLinks = this.querySelector('#tooltip-point-links'); this.bisectDialog = this.querySelector('#bisect-dialog-sk'); + this.tryJobDialog = this.querySelector('#pinpoint-try-job-dialog-sk'); this.addEventListener('anomaly-changed', () => { this._render(); @@ -504,12 +521,22 @@ this.bisectDialog!.open(); } + private openTryJobDialog() { + this.tryJobDialog!.open(); + } + setBisectInputParams(preloadInputs: BisectPreloadParams): void { this.preloadBisectInputs = preloadInputs; this.bisectDialog!.setBisectInputParams(this.preloadBisectInputs); this._render(); } + setTryJobInputParams(preloadInputs: TryJobPreloadParams): void { + this.preloadTryJobInputs = preloadInputs; + this.tryJobDialog!.setTryJobInputParams(this.preloadTryJobInputs); + this._render(); + } + get index(): number { return this._index; }
diff --git a/perf/modules/explore-simple-sk/BUILD.bazel b/perf/modules/explore-simple-sk/BUILD.bazel index 935ae5f..a33a644 100644 --- a/perf/modules/explore-simple-sk/BUILD.bazel +++ b/perf/modules/explore-simple-sk/BUILD.bazel
@@ -43,6 +43,7 @@ "//perf/modules/split-chart-menu-sk", "//perf/modules/bisect-dialog-sk", "//perf/modules/favorites-dialog-sk", + "//perf/modules/pinpoint-try-job-dialog-sk", ], ts_deps = [ "//infra-sk/modules/ElementSk:index_ts_lib",
diff --git a/perf/modules/explore-simple-sk/explore-simple-sk.ts b/perf/modules/explore-simple-sk/explore-simple-sk.ts index 7f39b31..7de7af7 100644 --- a/perf/modules/explore-simple-sk/explore-simple-sk.ts +++ b/perf/modules/explore-simple-sk/explore-simple-sk.ts
@@ -151,6 +151,7 @@ import { SplitChartSelectionEventDetails } from '../split-chart-menu-sk/split-chart-menu-sk'; import { getLegend, getTitle, isSingleTrace, legendFormatter } from '../dataframe/traceset'; import { BisectPreloadParams } from '../bisect-dialog-sk/bisect-dialog-sk'; +import { TryJobPreloadParams } from '../pinpoint-try-job-dialog-sk/pinpoint-try-job-dialog-sk'; import { FavoritesDialogSk } from '../favorites-dialog-sk/favorites-dialog-sk'; /** The type of trace we are adding to a plot. */ @@ -2200,6 +2201,13 @@ story: this.story ? this.story : '', }; + const preloadTryJobInputs: TryJobPreloadParams = { + testPath: this.testPath, + baseCommit: this.startCommit, + endCommit: this.endCommit, + story: this.story ? this.story : '', + }; + let unitValue: string = ''; traceName.split(',').forEach((test) => { if (test.startsWith('unit')) { @@ -2218,6 +2226,7 @@ const testName = legendFormatter(getLegendData)[legendIndex].replace('/' + unitValue, ''); tooltipElem!.setBisectInputParams(preloadBisectInputs); + tooltipElem!.setTryJobInputParams(preloadTryJobInputs); tooltipElem!.load( legendIndex, testName,
diff --git a/perf/modules/json/index.ts b/perf/modules/json/index.ts index fe098e8..1c481f7 100644 --- a/perf/modules/json/index.ts +++ b/perf/modules/json/index.ts
@@ -580,7 +580,7 @@ export interface CreateLegacyTryRequest { name: string; base_git_hash: string; - experiment_git_hash: string; + end_git_hash: string; base_patch: string; experiment_patch: string; configuration: string;
diff --git a/perf/modules/pinpoint-try-job-dialog-sk/pinpoint-try-job-dialog-sk.scss b/perf/modules/pinpoint-try-job-dialog-sk/pinpoint-try-job-dialog-sk.scss index fec3a00..f442fa5 100644 --- a/perf/modules/pinpoint-try-job-dialog-sk/pinpoint-try-job-dialog-sk.scss +++ b/perf/modules/pinpoint-try-job-dialog-sk/pinpoint-try-job-dialog-sk.scss
@@ -1,7 +1,7 @@ @import '../themes/themes.scss'; -ab-job-dialog-sk { - #ab-job-dialog { +pinpoint-try-job-dialog-sk { + #pinpoint-try-job-dialog { input { margin-bottom: 10px; width: 500px; @@ -22,7 +22,7 @@ font-family: 'Material Icons'; } - #ab-job-dialog-close { + #pinpoint-try-job-dialog-close { position: absolute; right: 0; top: 0;
diff --git a/perf/modules/pinpoint-try-job-dialog-sk/pinpoint-try-job-dialog-sk.ts b/perf/modules/pinpoint-try-job-dialog-sk/pinpoint-try-job-dialog-sk.ts index 128c304..177cafb 100644 --- a/perf/modules/pinpoint-try-job-dialog-sk/pinpoint-try-job-dialog-sk.ts +++ b/perf/modules/pinpoint-try-job-dialog-sk/pinpoint-try-job-dialog-sk.ts
@@ -1,6 +1,6 @@ /** * @module modules/pinpoint-try-job-dialog-sk - * @description <h2><code>ab-job-dialog-sk</code></h2> + * @description <h2><code>pinpoint-try-job-dialog-sk</code></h2> * * pinpoint-try-job-dialog-sk allows a user to trigger a Pinpoint A/B Try job. * While try jobs support more use cases, the use case for this dialog in perf @@ -74,12 +74,12 @@ private submitButton: HTMLButtonElement | null = null; private static template = (ele: PinpointTryJobDialogSk) => html` - <dialog id='ab-job-dialog'> + <dialog id='pinpoint-try-job-dialog'> <h2>Debug Trace</h2> - <button id="ab-job-dialog-close" @click=${ele.closeDialog}> + <button id="pinpoint-try-job-dialog-close" @click=${ele.closeDialog}> <close-icon-sk></close-icon-sk> </button> - <form id="ab-job-form"> + <form id="pinpoint-try-job-form"> <h3>Base Commit</h3> <input id="base-commit" type="text" value=${ele.baseCommit}></input> <h3>Experiment Commit</h3> @@ -92,7 +92,7 @@ <p> <input id="trace-args" type="text" value=${ele.traceArgs}></input> <div class=footer> - <button id="ab-job-dialog-submit" type="Submit">Send to Pinpoint</button> + <button id="pinpoint-try-job-dialog-submit" type="Submit">Send to Pinpoint</button> <spinner-sk id="dialog-spinner"></spinner-sk> ${ ele.jobUrl @@ -121,10 +121,10 @@ this._render(); - this._dialog = this.querySelector('#ab-job-dialog'); + this._dialog = this.querySelector('#pinpoint-try-job-dialog'); this._spinner = this.querySelector('#dialog-spinner'); - this.submitButton = this.querySelector('#ab-job-dialog-submit'); - this._form = this.querySelector('#ab-job-form'); + this.submitButton = this.querySelector('#pinpoint-try-job-dialog-submit'); + this._form = this.querySelector('#pinpoint-try-job-form'); this._form!.addEventListener('submit', (e) => { e.preventDefault(); this.postTryJob(); @@ -184,7 +184,7 @@ const req: CreateLegacyTryRequest = { name: 'Tracing Debug', base_git_hash: baseCommit.value, - experiment_git_hash: endCommit.value, + end_git_hash: endCommit.value, base_patch: '', experiment_patch: '', configuration: this.testPath.split('/')[1], @@ -197,7 +197,7 @@ }; req.name = `${req.name} on ${req.configuration}/${req.benchmark}/${req.story}`; console.log('here is the request', req); - fetch('/_/try/create', { + fetch('/_/try/', { method: 'POST', body: JSON.stringify(req), headers: {