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): Expose ignore and redact classes to hybrid SDKs #3891

Merged

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Apr 24, 2024

📜 Description

This PR exposes addIgnoreClasses and addRedactClasses for Hybrid SDKs.

#skip-changelog

💡 Motivation and Context

💚 How did you test it?

RN Sample App. And a few sanity checks, I have not found a way to check that the class will actually be redacted.

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • 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.
  • 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.

🔮 Next steps

@krystofwoldrich krystofwoldrich changed the title feat(replay): add without Event param for Hybrid SDKs feat(replay): Expose ignore and redact classes to hybrid SDKs Apr 24, 2024
Copy link

github-actions bot commented Apr 24, 2024

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

Generated by 🚫 dangerJS against 1cf71d8

Copy link

codecov bot commented Apr 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.863%. Comparing base (0cc5400) to head (1cf71d8).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3891       +/-   ##
=============================================
- Coverage   90.931%   90.863%   -0.068%     
=============================================
  Files          582       582               
  Lines        45432     45466       +34     
  Branches     16193     16197        +4     
=============================================
  Hits         41312     41312               
+ Misses        4050      3973       -77     
- Partials        70       181      +111     
Files Coverage Δ
Sources/Sentry/PrivateSentrySDKOnly.mm 91.549% <100.000%> (+1.149%) ⬆️
Sources/Swift/Tools/SentryViewPhotographer.swift 23.437% <100.000%> (+5.104%) ⬆️
Tests/SentryTests/PrivateSentrySDKOnlyTests.swift 99.428% <100.000%> (+0.049%) ⬆️

... and 32 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 0cc5400...1cf71d8. 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.

Perfect, simple solution. We just need to figure it out CI

Base automatically changed from kw-add-hybrid-sdks-replay-interface to main April 25, 2024 08:54
Copy link

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1240.89 ms 1259.49 ms 18.60 ms
Size 21.58 KiB 616.14 KiB 594.56 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
ad7cec6 1203.22 ms 1224.74 ms 21.52 ms
154f795 1225.53 ms 1231.04 ms 5.51 ms
189b629 1250.64 ms 1261.02 ms 10.38 ms
89b12eb 1236.02 ms 1246.63 ms 10.61 ms
caa37b6 1197.43 ms 1211.52 ms 14.09 ms
c5ff7b8 1226.68 ms 1235.04 ms 8.36 ms
3db3e35 1248.02 ms 1258.35 ms 10.33 ms
561fa74 1243.27 ms 1260.62 ms 17.35 ms
39b1c35 1244.71 ms 1248.60 ms 3.89 ms
d61b939 1238.61 ms 1240.08 ms 1.47 ms

App size

Revision Plain With Sentry Diff
ad7cec6 20.76 KiB 427.32 KiB 406.55 KiB
154f795 20.76 KiB 435.25 KiB 414.49 KiB
189b629 20.76 KiB 399.69 KiB 378.93 KiB
89b12eb 20.76 KiB 432.88 KiB 412.11 KiB
caa37b6 21.58 KiB 424.34 KiB 402.76 KiB
c5ff7b8 22.85 KiB 414.80 KiB 391.95 KiB
3db3e35 21.58 KiB 419.21 KiB 397.63 KiB
561fa74 20.76 KiB 427.23 KiB 406.46 KiB
39b1c35 22.85 KiB 408.88 KiB 386.03 KiB
d61b939 22.85 KiB 407.63 KiB 384.78 KiB

@krystofwoldrich krystofwoldrich merged commit d6ff82c into main Apr 25, 2024
69 of 70 checks passed
@krystofwoldrich krystofwoldrich deleted the kw-add-hybrid-sdks-register-components-for-replay branch April 25, 2024 10:19
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