Refactor and test RNSentryModuleImpl.fetchNativeDeviceContexts #4253
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 tasks
antonis
requested review from
krystofwoldrich and
lucas-zimerman
as code owners
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 17fc2f7+dirty | 409.50 ms | 462.68 ms | 53.18 ms |
| 093a11f+dirty | 415.54 ms | 461.00 ms | 45.46 ms |
| 9cee3c7+dirty | 397.49 ms | 454.02 ms | 56.53 ms |
| f7ad13b+dirty | 478.18 ms | 564.25 ms | 86.07 ms |
| 168e871+dirty | 508.96 ms | 599.09 ms | 90.13 ms |
| c84e457+dirty | 395.52 ms | 432.24 ms | 36.72 ms |
| e62bf30+dirty | 369.36 ms | 405.27 ms | 35.92 ms |
| e045c61+dirty | 434.98 ms | 463.36 ms | 28.38 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 17fc2f7+dirty | 7.15 MiB | 8.35 MiB | 1.20 MiB |
| 093a11f+dirty | 7.15 MiB | 8.35 MiB | 1.20 MiB |
| 9cee3c7+dirty | 7.15 MiB | 8.35 MiB | 1.20 MiB |
| f7ad13b+dirty | 7.15 MiB | 8.35 MiB | 1.20 MiB |
| 168e871+dirty | 7.15 MiB | 8.35 MiB | 1.20 MiB |
| c84e457+dirty | 7.15 MiB | 8.35 MiB | 1.20 MiB |
| e62bf30+dirty | 7.15 MiB | 8.39 MiB | 1.24 MiB |
| e045c61+dirty | 7.15 MiB | 8.35 MiB | 1.20 MiB |
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f7ad13b+dirty | 1217.18 ms | 1220.83 ms | 3.65 ms |
| 093a11f+dirty | 1212.48 ms | 1229.21 ms | 16.73 ms |
| c84e457+dirty | 1218.30 ms | 1224.08 ms | 5.78 ms |
| 168e871+dirty | 1205.87 ms | 1221.37 ms | 15.50 ms |
| 9cee3c7+dirty | 1211.82 ms | 1220.43 ms | 8.61 ms |
| e045c61+dirty | 1236.80 ms | 1238.87 ms | 2.08 ms |
| 17fc2f7+dirty | 1216.04 ms | 1220.39 ms | 4.35 ms |
| e62bf30+dirty | 1238.96 ms | 1243.39 ms | 4.43 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f7ad13b+dirty | 2.36 MiB | 3.10 MiB | 753.14 KiB |
| 093a11f+dirty | 2.36 MiB | 3.10 MiB | 752.44 KiB |
| c84e457+dirty | 2.36 MiB | 3.10 MiB | 751.68 KiB |
| 168e871+dirty | 2.36 MiB | 3.08 MiB | 736.99 KiB |
| 9cee3c7+dirty | 2.36 MiB | 3.08 MiB | 737.39 KiB |
| e045c61+dirty | 2.36 MiB | 3.10 MiB | 753.19 KiB |
| 17fc2f7+dirty | 2.36 MiB | 3.08 MiB | 737.22 KiB |
| e62bf30+dirty | 2.36 MiB | 3.15 MiB | 802.94 KiB |
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f7ad13b+dirty | 1237.70 ms | 1239.14 ms | 1.44 ms |
| 093a11f+dirty | 1238.33 ms | 1237.33 ms | -1.01 ms |
| c84e457+dirty | 1229.50 ms | 1226.76 ms | -2.74 ms |
| 168e871+dirty | 1234.31 ms | 1227.67 ms | -6.64 ms |
| 9cee3c7+dirty | 1237.08 ms | 1232.16 ms | -4.92 ms |
| e045c61+dirty | 1225.21 ms | 1230.33 ms | 5.12 ms |
| 17fc2f7+dirty | 1234.25 ms | 1234.78 ms | 0.53 ms |
| e62bf30+dirty | 1239.42 ms | 1242.14 ms | 2.73 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f7ad13b+dirty | 2.92 MiB | 3.66 MiB | 758.41 KiB |
| 093a11f+dirty | 2.92 MiB | 3.66 MiB | 757.73 KiB |
| c84e457+dirty | 2.92 MiB | 3.66 MiB | 756.01 KiB |
| 168e871+dirty | 2.92 MiB | 3.64 MiB | 742.68 KiB |
| 9cee3c7+dirty | 2.92 MiB | 3.64 MiB | 743.04 KiB |
| e045c61+dirty | 2.92 MiB | 3.66 MiB | 758.40 KiB |
| 17fc2f7+dirty | 2.92 MiB | 3.64 MiB | 743.07 KiB |
| e62bf30+dirty | 2.92 MiB | 3.71 MiB | 808.09 KiB |
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 168e871 | 471.56 ms | 451.59 ms | -19.97 ms |
| f7ad13b | 488.43 ms | 500.04 ms | 11.62 ms |
| e045c61 | 395.70 ms | 386.29 ms | -9.41 ms |
| 093a11f | 471.67 ms | 487.81 ms | 16.15 ms |
| 9cee3c7 | 509.72 ms | 513.58 ms | 3.86 ms |
| e62bf30 | 481.92 ms | 471.48 ms | -10.44 ms |
| 17fc2f7 | 420.78 ms | 512.84 ms | 92.07 ms |
| c84e457 | 490.41 ms | 489.28 ms | -1.13 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 168e871 | 17.74 MiB | 20.08 MiB | 2.34 MiB |
| f7ad13b | 17.74 MiB | 20.07 MiB | 2.34 MiB |
| e045c61 | 17.74 MiB | 20.07 MiB | 2.34 MiB |
| 093a11f | 17.74 MiB | 20.07 MiB | 2.34 MiB |
| 9cee3c7 | 17.74 MiB | 20.08 MiB | 2.34 MiB |
| e62bf30 | 17.73 MiB | 20.11 MiB | 2.38 MiB |
| 17fc2f7 | 17.74 MiB | 20.08 MiB | 2.34 MiB |
| c84e457 | 17.74 MiB | 20.08 MiB | 2.34 MiB |
krystofwoldrich
approved these changes
Nov 11, 2024
antonis
added a commit
that referenced
this pull request
Nov 18, 2024
#4240) * Adds breadcrumb origin in RNSentryBreadcrumb dictionary parsing * Use 8.38.0-beta.1 Cocoa SDK that has the new origin field * Adds changelog * Fixes sentry-java breaking changes * Adds origin native tests * Adds Capture exception with breadcrumb in the sample * Set react native as event origin * Filter out events with react-native origin from the native layer * Merge event breadcrumbs with native context * Lint: removes empty line * Use predicate to filter breadcrumbs * Respect max breadcrumbs limit * Updates changelog * Update test names * Fixes lint issue * Filter out DevServer and DSN related breadcrumbs * Adds changelog * Keep the last maxBreadcrumbs (default 100) when merging native and js breadcrumbs * Use client from function parameter * Refactor and test RNSentryModuleImpl.fetchNativeDeviceContexts (#4253) * Refactor fetchNativeDeviceContexts for testability * Test fetchNativeDeviceContexts * Adds new line at the end * Revert "Filter out DevServer and DSN related breadcrumbs" This reverts commit 87bdc77. * Passes development server url as an option to the native sdks * Filter out Dev Server and Sentry Dsn breadcrumbs on Android * Filter out Dev Server and Sentry Dsn breadcrumbs on iOS * Filter out Dev Server and Sentry Dsn breadcrumbs on JS * Adds Java tests * Adds Cocoa tests * Adds JS tests * Sets correct spacing in import Co-authored-by: LucasZF <[email protected]> * Fixes changelog typo Co-authored-by: LucasZF <[email protected]> * Handles undefined dev server urls Co-authored-by: LucasZF <[email protected]> * Handles undefined dsns * Adds tests for undefined dev servers and dsns * Handles undefined dev server urls in native code * Updates test cases * Uses the url from the passed dsn to filter breadcrumbs * Handles nil dsn though this state should never be reached due to SentryOptions validation * Use startsWith to check url matching Co-authored-by: LucasZF <[email protected]> * Check url with prefix instead of contains for efficiency * Fix comment typo Co-authored-by: LucasZF <[email protected]> * Fix comment typo Co-authored-by: LucasZF <[email protected]> * Fix comment typo Co-authored-by: LucasZF <[email protected]> * Update CHANGELOG.md * Safely parses dsn url * Adds test case for url exception --------- Co-authored-by: LucasZF <[email protected]> Co-authored-by: Krystof Woldrich <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📢 Type of change
📜 Description
Refactors RNSentryModuleImpl.fetchNativeDeviceContexts and adds tests
💡 Motivation and Context
See https://github.com/getsentry/sentry-react-native/pull/4124/files#r1832593973
💚 How did you test it?
CI, Manual testing
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
#skip-changelog