From 09aa74e2b3ad04d480880a1008bfdf476a423b7a Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 24 Apr 2025 11:09:00 +0200 Subject: [PATCH 1/5] refactor(session-replay): add separate method to get video info on finish writing --- .../iOS-Swift/SentrySDKWrapper.swift | 2 +- .../SessionReplay/SentryOnDemandReplay.swift | 71 +++++++++++++++---- 2 files changed, 59 insertions(+), 14 deletions(-) diff --git a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift index 6ef85ba0c30..942b07bd577 100644 --- a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift +++ b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift @@ -20,7 +20,7 @@ struct SentrySDKWrapper { if #available(iOS 16.0, *), !SentrySDKOverrides.Other.disableSessionReplay.boolValue { options.sessionReplay = SentryReplayOptions( - sessionSampleRate: 0, + sessionSampleRate: 1, onErrorSampleRate: 1, maskAllText: true, maskAllImages: true diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index d6b7cf7e35c..69d51da2821 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -212,21 +212,39 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { group.enter() videoWriter.inputs.forEach { $0.markAsFinished() } - videoWriter.finishWriting { + videoWriter.finishWriting { [weak self] in defer { group.leave() } - if videoWriter.status == .completed { + + SentryLog.debug("[Session Replay] Finished video writing, status: \(videoWriter.status)") + guard let strongSelf = self else { + SentryLog.warning("[Session Replay] On-demand replay is deallocated, completing writing session without output video info") + return + } + + switch videoWriter.status { + case .writing: + SentryLog.error("[Session Replay] Finish writing video was called with status writing, this is unexpected! Completing with no video info") + case .cancelled: + SentryLog.warning("[Session Replay] Finish writing video was cancelled, completing with no video info") + case .completed: + SentryLog.debug("[Session Replay] Finish writing video was completed, creating video info from file attributes") do { - let fileAttributes = try FileManager.default.attributesOfItem(atPath: outputFileURL.path) - guard let fileSize = fileAttributes[FileAttributeKey.size] as? Int else { - finishError = SentryOnDemandReplayError.cantReadVideoSize - return - } - guard let start = usedFrames.min(by: { $0.time < $1.time })?.time else { return } - let duration = TimeInterval(usedFrames.count / self.frameRate) - result = SentryVideoInfo(path: outputFileURL, height: Int(videoHeight), width: Int(videoWidth), duration: duration, frameCount: usedFrames.count, frameRate: self.frameRate, start: start, end: start.addingTimeInterval(duration), fileSize: fileSize, screens: usedFrames.compactMap({ $0.screenName })) + result = try strongSelf.getVideoInfo( + from: outputFileURL, + usedFrames: usedFrames, + videoWidth: Int(videoWidth), + videoHeight: Int(videoHeight) + ) } catch { + SentryLog.warning("[Session Replay] Failed to create video info from file attributes, reason: \(error.localizedDescription)") finishError = error } + case .failed, .unknown: + SentryLog.warning("[Session Replay] Finish writing video failed, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") + finishError = videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo + @unknown default: + SentryLog.warning("[Session Replay] Finish writing video failed, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") + finishError = videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo } } group.wait() @@ -237,13 +255,40 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { private func filterFrames(beginning: Date, end: Date) -> [SentryReplayFrame] { var frames = [SentryReplayFrame]() - //Using dispatch queue as sync mechanism since we need a queue already to generate the video. - workingQueue.dispatchSync({ + // Using dispatch queue as sync mechanism since we need a queue already to generate the video. + workingQueue.dispatchSync { frames = self._frames.filter { $0.time >= beginning && $0.time <= end } - }) + } return frames } + fileprivate func getVideoInfo(from outputFileURL: URL, usedFrames: [SentryReplayFrame], videoWidth: Int, videoHeight: Int) throws -> SentryVideoInfo { + SentryLog.debug("[Session Replay] Getting video info from file: \(outputFileURL.path), width: \(videoWidth), height: \(videoHeight), used frames count: \(usedFrames.count)") + let fileAttributes = try FileManager.default.attributesOfItem(atPath: outputFileURL.path) + guard let fileSize = fileAttributes[FileAttributeKey.size] as? Int else { + SentryLog.warning("[Session Replay] Failed to read video size from video file, reason: size attribute not found") + throw SentryOnDemandReplayError.cantReadVideoSize + } + let minFrame = usedFrames.min(by: { $0.time < $1.time }) + guard let start = minFrame?.time else { + SentryLog.warning("[Session Replay] Failed to read video start time from used frames, reason: no frames found") + throw SentryOnDemandReplayError.cantReadVideoStartTime + } + let duration = TimeInterval(usedFrames.count / self.frameRate) + return SentryVideoInfo( + path: outputFileURL, + height: videoHeight, + width: videoWidth, + duration: duration, + frameCount: usedFrames.count, + frameRate: self.frameRate, + start: start, + end: start.addingTimeInterval(duration), + fileSize: fileSize, + screens: usedFrames.compactMap({ $0.screenName }) + ) + } + private func createVideoSettings(width: CGFloat, height: CGFloat) -> [String: Any] { return [ AVVideoCodecKey: AVVideoCodecType.h264, From 5efded2641d66601c38d93965a035f42bc683ded Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 24 Apr 2025 11:20:56 +0200 Subject: [PATCH 2/5] fixes --- .../iOS-Swift/iOS-Swift/SentrySDKWrapper.swift | 2 +- .../SessionReplay/SentryOnDemandReplay.swift | 3 +++ .../SentryOnDemandReplayTests.swift | 17 ++++++++++++++++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift index 942b07bd577..6ef85ba0c30 100644 --- a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift +++ b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift @@ -20,7 +20,7 @@ struct SentrySDKWrapper { if #available(iOS 16.0, *), !SentrySDKOverrides.Other.disableSessionReplay.boolValue { options.sessionReplay = SentryReplayOptions( - sessionSampleRate: 1, + sessionSampleRate: 0, onErrorSampleRate: 1, maskAllText: true, maskAllImages: true diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index 69d51da2821..c32d508abd3 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -271,6 +271,9 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } let minFrame = usedFrames.min(by: { $0.time < $1.time }) guard let start = minFrame?.time else { + // Note: This code path is currently not reached, because the `getVideoInfo` method is only called after the video is successfully created, therefore at least one frame was used. + // The compiler still requires us to unwrap the optional value, and we + // do not permit force-unwrapping. SentryLog.warning("[Session Replay] Failed to read video start time from used frames, reason: no frames found") throw SentryOnDemandReplayError.cantReadVideoStartTime } diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift index 85c59e15a69..fae3979634c 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift @@ -204,6 +204,21 @@ class SentryOnDemandReplayTests: XCTestCase { XCTAssertEqual(secondVideo.width, 20) XCTAssertEqual(secondVideo.height, 10) } - + + func testGenerateVideoInfo_whenNoFramesAdded_shouldNotThrowError() throws { + // -- Arrange -- + let sut = getSut() + dateProvider.driftTimeForEveryRead = true + dateProvider.driftTimeInterval = 1 + + // -- Act -- + let videos = try sut.createVideoWith( + beginning: Date(timeIntervalSinceReferenceDate: 0), + end: Date(timeIntervalSinceReferenceDate: 10) + ) + + // -- Assert -- + XCTAssertNil(videos.first) + } } #endif From 6aa027bbde6008452662f3f51a8dc6277d0c549d Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 24 Apr 2025 11:25:20 +0200 Subject: [PATCH 3/5] update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46fc26417d1..3a9314d73b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Improvements + +- Add separate method to get video info with more logging (#5132) + ## 8.49.1 ### Fixes From 02c88579157724f8c61e0134d6648afdd8fbc009 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 24 Apr 2025 14:50:06 +0200 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Philipp Hofmann --- CHANGELOG.md | 2 +- .../Integrations/SessionReplay/SentryOnDemandReplay.swift | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a9314d73b4..08f21238f5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Improvements -- Add separate method to get video info with more logging (#5132) +- More logging for Session Replay video info (#5132) ## 8.49.1 diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index c32d508abd3..051a08e86c7 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -225,9 +225,9 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { case .writing: SentryLog.error("[Session Replay] Finish writing video was called with status writing, this is unexpected! Completing with no video info") case .cancelled: - SentryLog.warning("[Session Replay] Finish writing video was cancelled, completing with no video info") + SentryLog.warning("[Session Replay] Finish writing video was cancelled, completing with no video info.") case .completed: - SentryLog.debug("[Session Replay] Finish writing video was completed, creating video info from file attributes") + SentryLog.debug("[Session Replay] Finish writing video was completed, creating video info from file attributes.") do { result = try strongSelf.getVideoInfo( from: outputFileURL, From 0c037cb9cec5a9aafe0de60254509aab6c846e2c Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 24 Apr 2025 14:50:35 +0200 Subject: [PATCH 5/5] Update Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift --- .../Integrations/SessionReplay/SentryOnDemandReplay.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index 051a08e86c7..52e8030be9b 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -272,8 +272,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { let minFrame = usedFrames.min(by: { $0.time < $1.time }) guard let start = minFrame?.time else { // Note: This code path is currently not reached, because the `getVideoInfo` method is only called after the video is successfully created, therefore at least one frame was used. - // The compiler still requires us to unwrap the optional value, and we - // do not permit force-unwrapping. + // The compiler still requires us to unwrap the optional value, and we do not permit force-unwrapping. SentryLog.warning("[Session Replay] Failed to read video start time from used frames, reason: no frames found") throw SentryOnDemandReplayError.cantReadVideoStartTime }