Skip to content

Commit

Permalink
ui: proper handle of unset app name
Browse files Browse the repository at this point in the history
Previously, if a filter for `unset` app name
was selected on Statement page, the Stament Details
page was not finding the correct values.
Now if no filter is selected, no search params is
passed to the Details page. If there is an empty value
for the appNames on request, is looking for unset.

This commit also creates a constant for the unset value
and replaces all places to use the constant instead.

Fixes #83002

Release note (bug fix): Statement Details now find the stats
when the `unset` app filter is selected.
  • Loading branch information
maryliag committed Jun 24, 2022
1 parent 107fb89 commit d8954c5
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 25 deletions.
3 changes: 3 additions & 0 deletions pkg/server/combined_statement_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,9 @@ func getStatementDetailsQueryClausesAndArgs(
if !(len(req.AppNames) == 1 && req.AppNames[0] == "") {
buffer.WriteString(" AND (")
for i, app := range req.AppNames {
if app == "(unset)" {
app = ""
}
if i != 0 {
args = append(args, app)
buffer.WriteString(fmt.Sprintf(" OR app_name = $%d", len(args)))
Expand Down
13 changes: 8 additions & 5 deletions pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ import sessionPageStyles from "./sessionPage.module.scss";
import ColumnsSelector, {
SelectOption,
} from "../columnsSelector/columnsSelector";
import { TimestampToMoment } from "src/util";
import { TimestampToMoment, unset } from "src/util";
import moment from "moment";
import {
getLabel,
Expand Down Expand Up @@ -104,9 +104,12 @@ export type SessionsPageProps = OwnProps & RouteComponentProps;

function getSessionAppFilterOptions(sessions: SessionInfo[]): string[] {
const uniqueAppNames = new Set(
sessions.map(s =>
s.session.application_name ? s.session.application_name : "(unset)",
),
sessions.map(s => {
if (s.session.application_name.startsWith("$ internal")) {
return "$ internal";
}
return s.session.application_name ? s.session.application_name : unset;
}),
);

return Array.from(uniqueAppNames).sort();
Expand Down Expand Up @@ -280,7 +283,7 @@ export class SessionsPage extends React.Component<
if (apps.includes(internalAppNamePrefix)) {
showInternal = true;
}
if (apps.includes("(unset)")) {
if (apps.includes(unset)) {
apps.push("");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
statementKey,
StatementStatistics,
TimestampToMoment,
unset,
} from "src/util";
import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
import { RouteComponentProps } from "react-router-dom";
Expand Down Expand Up @@ -81,9 +82,7 @@ export const selectApps = createSelector(sqlStatsSelector, sqlStatsState => {
}
},
);
return []
.concat(sawBlank ? ["(unset)"] : [])
.concat(Object.keys(apps).sort());
return [].concat(sawBlank ? [unset] : []).concat(Object.keys(apps).sort());
});

// selectDatabases returns the array of all databases with statement statistics present
Expand All @@ -98,7 +97,7 @@ export const selectDatabases = createSelector(
return Array.from(
new Set(
sqlStatsState.data.statements.map(s =>
s.key.key_data.database ? s.key.key_data.database : "(unset)",
s.key.key_data.database ? s.key.key_data.database : unset,
),
),
)
Expand Down Expand Up @@ -154,7 +153,7 @@ export const selectStatements = createSelector(
if (criteria.includes(state.data.internal_app_name_prefix)) {
showInternal = true;
}
if (criteria.includes("(unset)")) {
if (criteria.includes(unset)) {
criteria.push("");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
unique,
containAny,
syncHistory,
unset,
} from "src/util";
import {
AggregateStatistics,
Expand Down Expand Up @@ -432,7 +433,7 @@ export class StatementsPage extends React.Component<
: [];
const databases =
filters.database.length > 0 ? filters.database.split(",") : [];
if (databases.includes("(unset)")) {
if (databases.includes(unset)) {
databases.push("");
}
const regions =
Expand Down
6 changes: 3 additions & 3 deletions pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ import {
FixLong,
longToInt,
TimestampToNumber,
TimestampToString,
addStatementStats,
flattenStatementStats,
DurationToNumber,
computeOrUseStmtSummary,
transactionScopedStatementKey,
unset,
} from "../util";

type Statement = protos.cockroach.server.serverpb.StatementsResponse.ICollectedStatementStatistics;
Expand All @@ -42,7 +42,7 @@ export const getTrxAppFilterOptions = (
const uniqueAppNames = new Set(
transactions
.filter(t => !t.stats_data.app.startsWith(prefix))
.map(t => (t.stats_data.app ? t.stats_data.app : "(unset)")),
.map(t => (t.stats_data.app ? t.stats_data.app : unset)),
);

return Array.from(uniqueAppNames).sort();
Expand Down Expand Up @@ -166,7 +166,7 @@ export const filterTransactions = (
if (apps.includes(internalAppNamePrefix)) {
showInternal = true;
}
if (apps.includes("(unset)")) {
if (apps.includes(unset)) {
apps.push("");
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/util/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ export const sessionAttr = "session";
export const tabAttr = "tab";
export const tableNameAttr = "table_name";
export const txnFingerprintIdAttr = "txn_fingerprint_id";
export const unset = "(unset)";
export const viewAttr = "view";

export const REMOTE_DEBUGGING_ERROR_TEXT =
"This information is not available due to the current value of the 'server.remote_debugging.mode' setting.";
Expand Down
2 changes: 2 additions & 0 deletions pkg/ui/workspaces/db-console/src/util/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,7 @@ export const {
tabAttr,
tableNameAttr,
txnFingerprintIdAttr,
unset,
viewAttr,
REMOTE_DEBUGGING_ERROR_TEXT,
} = util;
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ import { merge } from "lodash";

import "src/protobufInit";
import * as protos from "src/js/protos";
import { appAttr, appNamesAttr, statementAttr } from "src/util/constants";
import {
appAttr,
appNamesAttr,
statementAttr,
unset,
} from "src/util/constants";
import {
selectStatements,
selectApps,
Expand Down Expand Up @@ -170,7 +175,7 @@ describe("selectStatements", () => {
],
timeScale,
);
const props = makeRoutePropsWithApp("(unset)");
const props = makeRoutePropsWithApp(unset);

const result = selectStatements(state, props);

Expand Down Expand Up @@ -214,7 +219,7 @@ describe("selectApps", () => {

const result = selectApps(state);

assert.deepEqual(result, ["(unset)", "cockroach sql", "foobar"]);
assert.deepEqual(result, [unset, "cockroach sql", "foobar"]);
});
});

Expand Down Expand Up @@ -390,10 +395,7 @@ describe("selectStatement", () => {
detailsC,
]);
const stmtAFingerprintID = stmtA.id.toString();
const props = makeRoutePropsWithStatementAndApp(
stmtAFingerprintID,
"(unset)",
);
const props = makeRoutePropsWithStatementAndApp(stmtAFingerprintID, unset);

const { statementDetails } = selectStatementDetails(state, props);
const result = statementDetails.statement;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
import { CachedDataReducerState } from "src/redux/cachedDataReducer";
import { AdminUIState, AppDispatch } from "src/redux/state";
import { StatementsResponseMessage } from "src/util/api";
import { appAttr } from "src/util/constants";
import { appAttr, unset } from "src/util/constants";
import { PrintTime } from "src/views/reports/containers/range/print";
import { selectDiagnosticsReportsPerStatement } from "src/redux/statements/statementsSelectors";
import {
Expand Down Expand Up @@ -103,7 +103,7 @@ export const selectStatements = createSelector(
if (criteria.includes(state.data.internal_app_name_prefix)) {
showInternal = true;
}
if (criteria.includes("(unset)")) {
if (criteria.includes(unset)) {
criteria.push("");
}

Expand Down Expand Up @@ -189,7 +189,7 @@ export const selectApps = createSelector(
},
);
return []
.concat(sawBlank ? ["(unset)"] : [])
.concat(sawBlank ? [unset] : [])
.concat(Object.keys(apps))
.sort();
},
Expand All @@ -206,7 +206,7 @@ export const selectDatabases = createSelector(
return Array.from(
new Set(
state.data.statements.map(s =>
s.key.key_data.database ? s.key.key_data.database : "(unset)",
s.key.key_data.database ? s.key.key_data.database : unset,
),
),
)
Expand Down

0 comments on commit d8954c5

Please sign in to comment.