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

refactor: Add file IO tracker to dependency injection container #4861

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

philprime
Copy link
Contributor

@philprime philprime commented Feb 18, 2025

This PR is derived from #4605

It moves the File IO tracker to the dependency injection container because it will be used by automatic/swizzled tracking and manual tracking.

Note: We decided to use this approach for this case, because it is the simplest way to enable access to the shared file IO tracker in and #4862 and #4863. Eventually we have to revisit the dependency container.

#skip-changelog

Copy link

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNSDataSwizzling.m
  • Sources/Sentry/SentryNSFileManagerSwizzling.m

Copy link

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1224.20 ms 1258.78 ms 34.57 ms
Size 22.32 KiB 820.27 KiB 797.95 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
0ffe199 1209.51 ms 1223.94 ms 14.43 ms
4597906 1222.88 ms 1235.75 ms 12.88 ms
32ac934 1238.86 ms 1243.37 ms 4.52 ms
2df93b0 1210.73 ms 1228.76 ms 18.02 ms
bbcbaff 1216.82 ms 1242.34 ms 25.52 ms
0f4071f 1212.80 ms 1239.22 ms 26.43 ms
3297d6e 1228.57 ms 1255.04 ms 26.47 ms
4ea2c00 1216.19 ms 1237.12 ms 20.93 ms
44ce888 1208.98 ms 1224.72 ms 15.74 ms
1b69ee7 1246.08 ms 1257.71 ms 11.63 ms

App size

Revision Plain With Sentry Diff
0ffe199 20.76 KiB 436.50 KiB 415.74 KiB
4597906 21.58 KiB 678.19 KiB 656.61 KiB
32ac934 21.58 KiB 616.72 KiB 595.14 KiB
2df93b0 21.58 KiB 682.40 KiB 660.81 KiB
bbcbaff 22.85 KiB 414.09 KiB 391.24 KiB
0f4071f 21.58 KiB 681.72 KiB 660.14 KiB
3297d6e 21.58 KiB 418.45 KiB 396.86 KiB
4ea2c00 21.58 KiB 706.47 KiB 684.88 KiB
44ce888 22.85 KiB 414.65 KiB 391.80 KiB
1b69ee7 21.58 KiB 707.43 KiB 685.84 KiB

Copy link

codecov bot commented Feb 18, 2025

Codecov Report

Attention: Patch coverage is 81.25000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 92.245%. Comparing base (43a41b3) to head (3dab8d0).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
Sources/Sentry/SentryFileIOTracker.m 0.000% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4861       +/-   ##
=============================================
+ Coverage   92.164%   92.245%   +0.081%     
=============================================
  Files          658       658               
  Lines        77222     77243       +21     
  Branches     27170     27966      +796     
=============================================
+ Hits         71171     71253       +82     
+ Misses        5957      5891       -66     
- Partials        94        99        +5     
Files with missing lines Coverage Δ
Sources/Sentry/SentryDependencyContainer.m 96.835% <100.000%> (+0.124%) ⬆️
Sources/Sentry/SentryFileIOTrackingIntegration.m 100.000% <100.000%> (ø)
Sources/Sentry/SentryFileIOTracker.m 93.670% <0.000%> (-1.813%) ⬇️

... and 26 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 43a41b3...3dab8d0. Read the comment docs.

@philprime philprime marked this pull request as ready for review February 18, 2025 13:33
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.

@philprime philprime merged commit 9ef38ea into main Feb 20, 2025
72 of 73 checks passed
@philprime philprime deleted the philprime/refactor-file-tracker branch February 20, 2025 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants