ui: add proper checks and message for insights

Previously, no checks were done on insights when user
was not admin, making the page show empty results, even
when there was an error retrieving the data.
This commit passes on the proper error message when there
is one. It also checks for admin privilege to show
option to apply a recommendation.

Epic: CRDB-20388

Release note (ui change): Hide apply option for index
recommendation when user is not admin.

pkg/ui/workspaces/cluster-ui/src/api/insightsApi.ts

@@ -13,6 +13,7 @@ import {
   INTERNAL_SQL_API_APP,
   LARGE_RESULT_SIZE,
   LONG_TIMEOUT,
+  sqlApiErrorMessage,
   SqlExecutionRequest,
   SqlExecutionResponse,
   sqlResultsAreEmpty,
@@ -229,6 +230,13 @@ export async function getTxnInsightEvents(
     await executeInternalSql<TransactionContentionResponseColumns>(
       makeInsightsSqlRequest([txnContentionQuery(req)]),
     );
+  if (contentionResults.error) {
+    throw new Error(
+      `Error while retrieving contention information: ${sqlApiErrorMessage(
+        contentionResults.error.message,
+      )}`,
+    );
+  }
   if (sqlResultsAreEmpty(contentionResults)) {
     return [];
   }
@@ -245,6 +253,13 @@ export async function getTxnInsightEvents(
         txnStmtFingerprintsQuery(Array.from(txnFingerprintIDs)),
       ]),
     );
+  if (txnStmtFingerprintResults.error) {
+    throw new Error(
+      `Error while retrieving statements information: ${sqlApiErrorMessage(
+        txnStmtFingerprintResults.error.message,
+      )}`,
+    );
+  }
   if (sqlResultsAreEmpty(txnStmtFingerprintResults)) {
     return [];
   }
@@ -264,6 +279,13 @@ export async function getTxnInsightEvents(
     await executeInternalSql<FingerprintStmtsResponseColumns>(
       fingerprintStmtsRequest,
     );
+  if (fingerprintStmtResults.error) {
+    throw new Error(
+      `Error while retrieving statements information: ${sqlApiErrorMessage(
+        fingerprintStmtResults.error.message,
+      )}`,
+    );
+  }
 
   return buildTxnContentionInsightEvents(
     contentionEvents,
@@ -291,7 +313,8 @@ function buildTxnContentionInsightEvents(
     queries: txnRow.queryIDs.map(stmtID => fingerprintToQuery.get(stmtID)),
   }));
 
-  const res = txnContentionState
+  // remove null entries
+  return txnContentionState
     .map(txnContention => {
       const txnQueries = txnsWithStmtQueries.find(
         txn => txn.fingerprintID === txnContention.transactionFingerprintID,
@@ -307,9 +330,7 @@ function buildTxnContentionInsightEvents(
         execType: InsightExecEnum.TRANSACTION,
       };
     })
-    .filter(txn => txn); // remove null entries
-
-  return res;
+    .filter(txn => txn);
 }
 
 // Transaction insight details.
@@ -475,6 +496,13 @@ export async function getTransactionInsightEventDetailsState(
     await executeInternalSql<TxnContentionDetailsResponseColumns>(
       makeInsightsSqlRequest([txnContentionDetailsQuery(req)]),
     );
+  if (contentionResults.error) {
+    throw new Error(
+      `Error while retrieving contention information: ${sqlApiErrorMessage(
+        contentionResults.error.message,
+      )}`,
+    );
+  }
   if (sqlResultsAreEmpty(contentionResults)) {
     return;
   }
@@ -494,6 +522,13 @@ export async function getTransactionInsightEventDetailsState(
     await executeInternalSql<TxnStmtFingerprintsResponseColumns>(
       makeInsightsSqlRequest([txnStmtFingerprintsQuery(txnFingerprintIDs)]),
     );
+  if (getStmtFingerprintsResponse.error) {
+    throw new Error(
+      `Error while retrieving statements information: ${sqlApiErrorMessage(
+        getStmtFingerprintsResponse.error.message,
+      )}`,
+    );
+  }
   const txnsWithStmtFingerprints = formatTxnFingerprintsResults(
     getStmtFingerprintsResponse,
   );
@@ -509,6 +544,13 @@ export async function getTransactionInsightEventDetailsState(
         fingerprintStmtsQuery(Array.from(stmtFingerprintIDs)),
       ]),
     );
