Skip to content

Commit 5294cdb

Browse files
nsdeschenesclaude
andauthored
feat(explore): Validate trace item search keys asynchronously (#111189)
The goal of this PR is to validate the filter keys on any trace item based search query builder. To do this we've introduced a new API endpoint that accepts in a list of attributes and checks to see if they exist and are valid. We trigger this with a useQuery that runs on mount, and whenever the user modifies the query. We also have guards in place to ignore running the validation if the user is just modifying the filter values as those don't need to be validated, reducing the amount of calls we make. Validation is currently gated behind this flag: #111752 --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 4ab83b7 commit 5294cdb

File tree

16 files changed

+523
-10
lines changed

16 files changed

+523
-10
lines changed

static/app/components/searchQueryBuilder/context.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ interface SearchQueryBuilderContextData {
6161
getSuggestedFilterKey: (key: string) => string | null;
6262
getTagValues: GetTagValues;
6363
handleSearch: (query: string) => void;
64+
invalidFilterKeys: string[];
6465
parseQuery: (query: string) => ParseResult | null;
6566
parsedQuery: ParseResult | null;
6667
query: string;
@@ -128,6 +129,7 @@ export function SearchQueryBuilderProvider({
128129
filterKeyAliases,
129130
caseInsensitive,
130131
onCaseInsensitiveClick,
132+
invalidFilterKeys,
131133
}: SearchQueryBuilderProps & {children: React.ReactNode}) {
132134
const wrapperRef = useRef<HTMLDivElement>(null);
133135
const actionBarRef = useRef<HTMLDivElement>(null);
@@ -198,6 +200,11 @@ export function SearchQueryBuilderProvider({
198200

199201
const parsedQuery = useMemo(() => parseQuery(state.query), [parseQuery, state.query]);
200202

203+
const stableInvalidFilterKeys = useMemo(
204+
() => invalidFilterKeys ?? [],
205+
[invalidFilterKeys]
206+
);
207+
201208
const previousQuery = usePrevious(state.query);
202209
const firstRender = useRef(true);
203210
useEffect(() => {
@@ -242,6 +249,7 @@ export function SearchQueryBuilderProvider({
242249
return {
243250
...state,
244251
aiSearchBadgeType,
252+
invalidFilterKeys: stableInvalidFilterKeys,
245253
disabled,
246254
disallowFreeText: Boolean(disallowFreeText),
247255
disallowLogicalOperators: Boolean(disallowLogicalOperators),
@@ -300,6 +308,7 @@ export function SearchQueryBuilderProvider({
300308
getTagKeys,
301309
getTagValues,
302310
handleSearch,
311+
stableInvalidFilterKeys,
303312
matchKeySuggestions,
304313
onCaseInsensitiveClick,
305314
parseQuery,

static/app/components/searchQueryBuilder/index.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,12 @@ export interface SearchQueryBuilderProps {
126126
* and display the returned keys alongside any static filterKeys.
127127
*/
128128
getTagKeys?: GetTagKeys;
129+
/**
130+
* List of filter key strings that are invalid.
131+
* When provided, tokens with matching keys will display a warning state.
132+
* The parent component is responsible for fetching and determining invalid keys.
133+
*/
134+
invalidFilterKeys?: string[];
129135

130136
/**
131137
* Allows for customization of the invalid token messages.
@@ -174,11 +180,11 @@ export interface SearchQueryBuilderProps {
174180
*/
175181
portalTarget?: HTMLElement | null;
176182
queryInterface?: QueryInterfaceType;
183+
177184
/**
178185
* If provided, saves and displays recent searches of the given type.
179186
*/
180187
recentSearches?: SavedSearchType;
181-
182188
/**
183189
* When set, provided keys will override default raw search capabilities, while
184190
* replacing it with options that include the provided keys, and the user's input

static/app/components/searchQueryBuilder/tokens/filter/filter.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ export function SearchQueryBuilderFilter({item, state, token}: SearchQueryTokenP
203203

204204
const isFocused = item.key === state.selectionManager.focusedKey;
205205

206-
const {dispatch} = useSearchQueryBuilder();
206+
const {dispatch, invalidFilterKeys} = useSearchQueryBuilder();
207207
const {rowProps, gridCellProps} = useQueryBuilderGridItem(item, state, ref);
208208

209209
const onKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => {
@@ -227,12 +227,19 @@ export function SearchQueryBuilderFilter({item, state, token}: SearchQueryTokenP
227227

228228
const tokenHasError = 'invalid' in token && defined(token.invalid);
229229
const tokenHasWarning = 'warning' in token && defined(token.warning);
230+
const isInvalidFilterKey = invalidFilterKeys.includes(getKeyName(token.key));
230231

231232
return (
232233
<FilterWrapper
233234
aria-label={token.text}
234235
aria-invalid={tokenHasError}
235-
state={tokenHasWarning ? 'warning' : tokenHasError ? 'invalid' : 'valid'}
236+
state={
237+
tokenHasWarning || isInvalidFilterKey
238+
? 'warning'
239+
: tokenHasError
240+
? 'invalid'
241+
: 'valid'
242+
}
236243
ref={ref}
237244
{...modifiedRowProps}
238245
>
@@ -243,6 +250,11 @@ export function SearchQueryBuilderFilter({item, state, token}: SearchQueryTokenP
243250
columnCount={4}
244251
containerDisplayMode="grid"
245252
forceVisible={filterMenuOpen ? false : undefined}
253+
warning={
254+
isInvalidFilterKey
255+
? t('Invalid key. "%s" is not a supported search key.', getKeyName(token.key))
256+
: undefined
257+
}
246258
>
247259
{token.filter === FilterType.IS || token.filter === FilterType.HAS ? null : (
248260
<BaseGridCell {...gridCellProps}>

static/app/components/searchQueryBuilder/tokens/invalidTokenTooltip.tsx

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,23 @@ interface InvalidTokenTooltipProps extends Omit<TooltipProps, 'title'> {
1414
item: Node<ParseResultToken>;
1515
state: ListState<ParseResultToken>;
1616
token: ParseResultToken;
17+
warning?: ReactNode;
1718
}
1819

1920
function getForceVisible({
2021
isFocused,
2122
isInvalid,
23+
hasTokenWarning,
2224
hasWarning,
2325
forceVisible,
2426
}: {
27+
hasTokenWarning: boolean;
2528
hasWarning: boolean;
2629
isFocused: boolean;
2730
isInvalid: boolean;
2831
forceVisible?: boolean;
2932
}) {
30-
if (!isInvalid && !hasWarning) {
33+
if (!isInvalid && !hasTokenWarning && !hasWarning) {
3134
return false;
3235
}
3336

@@ -44,11 +47,13 @@ export function InvalidTokenTooltip({
4447
state,
4548
item,
4649
forceVisible,
50+
warning,
4751
...tooltipProps
4852
}: InvalidTokenTooltipProps) {
4953
const invalid = 'invalid' in token ? token.invalid : null;
50-
const warning = 'warning' in token ? token.warning : null;
54+
const tokenWarning = 'warning' in token ? token.warning : null;
5155

56+
const hasTokenWarning = Boolean(tokenWarning);
5257
const hasWarning = Boolean(warning);
5358
const isInvalid = Boolean(invalid);
5459
const isFocused =
@@ -57,9 +62,15 @@ export function InvalidTokenTooltip({
5762
return (
5863
<Tooltip
5964
skipWrapper
60-
forceVisible={getForceVisible({isFocused, isInvalid, hasWarning, forceVisible})}
65+
forceVisible={getForceVisible({
66+
isFocused,
67+
isInvalid,
68+
hasTokenWarning,
69+
hasWarning,
70+
forceVisible,
71+
})}
6172
position="bottom"
62-
title={warning ?? invalid?.reason ?? t('This token is invalid')}
73+
title={warning ?? tokenWarning ?? invalid?.reason ?? t('This token is invalid')}
6374
{...tooltipProps}
6475
>
6576
{children}
@@ -68,7 +79,6 @@ export function InvalidTokenTooltip({
6879
}
6980

7081
type GridInvalidTokenTooltipProps = InvalidTokenTooltipProps & {
71-
children: React.ReactNode;
7282
columnCount: number;
7383
};
7484

static/app/views/dashboards/widgetBuilder/buildSteps/filterResultsStep/spansSearchBar.spec.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,12 @@ describe('SpansSearchBar', () => {
8888
mockSpanTagValues({type: 'number', tagKey: 'span.op', mockedValues: []});
8989

9090
mockSpanTags({type: 'boolean', mockedTags: []});
91+
92+
MockApiClient.addMockResponse({
93+
url: `/organizations/org-slug/trace-items/attributes/validate/`,
94+
method: 'POST',
95+
body: {attributes: {}},
96+
});
9197
});
9298

9399
it('renders the initial query conditions', async () => {

static/app/views/detectors/components/forms/metric/metric.spec.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ describe('NewMetricDetectorForm', () => {
3939
url: '/organizations/org-slug/trace-items/attributes/',
4040
body: [],
4141
});
42+
MockApiClient.addMockResponse({
43+
url: '/organizations/org-slug/trace-items/attributes/validate/',
44+
method: 'POST',
45+
body: {attributes: {}},
46+
});
4247
MockApiClient.addMockResponse({
4348
url: '/organizations/org-slug/recent-searches/',
4449
body: [],

static/app/views/detectors/edit.spec.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ describe('DetectorEdit', () => {
9393
url: `/organizations/${organization.slug}/trace-items/attributes/`,
9494
body: [],
9595
});
96+
MockApiClient.addMockResponse({
97+
url: `/organizations/${organization.slug}/trace-items/attributes/validate/`,
98+
method: 'POST',
99+
body: {attributes: {}},
100+
});
96101

97102
MockApiClient.addMockResponse({
98103
url: `/organizations/${organization.slug}/workflows/`,

static/app/views/detectors/new-setting.spec.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ describe('DetectorEdit', () => {
6666
url: `/organizations/${organization.slug}/trace-items/attributes/`,
6767
body: [],
6868
});
69+
MockApiClient.addMockResponse({
70+
url: `/organizations/${organization.slug}/trace-items/attributes/validate/`,
71+
method: 'POST',
72+
body: {attributes: {}},
73+
});
6974

7075
MockApiClient.addMockResponse({
7176
url: `/organizations/${organization.slug}/workflows/`,

static/app/views/explore/components/traceItemSearchQueryBuilder.spec.tsx

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {OrganizationFixture} from 'sentry-fixture/organization';
22

3-
import {renderHookWithProviders} from 'sentry-test/reactTestingLibrary';
3+
import {renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLibrary';
44

55
import {PageFiltersStore} from 'sentry/components/pageFilters/store';
66
import {FieldKind} from 'sentry/utils/fields';
@@ -22,7 +22,7 @@ const defaultInitialProps: TraceItemSearchQueryBuilderProps = {
2222
searchSource: 'test',
2323
};
2424
const organization = OrganizationFixture({
25-
features: [],
25+
features: ['search-query-attribute-validation'],
2626
});
2727

2828
describe('useTraceItemSearchQueryBuilderProps', () => {
@@ -247,4 +247,88 @@ describe('useTraceItemSearchQueryBuilderProps', () => {
247247
'log.flag',
248248
]);
249249
});
250+
251+
it('calls validateQuery when filter keys change', async () => {
252+
const validateMock = MockApiClient.addMockResponse({
253+
url: '/organizations/org-slug/trace-items/attributes/validate/',
254+
method: 'POST',
255+
body: {attributes: {'span.op': {valid: true}}},
256+
});
257+
258+
renderHookWithProviders(useTraceItemSearchQueryBuilderProps, {
259+
initialProps: {
260+
...defaultInitialProps,
261+
initialQuery: 'span.op:db',
262+
},
263+
organization,
264+
});
265+
266+
await waitFor(() => {
267+
expect(validateMock).toHaveBeenCalledTimes(1);
268+
});
269+
});
270+
271+
it('does not call validateQuery when only filter values change', async () => {
272+
const validateMock = MockApiClient.addMockResponse({
273+
url: '/organizations/org-slug/trace-items/attributes/validate/',
274+
method: 'POST',
275+
body: {attributes: {'span.op': {valid: true}}},
276+
});
277+
278+
const {rerender} = renderHookWithProviders(useTraceItemSearchQueryBuilderProps, {
279+
initialProps: {
280+
...defaultInitialProps,
281+
initialQuery: 'span.op:db',
282+
},
283+
organization,
284+
});
285+
286+
await waitFor(() => {
287+
expect(validateMock).toHaveBeenCalledTimes(1);
288+
});
289+
290+
rerender({
291+
...defaultInitialProps,
292+
initialQuery: 'span.op:web',
293+
});
294+
295+
// Still only 1 call — value changed but keys didn't (React Query deduplicates)
296+
await waitFor(() => {
297+
expect(validateMock).toHaveBeenCalledTimes(1);
298+
});
299+
});
300+
301+
it('calls validateQuery when a new filter key is added', async () => {
302+
const validateMock = MockApiClient.addMockResponse({
303+
url: '/organizations/org-slug/trace-items/attributes/validate/',
304+
method: 'POST',
305+
body: {
306+
attributes: {
307+
'span.op': {valid: true},
308+
'other.key': {valid: true},
309+
},
310+
},
311+
});
312+
313+
const {rerender} = renderHookWithProviders(useTraceItemSearchQueryBuilderProps, {
314+
initialProps: {
315+
...defaultInitialProps,
316+
initialQuery: 'span.op:db',
317+
},
318+
organization,
319+
});
320+
321+
await waitFor(() => {
322+
expect(validateMock).toHaveBeenCalledTimes(1);
323+
});
324+
325+
rerender({
326+
...defaultInitialProps,
327+
initialQuery: 'span.op:db other.key:val',
328+
});
329+
330+
await waitFor(() => {
331+
expect(validateMock).toHaveBeenCalledTimes(2);
332+
});
333+
});
250334
});

static/app/views/explore/components/traceItemSearchQueryBuilder.tsx

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {useMemo} from 'react';
22

3+
import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters';
34
import type {SpanSearchQueryBuilderProps} from 'sentry/components/performance/spanSearchQueryBuilder';
45
import {
56
SearchQueryBuilder,
@@ -11,6 +12,7 @@ import {SavedSearchType, type TagCollection} from 'sentry/types/group';
1112
import type {AggregationKey} from 'sentry/utils/fields';
1213
import {FieldKind, getFieldDefinition} from 'sentry/utils/fields';
1314
import {getHasTag} from 'sentry/utils/tag';
15+
import {useAttributeValidation} from 'sentry/views/explore/hooks/useAttributeValidation';
1416
import {useExploreSuggestedAttribute} from 'sentry/views/explore/hooks/useExploreSuggestedAttribute';
1517
import {useGetTraceItemAttributeTagKeys} from 'sentry/views/explore/hooks/useGetTraceItemAttributeTagKeys';
1618
import {useGetTraceItemAttributeValues} from 'sentry/views/explore/hooks/useGetTraceItemAttributeValues';
@@ -105,6 +107,19 @@ export function useTraceItemSearchQueryBuilderProps({
105107
disableRecentSearches,
106108
}: TraceItemSearchQueryBuilderProps) {
107109
const placeholderText = itemTypeToDefaultPlaceholder(itemType);
110+
111+
const {selection} = usePageFilters();
112+
const effectiveProjects = projects ?? selection.projects;
113+
const validationSelection = useMemo(
114+
() => ({datetime: selection.datetime, projects: effectiveProjects}),
115+
[selection.datetime, effectiveProjects]
116+
);
117+
118+
const {invalidFilterKeys} = useAttributeValidation(
119+
itemType,
120+
initialQuery ?? '',
121+
validationSelection
122+
);
108123
const functionTags = useFunctionTags(itemType, supportedAggregates);
109124
const filterKeySections = useFilterKeySections(itemType, stringAttributes);
110125
const filterTags = useFilterTags({
@@ -166,6 +181,7 @@ export function useTraceItemSearchQueryBuilderProps({
166181
},
167182
caseInsensitive,
168183
onCaseInsensitiveClick,
184+
invalidFilterKeys,
169185
}),
170186
[
171187
booleanSecondaryAliases,
@@ -180,6 +196,7 @@ export function useTraceItemSearchQueryBuilderProps({
180196
getTagKeys,
181197
getTraceItemAttributeValues,
182198
initialQuery,
199+
invalidFilterKeys,
183200
itemType,
184201
matchKeySuggestions,
185202
namespace,

0 commit comments

Comments
 (0)