From 76a83dd5408d85702669175daa188c4b98302582 Mon Sep 17 00:00:00 2001 From: Joey <90795735+joey-grafana@users.noreply.github.com> Date: Thu, 13 Jul 2023 10:48:31 +0100 Subject: [PATCH] Traces: Add inline validation and greater precision to duration fields in span filters (#71404) * Add inline validation to span filters * Update filter spans by duration precision * Update IntervalInput props * Update validation * Update span filters * Update props * Update test * Update defaults and duration aria labels --- .../IntervalInput/IntervalInput.tsx | 74 +++++++++++-------- .../IntervalInput/validation.test.ts | 34 ++++----- .../components/IntervalInput/validation.ts | 7 +- .../TraceToLogs/TraceToLogsSettings.tsx | 41 +++++----- .../TraceToMetrics/TraceToMetricsSettings.tsx | 52 +++++++------ .../SpanFilters/SpanFilters.test.tsx | 8 +- .../SpanFilters/SpanFilters.tsx | 47 ++++++------ .../components/utils/filter-spans.test.ts | 6 ++ .../components/utils/filter-spans.tsx | 8 +- 9 files changed, 157 insertions(+), 120 deletions(-) diff --git a/public/app/core/components/IntervalInput/IntervalInput.tsx b/public/app/core/components/IntervalInput/IntervalInput.tsx index 5dc1f828184..692845a3cdd 100644 --- a/public/app/core/components/IntervalInput/IntervalInput.tsx +++ b/public/app/core/components/IntervalInput/IntervalInput.tsx @@ -1,53 +1,69 @@ import React, { useState } from 'react'; import { useDebounce } from 'react-use'; -import { InlineField, InlineFieldRow, Input } from '@grafana/ui'; +import { InlineField, Input } from '@grafana/ui'; -import { validateInterval } from './validation'; +import { validateInterval, validateIntervalRegex } from './validation'; interface Props { - label: string; - tooltip: string; value: string; onChange: (val: string) => void; isInvalidError: string; + placeholder?: string; + width?: number; + ariaLabel?: string; + label?: string; + tooltip?: string; disabled?: boolean; + validationRegex?: RegExp; } -export function IntervalInput(props: Props) { +interface FieldProps { + labelWidth: number; + disabled: boolean; + invalid: boolean; + error: string; + label?: string; + tooltip?: string; +} + +export const IntervalInput = (props: Props) => { + const validationRegex = props.validationRegex || validateIntervalRegex; const [intervalIsInvalid, setIntervalIsInvalid] = useState(() => { - return props.value ? validateInterval(props.value) : false; + return props.value ? validateInterval(props.value, validationRegex) : false; }); useDebounce( () => { - setIntervalIsInvalid(validateInterval(props.value)); + setIntervalIsInvalid(validateInterval(props.value, validationRegex)); }, 500, [props.value] ); + const fieldProps: FieldProps = { + labelWidth: 26, + disabled: props.disabled ?? false, + invalid: intervalIsInvalid, + error: props.isInvalidError, + }; + if (props.label) { + fieldProps.label = props.label; + fieldProps.tooltip = props.tooltip || ''; + } + return ( - - - { - props.onChange(e.currentTarget.value); - }} - value={props.value} - /> - - + + { + props.onChange(e.currentTarget.value); + }} + value={props.value} + aria-label={props.ariaLabel || 'interval input'} + /> + ); -} +}; diff --git a/public/app/core/components/IntervalInput/validation.test.ts b/public/app/core/components/IntervalInput/validation.test.ts index 5d6b3365f21..8de4e2d4327 100644 --- a/public/app/core/components/IntervalInput/validation.test.ts +++ b/public/app/core/components/IntervalInput/validation.test.ts @@ -1,28 +1,28 @@ -import { validateInterval } from './validation'; +import { validateInterval, validateIntervalRegex } from './validation'; describe('Validation', () => { it('should validate incorrect values correctly', () => { - expect(validateInterval('-')).toBeTruthy(); - expect(validateInterval('1')).toBeTruthy(); - expect(validateInterval('test')).toBeTruthy(); - expect(validateInterval('1ds')).toBeTruthy(); - expect(validateInterval('10Ms')).toBeTruthy(); - expect(validateInterval('-9999999')).toBeTruthy(); + expect(validateInterval('-', validateIntervalRegex)).toBeTruthy(); + expect(validateInterval('1', validateIntervalRegex)).toBeTruthy(); + expect(validateInterval('test', validateIntervalRegex)).toBeTruthy(); + expect(validateInterval('1ds', validateIntervalRegex)).toBeTruthy(); + expect(validateInterval('10Ms', validateIntervalRegex)).toBeTruthy(); + expect(validateInterval('-9999999', validateIntervalRegex)).toBeTruthy(); }); it('should validate correct values correctly', () => { - expect(validateInterval('1y')).toBeFalsy(); - expect(validateInterval('1M')).toBeFalsy(); - expect(validateInterval('1w')).toBeFalsy(); - expect(validateInterval('1d')).toBeFalsy(); - expect(validateInterval('2h')).toBeFalsy(); - expect(validateInterval('4m')).toBeFalsy(); - expect(validateInterval('8s')).toBeFalsy(); - expect(validateInterval('80ms')).toBeFalsy(); - expect(validateInterval('-80ms')).toBeFalsy(); + expect(validateInterval('1y', validateIntervalRegex)).toBeFalsy(); + expect(validateInterval('1M', validateIntervalRegex)).toBeFalsy(); + expect(validateInterval('1w', validateIntervalRegex)).toBeFalsy(); + expect(validateInterval('1d', validateIntervalRegex)).toBeFalsy(); + expect(validateInterval('2h', validateIntervalRegex)).toBeFalsy(); + expect(validateInterval('4m', validateIntervalRegex)).toBeFalsy(); + expect(validateInterval('8s', validateIntervalRegex)).toBeFalsy(); + expect(validateInterval('80ms', validateIntervalRegex)).toBeFalsy(); + expect(validateInterval('-80ms', validateIntervalRegex)).toBeFalsy(); }); it('should not return error if no value provided', () => { - expect(validateInterval('')).toBeFalsy(); + expect(validateInterval('', validateIntervalRegex)).toBeFalsy(); }); }); diff --git a/public/app/core/components/IntervalInput/validation.ts b/public/app/core/components/IntervalInput/validation.ts index fcd89d56725..44fd754fa2b 100644 --- a/public/app/core/components/IntervalInput/validation.ts +++ b/public/app/core/components/IntervalInput/validation.ts @@ -1,5 +1,6 @@ -export const validateInterval = (val: string) => { - const intervalRegex = /^(-?\d+(?:\.\d+)?)(ms|[Mwdhmsy])$/; - const matches = val.match(intervalRegex); +export const validateIntervalRegex = /^(-?\d+(?:\.\d+)?)(ms|[Mwdhmsy])$/; + +export const validateInterval = (val: string, regex: RegExp) => { + const matches = val.match(regex); return matches || !val ? false : true; }; diff --git a/public/app/core/components/TraceToLogs/TraceToLogsSettings.tsx b/public/app/core/components/TraceToLogs/TraceToLogsSettings.tsx index 341f83c55e9..56e68a6002b 100644 --- a/public/app/core/components/TraceToLogs/TraceToLogsSettings.tsx +++ b/public/app/core/components/TraceToLogs/TraceToLogsSettings.tsx @@ -125,24 +125,29 @@ export function TraceToLogsSettings({ options, onOptionsChange }: Props) { - { - updateTracesToLogs({ spanStartTimeShift: val }); - }} - isInvalidError={invalidTimeShiftError} - /> - { - updateTracesToLogs({ spanEndTimeShift: val }); - }} - isInvalidError={invalidTimeShiftError} - /> + + { + updateTracesToLogs({ spanStartTimeShift: val }); + }} + isInvalidError={invalidTimeShiftError} + /> + + + + { + updateTracesToLogs({ spanEndTimeShift: val }); + }} + isInvalidError={invalidTimeShiftError} + /> + - { - updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'tracesToMetrics', { - ...options.jsonData.tracesToMetrics, - spanStartTimeShift: val, - }); - }} - isInvalidError={invalidTimeShiftError} - /> + + { + updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'tracesToMetrics', { + ...options.jsonData.tracesToMetrics, + spanStartTimeShift: val, + }); + }} + isInvalidError={invalidTimeShiftError} + /> + - { - updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'tracesToMetrics', { - ...options.jsonData.tracesToMetrics, - spanEndTimeShift: val, - }); - }} - isInvalidError={invalidTimeShiftError} - /> + + { + updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'tracesToMetrics', { + ...options.jsonData.tracesToMetrics, + spanEndTimeShift: val, + }); + }} + isInvalidError={invalidTimeShiftError} + /> + diff --git a/public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.test.tsx b/public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.test.tsx index a6e2fe2f46b..7abb4348814 100644 --- a/public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.test.tsx +++ b/public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.test.tsx @@ -83,10 +83,10 @@ describe('SpanFilters', () => { const serviceValue = screen.getByLabelText('Select service name'); const spanOperator = screen.getByLabelText('Select span name operator'); const spanValue = screen.getByLabelText('Select span name'); - const fromOperator = screen.getByLabelText('Select from operator'); - const fromValue = screen.getByLabelText('Select from value'); - const toOperator = screen.getByLabelText('Select to operator'); - const toValue = screen.getByLabelText('Select to value'); + const fromOperator = screen.getByLabelText('Select min span operator'); + const fromValue = screen.getByLabelText('Select min span duration'); + const toOperator = screen.getByLabelText('Select max span operator'); + const toValue = screen.getByLabelText('Select max span duration'); const tagKey = screen.getByLabelText('Select tag key'); const tagOperator = screen.getByLabelText('Select tag operator'); const tagValue = screen.getByLabelText('Select tag value'); diff --git a/public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.tsx b/public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.tsx index 414b0678e9d..23681adbd37 100644 --- a/public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.tsx +++ b/public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.tsx @@ -19,17 +19,8 @@ import React, { useState, useEffect, memo, useCallback } from 'react'; import { GrafanaTheme2, SelectableValue, toOption } from '@grafana/data'; import { AccessoryButton } from '@grafana/experimental'; -import { - Collapse, - HorizontalGroup, - Icon, - InlineField, - InlineFieldRow, - Input, - Select, - Tooltip, - useStyles2, -} from '@grafana/ui'; +import { Collapse, HorizontalGroup, Icon, InlineField, InlineFieldRow, Select, Tooltip, useStyles2 } from '@grafana/ui'; +import { IntervalInput } from 'app/core/components/IntervalInput/IntervalInput'; import { defaultFilters, randomId, SearchProps, Tag } from '../../../useSearch'; import { KIND, LIBRARY_NAME, LIBRARY_VERSION, STATUS, STATUS_MESSAGE, TRACE_STATE, ID } from '../../constants/span'; @@ -70,6 +61,8 @@ export const SpanFilters = memo((props: SpanFilterProps) => { const [tagValues, setTagValues] = useState<{ [key: string]: Array> }>({}); const [focusedSpanIndexForSearch, setFocusedSpanIndexForSearch] = useState(-1); + const durationRegex = /^\d+(?:\.\d)?\d*(?:ns|us|µs|ms|s|m|h)$/; + const clear = useCallback(() => { setServiceNames(undefined); setSpanNames(undefined); @@ -343,33 +336,41 @@ export const SpanFilters = memo((props: SpanFilterProps) => { - - + + setSpanFiltersSearch({ ...search, from: v.currentTarget.value })} + setSpanFiltersSearch({ ...search, from: val })} + isInvalidError="Invalid duration" placeholder="e.g. 100ms, 1.2s" - value={search.from || ''} width={18} + value={search.from || ''} + validationRegex={durationRegex} /> setSpanFiltersSearch({ ...search, to: v.currentTarget.value })} + setSpanFiltersSearch({ ...search, to: val })} + isInvalidError="Invalid duration" placeholder="e.g. 100ms, 1.2s" - value={search.to || ''} width={18} + value={search.to || ''} + validationRegex={durationRegex} /> diff --git a/public/app/features/explore/TraceView/components/utils/filter-spans.test.ts b/public/app/features/explore/TraceView/components/utils/filter-spans.test.ts index 8bbbce58a06..09075f03d58 100644 --- a/public/app/features/explore/TraceView/components/utils/filter-spans.test.ts +++ b/public/app/features/explore/TraceView/components/utils/filter-spans.test.ts @@ -155,6 +155,12 @@ describe('filterSpans', () => { // Durations it('should return spans whose duration match a filter', () => { + expect(filterSpansNewTraceViewHeader({ ...defaultFilters, from: '2ns' }, spans)).toEqual( + new Set([spanID0, spanID2]) + ); + expect(filterSpansNewTraceViewHeader({ ...defaultFilters, from: '2us' }, spans)).toEqual( + new Set([spanID0, spanID2]) + ); expect(filterSpansNewTraceViewHeader({ ...defaultFilters, from: '2ms' }, spans)).toEqual( new Set([spanID0, spanID2]) ); diff --git a/public/app/features/explore/TraceView/components/utils/filter-spans.tsx b/public/app/features/explore/TraceView/components/utils/filter-spans.tsx index 6c4f9b69eed..beb6465e044 100644 --- a/public/app/features/explore/TraceView/components/utils/filter-spans.tsx +++ b/public/app/features/explore/TraceView/components/utils/filter-spans.tsx @@ -157,8 +157,12 @@ const getDurationMatches = (spans: TraceSpan[], searchProps: SearchProps) => { }; export const convertTimeFilter = (time: string) => { - if (time.includes('μs')) { - return parseFloat(time.split('μs')[0]); + if (time.includes('ns')) { + return parseFloat(time.split('ns')[0]) / 1000; + } else if (time.includes('us')) { + return parseFloat(time.split('us')[0]); + } else if (time.includes('µs')) { + return parseFloat(time.split('µs')[0]); } else if (time.includes('ms')) { return parseFloat(time.split('ms')[0]) * 1000; } else if (time.includes('s')) {