Skip to content
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

feat(replay): add captureReplay without Event param for Hybrid SDKs #3878

Merged
merged 3 commits into from Apr 25, 2024

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Apr 23, 2024

This PR splits captureReplayForEvent into captureReplay and captureReplayForEvent, as Hybrid SDKs need to capture replay without passing their event to native.

This PR is need for the alpha.0 of RN replay.

#skip-changelog

Copy link

github-actions bot commented Apr 23, 2024

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 7aac2b2

Copy link

codecov bot commented Apr 23, 2024

Codecov Report

Attention: Patch coverage is 82.35294% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 90.609%. Comparing base (676e429) to head (7aac2b2).
Report is 11 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3878       +/-   ##
=============================================
- Coverage   90.635%   90.609%   -0.026%     
=============================================
  Files          579       579               
  Lines        45298     45313       +15     
  Branches     16122     16124        +2     
=============================================
+ Hits         41056     41058        +2     
- Misses        4063      4076       +13     
  Partials       179       179               
Files Coverage Δ
...tions/SessionReplay/SentrySessionReplayTests.swift 98.360% <100.000%> (+0.099%) ⬆️
Sources/Sentry/SentrySessionReplay.m 87.581% <70.000%> (-1.384%) ⬇️

... and 4 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 676e429...7aac2b2. Read the comment docs.

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

Im not sure about exposing SentrySessionReplay to Hybrid SDK.
And btw, I plan to migrate this to swift.

What are the benefits of this instead of a start//stopReplay function at PrivateSentrySDKOnly?

@krystofwoldrich
Copy link
Member Author

@brustolin I tried to make minimum changes to get Replay working for RN as I know you are planning to change this class.

I opened this PR to establish an interface in PrivateSentrySDKOnly

Copy link

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1215.42 ms 1236.61 ms 21.19 ms
Size 21.58 KiB 614.79 KiB 593.21 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b9a9dca 1377.00 ms 1382.55 ms 5.55 ms
da5462e 1198.18 ms 1229.14 ms 30.96 ms
f8833c4 1230.00 ms 1245.27 ms 15.27 ms
8f397a7 1251.82 ms 1268.34 ms 16.52 ms
7fb7afb 1230.12 ms 1251.04 ms 20.92 ms
289c0b8 1245.63 ms 1253.76 ms 8.13 ms
6c31077 1233.80 ms 1245.34 ms 11.54 ms
b483671 1217.20 ms 1236.82 ms 19.62 ms
fc163f5 1219.18 ms 1242.74 ms 23.56 ms
154f795 1225.53 ms 1231.04 ms 5.51 ms

App size

Revision Plain With Sentry Diff
b9a9dca 21.58 KiB 414.59 KiB 393.01 KiB
da5462e 22.85 KiB 412.98 KiB 390.13 KiB
f8833c4 21.58 KiB 422.66 KiB 401.08 KiB
8f397a7 20.76 KiB 420.55 KiB 399.79 KiB
7fb7afb 20.76 KiB 419.70 KiB 398.94 KiB
289c0b8 22.85 KiB 407.67 KiB 384.82 KiB
6c31077 22.84 KiB 401.65 KiB 378.81 KiB
b483671 20.76 KiB 434.72 KiB 413.96 KiB
fc163f5 20.76 KiB 436.29 KiB 415.53 KiB
154f795 20.76 KiB 435.25 KiB 414.49 KiB

@krystofwoldrich krystofwoldrich merged commit b4f8dba into main Apr 25, 2024
67 of 71 checks passed
@krystofwoldrich krystofwoldrich deleted the kw-add-capture-replay-for-hybrids branch April 25, 2024 08:17
dKasabwala pushed a commit to dKasabwala/sentry-cocoa that referenced this pull request May 6, 2024
threema-matteo pushed a commit to threema-ch/sentry-cocoa that referenced this pull request May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants