feat(replay): Add ReplayFrameObserver for snapshot testing #5386
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,4 +32,5 @@ artifacts: | |
| when: always | ||
| match: | ||
| - junit.xml | ||
| - "*.png" | ||
| directory: ./artifacts/ | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| package io.sentry.uitest.android | ||
|
|
||
| import android.graphics.Bitmap | ||
| import android.os.Environment | ||
| import androidx.lifecycle.Lifecycle | ||
| import androidx.test.core.app.launchActivity | ||
| import io.sentry.SentryReplayOptions | ||
| import io.sentry.TypeCheckHint | ||
| import java.io.File | ||
| import java.util.concurrent.CopyOnWriteArraySet | ||
| import java.util.concurrent.CountDownLatch | ||
| import java.util.concurrent.TimeUnit | ||
| import kotlin.test.Test | ||
| import kotlin.test.assertTrue | ||
| import org.hamcrest.CoreMatchers.`is` | ||
| import org.junit.Assume.assumeThat | ||
| import org.junit.Before | ||
|
|
||
| class ReplaySnapshotTest : BaseUiTest() { | ||
|
|
||
| @Before | ||
| fun setup() { | ||
| // GH Actions emulators don't support capturing screenshots for replay | ||
| @Suppress("KotlinConstantConditions") | ||
| assumeThat(BuildConfig.ENVIRONMENT != "github", `is`(true)) | ||
| } | ||
|
|
||
| @Test | ||
| fun captureComposeReplayFrameSnapshots() { | ||
|
Contributor
Author
There was a problem hiding this comment. just FYI, this is a pretty contrived test but it just makes sure we can capture a replay using the new snapshot observer api |
||
| val snapshotsDir = | ||
| File( | ||
| Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS), | ||
| "sauce_labs_custom_screenshots", | ||
| ) | ||
| .apply { | ||
| deleteRecursively() | ||
| mkdirs() | ||
| } | ||
| val frameReceived = CountDownLatch(1) | ||
| val capturedScreens = CopyOnWriteArraySet<String>() | ||
|
|
||
| val activityScenario = launchActivity<ComposeActivity>() | ||
| activityScenario.moveToState(Lifecycle.State.RESUMED) | ||
|
|
||
| initSentry { | ||
| it.sessionReplay.sessionSampleRate = 1.0 | ||
| it.sessionReplay.frameObserver = | ||
| SentryReplayOptions.ReplayFrameObserver { hint, frameTimestamp, screenName -> | ||
| val bitmap = | ||
| hint.getAs(TypeCheckHint.REPLAY_FRAME_BITMAP, Bitmap::class.java) | ||
| ?: return@ReplayFrameObserver | ||
| val name = screenName ?: "unknown" | ||
| if (capturedScreens.add(name)) { | ||
| val file = File(snapshotsDir, "${name}_$frameTimestamp.png") | ||
| file.outputStream().use { out -> bitmap.compress(Bitmap.CompressFormat.PNG, 100, out) } | ||
| } | ||
| bitmap.recycle() | ||
| frameReceived.countDown() | ||
| } | ||
| } | ||
|
|
||
| assertTrue(frameReceived.await(10, TimeUnit.SECONDS), "Expected at least one replay frame") | ||
| assertTrue(capturedScreens.isNotEmpty(), "Expected at least one screen captured") | ||
|
|
||
| val files = snapshotsDir.listFiles()?.filter { it.extension == "png" } ?: emptyList() | ||
| assertTrue(files.isNotEmpty(), "Expected snapshot PNG files on disk") | ||
| assertTrue(files.all { it.length() > 0 }, "Snapshot files should not be empty") | ||
|
|
||
| activityScenario.moveToState(Lifecycle.State.DESTROYED) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,13 @@ package io.sentry.android.replay | |
|
|
||
| import android.content.Context | ||
| import android.graphics.Bitmap | ||
| import android.graphics.BitmapFactory | ||
| import android.os.Build | ||
| import android.view.MotionEvent | ||
| import io.sentry.Breadcrumb | ||
| import io.sentry.DataCategory.All | ||
| import io.sentry.DataCategory.Replay | ||
| import io.sentry.Hint | ||
| import io.sentry.IConnectionStatusProvider.ConnectionStatus | ||
| import io.sentry.IConnectionStatusProvider.ConnectionStatus.DISCONNECTED | ||
| import io.sentry.IConnectionStatusProvider.IConnectionStatusObserver | ||
|
|
@@ -17,8 +19,10 @@ import io.sentry.ReplayBreadcrumbConverter | |
| import io.sentry.ReplayController | ||
| import io.sentry.SentryIntegrationPackageStorage | ||
| import io.sentry.SentryLevel.DEBUG | ||
| import io.sentry.SentryLevel.ERROR | ||
| import io.sentry.SentryLevel.INFO | ||
| import io.sentry.SentryOptions | ||
| import io.sentry.TypeCheckHint | ||
| import io.sentry.android.replay.ReplayState.CLOSED | ||
| import io.sentry.android.replay.ReplayState.PAUSED | ||
| import io.sentry.android.replay.ReplayState.RESUMED | ||
|
|
@@ -308,13 +312,45 @@ public class ReplayIntegration( | |
| var screen: String? = null | ||
| scopes?.configureScope { screen = it.screen?.substringAfterLast('.') } | ||
| captureStrategy?.onScreenshotRecorded(bitmap) { frameTimeStamp -> | ||
| val observer = options.sessionReplay.frameObserver | ||
| if (observer != null) { | ||
| val copy = bitmap.copy(bitmap.config!!, false) | ||
|
cursor[bot] marked this conversation as resolved.
runningcode marked this conversation as resolved.
runningcode marked this conversation as resolved.
Member
There was a problem hiding this comment. Newbie question, but are we able to observe the performance hit of copying on our end? Esp if we want to expand our use-cases beyond testing or debugging at some point, it'd be good to know whether we need to optimize.
Contributor
Author
There was a problem hiding this comment. Memory-wise, each bitmap is roughly 900kb and we produce one frame per second.
Member
There was a problem hiding this comment. 900kb sounds like a lot, last time check it was way less 😅 I guess it depends on complexity of app's UI as well as the device used. We'll probably have to look into this as part of SDK Perf overhead initiative, and we even have an issue for that: #4154. I'd imagine we could use webp without losing anything, but that's for later |
||
| if (copy != null) { | ||
|
runningcode marked this conversation as resolved.
|
||
| try { | ||
| val hint = Hint() | ||
| hint.set(TypeCheckHint.REPLAY_FRAME_BITMAP, copy) | ||
| observer.onMaskedFrameCaptured(hint, frameTimeStamp, screen) | ||
| } catch (e: Throwable) { | ||
| options.logger.log(ERROR, "Error in ReplayFrameObserver", e) | ||
| copy.recycle() | ||
|
runningcode marked this conversation as resolved.
|
||
| } | ||
| } | ||
| } | ||
| addFrame(bitmap, frameTimeStamp, screen) | ||
| } | ||
|
Member
There was a problem hiding this comment. ❗- It's not in the diff, so apologies for missing it the first time round, but I see there's an
Member
There was a problem hiding this comment. yep, that one is used by Flutter exclusively, but would be good to align here and also call the observer 👍
Contributor
Author
There was a problem hiding this comment. Ok added below. Note, we read the file and decode it as a bitmap. |
||
| checkCanRecord() | ||
| } | ||
|
|
||
| override fun onScreenshotRecorded(screenshot: File, frameTimestamp: Long) { | ||
| var screen: String? = null | ||
| scopes?.configureScope { screen = it.screen?.substringAfterLast('.') } | ||
| captureStrategy?.onScreenshotRecorded { _ -> | ||
| val observer = options.sessionReplay.frameObserver | ||
| if (observer != null) { | ||
| val bitmap = BitmapFactory.decodeFile(screenshot.absolutePath) | ||
| if (bitmap != null) { | ||
| try { | ||
| val hint = Hint() | ||
| hint.set(TypeCheckHint.REPLAY_FRAME_BITMAP, bitmap) | ||
| observer.onMaskedFrameCaptured(hint, frameTimestamp, screen) | ||
| } catch (e: Throwable) { | ||
| options.logger.log(ERROR, "Error in ReplayFrameObserver", e) | ||
| bitmap.recycle() | ||
| } | ||
| } | ||
| } | ||
| addFrame(screenshot, frameTimestamp, screen) | ||
| } | ||
| checkCanRecord() | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 copied this from
ReplayTestbut why are we even running these on emulators in gh actions?Uh oh!
There was an error while loading. Please reload this page.
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 think the purpose of this tests running in gh actions is split in two parts:
sentry-android-corewithout integrations to catch issues like CIRCULAR REFERENCE: com.android.tools.r8.utils.b: Missing class io.sentry.compose.viewhierarchy.ComposeViewHierarchyExporter #2738 earlyThat said, we probably don't even need to run the app, just compiling it would be sufficient to catch these two potential issues. Actually running the tests was a nice addition to also verify the SDK behaviour at runtime, considering the two things above (e.g. if R8 strips out some code over-aggressively it'd crash at runtime)