-
-
Notifications
You must be signed in to change notification settings - Fork 226
Session Replay for iOS #4664
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
base: version6
Are you sure you want to change the base?
Session Replay for iOS #4664
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## version6 #4664 +/- ##
===========================================
Coverage ? 73.18%
===========================================
Files ? 480
Lines ? 17421
Branches ? 3437
===========================================
Hits ? 12749
Misses ? 3821
Partials ? 851 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| { | ||
| // For replay to work on iOS, session tracking must be enabled in the Cocoa SDKt | ||
| options.AutoSessionTracking = false; | ||
| nativeOptions.EnableAutoSessionTracking = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how we now want to handle situations like this:
sentry-dotnet/src/Sentry/SentryClient.cs
Lines 350 to 362 in 9d1b4fe
| var hasTerminalException = processedEvent.HasTerminalException(); | |
| if (hasTerminalException) | |
| { | |
| // Event contains a terminal exception -> end session as crashed | |
| _options.LogDebug("Ending session as Crashed, due to unhandled exception."); | |
| scope.SessionUpdate = _sessionManager.EndSession(SessionEndStatus.Crashed); | |
| } | |
| else if (processedEvent.HasException()) | |
| { | |
| // Event contains a non-terminal exception -> report error | |
| // (this might return null if the session has already reported errors before) | |
| scope.SessionUpdate = _sessionManager.ReportError(); | |
| } |
... or this:
sentry-dotnet/src/Sentry/Internal/Hub.cs
Lines 376 to 384 in 9d1b4fe
| // Start a new session | |
| try | |
| { | |
| var sessionUpdate = _sessionManager.StartSession(); | |
| if (sessionUpdate is not null) | |
| { | |
| CaptureSession(sessionUpdate); | |
| } | |
| } |
If the CocoaSdk owns the session, do we still need to be able to start/stop sessions from the .NET SDK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these sessions are not the same as session replay. let's not conflate these APIs
Co-authored-by: GitHub <[email protected]> Co-authored-by: James Crosswell <[email protected]>
Co-authored-by: GitHub <[email protected]>
CHANGELOG.md
Outdated
| ### Features | ||
|
|
||
| - The SDK now makes use of the new SessionEndStatus `Unhandled` when capturing an unhandled but non-terminal exception, i.e. through the UnobservedTaskExceptionIntegration ([#4633](https://github.com/getsentry/sentry-dotnet/pull/4633), [#4653](https://github.com/getsentry/sentry-dotnet/pull/4653)) | ||
| - The SDK now provides a `IsSessionActive` to allow checking the session state ([#4662](https://github.com/getsentry/sentry-dotnet/pull/4662)) | ||
| - The SDK now makes use of the new SessionEndStatus `Unhandled` when capturing an unhandled but non-terminal exception, i.e. through the UnobservedTaskExceptionIntegration ([#4633](https://github.com/getsentry/sentry-dotnet/pull/4633)) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: Intended changes?
I believe these are part in the current Unreleased section and got duplicated there through an auto-merge.
| /// to <c>false</c>. | ||
| /// </summary> | ||
| /// <remarks>Defaults to <c>true</c></remarks> | ||
| public bool EnableViewRendererV2 { get; set; } = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: are EnableViewRendererV2 and EnableFastViewRendering mutually exclusive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're separate flags in the underlying Cocoa SDK... we don't actually need to surface these in .NET though. I could just remove them unless/until we have some need to expose them.
Part of #2136
This PR adds basic support for Session Replay, including associating replays with exceptions and traces.
It does not implement any way to configure granular masks... only basic options to mask all text / images or none are provided. That can be done in a separate PR.
Note
Note that there are no new tests here as the enabling functionality was added in #4133 (those tests cover this functionality already)
Warning
We should bump the Cocoa SDK to 8.57.2 before releasing this