Skip to content

feat(explore): Validate trace item search keys asynchronously#111189

Merged
nsdeschenes merged 33 commits into
masterfrom
nd/EXP-641/feat-search-async-key-validation
Mar 30, 2026
Merged

feat(explore): Validate trace item search keys asynchronously#111189
nsdeschenes merged 33 commits into
masterfrom
nd/EXP-641/feat-search-async-key-validation

Conversation

@nsdeschenes

@nsdeschenes nsdeschenes commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

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

@linear-code

linear-code Bot commented Mar 20, 2026

Copy link
Copy Markdown

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 20, 2026
@nsdeschenes

Copy link
Copy Markdown
Contributor Author

@sentry review

@nsdeschenes

Copy link
Copy Markdown
Contributor Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Comment thread static/app/views/explore/hooks/useAsyncAttributeValidation.tsx Outdated
Comment thread static/app/components/searchQueryBuilder/hooks/useAsyncFilterKeyValidation.tsx Outdated
Comment thread static/app/components/searchQueryBuilder/hooks/useAsyncFilterKeyValidation.tsx Outdated
Comment thread static/app/components/searchQueryBuilder/hooks/useAsyncFilterKeyValidation.tsx Outdated
Comment thread static/app/components/searchQueryBuilder/tokens/invalidTokenTooltip.tsx Outdated
Comment thread static/app/components/searchQueryBuilder/index.tsx Outdated
@billyvg billyvg removed the request for review from a team March 24, 2026 22:03
@nsdeschenes nsdeschenes force-pushed the nd/EXP-641/feat-search-async-key-validation branch from ef85047 to 76bade1 Compare March 26, 2026 16:12
Comment thread static/app/views/explore/components/traceItemSearchQueryBuilder.tsx Outdated
Comment thread static/app/components/searchQueryBuilder/tokens/filter/filter.tsx

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Ref guard prevents re-validation on page filter changes
    • Removed the initialQueryValidatedRef guard that prevented the useEffect from re-running when page filters changed, allowing validation to occur on datetime and project changes.
  • ✅ Fixed: Concurrent validations can resolve out of order
    • Added a request counter that increments on each validation call and only applies results if the request counter matches, preventing stale results from overwriting newer ones.

Create PR

Or push these changes by commenting:

