Skip to content

Commit

Permalink
Merge #92357
Browse files Browse the repository at this point in the history
92357: ui: show regions in SQL Activity for tenants r=matthewtodd a=matthewtodd

Fixes #89949.

### Statements
| Regions Column | Regions Filter | Regions Field in Details |
|--|--|--|
|<img width="1628" alt="Screen Shot 2022-12-14 at 10 40 03 AM" src="https://user-images.githubusercontent.com/5261/207641874-ce92079b-0e98-4772-8a78-fb4f3ed1ea0f.png">|<img width="1628" alt="Screen Shot 2022-12-14 at 10 40 22 AM" src="https://user-images.githubusercontent.com/5261/207641883-0e5675ca-3959-4844-943c-a37943a32f2c.png">|<img width="1628" alt="Screen Shot 2022-12-14 at 10 40 43 AM" src="https://user-images.githubusercontent.com/5261/207641900-2a04bef4-398e-4b0b-a703-218778e2ad62.png">|

### Transactions
| Regions Column | Regions Filter | Regions Column in Details |
|--|--|--|
|<img width="1628" alt="Screen Shot 2022-12-14 at 10 41 10 AM" src="https://user-images.githubusercontent.com/5261/207641909-eddda97e-faa3-4695-8371-924f80c0f114.png">|<img width="1628" alt="Screen Shot 2022-12-14 at 10 41 21 AM" src="https://user-images.githubusercontent.com/5261/207641924-8543dada-a6eb-413e-bc2c-de58b737b86e.png">|<img width="1628" alt="Screen Shot 2022-12-14 at 10 41 56 AM" src="https://user-images.githubusercontent.com/5261/207641941-4d72b31f-99ab-4ab5-89b2-1d20baa9f8cb.png">|

Release note (ui change): The console statement and transaction pages for tenant clusters gained region columns and filters for multiregion tenants.

Co-authored-by: Matthew Todd <[email protected]>
  • Loading branch information
craig[bot] and matthewtodd committed Dec 14, 2022
2 parents 7e0ecb6 + 50340d0 commit 297b73b
Show file tree
Hide file tree
Showing 10 changed files with 154 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -635,20 +635,18 @@ export class StatementDetails extends React.Component<
<Col className="gutter-row" span={12}>
<SummaryCard id="first-card" className={cx("summary-card")}>
{!isTenant && (
<>
<SummaryCardItem
label="Nodes"
value={intersperse<ReactNode>(
nodes.map(n => <NodeLink node={n} key={n} />),
", ",
)}
/>
<SummaryCardItem
label="Regions"
value={intersperse<ReactNode>(regions, ", ")}
/>
</>
<SummaryCardItem
label="Nodes"
value={intersperse<ReactNode>(
nodes.map(n => <NodeLink node={n} key={n} />),
", ",
)}
/>
)}
<SummaryCardItem
label="Regions"
value={intersperse<ReactNode>(regions, ", ")}
/>
<SummaryCardItem label="Database" value={db} />
<SummaryCardItem
label="Application Name"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ const mapStateToProps = (state: AppState, props: RouteComponentProps) => {
isLoading: isLoading,
statementsError: lastError,
timeScale: selectTimeScale(state),
// TODO(todd): Remove this unused property!
nodeNames: selectIsTenant(state) ? {} : nodeDisplayNameByIDSelector(state),
nodeRegions: selectIsTenant(state) ? {} : nodeRegionsByIDSelector(state),
nodeRegions: nodeRegionsByIDSelector(state),
diagnosticsReports:
selectIsTenant(state) || selectHasViewActivityRedactedRole(state)
? []
Expand Down
24 changes: 11 additions & 13 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -525,10 +525,7 @@ export class StatementsPage extends React.Component<
.filter(
// 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 Down Expand Up @@ -578,7 +575,7 @@ export class StatementsPage extends React.Component<
const isEmptySearchResults = statements?.length > 0 && search?.length > 0;
// If the cluster is a tenant cluster we don't show info
// about nodes/regions.
populateRegionNodeForStatements(statements, nodeRegions, isTenant);
populateRegionNodeForStatements(statements, nodeRegions);

// Creates a list of all possible columns,
// hiding nodeRegions if is not multi-region and
Expand All @@ -595,6 +592,7 @@ export class StatementsPage extends React.Component<
onSelectDiagnosticsReportDropdownOption,
onStatementClick,
)
.filter(c => !(c.name === "regions" && regions.length < 2))
.filter(c => !(c.name === "regionNodes" && regions.length < 2))
.filter(c => !(isTenant && c.hideIfTenant));

Expand Down Expand Up @@ -673,14 +671,14 @@ export class StatementsPage extends React.Component<
nodeRegions,
} = this.props;

