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: Recording on Android #3552

Merged
merged 8 commits into from
Nov 4, 2024
Merged

Conversation

bruno-garcia
Copy link
Member

@bruno-garcia bruno-garcia commented Aug 23, 2024

image

Recording and redaction works fine on MAUI because it's all native under the hood.
To complete this we need to get breadcrumbs from C#, trace/replay id to the C# layer and other features.

References

Relates to:

@bruno-garcia bruno-garcia marked this pull request as ready for review October 4, 2024 15:47
@bruno-garcia
Copy link
Member Author

I'd love to test this out on Symbol Collector. How about we make a -alpha with this, so I can add it there?

it'll run on hundreds of devices.

@jamescrosswell
Copy link
Collaborator

I'd love to test this out on Symbol Collector. How about we make a -alpha with this, so I can add it there?

You mean make a release from this branch? We could definitely do that... just mark it -replayAndroid1 or something like that so it's clear on nuget.org that it's an experimental/prerelease.

Copy link
Collaborator

@vaind vaind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as an early alfa

Comment on lines 25 to 30
#if ANDROID
options.Native.ExperimentalOptions.SessionReplay.OnErrorSampleRate = 1.0;
options.Native.ExperimentalOptions.SessionReplay.SessionSampleRate = 1.0;
options.Native.ExperimentalOptions.SessionReplay.RedactAllImages = false;
options.Native.ExperimentalOptions.SessionReplay.RedactAllText = false;
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, we should probably have a facade that configures options for the current platform (Android/iOS), instead of letting the user specify them multiple times.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, we should probably have a facade that configures options for the current platform (Android/iOS), instead of letting the user specify them multiple times.

If the options had the same name for both iOS and Android (OnErrorSampleRate, SessionSampleRate etc.) then presumably SDK users could write something like this, even without any facade:

#if ANDROID || IOS
                options.Native.ExperimentalOptions.SessionReplay.OnErrorSampleRate = 1.0;
                options.Native.ExperimentalOptions.SessionReplay.SessionSampleRate = 1.0;
                options.Native.ExperimentalOptions.SessionReplay.RedactAllImages = false;
                options.Native.ExperimentalOptions.SessionReplay.RedactAllText = false;
#endif

When targeting android, options.Native would resolve to the appropriate platform specific options at build no?

@bruno-garcia
Copy link
Member Author

cc @bricefriha something we'd like to push through once the open issues are under control

@bruno-garcia
Copy link
Member Author

would love to try this too on Symbol Collector. cc @jamescrosswell

@jamescrosswell
Copy link
Collaborator

would love to try this too on Symbol Collector. cc @jamescrosswell

Is there anything blocking that? Can't we just make a pre-release from this branch? Or is it more about having someone integrated it into the Symbol Collector?

@bruno-garcia
Copy link
Member Author

bruno-garcia commented Nov 3, 2024

would love to try this too on Symbol Collector. cc @jamescrosswell

Is there anything blocking that? Can't we just make a pre-release from this branch? Or is it more about having someone integrated it into the Symbol Collector?

Other than CI being red, nothing blocking this

@bruno-garcia
Copy link
Member Author

woo it's green! THanks @jamescrosswell

can we merge this in? (it's an experimental feature so not too worried about lack of docs for example)

@jamescrosswell jamescrosswell merged commit ad80795 into main Nov 4, 2024
22 checks passed
@jamescrosswell jamescrosswell deleted the hackweek/replay-android branch November 4, 2024 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants