Skip to content

refactor(session-replay): add separate method to get video info with more logging #5132

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

Merged
merged 7 commits into from
Apr 25, 2025

Conversation

philprime
Copy link
Contributor

@philprime philprime commented Apr 24, 2025

📜 Description

Refactors the logic to get rendered session replay info to another method.
Furthermore it also adds more debug and error logging.

💡 Motivation and Context

This change has been a side-product of #5018 and I extracted it into another PR for easier review.

💚 How did you test it?

  • Unit tests & manual testing using sample.

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

@philprime philprime changed the title refactor(session-replay): add separate method to get video info refactor(session-replay): add separate method to get video info with more logging Apr 24, 2025
@philprime philprime moved this to Needs Review in [DEPRECATED] Mobile SDKs Apr 24, 2025
Copy link

codecov bot commented Apr 24, 2025

Codecov Report

Attention: Patch coverage is 83.78378% with 12 lines in your changes missing coverage. Please review.

Project coverage is 92.794%. Comparing base (bfcfc62) to head (161b545).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...egrations/SessionReplay/SentryOnDemandReplay.swift 79.661% 10 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5132       +/-   ##
=============================================
+ Coverage   92.786%   92.794%   +0.008%     
=============================================
  Files          675       676        +1     
  Lines        83937     84131      +194     
  Branches     30497     30614      +117     
=============================================
+ Hits         77882     78069      +187     
- Misses        5955      5961        +6     
- Partials       100       101        +1     
Files with missing lines Coverage Δ
...ions/SessionReplay/SentryOnDemandReplayTests.swift 92.792% <100.000%> (+0.339%) ⬆️
...egrations/SessionReplay/SentryOnDemandReplay.swift 93.518% <79.661%> (-2.165%) ⬇️

... and 33 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 bfcfc62...161b545. 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 24, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1236.45 ms 1257.37 ms 20.92 ms
Size 22.30 KiB 850.32 KiB 828.02 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
f25febb 1238.80 ms 1245.35 ms 6.56 ms
69d8759 1235.71 ms 1241.34 ms 5.63 ms
ce4cfaf 1203.61 ms 1218.86 ms 15.25 ms
7419285 1209.53 ms 1244.72 ms 35.19 ms
b15521e 1224.44 ms 1251.13 ms 26.68 ms
bd2afa6 1245.24 ms 1263.18 ms 17.94 ms
53b29ac 1228.81 ms 1244.96 ms 16.15 ms
443fb02 1231.06 ms 1252.60 ms 21.54 ms
888a145 1228.63 ms 1248.94 ms 20.30 ms
70c49ca 1206.44 ms 1233.07 ms 26.62 ms

App size

Revision Plain With Sentry Diff
f25febb 21.58 KiB 414.92 KiB 393.34 KiB
69d8759 20.76 KiB 393.05 KiB 372.29 KiB
ce4cfaf 20.76 KiB 423.19 KiB 402.43 KiB
7419285 20.76 KiB 432.99 KiB 412.22 KiB
b15521e 21.58 KiB 573.18 KiB 551.60 KiB
bd2afa6 20.76 KiB 420.55 KiB 399.79 KiB
53b29ac 22.32 KiB 819.55 KiB 797.24 KiB
443fb02 22.30 KiB 832.42 KiB 810.11 KiB
888a145 21.58 KiB 713.54 KiB 691.96 KiB
70c49ca 22.31 KiB 778.76 KiB 756.45 KiB

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Thanks 💯

Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
@philprime philprime merged commit 4795adc into main Apr 25, 2025
79 of 81 checks passed
@philprime philprime deleted the philprime/refactor-ondemand-replay-video-info branch April 25, 2025 07:43
@github-project-automation github-project-automation bot moved this from Needs Review to Done in [DEPRECATED] Mobile SDKs Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants