ui/server: add ability to create conditional statement diagnostics

Resolves cockroachdb#57634

Previously, we did not have the ability to create conditional statement
diagnostics in the frontend. This commit adds in the ability to specify
a minimum execution latency and an expiry time when creating a statement
diagnostics request. These changes apply to both DB and CC console.

Since expired requests are not surfaced at all in the frontend, we have
also modified the statement diagnostics API response to not return
already expired and incomplete requests.

Lastly, this commit also deletes some unused code related to statement
diagnostics modals.

Release note (ui change): Add ability to create conditional statement
diagnostics by adding two new fields 1) minimum execution latency, which
specifies the limit for when a statement should be tracked and 2) expiry
time, which specifies when a diagnostics request should expire.

pkg/server/statement_diagnostics_requests.go

@@ -20,6 +20,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
+	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
 )
 
@@ -115,10 +116,11 @@ func (s *statusServer) CancelStatementDiagnosticsReport(
 	return &response, nil
 }
 
-// StatementDiagnosticsRequests retrieves all of the statement
-// diagnostics requests in the `system.statement_diagnostics_requests` table.
+// StatementDiagnosticsRequests retrieves all statement diagnostics
+// requests in the `system.statement_diagnostics_requests` table that
+// have not yet expired.
 func (s *statusServer) StatementDiagnosticsRequests(
-	ctx context.Context, req *serverpb.StatementDiagnosticsReportsRequest,
+	ctx context.Context, _ *serverpb.StatementDiagnosticsReportsRequest,
 ) (*serverpb.StatementDiagnosticsReportsResponse, error) {
 	ctx = propagateGatewayMetadata(ctx)
 	ctx = s.AnnotateCtx(ctx)
@@ -179,6 +181,10 @@ func (s *statusServer) StatementDiagnosticsRequests(
 			}
 			if expiresAt, ok := row[6].(*tree.DTimestampTZ); ok {
 				req.ExpiresAt = expiresAt.Time
+				// Don't return already expired requests.
+				if req.ExpiresAt.Before(timeutil.Now()) {
+					continue
+				}
 			}
 		}
 

pkg/server/status_test.go

@@ -2446,6 +2446,50 @@ func TestStatementDiagnosticsCompleted(t *testing.T) {
 	}
 }
 
+func TestStatementDiagnosticsDoesNotReturnExpiredRequests(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{})
+	defer s.Stopper().Stop(context.Background())
+	db := sqlutils.MakeSQLRunner(sqlDB)
+
+	statementFingerprint := "INSERT INTO test VALUES (_)"
+	expiresAfter := 5 * time.Millisecond
+
+	// Create statement diagnostics request with defined expiry time.
+	req := &serverpb.CreateStatementDiagnosticsReportRequest{
+		StatementFingerprint: statementFingerprint,
+		MinExecutionLatency:  500 * time.Millisecond,
+		ExpiresAfter:         expiresAfter,
+	}
+	var resp serverpb.CreateStatementDiagnosticsReportResponse
+	if err := postStatusJSONProto(s, "stmtdiagreports", req, &resp); err != nil {
+		t.Fatal(err)
+	}
+
+	// Wait for request to expire.
+	time.Sleep(expiresAfter)
+
+	// Check that created statement diagnostics report is incomplete.
+	report := db.QueryStr(t, `
+SELECT completed
+FROM system.statement_diagnostics_requests
+WHERE statement_fingerprint = $1`, statementFingerprint)
+
+	require.Equal(t, report[0][0], "false")
+
+	// Check that expired report is not returned in API response.
+	var respGet serverpb.StatementDiagnosticsReportsResponse
+	if err := getStatusJSONProto(s, "stmtdiagreports", &respGet); err != nil {
+		t.Fatal(err)
+	}
+
+	for _, report := range respGet.Reports {
+		require.NotEqual(t, report.StatementFingerprint, statementFingerprint)
+	}
+}
+
 func TestJobStatusResponse(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)

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

@@ -14,6 +14,7 @@ import { fetchData } from "src/api";
 const STATEMENT_DIAGNOSTICS_PATH = "/_status/stmtdiagreports";
 const CREATE_STATEMENT_DIAGNOSTICS_REPORT_PATH = "/_status/stmtdiagreports";
 
+type CreateStatementDiagnosticsReportRequestMessage = cockroach.server.serverpb.CreateStatementDiagnosticsReportRequest;
 type CreateStatementDiagnosticsReportResponseMessage = cockroach.server.serverpb.CreateStatementDiagnosticsReportResponse;
 
 export function getStatementDiagnosticsReports(): Promise<
