Skip to content

Commit

Permalink
Merge #69444
Browse files Browse the repository at this point in the history
69444: ui: hide node/regions info on tenants r=maryliag a=maryliag

On tenant clusters we don't want to show information
about nodes and regions.

This commit hides that information on:
- statements page (table, filter, column selector)
- statement details page (overview, exec stats)
- transactions page (table, filter)

Release justification: Category 4
Release note (ui change): Hide node and regions information
on the new tenant plan (serverless / free tier)

Co-authored-by: Marylia Gutierrez <marylia@cockroachlabs.com>
  • Loading branch information
craig[bot] and maryliag committed Aug 28, 2021
2 parents 35e77d7 + 26a8b64 commit 4bf626a
Show file tree
Hide file tree
Showing 15 changed files with 173 additions and 80 deletions.
2 changes: 2 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/sortedtable/sortedtable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ export interface ColumnDescriptor<T> {
showByDefault?: boolean;
// If true, the user can't overwrite the setting for this column. False if not defined.
alwaysShow?: boolean;
// If true, hide this column for tenant clusters. False if not defined.
hideIfTenant?: boolean;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,4 +217,5 @@ export const getStatementDetailsPropsFixture = (): StatementDetailsProps => ({
uiConfig: {
showStatementDiagnosticsLink: true,
},
isTenant: false,
});
104 changes: 57 additions & 47 deletions pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ export interface StatementDetailsStateProps {
nodeRegions: { [nodeId: string]: string };
diagnosticsReports: cockroach.server.serverpb.IStatementDiagnosticsReport[];
uiConfig?: UIConfigState["pages"]["statementDetails"];
isTenant?: UIConfigState["isTenant"];
}

export type StatementDetailsOwnProps = StatementDetailsDispatchProps &
Expand Down Expand Up @@ -318,6 +319,7 @@ export class StatementDetails extends React.Component<
uiConfig: {
showStatementDiagnosticsLink: true,
},
isTenant: false,
};

changeSortSetting = (ss: SortSetting) => {
Expand Down Expand Up @@ -401,6 +403,7 @@ export class StatementDetails extends React.Component<
dismissStatementDiagnosticsAlertMessage,
onDiagnosticBundleDownload,
nodeRegions,
isTenant,
} = this.props;
const { currentTab } = this.state;

Expand Down Expand Up @@ -460,7 +463,7 @@ export class StatementDetails extends React.Component<

const statsByNode = this.props.statement.byNode;
const totalWorkload = calculateTotalWorkload(statsByNode);
populateRegionNodeForStatements(statsByNode, nodeRegions);
populateRegionNodeForStatements(statsByNode, nodeRegions, isTenant);
const nodes: string[] = unique(
(stats.nodes || []).map(node => node.toString()),
).sort();
Expand Down Expand Up @@ -576,19 +579,24 @@ export class StatementDetails extends React.Component<
<Col className="gutter-row" span={8}>
<SummaryCard className={cx("summary-card")}>
<Heading type="h5">Statement details</Heading>
<div className={summaryCardStylesCx("summary--card__item")}>
<Text>Nodes</Text>
<Text>
{intersperse<ReactNode>(
nodes.map(n => <NodeLink node={n} key={n} />),
", ",
)}
</Text>
</div>
<div className={summaryCardStylesCx("summary--card__item")}>
<Text>Regions</Text>
<Text>{intersperse<ReactNode>(regions, ", ")}</Text>
</div>
{!isTenant && (
<div>
<div className={summaryCardStylesCx("summary--card__item")}>
<Text>Nodes</Text>
<Text>
{intersperse<ReactNode>(
nodes.map(n => <NodeLink node={n} key={n} />),
", ",
)}
</Text>
</div>
<div className={summaryCardStylesCx("summary--card__item")}>
<Text>Regions</Text>
<Text>{intersperse<ReactNode>(regions, ", ")}</Text>
</div>
</div>
)}

