jobs: add a Profiler tab to the job details page

[adityamaru]

This change adds a Profiler tab to the job details page. This change also adds a row that allows collection of a cluster-wide CPU profile for 5 seconds, of all the samples corresponding to the job's execution.

Fixes: #102735

Release note (ui change): job details page now has a profiler tab for more advanced observability into a job's execution. Currently, we support collecting a cluster-wide CPU profile of the job.

[cockroach-teamcity]

This change is 

[adityamaru]

[adityamaru]

I still need to test that this works on CC but considering it uses the same linking logic as Advanced Debug, I think it should. I'll report back.

[maryliag]

but considering it uses the same linking logic as Advanced Debug, I think it should

keep in mind that Advanced Debug doesn't exist on CC, so the routes might not work there at all

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt)

[maryliag]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @adityamaru and @dt)

---

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx line 19 at r1 (raw file):

import Long from "long";
import Helmet from "react-helmet";
import { RouteComponentProps, useHistory, useLocation } from "react-router-dom";

I don't see these 2 imports being used anywhere, maybe you forgot to remove from some previous tests?

---

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx line 128 at r1 (raw file):

          <SummaryCard className={cardCx("summary-card")}>
            <SummaryCardItem
              label="Cluster-wide CPU Profile (profiles all nodes; MEMORY OVERHEAD)"

is the intention of this MEMORY OVERHEAD to be a warning?
If so, we have some components that make warnings more obvious and you could add a better description.
For example you could add something like this below the SummaryCartItem

<InlineAlert intent="warning" title="Creation of a Profile consume additional resources and can potentially negatively impact workload responsiveness." />

---

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx line 267 at r1 (raw file):

                    {this.renderOverviewTabContent(hasNextRun, nextRun, job)}
                  </TabPane>
                  <TabPane tab={TabKeysEnum.PROFILER} key="profiler">

if the profiler doesn't work on CC, you can always hide this tab completely on CC. We have a context that you can use, such as
const isCockroachCloud = useContext(CockroachCloudContext);

{!isCockroachCloud && (<TabPane>...</TabPane>)}

[adityamaru]

sorry for the delay here @maryliag I'm going to get to this soon. Other things have bumped this PR from the queue 😓

[maryliag]

no worries! 😄

[adityamaru]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt and @maryliag)

---

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx line 19 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

I don't see these 2 imports being used anywhere, maybe you forgot to remove from some previous tests?

Done.

---

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx line 128 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

is the intention of this MEMORY OVERHEAD to be a warning?
If so, we have some components that make warnings more obvious and you could add a better description.
For example you could add something like this below the SummaryCartItem

<InlineAlert intent="warning" title="Creation of a Profile consume additional resources and can potentially negatively impact workload responsiveness." />

Nice! Changed to use this now.

---

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx line 267 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

if the profiler doesn't work on CC, you can always hide this tab completely on CC. We have a context that you can use, such as
const isCockroachCloud = useContext(CockroachCloudContext);

{!isCockroachCloud && (<TabPane>...</TabPane>)}

nice, added this check in. I can't seem to add the const at the top level because this is a class component? React shouts at me with:

Invalid hook call. Hooks can only be called inside of the body of a function component.

[adityamaru]

[maryliag]

Reviewed all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @adityamaru and @dt)

---

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx line 267 at r1 (raw file):

Previously, adityamaru (Aditya Maru) wrote…

nice, added this check in. I can't seem to add the const at the top level because this is a class component? React shouts at me with:

Invalid hook call. Hooks can only be called inside of the body of a function component.

After you made this change, is the page loading as expected? At least for DB Console

---

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx line 134 at r3 (raw file):

            <InlineAlert
              intent="warning"
              title="This profiles all nodes in the cluster. This operation buffers profiles in memory and should only be run if there is sufficient overhead."

tagging @florence-crl to review the warning message
I don't know if the user will know what "sufficient overhead" means and how much is it, so maybe something that matches a few other performance warnings we have:

"This operation buffers profiles in memory for all the nodes in the cluster and can potentially negatively impact workload responsiveness."

[florence-crl]

I agree with @maryliag. Please use her recommendation for the warning message:
"This operation buffers profiles in memory for all the nodes in the cluster and can potentially negatively impact workload responsiveness."

[adityamaru]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @florence-crl, and @maryliag)

---

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx line 267 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

After you made this change, is the page loading as expected? At least for DB Console

Yup for the DB console its working as expected!

---

pkg/ui/workspaces/cluster-ui/src/jobs/jobDetailsPage/jobDetails.tsx line 134 at r3 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

tagging @florence-crl to review the warning message
I don't know if the user will know what "sufficient overhead" means and how much is it, so maybe something that matches a few other performance warnings we have:

"This operation buffers profiles in memory for all the nodes in the cluster and can potentially negatively impact workload responsiveness."

I don't know whether I'd say it negatively impacts workload responsiveness. I've changed it to say:

"This operation buffers profiles in memory for all the nodes in the cluster and can result in increased memory usage."

@florence-crl is this okay?

[maryliag]

Nice work on this!

I will just leave now to @florence-crl the approval on the message itself

Reviewed all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @dt and @florence-crl)

[florence-crl]

"This operation buffers profiles in memory for all the nodes in the cluster and can result in increased memory usage."
sounds good to me.

[adityamaru]

TFTR!

bors r=maryliag

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)