search

getsentry / sentry-cocoa

The official Sentry SDK for iOS, tvOS, macOS, watchOS.
https://sentry.io/for/cocoa/
MIT License
779 stars 311 forks source link

Session Replay for iOS native apps #2616

Open bruno-garcia opened 1 year ago

bruno-garcia commented 1 year ago

Support for Session Replay for Native iOS apps

Our goal is to support UIKit and SwiftUI. That means that apps rendering directly with Metal for example would not be supported.

Please let us know on the issue description if this aligns with what you need/expect.

We're working on it! Wanna join the early adopter release? Join the waitlist and discussion about the feature:

┆Issue is synchronized with this Jira Improvement by Unito

github-actions[bot] commented 1 year ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

bruno-garcia commented 7 months ago

We're doing a PoC of this as we speak. We should have some updates after the holidays

Dannark commented 7 months ago

Great work guys! Can't wait to see this feature on iOS. Your team's work is always top-notch. Even if there are any challenges along the way, it's great to witness these PoCs in progress. Keep up the excellent work! 👏🏼🚀

bruno-garcia commented 2 months ago

We released our first Alpha version of the SDK with support: https://github.com/getsentry/sentry-cocoa/releases/tag/8.24.0-alpha.0

To get access, it requires adding your Sentry org to our feature flag. This way data can be ingested and displayed in Sentry. Please let us know on the waitlist if you're interested

bruno-garcia commented 2 months ago

Reopening because we had a first alpha but we'll keep this open until it's more generally available.

vaind commented 2 months ago

I've given this a try to integrate into Flutter and I'm facing a couple of roadblocks so far:

  1. The APIs are currently private (SentrySessionReplayIntegration and/or SentrySessionReplay and its dependencies, e.g. SentryOnDemandReplay, ...).
  2. SentryOnDemandReplay would need a func addFrame(imagePath: NSString)
  3. The screenshot collection logic should be refactored so that we can flip the call flow. Currently, SentrySessionReplay requests a screenshot from a SentryViewScreenshotProvider. Instead, we'd need something like in the Java integration, where the ScreenshotRecorder is responsible for running its own loop & posts screenshots back to the integration via a method. This means that there's a custom implementation of the recorder in Flutter and this collects screenshots and posts them back to the native SentryReplay as file paths of already saved images (currently, flutter saves a resized PNG, which seems to be the format cocoa uses as well.)
  4. AFAIU, the DisplayLinkWrapper is also specific to native and would be also refactored into the recorder, but maybe I just don't understand what it's doing.

Unfortunately, I don't feel confident enough with the codebase to make a PR with the changes so I hope someone could pick this up?

brustolin commented 2 months ago
  1. We can't make it public but we can expose it through headers inside flutter project.
  2. Well, it seems to me that we would just move the the ownership of who decides when a new frame needs to be taken. I believe we can keep the current approach, we just need to implement a ScreenshotRecorder in the flutter side, that will be notified when a new screenshot needs to be provided.
  3. DisplayLink is related to how UI is renderer in iOS and its use in here to measure time should not be a problem.
vaind commented 2 months ago

will be notified when a new screenshot needs to be provided.

I'm not sure that's going to work well. In order to capture a screenshot in Flutter on demand, we would need to schedule a callback to happen after a next frame and then wait for that (async) callback to finish. Because cocoa screenshot provider is a synchronous function that's supposed to return a value (image) it would need to block, right? If it's the only way we can use in the cocoa and you're opposed to changing to the way the java SDK does this than OK, let's try that, it just doesn't feel right.

brustolin commented 2 months ago

Because cocoa screenshot provider is a synchronous function that's supposed to return a value (image) it would need to block, right?

Not anymore, this is being updated in my ongoing PR.

it's the only way we can use in the cocoa and you're opposed to changing to the way the java SDK does this than OK

It's not that I oppose to do it, it's a valid solution as the current one. I just dont think it's necessary to do it. We can change iOS for a new approach and then add Flutter support, or just add support to Flutter with the current approach. Unless the current approach is not suitable for Flutter, then lets change it.

vaind commented 2 months ago
  1. We can't make it public but we can expose it through headers inside flutter project.

Hmm, how does that work? Is it different to what is done with the rest of the functionality in Sources/Sentry/include/HybridPublic? Any chance you could make the changes?

Not anymore, this is being updated in my ongoing PR.

I haven't found the PR so please let me know when that is available on the main branch.

brustolin commented 2 months ago

@vaind This pr: https://github.com/getsentry/sentry-cocoa/pull/3877

Hmm, how does that work?

We create a .h file with the interface of the class.

But Im starting to believe that Flutter should have its own implementation of session replay since it has its own renderer. This way we wont have any dependencies in both native platforms and solve this with a dart only solution.

vaind commented 2 months ago

But Im starting to believe that Flutter should have its own implementation of session replay since it has its own renderer. This way we wont have any dependencies in both native platforms and solve this with a dart only solution.

I've considered that too, but with video encoding happening in native, it seemed to me that a mixed approach is a good way forward. With the java SDK, this works quite well so from my POV I don't see why this couldn't work with Cocoa as well.

brustolin commented 2 months ago

@vaind Sorry, I wont be able to help you with code anytime soon, Im focus in the native and RN side right now. But, the easy way to make SentrySessionReplay available for Swift is to move SentrySessionReplay.h to include/HybridSDKs/, you can open a PR to make this change if you think this is the path you want to take.

vaind commented 2 months ago

Thanks, I'm looking into Android at the moment so it's fine. I'll let you know if anything changes and I end up moving more stuff to Flutter, as you've suggested, if it would make it easier to achieve the overall goal (across both native SDKs).

brustolin commented 3 weeks ago

@bruno-garcia, can we close this and keep track at https://github.com/getsentry/sentry/issues/70065 ?

vaind commented 2 days ago

During development of replay for Flutter, I'm often getting [Sentry] [error] [SentrySessionReplay:282] Could not create replay video - Error Domain=AVFoundationErrorDomain Code=-11823 "Cannot Save" UserInfo={NSLocalizedRecoverySuggestion=The requested file name is already in use. Try a different file name or location., NSLocalizedDescription=Cannot Save, NSUnderlyingError=0x600000f11bc0 {Error Domain=NSOSStatusErrorDomain Code=-12412 "(null)"}}

AFAICT it happens when I build the app via Flutter and later stop the flutter command in order to start the debug session in Xcode.

vaind commented 2 days ago

Also, I get the same error repeatedly (every second) once the replay is sent due to an error. That is with both error replays and session replays at 1.0