feat: filter spans on search

[betegon]

Related to #731

We should have an actual filtering functionality, not just highlight

These changes add filtering to span search

• uplifted search functionality from SpanItem to SpanTree.
• Implemented DFS to handle nested search.
• Also removed highlighting from matching spans.

The results display not only the direct matches but also their hierarchical context up to the root:

Since spans are time-sorted, it would be useful to add a feature like +<#items> between results. This would allow you to jump directly to the result you’re looking for when debugging. I’ve been using Spotlight for the past few days, and this addition would have been very helpful.

Before opening this PR:

• I added a Changeset Entry with pnpm changeset:add
• I referenced issues that this PR addresses

[vercel]

@betegon is attempting to deploy a commit to the Sentry Team on Vercel.

A member of the Team first needs to authorize it.

[Shubhdeep12]

#731 (comment)

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
spotlightjs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 26, 2025 0:28am

[BYK]

I think this is a pretty good start but I'm concerned about the efficiency.

I also would like to keep the "find & highlight" approach as @Shubhdeep12 mentioned too. Maybe we can add a toggle as he mentioned?

[BYK]

Suggested change

	const matchesQuery = (span: Span): boolean | undefined => {
	return span.span_id.includes(query) || span.op?.includes(query) || span.description?.includes(query);
	};
	const matchesQuery = (span: Span): boolean | undefined => (span.span_id.includes(query) || span.op?.includes(query) || span.description?.includes(query));

[BYK]

I also wonder if we can make useSearch() return this matcher, wrapped in useCallback() or something to make things more efficient?

[betegon]

Done! I also made sure that it returns a boolean:

const matchesQuery = useCallback(
    (span: Span): boolean => {
      const q = context.query.toLowerCase();
      return (
        (span.span_id.toLowerCase().includes(q) ||
          span.op?.toLowerCase().includes(q) ||
          span.description?.toLowerCase().includes(q)) ??
        false
      );
    },
    [context.query],
  );

P.S. I added case-insensitive matching. I believe it’s one of the first things anyone would expect from a search feature.

[BYK]

I feel like we may want to cache this or do a more efficient depth-first search as we'd be scanning down the same spans as we do deeper.

[betegon]

Good call! I’ve memoized each of the spans we process now.

if (showOnlyMatched) {
  const spanMemo = new Map<string, boolean>();
  const hasMatchingDescendant = (span: Span): boolean => {
    if (spanMemo.has(span.span_id)) return spanMemo.get(span.span_id)!;
    const result = matchesQuery(span) || (span.children?.some(child => hasMatchingDescendant(child)) ?? false);
    spanMemo.set(span.span_id, result);
    return result;
  };
  return tree.filter(span => hasMatchingDescendant(span));
}

[codecov]

Codecov Report

Attention: Patch coverage is 13.41463% with 71 lines in your changes missing coverage. Please review.

Project coverage is 32.81%. Comparing base (a316fa1) to head (07db36f).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...s/traces/TraceDetails/components/TraceTreeview.tsx	12.90%	27 Missing ⚠️
.../src/integrations/sentry/context/SearchContext.tsx	16.00%	21 Missing ⚠️
...ations/sentry/components/traces/spans/SpanTree.tsx	9.09%	20 Missing ⚠️
...ations/sentry/components/traces/spans/SpanItem.tsx	25.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #747      +/-   ##
==========================================
- Coverage   33.03%   32.81%   -0.22%     
==========================================
  Files          93       93              
  Lines        5788     5838      +50     
  Branches      118      118              
==========================================
+ Hits         1912     1916       +4     
- Misses       3876     3922      +46     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[betegon]

Thanks for the detailed feedback, @BYK I’ve implemented your suggestions, including:

• Moved matchesQuery inside a useCallback in useSearch.
• Added case-insensitive matching to the search.
• Memoized spans to avoid processing each one more than once for each query.
• Added a toggle to switch between highlighting spans and showing only matches . I didn’t find a similar pattern in the Overlay, so I added the toggle inline to avoid taking up extra vertical space. cc: @Shubhdeep12 UX improvements in Search #731 (comment)

I think we’re in a better spot now, happy to keep iterating if you have more suggestions 😉

Here’s a quick screenshot video showing how the toggle works.

Screen.Recording.2025-04-06.at.19.47.50.mp4

[BYK]

I think it would be better to switch this into a checkbox and make the text shorter (or even use an icon with a very short label)

[BYK]

I think renaming this to shouldHighlight makes more sense with the new approach.

Suggested change

	const isQueried = !showOnlyMatched && query && matchesQuery(span);
	const shouldHighlight = !showOnlyMatched && query && matchesQuery(span);

[BYK]

Early return here with showOnlyMatched would make it easier to follow.

Suggested change

	if (!query) return tree;
	if (!query || !showOnlyMatched) return tree;

[BYK]

This implementation is not bad but I don't think the cache would be shared across children/siblings. I think this cache should be moved to useSearch() level and get reset when query changes.

[BYK]

Shouldn't we replace the tree here with filteredTree?

Suggested change

	if (!tree || !tree.length) return null;
	if (!filteredTree|| !tree.filteredTree) return null;

[BYK]

I think it would be more efficient if we create a:

const queryMatcher = new RegExp(RegExp.escape(query), 'i');

and then do queryMatcher.test(span.op) etc. afterwards. You may need to polyfill RegExp.escape as it doesn't seem to have wide browser support without flags.

Finally, a bit of a nitpick, we may wanna limit matching to only when the query is more than 2 characters as I don't want to match on a or even ab probably.

[BYK]

Let's not change any other files outside of the context of this patch. I'll revert this for you.

[BYK]

I think we may wanna expand all trees when there's a query set and "only show matches" is selected as otherwise it gets a bit confusing when trying to figure out why a top-level span was not filtered (because it has some child matching the query but is collapsed so the child is not visible).

I also have all these notes but I think the PR is a net improvement so gonna approve, merge, and cut a new release and push the improvements to future releases.