<div className={summaryCardStylesCx("summary--card__item")}>
<Text>Database</Text>
<Text>{database}</Text>
Expand Down Expand Up @@ -801,40 +809,42 @@ export class StatementDetails extends React.Component<
})}
/>
</SummaryCard>
<SummaryCard className={cx("fit-content-width")}>
<h2
className={classNames(
cx("base-heading"),
summaryCardStylesCx("summary--card__title"),
)}
>
Stats By Node
<div className={cx("numeric-stats-table__tooltip")}>
<Tooltip content="Execution statistics for this statement per gateway node.">
<div
className={cx("numeric-stats-table__tooltip-hover-area")}
>
<div className={cx("numeric-stats-table__info-icon")}>
i
{!isTenant && (
<SummaryCard className={cx("fit-content-width")}>
<h2
className={classNames(
cx("base-heading"),
summaryCardStylesCx("summary--card__title"),
)}
>
Stats By Node
<div className={cx("numeric-stats-table__tooltip")}>
<Tooltip content="Execution statistics for this statement per gateway node.">
<div
className={cx("numeric-stats-table__tooltip-hover-area")}
>
<div className={cx("numeric-stats-table__info-icon")}>
i
</div>
</div>
</div>
</Tooltip>
</div>
</h2>
<StatementsSortedTable
className={cx("statements-table")}
data={statsByNode}
columns={makeNodesColumns(
statsByNode,
this.props.nodeNames,
totalWorkload,
nodeRegions,
)}
sortSetting={this.state.sortSetting}
onChangeSortSetting={this.changeSortSetting}
firstCellBordered
/>
</SummaryCard>
</Tooltip>
</div>
</h2>
<StatementsSortedTable
className={cx("statements-table")}
data={statsByNode}
columns={makeNodesColumns(
statsByNode,
this.props.nodeNames,
totalWorkload,
nodeRegions,
)}
sortSetting={this.state.sortSetting}
onChangeSortSetting={this.changeSortSetting}
firstCellBordered
/>
</SummaryCard>
)}
</TabPane>
</Tabs>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ import {
StatementDetails,
StatementDetailsDispatchProps,
StatementDetailsProps,
StatementDetailsStateProps,
} from "./statementDetails";
import { AppState } from "../store";
import {
selectStatement,
selectStatementDetailsUiConfig,
} from "./statementDetails.selectors";
import { selectIsTenant } from "../store/uiConfig";
import {
nodeDisplayNameByIDSelector,
nodeRegionsByIDSelector,
Expand Down Expand Up @@ -49,6 +49,7 @@ const mapStateToProps = (state: AppState, props: StatementDetailsProps) => {
statementFingerprint,
),
uiConfig: selectStatementDetailsUiConfig(state),
isTenant: selectIsTenant(state),
};
};

