-
-
Notifications
You must be signed in to change notification settings - Fork 376
ref: Convert SentryUIEventTrackingIntegration to swift #7272
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?
ref: Convert SentryUIEventTrackingIntegration to swift #7272
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7272 +/- ##
=============================================
+ Coverage 85.011% 85.044% +0.032%
=============================================
Files 471 471
Lines 28375 28397 +22
Branches 12461 12464 +3
=============================================
+ Hits 24122 24150 +28
+ Misses 4204 4197 -7
- Partials 49 50 +1
... and 10 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 83bf9af | 1213.30 ms | 1234.18 ms | 20.89 ms |
| 5d67f5d | 1225.33 ms | 1262.76 ms | 37.43 ms |
| e01eafa | 1223.36 ms | 1252.43 ms | 29.07 ms |
| 86b8951 | 1211.20 ms | 1247.92 ms | 36.72 ms |
| 9a0a3cd | 1220.85 ms | 1243.69 ms | 22.83 ms |
| e701dc8 | 1215.89 ms | 1254.06 ms | 38.17 ms |
| fd24b7c | 1222.39 ms | 1259.67 ms | 37.28 ms |
| 013fd4d | 1216.02 ms | 1242.16 ms | 26.14 ms |
| 0d145c3 | 1224.04 ms | 1255.34 ms | 31.30 ms |
| 37bc095 | 1210.00 ms | 1242.69 ms | 32.69 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 83bf9af | 24.14 KiB | 1.04 MiB | 1.02 MiB |
| 5d67f5d | 24.14 KiB | 1.06 MiB | 1.04 MiB |
| e01eafa | 24.14 KiB | 1.07 MiB | 1.04 MiB |
| 86b8951 | 24.14 KiB | 1.08 MiB | 1.06 MiB |
| 9a0a3cd | 24.14 KiB | 1.08 MiB | 1.06 MiB |
| e701dc8 | 24.14 KiB | 1.06 MiB | 1.04 MiB |
| fd24b7c | 24.14 KiB | 1.07 MiB | 1.04 MiB |
| 013fd4d | 24.14 KiB | 1.04 MiB | 1.02 MiB |
| 0d145c3 | 24.14 KiB | 1.07 MiB | 1.05 MiB |
| 37bc095 | 24.14 KiB | 1.06 MiB | 1.04 MiB |
|
|
||
| func getUIEventTracker(_ options: Options) -> SentryUIEventTracker { | ||
| let mode = SentryUIEventTrackerTransactionMode(idleTimeout: options.idleTimeout) | ||
| return SentryUIEventTracker( |
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.
Why does this need to be a function in the dependency container? It's not really a dependency in the typical sense if you're creating a new instance each time. Seems like it would be cleaner to create in-line in the integration class
📜 Description
SentryUIEventTrackingIntegrationto Swift💡 Motivation and Context
💚 How did you test it?
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.#skip-changelog
Closes #7273