feat(Session Replay): RedactOptions#3842
feat(Session Replay): RedactOptions#3842brustolin merged 11 commits intofeat/experimental-optionsfrom
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/experimental-options #3842 +/- ##
===============================================================
- Coverage 89.052% 89.049% -0.004%
===============================================================
Files 556 556
Lines 60379 60408 +29
Branches 21663 21680 +17
===============================================================
+ Hits 53769 53793 +24
+ Misses 5670 5557 -113
- Partials 940 1058 +118
... and 38 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 23c1156 | 1218.55 ms | 1242.84 ms | 24.28 ms |
| 8ba1afe | 1232.96 ms | 1247.94 ms | 14.98 ms |
| 1ca681c | 1230.61 ms | 1252.46 ms | 21.85 ms |
| 42535e4 | 1231.21 ms | 1250.35 ms | 19.14 ms |
| bc9c60d | 1235.55 ms | 1252.02 ms | 16.47 ms |
| 16172e3 | 1225.07 ms | 1241.80 ms | 16.73 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 23c1156 | 21.58 KiB | 577.78 KiB | 556.20 KiB |
| 8ba1afe | 21.58 KiB | 578.58 KiB | 557.00 KiB |
| 1ca681c | 21.58 KiB | 578.08 KiB | 556.50 KiB |
| 42535e4 | 21.58 KiB | 577.78 KiB | 556.20 KiB |
| bc9c60d | 21.58 KiB | 577.78 KiB | 556.20 KiB |
| 16172e3 | 21.58 KiB | 577.80 KiB | 556.21 KiB |
Previous results on branch: feat(SR)/masking-options
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d71df0c | 1232.14 ms | 1248.31 ms | 16.16 ms |
| 5822059 | 1212.88 ms | 1242.69 ms | 29.82 ms |
| 3ed3ea8 | 1233.80 ms | 1236.36 ms | 2.57 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d71df0c | 21.58 KiB | 579.21 KiB | 557.62 KiB |
| 5822059 | 21.58 KiB | 579.10 KiB | 557.52 KiB |
| 3ed3ea8 | 21.58 KiB | 579.10 KiB | 557.52 KiB |
| /** | ||
| * Indicates whether session replay should redact all text in the app | ||
| * by drawing a black rectangle over it. | ||
| * | ||
| * - note: The default is true | ||
| */ |
There was a problem hiding this comment.
m: Swift comments should be like this if I'm not mistaken
sentry-cocoa/Sources/Swift/Metrics/SentryMetricsAPI.swift
Lines 43 to 48 in c0f08e7
There was a problem hiding this comment.
The way I did work, I tried.
But we can choose which way we prefer.
There was a problem hiding this comment.
Hmm, I got this from Alamofire. I don't mind.
| import Foundation | ||
|
|
||
| @objc | ||
| protocol SentryRedactOptions { |
There was a problem hiding this comment.
m: I don't get why we need that interface. Please explain.
There was a problem hiding this comment.
Decoupling.
If we continue along the current path, where everything depends on everything else, it will be impossible to make any major changes in the future.
I am trying to be a responsible engineer and adopting protocols-oriented programming, as suggested by Apple when Swift was released.
I will create a document for us to discuss swift architecture since we're in the beginning.
Also, this approach is highly beneficial for testing purposes. We no longer need to override a class with a multitude of behaviors; instead, we simply need to implement the protocol
There was a problem hiding this comment.
It's great you care about decoupling. What are you decoupling with this interface?
There was a problem hiding this comment.
SentryViewPhotographer to have a dependency on sentryreplayoptions,
SentryViewPhotographer will also be used for screenshot integration later.
philipphofmann
left a comment
philipphofmann
left a comment
There was a problem hiding this comment.
LGTM, I don't mind the extra protocol for now.
2 participants
Add options to enable or disabled redaction for session replay.
#skip-changelog