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

feat: Add extension for FileManager to track file I/O operations with Sentry #4863

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

philprime
Copy link
Contributor

@philprime philprime commented Feb 18, 2025

This PR is derived from #4605

It introduces an extension to FileManager with these methods:

  • createFileWithSentryTracing(atPath:contents:attributes:) mapped to createFile(atPath:contents:attributes:)
  • removeItemWithSentryTracing(at:) mapped to removeItem(at:)
  • removeItemWithSentryTracing(atPath:) mapped to removeItem(atPath:)
  • copyItemWithSentryTracing(at:to:) mapped to copyItem(at:to:)
  • copyItemWithSentryTracing(atPath:toPath:) mapped to copyItem(atPath:toPath)
  • moveItemWithSentryTracing(at:to:) mapped to moveItem(at:to:)
  • moveItemWithSentryTracing(atPath:toPath:) mapped to moveItem(atPath:toPath:)

Blocked by:

@philprime philprime self-assigned this Feb 18, 2025
Copy link

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 552405c

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

🚨 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

2 similar comments
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

🚨 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

@philprime philprime changed the base branch from main to philprime/file-io-swizzling February 20, 2025 14:10
@philprime philprime force-pushed the philprime/manual-file-manager-tracking branch from 9bb9c45 to 95d1308 Compare February 20, 2025 14:25
@philprime philprime changed the base branch from philprime/file-io-swizzling to main February 20, 2025 14:25
@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 99.50372% with 8 lines in your changes missing coverage. Please review.

Project coverage is 92.574%. Comparing base (bf809be) to head (552405c).

Files with missing lines Patch % Lines
...rmance/IO/FileManagerTracingIntegrationTests.swift 99.642% 5 Missing ⚠️
...ormance/IO/DataSentryTracingIntegrationTests.swift 95.161% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4863       +/-   ##
=============================================
+ Coverage   92.431%   92.574%   +0.142%     
=============================================
  Files          665       667        +2     
  Lines        78190     79788     +1598     
  Branches     28260     29019      +759     
=============================================
+ Hits         72272     73863     +1591     
- Misses        5819      5829       +10     
+ Partials        99        96        -3     
Files with missing lines Coverage Δ
...ons/Performance/IO/FileManager+SentryTracing.swift 100.000% <100.000%> (ø)
...formance/IO/SentryFileIOTracker+SwiftHelpers.swift 100.000% <100.000%> (ø)
...ormance/IO/DataSentryTracingIntegrationTests.swift 98.792% <95.161%> (-0.656%) ⬇️
...rmance/IO/FileManagerTracingIntegrationTests.swift 99.642% <99.642%> (ø)

... and 18 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 bf809be...552405c. Read the comment docs.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

This looks great; with a few improvements, this will be an excellent feature.

@philprime philprime marked this pull request as draft March 6, 2025 11:28
Copy link

github-actions bot commented Mar 6, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1224.06 ms 1236.18 ms 12.12 ms
Size 22.30 KiB 824.52 KiB 802.21 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
bec7914 1214.02 ms 1236.20 ms 22.18 ms
d914696 1225.19 ms 1247.16 ms 21.98 ms
371db89 1226.40 ms 1251.54 ms 25.14 ms
e89dc54 1207.86 ms 1218.27 ms 10.41 ms
a0cc9d6 1228.98 ms 1252.36 ms 23.38 ms
98cca71 1226.66 ms 1242.94 ms 16.28 ms
c5fef16 1221.71 ms 1241.49 ms 19.78 ms
df9fb5b 1239.18 ms 1254.00 ms 14.82 ms
46467d0 1229.00 ms 1241.27 ms 12.27 ms
c810e58 1221.57 ms 1250.45 ms 28.88 ms

App size

Revision Plain With Sentry Diff
bec7914 21.58 KiB 707.43 KiB 685.85 KiB
d914696 21.58 KiB 629.83 KiB 608.25 KiB
371db89 20.76 KiB 427.31 KiB 406.55 KiB
e89dc54 22.85 KiB 412.60 KiB 389.75 KiB
a0cc9d6 21.58 KiB 706.47 KiB 684.89 KiB
98cca71 22.85 KiB 411.14 KiB 388.29 KiB
c5fef16 22.31 KiB 760.65 KiB 738.34 KiB
df9fb5b 21.90 KiB 708.90 KiB 687.00 KiB
46467d0 21.58 KiB 698.11 KiB 676.53 KiB
c810e58 22.32 KiB 761.10 KiB 738.78 KiB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants