Skip to content

feat: Add extension for Data to track file I/O operations with Sentry #4862

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

Merged
merged 24 commits into from
Mar 4, 2025

Conversation

philprime
Copy link
Member

@philprime philprime commented Feb 18, 2025

This PR is derived from #4605

It introduces two methods to extend Data:

  • Data#init(contentsOfUrlWithSentryTracing:) is mapped to Data.init(contentsOfUrl:)
  • data.writeWithSentryTracing(to:options:) is mapped to data.write(to:options)

Blocked by:

Sorry, something went wrong.

Copy link
Contributor

github-actions bot commented Feb 18, 2025

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 103110e

Verified

This commit was signed with the committer’s verified signature.
renovate-bot Mend Renovate
@philprime philprime force-pushed the philprime/manual-data-tracking branch from dfce878 to 489ae5c Compare February 20, 2025 14:29
@philprime philprime changed the base branch from philprime/file-io-swizzling to main February 20, 2025 14:30
wip

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@philprime philprime marked this pull request as ready for review February 20, 2025 15:21
Copy link

codecov bot commented Feb 20, 2025

Codecov Report

Attention: Patch coverage is 95.31250% with 30 lines in your changes missing coverage. Please review.

Project coverage is 92.342%. Comparing base (c0c3eaf) to head (103110e).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ance/IO/SentryFileIOTrackingIntegrationTests.swift 77.235% 28 Missing ⚠️
...ormance/IO/DataSentryTracingIntegrationTests.swift 99.447% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4862       +/-   ##
=============================================
- Coverage   92.403%   92.342%   -0.061%     
=============================================
  Files          662       666        +4     
  Lines        77728     78300      +572     
  Branches     28142     27547      -595     
=============================================
+ Hits         71823     72304      +481     
- Misses        5807      5902       +95     
+ Partials        98        94        -4     
Files with missing lines Coverage Δ
SentryTestUtils/TestCurrentDateProvider.swift 91.666% <100.000%> (-0.171%) ⬇️
Sources/Sentry/SentryFileIOTracker.m 95.480% <100.000%> (+2.539%) ⬆️
...tegrations/Performance/IO/Data+SentryTracing.swift 100.000% <100.000%> (ø)
...formance/IO/SentryFileIOTracker+SwiftHelpers.swift 100.000% <100.000%> (ø)
...ance/IO/SentryFileIOTrackerSwiftHelpersTests.swift 100.000% <100.000%> (ø)
...ions/Performance/IO/SentryFileIOTrackerTests.swift 98.546% <ø> (ø)
...ormance/IO/DataSentryTracingIntegrationTests.swift 99.447% <99.447%> (ø)
...ance/IO/SentryFileIOTrackingIntegrationTests.swift 86.850% <77.235%> (-7.222%) ⬇️

... and 29 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 c0c3eaf...103110e. Read the comment docs.

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.

I found a few issues, but this is going to a great feature 💪

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, with one comment to improve the test assertion for the timestamp.

Copy link
Contributor

github-actions bot commented Feb 25, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1240.13 ms 1251.29 ms 11.16 ms
Size 22.30 KiB 821.37 KiB 799.07 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
f5623cd 1255.78 ms 1262.30 ms 6.52 ms
8001609 1217.87 ms 1231.31 ms 13.44 ms
39a4941 1210.69 ms 1219.83 ms 9.14 ms
e0abb7e 1235.60 ms 1253.13 ms 17.53 ms
9d61bea 1243.88 ms 1254.36 ms 10.48 ms
f4a6925 1230.82 ms 1243.94 ms 13.12 ms
1d11695 1219.57 ms 1243.52 ms 23.95 ms
a71f5e2 1245.84 ms 1258.81 ms 12.98 ms
98cca71 1210.75 ms 1240.64 ms 29.89 ms
3437454 1235.54 ms 1244.82 ms 9.28 ms

App size

Revision Plain With Sentry Diff
f5623cd 22.85 KiB 412.98 KiB 390.13 KiB
8001609 21.58 KiB 670.46 KiB 648.88 KiB
39a4941 22.84 KiB 402.63 KiB 379.79 KiB
e0abb7e 21.58 KiB 704.93 KiB 683.35 KiB
9d61bea 20.76 KiB 436.29 KiB 415.53 KiB
f4a6925 21.58 KiB 706.47 KiB 684.89 KiB
1d11695 20.76 KiB 401.60 KiB 380.84 KiB
a71f5e2 21.58 KiB 424.34 KiB 402.76 KiB
98cca71 22.85 KiB 411.14 KiB 388.29 KiB
3437454 22.85 KiB 408.88 KiB 386.03 KiB

Previous results on branch: philprime/manual-data-tracking

Startup times

Revision Plain With Sentry Diff
da2c7f6 1243.33 ms 1260.06 ms 16.73 ms

App size

Revision Plain With Sentry Diff
da2c7f6 22.31 KiB 822.71 KiB 800.41 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

@philprime philprime merged commit 3fc6a6f into main Mar 4, 2025
70 of 75 checks passed
@philprime philprime deleted the philprime/manual-data-tracking branch March 4, 2025 16:05
@github-project-automation github-project-automation bot moved this from Needs Review to Done in [DEPRECATED] Mobile SDKs Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants