-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(tracemetrics): Improve samples panel rendering #101448
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(tracemetrics): Improve samples panel rendering #101448
Conversation
| limit: traceIds.length, | ||
| referrer: 'api.explore.trace-errors-count', | ||
| orgSlug: organization.slug, | ||
| location, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential bug: The useDiscoverQuery hook is called with an undefined location variable, which will cause a runtime ReferenceError.
-
Description: The
useDiscoverQueryhook is called on line 56 with alocationvariable. However, this variable is not defined within the scope of theuseTraceTelemetryhook. It is not passed as a parameter, imported, or declared locally. SinceuseDiscoverQueryrequireslocationas a mandatory prop, this will result in aReferenceError: location is not definedwhen the component renders and the query is executed, causing a runtime crash. -
Suggested fix: The
useTraceTelemetryhook needs to acceptlocationas a parameter. Update its options interface, add it to the function signature, and pass it down touseDiscoverQuery. The calling component,samplesTab.tsx, should then pass itslocationobject to the hook.
severity: 0.85, confidence: 1.0
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird, this wasn't caught by my linter earlier.
|
|
||
| return dataMap; | ||
| }, [traceIds, spansResult.data, logsResult.data]); | ||
| }, [traceIds, spansResult.data, logsResult.data, errorsResult.data]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Incorrect Dependency in useMemo Hook
The telemetryData useMemo hook's dependency array includes errorsResult.data, but the error count data is accessed via errorsResult.data.data. This means the memoized value won't update when the actual error data changes, leading to stale error counts and an inconsistent data access pattern compared to other queries.
Additional Locations (1)
- Removes spans and logs columns - Add call to discover endpoint to get error counts by trace ID - Renders timestamp using the same renderer as logs Closes LOGS-407
Closes LOGS-407