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

philprime · 2025-02-18T10:46:18Z

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:

github-actions · 2025-02-18T10:46:53Z

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

Generated by 🚫 dangerJS against 391c6d8

github-actions · 2025-02-18T14:33:05Z

🚨 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

github-actions · 2025-02-20T10:19:50Z

🚨 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

github-actions · 2025-02-20T14:04:53Z

🚨 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

github-actions · 2025-02-20T14:10:19Z

🚨 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

Sources/Sentry/SentryFileIOTrackingIntegration.m

codecov · 2025-02-20T15:28:36Z

Codecov Report

Attention: Patch coverage is 96.63512% with 32 lines in your changes missing coverage. Please review.

Project coverage is 92.338%. Comparing base (25f4e43) to head (391c6d8).

Files with missing lines	Patch %	Lines
...formance/IO/SentryFileIOTracker+SwiftHelpers.swift	72.649%	32 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #4863       +/-   ##
=============================================
+ Coverage   92.175%   92.338%   +0.163%     
=============================================
  Files          659       662        +3     
  Lines        77491     78435      +944     
  Branches     27260     28458     +1198     
=============================================
+ Hits         71428     72426      +998     
+ Misses        5968      5913       -55     
- Partials        95        96        +1

Files with missing lines	Coverage Δ
Sources/Sentry/SentryFileIOTracker.m	`96.470% <100.000%> (+2.458%)`	⬆️
...ons/Performance/IO/FileManager+SentryTracing.swift	`100.000% <100.000%> (ø)`
...rmance/IO/FileManagerTracingIntegrationTests.swift	`100.000% <100.000%> (ø)`
...formance/IO/SentryFileIOTracker+SwiftHelpers.swift	`72.649% <72.649%> (ø)`

... and 24 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 25f4e43...391c6d8. Read the comment docs.

…-manager-tracking

philipphofmann

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

philipphofmann · 2025-02-21T15:42:26Z

Tests/SentryTests/Integrations/Performance/IO/FileManagerTracingIntegrationTests.swift

+            let tempDir = URL(fileURLWithPath: NSTemporaryDirectory())
+                .appendingPathComponent("test-\(testName.hashValue.description)")
+            try! FileManager.default
+                .createDirectory(at: tempDir, withIntermediateDirectories: true)


m: Same as #4862 (comment).

philipphofmann · 2025-02-21T15:42:36Z

Tests/SentryTests/Integrations/Performance/IO/FileManagerTracingIntegrationTests.swift

