feat(preprod): Xray diff #105016
feat(preprod): Xray diff #105016
Conversation
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #105016 +/- ##
============================================
- Coverage 80.28% 58.17% -22.11%
============================================
Files 9397 9273 -124
Lines 403677 400242 -3435
Branches 25925 25924 -1
============================================
- Hits 324073 232825 -91248
- Misses 79164 166977 +87813
Partials 440 440 |
| // Determine the diff type based on the aggregated size diff | ||
| if (totalSizeDiff > 0) { | ||
| node.diff_type = 'increased'; | ||
| } else if (totalSizeDiff < 0) { | ||
| node.diff_type = 'decreased'; | ||
| } | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| <Separator orientation="horizontal" border="primary" /> | ||
|
|
||
| <Stack gap="md"> | ||
| <Heading as="h2">{t('Size diff')}</Heading> |
There was a problem hiding this comment.
Can you make this "Size Diff"? Or maybe we go back to our tried and true "X-Ray Diff", main thing is capitalizing second letter, thats convention in rest of sentry for these headings
There was a problem hiding this comment.
Will do "X-Ray Diff" - copied this from designs so that's why I chose the caps
| throw new Error(`Diff type ${element.diff_type} not found`); | ||
| } | ||
|
|
||
| const data: any = { |
There was a problem hiding this comment.
nit: is there a type to use for this? I see we are using TreemapSeriesOption below
There was a problem hiding this comment.
Yeah but leads to some strange type errors, but I just added ignores for those.
| ); | ||
| } | ||
|
|
||
| const ButtonContainer = styled(Flex)` |
There was a problem hiding this comment.
nit: it seems like most of this could be done via component props but will leave that up to you
| tooltip={tooltip} | ||
| onChartReady={handleChartReady} | ||
| /> | ||
| <ButtonContainer |
There was a problem hiding this comment.
nit: or maybe ButtonContainer could be shared, but also up to you
| import {buildTreeFromDiffItems, buildTreemapDiff} from './treemapDiffUtils'; | ||
|
|
||
| describe('treemapDiffUtils', () => { | ||
| describe('buildTreeFromDiffItems', () => { |
There was a problem hiding this comment.
One edge case we dealt with at Emerge that might be worth having a test case for:
(A -50MB)
| |
(B -51MB) (C +1MB)
B and C are children of A. In this case A and B should show green since they were reduced, but C should show red since it increased despite the overall parent decreasing.
001d46d to
2195773
Compare
Adds Xray diff to size comparison page. Implements the treemap diff client-side leveraging diff_items, allowing us to never be tied to backend comparison for this, which should allow us to easily iterate on this in the future.
Resolves EME-207