@@ -26,14 +27,12 @@ export function getStatementDiagnosticsReports(): Promise<
 }
 
 export function createStatementDiagnosticsReport(
-  statementsFingerprint: string,
+  req: CreateStatementDiagnosticsReportRequestMessage,
 ): Promise<CreateStatementDiagnosticsReportResponseMessage> {
   return fetchData(
     cockroach.server.serverpb.CreateStatementDiagnosticsReportResponse,
     CREATE_STATEMENT_DIAGNOSTICS_REPORT_PATH,
     cockroach.server.serverpb.CreateStatementDiagnosticsReportRequest,
-    {
-      statement_fingerprint: statementsFingerprint,
-    },
+    req,
   );
 }

pkg/ui/workspaces/cluster-ui/src/modal/modal.tsx

@@ -22,6 +22,7 @@ export interface ModalProps {
   okText?: string;
   cancelText?: string;
   visible: boolean;
+  className?: string;
 }
 
 const cx = classNames.bind(styles);
@@ -34,11 +35,12 @@ export const Modal: React.FC<ModalProps> = ({
   cancelText,
   visible,
   title,
+  className,
 }) => {
   return (
     <AntModal
       title={title && <Text textType={TextTypes.Heading3}>{title}</Text>}
-      className={cx("crl-modal")}
+      className={cx("crl-modal", className)}
       visible={visible}
       closeIcon={
         <div className={cx("crl-modal__close-icon")} onClick={onCancel}>

pkg/ui/workspaces/cluster-ui/src/statementDetails/diagnostics/diagnosticsView.spec.tsx

@@ -11,7 +11,7 @@
 import React from "react";
 import { assert } from "chai";
 import { mount, ReactWrapper } from "enzyme";
-import sinon, { SinonSpy } from "sinon";
+import sinon from "sinon";
 import Long from "long";
 import { MemoryRouter } from "react-router-dom";
 import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
@@ -20,11 +20,14 @@ import { Button } from "@cockroachlabs/ui-components";
 import { DiagnosticsView } from "./diagnosticsView";
 import { Table } from "src/table";
 import { TestStoreProvider } from "src/test-utils";
+import { ActivateDiagnosticsModalRef } from "../../statementsDiagnostics";
 
 type IStatementDiagnosticsReport = cockroach.server.serverpb.IStatementDiagnosticsReport;
 
 const sandbox = sinon.createSandbox();
 
+const activateDiagnosticsRef: React.RefObject<ActivateDiagnosticsModalRef> = React.createRef();
+
 function generateDiagnosticsRequest(
   extendObject: Partial<IStatementDiagnosticsReport> = {},
 ): IStatementDiagnosticsReport {
@@ -42,21 +45,20 @@ function generateDiagnosticsRequest(
 
 describe("DiagnosticsView", () => {
   let wrapper: ReactWrapper;
-  let activateFn: SinonSpy;
   const statementFingerprint = "some-id";
+  activateDiagnosticsRef.current.showModalFor = jest.fn();
 
   beforeEach(() => {
     sandbox.reset();
-    activateFn = sandbox.spy();
   });
 
   describe("With Empty state", () => {
     beforeEach(() => {
       wrapper = mount(
         <MemoryRouter>
           <DiagnosticsView
+            activateDiagnosticsRef={activateDiagnosticsRef}
             statementFingerprint={statementFingerprint}
-            activate={activateFn}
             hasData={false}
             diagnosticsReports={[]}
             dismissAlertMessage={() => {}}
@@ -65,10 +67,12 @@ describe("DiagnosticsView", () => {
       );
     });
 
-    it("calls activate callback with statementFingerprintId when click on Activate button", () => {
+    it("opens the statement diagnostics modal when Activate button is clicked", () => {
       const activateButtonComponent = wrapper.find(Button).first();
       activateButtonComponent.simulate("click");
-      activateFn.calledOnceWith(statementFingerprint);
+      expect(activateDiagnosticsRef.current.showModalFor).toBeCalledWith(
+        statementFingerprint,
+      );
     });
   });
 
@@ -82,8 +86,8 @@ describe("DiagnosticsView", () => {
       wrapper = mount(
         <TestStoreProvider>
           <DiagnosticsView
+            activateDiagnosticsRef={activateDiagnosticsRef}
             statementFingerprint={statementFingerprint}
-            activate={activateFn}
             hasData={true}
             diagnosticsReports={diagnosticsRequests}
             dismissAlertMessage={() => {}}
@@ -96,12 +100,14 @@ describe("DiagnosticsView", () => {
       assert.isTrue(wrapper.find(Table).exists());
     });
 
-    it("calls activate callback with statementFingerprintId when click on Activate button", () => {
+    it("opens the statement diagnostics modal when Activate button is clicked", () => {
       const activateButtonComponent = wrapper
         .findWhere(n => n.prop("children") === "Activate diagnostics")
         .first();
       activateButtonComponent.simulate("click");
-      activateFn.calledOnceWith(statementFingerprint);
+      expect(activateDiagnosticsRef.current.showModalFor).toBeCalledWith(
+        statementFingerprint,
+      );
     });
 
     it("Activate button is hidden if diagnostics is requested and waiting query", () => {
@@ -112,8 +118,8 @@ describe("DiagnosticsView", () => {
       wrapper = mount(
         <TestStoreProvider>
           <DiagnosticsView
+            activateDiagnosticsRef={activateDiagnosticsRef}
             statementFingerprint={statementFingerprint}
-            activate={activateFn}
             hasData={true}
             diagnosticsReports={diagnosticsRequests}
             dismissAlertMessage={() => {}}

pkg/ui/workspaces/cluster-ui/src/statementDetails/diagnostics/diagnosticsView.tsx

@@ -8,7 +8,7 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-import React, { useCallback } from "react";
+import React from "react";
 import { Link } from "react-router-dom";
 import moment from "moment";
 import classnames from "classnames/bind";
@@ -17,29 +17,30 @@ import { Button, Icon } from "@cockroachlabs/ui-components";
 import { Text, TextTypes } from "src/text";
 import { Table, ColumnsConfig } from "src/table";
 import { SummaryCard } from "src/summaryCard";
-import { DiagnosticStatusBadge } from "src/statementsDiagnostics";
+import {
+  ActivateDiagnosticsModalRef,
+  DiagnosticStatusBadge,
+} from "src/statementsDiagnostics";
 import emptyListResultsImg from "src/assets/emptyState/empty-list-results.svg";
 import {
   getDiagnosticsStatus,
   sortByCompletedField,
   sortByRequestedAtField,
 } from "./diagnosticsUtils";
-import { statementDiagnostics } from "src/util/docs";
 import { EmptyTable } from "src/empty";
 import styles from "./diagnosticsView.module.scss";
 import { getBasePath } from "../../api";
-import { Anchor } from "../../anchor";
 
 type IStatementDiagnosticsReport = cockroach.server.serverpb.IStatementDiagnosticsReport;
 
 export interface DiagnosticsViewStateProps {
   hasData: boolean;
   diagnosticsReports: cockroach.server.serverpb.IStatementDiagnosticsReport[];
   showDiagnosticsViewLink?: boolean;
+  activateDiagnosticsRef: React.RefObject<ActivateDiagnosticsModalRef>;
 }
 
 export interface DiagnosticsViewDispatchProps {
-  activate: (statementFingerprint: string) => void;
   dismissAlertMessage: () => void;
   onDownloadDiagnosticBundleClick?: (statementFingerprint: string) => void;
   onSortingChange?: (
@@ -72,30 +73,24 @@ const NavButton: React.FC = props => (
 );
 
 export const EmptyDiagnosticsView = ({
-  activate,
   statementFingerprint,
   showDiagnosticsViewLink,
+  activateDiagnosticsRef,
 }: DiagnosticsViewProps) => {
-  const onActivateButtonClick = useCallback(() => {
-    activate(statementFingerprint);
-  }, [activate, statementFingerprint]);
   return (
     <EmptyTable
       icon={emptyListResultsImg}
       title="Activate statement diagnostics"
-      message={
-        <span>
-          When you activate statement diagnostics, CockroachDB will wait for the
-          next query that matches this statement fingerprint. A download button
-          will appear on the statement list and detail pages when the query is
-          ready. The statement diagnostic will include EXPLAIN plans, table
-          statistics, and traces.{" "}
-          <Anchor href={statementDiagnostics}>Learn More</Anchor>
-        </span>
-      }
       footer={
         <footer className={cx("empty-view__footer")}>
-          <Button intent="primary" onClick={onActivateButtonClick}>
+          <Button
+            intent="primary"
+            onClick={() =>
+              activateDiagnosticsRef?.current?.showModalFor(
+                statementFingerprint,
+              )
+            }
+          >
             Activate Diagnostics
           </Button>
           {showDiagnosticsViewLink && (
@@ -186,11 +181,6 @@ export class DiagnosticsView extends React.Component<
     },
   ];
 
-  onActivateButtonClick = () => {
-    const { activate, statementFingerprint } = this.props;
-    activate(statementFingerprint);
-  };
-
   componentWillUnmount() {
     this.props.dismissAlertMessage();
   }
@@ -202,7 +192,13 @@ export class DiagnosticsView extends React.Component<
   };
 
   render() {
-    const { hasData, diagnosticsReports, showDiagnosticsViewLink } = this.props;
+    const {
+      hasData,
+      diagnosticsReports,
+      showDiagnosticsViewLink,
+      statementFingerprint,
+      activateDiagnosticsRef,
+    } = this.props;
 
     const canRequestDiagnostics = diagnosticsReports.every(
       diagnostic => diagnostic.completed,
@@ -227,7 +223,11 @@ export class DiagnosticsView extends React.Component<
           <Text textType={TextTypes.Heading3}>Statement diagnostics</Text>
           {canRequestDiagnostics && (
             <Button
-              onClick={this.onActivateButtonClick}
+              onClick={() =>
+                activateDiagnosticsRef?.current?.showModalFor(
+                  statementFingerprint,
+                )
+              }
               disabled={!canRequestDiagnostics}
               intent="secondary"
             >