Expand Down
40 changes: 28 additions & 12 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import {
} from "../statementsTable";
import {
getLabel,
statisticsColumnLabels,
StatisticTableColumnKeys,
} from "../statsTableUtil/statsTableUtil";
import {
Expand All @@ -59,6 +58,7 @@ type IStatementDiagnosticsReport = cockroach.server.serverpb.IStatementDiagnosti
import sortableTableStyles from "src/sortedtable/sortedtable.module.scss";
import ColumnsSelector from "../columnsSelector/columnsSelector";
import { SelectOption } from "../multiSelectCheckbox/multiSelectCheckbox";
import { UIConfigState } from "../store/uiConfig";

const cx = classNames.bind(styles);
const sortableTableCx = classNames.bind(sortableTableStyles);
Expand Down Expand Up @@ -97,6 +97,7 @@ export interface StatementsPageStateProps {
lastReset: string;
columns: string[];
nodeRegions: { [key: string]: string };
isTenant?: UIConfigState["isTenant"];
}

export interface StatementsPageState {
Expand Down Expand Up @@ -141,6 +142,10 @@ export class StatementsPage extends React.Component<
this.activateDiagnosticsRef = React.createRef();
}

static defaultProps: Partial<StatementsPageProps> = {
isTenant: false,
};

getStateFromHistory = (): Partial<StatementsPageState> => {
const { history } = this.props;
const searchParams = new URLSearchParams(history.location.search);
Expand Down Expand Up @@ -294,7 +299,7 @@ export class StatementsPage extends React.Component<

filteredStatementsData = () => {
const { search, filters } = this.state;
const { statements, nodeRegions } = this.props;
const { statements, nodeRegions, isTenant } = this.props;
const timeValue = getTimeValueInSeconds(filters);
const sqlTypes =
filters.sqlType.length > 0
Expand Down Expand Up @@ -340,9 +345,12 @@ export class StatementsPage extends React.Component<
sqlTypes.length == 0 || sqlTypes.includes(statement.stats.sql_type),
)
.filter(
// The statement must contain at least one value from the selected nodes
// The statement must contain at least one value from the selected regions
// list if the list is not empty.
// If the cluster is a tenant cluster we don't care
// about regions.
statement =>
isTenant ||
regions.length == 0 ||
(statement.stats.nodes &&
containAny(
Expand All @@ -354,9 +362,12 @@ export class StatementsPage extends React.Component<
)),
)
.filter(
// The statement must contain at least one value from the selected regions
// The statement must contain at least one value from the selected nodes
// list if the list is not empty.
// If the cluster is a tenant cluster we don't care
// about nodes.
statement =>
isTenant ||
nodes.length == 0 ||
(statement.stats.nodes &&
containAny(
Expand All @@ -379,6 +390,7 @@ export class StatementsPage extends React.Component<
columns: userSelectedColumnsToShow,
onColumnsChange,
nodeRegions,
isTenant,
} = this.props;
const appAttrValue = getMatchParamByName(match, appAttr);
const selectedApp = appAttrValue || "";
Expand All @@ -388,13 +400,17 @@ export class StatementsPage extends React.Component<
const totalWorkload = calculateTotalWorkload(data);
const totalCount = data.length;
const isEmptySearchResults = statements?.length > 0 && search?.length > 0;
const nodes = Object.keys(nodeRegions)
.map(n => Number(n))
.sort();
const regions = unique(
nodes.map(node => nodeRegions[node.toString()]),
).sort();
populateRegionNodeForStatements(statements, nodeRegions);
// If the cluster is a tenant cluster we don't show info
// about nodes/regions.
const nodes = isTenant
? []
: Object.keys(nodeRegions)
.map(n => Number(n))
.sort();
const regions = isTenant
? []
: unique(nodes.map(node => nodeRegions[node.toString()])).sort();
populateRegionNodeForStatements(statements, nodeRegions, isTenant);

// Creates a list of all possible columns
const columns = makeStatementsColumns(
Expand All @@ -407,7 +423,7 @@ export class StatementsPage extends React.Component<
this.activateDiagnosticsRef,
onDiagnosticsReportDownload,
onStatementClick,
);
).filter(c => !(isTenant && c.hideIfTenant));

// If it's multi-region, we want to show the Regions/Nodes column by default
// and hide otherwise.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
selectTotalFingerprints,
selectColumns,
} from "./statementsPage.selectors";
import { selectIsTenant } from "../store/uiConfig";
import { AggregateStatistics } from "../statementsTable";
import { nodeRegionsByIDSelector } from "../store/nodes";

Expand All @@ -51,6 +52,7 @@ export const ConnectedStatementsPage = withRouter(
lastReset: selectLastReset(state),
columns: selectColumns(state),
nodeRegions: nodeRegionsByIDSelector(state),
isTenant: selectIsTenant(state),
}),
(dispatch: Dispatch) => ({
refreshStatements: () => dispatch(statementActions.refresh()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ function makeCommonColumns(
},
sort: (stmt: AggregateStatistics) => stmt.regionNodes.sort().join(", "),
showByDefault: false,
hideIfTenant: true,
},
];
return columns;
Expand Down Expand Up @@ -311,8 +312,13 @@ export function makeNodesColumns(
export function populateRegionNodeForStatements(
statements: AggregateStatistics[],
nodeRegions: { [p: string]: string },
isTenant: boolean,
) {
statements.forEach(stmt => {
if (isTenant) {
stmt.regionNodes = [];
return;
}
const regions: { [region: string]: Set<number> } = {};
// For each region, populate a list of all nodes where the statement was executed.
// E.g. {"gcp-us-east1" : [1,3,4]}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import { createSlice, PayloadAction } from "@reduxjs/toolkit";
import { merge } from "lodash";
import { DOMAIN_NAME } from "../utils";
import { createSelector } from "reselect";
import { AppState } from "../reducers";

export type UIConfigState = {
isTenant: boolean;
Expand Down Expand Up @@ -52,4 +54,14 @@ const uiConfigSlice = createSlice({
},
});

export const selectUIConfig = createSelector(
(state: AppState) => state.adminUI.uiConfig,
uiConfig => uiConfig,
);

export const selectIsTenant = createSelector(
selectUIConfig,
uiConfig => uiConfig.isTenant,
);

export const { actions, reducer } = uiConfigSlice;
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ storiesOf("Transactions Details", module)
lastReset={Date().toString()}
handleDetails={() => {}}
resetSQLStats={() => {}}
isTenant={false}
/>
))
.add("with loading indicator", () => (
Expand All @@ -42,6 +43,7 @@ storiesOf("Transactions Details", module)
lastReset={Date().toString()}
handleDetails={() => {}}
resetSQLStats={() => {}}
isTenant={false}
/>
))
.add("with error alert", () => (
Expand All @@ -53,5 +55,6 @@ storiesOf("Transactions Details", module)
lastReset={Date().toString()}
handleDetails={() => {}}
resetSQLStats={() => {}}
isTenant={false}
/>
));
Loading

0 comments on commit 4bf626a

Please sign in to comment.