Added Search in Trace detail by Shubhdeep12 · Pull Request #424 · getsentry/spotlight

Shubhdeep12 · 2024-06-23T13:59:58Z

Before opening this PR:

I added a Changeset Entry with pnpm changeset:add
I referenced issues that this PR addresses
To add a search capability in trace view #421

Screencast.from.06-23-2024.07.28.07.PM.webm

vercel · 2024-06-23T14:00:02Z

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	Jun 28, 2024 11:15pm

BYK

Mainly rejecting for debounce issue. But please take a look at my other comments too.

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

BYK · 2024-06-26T21:00:21Z

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

+  const isQueried = query
+    ? span.op?.includes(query) || span.description?.includes(query) || span.span_id.includes(query)
+    : false;


I'd put the check for description last as that'd be the most expensive check I presume (due to length?)

Suggested change

const isQueried = query

? span.op?.includes(query) || span.description?.includes(query) || span.span_id.includes(query)

: false;

const isQueried = query

? span.span_id.includes(query) || span.op?.includes(query) || span.description?.includes(query)

: false;

Also not a fan of those op? and description? optionals but I'm guessing we have to live with it?

Yes, it can be possible of having spans here without op or description. so we need a check here.

Definitely i'll make the change suggested above

BYK · 2024-06-26T21:06:41Z

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

        </div>
        <div
-          className="waterfall"
+          className={classNames('waterfall', isQueried ? '!bg-transparent' : '')}


Not happy that we had do spread query and isQueried around so much. I understand that this is kind of the result of us using classNames instead of creating specific CSS classes/styled components. Is there a way out of this? :D

Yeah, that is because of tailwind CSS.
Although if needed we can pass the classnames and create the styles in .css files.

BYK · 2024-06-26T21:07:18Z

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

            totalDuration={totalDuration}
            spanNodeWidth={spanNodeWidth}
            setSpanNodeWidth={setSpanNodeWidth}
+            query={query}


I'm wondering if this should be passed in the context at this point. Have you considered that?

hmm, yes we can use context here as well.
should i create one or we can fix it as a whole in sentry integration for other states as well in a separate PR?

should i create one or we can fix it as a whole in sentry integration for other states as well in a separate PR?

Separate PR as I understand there are more things we can move to context from what you say.

BYK · 2024-06-26T21:17:35Z

packages/overlay/src/integrations/sentry/hooks/useDebounce.ts

