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

Session Replay for iOS native apps #2616

Open
bruno-garcia opened this issue Jan 16, 2023 · 18 comments · Fixed by #3625
Open

Session Replay for iOS native apps #2616

bruno-garcia opened this issue Jan 16, 2023 · 18 comments · Fixed by #3625
Labels

Comments

@bruno-garcia
Copy link
Member

bruno-garcia commented Jan 16, 2023

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
Copy link

github-actions bot commented Feb 7, 2023

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
Copy link
Member Author

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

@Dannark
Copy link

Dannark commented Dec 8, 2023

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
Copy link
Member Author

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
Copy link
Member Author

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

@vaind
Copy link
Collaborator

vaind commented Apr 24, 2024

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
Copy link
Contributor

brustolin commented Apr 24, 2024

  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
Copy link
Collaborator

vaind commented Apr 24, 2024

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
Copy link
Contributor

brustolin commented Apr 25, 2024

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
Copy link
Collaborator

vaind commented Apr 29, 2024

  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
Copy link
Contributor

@vaind This pr: #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
Copy link
Collaborator

vaind commented Apr 29, 2024

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
Copy link
Contributor

@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
Copy link
Collaborator

vaind commented Apr 29, 2024

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
Copy link
Contributor

@bruno-garcia, can we close this and keep track at getsentry/sentry#70065 ?

@vaind
Copy link
Collaborator

vaind commented Jul 3, 2024

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
Copy link
Collaborator

vaind commented Jul 3, 2024

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

@bruno-garcia
Copy link
Member Author

@bruno-garcia, can we close this and keep track at getsentry/sentry#70065 ?

That ticket tracks the Open Beta release. This has been useful to track the reactions to it as a signal of demand (outside the waitlist). We can close this after open beta but possibly GA is a better milesstone

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

Successfully merging a pull request may close this issue.

8 participants