From f1338cee60f52f0c5570edb10ebcbb6c9d8956c0 Mon Sep 17 00:00:00 2001 From: Joey <90795735+joey-grafana@users.noreply.github.com> Date: Tue, 4 Jul 2023 10:49:21 +0100 Subject: [PATCH] Tracing: Add inline validation to time shift configuration fields (#70879) * Inline validation * Update invalidTimeShiftError after self review * Renames and moved err msg * Update validation * Remove local state --- .../IntervalInput/IntervalInput.test.tsx | 76 +++++++++++++++++ .../IntervalInput/IntervalInput.tsx | 53 ++++++++++++ .../IntervalInput/validation.test.ts | 28 +++++++ .../components/IntervalInput/validation.ts | 5 ++ .../TraceToLogs/TraceToLogsSettings.tsx | 56 ++++++------- .../TraceToMetrics/TraceToMetricsSettings.tsx | 68 ++++++--------- .../tempo/configuration/QuerySettings.tsx | 84 +++++++++---------- 7 files changed, 252 insertions(+), 118 deletions(-) create mode 100644 public/app/core/components/IntervalInput/IntervalInput.test.tsx create mode 100644 public/app/core/components/IntervalInput/IntervalInput.tsx create mode 100644 public/app/core/components/IntervalInput/validation.test.ts create mode 100644 public/app/core/components/IntervalInput/validation.ts diff --git a/public/app/core/components/IntervalInput/IntervalInput.test.tsx b/public/app/core/components/IntervalInput/IntervalInput.test.tsx new file mode 100644 index 00000000000..ed3464bfd6d --- /dev/null +++ b/public/app/core/components/IntervalInput/IntervalInput.test.tsx @@ -0,0 +1,76 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React, { useState } from 'react'; + +import { invalidTimeShiftError } from '../TraceToLogs/TraceToLogsSettings'; + +import { IntervalInput } from './IntervalInput'; + +describe('IntervalInput', () => { + const IntervalInputtWithProps = ({ val }: { val: string }) => { + const [value, setValue] = useState(val); + + return ( + { + setValue(v); + }} + isInvalidError={invalidTimeShiftError} + /> + ); + }; + + describe('validates time shift correctly', () => { + it('for previosuly saved invalid value', async () => { + render(); + expect(screen.getByDisplayValue('77')).toBeInTheDocument(); + expect(screen.getByText(invalidTimeShiftError)).toBeInTheDocument(); + }); + + it('for previously saved empty value', async () => { + render(); + expect(screen.getByPlaceholderText('0')).toBeInTheDocument(); + expect(screen.queryByText(invalidTimeShiftError)).not.toBeInTheDocument(); + }); + + it('for empty (valid) value', async () => { + render(); + await userEvent.clear(screen.getByDisplayValue('1ms')); + await waitFor(() => { + expect(screen.queryByText(invalidTimeShiftError)).not.toBeInTheDocument(); + }); + }); + + it('for valid value', async () => { + render(); + expect(screen.queryByText(invalidTimeShiftError)).not.toBeInTheDocument(); + + const input = screen.getByDisplayValue('10ms'); + await userEvent.clear(input); + await userEvent.type(input, '100s'); + await waitFor(() => { + expect(screen.queryByText(invalidTimeShiftError)).not.toBeInTheDocument(); + }); + + await userEvent.clear(input); + await userEvent.type(input, '-77ms'); + await waitFor(() => { + expect(screen.queryByText(invalidTimeShiftError)).not.toBeInTheDocument(); + }); + }); + + it('for invalid value', async () => { + render(); + const input = screen.getByDisplayValue('10ms'); + await userEvent.clear(input); + await userEvent.type(input, 'abc'); + await waitFor(() => { + expect(screen.queryByText(invalidTimeShiftError)).toBeInTheDocument(); + }); + }); + }); +}); diff --git a/public/app/core/components/IntervalInput/IntervalInput.tsx b/public/app/core/components/IntervalInput/IntervalInput.tsx new file mode 100644 index 00000000000..5dc1f828184 --- /dev/null +++ b/public/app/core/components/IntervalInput/IntervalInput.tsx @@ -0,0 +1,53 @@ +import React, { useState } from 'react'; +import { useDebounce } from 'react-use'; + +import { InlineField, InlineFieldRow, Input } from '@grafana/ui'; + +import { validateInterval } from './validation'; + +interface Props { + label: string; + tooltip: string; + value: string; + onChange: (val: string) => void; + isInvalidError: string; + disabled?: boolean; +} + +export function IntervalInput(props: Props) { + const [intervalIsInvalid, setIntervalIsInvalid] = useState(() => { + return props.value ? validateInterval(props.value) : false; + }); + + useDebounce( + () => { + setIntervalIsInvalid(validateInterval(props.value)); + }, + 500, + [props.value] + ); + + return ( + + + { + props.onChange(e.currentTarget.value); + }} + value={props.value} + /> + + + ); +} diff --git a/public/app/core/components/IntervalInput/validation.test.ts b/public/app/core/components/IntervalInput/validation.test.ts new file mode 100644 index 00000000000..5d6b3365f21 --- /dev/null +++ b/public/app/core/components/IntervalInput/validation.test.ts @@ -0,0 +1,28 @@ +import { validateInterval } 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(); + }); + + 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(); + }); + + it('should not return error if no value provided', () => { + expect(validateInterval('')).toBeFalsy(); + }); +}); diff --git a/public/app/core/components/IntervalInput/validation.ts b/public/app/core/components/IntervalInput/validation.ts new file mode 100644 index 00000000000..fcd89d56725 --- /dev/null +++ b/public/app/core/components/IntervalInput/validation.ts @@ -0,0 +1,5 @@ +export const validateInterval = (val: string) => { + const intervalRegex = /^(-?\d+(?:\.\d+)?)(ms|[Mwdhmsy])$/; + const matches = val.match(intervalRegex); + return matches || !val ? false : true; +}; diff --git a/public/app/core/components/TraceToLogs/TraceToLogsSettings.tsx b/public/app/core/components/TraceToLogs/TraceToLogsSettings.tsx index 775bae9adc8..341f83c55e9 100644 --- a/public/app/core/components/TraceToLogs/TraceToLogsSettings.tsx +++ b/public/app/core/components/TraceToLogs/TraceToLogsSettings.tsx @@ -7,6 +7,8 @@ import { DataSourcePicker } from '@grafana/runtime'; import { InlineField, InlineFieldRow, Input, InlineSwitch } from '@grafana/ui'; import { ConfigDescriptionLink } from 'app/core/components/ConfigDescriptionLink'; +import { IntervalInput } from '../IntervalInput/IntervalInput'; + import { TagMappingInput } from './TagMappingInput'; // @deprecated use getTraceToLogsOptions to get the v2 version of this config from jsonData @@ -123,15 +125,23 @@ export function TraceToLogsSettings({ options, onOptionsChange }: Props) { - updateTracesToLogs({ spanStartTimeShift: val })} + onChange={(val) => { + updateTracesToLogs({ spanStartTimeShift: val }); + }} + isInvalidError={invalidTimeShiftError} /> - updateTracesToLogs({ spanEndTimeShift: val })} + onChange={(val) => { + updateTracesToLogs({ spanEndTimeShift: val }); + }} + isInvalidError={invalidTimeShiftError} /> @@ -222,31 +232,15 @@ function IdFilter(props: IdFilterProps) { ); } -interface TimeRangeShiftProps { - type: 'start' | 'end'; - value: string; - onChange: (val: string) => void; -} -function TimeRangeShift(props: TimeRangeShiftProps) { - return ( - - - props.onChange(e.currentTarget.value)} - value={props.value} - /> - - - ); -} +export const getTimeShiftLabel = (type: 'start' | 'end') => { + return `Span ${type} time shift`; +}; + +export const getTimeShiftTooltip = (type: 'start' | 'end') => { + return `Shifts the ${type} time of the span. Default: 0 (Time units can be used here, for example: 5s, -1m, 3h)`; +}; + +export const invalidTimeShiftError = 'Invalid time shift. See tooltip for examples.'; export const TraceToLogsSection = ({ options, onOptionsChange }: DataSourcePluginOptionsEditorProps) => { return ( diff --git a/public/app/core/components/TraceToMetrics/TraceToMetricsSettings.tsx b/public/app/core/components/TraceToMetrics/TraceToMetricsSettings.tsx index 52bf8a8c6c0..bc3e40da3d2 100644 --- a/public/app/core/components/TraceToMetrics/TraceToMetricsSettings.tsx +++ b/public/app/core/components/TraceToMetrics/TraceToMetricsSettings.tsx @@ -12,7 +12,9 @@ import { DataSourcePicker } from '@grafana/runtime'; import { Button, InlineField, InlineFieldRow, Input, useStyles2 } from '@grafana/ui'; import { ConfigDescriptionLink } from '../ConfigDescriptionLink'; +import { IntervalInput } from '../IntervalInput/IntervalInput'; import { TagMappingInput } from '../TraceToLogs/TagMappingInput'; +import { getTimeShiftLabel, getTimeShiftTooltip, invalidTimeShiftError } from '../TraceToLogs/TraceToLogsSettings'; export interface TraceToMetricsOptions { datasourceUid?: string; @@ -76,49 +78,31 @@ export function TraceToMetricsSettings({ options, onOptionsChange }: Props) { ) : null} - - - - updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'tracesToMetrics', { - ...options.jsonData.tracesToMetrics, - spanStartTimeShift: v.currentTarget.value, - }) - } - value={options.jsonData.tracesToMetrics?.spanStartTimeShift || ''} - /> - - + { + updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'tracesToMetrics', { + ...options.jsonData.tracesToMetrics, + spanStartTimeShift: val, + }); + }} + isInvalidError={invalidTimeShiftError} + /> - - - - updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'tracesToMetrics', { - ...options.jsonData.tracesToMetrics, - spanEndTimeShift: v.currentTarget.value, - }) - } - value={options.jsonData.tracesToMetrics?.spanEndTimeShift || ''} - /> - - + { + updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'tracesToMetrics', { + ...options.jsonData.tracesToMetrics, + spanEndTimeShift: val, + }); + }} + isInvalidError={invalidTimeShiftError} + /> diff --git a/public/app/plugins/datasource/tempo/configuration/QuerySettings.tsx b/public/app/plugins/datasource/tempo/configuration/QuerySettings.tsx index 764f8ec344b..76f0ba7ff71 100644 --- a/public/app/plugins/datasource/tempo/configuration/QuerySettings.tsx +++ b/public/app/plugins/datasource/tempo/configuration/QuerySettings.tsx @@ -2,7 +2,9 @@ import { css } from '@emotion/css'; import React from 'react'; import { DataSourcePluginOptionsEditorProps, GrafanaTheme2, updateDatasourcePluginJsonDataOption } from '@grafana/data'; -import { InlineField, InlineFieldRow, InlineSwitch, Input, useStyles2 } from '@grafana/ui'; +import { InlineField, InlineSwitch, useStyles2 } from '@grafana/ui'; +import { IntervalInput } from 'app/core/components/IntervalInput/IntervalInput'; +import { invalidTimeShiftError } from 'app/core/components/TraceToLogs/TraceToLogsSettings'; import { TempoJsonData } from '../types'; @@ -11,6 +13,14 @@ interface Props extends DataSourcePluginOptionsEditorProps {} export function QuerySettings({ options, onOptionsChange }: Props) { const styles = useStyles2(getStyles); + const getLabel = (type: 'start' | 'end') => { + return `Time shift for ${type} of search`; + }; + + const getTooltip = (type: 'start' | 'end') => { + return `Shifts the ${type} of the time range when searching by TraceID. Searching can return traces that do not fully fall into the search time range, so we recommend using higher time shifts for longer traces. Default: 30m (Time units can be used here, for example: 5s, 1m, 3h`; + }; + return (
- - - - updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'traceQuery', { - ...options.jsonData.traceQuery, - spanStartTimeShift: v.currentTarget.value, - }) - } - value={options.jsonData.traceQuery?.spanStartTimeShift || ''} - /> - - - - - - updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'traceQuery', { - ...options.jsonData.traceQuery, - spanEndTimeShift: v.currentTarget.value, - }) - } - value={options.jsonData.traceQuery?.spanEndTimeShift || ''} - /> - - + + { + updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'traceQuery', { + ...options.jsonData.traceQuery, + spanStartTimeShift: val, + }); + }} + isInvalidError={invalidTimeShiftError} + /> + + { + updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'traceQuery', { + ...options.jsonData.traceQuery, + spanEndTimeShift: val, + }); + }} + isInvalidError={invalidTimeShiftError} + />
); }