+  if (stmtQueriesResponse.error) {
+    throw new Error(
+      `Error while retrieving statements information: ${sqlApiErrorMessage(
+        stmtQueriesResponse.error.message,
+      )}`,
+    );
+  }
 
   return buildTxnContentionInsightDetails(
     contentionDetails,
@@ -774,7 +816,7 @@ export type ExecutionInsights = TxnInsightEvent[];
 
 export type ExecutionInsightsRequest = Pick<QueryFilterFields, "start" | "end">;
 
-export function getClusterInsightsApi(
+export async function getClusterInsightsApi(
   req?: ExecutionInsightsRequest,
 ): Promise<ExecutionInsights> {
   const insightsQuery = workloadInsightsQuery(req);
@@ -785,12 +827,20 @@ export function getClusterInsightsApi(
       },
     ],
     execute: true,
-    max_result_size: LARGE_RESULT_SIZE,
+    // max_result_size: LARGE_RESULT_SIZE,
     timeout: LONG_TIMEOUT,
   };
-  return executeInternalSql<ExecutionInsightsResponseRow>(request).then(
-    result => {
-      return insightsQuery.toState(result);
-    },
+
+  const result = await executeInternalSql<ExecutionInsightsResponseRow>(
+    request,
   );
+  if (result.error) {
+    throw new Error(
+      `Error while retrieving insights information: ${sqlApiErrorMessage(
+        result.error.message,
+      )}`,
+    );
+  }
+
+  return insightsQuery.toState(result);
 }

pkg/ui/workspaces/cluster-ui/src/api/schemaInsightsApi.ts

@@ -14,6 +14,7 @@ import {
   executeInternalSql,
   LONG_TIMEOUT,
   sqlResultsAreEmpty,
+  sqlApiErrorMessage,
 } from "./sqlApi";
 import {
   InsightRecommendation,
@@ -180,29 +181,34 @@ const schemaInsightQueries: SchemaInsightQuery<SchemaInsightResponse>[] = [
 
 // getSchemaInsights makes requests over the SQL API and transforms the corresponding
 // SQL responses into schema insights.
-export function getSchemaInsights(): Promise<InsightRecommendation[]> {
+export async function getSchemaInsights(): Promise<InsightRecommendation[]> {
   const request: SqlExecutionRequest = {
     statements: schemaInsightQueries.map(insightQuery => ({
       sql: insightQuery.query,
     })),
     execute: true,
     timeout: LONG_TIMEOUT,
   };
-  return executeInternalSql<SchemaInsightResponse>(request).then(result => {
-    const results: InsightRecommendation[] = [];
-    if (sqlResultsAreEmpty(result)) {
-      // No data.
-      return results;
-    }
-
-    result.execution.txn_results.map(txn_result => {
-      // Note: txn_result.statement values begin at 1, not 0.
-      const insightQuery: SchemaInsightQuery<SchemaInsightResponse> =
-        schemaInsightQueries[txn_result.statement - 1];
-      if (txn_result.rows) {
-        results.push(...insightQuery.toSchemaInsight(txn_result));
-      }
-    });
+  const result = await executeInternalSql<SchemaInsightResponse>(request);
+  if (result.error) {
+    throw new Error(
+      `Error while retrieving insights information: ${sqlApiErrorMessage(
+        result.error.message,
+      )}`,
+    );
+  }
+  const results: InsightRecommendation[] = [];
+  if (sqlResultsAreEmpty(result)) {
+    // No data.
     return results;
+  }
+  result.execution.txn_results.map(txn_result => {
+    // Note: txn_result.statement values begin at 1, not 0.
+    const insightQuery: SchemaInsightQuery<SchemaInsightResponse> =
+      schemaInsightQueries[txn_result.statement - 1];
+    if (txn_result.rows) {
+      results.push(...insightQuery.toSchemaInsight(txn_result));
+    }
   });
+  return results;
 }

pkg/ui/workspaces/cluster-ui/src/api/sqlApi.ts

@@ -119,3 +119,25 @@ export function sqlResultsAreEmpty(
     )
   );
 }
+
+/**
+ * errorMessage cleans the error message returned by the sqlApi,
+ * removing information not useful for the user.
+ * e.g. the error message
+ * "$executing stmt 1: run-query-via-api: only users with either MODIFYCLUSTERSETTING
+ * or VIEWCLUSTERSETTING privileges are allowed to show cluster settings"
+ * became
+ * "only users with either MODIFYCLUSTERSETTING or VIEWCLUSTERSETTING privileges are allowed to show cluster settings"
+ * and the error message
+ * "executing stmt 1: max result size exceeded"
+ * became
+ * "max result size exceeded"
+ * @param message
+ */
+export function sqlApiErrorMessage(message: string): string {
+  message = message.replace("run-query-via-api: ", "");
+  if (message.includes(":")) {
+    return message.split(":")[1];
+  }
+  return message;
+}

pkg/ui/workspaces/cluster-ui/src/insights/insightsErrorComponent.tsx

@@ -11,20 +11,29 @@
 import React from "react";
 import classNames from "classnames/bind";
 import styles from "./workloadInsights/util/workloadInsights.module.scss";
+import { sqlApiErrorMessage } from "../api";
 
 const cx = classNames.bind(styles);
 
-export const InsightsError = (): React.ReactElement => (
-  <div className={cx("row")}>
-    <span>This page had an unexpected error while loading insights.</span>
-    &nbsp;
-    <a
-      className={cx("action")}
-      onClick={() => {
-        window.location.reload();
-      }}
-    >
-      Reload this page
-    </a>
-  </div>
-);
+export const InsightsError = (errMsg?: string): React.ReactElement => {
+  const message = errMsg
+    ? errMsg
+    : "This page had an unexpected error while loading insights.";
+  const showReload = !message.toLowerCase().includes("size exceeded");
+  return (
+    <div className={cx("row")}>
+      <span>{message}</span>
+      &nbsp;
+      {showReload && (
+        <a
+          className={cx("action")}
+          onClick={() => {
+            window.location.reload();
+          }}
+        >
+          Reload this page
+        </a>
+      )}
+    </div>
+  );
+};

pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsights.fixture.ts

@@ -70,7 +70,9 @@ export const SchemaInsightsPropsFixture: SchemaInsightsViewProps = {
     database: "",
     schemaInsightType: "",
   },
+  hasAdminRole: true,
   refreshSchemaInsights: () => {},
   onSortChange: () => {},
   onFiltersChange: () => {},
+  refreshUserSQLRoles: () => {},
 };

pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsightsPageConnected.tsx

@@ -19,7 +19,7 @@ import {
   selectFilters,
   selectSortSetting,
 } from "src/store/schemaInsights";
-import { AppState } from "src/store";
+import { AppState, uiConfigActions } from "src/store";
 import {
   SchemaInsightsView,
   SchemaInsightsViewDispatchProps,
@@ -29,6 +29,7 @@ import { SchemaInsightEventFilters } from "../types";
 import { SortSetting } from "src/sortedtable";
 import { actions as localStorageActions } from "../../store/localStorage";
 import { Dispatch } from "redux";
+import { selectHasAdminRole } from "../../store/uiConfig";
 
 const mapStateToProps = (
   state: AppState,
@@ -40,6 +41,7 @@ const mapStateToProps = (
   schemaInsightsError: selectSchemaInsightsError(state),
   filters: selectFilters(state),
   sortSetting: selectSortSetting(state),
+  hasAdminRole: selectHasAdminRole(state),
 });
 
 const mapDispatchToProps = (
@@ -64,6 +66,7 @@ const mapDispatchToProps = (
   refreshSchemaInsights: () => {
     dispatch(actions.refresh());
   },
+  refreshUserSQLRoles: () => dispatch(uiConfigActions.refreshUserSQLRoles()),
 });
 
 export const SchemaInsightsPageConnected = withRouter(

pkg/ui/workspaces/cluster-ui/src/insights/schemaInsights/schemaInsightsView.tsx

@@ -48,12 +48,14 @@ export type SchemaInsightsViewStateProps = {
   schemaInsightsError: Error | null;
   filters: SchemaInsightEventFilters;
   sortSetting: SortSetting;
+  hasAdminRole: boolean;
 };
 
 export type SchemaInsightsViewDispatchProps = {
   onFiltersChange: (filters: SchemaInsightEventFilters) => void;
   onSortChange: (ss: SortSetting) => void;
   refreshSchemaInsights: () => void;
+  refreshUserSQLRoles: () => void;
 };
 
 export type SchemaInsightsViewProps = SchemaInsightsViewStateProps &
@@ -68,7 +70,9 @@ export const SchemaInsightsView: React.FC<SchemaInsightsViewProps> = ({
   schemaInsightsTypes,
   schemaInsightsError,
   filters,
+  hasAdminRole,
   refreshSchemaInsights,
+  refreshUserSQLRoles,
   onFiltersChange,
   onSortChange,
 }: SchemaInsightsViewProps) => {
@@ -91,6 +95,15 @@ export const SchemaInsightsView: React.FC<SchemaInsightsViewProps> = ({
     };
   }, [refreshSchemaInsights]);
 
+  useEffect(() => {
+    // Refresh every 5 minutes.
+    refreshUserSQLRoles();
+    const interval = setInterval(refreshUserSQLRoles, 60 * 5000);
+    return () => {
+      clearInterval(interval);
+    };
+  }, [refreshUserSQLRoles]);
+
   useEffect(() => {
     // We use this effect to sync settings defined on the URL (sort, filters),
     // with the redux store. The only time we do this is when the user navigates
@@ -205,7 +218,7 @@ export const SchemaInsightsView: React.FC<SchemaInsightsViewProps> = ({
           loading={schemaInsights === null}
           page="schema insights"
           error={schemaInsightsError}
-          renderError={() => InsightsError()}
+          renderError={() => InsightsError(schemaInsightsError?.message)}
         >
           <div>
             <section className={sortableTableCx("cl-table-container")}>
@@ -220,7 +233,12 @@ export const SchemaInsightsView: React.FC<SchemaInsightsViewProps> = ({
                 />
               </div>
               <InsightsSortedTable
-                columns={makeInsightsColumns(isCockroachCloud)}
+                columns={makeInsightsColumns(
+                  isCockroachCloud,
+                  hasAdminRole,
+                  true,
+                  false,
+                )}
                 data={filteredSchemaInsights}
                 sortSetting={sortSetting}
                 onChangeSortSetting={onChangeSortSetting}