@@ -0,0 +1,17 @@
+import { useEffect, useState } from 'react';
+
+export default function useDebounce<T>(value: T, delay: number): T {


This is definitely NOT proper debouncing. It just delays setting the value. A real debounce would not change anything until the last update + debounce period. It also looks like this code was lifted from https://github.com/uidotdev/usehooks/blob/90fbbb4cc085e74e50c36a62a5759a40c62bb98e/index.js#L239-L253. If that's the case, we should give a reference back there.

Finally, if you want to have proper debouncing this answer seems like the way: https://stackoverflow.com/a/75556586/90297. The one below that also shows "the hook way" with useMemo and proper cancellation as it apparently uses the debounce decorater from lodash.

Hi @BYK

https://github.com/uidotdev/usehooks/blob/90fbbb4cc085e74e50c36a62a5759a40c62bb98e/index.js#L239-L253. If that's the case, we should give a reference back there.

No, not lifted from here.

Finally, if you want to have proper debouncing this answer seems like the way: https://stackoverflow.com/a/75556586/90297. The one below that also shows "the hook way" with useMemo and proper cancellation as it apparently uses the debounce decorater from lodash.

Sure, let me enhance the hook based on the example above.
But since we are debouncing search. here we will have to maintain 2 states - one for instantly updating the search query in input(this cannot be debounced) and another for passing the search query in children components(this one will be debounced).

But since we are debouncing search. here we will have to maintain 2 states - one for instantly updating the search query in input(this cannot be debounced) and another for passing the search query in children components(this one will be debounced).

Not against debouncing, on the contrary I like it. It's just the debouncing logic is not right here.

Yes, even i found that an issue, because we were setting the state again and again in hook, which was not an efficient solution and that would cost us multiple re-renders.

BYK

It's okay but I think your initial pass where useDebounce was containing the logic was better. This version feels a bit less polished and usable.

To emphasize: my main issue was the debounce logic not being correct in the initial version. It seems to be fixed in this version.

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

packages/overlay/src/integrations/sentry/hooks/useDebounce.ts

…rybook-8-compatibility * origin/HEAD: fix: Overhaul envelope parsing to be spec compliant (getsentry#439) fix: Always trim befor trying to JSON.parse (getsentry#438) Highlighted active span and event (getsentry#437) ref: Use new event helpers (getsentry#433) fix: Remove forgotton console.log statement feat: POC for adding spotlight overlay for ssr-errors (getsentry#364) ref: More concise event helpers (getsentry#431) fix: Fix CI issues (getsentry#432) fix: Send something immediately to trigger open event (getsentry#429) ref: More robust client init code generation (getsentry#426) docs: Add note about Volta - pnpm support (getsentry#425) Added Search in Trace detail (getsentry#424) Added Trace Info (getsentry#423) chore(deps-dev): bump vite from 4.5.2 to 4.5.3 (getsentry#392) chore(deps): bump braces from 3.0.2 to 3.0.3 (getsentry#415) docs: Add build step to initial setup (getsentry#418)

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/astro@2.1.0 ### Minor Changes - A new Vite plugin under the main `@spotlightjs/spotlight` package that automatically injects spotlight for dev mode. It ([#434](#434)) also replaces Vite's error page shown on compilation errors with Spotlight. - 1. Altered the ssr-page vite plugin in @spotlight/astro to run spotlight overlay in fullscreen mode in ssr-error page. ([#364](#364)) 2. Closed astro erro overlay. 3. Added a option in sentry integration to open first error encountered in spotlight automatically. ### Patch Changes - Updated dependencies \[[`a8c09cd8629677ab3eed4bf7000de4c7068538ee`](a8c09cd)]: - @spotlightjs/spotlight@2.1.0 ## @spotlightjs/overlay@2.1.0 ### Minor Changes - 1. Altered the ssr-page vite plugin in @spotlight/astro to run spotlight overlay in fullscreen mode in ssr-error page. ([#364](#364)) 2. Closed astro erro overlay. 3. Added a option in sentry integration to open first error encountered in spotlight automatically. ### Patch Changes - Added search bar in trace ([#424](#424)) - - Show active span item in trace when span info is opened. ([#437](#437)) - Show active event in DeveloperInfo tab when event is info is opened. - added traceInfo ([#423](#423)) - Overhaul envelope parsing to be spec compliant ([#439](#439)) ## @spotlightjs/sidecar@1.5.0 ### Minor Changes - A new Vite plugin under the main `@spotlightjs/spotlight` package that automatically injects spotlight for dev mode. It ([#434](#434)) also replaces Vite's error page shown on compilation errors with Spotlight. ## @spotlightjs/spotlight@2.1.0 ### Minor Changes - A new Vite plugin under the main `@spotlightjs/spotlight` package that automatically injects spotlight for dev mode. It ([#434](#434)) also replaces Vite's error page shown on compilation errors with Spotlight. ### Patch Changes - Updated dependencies \[[`01321f8824ae133dc02a1d829c25952c884bf631`](01321f8), [`a8c09cd8629677ab3eed4bf7000de4c7068538ee`](a8c09cd), [`3792a5e742b3888a980a0b865fd23be941809040`](3792a5e), [`b5249aa761c783739543dc7bf27cdd8d0fe8cebe`](b5249aa), [`1c7896e02a2b81715c4e5c47cbb2fd6145868ab1`](1c7896e), [`41d90455fa94df0a01e93fd90574974dfca96764`](41d9045)]: - @spotlightjs/overlay@2.1.0 - @spotlightjs/sidecar@1.5.0 ## @spotlightjs/electron@1.0.1 ### Patch Changes - Updated dependencies \[[`01321f8824ae133dc02a1d829c25952c884bf631`](01321f8), [`a8c09cd8629677ab3eed4bf7000de4c7068538ee`](a8c09cd), [`3792a5e742b3888a980a0b865fd23be941809040`](3792a5e), [`b5249aa761c783739543dc7bf27cdd8d0fe8cebe`](b5249aa), [`1c7896e02a2b81715c4e5c47cbb2fd6145868ab1`](1c7896e), [`41d90455fa94df0a01e93fd90574974dfca96764`](41d9045)]: - @spotlightjs/overlay@2.1.0 - @spotlightjs/sidecar@1.5.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Before opening this PR: - [x] I added a [Changeset Entry](https://spotlightjs.com/contribute/changesets/) with `pnpm changeset:add` - [ ] I referenced issues that this PR addresses - Added back Search input - used context for search query(related #424 (comment)) - we can extend this and make a context only for trace detail. - added a hook for search input --------- Co-authored-by: Burak Yigit Kaya <byk@sentry.io>

Closes #421. ![image](https://github.com/getsentry/spotlight/assets/43654389/17b15356-010d-48f5-88ca-a0c3e1fa278c) [Screencast from 06-23-2024 07:28:07 PM.webm](https://github.com/getsentry/spotlight/assets/43654389/eb5bb8d1-8cd9-4051-ac84-c603b5cb6989)

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/astro@2.1.0 ### Minor Changes - A new Vite plugin under the main `@spotlightjs/spotlight` package that automatically injects spotlight for dev mode. It ([#434](#434)) also replaces Vite's error page shown on compilation errors with Spotlight. - 1. Altered the ssr-page vite plugin in @spotlight/astro to run spotlight overlay in fullscreen mode in ssr-error page. ([#364](#364)) 2. Closed astro erro overlay. 3. Added a option in sentry integration to open first error encountered in spotlight automatically. ### Patch Changes - Updated dependencies \[[`a8c09cd8629677ab3eed4bf7000de4c7068538ee`](a8c09cd)]: - @spotlightjs/spotlight@2.1.0 ## @spotlightjs/overlay@2.1.0 ### Minor Changes - 1. Altered the ssr-page vite plugin in @spotlight/astro to run spotlight overlay in fullscreen mode in ssr-error page. ([#364](#364)) 2. Closed astro erro overlay. 3. Added a option in sentry integration to open first error encountered in spotlight automatically. ### Patch Changes - Added search bar in trace ([#424](#424)) - - Show active span item in trace when span info is opened. ([#437](#437)) - Show active event in DeveloperInfo tab when event is info is opened. - added traceInfo ([#423](#423)) - Overhaul envelope parsing to be spec compliant ([#439](#439)) ## @spotlightjs/sidecar@1.5.0 ### Minor Changes - A new Vite plugin under the main `@spotlightjs/spotlight` package that automatically injects spotlight for dev mode. It ([#434](#434)) also replaces Vite's error page shown on compilation errors with Spotlight. ## @spotlightjs/spotlight@2.1.0 ### Minor Changes - A new Vite plugin under the main `@spotlightjs/spotlight` package that automatically injects spotlight for dev mode. It ([#434](#434)) also replaces Vite's error page shown on compilation errors with Spotlight. ### Patch Changes - Updated dependencies \[[`01321f8824ae133dc02a1d829c25952c884bf631`](01321f8), [`a8c09cd8629677ab3eed4bf7000de4c7068538ee`](a8c09cd), [`3792a5e742b3888a980a0b865fd23be941809040`](3792a5e), [`b5249aa761c783739543dc7bf27cdd8d0fe8cebe`](b5249aa), [`1c7896e02a2b81715c4e5c47cbb2fd6145868ab1`](1c7896e), [`41d90455fa94df0a01e93fd90574974dfca96764`](41d9045)]: - @spotlightjs/overlay@2.1.0 - @spotlightjs/sidecar@1.5.0 ## @spotlightjs/electron@1.0.1 ### Patch Changes - Updated dependencies \[[`01321f8824ae133dc02a1d829c25952c884bf631`](01321f8), [`a8c09cd8629677ab3eed4bf7000de4c7068538ee`](a8c09cd), [`3792a5e742b3888a980a0b865fd23be941809040`](3792a5e), [`b5249aa761c783739543dc7bf27cdd8d0fe8cebe`](b5249aa), [`1c7896e02a2b81715c4e5c47cbb2fd6145868ab1`](1c7896e), [`41d90455fa94df0a01e93fd90574974dfca96764`](41d9045)]: - @spotlightjs/overlay@2.1.0 - @spotlightjs/sidecar@1.5.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Before opening this PR: - [x] I added a [Changeset Entry](https://spotlightjs.com/contribute/changesets/) with `pnpm changeset:add` - [ ] I referenced issues that this PR addresses - Added back Search input - used context for search query(related #424 (comment)) - we can extend this and make a context only for trace detail. - added a hook for search input --------- Co-authored-by: Burak Yigit Kaya <byk@sentry.io>

Added Search in Trace detail

8e5e31e

Shubhdeep12 requested review from HazAT and Lms24 June 23, 2024 14:00

BYK requested changes Jun 26, 2024

View reviewed changes

PR reviews resolved and modify the useDebounce hook

e5826fe

vercel bot deployed to Preview June 27, 2024 03:40 View deployment

Shubhdeep12 requested a review from BYK June 27, 2024 03:40

BYK approved these changes Jun 27, 2024

View reviewed changes

packages/overlay/src/integrations/sentry/components/traces/TraceDetails.tsx Outdated Show resolved Hide resolved

packages/overlay/src/integrations/sentry/hooks/useDebounce.ts Show resolved Hide resolved

Burak Yigit Kaya added 2 commits June 28, 2024 23:26

use relative path for cross icon import

323a289

add some docs, remove unnecessary controlled input

a6b40ca

vercel bot had a problem deploying to Preview June 28, 2024 22:35 Failure

Merge branch 'main' into feat/search-trace-detail

22df283

vercel bot had a problem deploying to Preview June 28, 2024 22:54 Failure

revert lockfile change

2e3dbf1

vercel bot had a problem deploying to Preview June 28, 2024 22:57 Failure

BYK approved these changes Jun 28, 2024

View reviewed changes

fix type errors

2fc1863

vercel bot deployed to Preview June 28, 2024 23:07 View deployment

revert lockfile change

c8c76f0

vercel bot deployed to Preview June 28, 2024 23:15 View deployment

BYK merged commit 01321f8 into main Jun 28, 2024

BYK deleted the feat/search-trace-detail branch June 28, 2024 23:19

github-actions bot mentioned this pull request Jul 22, 2024

meta(changelog): Update package versions #444

Merged

Shubhdeep12 mentioned this pull request Mar 7, 2025

feat: Added Search support in Trace details #730

Merged

2 tasks

		@@ -0,0 +1,17 @@
		import { useEffect, useState } from 'react';

		export default function useDebounce<T>(value: T, delay: number): T {

Uh oh!

Conversation

Shubhdeep12 commented Jun 23, 2024

Uh oh!

vercel bot commented Jun 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BYK left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shubhdeep12 Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BYK left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel bot commented Jun 23, 2024 •

edited

Loading

Shubhdeep12 Jun 27, 2024 •

edited

Loading