diff --git a/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts index 9bde8db1fc9f..64657f8f43d5 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts @@ -35,7 +35,7 @@ export type StatementDetailsResponseWithKey = { key: string; }; -export type SqlStatsResponse = cockroach.server.serverpb.StatementsResponse; +export type SqlStatsResponse = cockroach.server.serverpb.IStatementsResponse; export const SqlStatsSortOptions = cockroach.server.serverpb.StatsSortOptions; export type SqlStatsSortType = cockroach.server.serverpb.StatsSortOptions; diff --git a/pkg/ui/workspaces/cluster-ui/src/api/testUtils.tsx b/pkg/ui/workspaces/cluster-ui/src/api/testUtils.tsx index f3c3f9ca2119..791e5b1184bd 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/testUtils.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/api/testUtils.tsx @@ -13,7 +13,7 @@ import Long from "long"; export type Stmt = cockroach.server.serverpb.StatementsResponse.ICollectedStatementStatistics; -type Txn = +export type Txn = cockroach.server.serverpb.StatementsResponse.IExtendedCollectedTransactionStatistics; type ILatencyInfo = cockroach.sql.ILatencyInfo; @@ -27,6 +27,7 @@ const latencyInfo: Required = { const baseStmt: Partial = { id: Long.fromInt(11871906682067483964), + txn_fingerprint_ids: [Long.fromInt(1)], key: { key_data: { query: "SELECT node_id FROM system.statement_statistics", @@ -193,13 +194,18 @@ const assignObjectPropsIfExists = ( overrides: Partial, ): T => { const copiedObj: T = { ...baseObj }; + for (const prop in baseObj) { if (overrides[prop] === undefined) { continue; } const val = copiedObj[prop]; - if (typeof val === "object") { + if ( + typeof val === "object" && + !Array.isArray(val) && + overrides[prop] != null + ) { copiedObj[prop] = assignObjectPropsIfExists( val as Record, overrides[prop] as Record, diff --git a/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.reducer.ts b/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.reducer.ts index d3b6ed1c5507..bcffd6201acf 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.reducer.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.reducer.ts @@ -11,27 +11,15 @@ import { createSlice, PayloadAction } from "@reduxjs/toolkit"; import { DOMAIN_NAME } from "../utils"; import { StatementsRequest } from "src/api/statementsApi"; -import { TimeScale } from "../../timeScaleDropdown"; import moment from "moment-timezone"; import { StatementsResponse } from "../sqlStats"; +import { createInitialState, RequestState } from "src/api/types"; -export type TxnStatsState = { - // Note that we request transactions from the - // statements api, hence the StatementsResponse type here. - data: StatementsResponse; - inFlight: boolean; - lastError: Error; - valid: boolean; - lastUpdated: moment.Moment | null; -}; +// Note that we request transactions from the +// statements api, hence the StatementsResponse type here. +export type TxnStatsState = RequestState; -const initialState: TxnStatsState = { - data: null, - inFlight: false, - lastError: null, - valid: false, - lastUpdated: null, -}; +const initialState = createInitialState(); const txnStatsSlice = createSlice({ name: `${DOMAIN_NAME}/txnStats`, @@ -41,13 +29,13 @@ const txnStatsSlice = createSlice({ state.inFlight = false; state.data = action.payload; state.valid = true; - state.lastError = null; + state.error = null; state.lastUpdated = moment.utc(); }, failed: (state, action: PayloadAction) => { state.inFlight = false; state.valid = false; - state.lastError = action.payload; + state.error = action.payload; state.lastUpdated = moment.utc(); }, invalidated: state => { diff --git a/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.sagas.spec.ts b/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.sagas.spec.ts index 09dcad9d55ad..89340f56593a 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.sagas.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.sagas.spec.ts @@ -75,7 +75,7 @@ describe("txnStats sagas", () => { .hasFinalState({ inFlight: false, data: txnStatsResponse, - lastError: null, + error: null, valid: true, lastUpdated, }) @@ -91,7 +91,7 @@ describe("txnStats sagas", () => { .hasFinalState({ inFlight: false, data: null, - lastError: error, + error: error, valid: false, lastUpdated, }) diff --git a/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.selector.ts b/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.selector.ts deleted file mode 100644 index b232cdd18ce4..000000000000 --- a/pkg/ui/workspaces/cluster-ui/src/store/transactionStats/txnStats.selector.ts +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright 2023 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -import { createSelector } from "reselect"; -import { adminUISelector } from "../utils/selectors"; - -export const txnStatsSelector = createSelector( - adminUISelector, - adminUiState => adminUiState?.transactions, -); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx index 6146870573e0..0ea5856d5ff2 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx @@ -29,10 +29,7 @@ import { baseHeadingClasses } from "../transactionsPage/transactionsPageClasses" import { Button } from "../button"; import { tableClasses } from "../transactionsTable/transactionsTableClasses"; import { SqlBox } from "../sql"; -import { - aggregateStatements, - statementFingerprintIdsToText, -} from "../transactionsPage/utils"; +import { aggregateStatements } from "../transactionsPage/utils"; import { Loading } from "../loading"; import { SummaryCard, SummaryCardItem } from "../summaryCard"; import { @@ -63,6 +60,8 @@ import Long from "long"; import { createCombinedStmtsRequest, InsightRecommendation, + RequestState, + SqlStatsResponse, StatementsRequest, TxnInsightsRequest, } from "../api"; @@ -79,7 +78,6 @@ import { timeScaleRangeToObj, toRoundedDateRange, } from "../timeScaleDropdown"; -import moment from "moment-timezone"; import timeScaleStyles from "../timeScaleDropdown/timeScale.module.scss"; import insightTableStyles from "../insightsTable/insightsTable.module.scss"; @@ -90,6 +88,11 @@ import { import { CockroachCloudContext } from "../contexts"; import { SqlStatsSortType } from "src/api/statementsApi"; import { FormattedTimescale } from "../timeScaleDropdown/formattedTimeScale"; +import { + getStatementsForTransaction, + getTxnFromSqlStatsMemoized, + getTxnQueryString, +} from "./transactionDetailsUtils"; const { containerClass } = tableClasses; const cx = classNames.bind(statementsStyles); const timeScaleStylesCx = classNames.bind(timeScaleStyles); @@ -105,18 +108,13 @@ export interface TransactionDetailsStateProps { timeScale: TimeScale; limit: number; reqSortSetting: SqlStatsSortType; - error?: Error | null; isTenant: UIConfigState["isTenant"]; hasViewActivityRedactedRole?: UIConfigState["hasViewActivityRedactedRole"]; nodeRegions: { [nodeId: string]: string }; - statements?: Statement[]; - transaction: Transaction; transactionFingerprintId: string; - isLoading: boolean; - lastUpdated: moment.Moment | null; transactionInsights: TxnInsightEvent[]; hasAdminRole?: UIConfigState["hasAdminRole"]; - isDataValid: boolean; + txnStatsResp: RequestState; } export interface TransactionDetailsDispatchProps { @@ -135,6 +133,8 @@ interface TState { sortSetting: SortSetting; pagination: ISortedTablePagination; latestTransactionText: string; + txnDetails: Transaction | null; + statements: Statement[] | null; } function statementsRequestFromProps( @@ -155,6 +155,18 @@ export class TransactionDetails extends React.Component< > { constructor(props: TransactionDetailsProps) { super(props); + + const txnDetails = getTxnFromSqlStatsMemoized( + this.props.txnStatsResp?.data, + this.props.match, + this.props.location, + ); + + const stmts = getStatementsForTransaction( + txnDetails, + this.props.txnStatsResp?.data?.statements, + ); + this.state = { sortSetting: { // Sort by statement latency as default column. @@ -165,7 +177,9 @@ export class TransactionDetails extends React.Component< pageSize: 10, current: 1, }, - latestTransactionText: this.getTxnQueryString(), + latestTransactionText: getTxnQueryString(txnDetails, stmts), + txnDetails, + statements: stmts, }; // In case the user selected a option not available on this page, @@ -177,33 +191,33 @@ export class TransactionDetails extends React.Component< } } - getTxnQueryString = (): string => { - const { transaction } = this.props; - - const statementFingerprintIds = - transaction?.stats_data?.statement_fingerprint_ids; + setTxnDetails = (): void => { + const txnDetails = getTxnFromSqlStatsMemoized( + this.props.txnStatsResp?.data, + this.props.match, + this.props.location, + ); - return ( - (statementFingerprintIds && - statementFingerprintIdsToText( - statementFingerprintIds, - this.getStatementsForTransaction(), - )) ?? - "" + const statements = getStatementsForTransaction( + txnDetails, + this.props.txnStatsResp?.data?.statements, ); - }; - setTxnQueryString = (): void => { - const transactionText = this.getTxnQueryString(); + // Only overwrite the transaction text if it is non-null. + const transactionText = + getTxnQueryString(txnDetails, statements) ?? + this.state.latestTransactionText; // If a new, non-empty-string transaction text is available (derived from the time-frame-specific endpoint // response), cache the text. if ( - transactionText && - transactionText !== this.state.latestTransactionText + transactionText !== this.state.latestTransactionText || + txnDetails !== this.state.txnDetails ) { this.setState({ latestTransactionText: transactionText, + txnDetails, + statements, }); } }; @@ -222,7 +236,7 @@ export class TransactionDetails extends React.Component< }; componentDidMount(): void { - if (!this.props.transaction || !this.props.isDataValid) { + if (!this.props.txnStatsResp?.data || !this.props.txnStatsResp?.valid) { this.refreshData(); } this.props.refreshUserSQLRoles(); @@ -239,9 +253,9 @@ export class TransactionDetails extends React.Component< if ( prevProps.transactionFingerprintId !== this.props.transactionFingerprintId || - prevProps.statements !== this.props.statements + prevProps.txnStatsResp !== this.props.txnStatsResp ) { - this.setTxnQueryString(); + this.setTxnDetails(); } if (this.props.timeScale !== prevProps.timeScale) { @@ -265,28 +279,11 @@ export class TransactionDetails extends React.Component< this.props.history.push("/sql-activity?tab=Transactions&view=fingerprints"); }; - getStatementsForTransaction = (): Statement[] => { - const { transaction, statements } = this.props; - - const statementFingerprintIds = - transaction?.stats_data?.statement_fingerprint_ids; - if (!statementFingerprintIds) { - return []; - } - - return ( - statements?.filter( - s => - s.key.key_data.transaction_fingerprint_id.toString() === - this.props.transactionFingerprintId, - ) ?? [] - ); - }; - render(): React.ReactElement { - const { error, nodeRegions, transaction } = this.props; - const { latestTransactionText } = this.state; - const statementsForTransaction = this.getStatementsForTransaction(); + const { nodeRegions } = this.props; + const transaction = this.state.txnDetails; + const error = this.props.txnStatsResp?.error; + const { latestTransactionText, statements } = this.state; const transactionStats = transaction?.stats_data?.stats; const period = ; @@ -324,7 +321,7 @@ export class TransactionDetails extends React.Component< { if (!transaction) { return ( @@ -358,9 +355,7 @@ export class TransactionDetails extends React.Component< } = this.props; const { sortSetting, pagination } = this.state; - const aggregatedStatements = aggregateStatements( - statementsForTransaction, - ); + const aggregatedStatements = aggregateStatements(statements); populateRegionNodeForStatements(aggregatedStatements, nodeRegions); const duration = (v: number) => Duration(v * 1e9); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsConnected.tsx index 8b7dac4bf2db..ffce80391571 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsConnected.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsConnected.tsx @@ -8,9 +8,8 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -import { createSelector } from "@reduxjs/toolkit"; import { connect } from "react-redux"; -import { RouteComponentProps, withRouter } from "react-router-dom"; +import { withRouter } from "react-router-dom"; import { Dispatch } from "redux"; import { AppState, uiConfigActions } from "src/store"; @@ -28,10 +27,6 @@ import { TransactionDetailsProps, TransactionDetailsStateProps, } from "./transactionDetails"; -import { - selectTransactionsData, - selectTransactionsLastError, -} from "../transactionsPage/transactionsPage.selectors"; import { selectIsTenant, selectHasViewActivityRedactedRole, @@ -44,79 +39,26 @@ import { selectTxnsPageReqSort, } from "../store/utils/selectors"; import { StatementsRequest } from "src/api/statementsApi"; -import { - txnFingerprintIdAttr, - getMatchParamByName, - queryByName, - appNamesAttr, - unset, -} from "../util"; +import { txnFingerprintIdAttr, getMatchParamByName } from "../util"; import { TimeScale } from "../timeScaleDropdown"; import { actions as analyticsActions } from "../store/analytics"; -export const selectTransaction = createSelector( - (state: AppState) => state.adminUI?.transactions, - (_state: AppState, props: RouteComponentProps) => props, - (transactionState, props) => { - const transactions = transactionState.data?.transactions; - if (!transactions) { - return { - isLoading: transactionState.inFlight, - transaction: null, - isValid: transactionState.valid, - }; - } - - const apps = queryByName(props.location, appNamesAttr) - ?.split(",") - .map(s => s.trim()); - - const txnFingerprintId = getMatchParamByName( - props.match, - txnFingerprintIdAttr, - ); - - const transaction = transactions.find( - txn => - txn.stats_data.transaction_fingerprint_id.toString() === - txnFingerprintId && - (apps?.length ? apps.includes(txn.stats_data.app ?? unset) : true), - ); - - return { - isLoading: transactionState.inFlight, - transaction: transaction, - lastUpdated: transactionState.lastUpdated, - isValid: transactionState.valid, - }; - }, -); - const mapStateToProps = ( state: AppState, props: TransactionDetailsProps, ): TransactionDetailsStateProps => { - const { isLoading, transaction, lastUpdated, isValid } = selectTransaction( - state, - props, - ); return { timeScale: selectTimeScale(state), - error: selectTransactionsLastError(state), isTenant: selectIsTenant(state), nodeRegions: nodeRegionsByIDSelector(state), - statements: selectTransactionsData(state)?.statements, - transaction, + txnStatsResp: state?.adminUI?.transactions, transactionFingerprintId: getMatchParamByName( props.match, txnFingerprintIdAttr, ), - isLoading: isLoading, - lastUpdated: lastUpdated, hasViewActivityRedactedRole: selectHasViewActivityRedactedRole(state), transactionInsights: selectTxnInsightsByFingerprint(state, props), hasAdminRole: selectHasAdminRole(state), - isDataValid: isValid, limit: selectTxnsPageLimit(state), reqSortSetting: selectTxnsPageReqSort(state), }; diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsUtils.spec.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsUtils.spec.tsx new file mode 100644 index 000000000000..0e4551d22a9f --- /dev/null +++ b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsUtils.spec.tsx @@ -0,0 +1,245 @@ +// Copyright 2023 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +import { + getStatementsForTransaction, + getTxnFromSqlStatsTxns, + getTxnQueryString, +} from "./transactionDetailsUtils"; +import { mockTxnStats, Txn, Stmt, mockStmtStats } from "../api/testUtils"; +import { shuffle } from "lodash"; +import Long from "long"; + +describe("getTxnFromSqlStatsTxns", () => { + //TODO + it("should return the first txn with the fingerprint ID and app name specified", () => { + const app = "cockroach"; + const txns = [ + { id: 1, app: "hello_world" }, + { id: 2, app }, + { id: 3, app: "cockrooch" }, + { id: 3, app }, + { id: 4, app: "my_app" }, + ].map(txn => + mockTxnStats({ + stats_data: { + transaction_fingerprint_id: Long.fromInt(txn.id), + app: txn.app, + }, + }), + ); + + const expectedTxn = txns[3]; + const txn = getTxnFromSqlStatsTxns(txns, "3", [app]); + expect(txn).toEqual(expectedTxn); + }); + + it("should return null if no txn can be found", () => { + const txns = [1, 2, 3, 4, 5, 6].map(txn => + mockTxnStats({ + stats_data: { + transaction_fingerprint_id: Long.fromInt(txn), + app: "uncool_app", + }, + }), + ); + + const txn = getTxnFromSqlStatsTxns(txns, "1", ["cool_app"]); + expect(txn == null).toBe(true); + }); + + it.each([ + [null, null, null], + [null, "123", null], + [null, null, ["app"]], + [[mockTxnStats()], null, null], + [[mockTxnStats()], "123", null], + [[mockTxnStats()], "123", []], + [[mockTxnStats()], null, ["app"]], + [[mockTxnStats()], "", ["app"]], + [null, "123", ["app"]], + ])( + "should return null when given invalid parameters: (%p, %p, %p)", + ( + txns: Txn[] | null, + fingerprintID: string | null, + apps: string[] | null, + ) => { + const txn = getTxnFromSqlStatsTxns(txns, fingerprintID, apps); + expect(txn == null).toBe(true); + }, + ); +}); + +describe("getTxnQueryString", () => { + const extraStmts = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].map(i => + mockStmtStats({ + id: Long.fromInt(i + 100), + txn_fingerprint_ids: [Long.fromInt(9999999999)], + }), + ); + + const queryStrTestCases = [ + { + txnID: 1, + stmtIDs: [4, 5, 6], + queries: ["SELECT 1", "SELECT 2", "SELECT 3"], + }, + { + txnID: 2, + stmtIDs: [2, 11], + queries: ["INSERT INTO foo VALUES (1, 2), (3, 4)", "SELECT * FROM foo"], + }, + { + txnID: 3, + stmtIDs: [8], + queries: ["SELECT * FROM foo"], + }, + { + txnID: 4, + stmtIDs: [3, 5, 7, 9], + queries: ["a", "b", "c", "d"], + }, + ].map(tc => { + const txnID = Long.fromInt(tc.txnID); + + const txn = mockTxnStats({ + stats_data: { + transaction_fingerprint_id: txnID, + statement_fingerprint_ids: tc.stmtIDs.map(id => Long.fromInt(id)), + }, + }); + + // Stub statements to have the test case txn id and appropriate query strings. + const stmts = tc.queries.map((query, i) => + mockStmtStats({ + id: Long.fromInt(tc.stmtIDs[i]), + key: { key_data: { query, transaction_fingerprint_id: txnID } }, + }), + ); + + return [txn, stmts, tc.queries.join("\n")]; + }); + + it.each(queryStrTestCases)( + "should build the full txn query string from the provided txn and stmt list", + (txn: Txn, stmts: Stmt[], expected: string) => { + const txnStr = getTxnQueryString(txn, shuffle([...extraStmts, ...stmts])); + expect(txnStr).toEqual(expected); + }, + ); + + it("builds partial query when there is a stmt missing from the provided list", () => { + const txn = mockTxnStats({ + stats_data: { + transaction_fingerprint_id: Long.fromInt(1), + statement_fingerprint_ids: [Long.fromInt(1), Long.fromInt(2)], + }, + }); + const stmts = [ + mockStmtStats({ + id: Long.fromInt(2), + key: { key_data: { query: "HELLO" } }, + }), + ]; + + const txnStr = getTxnQueryString(txn, stmts); + expect(txnStr).toEqual("\nHELLO"); + }); + + it.each([ + [null, null], + [null, extraStmts], + [mockTxnStats(), null], + [mockTxnStats(), []], + ])( + "should return the empty string when given invalid params", + (txn: Txn, stmts: Stmt[] | null) => { + const txnStr = getTxnQueryString(txn, stmts); + expect(txnStr).toEqual(""); + }, + ); +}); + +describe("getStatementsForTransaction", () => { + const extraStmts = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].map(i => + mockStmtStats({ + id: Long.fromInt(i), + key: { + key_data: { + transaction_fingerprint_id: Long.fromInt(9999999999), + }, + }, + txn_fingerprint_ids: [Long.fromInt(9999999999)], + }), + ); + + const testCases = [ + { + txnID: 1, + stmtIDs: [2, 4, 6, 8], + }, + { + txnID: 2, + stmtIDs: [1], + }, + { + txnID: 3, + stmtIDs: [], + }, + { + txnID: 3, + stmtIDs: [4, 5, 6], + useArrayProp: true, + }, + ].map(tc => { + const txnID = Long.fromInt(tc.txnID); + + const txn = mockTxnStats({ + stats_data: { transaction_fingerprint_id: txnID }, + }); + + const stmts = tc.stmtIDs.map(id => + mockStmtStats({ + id: Long.fromInt(id), + txn_fingerprint_ids: tc.useArrayProp ? [txnID] : null, + key: { + key_data: { + transaction_fingerprint_id: !tc.useArrayProp ? txnID : null, + }, + }, + }), + ); + return [txn, stmts]; + }); + + it.each(testCases)( + "should return the list of stmts that have txn ids matching the provided txn", + (txn: Txn, stmts: Stmt[]) => { + const stmtsRes = getStatementsForTransaction( + txn, + shuffle([...extraStmts, ...stmts]), + ); + + expect(stmtsRes.length).toEqual(stmts.length); + }, + ); + + it.each([ + [null, null], + [mockTxnStats(), null], + [null, []], + ])( + "should return empty array when given invalid params", + (txn: Txn | null, stmts: Stmt[] | null) => { + expect(getStatementsForTransaction(txn, stmts)).toEqual([]); + }, + ); +}); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsUtils.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsUtils.tsx new file mode 100644 index 000000000000..623d2f46644f --- /dev/null +++ b/pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetailsUtils.tsx @@ -0,0 +1,109 @@ +// Copyright 2023 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +import { createSelector } from "@reduxjs/toolkit"; +import { + appNamesAttr, + getMatchParamByName, + queryByName, + txnFingerprintIdAttr, + unset, +} from "../util"; +import { SqlStatsResponse } from "../api"; +import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; +import { match } from "react-router"; +import { Location } from "history"; +import { statementFingerprintIdsToText } from "../transactionsPage/utils"; + +type Transaction = + cockroach.server.serverpb.StatementsResponse.IExtendedCollectedTransactionStatistics; + +type Statement = + cockroach.server.serverpb.StatementsResponse.ICollectedStatementStatistics; + +/** + * getTxnFromSqlStatsTxns returns the txn from the txns list with the + * specified txn fingerprint ID and app anme. + * + * @param txns List of txns to search through. + * @param txnFingerprintID + * @param apps + */ +export const getTxnFromSqlStatsTxns = ( + txns: Transaction[] | null, + txnFingerprintID: string | null, + apps: string[] | null, +): Transaction | null => { + if (!txns?.length || !apps?.length || !txnFingerprintID) { + return null; + } + + return txns.find( + txn => + txn.stats_data.transaction_fingerprint_id.toString() === + txnFingerprintID && + (apps.length ? apps.includes(txn.stats_data.app ?? unset) : true), + ); +}; + +export const getTxnFromSqlStatsMemoized = createSelector( + (resp: SqlStatsResponse, _match: match, _location: Location): Transaction[] => + resp?.transactions ?? [], + (_resp, _match, location): string[] => + queryByName(location, appNamesAttr) + ?.split(",") + .map(s => s.trim()) ?? [], + (_resp, routeMatch, _location: Location): string => + getMatchParamByName(routeMatch, txnFingerprintIdAttr), + (txns, apps, fingerprintID): Transaction => { + return getTxnFromSqlStatsTxns(txns, fingerprintID, apps); + }, +); + +/** + * getTxnQueryString returns the transaction query built from the provided stmt list + * @param txn Transaction to build query for. + * @param stmts List of stmts from which to get query strings. + */ +export const getTxnQueryString = ( + txn: Transaction | null, + stmts: Statement[], +): string => { + if (!txn || !stmts?.length) return ""; + + const statementFingerprintIds = txn?.stats_data?.statement_fingerprint_ids; + + return ( + (statementFingerprintIds && + statementFingerprintIdsToText(statementFingerprintIds, stmts)) ?? + "" + ); +}; + +/** + * getStatementsForTransaction returns the list of stmts with transaction ids + * matching the txn id of the provided txn. + * @param txn Txn for which we will find matching stmts. + * @param stmts List of available stmts. + */ +export const getStatementsForTransaction = ( + txn: Transaction | null, + stmts: Statement[] | null, +): Statement[] => { + if (!txn || !stmts?.length) return []; + + const txnIDString = txn.stats_data?.transaction_fingerprint_id?.toString(); + + return stmts.filter( + s => + s.key.key_data?.transaction_fingerprint_id?.toString() === txnIDString || + s.txn_fingerprint_ids?.map(t => t.toString()).includes(txnIDString), + ); +}; diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.selectors.ts index b5ede47c03ba..b7b24b273d10 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.selectors.ts +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.selectors.ts @@ -11,32 +11,6 @@ import { createSelector } from "reselect"; import { localStorageSelector } from "../store/utils/selectors"; -import { txnStatsSelector } from "../store/transactionStats/txnStats.selector"; - -export const selectTransactionsData = createSelector( - txnStatsSelector, - transactionsState => transactionsState?.data, -); - -export const selectTransactionsDataValid = createSelector( - txnStatsSelector, - state => state?.valid, -); - -export const selectTransactionsDataInFlight = createSelector( - txnStatsSelector, - state => state?.inFlight, -); - -export const selectTransactionsLastUpdated = createSelector( - txnStatsSelector, - state => state.lastUpdated, -); - -export const selectTransactionsLastError = createSelector( - txnStatsSelector, - state => state.lastError, -); export const selectTxnColumns = createSelector( localStorageSelector, diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.stories.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.stories.tsx index 3e5b9dd423c2..4eddfc992b58 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.stories.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.stories.tsx @@ -25,51 +25,68 @@ import { import { TransactionsPage } from "."; import { RequestError } from "../util"; -import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; -import StatsSortOptions = cockroach.server.serverpb.StatsSortOptions; +import { RequestState, SqlStatsResponse, SqlStatsSortOptions } from "../api"; const getEmptyData = () => extend({}, data, { transactions: [], statements: [] }); +const defaultLimitAndSortProps = { + limit: 100, + reqSortSetting: SqlStatsSortOptions.PCT_RUNTIME, + onChangeLimit: noop, + onChangeReqSort: noop, + onApplySearchCriteria: noop, +}; + storiesOf("Transactions Page", module) .addDecorator(storyFn => {storyFn()}) .addDecorator(storyFn => (
{storyFn()}
)) - .add("with data", () => ( - - )) + .add("with data", () => { + const resp: RequestState = { + valid: true, + inFlight: false, + data, + lastUpdated, + error: null, + }; + + return ( + + ); + }) .add("without data", () => { + const resp: RequestState = { + valid: true, + inFlight: false, + data: getEmptyData(), + lastUpdated, + error: null, + }; + return ( ); }) @@ -99,12 +110,19 @@ storiesOf("Transactions Page", module) searchParams.set("q", "aaaaaaa"); history.location.search = searchParams.toString(); + const resp: RequestState = { + valid: true, + inFlight: false, + data: getEmptyData(), + lastUpdated, + error: null, + }; + return ( ); }) .add("with loading indicator", () => { + const resp: RequestState = { + valid: true, + inFlight: true, + data: undefined, + lastUpdated, + error: null, + }; + return ( ); }) .add("with error alert", () => { + const resp: RequestState = { + valid: true, + inFlight: true, + data: undefined, + lastUpdated, + error: new RequestError( + "Forbidden", + 403, + "this operation requires admin privilege", + ), + }; + return ( ); }); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx index 258b17787f6b..9bd6bdd5a254 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx @@ -9,7 +9,6 @@ // licenses/APL.txt. import React from "react"; -import * as protos from "@cockroachlabs/crdb-protobuf-client"; import classNames from "classnames/bind"; import styles from "../statementsPage/statementsPage.module.scss"; import { RouteComponentProps } from "react-router-dom"; @@ -54,6 +53,7 @@ import { createCombinedStmtsRequest, StatementsRequest, SqlStatsSortOptions, + SqlStatsResponse, } from "src/api/statementsApi"; import ColumnsSelector from "../columnsSelector/columnsSelector"; import { SelectOption } from "../multiSelectCheckbox/multiSelectCheckbox"; @@ -73,7 +73,6 @@ import { import { InlineAlert } from "@cockroachlabs/ui-components"; import { TransactionViewType } from "./transactionsPageTypes"; import { isSelectedColumn } from "../columnsSelector/utils"; -import moment from "moment-timezone"; import { STATS_LONG_LOADING_DURATION, txnRequestSortOptions, @@ -85,8 +84,7 @@ import { import { SearchCriteria } from "src/searchCriteria/searchCriteria"; import timeScaleStyles from "../timeScaleDropdown/timeScale.module.scss"; import { FormattedTimescale } from "../timeScaleDropdown/formattedTimeScale"; - -type IStatementsResponse = protos.cockroach.server.serverpb.IStatementsResponse; +import { RequestState } from "../api"; const cx = classNames.bind(styles); const timeScaleStylesCx = classNames.bind(timeScaleStyles); @@ -101,14 +99,10 @@ interface TState { export interface TransactionsPageStateProps { columns: string[]; - data: IStatementsResponse; - isDataValid: boolean; - isReqInFlight: boolean; - lastUpdated: moment.Moment | null; + txnsResp: RequestState; timeScale: TimeScale; limit: number; reqSortSetting: SqlStatsSortType; - error?: Error | null; filters: Filters; isTenant?: UIConfigState["isTenant"]; nodeRegions: { [nodeId: string]: string }; @@ -226,9 +220,9 @@ export class TransactionsPage extends React.Component< if (ts !== this.props.timeScale) { this.changeTimeScale(ts); } else if ( - !this.props.isDataValid || - !this.props.data || - !this.props.lastUpdated + !this.props.txnsResp.valid || + !this.props.txnsResp.data || + !this.props.txnsResp.lastUpdated ) { this.refreshData(); } @@ -385,7 +379,9 @@ export class TransactionsPage extends React.Component< }; lastReset = (): Date => { - return new Date(Number(this.props.data?.last_reset.seconds) * 1000); + return new Date( + Number(this.props.txnsResp?.data?.last_reset.seconds) * 1000, + ); }; changeTimeScale = (ts: TimeScale): void => { @@ -451,7 +447,6 @@ export class TransactionsPage extends React.Component< renderTransactions(): React.ReactElement { const { - data, nodeRegions, isTenant, onColumnsChange, @@ -460,6 +455,7 @@ export class TransactionsPage extends React.Component< search, hasAdminRole, } = this.props; + const data = this.props.txnsResp.data; const { pagination, filters } = this.state; const internal_app_name_prefix = data?.internal_app_name_prefix || ""; const statements = data?.statements || []; @@ -677,20 +673,20 @@ export class TransactionsPage extends React.Component< />
this.renderTransactions()} renderError={() => LoadingError({ statsType: "transactions", - timeout: this.props?.error?.name + timeout: this.props.txnsResp?.error?.name ?.toLowerCase() .includes("timeout"), }) } /> - {this.props.isReqInFlight && longLoadingMessage} + {this.props.txnsResp.inFlight && longLoadingMessage}
); diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx index a117a10f94c0..fc732bfea116 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPageConnected.tsx @@ -21,15 +21,10 @@ import { TransactionsPageDispatchProps, } from "./transactionsPage"; import { - selectTransactionsData, - selectTransactionsLastError, selectTxnColumns, selectSortSetting, selectFilters, selectSearch, - selectTransactionsDataValid, - selectTransactionsLastUpdated, - selectTransactionsDataInFlight, } from "./transactionsPage.selectors"; import { selectHasAdminRole, selectIsTenant } from "../store/uiConfig"; import { nodeRegionsByIDSelector } from "../store/nodes"; @@ -81,12 +76,8 @@ export const TransactionsPageConnected = withRouter( fingerprintsPageProps: { ...props, columns: selectTxnColumns(state), - data: selectTransactionsData(state), - isDataValid: selectTransactionsDataValid(state), - isReqInFlight: selectTransactionsDataInFlight(state), - lastUpdated: selectTransactionsLastUpdated(state), + txnsResp: state.adminUI?.transactions, timeScale: selectTimeScale(state), - error: selectTransactionsLastError(state), filters: selectFilters(state), isTenant: selectIsTenant(state), nodeRegions: nodeRegionsByIDSelector(state), diff --git a/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts b/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts index 2873b7d2ee37..49d2ef7d07aa 100644 --- a/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts +++ b/pkg/ui/workspaces/db-console/src/redux/apiReducers.ts @@ -557,8 +557,8 @@ export interface APIReducersState { sessions: CachedDataReducerState; settings: CachedDataReducerState; stores: KeyedCachedDataReducerState; - statements: CachedDataReducerState; - transactions: CachedDataReducerState; + statements: CachedDataReducerState; + transactions: CachedDataReducerState; statementDetails: KeyedCachedDataReducerState; dataDistribution: CachedDataReducerState; metricMetadata: CachedDataReducerState; diff --git a/pkg/ui/workspaces/db-console/src/selectors/executionFingerprintsSelectors.tsx b/pkg/ui/workspaces/db-console/src/selectors/executionFingerprintsSelectors.tsx deleted file mode 100644 index ae81d31fbf54..000000000000 --- a/pkg/ui/workspaces/db-console/src/selectors/executionFingerprintsSelectors.tsx +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2022 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -import { AdminUIState } from "../redux/state"; - -// Transactions -export const selectTxnsLastUpdated = (state: AdminUIState) => - state.cachedData.transactions?.setAt?.utc(); - -export const selectTxnsDataValid = (state: AdminUIState) => - state.cachedData.transactions?.valid; - -export const selectTxnsDataInFlight = (state: AdminUIState) => - state.cachedData.transactions?.inFlight; diff --git a/pkg/ui/workspaces/db-console/src/views/transactions/transactionDetails.tsx b/pkg/ui/workspaces/db-console/src/views/transactions/transactionDetails.tsx index f2f26389dd26..05d5ef6dbda6 100644 --- a/pkg/ui/workspaces/db-console/src/views/transactions/transactionDetails.tsx +++ b/pkg/ui/workspaces/db-console/src/views/transactions/transactionDetails.tsx @@ -9,8 +9,7 @@ // licenses/APL.txt. import { connect } from "react-redux"; -import { createSelector } from "reselect"; -import { RouteComponentProps, withRouter } from "react-router-dom"; +import { withRouter } from "react-router-dom"; import { refreshNodes, refreshTxns, @@ -18,14 +17,11 @@ import { refreshUserSQLRoles, } from "src/redux/apiReducers"; import { AdminUIState } from "src/redux/state"; -import { appNamesAttr, txnFingerprintIdAttr, unset } from "src/util/constants"; -import { getMatchParamByName, queryByName } from "src/util/query"; import { nodeRegionsByIDSelector } from "src/redux/nodes"; import { reqSortSetting, - selectData, - selectLastError, limitSetting, + selectTxns, } from "src/views/transactions/transactionsPage"; import { TransactionDetailsStateProps, @@ -37,45 +33,8 @@ import { setGlobalTimeScaleAction } from "src/redux/statements"; import { selectTimeScale } from "src/redux/timeScale"; import { selectTxnInsightsByFingerprint } from "src/views/insights/insightsSelectors"; import { selectHasAdminRole } from "src/redux/user"; - -export const selectTransaction = createSelector( - (state: AdminUIState) => state.cachedData.transactions, - (_state: AdminUIState, props: RouteComponentProps) => props, - (transactionState, props) => { - const transactions = transactionState.data?.transactions; - if (!transactions) { - return { - isLoading: transactionState.inFlight, - transaction: null, - lastUpdated: null, - isValid: transactionState.valid, - }; - } - - const apps = queryByName(props.location, appNamesAttr) - ?.split(",") - .map(s => s.trim()); - - const txnFingerprintId = getMatchParamByName( - props.match, - txnFingerprintIdAttr, - ); - - const transaction = transactions.find( - txn => - txn.stats_data.transaction_fingerprint_id.toString() === - txnFingerprintId && - (apps?.length ? apps.includes(txn.stats_data.app ?? unset) : true), - ); - - return { - isLoading: transactionState.inFlight, - transaction: transaction, - lastUpdated: transactionState?.setAt?.utc(), - isValid: transactionState.valid, - }; - }, -); +import { getMatchParamByName } from "src/util/query"; +import { txnFingerprintIdAttr } from "src/util/constants"; export default withRouter( connect( @@ -83,24 +42,17 @@ export default withRouter( state: AdminUIState, props: TransactionDetailsProps, ): TransactionDetailsStateProps => { - const { isLoading, transaction, lastUpdated, isValid } = - selectTransaction(state, props); return { timeScale: selectTimeScale(state), - error: selectLastError(state), isTenant: false, nodeRegions: nodeRegionsByIDSelector(state), - statements: selectData(state)?.statements, - transaction: transaction, + txnStatsResp: selectTxns(state), transactionFingerprintId: getMatchParamByName( props.match, txnFingerprintIdAttr, ), - isLoading: isLoading, - lastUpdated: lastUpdated, transactionInsights: selectTxnInsightsByFingerprint(state, props), hasAdminRole: selectHasAdminRole(state), - isDataValid: isValid, limit: limitSetting.selector(state), reqSortSetting: reqSortSetting.selector(state), }; diff --git a/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx b/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx index a577acf7f6af..9e392a2a4467 100644 --- a/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx +++ b/pkg/ui/workspaces/db-console/src/views/transactions/transactionsPage.tsx @@ -12,6 +12,7 @@ import { connect } from "react-redux"; import { createSelector } from "reselect"; import { RouteComponentProps, withRouter } from "react-router-dom"; import { + createSelectorForCachedDataField, refreshNodes, refreshTxns, refreshUserSQLRoles, @@ -45,23 +46,8 @@ import { mapStateToRecentTransactionsPageProps, } from "./recentTransactionsSelectors"; import { selectTimeScale } from "src/redux/timeScale"; -import { - selectTxnsLastUpdated, - selectTxnsDataValid, - selectTxnsDataInFlight, -} from "src/selectors/executionFingerprintsSelectors"; import { trackApplySearchCriteriaAction } from "src/redux/analyticsActions"; -// selectData returns the array of AggregateStatistics to show on the -// TransactionsPage, based on if the appAttr route parameter is set. -export const selectData = createSelector( - (state: AdminUIState) => state.cachedData.transactions, - (state: CachedDataReducerState) => { - if (!state.data || !state.valid) return null; - return state.data; - }, -); - // selectLastReset returns a string displaying the last time the statement // statistics were reset. export const selectLastReset = createSelector( @@ -75,11 +61,6 @@ export const selectLastReset = createSelector( }, ); -export const selectLastError = createSelector( - (state: AdminUIState) => state.cachedData.transactions, - (state: CachedDataReducerState) => state.lastError, -); - export const sortSettingLocalSetting = new LocalSetting( "sortSetting/TransactionsPage", (state: AdminUIState) => state.localSettings, @@ -116,6 +97,9 @@ export const limitSetting = new LocalSetting( api.DEFAULT_STATS_REQ_OPTIONS.limit, ); +export const selectTxns = + createSelectorForCachedDataField("transactions"); + const fingerprintsPageActions = { refreshData: refreshTxns, refreshNodes, @@ -167,18 +151,13 @@ const TransactionsPageConnected = withRouter( fingerprintsPageProps: { ...props, columns: transactionColumnsLocalSetting.selectorToArray(state), - data: selectData(state), - isDataValid: selectTxnsDataValid(state), - isReqInFlight: selectTxnsDataInFlight(state), - lastUpdated: selectTxnsLastUpdated(state), + txnsResp: selectTxns(state), timeScale: selectTimeScale(state), - error: selectLastError(state), filters: filtersLocalSetting.selector(state), lastReset: selectLastReset(state), nodeRegions: nodeRegionsByIDSelector(state), search: searchLocalSetting.selector(state), sortSetting: sortSettingLocalSetting.selector(state), - statementsError: state.cachedData.transactions.lastError, hasAdminRole: selectHasAdminRole(state), limit: limitSetting.selector(state), reqSortSetting: reqSortSetting.selector(state),