diff --git a/pkg/sql/stmtdiagnostics/statement_diagnostics.go b/pkg/sql/stmtdiagnostics/statement_diagnostics.go index acb28873d26f..47adb48dca73 100644 --- a/pkg/sql/stmtdiagnostics/statement_diagnostics.go +++ b/pkg/sql/stmtdiagnostics/statement_diagnostics.go @@ -297,7 +297,7 @@ func (r *Registry) insertRequestInternal( if minExecutionLatency == 0 { return 0, errors.Newf( "got non-zero sampling probability %f and empty min exec latency", - minExecutionLatency) + samplingProbability) } } diff --git a/pkg/ui/workspaces/cluster-ui/src/api/statementDiagnosticsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/statementDiagnosticsApi.ts index f290fb534372..fb7498707ece 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/statementDiagnosticsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/statementDiagnosticsApi.ts @@ -9,6 +9,7 @@ // licenses/APL.txt. import moment from "moment-timezone"; +import { Duration } from "src/util/format"; import { executeInternalSql, SqlExecutionRequest, @@ -74,7 +75,7 @@ export type InsertStmtDiagnosticRequest = { }; export type InsertStmtDiagnosticResponse = { - stmt_diag_req_id: string; + req_resp: boolean; }; export function createStatementDiagnosticsReport({ @@ -83,32 +84,26 @@ export function createStatementDiagnosticsReport({ minExecutionLatencySeconds, expiresAfterSeconds, }: InsertStmtDiagnosticRequest): Promise { - const requestedAt = moment.now(); // milliseconds - const args: any = [stmtFingerprint, moment.utc(requestedAt).toISOString()]; - const cols = ["statement_fingerprint", "requested_at"]; + const args: any = [stmtFingerprint]; - if (samplingProbability && samplingProbability !== 0) { + if (samplingProbability) { args.push(samplingProbability); - cols.push("sampling_probability"); + } else { + args.push(0); } - if (minExecutionLatencySeconds && minExecutionLatencySeconds !== 0) { - args.push(minExecutionLatencySeconds.toString()); - cols.push("min_execution_latency"); + if (minExecutionLatencySeconds) { + args.push(Duration(minExecutionLatencySeconds * 1e9)); + } else { + args.push("0"); } if (expiresAfterSeconds && expiresAfterSeconds !== 0) { - const expiresAt = requestedAt + expiresAfterSeconds * 1000; - args.push(moment.utc(expiresAt).toISOString()); - cols.push("expires_at"); + args.push(Duration(expiresAfterSeconds * 1e9)); + } else { + args.push("0"); } - const queryCols = cols.join(", "); - const placeHolders = args.map((elem: any, idx: number) => `$${idx + 1}`); - const createStmtDiag = { - sql: ` - INSERT INTO system.statement_diagnostics_requests - (${queryCols}) - VALUES (${placeHolders}) RETURNING id as stmt_diag_req_id;`, + sql: `SELECT crdb_internal.request_statement_bundle($1, $2, $3::INTERVAL, $4::INTERVAL) as req_resp`, arguments: args, }; @@ -124,7 +119,10 @@ export function createStatementDiagnosticsReport({ throw res.error; } - if (res.execution?.txn_results[0]?.rows?.length === 0) { + if ( + res.execution?.txn_results[0]?.rows?.length === 0 || + res.execution?.txn_results[0]?.rows[0]["req_resp"] === false + ) { throw new Error("Failed to insert statement diagnostics request"); } @@ -178,7 +176,11 @@ export type CancelStmtDiagnosticResponse = { export function cancelStatementDiagnosticsReport({ requestId, }: CancelStmtDiagnosticRequest): Promise { - const query = `UPDATE system.statement_diagnostics_requests SET expires_at = '1970-01-01' WHERE completed = false AND id = $1::INT8 AND (expires_at IS NULL OR expires_at > now()) RETURNING id as stmt_diag_req_id`; + const query = `UPDATE system.statement_diagnostics_requests +SET expires_at = '1970-01-01' +WHERE completed = false +AND id = $1::INT8 +AND (expires_at IS NULL OR expires_at > now()) RETURNING id as stmt_diag_req_id`; const req: SqlExecutionRequest = { execute: true, statements: [ diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/diagnostics/diagnosticsView.tsx b/pkg/ui/workspaces/cluster-ui/src/statementDetails/diagnostics/diagnosticsView.tsx index 0c67216295b6..0ceea0acaa82 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementDetails/diagnostics/diagnosticsView.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementDetails/diagnostics/diagnosticsView.tsx @@ -12,7 +12,6 @@ import React from "react"; import { Link } from "react-router-dom"; import moment from "moment-timezone"; import classnames from "classnames/bind"; -import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; import { Button, Icon } from "@cockroachlabs/ui-components"; import { Button as CancelButton } from "src/button"; import { Text, TextTypes } from "src/text"; diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsDiagnostics/activateStatementDiagnosticsModal.scss b/pkg/ui/workspaces/cluster-ui/src/statementsDiagnostics/activateStatementDiagnosticsModal.scss index 618437f0abb6..1cf9369c6a49 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsDiagnostics/activateStatementDiagnosticsModal.scss +++ b/pkg/ui/workspaces/cluster-ui/src/statementsDiagnostics/activateStatementDiagnosticsModal.scss @@ -40,6 +40,7 @@ &__heading { font-family: $font-family--semi-bold; + margin-bottom: $spacing-smaller; } &__btn-group { @@ -48,7 +49,6 @@ } &__radio-btn { - margin-top: $spacing-smaller; font-family: $font-family--base; font-weight: $font-weight--light; } @@ -57,6 +57,7 @@ display: flex; flex-direction: column; margin-left: $spacing-medium; + margin-top: $spacing-smaller; } &__min-latency-container { @@ -65,6 +66,22 @@ margin-top: $spacing-smaller; } + &__trace-container { + display: flex; + flex-direction: row; + margin-top: $spacing-smaller; + margin-bottom: $spacing-smaller; + } + + &__trace-warning { + font-family: $font-family--base; + font-size: $font-size--small; + font-weight: $font-weight--light; + margin-left: $spacing-x-small; + white-space: break-spaces; + width: 240px; + } + &__expires-after-container { margin-top: $spacing-smaller; margin-left: $spacing-medium; @@ -76,6 +93,12 @@ display: inline; } + &__select-text { + font-family: $font-family--base; + font-weight: $font-weight--light; + display: block; + } + &__input { &__min-latency-time { width: 80px; @@ -95,6 +118,15 @@ height: 40px; line-height: 40px; + .ant-select-selection__rendered { + margin-top: 0; + } + } + &__trace { + width: 215px; + height: 40px; + line-height: 40px; + .ant-select-selection__rendered { margin-top: 0; } @@ -104,20 +136,11 @@ &__alert { margin-top: $spacing-small; margin-left: $spacing-medium; + white-space: normal; + } - .ant-alert-info { - background-color: $colors--primary-blue-alert; - border: none; - } - - &-icon { - margin-top: $spacing-medium-small; - } - - &-message { - font-family: $font-family--base; - font-weight: $font-weight--light; - margin-left: $spacing-smaller; - } + &__warning { + margin: $spacing-xx-small 0px; + white-space: normal; } } diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsDiagnostics/activateStatementDiagnosticsModal.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsDiagnostics/activateStatementDiagnosticsModal.tsx index 60ead5046634..767cc77a3d80 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsDiagnostics/activateStatementDiagnosticsModal.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsDiagnostics/activateStatementDiagnosticsModal.tsx @@ -8,14 +8,13 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -import { Alert, Button, Checkbox, Divider, Input, Radio, Select } from "antd"; +import { Button, Checkbox, Divider, Input, Radio, Select } from "antd"; import "antd/lib/radio/style"; import "antd/lib/button/style"; import "antd/lib/input/style"; import "antd/lib/checkbox/style"; import "antd/lib/divider/style"; import "antd/lib/select/style"; -import "antd/lib/alert/style"; import React, { useCallback, useImperativeHandle, useState } from "react"; import { Modal } from "src/modal"; import { Anchor } from "src/anchor"; @@ -23,8 +22,8 @@ import { Text } from "src/text"; import { statementDiagnostics, statementsSql } from "src/util"; import classNames from "classnames/bind"; import styles from "./activateStatementDiagnosticsModal.scss"; -import { InfoCircleFilled } from "@cockroachlabs/icons"; import { InsertStmtDiagnosticRequest } from "../api"; +import { InlineAlert } from "@cockroachlabs/ui-components"; const cx = classNames.bind(styles); const { Option } = Select; @@ -46,12 +45,13 @@ export const ActivateStatementDiagnosticsModal = React.forwardRef( ) => { const [visible, setVisible] = useState(false); const [statement, setStatement] = useState(); - const [conditional, setConditional] = useState(false); + const [conditional, setConditional] = useState(true); const [expires, setExpires] = useState(true); const [minExecLatency, setMinExecLatency] = useState(100); const [minExecLatencyUnit, setMinExecLatencyUnit] = useState("milliseconds"); const [expiresAfter, setExpiresAfter] = useState(15); + const [traceSampleRate, setTraceSampleRate] = useState(0.01); const handleSelectChange = (value: string) => { setMinExecLatencyUnit(value); @@ -71,6 +71,16 @@ export const ActivateStatementDiagnosticsModal = React.forwardRef( return numMinutes * 60; // num seconds }; + const getTraceSampleRate = ( + conditional: boolean, + traceSampleRate: number, + ) => { + if (conditional) { + return traceSampleRate; + } + return 0; + }; + const onOkHandler = useCallback(() => { activate({ stmtFingerprint: statement, @@ -80,6 +90,7 @@ export const ActivateStatementDiagnosticsModal = React.forwardRef( minExecLatencyUnit, ), expiresAfterSeconds: getExpiresAfter(expires, expiresAfter), + samplingProbability: getTraceSampleRate(conditional, traceSampleRate), }); setVisible(false); }, [ @@ -90,6 +101,7 @@ export const ActivateStatementDiagnosticsModal = React.forwardRef( minExecLatencyUnit, expires, expiresAfter, + traceSampleRate, ]); const onCancelHandler = useCallback(() => setVisible(false), []); @@ -116,29 +128,60 @@ export const ActivateStatementDiagnosticsModal = React.forwardRef( > Diagnostics will be collected for the next execution that matches this{" "} - statement fingerprint, or when - the execution of the statement fingerprint exceeds a specified - latency. The request is cancelled when a single bundle is captured.{" "} + statement fingerprint, or + according to the trace and latency thresholds set below. The request + is cancelled when a single diagnostics bundle is captured.{" "} Learn more
- Collect diagnostics + + Collect diagnostics: + - setConditional(false)} - > - On the next execution - setConditional(true)} > - On the next execution where the latency exceeds + Trace and collect diagnostics
+
+ At a sampled rate of: +
+
+ + + We recommend starting at 1% to minimize the impact on + performance. + +
+ {getTraceSampleRate(conditional, traceSampleRate) == 1 && ( +
+ +
+ )} +
+ When the statement execution latency exceeds: +
milliseconds
-
+ setConditional(false)} + > + Trace and collect diagnostics on the next statement execution + + setExpires(!expires)}>
Diagnostics request expires after: @@ -189,22 +239,12 @@ export const ActivateStatementDiagnosticsModal = React.forwardRef(
{conditional && !expires && (
- - -
- } - message={ -
- Executions of the same statement fingerprint will run +
)} diff --git a/pkg/ui/workspaces/cluster-ui/src/store/statementDiagnostics/statementDiagnostics.sagas.spec.ts b/pkg/ui/workspaces/cluster-ui/src/store/statementDiagnostics/statementDiagnostics.sagas.spec.ts index 0872a6a5ae40..01988d7214e2 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/statementDiagnostics/statementDiagnostics.sagas.spec.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/statementDiagnostics/statementDiagnostics.sagas.spec.ts @@ -43,7 +43,7 @@ describe("statementsDiagnostics sagas", () => { }; const insertResponse: InsertStmtDiagnosticResponse = { - stmt_diag_req_id: "4132456789089978654", + req_resp: true, }; const reportsResponse: StatementDiagnosticsResponse = [];