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 Hybrid SDKs Replay interface #3879

Merged
merged 12 commits into from Apr 25, 2024

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Apr 23, 2024

This PR adds a simple captureReplay interface hiding implementation details from the Hybrid SDKs.

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 6aa2e80

@krystofwoldrich krystofwoldrich marked this pull request as ready for review April 23, 2024 12:39
Copy link

codecov bot commented Apr 23, 2024

Codecov Report

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

Project coverage is 90.586%. Comparing base (676e429) to head (6aa2e80).
Report is 12 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3879       +/-   ##
=============================================
- Coverage   90.635%   90.586%   -0.050%     
=============================================
  Files          579       581        +2     
  Lines        45298     45349       +51     
  Branches     16122     16148       +26     
=============================================
+ Hits         41056     41080       +24     
- Misses        4063      4199      +136     
+ Partials       179        70      -109     
Files Coverage Δ
SentryTestUtils/TestOptions.swift 100.000% <100.000%> (ø)
Sources/Sentry/SentryHub.m 98.533% <ø> (ø)
...tions/SessionReplay/SentrySessionReplayTests.swift 98.360% <100.000%> (+0.099%) ⬆️
Tests/SentryTests/PrivateSentrySDKOnlyTests.swift 99.378% <100.000%> (+0.142%) ⬆️
Sources/Sentry/PrivateSentrySDKOnly.mm 90.400% <94.444%> (+3.244%) ⬆️
Sources/Sentry/SentrySessionReplayIntegration.m 90.196% <0.000%> (-1.804%) ⬇️
Sources/Sentry/SentrySessionReplay.m 87.581% <70.000%> (-1.384%) ⬇️

... and 41 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...6aa2e80. Read the comment docs.

Copy link

github-actions bot commented Apr 23, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1240.14 ms 1262.40 ms 22.25 ms
Size 21.58 KiB 615.59 KiB 594.01 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
06548c0 1225.58 ms 1244.70 ms 19.12 ms
4259afd 1222.12 ms 1249.74 ms 27.62 ms
189b629 1211.16 ms 1224.30 ms 13.14 ms
af1f4dd 1232.39 ms 1255.88 ms 23.49 ms
89bc37d 1228.20 ms 1257.10 ms 28.90 ms
50bb751 1230.94 ms 1238.69 ms 7.75 ms
a423161 1234.41 ms 1246.41 ms 12.00 ms
39b1c35 1236.35 ms 1239.90 ms 3.55 ms
28333b6 1186.29 ms 1225.18 ms 38.89 ms
ca91a5c 1234.53 ms 1249.86 ms 15.33 ms

App size

Revision Plain With Sentry Diff
06548c0 20.76 KiB 427.35 KiB 406.59 KiB
4259afd 20.76 KiB 419.70 KiB 398.94 KiB
189b629 20.76 KiB 399.69 KiB 378.93 KiB
af1f4dd 22.85 KiB 414.71 KiB 391.86 KiB
89bc37d 20.76 KiB 401.53 KiB 380.77 KiB
50bb751 21.58 KiB 417.86 KiB 396.27 KiB
a423161 22.85 KiB 406.69 KiB 383.85 KiB
39b1c35 22.85 KiB 408.88 KiB 386.03 KiB
28333b6 20.76 KiB 424.69 KiB 403.93 KiB
ca91a5c 22.84 KiB 403.19 KiB 380.34 KiB

Previous results on branch: kw-add-hybrid-sdks-replay-interface

Startup times

Revision Plain With Sentry Diff
85acf3b 1234.26 ms 1250.88 ms 16.62 ms
54ce03c 1232.27 ms 1258.22 ms 25.96 ms

App size

Revision Plain With Sentry Diff
85acf3b 21.58 KiB 615.52 KiB 593.94 KiB
54ce03c 21.58 KiB 615.52 KiB 593.93 KiB

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.

I reviewed this in real life! It looks good!

@krystofwoldrich
Copy link
Member Author

The #3878 needs to be merged first. As this PR uses the new captureReplay method.

Base automatically changed from kw-add-capture-replay-for-hybrids to main April 25, 2024 08:17
@krystofwoldrich krystofwoldrich merged commit 1cafec7 into main Apr 25, 2024
69 of 70 checks passed
@krystofwoldrich krystofwoldrich deleted the kw-add-hybrid-sdks-replay-interface branch April 25, 2024 08:54
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