-
-
Notifications
You must be signed in to change notification settings - Fork 375
chore: Convert SentryFramesTrackingIntegration to Swift
#7044
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
base: main
Are you sure you want to change the base?
chore: Convert SentryFramesTrackingIntegration to Swift
#7044
Conversation
|
Lint fails, and that's expected for now, enable |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7044 +/- ##
=============================================
- Coverage 84.710% 84.691% -0.020%
=============================================
Files 459 459
Lines 27490 27482 -8
Branches 12117 12116 -1
=============================================
- Hits 23287 23275 -12
- Misses 4162 4166 +4
Partials 41 41
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
…coa-999-refactor-sentryframestrackingintegrationm-in-swift
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| c6d2354 | 1226.53 ms | 1252.69 ms | 26.16 ms |
| feaf817 | 1217.00 ms | 1244.92 ms | 27.92 ms |
| 51b7dd3 | 1235.06 ms | 1258.21 ms | 23.15 ms |
| 543b791 | 1234.23 ms | 1265.49 ms | 31.26 ms |
| 9766cb1 | 1233.54 ms | 1267.65 ms | 34.10 ms |
| 397b9c9 | 1230.23 ms | 1249.29 ms | 19.06 ms |
| 27e7514 | 1229.47 ms | 1245.60 ms | 16.13 ms |
| 5cbd333 | 1219.93 ms | 1241.76 ms | 21.83 ms |
| 67e2286 | 1217.83 ms | 1250.04 ms | 32.21 ms |
| fa27d5b | 1195.50 ms | 1218.19 ms | 22.69 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| c6d2354 | 23.75 KiB | 919.70 KiB | 895.95 KiB |
| feaf817 | 24.14 KiB | 1.03 MiB | 1.01 MiB |
| 51b7dd3 | 23.75 KiB | 913.26 KiB | 889.52 KiB |
| 543b791 | 24.14 KiB | 1.01 MiB | 1010.98 KiB |
| 9766cb1 | 23.75 KiB | 1023.78 KiB | 1000.03 KiB |
| 397b9c9 | 23.75 KiB | 959.44 KiB | 935.70 KiB |
| 27e7514 | 23.75 KiB | 919.69 KiB | 895.94 KiB |
| 5cbd333 | 23.74 KiB | 969.77 KiB | 946.02 KiB |
| 67e2286 | 24.14 KiB | 1.02 MiB | 1022.54 KiB |
| fa27d5b | 24.14 KiB | 1.01 MiB | 1015.38 KiB |
Previous results on branch: itay/cocoa-999-refactor-sentryframestrackingintegrationm-in-swift
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 638b07b | 1216.77 ms | 1245.51 ms | 28.74 ms |
| 7c7d5e2 | 1224.90 ms | 1255.13 ms | 30.23 ms |
| 9b8916c | 1223.80 ms | 1245.29 ms | 21.49 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 638b07b | 24.14 KiB | 1.03 MiB | 1.01 MiB |
| 7c7d5e2 | 24.14 KiB | 1.03 MiB | 1.00 MiB |
| 9b8916c | 24.14 KiB | 1.03 MiB | 1.00 MiB |
|
|
||
| init?(with options: Options, dependencies: Dependencies) { | ||
| // Check hybrid SDK mode first - if enabled, always start frames tracking | ||
| if !PrivateSentrySDKOnly.framesTrackingMeasurementHybridSDKMode { |
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.
h: I don't understand where all of this logic comes from, can you please elaborate why the Swift version has much more logic here?
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.
Looks like I forgot to remove the old code.
But this is the code in SentryBaseIntegration: https://github.com/getsentry/sentry-cocoa/blob/main/Sources/Sentry/SentryBaseIntegration.m#L157
This is what determines if the integration is enabled or not
…ntryBaseIntegration.m
…coa-999-refactor-sentryframestrackingintegrationm-in-swift
|
|
||
| #if (os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) || os(visionOS)) && !SENTRY_NO_UIKIT | ||
| integrations.append(.init(SentryFramesTrackingIntegration<SentryDependencyContainer>.self)) | ||
| #endif |
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.
Integration order change breaks frames tracker dependency
The conversion moves SentryFramesTrackingIntegration from ObjC integrations to Swift integrations, which changes when it's installed. Previously it was installed early, before SentryWatchdogTerminationTrackingIntegration. Now Swift integrations are installed after all ObjC integrations, so the frames tracker starts after the watchdog termination integration. Since SentryANRTrackerV2 (used by watchdog tracking) depends on the frames tracker to compute frame delays, calling getFramesDelaySPI returns -1 when isRunning is false, causing ANR detection to skip frames-based analysis until the Swift integrations complete installation. The comment in the old code states "the watchdog tracker uses the frames tracker, so frame tracking must be enabled if watchdog tracking is enabled."
Additional Locations (1)
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.
Interesting, worth checking out
…coa-999-refactor-sentryframestrackingintegrationm-in-swift
Converts
SentryFramesTrackingIntegrationto Swift as part of the Swift project#skip-changelog
Closes #7066