feat: filter spans on search #747

betegon · 2025-03-24T21:37:42Z

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

- Removed the search context and related logic from SpanItem -Uplifted a search functionality in SpanTree to filter spans based on the query - Updated rendering logic to display only matching spans in the tree

vercel · 2025-03-24T21:37:48Z

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

A member of the Team first needs to authorize it.

Shubhdeep12 · 2025-03-25T11:24:16Z

#731 (comment)

vercel · 2025-03-26T12:27:56Z

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 · 2025-03-26T12:31:00Z

packages/overlay/src/integrations/sentry/components/explore/traces/spans/SpanTree.tsx

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


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));

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

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 · 2025-03-26T12:33:25Z

packages/overlay/src/integrations/sentry/components/explore/traces/spans/SpanTree.tsx

+  const hasMatchingDescendant = (span: Span): boolean => {
+    if (matchesQuery(span)) return true;
+    if (!span.children) return false;
+    return span.children.some(hasMatchingDescendant);
+  };


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.

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)); }

…ing search queries

codecov · 2025-04-06T16:42:32Z

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 · 2025-04-06T18:06:09Z

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 · 2025-04-16T12:08:33Z

.../overlay/src/integrations/sentry/components/traces/TraceDetails/components/TraceTreeview.tsx

+          }`}
+          onClick={() => setShowOnlyMatched(!showOnlyMatched)}
+        >
+          Only Show Matches


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 · 2025-04-16T12:09:49Z

packages/overlay/src/integrations/sentry/components/traces/spans/SpanItem.tsx

-    return span.span_id.includes(query) || span.op?.includes(query) || span.description?.includes(query);
-  }, [query, span.span_id, span.op, span.description]);
+
+  const isQueried = !showOnlyMatched && query && matchesQuery(span);


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 · 2025-04-16T12:11:31Z

packages/overlay/src/integrations/sentry/components/traces/spans/SpanTree.tsx

+  const { query, matchesQuery, showOnlyMatched } = useSearch();
+
+  const filteredTree = useMemo(() => {
+    if (!query) return tree;


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

Suggested change

if (!query) return tree;

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

BYK · 2025-04-16T12:14:24Z

packages/overlay/src/integrations/sentry/components/traces/spans/SpanTree.tsx

+  const filteredTree = useMemo(() => {
+    if (!query) return tree;
+    if (showOnlyMatched) {
+      const spanMemo = new Map<string, boolean>();


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 · 2025-04-16T12:17:31Z

packages/overlay/src/integrations/sentry/components/traces/spans/SpanTree.tsx

+    }
+    return tree;
+  }, [query, tree, showOnlyMatched, matchesQuery]);
+
  if (!tree || !tree.length) return null;


Shouldn't we replace the tree here with filteredTree?

Suggested change

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

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

BYK · 2025-04-16T12:25:51Z

packages/overlay/src/integrations/sentry/context/SearchContext.tsx

+      const q = context.query.toLowerCase();
+      return (
+        (span.span_id.toLowerCase().includes(q) ||
+          span.op?.toLowerCase().includes(q) ||
+          span.description?.toLowerCase().includes(q)) ??
+        false


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 · 2025-04-16T12:26:59Z

packages/sidecar/src/contextlines.ts

@@ -1,8 +1,8 @@
+import { LEAST_UPPER_BOUND, TraceMap, originalPositionFor, sourceContentFor } from '@jridgewell/trace-mapping';


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.

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and publish to npm yourself or [setup this action to publish automatically](https://github.com/changesets/action#with-publishing). If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @spotlightjs/[email protected] ### Minor Changes - feat: Keep profile spans collapsed ([#737](#737)) ### Patch Changes - fix: Null-check sentryClient.\_options ([#736](#736)) - Updated dependencies \[[`3d569f30fb746d90ecabaac35d23d980360ea99c`](3d569f3), [`d3a2f0a0fae9074802b0551f3e1662833c1423c9`](d3a2f0a), [`d0315bc2ead9ccea722cc73c6b5fd7d9fed3f4a4`](d0315bc)]: - @spotlightjs/[email protected] ## @spotlightjs/[email protected] ### Minor Changes - feat: Keep profile spans collapsed ([#737](#737)) ### Patch Changes - fix: Null-check sentryClient.\_options ([#736](#736)) - Updated dependencies \[[`3d569f30fb746d90ecabaac35d23d980360ea99c`](3d569f3), [`7ea7de17b7ccf9f0c8edb8b413176d2f3482780c`](7ea7de1), [`263ba283efa247e05a022d7f1305cbf819e3e783`](263ba28), [`7ac9fd255cfe85d66e79d632e1d309c3e88d8092`](7ac9fd2), [`a76b2dadb28ce8699c80fc81b709050655bd4629`](a76b2da), [`d3a2f0a0fae9074802b0551f3e1662833c1423c9`](d3a2f0a), [`f2a48b05a41f80e08d1666247f7ccae60bc3d9e7`](f2a48b0), [`6d26f0d21b3ae75e3d81e48ceb2d8f727a94420f`](6d26f0d), [`a8f632357d9dcc46187b00724c14dd4423dfa467`](a8f6323), [`9888dbfc6778de910a2aeae9f3e86f0b2f716a18`](9888dbf), [`ced3e736dfef15d368cac202a10eec4eba7508be`](ced3e73)]: - @spotlightjs/[email protected] - @spotlightjs/[email protected] ## @spotlightjs/[email protected] ### Minor Changes - feat: Keep profile spans collapsed ([#737](#737)) - Swap "Errors" and "Explore" tab places, making "Explore" the default. It makes more sense this way as there is almost ([#758](#758)) guaranteed to have traces in a good setup but not really errors. - Restructured Sentry integration tabs and components ([#765](#765)) - - \#731: Search Bar sticky and fixed overflow across the overlay. ([#740](#740)) - Include filter functionality when searching spans ([#747](#747)) ### Patch Changes - Fixed parsing of envelope data. ([#751](#751)) - fix: Null-check sentryClient.\_options ([#736](#736)) - format node internals correctly in stacktrace ([#739](#739)) - Fixed #738 ([#741](#741)) ## @spotlightjs/[email protected] ### Minor Changes - feat: Keep profile spans collapsed ([#737](#737)) ### Patch Changes - fix: Null-check sentryClient.\_options ([#736](#736)) - Remove obsolete packages from dependencies ([#761](#761)) - Updated dependencies \[[`3d569f30fb746d90ecabaac35d23d980360ea99c`](3d569f3), [`7ea7de17b7ccf9f0c8edb8b413176d2f3482780c`](7ea7de1), [`263ba283efa247e05a022d7f1305cbf819e3e783`](263ba28), [`7ac9fd255cfe85d66e79d632e1d309c3e88d8092`](7ac9fd2), [`a76b2dadb28ce8699c80fc81b709050655bd4629`](a76b2da), [`d3a2f0a0fae9074802b0551f3e1662833c1423c9`](d3a2f0a), [`f2a48b05a41f80e08d1666247f7ccae60bc3d9e7`](f2a48b0), [`6d26f0d21b3ae75e3d81e48ceb2d8f727a94420f`](6d26f0d), [`a8f632357d9dcc46187b00724c14dd4423dfa467`](a8f6323), [`9888dbfc6778de910a2aeae9f3e86f0b2f716a18`](9888dbf), [`ced3e736dfef15d368cac202a10eec4eba7508be`](ced3e73)]: - @spotlightjs/[email protected] - @spotlightjs/[email protected] ## @spotlightjs/[email protected] ### Patch Changes - use nanosecond timestamp for captured filenames ([#776](#776)) - Improve DX by always showing the Spotlight URL, even if the sidecar was already running. Makes it easy to cmd/ctrl+click ([#748](#748)) and open in browser. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

betegon added 2 commits March 24, 2025 22:20

Filter on search.

2e3e07c

- Removed the search context and related logic from SpanItem -Uplifted a search functionality in SpanTree to filter spans based on the query - Updated rendering logic to display only matching spans in the tree

add changeset

1f257cd

vercel bot deployed to Preview March 26, 2025 12:28 View deployment

BYK requested changes Mar 26, 2025

View reviewed changes

betegon added 3 commits April 6, 2025 14:39

Optimize matchesQuery to only render on query change

c86abcd

refactor matchesQuery to ensure a boolean return type

97640dd

Optimize SpanTree filtering logic by memoizing descendant matches dur…

d8e9035

…ing search queries

betegon added 2 commits April 6, 2025 19:10

case insensitve search

9177d14

add toggle to switch between highlighting and filtering search results

c051d7c

betegon requested a review from BYK April 6, 2025 18:06

betegon and others added 6 commits April 8, 2025 18:41

Adaptation to use TraceTreeWithSearch

78e656d

fix search bar height

4805821

Merge branch 'main' into feat/filter-spans-on-search

0f24c5a

Merge branch 'main' into feat/filter-spans-on-search

e5b6e4e

bump to minor change

fb1fa87

fix import

f9b73c8

BYK reviewed Apr 16, 2025

View reviewed changes

revert unrelated changes

07db36f

BYK approved these changes Apr 16, 2025

View reviewed changes

BYK merged commit 9888dbf into getsentry:main Apr 16, 2025
14 of 17 checks passed

sentry-release-bot bot mentioned this pull request Apr 16, 2025

meta(changelog): Update package versions #774

Closed

betegon deleted the feat/filter-spans-on-search branch April 17, 2025 17:02

sentry-release-bot bot mentioned this pull request Apr 17, 2025

meta(changelog): Update package versions #777

Merged

BYK mentioned this pull request Apr 17, 2025

UX improvements in Search #731

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: filter spans on search #747

feat: filter spans on search #747

betegon commented Mar 24, 2025

vercel bot commented Mar 24, 2025

Shubhdeep12 commented Mar 25, 2025

vercel bot commented Mar 26, 2025 •

edited

Loading

BYK left a comment

BYK Mar 26, 2025

BYK Mar 26, 2025

betegon Apr 6, 2025

BYK Mar 26, 2025

betegon Apr 6, 2025

codecov bot commented Apr 6, 2025 •

edited

Loading

betegon commented Apr 6, 2025

BYK Apr 16, 2025

BYK Apr 16, 2025

BYK Apr 16, 2025

BYK Apr 16, 2025

BYK Apr 16, 2025

BYK Apr 16, 2025

BYK Apr 16, 2025

BYK left a comment

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

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

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

		@@ -1,8 +1,8 @@
		import { LEAST_UPPER_BOUND, TraceMap, originalPositionFor, sourceContentFor } from '@jridgewell/trace-mapping';

feat: filter spans on search #747

feat: filter spans on search #747

Conversation

betegon commented Mar 24, 2025

vercel bot commented Mar 24, 2025

Shubhdeep12 commented Mar 25, 2025

vercel bot commented Mar 26, 2025 • edited Loading

BYK left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 6, 2025 • edited Loading

Codecov Report

betegon commented Apr 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BYK left a comment

Choose a reason for hiding this comment

vercel bot commented Mar 26, 2025 •

edited

Loading

codecov bot commented Apr 6, 2025 •

edited

Loading