Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
- Remove the Android `enablePerformanceV2` option and its `io.sentry.performance-v2.enable` manifest metadata. Performance V2 is always on ([#6458](https://github.com/getsentry/sentry-java/pull/6458))
- App starts are measured from process start on Android 7.0+ (API 24), and slow and frozen frames are measured per span
- `ActivityFramesTracker` is removed, and the `ActivityLifecycleIntegration` constructor no longer takes it
- Screenshots attached to errors and user feedback now mask all text and images by default. Turn it off with `options.screenshot.setMaskAllText(false)` and `setMaskAllImages(false)`, or the `io.sentry.screenshot.mask-all-text` and `io.sentry.screenshot.mask-all-images` manifest options. Masking needs `sentry-android-replay`, so apps that depend on `sentry-android-core` alone get no screenshot until they add it or turn masking off ([#6437](https://github.com/getsentry/sentry-java/pull/6437))

### Improvements

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -801,13 +801,12 @@ static void applyMetadata(
options.setSpotlightConnectionUrl(spotlightUrl);
}

// Screenshot masking options (default to false for backwards compatibility)
options
.getScreenshot()
.setMaskAllText(readBool(metadata, logger, SCREENSHOT_MASK_ALL_TEXT, false));
.setMaskAllText(readBool(metadata, logger, SCREENSHOT_MASK_ALL_TEXT, true));
options
.getScreenshot()
.setMaskAllImages(readBool(metadata, logger, SCREENSHOT_MASK_ALL_IMAGES, false));
.setMaskAllImages(readBool(metadata, logger, SCREENSHOT_MASK_ALL_IMAGES, true));

if (options.getAnrProfilingSampleRate() == null) {
final double anrProfilingSampleRate =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public ScreenshotEventProcessor(
if (!isReplayAvailable && !options.getScreenshot().getMaskViewClasses().isEmpty()) {
if (!isReplayModuleAbsenceLogged.getAndSet(true)) {
options
.getLogger()
.getFatalLogger()
.log(SentryLevel.WARNING, "Screenshot masking requires sentry-android-replay module");
}
return event;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
* Screenshot masking options for error screenshots. Extends the base {@link SentryMaskingOptions}
* with screenshot-specific defaults.
*
* <p>By default, masking is disabled for screenshots. Enable masking by calling {@link
* #setMaskAllText(boolean)} and/or {@link #setMaskAllImages(boolean)}.
* <p>By default, all text and images are masked. Disable masking by calling {@link
* #setMaskAllText(boolean)} and/or {@link #setMaskAllImages(boolean)} with {@code false}.
*
* <p>Note: Screenshot masking requires the {@code sentry-android-replay} module to be present at
* runtime. If masking is enabled but the replay module is not available, no screenshot is captured

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh, wait this is a deal-breaker, right? Basically whoever is not using replay now, will stop getting screenshots once they bump to v9 🙈

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah I see we mention in the changelog... On a second thought it's probably fine, that's why it goes into a new major. So customers would have to explicitly "opt-in" for screenshots without masking.

Junior estimated around 30-40 orgs that would stop receiving screenshots in v9 because of this, which is fine given we'll have the migration guide I think.

What do you think of maybe logging the warning with a options.getFatalLogger() to bypass the options.debug flag in ScreenshotUtils:89 when we found such misconfiguration (masking on, but no replay module)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will do!

Expand All @@ -16,8 +16,8 @@
public final class SentryScreenshotOptions extends SentryMaskingOptions {

public SentryScreenshotOptions() {
// Default to NO masking until next major version.
// maskViewClasses starts empty, so nothing is masked by default.
setMaskAllText(true);
setMaskAllImages(true);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public class ScreenshotUtils {
final boolean isMaskingConfigured = !options.getScreenshot().getMaskViewClasses().isEmpty();
if (isMaskingConfigured && !isReplayAvailable) {
options
.getLogger()
.getFatalLogger()
Comment thread
cursor[bot] marked this conversation as resolved.
Resolved
.log(SentryLevel.WARNING, "Screenshot masking requires sentry-android-replay module");
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2843,51 +2843,51 @@ class ManifestMetadataReaderTest {
@Test
fun `applyMetadata reads screenshot mask-all-text to options`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.SCREENSHOT_MASK_ALL_TEXT to true)
val bundle = bundleOf(ManifestMetadataReader.SCREENSHOT_MASK_ALL_TEXT to false)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertTrue(fixture.options.screenshot.maskViewClasses.contains("android.widget.TextView"))
assertFalse(fixture.options.screenshot.maskViewClasses.contains("android.widget.TextView"))
}

@Test
fun `applyMetadata reads screenshot mask-all-images to options`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.SCREENSHOT_MASK_ALL_IMAGES to true)
val bundle = bundleOf(ManifestMetadataReader.SCREENSHOT_MASK_ALL_IMAGES to false)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertTrue(fixture.options.screenshot.maskViewClasses.contains("android.widget.ImageView"))
assertFalse(fixture.options.screenshot.maskViewClasses.contains("android.widget.ImageView"))
}

@Test
fun `applyMetadata without specifying screenshot mask-all-text, stays false`() {
fun `applyMetadata without specifying screenshot mask-all-text, stays true`() {
// Arrange
val context = fixture.getContext()

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertFalse(fixture.options.screenshot.maskViewClasses.contains("android.widget.TextView"))
assertTrue(fixture.options.screenshot.maskViewClasses.contains("android.widget.TextView"))
}

@Test
fun `applyMetadata without specifying screenshot mask-all-images, stays false`() {
fun `applyMetadata without specifying screenshot mask-all-images, stays true`() {
// Arrange
val context = fixture.getContext()

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertFalse(fixture.options.screenshot.maskViewClasses.contains("android.widget.ImageView"))
assertTrue(fixture.options.screenshot.maskViewClasses.contains("android.widget.ImageView"))
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,13 @@ class ScreenshotEventProcessorTest {
private class Fixture {
lateinit var activity: Activity
val threadChecker = mock<IThreadChecker>()
val options = SentryAndroidOptions().apply { dsn = "https://key@sentry.io/proj" }
val options =
SentryAndroidOptions().apply {
dsn = "https://key@sentry.io/proj"
// Tests opt in to masking, so they also run without sentry-android-replay.
screenshot.setMaskAllText(false)
screenshot.setMaskAllImages(false)
}
val mainProcessor = MainEventProcessor(options)

init {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ import kotlin.test.assertTrue
class SentryScreenshotOptionsTest {

@Test
fun `maskViewClasses is empty by default`() {
fun `masks all text and images by default`() {
val options = SentryScreenshotOptions()
assertTrue(options.maskViewClasses.isEmpty())
assertTrue(options.maskViewClasses.contains(SentryMaskingOptions.TEXT_VIEW_CLASS_NAME))
assertTrue(options.maskViewClasses.contains(SentryMaskingOptions.IMAGE_VIEW_CLASS_NAME))
assertTrue(options.maskViewClasses.contains(SentryMaskingOptions.WEB_VIEW_CLASS_NAME))
assertEquals(8, options.maskViewClasses.size)
}

@Test
Expand All @@ -23,6 +26,8 @@ class SentryScreenshotOptionsTest {
@Test
fun `setMaskAllText true only adds TextView`() {
val options = SentryScreenshotOptions()
options.setMaskAllText(false)
options.setMaskAllImages(false)
options.setMaskAllText(true)

assertTrue(options.maskViewClasses.contains(SentryMaskingOptions.TEXT_VIEW_CLASS_NAME))
Expand All @@ -35,6 +40,7 @@ class SentryScreenshotOptionsTest {
@Test
fun `setMaskAllImages true adds ImageView and sensitive view classes`() {
val options = SentryScreenshotOptions()
options.setMaskAllImages(false)
options.setMaskAllImages(true)

assertTrue(options.maskViewClasses.contains(SentryMaskingOptions.IMAGE_VIEW_CLASS_NAME))
Expand Down Expand Up @@ -68,7 +74,7 @@ class SentryScreenshotOptionsTest {
options.setMaskAllImages(true)

// CopyOnWriteArraySet should prevent duplicates
assertEquals(7, options.maskViewClasses.size)
assertEquals(8, options.maskViewClasses.size)
}

@Test
Expand Down Expand Up @@ -108,6 +114,6 @@ class SentryScreenshotOptionsTest {

options.setMaskAllImages(false)

assertTrue(options.maskViewClasses.isEmpty())
assertEquals(setOf(SentryMaskingOptions.TEXT_VIEW_CLASS_NAME), options.maskViewClasses)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ class ScreenshotUtilsMaskedScreenshotTest {
SentryAndroidOptions().apply {
dsn = "https://key@sentry.io/proj"
threadChecker = this@Fixture.threadChecker
// Tests opt in to masking, so they also run without sentry-android-replay.
screenshot.setMaskAllText(false)
screenshot.setMaskAllImages(false)
}
val buildInfoProvider = BuildInfoProvider(options.logger)

Expand Down
Loading