spotlight icon indicating copy to clipboard operation
spotlight copied to clipboard

Trace & Error count in tabs don't update accurately

Open BYK opened this issue 9 months ago • 5 comments

Steps to Reproduce

  1. Launch Spotlight in dev mode: pnpm dev
  2. Open the browser for Spotlight web UI
  3. Go to fixtures directory: cd packages/overlay/_fixtures
  4. Inject all the sample envelopes: ./send_to_sidecar.cjs

Expected Result

Explore tab should show 17 and Errors tab should show 7.

Actual Result

Explore and Errors tabs show 13 and 3 respectively. They update to correct values upon any kind of navigation.

BYK avatar Apr 02 '25 13:04 BYK

I think we should wrap tab generation code from integrations into a useMemo here: https://github.com/getsentry/spotlight/blob/6aea29bd34b836e86e08ceb040003cb8cb89f622/packages/overlay/src/components/Overview.tsx#L20-L29

I tried that but using integrationData as the dependencies for useMemo doesn't really work as the object doesn't change but it's values do. So I actually got worse results. Not sure what's the best fix for this TBH.

BYK avatar Apr 02 '25 14:04 BYK

@BYK yeah looks like data is not updated in sentry-integration before rendering the UI causing mismatch in the count.

We can move the tabs to a state and wait for integration to finalized the updates before rendering on the UI.

Shubhdeep12 avatar Apr 05 '25 07:04 Shubhdeep12

Looks like it might be time for us to use an established data layer for our React app instead of rolling our own.

@Shubhdeep12 suggested using zustand and it looks quite promising.

BYK avatar Apr 15 '25 08:04 BYK

Yeah @BYK, we can definitely add Zustand.

But I was thinking about the integrations and the above bug.

Currently, an integration is registered in Spotlight with a specific format. This format includes a tab function that returns the tabs to be rendered by Spotlight.

In sentry-integration, the tab count is fetched from the data store inside the tab function. However, this isn’t reactive, so the data store (sentryDataCache) doesn’t automatically update the tab count.

The tab function only runs again when the Debugger or Overview components re-render. That’s when the data gets updated.

@BYK moving to Zustand would definitely be an improvement for the data store, but I don’t think it will solve this specific issue.

Shubhdeep12 avatar Apr 15 '25 09:04 Shubhdeep12

@Shubhdeep12 oh, so you are saying we need to refactor how we do integrations. That actually makes sense. We probably need to allow them to pass a "on data updated" hook function upstream. And I think switching to zustand fist would make that a lot easier?

BYK avatar Apr 15 '25 10:04 BYK