Tempo: Improve handling of multiple values in the Search tab query generation (#98427)

Fix #67082. Fix selecting more than one value for an attribute
This commit is contained in:
Andre Pereira 2025-01-06 12:08:16 +00:00 committed by GitHub
parent 5482742891
commit c48406753f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 111 additions and 39 deletions

View File

@ -1,6 +1,6 @@
import { css } from '@emotion/css';
import { uniq } from 'lodash';
import { useState, useEffect, useMemo } from 'react';
import { useState, useMemo } from 'react';
import useAsync from 'react-use/lib/useAsync';
import { SelectableValue } from '@grafana/data';
@ -57,11 +57,6 @@ const SearchField = ({
() => filterScopedTag(filter, datasource.languageProvider),
[datasource.languageProvider, filter]
);
// 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
const [prevOperator, setPrevOperator] = useState(filter.operator);
const [prevValue, setPrevValue] = useState(filter.value);
const [tagQuery, setTagQuery] = useState<string>('');
const [tagValuesQuery, setTagValuesQuery] = useState<string>('');
@ -94,25 +89,6 @@ const SearchField = ({
options.push({ label: filter.value.toString(), value: filter.value.toString(), type: filter.valueType });
}
useEffect(() => {
if (
Array.isArray(filter.value) &&
filter.value.length > 1 &&
filter.operator !== '=~' &&
filter.operator !== '!~'
) {
setPrevOperator(filter.operator);
updateFilter({ ...filter, operator: '=~' });
}
if (Array.isArray(filter.value) && filter.value.length <= 1 && (prevValue?.length || 0) > 1) {
updateFilter({ ...filter, operator: prevOperator, value: filter.value[0] });
}
}, [prevValue, prevOperator, updateFilter, filter]);
useEffect(() => {
setPrevValue(filter.value);
}, [filter.value]);
const scopeOptions = Object.values(TraceqlSearchScope)
.filter((s) => {
// only add scope if it has tags

View File

@ -126,7 +126,7 @@ describe('TagsInput', () => {
setError={() => {}}
staticTags={[]}
isTagsLoading={false}
query={''}
generateQueryWithoutFilter={() => ''}
addVariablesToOptions={true}
/>
);

View File

@ -32,6 +32,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
interface Props {
updateFilter: (f: TraceqlFilter) => void;
deleteFilter: (f: TraceqlFilter) => void;
generateQueryWithoutFilter: (f?: TraceqlFilter) => string;
filters: TraceqlFilter[];
datasource: TempoDatasource;
setError: (error: FetchError | null) => void;
@ -39,7 +40,6 @@ interface Props {
isTagsLoading: boolean;
hideValues?: boolean;
requireTagAndValue?: boolean;
query: string;
addVariablesToOptions?: boolean;
}
const TagsInput = ({
@ -52,7 +52,7 @@ const TagsInput = ({
isTagsLoading,
hideValues,
requireTagAndValue,
query,
generateQueryWithoutFilter,
addVariablesToOptions,
}: Props) => {
const styles = useStyles2(getStyles);
@ -89,7 +89,7 @@ const TagsInput = ({
tags={getTags(f)}
isTagsLoading={isTagsLoading}
hideValue={hideValues}
query={query}
query={generateQueryWithoutFilter(f)}
addVariablesToOptions={addVariablesToOptions}
/>
{(validInput(f) || filters.length > 1) && (

View File

@ -113,6 +113,19 @@ const TraceQLSearch = ({ datasource, query, onChange, onClearResults, app, addVa
f.id !== 'duration-type'
);
// We use this function to generate queries without a specfic filter.
// This is useful because we're sending the query to Tempo so it can return the attributes and values filtered down.
// However, if we send the full query then we won't see more values for the filter we're trying to edit.
// For example, if we already have a service.name value selected and try to add another one, we won't see the other
// values if we send the full query since Tempo will only return the service.name that's already selected.
const generateQueryWithoutFilter = (filter?: TraceqlFilter) => {
if (!filter) {
return traceQlQuery;
}
const filtersAfterRemoval = query.filters?.filter((f) => f.id !== filter.id) || [];
return datasource.languageProvider.generateQueryFromFilters(interpolateFilters(filtersAfterRemoval || []));
};
return (
<>
<div className={styles.container}>
@ -136,7 +149,7 @@ const TraceQLSearch = ({ datasource, query, onChange, onClearResults, app, addVa
tags={[]}
hideScope={true}
hideTag={true}
query={traceQlQuery}
query={generateQueryWithoutFilter(findFilter(f.id))}
addVariablesToOptions={addVariablesToOptions}
/>
</InlineSearchField>
@ -158,7 +171,7 @@ const TraceQLSearch = ({ datasource, query, onChange, onClearResults, app, addVa
tags={[]}
hideScope={true}
hideTag={true}
query={traceQlQuery}
query={generateQueryWithoutFilter(findFilter('status'))}
isMulti={false}
allowCustomValue={false}
addVariablesToOptions={addVariablesToOptions}
@ -219,7 +232,7 @@ const TraceQLSearch = ({ datasource, query, onChange, onClearResults, app, addVa
deleteFilter={deleteFilter}
staticTags={staticTags}
isTagsLoading={isTagsLoading}
query={traceQlQuery}
generateQueryWithoutFilter={generateQueryWithoutFilter}
requireTagAndValue={true}
addVariablesToOptions={addVariablesToOptions}
/>

View File

@ -1,10 +1,17 @@
import { uniq } from 'lodash';
import { TraceqlSearchScope } from '../dataquery.gen';
import { TraceqlFilter, TraceqlSearchScope } from '../dataquery.gen';
import { TempoDatasource } from '../datasource';
import TempoLanguageProvider from '../language_provider';
import { getUnscopedTags, getFilteredTags, getAllTags, getTagsByScope, generateQueryFromAdHocFilters } from './utils';
import {
filterToQuerySection,
generateQueryFromAdHocFilters,
getAllTags,
getFilteredTags,
getTagsByScope,
getUnscopedTags,
} from './utils';
const datasource: TempoDatasource = {
search: {
@ -101,6 +108,76 @@ describe('gets correct tags', () => {
});
});
describe('filterToQuerySection returns the correct query section for a filter', () => {
it('filter with single value', () => {
const filter: TraceqlFilter = { id: 'abc', tag: 'foo', operator: '=', value: 'bar' };
const result = filterToQuerySection(filter, [], lp);
expect(result).toBe('.foo=bar');
});
it('filter with regex operator', () => {
const filter: TraceqlFilter = { id: 'abc', tag: 'foo', operator: '=~', value: 'bar.*', valueType: 'string' };
const result = filterToQuerySection(filter, [], lp);
expect(result).toBe('.foo=~"bar.*"');
});
it('filter with scope', () => {
const filter: TraceqlFilter = {
id: 'abc',
tag: 'foo',
operator: '=',
value: 'bar',
scope: TraceqlSearchScope.Resource,
};
const result = filterToQuerySection(filter, [], lp);
expect(result).toBe('resource.foo=bar');
});
it('filter with intrinsic tag', () => {
const filter: TraceqlFilter = { id: 'abc', tag: 'duration', operator: '=', value: '100ms' };
const result = filterToQuerySection(filter, [], lp);
expect(result).toBe('duration=100ms');
});
it('filter with multiple non-string values and scope', () => {
const filter: TraceqlFilter = {
id: 'abc',
tag: 'foo',
operator: '=',
value: ['bar', 'baz'],
scope: TraceqlSearchScope.Span,
};
const result = filterToQuerySection(filter, [], lp);
expect(result).toBe('(span.foo=bar || span.foo=baz)');
});
it('filter with multiple string values and scope', () => {
const filter: TraceqlFilter = {
id: 'abc',
tag: 'foo',
operator: '=',
value: ['bar', 'baz'],
scope: TraceqlSearchScope.Span,
valueType: 'string',
};
const result = filterToQuerySection(filter, [], lp);
expect(result).toBe('(span.foo="bar" || span.foo="baz")');
});
it('filter with multiple string values with regex', () => {
const filter: TraceqlFilter = {
id: 'abc',
tag: 'foo',
operator: '=~',
value: ['bar', 'baz'],
scope: TraceqlSearchScope.Span,
valueType: 'string',
};
const result = filterToQuerySection(filter, [], lp);
expect(result).toBe('span.foo=~"bar|baz"');
});
});
export const emptyTags = [];
export const testIntrinsics = ['duration', 'kind', 'name', 'status'];
export const v1Tags = ['bar', 'foo'];

View File

@ -72,6 +72,14 @@ export const tagHelper = (f: TraceqlFilter, filters: TraceqlFilter[]) => {
return f.tag;
};
export const filterToQuerySection = (f: TraceqlFilter, filters: TraceqlFilter[], lp: TempoLanguageProvider) => {
if (Array.isArray(f.value) && f.value.length > 1 && !isRegExpOperator(f.operator!)) {
return `(${f.value.map((v) => `${scopeHelper(f, lp)}${tagHelper(f, filters)}${f.operator}${valueHelper({ ...f, value: v })}`).join(' || ')})`;
}
return `${scopeHelper(f, lp)}${tagHelper(f, filters)}${f.operator}${valueHelper(f)}`;
};
export const generateQueryFromAdHocFilters = (filters: AdHocVariableFilter[], lp: TempoLanguageProvider) => {
return `{${filters
.filter((f) => f.key && f.operator && f.value)

View File

@ -91,7 +91,7 @@ export function TraceQLSearchTags({ options, onOptionsChange, datasource }: Prop
staticTags={staticTags}
isTagsLoading={loading}
hideValues={true}
query={'{}'}
generateQueryWithoutFilter={() => '{}'}
/>
) : (
<div>Invalid data source, please create a valid data source and try again</div>

View File

@ -3,13 +3,11 @@ import { getTemplateSrv } from '@grafana/runtime';
import { VariableFormatID } from '@grafana/schema';
import {
filterToQuerySection,
getAllTags,
getIntrinsicTags,
getTagsByScope,
getUnscopedTags,
scopeHelper,
tagHelper,
valueHelper,
} from './SearchTraceQLEditor/utils';
import { TraceqlFilter, TraceqlSearchScope } from './dataquery.gen';
import { TempoDatasource } from './datasource';
@ -188,7 +186,7 @@ export default class TempoLanguageProvider extends LanguageProvider {
return `{${filters
.filter((f) => f.tag && f.operator && f.value?.length)
.map((f) => `${scopeHelper(f, this)}${tagHelper(f, filters)}${f.operator}${valueHelper(f)}`)
.map((f) => filterToQuerySection(f, filters, this))
.join(' && ')}}`;
}
}