From 051ddb71684247b91b8269db003e47c6146ff1dd Mon Sep 17 00:00:00 2001 From: Matthew Todd Date: Tue, 15 Nov 2022 11:42:26 -0500 Subject: [PATCH 1/5] ui: remove unused function Release note: None --- .../src/statementsTable/statementsTable.tsx | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx index 40a2b9a31fc2..2a3e6cb01e65 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx @@ -43,7 +43,6 @@ import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; import { StatementTableCell } from "./statementsTableContent"; import { statisticsTableTitles, - NodeNames, StatisticType, } from "../statsTableUtil/statsTableUtil"; @@ -327,25 +326,6 @@ export function makeStatementsColumns( return columns; } -export function makeNodesColumns( - statements: AggregateStatistics[], - nodeNames: NodeNames, - totalWorkload: number, - nodeRegions: { [nodeId: string]: string }, -): ColumnDescriptor[] { - const original: ColumnDescriptor[] = [ - { - name: "nodes", - title: null, - cell: StatementTableCell.nodeLink(nodeNames), - }, - ]; - - return original.concat( - makeCommonColumns(statements, totalWorkload, nodeRegions, "statement"), - ); -} - /** * 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 From 33ff50d53965dee9601341162e77a71051f2dbd4 Mon Sep 17 00:00:00 2001 From: Matthew Todd Date: Tue, 15 Nov 2022 12:03:11 -0500 Subject: [PATCH 2/5] ui: inline makeCommonColumns It only had one caller, and I was about to change its signature. Since we don't need to reuse it in its current form, better to inline it and see if something else emerges. Release note: None --- .../src/statementsTable/statementsTable.tsx | 175 ++++++++---------- 1 file changed, 81 insertions(+), 94 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx index 2a3e6cb01e65..79e235c0bb34 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx @@ -52,11 +52,84 @@ import styles from "./statementsTable.module.scss"; import { StatementDiagnosticsReport } from "../api"; const cx = classNames.bind(styles); -function makeCommonColumns( +export interface AggregateStatistics { + aggregatedFingerprintID: string; + aggregatedFingerprintHexID: string; + // label is either shortStatement (StatementsPage) or nodeId (StatementDetails). + label: string; + // summary exists only for SELECT/INSERT/UPSERT/UPDATE statements, and is + // replaced with shortStatement otherwise. + summary: string; + aggregatedTs: number; + aggregationInterval: number; + implicitTxn: boolean; + fullScan: boolean; + database: string; + applicationName: string; + stats: StatementStatistics; + drawer?: boolean; + firstCellBordered?: boolean; + diagnosticsReports?: StatementDiagnosticsReport[]; + // totalWorkload is the sum of service latency of all statements listed on the table. + totalWorkload?: Long; + regionNodes?: string[]; +} + +export class StatementsSortedTable extends SortedTable {} + +export function shortStatement( + summary: StatementSummary, + original: string, +): string { + switch (summary.statement) { + case "update": + return "UPDATE " + summary.table; + case "insert": + return "INSERT INTO " + summary.table; + case "select": + return "SELECT FROM " + summary.table; + case "delete": + return "DELETE FROM " + summary.table; + case "create": + return "CREATE TABLE " + summary.table; + case "set": + return "SET " + summary.table; + default: + return original; + } +} + +export function makeStatementFingerprintColumn( + statType: StatisticType, + selectedApps: string[], + search?: string, + onStatementClick?: (statement: string) => void, +): ColumnDescriptor { + return { + name: "statements", + title: statisticsTableTitles.statements(statType), + className: cx("cl-table__col-query-text"), + cell: StatementTableCell.statements(search, selectedApps, onStatementClick), + sort: stmt => stmt.label, + alwaysShow: true, + }; +} + +export function makeStatementsColumns( statements: AggregateStatistics[], + selectedApps: string[], + // totalWorkload is the sum of service latency of all statements listed on the table. totalWorkload: number, nodeRegions: { [nodeId: string]: string }, statType: StatisticType, + isTenant: boolean, + hasViewActivityRedactedRole: boolean, + search?: string, + activateDiagnosticsRef?: React.RefObject, + onSelectDiagnosticsReportDropdownOption?: ( + report: StatementDiagnosticsReport, + ) => void, + onStatementClick?: (statement: string) => void, ): ColumnDescriptor[] { const defaultBarChartOptions = { classes: { @@ -88,7 +161,13 @@ function makeCommonColumns( ); const retryBar = retryBarChart(statements, defaultBarChartOptions); - return [ + const columns: ColumnDescriptor[] = [ + makeStatementFingerprintColumn( + statType, + selectedApps, + search, + onStatementClick, + ), { name: "executionCount", title: statisticsTableTitles.executionCount(statType), @@ -209,98 +288,6 @@ function makeCommonColumns( showByDefault: false, }, ]; -} - -export interface AggregateStatistics { - aggregatedFingerprintID: string; - aggregatedFingerprintHexID: string; - // label is either shortStatement (StatementsPage) or nodeId (StatementDetails). - label: string; - // summary exists only for SELECT/INSERT/UPSERT/UPDATE statements, and is - // replaced with shortStatement otherwise. - summary: string; - aggregatedTs: number; - aggregationInterval: number; - implicitTxn: boolean; - fullScan: boolean; - database: string; - applicationName: string; - stats: StatementStatistics; - drawer?: boolean; - firstCellBordered?: boolean; - diagnosticsReports?: StatementDiagnosticsReport[]; - // totalWorkload is the sum of service latency of all statements listed on the table. - totalWorkload?: Long; - regionNodes?: string[]; -} - -export class StatementsSortedTable extends SortedTable {} - -export function shortStatement( - summary: StatementSummary, - original: string, -): string { - switch (summary.statement) { - case "update": - return "UPDATE " + summary.table; - case "insert": - return "INSERT INTO " + summary.table; - case "select": - return "SELECT FROM " + summary.table; - case "delete": - return "DELETE FROM " + summary.table; - case "create": - return "CREATE TABLE " + summary.table; - case "set": - return "SET " + summary.table; - default: - return original; - } -} - -export function makeStatementFingerprintColumn( - statType: StatisticType, - selectedApps: string[], - search?: string, - onStatementClick?: (statement: string) => void, -): ColumnDescriptor { - return { - name: "statements", - title: statisticsTableTitles.statements(statType), - className: cx("cl-table__col-query-text"), - cell: StatementTableCell.statements(search, selectedApps, onStatementClick), - sort: stmt => stmt.label, - alwaysShow: true, - }; -} - -export function makeStatementsColumns( - statements: AggregateStatistics[], - selectedApps: string[], - // totalWorkload is the sum of service latency of all statements listed on the table. - totalWorkload: number, - nodeRegions: { [nodeId: string]: string }, - statType: StatisticType, - isTenant: boolean, - hasViewActivityRedactedRole: boolean, - search?: string, - activateDiagnosticsRef?: React.RefObject, - onSelectDiagnosticsReportDropdownOption?: ( - report: StatementDiagnosticsReport, - ) => void, - onStatementClick?: (statement: string) => void, -): ColumnDescriptor[] { - const columns: ColumnDescriptor[] = [ - makeStatementFingerprintColumn( - statType, - selectedApps, - search, - onStatementClick, - ), - ]; - columns.push( - ...makeCommonColumns(statements, totalWorkload, nodeRegions, statType), - ); if (activateDiagnosticsRef && !isTenant && !hasViewActivityRedactedRole) { const diagnosticsColumn: ColumnDescriptor = { From ef3578a06567cfb8235e45069eead33d5583a3eb Mon Sep 17 00:00:00 2001 From: Matthew Todd Date: Tue, 15 Nov 2022 12:10:44 -0500 Subject: [PATCH 3/5] ui: inline makeStatementFingerprintColumn It only had one caller, and this change brings it together with its peers. Release note: None --- .../src/statementsTable/statementsTable.tsx | 30 +++++-------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx index 79e235c0bb34..aa5de77bc44a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx @@ -99,22 +99,6 @@ export function shortStatement( } } -export function makeStatementFingerprintColumn( - statType: StatisticType, - selectedApps: string[], - search?: string, - onStatementClick?: (statement: string) => void, -): ColumnDescriptor { - return { - name: "statements", - title: statisticsTableTitles.statements(statType), - className: cx("cl-table__col-query-text"), - cell: StatementTableCell.statements(search, selectedApps, onStatementClick), - sort: stmt => stmt.label, - alwaysShow: true, - }; -} - export function makeStatementsColumns( statements: AggregateStatistics[], selectedApps: string[], @@ -162,12 +146,14 @@ export function makeStatementsColumns( const retryBar = retryBarChart(statements, defaultBarChartOptions); const columns: ColumnDescriptor[] = [ - makeStatementFingerprintColumn( - statType, - selectedApps, - search, - onStatementClick, - ), + { + name: "statements", + title: statisticsTableTitles.statements(statType), + className: cx("cl-table__col-query-text"), + cell: StatementTableCell.statements(search, selectedApps, onStatementClick), + sort: stmt => stmt.label, + alwaysShow: true, + }, { name: "executionCount", title: statisticsTableTitles.executionCount(statType), From 8fbf453a1461e5725ef841e44971be71a67d2da2 Mon Sep 17 00:00:00 2001 From: Matthew Todd Date: Tue, 15 Nov 2022 12:13:49 -0500 Subject: [PATCH 4/5] ui: remove unused nodeRegions parameter Release note: None --- .../workspaces/cluster-ui/src/statementsPage/statementsPage.tsx | 1 - .../cluster-ui/src/statementsTable/statementsTable.stories.tsx | 2 -- .../cluster-ui/src/statementsTable/statementsTable.tsx | 1 - .../cluster-ui/src/transactionDetails/transactionDetails.tsx | 1 - 4 files changed, 5 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx index 66025d122043..2e3e2820658b 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx @@ -587,7 +587,6 @@ export class StatementsPage extends React.Component< statements, filters.app.split(","), totalWorkload, - nodeRegions, "statement", isTenant, hasViewActivityRedactedRole, diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.stories.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.stories.tsx index 49bbed4ee494..602bed6f39d8 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.stories.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.stories.tsx @@ -30,7 +30,6 @@ storiesOf("StatementsSortedTable", module) statements, ["$ internal"], calculateTotalWorkload(statements), - { "1": "gcp-europe-west1", "2": "gcp-us-east1", "3": "gcp-us-west1" }, "statement", false, false, @@ -55,7 +54,6 @@ storiesOf("StatementsSortedTable", module) statements, ["$ internal"], calculateTotalWorkload(statements), - { "1": "gcp-europe-west1", "2": "gcp-us-east1", "3": "gcp-us-west1" }, "statement", false, true, diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx index aa5de77bc44a..158c4fdfe6de 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx @@ -104,7 +104,6 @@ export function makeStatementsColumns( selectedApps: string[], // totalWorkload is the sum of service latency of all statements listed on the table. totalWorkload: number, - nodeRegions: { [nodeId: string]: string }, statType: StatisticType, isTenant: boolean, hasViewActivityRedactedRole: boolean, diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx index f98aace149bb..f6a836417da2 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx @@ -472,7 +472,6 @@ export class TransactionDetails extends React.Component< aggregatedStatements, [], calculateTotalWorkload(aggregatedStatements), - nodeRegions, "transactionDetails", isTenant, hasViewActivityRedactedRole, From 8a9068e134dec481b4642389e2e906509cc8a976 Mon Sep 17 00:00:00 2001 From: Matthew Todd Date: Tue, 15 Nov 2022 17:36:54 -0500 Subject: [PATCH 5/5] ui: fix lint warnings. Release note: None --- .../cluster-ui/src/statementsPage/statementsPage.tsx | 2 +- .../cluster-ui/src/statementsTable/statementsTable.tsx | 6 +++++- .../cluster-ui/src/statsTableUtil/statsTableUtil.tsx | 10 +++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx index 2e3e2820658b..106dfaab1f48 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx @@ -57,7 +57,7 @@ import { import { ISortedTablePagination } from "../sortedtable"; import styles from "./statementsPage.module.scss"; import { EmptyStatementsPlaceholder } from "./emptyStatementsPlaceholder"; -import { cockroach, google } from "@cockroachlabs/crdb-protobuf-client"; +import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; import { InlineAlert } from "@cockroachlabs/ui-components"; import sortableTableStyles from "src/sortedtable/sortedtable.module.scss"; import ColumnsSelector from "../columnsSelector/columnsSelector"; diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx index 158c4fdfe6de..df1d33fb07f3 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx @@ -149,7 +149,11 @@ export function makeStatementsColumns( name: "statements", title: statisticsTableTitles.statements(statType), className: cx("cl-table__col-query-text"), - cell: StatementTableCell.statements(search, selectedApps, onStatementClick), + cell: StatementTableCell.statements( + search, + selectedApps, + onStatementClick, + ), sort: stmt => stmt.label, alwaysShow: true, }, diff --git a/pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx b/pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx index 7506166aab4f..a6f162dd529f 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx @@ -363,7 +363,7 @@ export const statisticsTableTitles: StatisticTableTitleType = { ); }, - transactions: (statType: StatisticType) => { + transactions: () => { return ( { - let columnLabel = ""; let contentModifier = ""; let fingerprintModifier = ""; switch (statType) { case "transaction": contentModifier = contentModifiers.transactions; - columnLabel = contentModifiers.transactionCapital; break; case "statement": contentModifier = contentModifiers.statements; - columnLabel = contentModifiers.statementCapital; break; case "transactionDetails": - columnLabel = contentModifiers.statementCapital; contentModifier = contentModifiers.statements; fingerprintModifier = " for this " + contentModifiers.transactionFingerprint; @@ -809,7 +805,7 @@ export const statisticsTableTitles: StatisticTableTitleType = { ); }, - diagnostics: (statType: StatisticType) => { + diagnostics: () => { return ( ); }, - statementsCount: (statType: StatisticType) => { + statementsCount: () => { return (