ui/server: add ability to create conditional statement diagnostics

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";
@@ -25,6 +25,8 @@ type IStatementDiagnosticsReport = cockroach.server.serverpb.IStatementDiagnosti
 
 const sandbox = sinon.createSandbox();
 
+const activateDiagnosticsRef = { current: { showModalFor: jest.fn() } };
+
 function generateDiagnosticsRequest(
   extendObject: Partial<IStatementDiagnosticsReport> = {},
 ): IStatementDiagnosticsReport {
@@ -42,21 +44,19 @@ function generateDiagnosticsRequest(
 
 describe("DiagnosticsView", () => {
   let wrapper: ReactWrapper;
-  let activateFn: SinonSpy;
   const statementFingerprint = "some-id";
 
   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 +65,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 +84,8 @@ describe("DiagnosticsView", () => {
       wrapper = mount(
         <TestStoreProvider>
           <DiagnosticsView
+            activateDiagnosticsRef={activateDiagnosticsRef}
             statementFingerprint={statementFingerprint}
-            activate={activateFn}
             hasData={true}
             diagnosticsReports={diagnosticsRequests}
             dismissAlertMessage={() => {}}
@@ -96,12 +98,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 +116,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"
             >