-
-
Couldn't load subscription status.
- Fork 353
feat(replay): Add screenshotStrategy option for Android #5301
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,6 +37,7 @@ | |||||||||||||||||||||||||||||||||
| import io.sentry.ISerializer; | ||||||||||||||||||||||||||||||||||
| import io.sentry.Integration; | ||||||||||||||||||||||||||||||||||
| import io.sentry.ScopesAdapter; | ||||||||||||||||||||||||||||||||||
| import io.sentry.ScreenshotStrategyType; | ||||||||||||||||||||||||||||||||||
| import io.sentry.Sentry; | ||||||||||||||||||||||||||||||||||
| import io.sentry.SentryDate; | ||||||||||||||||||||||||||||||||||
| import io.sentry.SentryDateProvider; | ||||||||||||||||||||||||||||||||||
|
|
@@ -415,12 +416,36 @@ private SentryReplayOptions getReplayOptions(@NotNull ReadableMap rnOptions) { | |||||||||||||||||||||||||||||||||
| androidReplayOptions.addMaskViewClass("com.horcrux.svg.SvgView"); // react-native-svg | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (rnMobileReplayOptions.hasKey("screenshotStrategy")) { | ||||||||||||||||||||||||||||||||||
| final String screenshotStrategyString = rnMobileReplayOptions.getString("screenshotStrategy"); | ||||||||||||||||||||||||||||||||||
| final ScreenshotStrategyType screenshotStrategy = | ||||||||||||||||||||||||||||||||||
| parseScreenshotStrategy(screenshotStrategyString); | ||||||||||||||||||||||||||||||||||
| androidReplayOptions.setScreenshotStrategy(screenshotStrategy); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| androidReplayOptions.setMaskViewContainerClass(RNSentryReplayMask.class.getName()); | ||||||||||||||||||||||||||||||||||
| androidReplayOptions.setUnmaskViewContainerClass(RNSentryReplayUnmask.class.getName()); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return androidReplayOptions; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| private ScreenshotStrategyType parseScreenshotStrategy(@Nullable String strategyString) { | ||||||||||||||||||||||||||||||||||
| if (strategyString == null) { | ||||||||||||||||||||||||||||||||||
| return ScreenshotStrategyType.PIXEL_COPY; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. q: why the try/catch block here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just followed the pattern for quality, but can remove it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't recall why I added the try/catch block for quality 😓 |
||||||||||||||||||||||||||||||||||
| switch (strategyString.toLowerCase(Locale.ROOT)) { | ||||||||||||||||||||||||||||||||||
| case "canvas": | ||||||||||||||||||||||||||||||||||
| return ScreenshotStrategyType.CANVAS; | ||||||||||||||||||||||||||||||||||
| default: | ||||||||||||||||||||||||||||||||||
| return ScreenshotStrategyType.PIXEL_COPY; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||||||||||||||||
| return ScreenshotStrategyType.PIXEL_COPY; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+437
to
+446
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Following up on https://github.com/getsentry/sentry-react-native/pull/5301/files#r2461084612
Suggested change
|
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| private SentryReplayQuality parseReplayQuality(@Nullable String qualityString) { | ||||||||||||||||||||||||||||||||||
| if (qualityString == null) { | ||||||||||||||||||||||||||||||||||
| return SentryReplayQuality.MEDIUM; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,14 @@ import { enrichXhrBreadcrumbsForMobileReplay } from './xhrUtils'; | |
|
|
||
| export const MOBILE_REPLAY_INTEGRATION_NAME = 'MobileReplay'; | ||
|
|
||
| /** | ||
| * Screenshot strategy type for Android Session Replay. | ||
| * | ||
| * - `'canvas'`: Canvas-based screenshot strategy. This strategy does **not** support any masking options, it always masks text and images. Use this if your application has strict PII requirements. | ||
| * - `'pixelCopy'`: Pixel copy screenshot strategy (default). Supports all masking options. | ||
| */ | ||
| export type ScreenshotStrategy = 'canvas' | 'pixelCopy'; | ||
|
|
||
| export interface MobileReplayOptions { | ||
| /** | ||
| * Mask all text in recordings | ||
|
|
@@ -69,6 +77,19 @@ export interface MobileReplayOptions { | |
| * @default false | ||
| */ | ||
| enableFastViewRendering?: boolean; | ||
|
|
||
| /** | ||
| * Sets the screenshot strategy used by the Session Replay integration on Android. | ||
| * | ||
| * If your application has strict PII requirements we recommend using `'canvas'`. | ||
| * This strategy does **not** support any masking options, it always masks text and images. | ||
| * | ||
| * - Experiment: In case you are noticing issues with the canvas screenshot strategy, please report the issue on [GitHub](https://github.com/getsentry/sentry-java). | ||
| * | ||
| * @default 'pixelCopy' | ||
| * @platform android | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's a good idea 👍 |
||
| */ | ||
| screenshotStrategy?: ScreenshotStrategy; | ||
| } | ||
|
|
||
| const defaultOptions: Required<MobileReplayOptions> = { | ||
|
|
@@ -78,6 +99,7 @@ const defaultOptions: Required<MobileReplayOptions> = { | |
| enableExperimentalViewRenderer: false, | ||
| enableViewRendererV2: true, | ||
| enableFastViewRendering: false, | ||
| screenshotStrategy: 'pixelCopy', | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. might be worth on a major change to default it to canvas if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah we'll monitor the adoption and possibly make it a default in the future |
||
| }; | ||
|
|
||
| function mergeOptions(initOptions: Partial<MobileReplayOptions>): Required<MobileReplayOptions> { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.