jobs: add button to request execution details #107759
Conversation
|
This change is |
|
Loom video: https://www.loom.com/share/57ec04141b1443c2b41a630e1771dd2c?sid=0610ee20-cb72-4952-89ae-8c1a2202e2f2 Fist commit is from #107210 which should be merged soon. |
There was a problem hiding this comment.
Discussed offline, but adding here for tracking:
- Hide the Advanced Debugging tab if the user doesn't have ADMIN or VIEWACTIVITY role
- Change the request button to keep consistent with other pages, so move it to the right and increase its height, an example that can be follow is the Activate Diagnostics on the Statement Details page
Reviewed 12 of 12 files at r1, 2 of 10 files at r2, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @dt)
This is the last of the three PRs to add support for requesting, viewing and downloading execution details from the job details page. This change wires up the logic needed to request the execution details for a given job. The request is powered by the crdb_internal.request_job_execution_details builtin that triggers the collection of execution details. This change also adds logic to hide the Advanced Debugging tab if the user is not an admin. Fixes: cockroachdb#105076 Release note: None
|
thanks for your help! I changed the appearance of the button and also added logic to only show the tab if we are admin. This may change in the future but for now I think its better to take a conservative approach. 
|
adityamaru
force-pushed
the
request-execution-details
branch
from
f23c468 to
6ea4efb
Compare
There was a problem hiding this comment.
Nice!
The only thing that I would like to see is now with the permission checks, the page loading for an admin and then how it shows for non-admins, just to confirm everything is working as expected.
Reviewed 6 of 10 files at r2, 9 of 9 files at r3, all commit messages.
Reviewable status:
pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobProfilerView.tsx line 118 at r2 (raw file):
onDownloadExecutionFileClicked(req).then(resp => { const type = getContentTypeForFile(executionDetailFile); const executionFileBytes = new Blob([resp.data], {
add resp?.data because when initially load you might not have the resp yet and this can cause a crash
|
DBConsole as a non-admin user: 
Granted the user admin and refreshed the page: 
|
There was a problem hiding this comment.
Reviewable status:
pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobProfilerView.tsx line 118 at r2 (raw file):
Previously, maryliag (Marylia Gutierrez) wrote…
add
resp?.databecause when initially load you might not have the resp yet and this can cause a crash
Done.
|
TFTR! bors r=maryliag |
|
Build failed (retrying...): |
|
Build succeeded: |
This is the last of the three PRs to add support
for requesting, viewing and downloading execution
details from the job details page.
This change wires up the logic needed to request
the execution details for a given job. The request
is powered by the crdb_internal.request_job_execution_details
builtin that triggers the collection of execution details.
Fixes: #105076
Release note: None