Tempo: Improve search form defaults and validation (#39534)

* Tempo: add default limit, option to hide Loki search, and run query on hotkey in dropdowns

packages/grafana-data/src/datetime/durationutil.test.ts

@@ -1,4 +1,10 @@
-import { intervalToAbbreviatedDurationString, addDurationToDate, parseDuration } from './durationutil';
+import {
+  intervalToAbbreviatedDurationString,
+  addDurationToDate,
+  parseDuration,
+  isValidDuration,
+  isValidGoDuration,
+} from './durationutil';
 
 describe('Duration util', () => {
   describe('intervalToAbbreviatedDurationString', () => {
@@ -20,4 +26,28 @@ describe('Duration util', () => {
       expect(parseDuration(durationString)).toEqual({ months: '3', minutes: '4' });
     });
   });
+
+  describe('isValidDuration', () => {
+    it('valid duration string returns true', () => {
+      const durationString = '3M 5d 20m';
+      expect(isValidDuration(durationString)).toEqual(true);
+    });
+
+    it('invalid duration string returns false', () => {
+      const durationString = '3M 6v 5b 4m';
+      expect(isValidDuration(durationString)).toEqual(false);
+    });
+  });
+
+  describe('isValidGoDuration', () => {
+    it('valid duration string returns true', () => {
+      const durationString = '3h 4m 1s 2ms 3us 5ns';
+      expect(isValidGoDuration(durationString)).toEqual(true);
+    });
+
+    it('invalid duration string returns false', () => {
+      const durationString = '3M 6v 5b 4m';
+      expect(isValidGoDuration(durationString)).toEqual(false);
+    });
+  });
 });

packages/grafana-data/src/datetime/durationutil.ts

@@ -13,7 +13,7 @@ const durationMap: { [key in Required<keyof Duration>]: string[] } = {
 };
 
 /**
- * intervalToAbbreviatedDurationString convers interval to readable duration string
+ * intervalToAbbreviatedDurationString converts interval to readable duration string
  *
  * @param interval - interval to convert
  * @param includeSeconds - optional, default true. If false, will not include seconds unless interval is less than 1 minute
@@ -85,3 +85,55 @@ export function durationToMilliseconds(duration: Duration): number {
 export function isValidDate(dateString: string): boolean {
   return !isNaN(Date.parse(dateString));
 }
+
+/**
+ * isValidDuration returns true if the given string can be parsed into a valid Duration object, false otherwise
+ *
+ * @param durationString - string representation of a duration
+ *
+ * @public
+ */
+export function isValidDuration(durationString: string): boolean {
+  for (const value of durationString.trim().split(' ')) {
+    const match = value.match(/(\d+)(.+)/);
+    if (match === null || match.length !== 3) {
+      return false;
+    }
+
+    const key = Object.entries(durationMap).find(([_, abbreviations]) => abbreviations?.includes(match[2]))?.[0];
+    if (!key) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+/**
+ * isValidGoDuration returns true if the given string can be parsed into a valid Duration object based on
+ * Go's time.parseDuration, false otherwise.
+ *
+ * Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
+ *
+ * Go docs: https://pkg.go.dev/time#ParseDuration
+ *
+ * @param durationString - string representation of a duration
+ *
+ * @internal
+ */
+export function isValidGoDuration(durationString: string): boolean {
+  const timeUnits = ['h', 'm', 's', 'ms', 'us', 'µs', 'ns'];
+  for (const value of durationString.trim().split(' ')) {
+    const match = value.match(/(\d+)(.+)/);
+    if (match === null || match.length !== 3) {
+      return false;
+    }
+
+    const isValidUnit = timeUnits.includes(match[2]);
+    if (!isValidUnit) {
+      return false;
+    }
+  }
+
+  return true;
+}

public/app/core/components/TraceToLogsSettings.tsx

@@ -16,6 +16,7 @@ export interface TraceToLogsOptions {
   spanEndTimeShift?: string;
   filterByTraceID?: boolean;
   filterBySpanID?: boolean;
+  lokiSearch?: boolean;
 }
 
 export interface TraceToLogsData extends DataSourceJsonData {
@@ -152,6 +153,20 @@ export function TraceToLogsSettings({ options, onOptionsChange }: Props) {
           />
         </InlineField>
       </InlineFieldRow>
+      <InlineFieldRow>
+        <InlineField label="Loki Search" labelWidth={26} grow tooltip="Use this logs data source to search for traces.">
+          <InlineSwitch
+            defaultChecked={true}
+            value={options.jsonData.tracesToLogs?.lokiSearch}
+            onChange={(event: React.SyntheticEvent<HTMLInputElement>) =>
+              updateDatasourcePluginJsonDataOption({ onOptionsChange, options }, 'tracesToLogs', {
+                ...options.jsonData.tracesToLogs,
+                lokiSearch: event.currentTarget.checked,
+              })
+            }
+          />
+        </InlineField>
+      </InlineFieldRow>
     </div>
   );
 }

public/app/plugins/datasource/tempo/NativeSearch.tsx

@@ -16,7 +16,7 @@ import { tokenizer } from './syntax';
 import Prism from 'prismjs';
 import { Node } from 'slate';
 import { css } from '@emotion/css';
-import { GrafanaTheme2, SelectableValue } from '@grafana/data';
+import { GrafanaTheme2, isValidGoDuration, SelectableValue } from '@grafana/data';
 import TempoLanguageProvider from './language_provider';
 import { TempoDatasource, TempoQuery } from './datasource';
 import { debounce } from 'lodash';
@@ -56,6 +56,7 @@ const NativeSearch = ({ datasource, query, onChange, onBlur, onRunQuery }: Props
     spanNameOptions: [],
   });
   const [error, setError] = useState(null);
+  const [inputErrors, setInputErrors] = useState<{ [key: string]: boolean }>({});
 
   const fetchServiceNameOptions = useMemo(
     () =>
@@ -139,6 +140,7 @@ const NativeSearch = ({ datasource, query, onChange, onBlur, onRunQuery }: Props
               placeholder="Select a service"
               onOpenMenu={fetchServiceNameOptions}
               isClearable
+              onKeyDown={onKeyDown}
             />
           </InlineField>
         </InlineFieldRow>
@@ -157,6 +159,7 @@ const NativeSearch = ({ datasource, query, onChange, onBlur, onRunQuery }: Props
               placeholder="Select a span"
               onOpenMenu={fetchSpanNameOptions}
               isClearable
+              onKeyDown={onKeyDown}
             />
           </InlineField>
         </InlineFieldRow>
@@ -182,10 +185,17 @@ const NativeSearch = ({ datasource, query, onChange, onBlur, onRunQuery }: Props
           </InlineField>
         </InlineFieldRow>
         <InlineFieldRow>
-          <InlineField label="Min Duration" labelWidth={14} grow>
+          <InlineField label="Min Duration" invalid={!!inputErrors.minDuration} labelWidth={14} grow>
             <Input
               value={query.minDuration || ''}
               placeholder={durationPlaceholder}
+              onBlur={() => {
+                if (query.minDuration && !isValidGoDuration(query.minDuration)) {
+                  setInputErrors({ ...inputErrors, minDuration: true });
+                } else {
+                  setInputErrors({ ...inputErrors, minDuration: false });
+                }
+              }}
               onChange={(v) =>
                 onChange({
                   ...query,
@@ -197,10 +207,17 @@ const NativeSearch = ({ datasource, query, onChange, onBlur, onRunQuery }: Props
           </InlineField>
         </InlineFieldRow>
         <InlineFieldRow>
-          <InlineField label="Max Duration" labelWidth={14} grow>
+          <InlineField label="Max Duration" invalid={!!inputErrors.maxDuration} labelWidth={14} grow>
             <Input
               value={query.maxDuration || ''}
               placeholder={durationPlaceholder}
+              onBlur={() => {
+                if (query.maxDuration && !isValidGoDuration(query.maxDuration)) {
+                  setInputErrors({ ...inputErrors, maxDuration: true });
+                } else {
+                  setInputErrors({ ...inputErrors, maxDuration: false });
+                }
+              }}
               onChange={(v) =>
                 onChange({
                   ...query,
@@ -212,16 +229,29 @@ const NativeSearch = ({ datasource, query, onChange, onBlur, onRunQuery }: Props
           </InlineField>
         </InlineFieldRow>
         <InlineFieldRow>
-          <InlineField label="Limit" labelWidth={14} grow tooltip="Maximum numbers of returned results">
+          <InlineField
+            label="Limit"
+            invalid={!!inputErrors.limit}
+            labelWidth={14}
+            grow
+            tooltip="Maximum numbers of returned results"
+          >
             <Input
               value={query.limit || ''}
               type="number"
-              onChange={(v) =>
+              onChange={(v) => {
+                let limit = v.currentTarget.value ? parseInt(v.currentTarget.value, 10) : undefined;
+                if (limit && (!Number.isInteger(limit) || limit <= 0)) {
+                  setInputErrors({ ...inputErrors, limit: true });
+                } else {
+                  setInputErrors({ ...inputErrors, limit: false });
+                }
+
                 onChange({
                   ...query,
                   limit: v.currentTarget.value ? parseInt(v.currentTarget.value, 10) : undefined,
-                })
-              }
+                });
+              }}
               onKeyDown={onKeyDown}
             />
           </InlineField>
@@ -230,7 +260,7 @@ const NativeSearch = ({ datasource, query, onChange, onBlur, onRunQuery }: Props
       {error ? (
         <Alert title="Unable to connect to Tempo search" severity="info" className={styles.alert}>
           Please ensure that Tempo is configured with search enabled. If you would like to hide this tab, you can
-          configure it in the <a href={`/datasources/${datasource.uid}`}>datasource settings</a>.
+          configure it in the <a href={`/datasources/edit/${datasource.uid}`}>datasource settings</a>.
         </Alert>
       ) : null}
     </>

public/app/plugins/datasource/tempo/QueryField.tsx

@@ -103,7 +103,7 @@ class TempoQueryFieldComponent extends React.PureComponent<Props, State> {
       queryTypeOptions.unshift({ value: 'nativeSearch', label: 'Search - Beta' });
     }
 
-    if (logsDatasourceUid) {
+    if (logsDatasourceUid && tracesToLogsOptions?.lokiSearch !== false) {
       if (!config.featureToggles.tempoSearch) {
         // Place at beginning as Search if no native search
         queryTypeOptions.unshift({ value: 'search', label: 'Search' });

public/app/plugins/datasource/tempo/datasource.test.ts

@@ -155,6 +155,20 @@ describe('Tempo data source', () => {
     });
   });
 
+  it('should include a default limit of 100', () => {
+    const ds = new TempoDatasource(defaultSettings);
+    const tempoQuery: TempoQuery = {
+      queryType: 'search',
+      refId: 'A',
+      query: '',
+      search: '',
+    };
+    const builtQuery = ds.buildSearchQuery(tempoQuery);
+    expect(builtQuery).toStrictEqual({
+      limit: 100,
+    });
+  });
+
   it('should ignore incomplete tag queries', () => {
     const ds = new TempoDatasource(defaultSettings);
     const tempoQuery: TempoQuery = {
@@ -165,6 +179,7 @@ describe('Tempo data source', () => {
     };
     const builtQuery = ds.buildSearchQuery(tempoQuery);
     expect(builtQuery).toStrictEqual({
+      limit: 100,
       'root.http.status_code': '500',
     });
   });

public/app/plugins/datasource/tempo/datasource.ts

@@ -1,5 +1,5 @@
 import { from, merge, Observable, of, throwError } from 'rxjs';
-import { map, mergeMap, toArray } from 'rxjs/operators';
+import { catchError, map, mergeMap, toArray } from 'rxjs/operators';
 import {
   DataQuery,
   DataQueryRequest,
@@ -7,6 +7,7 @@ import {
   DataSourceApi,
   DataSourceInstanceSettings,
   DataSourceJsonData,
+  isValidGoDuration,
   LoadingState,
 } from '@grafana/data';
 import { TraceToLogsOptions } from 'app/core/components/TraceToLogsSettings';
@@ -109,6 +110,7 @@ export class TempoDatasource extends DataSourceWithBackend<TempoQuery, TempoJson
     }
 
     if (targets.nativeSearch?.length) {
+      try {
         const searchQuery = this.buildSearchQuery(targets.nativeSearch[0]);
         subQueries.push(
           this._request('/api/search', searchQuery).pipe(
@@ -116,9 +118,15 @@ export class TempoDatasource extends DataSourceWithBackend<TempoQuery, TempoJson
               return {
                 data: [createTableFrameFromSearch(response.data.traces, this.instanceSettings)],
               };
+            }),
+            catchError((error) => {
+              return of({ error: { message: error.data.message }, data: [] });
             })
           )
         );
+      } catch (error) {
+        return of({ error: { message: error.message }, data: [] });
+      }
     }
 
     if (targets.upload?.length) {
@@ -204,6 +212,8 @@ export class TempoDatasource extends DataSourceWithBackend<TempoQuery, TempoJson
 
       // Ensure there is a valid key value pair with accurate types
       if (
+        token &&
+        lookupToken &&
         typeof token !== 'string' &&
         token.type === 'key' &&
         typeof token.content === 'string' &&
@@ -218,12 +228,37 @@ export class TempoDatasource extends DataSourceWithBackend<TempoQuery, TempoJson
     let tempoQuery = pick(query, ['minDuration', 'maxDuration', 'limit']);
     // Remove empty properties
     tempoQuery = pickBy(tempoQuery, identity);
+
     if (query.serviceName) {
       tagsQuery.push({ ['service.name']: query.serviceName });
     }
     if (query.spanName) {
       tagsQuery.push({ ['name']: query.spanName });
     }
+
+    // Set default limit
+    if (!tempoQuery.limit) {
+      tempoQuery.limit = 100;
+    }
+
+    // Validate query inputs and remove spaces if valid
+    if (tempoQuery.minDuration) {
+      if (!isValidGoDuration(tempoQuery.minDuration)) {
+        throw new Error('Please enter a valid min duration.');
+      }
+      tempoQuery.minDuration = tempoQuery.minDuration.replace(/\s/g, '');
+    }
+    if (tempoQuery.maxDuration) {
+      if (!isValidGoDuration(tempoQuery.maxDuration)) {
+        throw new Error('Please enter a valid max duration.');
+      }
+      tempoQuery.maxDuration = tempoQuery.maxDuration.replace(/\s/g, '');
+    }
+
+    if (!Number.isInteger(tempoQuery.limit) || tempoQuery.limit <= 0) {
+      throw new Error('Please enter a valid limit.');
+    }
+
     const tagsQueryObject = tagsQuery.reduce((tagQuery, item) => ({ ...tagQuery, ...item }), {});
     return { ...tagsQueryObject, ...tempoQuery };
   }