@cursor push e2a2ed7b7d
Preview (e2a2ed7b7d)
diff --git a/static/app/views/explore/components/traceItemSearchQueryBuilder.tsx b/static/app/views/explore/components/traceItemSearchQueryBuilder.tsx
--- a/static/app/views/explore/components/traceItemSearchQueryBuilder.tsx
+++ b/static/app/views/explore/components/traceItemSearchQueryBuilder.tsx
@@ -1,4 +1,4 @@
-import {useCallback, useEffect, useMemo, useRef} from 'react';
+import {useCallback, useEffect, useMemo} from 'react';
 
 import type {SpanSearchQueryBuilderProps} from 'sentry/components/performance/spanSearchQueryBuilder';
 import {
@@ -110,11 +110,9 @@
 
   const {invalidFilterKeys, validateQuery} = useAttributeValidation(itemType, projects);
 
-  const initialQueryValidatedRef = useRef(false);
   useEffect(() => {
-    if (initialQuery && !initialQueryValidatedRef.current) {
+    if (initialQuery) {
       validateQuery(initialQuery);
-      initialQueryValidatedRef.current = true;
     }
   }, [initialQuery, validateQuery]);
 

diff --git a/static/app/views/explore/hooks/useAttributeValidation.tsx b/static/app/views/explore/hooks/useAttributeValidation.tsx
--- a/static/app/views/explore/hooks/useAttributeValidation.tsx
+++ b/static/app/views/explore/hooks/useAttributeValidation.tsx
@@ -1,4 +1,4 @@
-import {useCallback, useState} from 'react';
+import {useCallback, useRef, useState} from 'react';
 
 import {normalizeDateTimeParams} from 'sentry/components/pageFilters/parse';
 import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters';
@@ -85,6 +85,7 @@
   const organization = useOrganization();
   const {selection} = usePageFilters();
   const effectiveProjects = projects ?? selection.projects;
+  const requestCounterRef = useRef(0);
 
   const validateQuery = useCallback(
     async (query: string) => {
@@ -93,6 +94,7 @@
         setInvalidFilterKeys([]);
         return;
       }
+      const currentRequest = ++requestCounterRef.current;
       try {
         const [data] = await queryClient.fetchQuery(
           validateAttributesQueryOptions({
@@ -103,11 +105,13 @@
             projects: effectiveProjects,
           })
         );
-        setInvalidFilterKeys(
-          Object.entries(data.attributes)
-            .filter(([, result]) => !result.valid)
-            .map(([key]) => key)
-        );
+        if (currentRequest === requestCounterRef.current) {
+          setInvalidFilterKeys(
+            Object.entries(data.attributes)
+              .filter(([, result]) => !result.valid)
+              .map(([key]) => key)
+          );
+        }
       } catch {
         // leave previous state on error
       }

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Comment thread static/app/views/explore/components/traceItemSearchQueryBuilder.tsx Outdated
Comment thread static/app/views/explore/hooks/useAttributeValidation.tsx
Comment thread static/app/views/explore/hooks/useAttributeValidation.tsx Outdated
Comment thread static/app/views/explore/components/traceItemSearchQueryBuilder.tsx Outdated
nsdeschenes and others added 24 commits March 30, 2026 10:59
Move the early return before the keySet creation to also handle the
null case upfront.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Check keySet.size after token extraction instead of parsedQuery.length,
since a non-empty query can still contain zero filter tokens.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Parse and validate filter keys from initialQuery on mount so that
keys loaded from URL params are validated without requiring user
interaction. Uses a ref guard to ensure validation only fires once.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract the filter key extraction logic into a standalone exported
utility function. This prepares for migrating the validation hook
from useMutation to useApiQuery by making key extraction reusable.
…iQuery

Replace the imperative useMutation pattern with a declarative useApiQuery
approach. The hook now accepts filter keys as input and returns invalid
keys, letting React Query handle deduplication, caching, and stale
response management via keepPreviousData.

The consumer tracks the current query in state and derives filter keys
with extractFilterKeys, removing the need for useRef/useEffect-based
initial validation.
Cover extractFilterKeys (null input, empty input, sorting, deduplication,
stable references) and the useAsyncAttributeValidation hook (empty keys,
invalid key detection, request parameters, custom projects override,
query skipping when no keys present).

Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
…ation

Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
…onent

Remove the useAttributeValidation hook and move the pure validation logic
into a new utils/attributeValidation.ts module. Inline the async query call
directly into traceItemSearchQueryBuilder using queryClient.fetchQuery, which
avoids an intermediate hook layer and makes the data flow more explicit.

Refs EXP-641
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…irectory

Move attribute validation logic out of the component into a dedicated
useAttributeValidation hook in the hooks directory. The hook encapsulates
invalidFilterKeys state and the validateQuery callback, keeping the
component lean. Also clean up test casts by using parseSearch and
parseQueryKey instead of unsafe type assertions.

Refs EXP-641
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ests

Remove the export from validateAttributesQueryOptions since it is an
implementation detail of the hook. Replace the validateAttributesQueryOptions
test suite with hook-level tests that cover the same behaviour through the
public API, including asserting the API is not called when the query has no
filter keys.

Refs EXP-641
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use a ref to guard the initial validation effect so it only fires once,
and remove the eslint-disable comment by including the proper dependencies.

Refs EXP-641
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When validateQuery is called rapidly, concurrent fetchQuery calls with
different cache keys can resolve out of order, causing stale results to
overwrite newer ones. Cancel any in-flight validation queries before
starting a new fetch to prevent race conditions.

Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
The initialQueryValidatedRef guard permanently prevented the validation
effect from re-running after mount. When page filters (datetime or
projects) change, validateQuery gets a new reference but the ref was
already true, so validation was skipped. Remove the ref guard so the
effect re-runs whenever validateQuery identity changes.

Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
Move page filter selection out of useAttributeValidation and accept it
as a parameter. This gives the hook a stable identity and lets the call
site control when re-validation happens. Use staleTime: Infinity on the
query options so TanStack Query deduplicates calls with the same filter
keys and selection without extra refs.

Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
The cancelQueries call used only the API URL as a partial query key,
which cancelled all in-flight validation requests across every search
bar on the page. This caused a race condition where one component's
validation would silently cancel another's, leaving stale invalid key
warnings.

Narrow the cancellation key to include itemType so each hook instance
only cancels its own previous requests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move useAttributeValidation from a callback-based approach (manual
fetchQuery + useState) to a declarative useQuery hook. The hook now
accepts query and selection as parameters, letting React Query handle
caching, deduplication, and cancellation automatically. This simplifies
the consumer in traceItemSearchQueryBuilder by removing the useEffect
and wrapped onChange callback.

Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
extractFilterKeys only iterated top-level tokens, so filter keys nested
inside parenthesized groups (Token.LOGIC_GROUP) were never sent to the
validation API. Add a recursive walk to inspect inner tokens.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…efactor

The tests were still using the old imperative pattern of calling
onChange to trigger validation. After the refactor to declarative
useQuery, validation is driven by the initialQuery prop, so update
tests to use initialQuery and rerender instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Only validate search query builder filter keys when the
organization has the 'search-query-attribute-validation' feature
flag enabled. When disabled, skip parsing and API calls entirely.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ture

Combine hasValidation with filterKeys.length > 0 in the useQuery enabled
option so the validation API is not called for empty queries. Add the
search-query-attribute-validation feature flag to the query builder
test fixture so validation tests actually exercise the code path.

Co-Authored-By: Claude Opus 4.6 <noreply@example.com>
Add nullish coalescing fallback when accessing the attributes field
from the validation API response. The prior optional chaining on
data[0]?.attributes could pass undefined to Object.entries() if
the response body lacked an attributes field, causing a TypeError.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@nsdeschenes nsdeschenes merged commit 5294cdb into master Mar 30, 2026
70 checks passed
@nsdeschenes nsdeschenes deleted the nd/EXP-641/feat-search-async-key-validation branch March 30, 2026 17:18
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants