Skip to content

fix(session-replay): add masking improvements #5073

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

Closed
wants to merge 6 commits into from

Conversation

philprime
Copy link
Contributor

📜 Description

This is a WIP PR to review analysis attempts and improvements for session replay masking

💡 Motivation and Context

See #4864

#skip-changelog

Copy link

codecov bot commented Apr 9, 2025

Codecov Report

Attention: Patch coverage is 0% with 186 lines in your changes missing coverage. Please review.

Project coverage is 9.128%. Comparing base (f96ab19) to head (18efa31).

Files with missing lines Patch % Lines
...es/Swift/Core/Tools/ViewCapture/RedactRegion.swift 0.000% 51 Missing ⚠️
...Swift/Core/Tools/ViewCapture/UIRedactBuilder.swift 0.000% 39 Missing ⚠️
...ift/Core/Tools/ViewCapture/ViewHierarchyNode.swift 0.000% 35 Missing ⚠️
...tegrations/SessionReplay/SentrySessionReplay.swift 0.000% 20 Missing ⚠️
...egrations/SessionReplay/SentryOnDemandReplay.swift 0.000% 17 Missing ⚠️
...rations/SessionReplay/SentryReplayVideoMaker.swift 0.000% 9 Missing ⚠️
Sources/Sentry/SentrySessionReplayIntegration.m 0.000% 7 Missing ⚠️
.../Tools/ViewCapture/SentryDefaultMaskRenderer.swift 0.000% 3 Missing ⚠️
...ore/Tools/ViewCapture/SentryViewPhotographer.swift 0.000% 3 Missing ⚠️
.../Core/Tools/ViewCapture/SentryMaskRendererV2.swift 0.000% 1 Missing ⚠️
... and 1 more

❗ There is a different number of reports uploaded between BASE (f96ab19) and HEAD (18efa31). Click for more details.

HEAD has 3 uploads less than BASE
Flag BASE (f96ab19) HEAD (18efa31)
4 1
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main    #5073        +/-   ##
=============================================
- Coverage   92.903%   9.128%   -83.776%     
=============================================
  Files          691      364       -327     
  Lines        86707    26665     -60042     
  Branches     31235      121     -31114     
=============================================
- Hits         80554     2434     -78120     
- Misses        6054    24231     +18177     
+ Partials        99        0        -99     
Files with missing lines Coverage Δ
...tegrations/SessionReplay/SentryReplayOptions.swift 31.250% <ø> (-68.750%) ⬇️
.../Core/Tools/ViewCapture/SentryMaskRendererV2.swift 0.000% <0.000%> (-100.000%) ⬇️
...ssionReplay/Preview/SentryMaskingPreviewView.swift 0.000% <0.000%> (-86.275%) ⬇️
.../Tools/ViewCapture/SentryDefaultMaskRenderer.swift 0.000% <0.000%> (-100.000%) ⬇️
...ore/Tools/ViewCapture/SentryViewPhotographer.swift 0.000% <0.000%> (-73.334%) ⬇️
Sources/Sentry/SentrySessionReplayIntegration.m 0.000% <0.000%> (-86.982%) ⬇️
...rations/SessionReplay/SentryReplayVideoMaker.swift 0.000% <0.000%> (-100.000%) ⬇️
...egrations/SessionReplay/SentryOnDemandReplay.swift 0.000% <0.000%> (-89.888%) ⬇️
...tegrations/SessionReplay/SentrySessionReplay.swift 0.000% <0.000%> (-90.554%) ⬇️
...ift/Core/Tools/ViewCapture/ViewHierarchyNode.swift 0.000% <0.000%> (ø)
... and 2 more

... and 672 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f96ab19...18efa31. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

github-actions bot commented Apr 9, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1187.61 ms 1225.28 ms 37.67 ms
Size 23.76 KiB 852.24 KiB 828.48 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8cf5bca 1212.35 ms 1223.90 ms 11.54 ms
3a31fc9 1237.35 ms 1249.02 ms 11.67 ms
98752f3 1240.61 ms 1259.80 ms 19.18 ms
f4e0299 1230.33 ms 1249.68 ms 19.35 ms
65af68e 1226.69 ms 1250.69 ms 24.00 ms
d10ae0c 1250.02 ms 1253.74 ms 3.72 ms
2124551 1265.50 ms 1276.44 ms 10.94 ms
b9b0f0a 1239.76 ms 1257.68 ms 17.92 ms
2a769ba 1217.92 ms 1239.78 ms 21.86 ms
b4f8dba 1343.92 ms 1362.96 ms 19.04 ms

App size

Revision Plain With Sentry Diff
8cf5bca 21.58 KiB 671.30 KiB 649.72 KiB
3a31fc9 20.76 KiB 414.45 KiB 393.69 KiB
98752f3 20.76 KiB 435.09 KiB 414.33 KiB
f4e0299 20.76 KiB 427.54 KiB 406.78 KiB
65af68e 23.76 KiB 861.21 KiB 837.46 KiB
d10ae0c 20.76 KiB 419.86 KiB 399.10 KiB
2124551 22.85 KiB 411.69 KiB 388.84 KiB
b9b0f0a 20.76 KiB 434.94 KiB 414.18 KiB
2a769ba 21.58 KiB 683.64 KiB 662.05 KiB
b4f8dba 21.58 KiB 614.87 KiB 593.29 KiB

@philprime philprime added the dontmerge A branch that absolutely should not be merged while this label is applied. label Apr 9, 2025
@philprime philprime force-pushed the philprime/sr-masking-analysis branch from 04fd9a6 to a5031ed Compare May 30, 2025 14:27
@philprime philprime force-pushed the philprime/sr-masking-analysis branch from a5031ed to 18efa31 Compare May 30, 2025 14:48
@philprime
Copy link
Contributor Author

Due to changes in the samples apps I am no longer able to reproduce the masking issue during transitions. I'll have to re-create a custom sample to investigate this isolated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dontmerge A branch that absolutely should not be merged while this label is applied. Platform: Cocoa Session Replay
Projects
Status: In Investigation
Development

Successfully merging this pull request may close these issues.

1 participant