Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ui: add plan gist as option on bundle collection #110931

Merged
merged 1 commit into from Sep 20, 2023
Merged

ui: add plan gist as option on bundle collection #110931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion pkg/ui/workspaces/cluster-ui/src/api/statementDiagnosticsApi.ts
View file
Open in desktop
Expand Up @@ -74,6 +74,7 @@ export type InsertStmtDiagnosticRequest = {
samplingProbability?: number;
minExecutionLatencySeconds?: number;
expiresAfterSeconds?: number;
planGist: string;
};

export type InsertStmtDiagnosticResponse = {
Expand All @@ -85,8 +86,15 @@ export function createStatementDiagnosticsReport({
samplingProbability,
minExecutionLatencySeconds,
expiresAfterSeconds,
planGist,
}: InsertStmtDiagnosticRequest): Promise<InsertStmtDiagnosticResponse> {
const args: any = [stmtFingerprint];
let query = `SELECT crdb_internal.request_statement_bundle($1, $2, $3::INTERVAL, $4::INTERVAL) as req_resp`;

if (planGist) {
args.push(planGist);
query = `SELECT crdb_internal.request_statement_bundle($1, $2, $3, $4::INTERVAL, $5::INTERVAL) as req_resp`;
}

if (samplingProbability) {
args.push(samplingProbability);
Expand All @@ -105,7 +113,7 @@ export function createStatementDiagnosticsReport({
}

const createStmtDiag = {
sql: `SELECT crdb_internal.request_statement_bundle($1, $2, $3::INTERVAL, $4::INTERVAL) as req_resp`,
sql: query,
arguments: args,
};

Expand Down
6 changes: 6 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/statementDetails/diagnostics/diagnosticsView.spec.tsx
View file
Open in desktop
Expand Up @@ -63,6 +63,7 @@ describe("DiagnosticsView", () => {
requestTime={undefined}
currentScale={ts}
onChangeTimeScale={mockSetTimeScale}
planGists={["gist"]}
/>
</MemoryRouter>,
);
Expand All @@ -73,6 +74,7 @@ describe("DiagnosticsView", () => {
activateButtonComponent.simulate("click");
expect(activateDiagnosticsRef.current.showModalFor).toBeCalledWith(
statementFingerprint,
["gist"],
);
});
});
Expand All @@ -94,6 +96,7 @@ describe("DiagnosticsView", () => {
dismissAlertMessage={() => {}}
currentScale={ts}
onChangeTimeScale={mockSetTimeScale}
planGists={["gist"]}
/>
</TestStoreProvider>,
);
Expand All @@ -110,6 +113,7 @@ describe("DiagnosticsView", () => {
activateButtonComponent.simulate("click");
expect(activateDiagnosticsRef.current.showModalFor).toBeCalledWith(
statementFingerprint,
["gist"],
);
});

Expand All @@ -128,6 +132,7 @@ describe("DiagnosticsView", () => {
currentScale={ts}
requestTime={requestTime}
onChangeTimeScale={mockSetTimeScale}
planGists={["gist"]}
/>
</TestStoreProvider>,
);
Expand All @@ -152,6 +157,7 @@ describe("DiagnosticsView", () => {
currentScale={ts}
requestTime={requestTime}
onChangeTimeScale={mockSetTimeScale}
planGists={["gist"]}
/>
</TestStoreProvider>,
);
Expand Down
5 changes: 5 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/statementDetails/diagnostics/diagnosticsView.tsx
View file
Open in desktop
Expand Up @@ -60,6 +60,7 @@ export interface DiagnosticsViewDispatchProps {

export interface DiagnosticsViewOwnProps {
statementFingerprint?: string;
planGists?: string[];
}

export type DiagnosticsViewProps = DiagnosticsViewOwnProps &
Expand All @@ -80,6 +81,7 @@ const NavButton: React.FC = props => (

export const EmptyDiagnosticsView = ({
statementFingerprint,
planGists,
showDiagnosticsViewLink,
activateDiagnosticsRef,
}: DiagnosticsViewProps): React.ReactElement => {
Expand All @@ -94,6 +96,7 @@ export const EmptyDiagnosticsView = ({
onClick={() =>
activateDiagnosticsRef?.current?.showModalFor(
statementFingerprint,
planGists,
)
}
>
Expand Down Expand Up @@ -272,6 +275,7 @@ export class DiagnosticsView extends React.Component<
activateDiagnosticsRef,
currentScale,
onChangeTimeScale,
planGists,
} = this.props;

const readyToRequestDiagnostics = diagnosticsReports.every(
Expand Down Expand Up @@ -329,6 +333,7 @@ export class DiagnosticsView extends React.Component<
onClick={() =>
activateDiagnosticsRef?.current?.showModalFor(
statementFingerprint,
planGists,
)
}
disabled={!readyToRequestDiagnostics}
Expand Down
1 change: 1 addition & 0 deletions pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx
View file
Open in desktop
Expand Up @@ -1021,6 +1021,7 @@ export class StatementDetails extends React.Component<
onSortingChange={this.props.onSortingChange}
currentScale={this.props.timeScale}
onChangeTimeScale={this.changeTimeScale}
planGists={this.props.statementDetails.statement.stats.plan_gists}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is plan_gists always going to exists or do we need to do some optional chaining here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always going to exist

/>
);
};
Expand Down
25 changes: 25 additions & 0 deletions ...ui/workspaces/cluster-ui/src/statementsDiagnostics/activateStatementDiagnosticsModal.scss
View file
Open in desktop
Expand Up @@ -53,6 +53,10 @@
font-weight: $font-weight--light;
}

&__plan-gist-group {
margin-bottom: -25px;
}

&__conditional-container {
display: flex;
flex-direction: column;
Expand All @@ -73,6 +77,14 @@
margin-bottom: $spacing-smaller;
}

&__plan-gist-container {
display: flex;
flex-direction: row;
margin-top: $spacing-smaller;
margin-bottom: $spacing-smaller;
margin-left: $spacing-medium;
}

&__trace-warning {
font-family: $font-family--base;
font-size: $font-size--small;
Expand Down Expand Up @@ -127,6 +139,15 @@
height: 40px;
line-height: 40px;

.ant-select-selection__rendered {
margin-top: 0;
}
}
&__plan-gist {
width: 400px;
height: 40px;
line-height: 40px;

.ant-select-selection__rendered {
margin-top: 0;
}
Expand All @@ -144,3 +165,7 @@
white-space: normal;
}
}

.margin-bottom {
margin-bottom: 10px;
}
67 changes: 63 additions & 4 deletions pkg/ui/workspaces/cluster-ui/src/statementsDiagnostics/activateStatementDiagnosticsModal.tsx
View file
Open in desktop
Expand Up @@ -31,11 +31,11 @@ const { Option } = Select;
export interface ActivateDiagnosticsModalProps {
activate: (insertStmtDiagnosticsRequest: InsertStmtDiagnosticRequest) => void;
refreshDiagnosticsReports: () => void;
onOpenModal?: (statement: string) => void;
onOpenModal?: (statement: string, planGists: string[]) => void;
}

export interface ActivateDiagnosticsModalRef {
showModalFor: (statement: string) => void;
showModalFor: (statement: string, planGists: string[]) => void;
}

export const ActivateStatementDiagnosticsModal = React.forwardRef(
Expand All @@ -45,7 +45,10 @@ export const ActivateStatementDiagnosticsModal = React.forwardRef(
) => {
const [visible, setVisible] = useState(false);
const [statement, setStatement] = useState<string>();
const [planGists, setPlanGists] = useState<string[]>();
const [conditional, setConditional] = useState(true);
const [filterPerPlanGist, setFilterPerPlanGist] = useState(false);
const [selectedPlanGist, setSelectedPlanGist] = useState<string>("");
const [expires, setExpires] = useState(true);
const [minExecLatency, setMinExecLatency] = useState(100);
const [minExecLatencyUnit, setMinExecLatencyUnit] =
Expand Down Expand Up @@ -84,6 +87,7 @@ export const ActivateStatementDiagnosticsModal = React.forwardRef(
const onOkHandler = useCallback(() => {
activate({
stmtFingerprint: statement,
planGist: filterPerPlanGist ? selectedPlanGist : null,
minExecutionLatencySeconds: getMinExecLatency(
conditional,
minExecLatency,
Expand All @@ -102,20 +106,30 @@ export const ActivateStatementDiagnosticsModal = React.forwardRef(
expires,
expiresAfter,
traceSampleRate,
filterPerPlanGist,
selectedPlanGist,
]);

const onCancelHandler = useCallback(() => setVisible(false), []);

useImperativeHandle(ref, () => {
return {
showModalFor: (forwardStatement: string) => {
showModalFor: (
forwardStatement: string,
forwardPlanGists: string[],
) => {
setStatement(forwardStatement);
setPlanGists(forwardPlanGists);
setVisible(true);
onOpenModal && onOpenModal(forwardStatement);
onOpenModal && onOpenModal(forwardStatement, forwardPlanGists);
},
};
});

if (planGists && selectedPlanGist == "") {
setSelectedPlanGist(planGists[0]);
}

return (
<Modal
visible={visible}
Expand Down Expand Up @@ -218,6 +232,51 @@ export const ActivateStatementDiagnosticsModal = React.forwardRef(
</Button.Group>
</Radio.Group>
<Divider type="horizontal" />

<Radio.Group
value={filterPerPlanGist}
className={cx("diagnostic__plan-gist-group")}
>
<Button.Group className={cx("diagnostic__btn-group")}>
<Radio
value={false}
className={cx("diagnostic__radio-btn", "margin-bottom")}
onChange={() => setFilterPerPlanGist(false)}
>
For all plan gists
</Radio>
<br />
<Radio
value={true}
className={cx("diagnostic__radio-btn")}
onChange={() => setFilterPerPlanGist(true)}
>
For the following plan gist:
<div className={cx("diagnostic__plan-gist-container")}>
<Select
disabled={!filterPerPlanGist}
value={selectedPlanGist}
defaultValue={planGists ? planGists[0] : ""}
onChange={(selected: string) =>
setSelectedPlanGist(selected)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can skip creating the anon function here and just pass in setSelectedPlanGist since the params line up and there's no extra work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting a lint error, because the default is int, so I need to create this way to get the parameter as string

}
className={cx("diagnostic__select__plan-gist")}
size="large"
showSearch={true}
>
{planGists?.map((gist: string) => {
return (
<Option value={gist} key={gist}>
{gist}
</Option>
);
})}
</Select>
</div>
</Radio>
</Button.Group>
</Radio.Group>
<Divider type="horizontal" />
<Checkbox checked={expires} onChange={() => setExpires(!expires)}>
<div className={cx("diagnostic__checkbox-text")}>
Diagnostics request expires after:
Expand Down
5 changes: 4 additions & 1 deletion pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTableContent.tsx
View file
Open in desktop
Expand Up @@ -95,7 +95,10 @@ export const StatementTableCell = {
{canActivateDiagnosticReport ? (
<Button
onClick={() =>
activateDiagnosticsRef?.current?.showModalFor(stmt.label)
activateDiagnosticsRef?.current?.showModalFor(
stmt.label,
stmt.stats.plan_gists,
)
}
type="secondary"
size="small"
Expand Down
2 changes: 2 additions & 0 deletions ...i/workspaces/cluster-ui/src/store/statementDiagnostics/statementDiagnostics.sagas.spec.ts
View file
Open in desktop
Expand Up @@ -33,13 +33,15 @@ import moment from "moment-timezone";
describe("statementsDiagnostics sagas", () => {
describe("createDiagnosticsReportSaga", () => {
const statementFingerprint = "SELECT * FROM table";
const planGist = "gist";
const minExecLatency = 100; // num seconds
const expiresAfter = 0; // num seconds, setting expiresAfter to 0 means the request won't expire.

const insertRequest: InsertStmtDiagnosticRequest = {
stmtFingerprint: statementFingerprint,
minExecutionLatencySeconds: minExecLatency,
expiresAfterSeconds: expiresAfter,
planGist: planGist,
};

const insertResponse: InsertStmtDiagnosticResponse = {
Expand Down
4 changes: 4 additions & 0 deletions pkg/ui/workspaces/db-console/src/redux/statements/statementsSagas.spec.ts
View file
Open in desktop
Expand Up @@ -37,12 +37,14 @@ describe("statementsSagas", () => {
describe("requestDiagnostics generator", () => {
it("calls api#createStatementDiagnosticsReport with statement fingerprint, min exec latency, and expires after fields as payload", () => {
const statementFingerprint = "some-id";
const planGist = "gist";
const minExecLatency = 10; // num seconds
const expiresAfter = 15 * 60; // num seconds (num mins * num seconds per min)
const insertStmtDiagnosticsRequest = {
stmtFingerprint: statementFingerprint,
minExecutionLatencySeconds: minExecLatency,
expiresAfterSeconds: expiresAfter,
planGist: planGist,
};
const action = createStatementDiagnosticsReportAction(
insertStmtDiagnosticsRequest,
Expand All @@ -66,12 +68,14 @@ describe("statementsSagas", () => {

it("calls dispatched failed action if api#createStatementDiagnosticsReport request failed ", () => {
const statementFingerprint = "some-id";
const planGist = "gist";
const minExecLatency = 10; // num seconds
const expiresAfter = 15 * 60; // num seconds (num mins * num seconds per min)
const insertStmtDiagnosticsRequest = {
stmtFingerprint: statementFingerprint,
minExecutionLatencySeconds: minExecLatency,
expiresAfterSeconds: expiresAfter,
planGist: planGist,
};
const action = createStatementDiagnosticsReportAction(
insertStmtDiagnosticsRequest,
Expand Down