const nodes = isTenant
? []
: Object.keys(nodeRegions)
.map(n => Number(n))
.sort();
const regions = isTenant
? []
: unique(nodes.map(node => nodeRegions[node.toString()])).sort();
const nodes = Object.keys(nodeRegions)
.map(n => Number(n))
.sort();

const regions = unique(
nodes.map(node => nodeRegions[node.toString()]),
).sort();

const { filters, activeFilters } = this.state;

const longLoadingMessage = isNil(this.props.statements) &&
Expand Down Expand Up @@ -716,7 +714,7 @@ export class StatementsPage extends React.Component<
showSqlType={true}
showScan={true}
showRegions={regions.length > 1}
showNodes={nodes.length > 1}
showNodes={!isTenant && nodes.length > 1}
/>
</PageConfigItem>
<PageConfigItem className={commonStyles("separator")}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,7 @@ export const ConnectedStatementsPage = withRouter(
isTenant: selectIsTenant(state),
hasViewActivityRedactedRole: selectHasViewActivityRedactedRole(state),
lastReset: selectLastReset(state),
nodeRegions: selectIsTenant(state)
? {}
: nodeRegionsByIDSelector(state),
nodeRegions: nodeRegionsByIDSelector(state),
search: selectSearch(state),
sortSetting: selectSortSetting(state),
statements: selectStatements(state, props),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export interface AggregateStatistics {
diagnosticsReports?: StatementDiagnosticsReport[];
// totalWorkload is the sum of service latency of all statements listed on the table.
totalWorkload?: Long;
regions?: string[];
regionNodes?: string[];
}

Expand Down Expand Up @@ -250,16 +251,7 @@ export function makeStatementsColumns(
(stmt.stats.service_lat.mean * longToInt(stmt.stats.count)) /
totalWorkload,
},
{
name: "regionNodes",
title: statisticsTableTitles.regionNodes(statType),
className: cx("statements-table__col-regions"),
cell: (stmt: AggregateStatistics) => {
return longListWithTooltip(stmt.regionNodes.sort().join(", "), 50);
},
sort: (stmt: AggregateStatistics) => stmt.regionNodes.sort().join(", "),
hideIfTenant: true,
},
makeRegionsColumn(statType, isTenant),
{
name: "lastExecTimestamp",
title: statisticsTableTitles.lastExecTimestamp(statType),
Expand Down Expand Up @@ -302,6 +294,33 @@ export function makeStatementsColumns(
return columns;
}

function makeRegionsColumn(
statType: StatisticType,
isTenant: boolean,
): ColumnDescriptor<AggregateStatistics> {
if (isTenant) {
return {
name: "regions",
title: statisticsTableTitles.regions(statType),
className: cx("statements-table__col-regions"),
cell: (stmt: AggregateStatistics) => {
return longListWithTooltip(stmt.regions.sort().join(", "), 50);
},
sort: (stmt: AggregateStatistics) => stmt.regions.sort().join(", "),
};
} else {
return {
name: "regionNodes",
title: statisticsTableTitles.regionNodes(statType),
className: cx("statements-table__col-regions"),
cell: (stmt: AggregateStatistics) => {
return longListWithTooltip(stmt.regionNodes.sort().join(", "), 50);
},
sort: (stmt: AggregateStatistics) => stmt.regionNodes.sort().join(", "),
};
}
}

/**
* For each statement, generate the list of regions and nodes it was
* executed on. Each node is assigned to only one region and a region can
Expand All @@ -311,19 +330,12 @@ export function makeStatementsColumns(
* node it was executed on.
* @param nodeRegions: object with keys being the node id and the value
* which region it belongs to.
* @param isTenant: boolean indicating if the cluster is tenant, since
* node information doesn't need to be populated on this case.
*/
export function populateRegionNodeForStatements(
statements: AggregateStatistics[],
nodeRegions: { [p: string]: string },
isTenant: boolean,
): void {
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 All @@ -350,6 +362,7 @@ export function populateRegionNodeForStatements(
")",
);
});
stmt.regions = Object.keys(regions).sort();
stmt.regionNodes = regionNodes;
});
}
22 changes: 22 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export const statisticsColumnLabels = {
executionCount: "Execution Count",
maxMemUsage: "Max Memory",
networkBytes: "Network",
regions: "Regions",
regionNodes: "Regions/Nodes",
retries: "Retries",
rowsProcessed: "Rows Processed",
Expand Down Expand Up @@ -782,6 +783,27 @@ export const statisticsTableTitles: StatisticTableTitleType = {
</Tooltip>
);
},
regions: (statType: StatisticType) => {
let contentModifier = "";
switch (statType) {
case "transaction":
contentModifier = contentModifiers.transaction;
break;
case "statement":
contentModifier = contentModifiers.statement;
break;
}

return (
<Tooltip
placement="bottom"
style="tableTitle"
content={<p>Regions in which the {contentModifier} was executed.</p>}
>
{getLabel("regions")}
</Tooltip>
);
},
regionNodes: (statType: StatisticType) => {
let contentModifier = "";
switch (statType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,11 +319,7 @@ export class TransactionDetails extends React.Component<
const aggregatedStatements = aggregateStatements(
statementsForTransaction,
);
populateRegionNodeForStatements(
aggregatedStatements,
nodeRegions,
isTenant,
);
populateRegionNodeForStatements(aggregatedStatements, nodeRegions);
const duration = (v: number) => Duration(v * 1e9);

const transactionSampled =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { TableStatistics } from "../tableStatistics";
import { statisticsClasses } from "./transactionsPageClasses";
import {
aggregateAcrossNodeIDs,
generateRegion,
generateRegionNode,
getTrxAppFilterOptions,
searchTransactionsData,
Expand Down Expand Up @@ -409,14 +410,14 @@ export class TransactionsPage extends React.Component<

// 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();
const nodes = Object.keys(nodeRegions)
.map(n => Number(n))
.sort();

const regions = unique(
nodes.map(node => nodeRegions[node.toString()]),
).sort();

// We apply the search filters and app name filters prior to aggregating across Node IDs
// in order to match what's done on the Statements Page.
//
Expand Down Expand Up @@ -466,7 +467,7 @@ export class TransactionsPage extends React.Component<
activeFilters={activeFilters}
filters={filters}
showRegions={regions.length > 1}
showNodes={nodes.length > 1}
showNodes={!isTenant && nodes.length > 1}
/>
</PageConfigItem>
<PageConfigItem className={commonStyles("separator")}>
Expand Down Expand Up @@ -495,9 +496,8 @@ export class TransactionsPage extends React.Component<
t => ({
stats_data: t.stats_data,
node_id: t.node_id,
regionNodes: isTenant
? []
: generateRegionNode(t, statements, nodeRegions),
regions: generateRegion(t, statements, nodeRegions),
regionNodes: generateRegionNode(t, statements, nodeRegions),
}),
);
const { current, pageSize } = pagination;
Expand All @@ -513,6 +513,7 @@ export class TransactionsPage extends React.Component<
isTenant,
search,
)
.filter(c => !(c.name === "regions" && regions.length < 2))
.filter(c => !(c.name === "regionNodes" && regions.length < 2))
.filter(c => !(isTenant && c.hideIfTenant));

Expand Down
37 changes: 34 additions & 3 deletions pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,9 @@ export const filterTransactions = (
// and regions list (if the list is not empty).
if (regions.length == 0 && nodes.length == 0) return true;
// If the cluster is a tenant cluster we don't care
// about node/regions.
if (isTenant) return true;
// about nodes.
let foundRegion: boolean = regions.length == 0;
let foundNode: boolean = nodes.length == 0;
let foundNode: boolean = isTenant || nodes.length == 0;

getStatementsByFingerprintId(
t.stats_data.statement_fingerprint_ids,
Expand All @@ -245,6 +244,38 @@ export const filterTransactions = (
};
};

/**
* For each transaction, generate the list of regions all
* its statements were executed on.
* E.g. of one element of the list: `gcp-us-east1`
* @param transaction: list of transactions.
* @param statements: list of all statements collected.
* @param nodeRegions: object with keys being the node id and the value
* which region it belongs to.
*/
export const generateRegion = (
transaction: Transaction,
statements: Statement[],
nodeRegions: { [p: string]: string },
): string[] => {
const regions: Set<string> = new Set<string>();
// Get the list of statements that were executed on the transaction. Combine all
// nodes and regions of all the statements to a single list of `region: nodes`
// for the transaction.
// E.g. {"gcp-us-east1" : [1,3,4]}
getStatementsByFingerprintId(
transaction.stats_data.statement_fingerprint_ids,
statements,
).forEach(stmt => {
stmt.stats.nodes &&
stmt.stats.nodes.forEach(n => {
regions.add(nodeRegions[n.toString()]);
});
});

return Array.from(regions).sort();
};

/**
* For each transaction, generate the list of regions and nodes all
* its statements were executed on.
Expand Down
Loading

0 comments on commit 297b73b

Please sign in to comment.