-
-
Notifications
You must be signed in to change notification settings - Fork 343
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
feat: append launch profile data to app start transactions/spans #3736
feat: append launch profile data to app start transactions/spans #3736
Conversation
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- append launch profile data to app start transactions/spans ([#3736](https://github.com/getsentry/sentry-cocoa/pull/3736)) If none of the above apply, you can opt out of this check by adding |
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
7054676 | 1229.46 ms | 1246.40 ms | 16.94 ms |
861d361 | 1227.90 ms | 1231.45 ms | 3.55 ms |
01a28a9 | 1237.78 ms | 1259.04 ms | 21.27 ms |
72c8d84 | 1218.69 ms | 1241.08 ms | 22.39 ms |
282cc99 | 1232.59 ms | 1245.88 ms | 13.29 ms |
d0deebd | 1235.17 ms | 1255.54 ms | 20.37 ms |
c2acec5 | 1243.04 ms | 1254.15 ms | 11.10 ms |
8c50edb | 1212.98 ms | 1233.72 ms | 20.74 ms |
ecd9ecd | 1215.77 ms | 1238.70 ms | 22.93 ms |
7cd187e | 1243.04 ms | 1244.79 ms | 1.75 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
7054676 | 21.58 KiB | 542.28 KiB | 520.70 KiB |
861d361 | 20.76 KiB | 435.65 KiB | 414.89 KiB |
01a28a9 | 22.85 KiB | 405.39 KiB | 382.54 KiB |
72c8d84 | 22.85 KiB | 408.88 KiB | 386.03 KiB |
282cc99 | 22.85 KiB | 414.09 KiB | 391.24 KiB |
d0deebd | 21.58 KiB | 542.28 KiB | 520.69 KiB |
c2acec5 | 22.85 KiB | 408.88 KiB | 386.03 KiB |
8c50edb | 20.76 KiB | 432.31 KiB | 411.55 KiB |
ecd9ecd | 20.76 KiB | 420.23 KiB | 399.47 KiB |
7cd187e | 20.76 KiB | 401.65 KiB | 380.89 KiB |
Previous results on branch: armcknight/feat/launch-profile-data-in-app-start-spans
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
82b16fe | 1230.27 ms | 1244.67 ms | 14.40 ms |
07e5edc | 1244.73 ms | 1253.24 ms | 8.51 ms |
c32b818 | 1237.06 ms | 1251.63 ms | 14.56 ms |
9f00605 | 1233.63 ms | 1246.52 ms | 12.90 ms |
389fb12 | 1228.85 ms | 1249.57 ms | 20.72 ms |
0dd6fa2 | 1237.14 ms | 1255.69 ms | 18.55 ms |
9d7d5a1 | 1207.23 ms | 1230.02 ms | 22.79 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
82b16fe | 21.58 KiB | 546.50 KiB | 524.92 KiB |
07e5edc | 21.58 KiB | 545.19 KiB | 523.61 KiB |
c32b818 | 21.58 KiB | 546.60 KiB | 525.02 KiB |
9f00605 | 21.58 KiB | 542.87 KiB | 521.29 KiB |
389fb12 | 21.58 KiB | 542.88 KiB | 521.29 KiB |
0dd6fa2 | 21.58 KiB | 546.57 KiB | 524.99 KiB |
9d7d5a1 | 21.58 KiB | 546.58 KiB | 525.00 KiB |
5da3e24
to
90c0bf1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3736 +/- ##
=============================================
- Coverage 89.446% 89.437% -0.010%
=============================================
Files 556 557 +1
Lines 60045 60240 +195
Branches 21560 21668 +108
=============================================
+ Hits 53708 53877 +169
- Misses 5303 5437 +134
+ Partials 1034 926 -108
... and 43 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
90c0bf1
to
646ede9
Compare
…racking UIVC load
…t property back in .m
…at wont have that recorded in the app start spans; we'll just have to see what that data looks like
…okkeeping correctly stops the profiler, even if the trace will be sampled out later
1534090
to
69fcf6f
Compare
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.
Looking good.
Just a couple of comments.
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.
I found a few issues.
…h-profile-data-in-app-start-spans
Co-authored-by: Philipp Hofmann <[email protected]>
… an app start transaction
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.
Thanks for including most of the feedback 😃
…h-profile-data-in-app-start-spans
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.
Thanks, @armcknight. LGTM
…h-profile-data-in-app-start-spans
Following on app launch profiling that only used a dedicated span to attach the profiles, we will now attach the profile data to an automatic app start transaction if one will be recorded, otherwise continue with the same dedicated txn.
Closes #3725
Changes
SentryUIViewControllerPerformanceTracker.shared.enableWaitForFullDisplay
isNO
, otherwise, that trace will eventually end and get all the profile data. Then,SentryTimeToDisplayTracker
will callstopLaunchProfile
passingnil
as the hub argument which will stop the dedicated tracer without actually sending a duplicate dedicated transaction, since the profile data we previously attached to it is now on the app start txn.SentrySysctl.load
is called and keeps that as theruntimeInitSystemTimestamp
to report toSentryAppStartMeasurement
, which is used when SentryTracer requests a slice of profile data to attach to the app start transaction. Profile slicing needs a system timestamp which it uses to keep relevant samples and compute relative timestamps. it's set as theSentryTransaction.startSystemTime
for app start transactions.originalStartTimestamp
inSentryTracer
. We now take either the actual start time of the tracer (which was always the case outside of app start transactions, which modifiesSentryTracer.startTimestamp
) orSentryAppStartMeasurement.runtimeInitTimestamp
in the case of app start txns, since this corresponds to the time that we're recordingSentrySysctl.processStartSystemTimestamp
as mentioned above.runtimeInitTimestamp
. But, we have no reliable way to test what will happen for these launch types, so we are going to postpone any further treatment of that edge case.Before (the profile data starts at the extreme right end):
After: