Skip to content
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

fix(launch profiling): set a new trace origin for app launch profiles #3636

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented Feb 14, 2024

follows #3635

It was requested to make a new origin for these transactions: #3621 (comment)

I wasn't sure what to do for nameSource, another parameter required on the init I needed to use, so I stuck with custom as that's the default value used in some of the other init implementations.

#skip-changelog

Copy link

codecov bot commented Feb 14, 2024

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (54c5990) 89.167% compared to head (2fdf6e9) 89.221%.

Additional details and impacted files

Impacted file tree graph

@@                               Coverage Diff                               @@
##           armcknight/feat/launch-profiling5-cleanup     #3636       +/-   ##
===============================================================================
+ Coverage                                     89.167%   89.221%   +0.054%     
===============================================================================
  Files                                            532       532               
  Lines                                          58659     58675       +16     
  Branches                                       21046     21052        +6     
===============================================================================
+ Hits                                           52305     52351       +46     
+ Misses                                          5309      5289       -20     
+ Partials                                        1045      1035       -10     
Files Coverage Δ
...entryProfilerTests/SentryAppLaunchProfilingTests.m 100.000% <100.000%> (ø)
Sources/Sentry/Profiling/SentryLaunchProfiling.m 61.290% <44.000%> (-1.673%) ⬇️

... and 14 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54c5990...2fdf6e9. Read the comment docs.

Copy link

github-actions bot commented Feb 14, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1201.39 ms 1210.33 ms 8.95 ms
Size 21.58 KiB 422.62 KiB 401.04 KiB

Baseline results on branch: armcknight/feat/launch-profiling5-cleanup

Startup times

Revision Plain With Sentry Diff
4916322 1211.00 ms 1220.00 ms 9.00 ms
b4dffe4 1205.65 ms 1222.49 ms 16.84 ms

App size

Revision Plain With Sentry Diff
4916322 21.58 KiB 422.53 KiB 400.95 KiB
b4dffe4 21.58 KiB 422.53 KiB 400.95 KiB

Previous results on branch: armcknight/feat/launch-profiling6-trace-origin

Startup times

Revision Plain With Sentry Diff
a66372c 1236.47 ms 1255.33 ms 18.86 ms
990435a 1207.18 ms 1215.37 ms 8.18 ms
3d78f77 1214.04 ms 1223.23 ms 9.19 ms

App size

Revision Plain With Sentry Diff
a66372c 21.58 KiB 422.67 KiB 401.09 KiB
990435a 21.58 KiB 422.67 KiB 401.09 KiB
3d78f77 21.58 KiB 422.67 KiB 401.09 KiB

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sources/Sentry/Profiling/SentryLaunchProfiling.m Outdated Show resolved Hide resolved
@armcknight armcknight force-pushed the armcknight/feat/launch-profiling6-trace-origin branch from 25c6208 to 45e58d4 Compare February 15, 2024 04:20
@armcknight armcknight merged commit 43fb80d into armcknight/feat/launch-profiling5-cleanup Feb 15, 2024
65 of 72 checks passed
@armcknight armcknight deleted the armcknight/feat/launch-profiling6-trace-origin branch February 15, 2024 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants