-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Trace:Details Follow up to pr10745 and additional bug fixes. #10778
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
Conversation
0721d38 to
9acb317
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10778 +/- ##
==========================================
- Coverage 60.53% 60.52% -0.01%
==========================================
Files 4486 4486
Lines 120223 120213 -10
Branches 19925 19924 -1
==========================================
- Hits 72772 72761 -11
+ Misses 42410 42408 -2
- Partials 5041 5044 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| spanList.forEach((span) => { | ||
| allSpanIds.add(span.spanId); | ||
| if (span.children.length > 0) { | ||
| gather(span.children); |
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.
Do we not need the recursion for any use case?
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.
gatherAllSpanIds replaced by simpler new Set(filteredSpans.map((span) => span.spanId)). The recursive function went through the hierarchical span list to get all spanId's, while this new approach is simpler by just going through the filtered span list to get all spanId's. The hierarchical span list is built from the filtered span list anyways.
Signed-off-by: Nathan Yang <[email protected]>
9acb317 to
c12fe49
Compare
Signed-off-by: Nathan Yang <[email protected]>
da35bf0 to
882ab23
Compare
Description
Follow up items to #10745 (comment) and additional bug fixes
SpanStatusFilterfrom multi-select to single-selectSpanStatusFilteras FCsetSpanFiltersWithStorageand add non-null assertionisSpanOkfunction, just directly use!isSpanErrorSpanHierarchyTableheight to just change based on whether it's rendered in flyout or on trace details pageScreenshot
Testing the changes
Changelog
Check List
yarn test:jestyarn test:jest_integration