-
-
Notifications
You must be signed in to change notification settings - Fork 372
ref: Convert SentryMetricKitIntegration to Swift #7076
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7076 +/- ##
========================================
Coverage ? 84.860%
========================================
Files ? 459
Lines ? 27517
Branches ? 12135
========================================
Hits ? 23351
Misses ? 4124
Partials ? 42
Continue to review full report in Codecov by Sentry.
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 43597ba | 1214.88 ms | 1243.52 ms | 28.65 ms |
| 9214f4c | 1223.19 ms | 1246.96 ms | 23.77 ms |
| e70670c | 1223.47 ms | 1238.67 ms | 15.20 ms |
| 59fc426 | 1240.49 ms | 1256.94 ms | 16.45 ms |
| 6cb4338 | 1238.47 ms | 1256.96 ms | 18.49 ms |
| 41c0aa8 | 1222.75 ms | 1262.40 ms | 39.65 ms |
| 35f6d9e | 1222.24 ms | 1253.48 ms | 31.23 ms |
| d6ab868 | 1228.98 ms | 1260.51 ms | 31.53 ms |
| ec3fc3a | 1212.92 ms | 1245.06 ms | 32.14 ms |
| 781f560 | 1232.83 ms | 1263.56 ms | 30.73 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 43597ba | 23.75 KiB | 880.32 KiB | 856.58 KiB |
| 9214f4c | 24.14 KiB | 1.02 MiB | 1017.26 KiB |
| e70670c | 23.75 KiB | 975.19 KiB | 951.45 KiB |
| 59fc426 | 23.75 KiB | 920.64 KiB | 896.89 KiB |
| 6cb4338 | 23.75 KiB | 913.63 KiB | 889.88 KiB |
| 41c0aa8 | 23.75 KiB | 986.80 KiB | 963.05 KiB |
| 35f6d9e | 23.75 KiB | 1.01 MiB | 1006.48 KiB |
| d6ab868 | 24.14 KiB | 1.01 MiB | 1015.23 KiB |
| ec3fc3a | 23.74 KiB | 1022.75 KiB | 999.01 KiB |
| 781f560 | 23.75 KiB | 1.02 MiB | 1016.46 KiB |
Previous results on branch: metricKitIntegrationSwift
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 4224e11 | 1215.87 ms | 1247.00 ms | 31.13 ms |
| f3c0092 | 1229.67 ms | 1258.32 ms | 28.65 ms |
| 7ebbb60 | 1216.83 ms | 1249.49 ms | 32.66 ms |
| 443cbc8 | 1224.02 ms | 1251.16 ms | 27.14 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 4224e11 | 24.14 KiB | 1.03 MiB | 1.00 MiB |
| f3c0092 | 24.14 KiB | 1.03 MiB | 1.00 MiB |
| 7ebbb60 | 24.14 KiB | 1.03 MiB | 1.00 MiB |
| 443cbc8 | 24.14 KiB | 1.03 MiB | 1.00 MiB |
5d0c649 to
9f39761
Compare
9f39761 to
af78978
Compare
Tests/SentryTests/Integrations/MetricKit/SentryMetricKitEventTests.swift
Outdated
Show resolved
Hide resolved
37d4a1a to
7062639
Compare
b8ff92c to
9529672
Compare
Tests/SentryTests/Integrations/MetricKit/SentryMetricKitIntegrationTests.swift
Outdated
Show resolved
Hide resolved
9529672 to
94d6460
Compare
itaybre
left a comment
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.
LGTM
94d6460 to
2028321
Compare
Resolves https://linear.app/getsentry/issue/COCOA-989/refactor-sentrymetrickitintegrationm-in-swift
I converted most of this in my last PR so this just follows up to convert the rest
There is one logic change here. If MetricKit ever delivered a CPU exception event with
callStackPerThread = truewe would now still handle it. It's not expected to happen, but there's no reason to have a guard case specifically for it, just like we don't have any for app hangs.