Tempo: Search using TraceQL improvements (#64616)

* Added scope to filter

* Intrinsic fields don't have scope

* Consistent plus button placement next to the last tag. Changed All Scopes to "unscoped"

* Added validation to duration fields

* Disable options load when dropdown is opened

* Use focused list of operators when all values are of type string or int/float

* Fixed and added tests

* Fix another test

* Better way to prevent duplicate and redundant backend requests when a filter updates
This commit is contained in:
Andre Pereira 2023-03-21 15:59:16 +00:00 committed by GitHub
parent 6093e45178
commit bfb0dde4a8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 243 additions and 98 deletions

View File

@ -9,6 +9,13 @@
package dataquery
// Defines values for TempoQueryFiltersScope.
const (
TempoQueryFiltersScopeResource TempoQueryFiltersScope = "resource"
TempoQueryFiltersScopeSpan TempoQueryFiltersScope = "span"
TempoQueryFiltersScopeUnscoped TempoQueryFiltersScope = "unscoped"
)
// Defines values for TempoQueryFiltersType.
const (
TempoQueryFiltersTypeDynamic TempoQueryFiltersType = "dynamic"
@ -26,6 +33,13 @@ const (
TempoQueryTypeUpload TempoQueryType = "upload"
)
// Defines values for TraceqlFilterScope.
const (
TraceqlFilterScopeResource TraceqlFilterScope = "resource"
TraceqlFilterScopeSpan TraceqlFilterScope = "span"
TraceqlFilterScopeUnscoped TraceqlFilterScope = "unscoped"
)
// Defines values for TraceqlFilterType.
const (
TraceqlFilterTypeDynamic TraceqlFilterType = "dynamic"
@ -38,6 +52,13 @@ const (
TraceqlSearchFilterTypeStatic TraceqlSearchFilterType = "static"
)
// Defines values for TraceqlSearchScope.
const (
TraceqlSearchScopeResource TraceqlSearchScope = "resource"
TraceqlSearchScopeSpan TraceqlSearchScope = "span"
TraceqlSearchScopeUnscoped TraceqlSearchScope = "unscoped"
)
// TempoDataQuery defines model for TempoDataQuery.
type TempoDataQuery = map[string]interface{}
@ -55,6 +76,9 @@ type TempoQuery struct {
// The operator that connects the tag to the value, for example: =, >, !=, =~
Operator *string `json:"operator,omitempty"`
// The scope of the filter, can either be unscoped/all scopes, resource or span
Scope *TempoQueryFiltersScope `json:"scope,omitempty"`
// The tag for the search filter, for example: .http.status_code, .service.name, status
Tag *string `json:"tag,omitempty"`
@ -107,6 +131,9 @@ type TempoQuery struct {
SpanName *string `json:"spanName,omitempty"`
}
// The scope of the filter, can either be unscoped/all scopes, resource or span
type TempoQueryFiltersScope string
// The type of the filter, can either be static (pre defined in the UI) or dynamic
type TempoQueryFiltersType string
@ -121,6 +148,9 @@ type TraceqlFilter struct {
// The operator that connects the tag to the value, for example: =, >, !=, =~
Operator *string `json:"operator,omitempty"`
// The scope of the filter, can either be unscoped/all scopes, resource or span
Scope *TraceqlFilterScope `json:"scope,omitempty"`
// The tag for the search filter, for example: .http.status_code, .service.name, status
Tag *string `json:"tag,omitempty"`
@ -134,8 +164,14 @@ type TraceqlFilter struct {
ValueType *string `json:"valueType,omitempty"`
}
// The scope of the filter, can either be unscoped/all scopes, resource or span
type TraceqlFilterScope string
// The type of the filter, can either be static (pre defined in the UI) or dynamic
type TraceqlFilterType string
// TraceqlSearchFilterType static fields are pre-set in the UI, dynamic fields are added by the user
type TraceqlSearchFilterType string
// TraceqlSearchScope defines model for TraceqlSearchScope.
type TraceqlSearchScope string

View File

@ -12,7 +12,15 @@ interface Props {
isTagsLoading?: boolean;
operators: string[];
}
const validationRegex = /^\d+(?:\.\d)?\d*(?:ms|s|ns)$/;
const DurationInput = ({ filter, operators, updateFilter }: Props) => {
let invalid = false;
if (typeof filter.value === 'string') {
invalid = filter.value ? !validationRegex.test(filter.value.concat('')) : false;
}
return (
<HorizontalGroup spacing={'none'}>
<Select
@ -34,6 +42,7 @@ const DurationInput = ({ filter, operators, updateFilter }: Props) => {
}}
placeholder="e.g. 100ms, 1.2s"
aria-label={`select ${filter.id} value`}
invalid={invalid}
width={18}
/>
</HorizontalGroup>

View File

@ -73,10 +73,10 @@ describe('SearchField', () => {
if (select) {
await user.click(select);
jest.advanceTimersByTime(1000);
const largerThanOp = await screen.findByText('>');
const largerThanOp = await screen.findByText('!=');
await user.click(largerThanOp);
expect(updateFilter).toHaveBeenCalledWith({ ...filter, operator: '>' });
expect(updateFilter).toHaveBeenCalledWith({ ...filter, operator: '!=' });
}
});

View File

@ -1,19 +1,26 @@
import React, { useState, useEffect, useCallback, useMemo } from 'react';
import { css } from '@emotion/css';
import React, { useState, useEffect, useMemo, useCallback } from 'react';
import { SelectableValue } from '@grafana/data';
import { AccessoryButton } from '@grafana/experimental';
import { FetchError, isFetchError } from '@grafana/runtime';
import { Select, HorizontalGroup } from '@grafana/ui';
import { Select, HorizontalGroup, useStyles2 } from '@grafana/ui';
import { createErrorNotification } from '../../../../core/copy/appNotification';
import { notifyApp } from '../../../../core/reducers/appNotification';
import { dispatch } from '../../../../store/store';
import { TraceqlFilter } from '../dataquery.gen';
import { TraceqlFilter, TraceqlSearchScope } from '../dataquery.gen';
import { TempoDatasource } from '../datasource';
import TempoLanguageProvider from '../language_provider';
import { operators as allOperators } from '../traceql/traceql';
import { operators as allOperators, stringOperators, numberOperators } from '../traceql/traceql';
import { operatorSelectableValue } from './utils';
import { operatorSelectableValue, scopeHelper } from './utils';
const getStyles = () => ({
dropdown: css`
box-shadow: none;
`,
});
interface Props {
filter: TraceqlFilter;
@ -23,21 +30,13 @@ interface Props {
setError: (error: FetchError) => void;
isTagsLoading?: boolean;
tags: string[];
operators?: string[];
}
const SearchField = ({
filter,
datasource,
updateFilter,
deleteFilter,
isTagsLoading,
tags,
setError,
operators,
}: Props) => {
const SearchField = ({ filter, datasource, updateFilter, deleteFilter, isTagsLoading, tags, setError }: Props) => {
const styles = useStyles2(getStyles);
const languageProvider = useMemo(() => new TempoLanguageProvider(datasource), [datasource]);
const [isLoadingValues, setIsLoadingValues] = useState(false);
const [options, setOptions] = useState<Array<SelectableValue<string>>>([]);
const [scopedTag, setScopedTag] = useState(scopeHelper(filter) + filter.tag);
// We automatically change the operator to the regex op when users select 2 or more values
// However, they expect this to be automatically rolled back to the previous operator once
// there's only one value selected, so we store the previous operator and value
@ -58,53 +57,69 @@ const SearchField = ({
setPrevValue(filter.value);
}, [filter.value]);
const loadOptions = useCallback(
async (name: string) => {
setIsLoadingValues(true);
useEffect(() => {
const newScopedTag = scopeHelper(filter) + filter.tag;
if (newScopedTag !== scopedTag) {
setScopedTag(newScopedTag);
}
}, [filter, scopedTag]);
try {
const options = await languageProvider.getOptionsV2(name);
return options;
} catch (error) {
if (isFetchError(error) && error?.status === 404) {
setError(error);
} else if (error instanceof Error) {
dispatch(notifyApp(createErrorNotification('Error', error)));
}
return [];
} finally {
setIsLoadingValues(false);
const updateOptions = useCallback(async () => {
try {
setIsLoadingValues(true);
setOptions(await languageProvider.getOptionsV2(scopedTag));
} catch (error) {
// Display message if Tempo is connected but search 404's
if (isFetchError(error) && error?.status === 404) {
setError(error);
} else if (error instanceof Error) {
dispatch(notifyApp(createErrorNotification('Error', error)));
}
},
[setError, languageProvider]
);
} finally {
setIsLoadingValues(false);
}
}, [scopedTag, languageProvider, setError]);
useEffect(() => {
const fetchOptions = async () => {
try {
if (filter.tag) {
setOptions(await loadOptions(filter.tag));
}
} catch (error) {
// Display message if Tempo is connected but search 404's
if (isFetchError(error) && error?.status === 404) {
setError(error);
} else if (error instanceof Error) {
dispatch(notifyApp(createErrorNotification('Error', error)));
}
}
};
fetchOptions();
}, [languageProvider, loadOptions, setError, filter.tag]);
updateOptions();
}, [updateOptions]);
const scopeOptions = Object.values(TraceqlSearchScope).map((t) => ({ label: t, value: t }));
// If all values have type string or int/float use a focused list of operators instead of all operators
const optionsOfFirstType = options.filter((o) => o.type === options[0]?.type);
const uniqueOptionType = options.length === optionsOfFirstType.length ? options[0]?.type : undefined;
let operatorList = allOperators;
switch (uniqueOptionType) {
case 'string':
operatorList = stringOperators;
break;
case 'int':
case 'float':
operatorList = numberOperators;
}
return (
<HorizontalGroup spacing={'none'}>
<HorizontalGroup spacing={'none'} width={'auto'}>
{filter.type === 'dynamic' && (
<Select
className={styles.dropdown}
inputId={`${filter.id}-scope`}
options={scopeOptions}
value={filter.scope}
onChange={(v) => {
updateFilter({ ...filter, scope: v?.value });
}}
placeholder="Select scope"
aria-label={`select ${filter.id} scope`}
/>
)}
{filter.type === 'dynamic' && (
<Select
className={styles.dropdown}
inputId={`${filter.id}-tag`}
isLoading={isTagsLoading}
options={tags.map((t) => ({ label: t, value: t }))}
onOpenMenu={() => tags}
value={filter.tag}
onChange={(v) => {
updateFilter({ ...filter, tag: v?.value });
@ -116,8 +131,9 @@ const SearchField = ({
/>
)}
<Select
className={styles.dropdown}
inputId={`${filter.id}-operator`}
options={(operators || allOperators).map(operatorSelectableValue)}
options={operatorList.map(operatorSelectableValue)}
value={filter.operator}
onChange={(v) => {
updateFilter({ ...filter, operator: v?.value });
@ -128,14 +144,10 @@ const SearchField = ({
width={8}
/>
<Select
className={styles.dropdown}
inputId={`${filter.id}-value`}
isLoading={isLoadingValues}
options={options}
onOpenMenu={() => {
if (filter.tag) {
loadOptions(filter.tag);
}
}}
value={filter.value}
onChange={(val) => {
if (Array.isArray(val)) {

View File

@ -1,15 +1,29 @@
import React, { useEffect, useCallback } from 'react';
import { css } from '@emotion/css';
import React, { useCallback, useEffect } from 'react';
import { v4 as uuidv4 } from 'uuid';
import { AccessoryButton } from '@grafana/experimental';
import { FetchError } from '@grafana/runtime';
import { HorizontalGroup, VerticalGroup } from '@grafana/ui';
import { useStyles2 } from '@grafana/ui';
import { TraceqlFilter } from '../dataquery.gen';
import { TraceqlFilter, TraceqlSearchScope } from '../dataquery.gen';
import { TempoDatasource } from '../datasource';
import SearchField from './SearchField';
const getStyles = () => ({
vertical: css`
display: flex;
flex-direction: column;
gap: 0.25rem;
`,
horizontal: css`
display: flex;
flex-direction: row;
gap: 1rem;
`,
});
interface Props {
updateFilter: (f: TraceqlFilter) => void;
deleteFilter: (f: TraceqlFilter) => void;
@ -20,9 +34,10 @@ interface Props {
isTagsLoading: boolean;
}
const TagsInput = ({ updateFilter, deleteFilter, filters, datasource, setError, tags, isTagsLoading }: Props) => {
const styles = useStyles2(getStyles);
const generateId = () => uuidv4().slice(0, 8);
const handleOnAdd = useCallback(
() => updateFilter({ id: generateId(), type: 'dynamic', operator: '=' }),
() => updateFilter({ id: generateId(), type: 'dynamic', operator: '=', scope: TraceqlSearchScope.Span }),
[updateFilter]
);
@ -32,26 +47,27 @@ const TagsInput = ({ updateFilter, deleteFilter, filters, datasource, setError,
}
}, [filters, handleOnAdd]);
const dynamicFilters = filters?.filter((f) => f.type === 'dynamic');
return (
<HorizontalGroup spacing={'md'} align={'flex-start'}>
<VerticalGroup spacing={'xs'}>
{filters
?.filter((f) => f.type === 'dynamic')
.map((f) => (
<SearchField
filter={f}
key={f.id}
datasource={datasource}
setError={setError}
updateFilter={updateFilter}
tags={tags}
isTagsLoading={isTagsLoading}
deleteFilter={deleteFilter}
/>
))}
</VerticalGroup>
<AccessoryButton variant={'secondary'} icon={'plus'} onClick={handleOnAdd} title={'Add tag'} />
</HorizontalGroup>
<div className={styles.vertical}>
{dynamicFilters?.map((f, i) => (
<div className={styles.horizontal} key={f.id}>
<SearchField
filter={f}
datasource={datasource}
setError={setError}
updateFilter={updateFilter}
tags={tags}
isTagsLoading={isTagsLoading}
deleteFilter={deleteFilter}
/>
{i === dynamicFilters.length - 1 && (
<AccessoryButton variant={'secondary'} icon={'plus'} onClick={handleOnAdd} title={'Add tag'} />
)}
</div>
))}
</div>
);
};

View File

@ -2,6 +2,7 @@ import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React from 'react';
import { TraceqlSearchScope } from '../dataquery.gen';
import { TempoDatasource } from '../datasource';
import { TempoQuery } from '../types';
@ -101,7 +102,8 @@ describe('TraceQLSearch', () => {
expect(nameFilter).not.toBeNull();
expect(nameFilter?.operator).toBe('=');
expect(nameFilter?.value).toStrictEqual(['customer']);
expect(nameFilter?.tag).toBe('.service.name');
expect(nameFilter?.tag).toBe('service.name');
expect(nameFilter?.scope).toBe(TraceqlSearchScope.Resource);
}
});

View File

@ -1,5 +1,5 @@
import { css } from '@emotion/css';
import React, { useState, useEffect } from 'react';
import React, { useEffect, useState } from 'react';
import { GrafanaTheme2 } from '@grafana/data';
import { EditorRow } from '@grafana/experimental';
@ -10,7 +10,7 @@ import { createErrorNotification } from '../../../../core/copy/appNotification';
import { notifyApp } from '../../../../core/reducers/appNotification';
import { dispatch } from '../../../../store/store';
import { RawQuery } from '../../prometheus/querybuilder/shared/RawQuery';
import { TraceqlFilter } from '../dataquery.gen';
import { TraceqlFilter, TraceqlSearchScope } from '../dataquery.gen';
import { TempoDatasource } from '../datasource';
import { TempoQueryBuilderOptions } from '../traceql/TempoQueryBuilderOptions';
import { CompletionProvider } from '../traceql/autocomplete';
@ -73,8 +73,7 @@ const TraceQLSearch = ({ datasource, query, onChange }: Props) => {
if (!tags.find((t) => t === 'status')) {
tags.push('status');
}
const tagsWithDot = tags.sort().map((t) => `.${t}`);
setTags(tagsWithDot);
setTags(tags);
setIsTagsLoading(false);
}
} catch (error) {
@ -96,15 +95,15 @@ const TraceQLSearch = ({ datasource, query, onChange }: Props) => {
findFilter('service-name') || {
id: 'service-name',
type: 'static',
tag: '.service.name',
tag: 'service.name',
operator: '=',
scope: TraceqlSearchScope.Resource,
}
}
datasource={datasource}
setError={setError}
updateFilter={updateFilter}
tags={[]}
operators={['=', '!=', '=~']}
/>
</InlineSearchField>
<InlineSearchField label={'Span Name'}>
@ -114,7 +113,6 @@ const TraceQLSearch = ({ datasource, query, onChange }: Props) => {
setError={setError}
updateFilter={updateFilter}
tags={[]}
operators={['=', '!=', '=~']}
/>
</InlineSearchField>
<InlineSearchField label={'Duration'} tooltip="The span duration, i.e. end - start time of the span">

View File

@ -1,3 +1,5 @@
import { TraceqlSearchScope } from '../dataquery.gen';
import { generateQueryFromFilters } from './utils';
describe('generateQueryFromFilters generates the correct query for', () => {
@ -20,7 +22,7 @@ describe('generateQueryFromFilters generates the correct query for', () => {
it('a field with tag, operator and tag', () => {
expect(
generateQueryFromFilters([{ id: 'foo', type: 'static', tag: 'footag', value: 'foovalue', operator: '=' }])
).toBe('{footag="foovalue"}');
).toBe('{.footag="foovalue"}');
});
it('a field with valueType as integer', () => {
@ -28,7 +30,7 @@ describe('generateQueryFromFilters generates the correct query for', () => {
generateQueryFromFilters([
{ id: 'foo', type: 'static', tag: 'footag', value: '1234', operator: '>', valueType: 'integer' },
])
).toBe('{footag>1234}');
).toBe('{.footag>1234}');
});
it('two fields with everything filled in', () => {
expect(
@ -36,7 +38,7 @@ describe('generateQueryFromFilters generates the correct query for', () => {
{ id: 'foo', type: 'static', tag: 'footag', value: '1234', operator: '>=', valueType: 'integer' },
{ id: 'bar', type: 'dynamic', tag: 'bartag', value: 'barvalue', operator: '=', valueType: 'string' },
])
).toBe('{footag>=1234 && bartag="barvalue"}');
).toBe('{.footag>=1234 && .bartag="barvalue"}');
});
it('two fields but one is missing a value', () => {
expect(
@ -44,7 +46,7 @@ describe('generateQueryFromFilters generates the correct query for', () => {
{ id: 'foo', type: 'static', tag: 'footag', value: '1234', operator: '>=', valueType: 'integer' },
{ id: 'bar', type: 'dynamic', tag: 'bartag', operator: '=', valueType: 'string' },
])
).toBe('{footag>=1234}');
).toBe('{.footag>=1234}');
});
it('two fields but one is missing a value and the other a tag', () => {
expect(
@ -54,4 +56,49 @@ describe('generateQueryFromFilters generates the correct query for', () => {
])
).toBe('{}');
});
it('scope is unscoped', () => {
expect(
generateQueryFromFilters([
{
id: 'foo',
type: 'static',
tag: 'footag',
value: '1234',
operator: '>=',
scope: TraceqlSearchScope.Unscoped,
valueType: 'integer',
},
])
).toBe('{.footag>=1234}');
});
it('scope is span', () => {
expect(
generateQueryFromFilters([
{
id: 'foo',
type: 'static',
tag: 'footag',
value: '1234',
operator: '>=',
scope: TraceqlSearchScope.Span,
valueType: 'integer',
},
])
).toBe('{span.footag>=1234}');
});
it('scope is resource', () => {
expect(
generateQueryFromFilters([
{
id: 'foo',
type: 'static',
tag: 'footag',
value: '1234',
operator: '>=',
scope: TraceqlSearchScope.Resource,
valueType: 'integer',
},
])
).toBe('{resource.footag>=1234}');
});
});

View File

@ -1,11 +1,12 @@
import { SelectableValue } from '@grafana/data';
import { TraceqlFilter } from '../dataquery.gen';
import { TraceqlFilter, TraceqlSearchScope } from '../dataquery.gen';
import { CompletionProvider } from '../traceql/autocomplete';
export const generateQueryFromFilters = (filters: TraceqlFilter[]) => {
return `{${filters
.filter((f) => f.tag && f.operator && f.value?.length)
.map((f) => `${f.tag}${f.operator}${valueHelper(f)}`)
.map((f) => `${scopeHelper(f)}${f.tag}${f.operator}${valueHelper(f)}`)
.join(' && ')}}`;
};
@ -18,6 +19,15 @@ const valueHelper = (f: TraceqlFilter) => {
}
return f.value;
};
export const scopeHelper = (f: TraceqlFilter) => {
// Intrinsic fields don't have a scope
if (CompletionProvider.intrinsics.find((t) => t === f.tag)) {
return '';
}
return (
(f.scope === TraceqlSearchScope.Resource || f.scope === TraceqlSearchScope.Span ? f.scope?.toLowerCase() : '') + '.'
);
};
export function replaceAt<T>(array: T[], index: number, value: T) {
const ret = array.slice(0);

View File

@ -54,7 +54,8 @@ composableKinds: DataQuery: {
#TempoQueryType: "traceql" | "traceqlSearch" | "search" | "serviceMap" | "upload" | "nativeSearch" | "clear" @cuetsy(kind="type")
// static fields are pre-set in the UI, dynamic fields are added by the user
#TraceqlSearchFilterType: "static" | "dynamic" @cuetsy(kind="type")
#TraceqlSearchFilterType: "static" | "dynamic" @cuetsy(kind="type")
#TraceqlSearchScope: "unscoped" | "resource" | "span" @cuetsy(kind="enum")
#TraceqlFilter: {
// Uniquely identify the filter, will not be used in the query generation
id: string
@ -68,6 +69,8 @@ composableKinds: DataQuery: {
value?: string | [...string]
// The type of the value, used for example to check whether we need to wrap the value in quotes when generating the query
valueType?: string
// The scope of the filter, can either be unscoped/all scopes, resource or span
scope?: #TraceqlSearchScope
} @cuetsy(kind="interface")
},
]

View File

@ -62,6 +62,12 @@ export type TempoQueryType = ('traceql' | 'traceqlSearch' | 'search' | 'serviceM
*/
export type TraceqlSearchFilterType = ('static' | 'dynamic');
export enum TraceqlSearchScope {
Resource = 'resource',
Span = 'span',
Unscoped = 'unscoped',
}
export interface TraceqlFilter {
/**
* Uniquely identify the filter, will not be used in the query generation
@ -71,6 +77,10 @@ export interface TraceqlFilter {
* The operator that connects the tag to the value, for example: =, >, !=, =~
*/
operator?: string;
/**
* The scope of the filter, can either be unscoped/all scopes, resource or span
*/
scope?: TraceqlSearchScope;
/**
* The tag for the search filter, for example: .http.status_code, .service.name, status
*/

View File

@ -23,6 +23,8 @@ export const languageConfiguration = {
};
export const operators = ['=', '!=', '>', '<', '>=', '<=', '=~'];
export const stringOperators = ['=', '!=', '=~'];
export const numberOperators = ['=', '!=', '>', '<', '>=', '<='];
const intrinsics = ['duration', 'name', 'status', 'parent'];