+    private class Fixture {
+
+        let data = "SOME DATA".data(using: .utf8)!


m: Same as #4862 (comment).

philipphofmann · 2025-02-21T15:45:06Z

Sources/Swift/Integrations/Performance/IO/FileManager+SentryTracing.swift

+                contents: data,
+                attributes: attr,
+                origin: SentryTraceOrigin.manualFileData) { path, data, attr in
+                    self.createFile(atPath: path, contents: data, attributes: attr)


m: It would be great to have a test that fails when doing this because then we could seriously break users.

Suggested change

self.createFile(atPath: path, contents: data, attributes: attr)

self.createFile(atPath: path, contents: data, attributes: nil)

philipphofmann · 2025-02-21T15:47:54Z

Tests/SentryTests/Integrations/Performance/IO/FileManagerTracingIntegrationTests.swift

+        XCTAssertEqual(span.data["file.path"] as? String, fixture.filePathToCreate)
+        XCTAssertNil(span.data["file.size"])


m: Same as #4862 (comment)

philipphofmann · 2025-02-21T15:49:22Z

Sources/Swift/Integrations/Performance/IO/SentryFileIOTracker+SwiftHelpers.swift

+        guard let span = self.span(forPath: url.path, origin: origin, operation: SentrySpanOperation.fileRead) else {
+            return try method(url, options)
+        }
+        defer {
+            span.finish()
+        }


m: Same as #4862 (comment) and this applies to all methods here.

philipphofmann · 2025-02-21T15:53:49Z

Sources/Swift/Integrations/Performance/IO/FileManager+SentryTracing.swift

+    ///   - dstPath: The path at which to place the copy of `srcPath`. 
+    ///              This path must include the name of the file or directory in its new location. 
+    /// - Note: See ``FileManager.copyItem(atPath:toPath:)`` for more information.
+    func copyItemWithSentryTracing(at srcPath: String, to dstPath: String) throws {


h: This doesn't fully align with the naming of the FileManager API, if I'm not mistaken.

Suggested change

func copyItemWithSentryTracing(at srcPath: String, to dstPath: String) throws {

func copyItemWithSentryTracing(atPath srcPath: String, toPath dstPath: String) throws {

philipphofmann · 2025-02-21T15:54:57Z

Sources/Swift/Integrations/Performance/IO/FileManager+SentryTracing.swift

+    ///   - dstPath: The new path for the item in `srcPath`. 
+    ///              This path must include the name of the file or directory in its new location.
+    /// - Note: See ``FileManager.moveItem(atPath:toPath:)`` for more information.
+    func moveItemWithSentryTracing(at srcPath: String, to dstPath: String) throws {


h: Again please align with the FileManager naming

Suggested change

func moveItemWithSentryTracing(at srcPath: String, to dstPath: String) throws {

func moveItemWithSentryTracing(atPath srcPath: String, toPath dstPath: String) throws {

philipphofmann · 2025-02-21T15:57:45Z

Tests/SentryTests/Integrations/Performance/IO/FileManagerTracingIntegrationTests.swift

+import SentryTestUtils
+import XCTest
+
+class FileManagerSentryTracingIntegrationTests: XCTestCase {


h: These tests are quite extensive. It would be great to add some more to validate that the FileManager operations work correctly when the SDK isn't started.

philprime self-assigned this Feb 18, 2025

philprime mentioned this pull request Feb 18, 2025

feat: Add manual file IO tracking for Swift.Data and Swift.FileManager #4605

Open

10 tasks

philprime force-pushed the philprime/manual-file-manager-tracking branch from 0db92ad to 4182d91 Compare February 18, 2025 14:32

This was referenced Feb 19, 2025

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

Merged

feat: Add experimental flag to disable swizzling of NSData #4859

Merged

philprime added the Platform: Cocoa label Feb 19, 2025

philprime force-pushed the philprime/manual-file-manager-tracking branch from d10e11b to d3e66be Compare February 20, 2025 10:19

philprime changed the base branch from main to philprime/file-io-swizzling February 20, 2025 14:10

squash rebase changes

95d1308

philprime force-pushed the philprime/manual-file-manager-tracking branch from 9bb9c45 to 95d1308 Compare February 20, 2025 14:25

philprime changed the base branch from philprime/file-io-swizzling to main February 20, 2025 14:25

philprime added 3 commits February 20, 2025 15:30

Update CHANGELOG.md

a0c1fa7

wip

298ab52

wip

3f157d5

philprime commented Feb 20, 2025

View reviewed changes

Sources/Sentry/SentryFileIOTrackingIntegration.m Outdated Show resolved Hide resolved

philprime marked this pull request as ready for review February 20, 2025 15:21

philprime requested review from philipphofmann and armcknight as code owners February 20, 2025 15:21

philprime added 2 commits February 21, 2025 14:09

Merge remote-tracking branch 'origin/main' into philprime/manual-file…

fc972a2

…-manager-tracking

update changelog

391c6d8

philipphofmann mentioned this pull request Feb 21, 2025

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

Open

2 tasks

philipphofmann reviewed Feb 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

philprime commented Feb 18, 2025 •

edited

Loading

github-actions bot commented Feb 18, 2025 •

edited

Loading

github-actions bot commented Feb 18, 2025

github-actions bot commented Feb 20, 2025

github-actions bot commented Feb 20, 2025

github-actions bot commented Feb 20, 2025

codecov bot commented Feb 20, 2025 •

edited

Loading

philipphofmann left a comment

philipphofmann Feb 21, 2025

philipphofmann Feb 21, 2025

philipphofmann Feb 21, 2025

philipphofmann Feb 21, 2025

philipphofmann Feb 21, 2025

philipphofmann Feb 21, 2025

philipphofmann Feb 21, 2025

philipphofmann Feb 21, 2025

		private class Fixture {

		let data = "SOME DATA".data(using: .utf8)!

	self.createFile(atPath: path, contents: data, attributes: attr)
	self.createFile(atPath: path, contents: data, attributes: nil)

		XCTAssertEqual(span.data["file.path"] as? String, fixture.filePathToCreate)
		XCTAssertNil(span.data["file.size"])

	func copyItemWithSentryTracing(at srcPath: String, to dstPath: String) throws {
	func copyItemWithSentryTracing(atPath srcPath: String, toPath dstPath: String) throws {

	func moveItemWithSentryTracing(at srcPath: String, to dstPath: String) throws {
	func moveItemWithSentryTracing(atPath srcPath: String, toPath dstPath: String) throws {

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

Are you sure you want to change the base?

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

Conversation

philprime commented Feb 18, 2025 • edited Loading

github-actions bot commented Feb 18, 2025 • edited Loading

github-actions bot commented Feb 18, 2025

🚨 Detected changes in high risk code 🚨

github-actions bot commented Feb 20, 2025

🚨 Detected changes in high risk code 🚨

github-actions bot commented Feb 20, 2025

🚨 Detected changes in high risk code 🚨

github-actions bot commented Feb 20, 2025

🚨 Detected changes in high risk code 🚨

codecov bot commented Feb 20, 2025 • edited Loading

Codecov Report

philipphofmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philprime commented Feb 18, 2025 •

edited

Loading

github-actions bot commented Feb 18, 2025 •

edited

Loading

codecov bot commented Feb 20, 2025 •

edited

Loading