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: Read sentry.options.json during cocoa init #4447

Merged
merged 36 commits into from
Jan 22, 2025

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Jan 15, 2025

📢 Type of change

  • New feature

⛓️ PR Chain

📜 Description

This PR adds [RNSentrySDK start] function (exported by RNSentry/RNSentry.h) which allows users to initialize Sentry Cocoa in RN applications without any additional params because the options are loaded from sentry.options.json.

This PR adds automatic copy of sentry.options.json resource from root of the RN project. This works as long as users setuped sentry-xcode.sh on their Bundle RN build phase.

💡 Motivation and Context

💚 How did you test it?

Only in Sample app for now, tests will follow.

📝 Checklist

  • I added tests to verify 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.
  • All tests passing
  • No breaking changes

🔮 Next steps

Copy link
Contributor

github-actions bot commented Jan 15, 2025

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

Generated by 🚫 dangerJS against 7eb1786

Copy link
Contributor

github-actions bot commented Jan 15, 2025

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 650.96 ms 736.00 ms 85.04 ms
Size 7.15 MiB 8.38 MiB 1.23 MiB

Baseline results on branch: kw-add-react-native-ios-manual-init

Startup times

Revision Plain With Sentry Diff
a1ab76a+dirty 412.92 ms 538.19 ms 125.27 ms

App size

Revision Plain With Sentry Diff
a1ab76a+dirty 7.15 MiB 8.38 MiB 1.23 MiB

Previous results on branch: kw-add-read-ios-options-file

Startup times

Revision Plain With Sentry Diff
eda229b+dirty 424.00 ms 469.60 ms 45.60 ms

App size

Revision Plain With Sentry Diff
eda229b+dirty 7.15 MiB 8.38 MiB 1.23 MiB

Copy link
Contributor

github-actions bot commented Jan 15, 2025

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1227.22 ms 1229.13 ms 1.90 ms
Size 2.63 MiB 3.69 MiB 1.05 MiB

Baseline results on branch: capture-app-start-errors

Startup times

Revision Plain With Sentry Diff
555070f+dirty 1213.59 ms 1217.79 ms 4.20 ms
208f4af+dirty 1209.44 ms 1217.13 ms 7.70 ms

App size

Revision Plain With Sentry Diff
555070f+dirty 2.63 MiB 3.69 MiB 1.05 MiB
208f4af+dirty 2.63 MiB 3.69 MiB 1.05 MiB

Previous results on branch: kw-add-read-ios-options-file

Startup times

Revision Plain With Sentry Diff
4682761+dirty 1219.41 ms 1228.23 ms 8.82 ms
eda229b+dirty 1224.98 ms 1225.46 ms 0.48 ms

App size

Revision Plain With Sentry Diff
4682761+dirty 2.63 MiB 3.69 MiB 1.05 MiB
eda229b+dirty 2.63 MiB 3.69 MiB 1.05 MiB

Copy link
Contributor

github-actions bot commented Jan 15, 2025

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1218.25 ms 1216.94 ms -1.31 ms
Size 3.19 MiB 4.25 MiB 1.07 MiB

Baseline results on branch: capture-app-start-errors

Startup times

Revision Plain With Sentry Diff
555070f+dirty 1223.61 ms 1227.57 ms 3.96 ms
208f4af+dirty 1213.08 ms 1223.82 ms 10.73 ms

App size

Revision Plain With Sentry Diff
555070f+dirty 3.19 MiB 4.25 MiB 1.06 MiB
208f4af+dirty 3.19 MiB 4.25 MiB 1.06 MiB

Previous results on branch: kw-add-read-ios-options-file

Startup times

Revision Plain With Sentry Diff
4682761+dirty 1221.30 ms 1225.33 ms 4.03 ms
eda229b+dirty 1227.17 ms 1235.08 ms 7.91 ms

App size

Revision Plain With Sentry Diff
4682761+dirty 3.19 MiB 4.25 MiB 1.07 MiB
eda229b+dirty 3.19 MiB 4.25 MiB 1.07 MiB

Copy link
Contributor

github-actions bot commented Jan 15, 2025

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 483.20 ms 503.78 ms 20.59 ms
Size 17.75 MiB 20.11 MiB 2.37 MiB

Baseline results on branch: capture-app-start-errors

Startup times

Revision Plain With Sentry Diff
555070f 438.67 ms 428.30 ms -10.37 ms
208f4af 439.23 ms 427.31 ms -11.92 ms

App size

Revision Plain With Sentry Diff
555070f 17.75 MiB 20.11 MiB 2.37 MiB
208f4af 17.75 MiB 20.11 MiB 2.37 MiB

Previous results on branch: kw-add-read-ios-options-file

Startup times

Revision Plain With Sentry Diff
4682761 398.02 ms 424.46 ms 26.44 ms
eda229b 461.02 ms 578.98 ms 117.96 ms

App size

Revision Plain With Sentry Diff
4682761 17.75 MiB 20.11 MiB 2.37 MiB
eda229b 17.75 MiB 20.11 MiB 2.37 MiB

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.

ObjC code looks good.

@krystofwoldrich krystofwoldrich marked this pull request as ready for review January 21, 2025 16:16
Copy link
Collaborator

@antonis antonis left a comment

Choose a reason for hiding this comment

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

The code looks good and the sample app worked as expected for me with [RNSentrySDK start] 🚀
Thank you for the added tests 🙇

Base automatically changed from kw-add-react-native-ios-manual-init to capture-app-start-errors January 22, 2025 13:24
@krystofwoldrich krystofwoldrich merged commit 7850677 into capture-app-start-errors Jan 22, 2025
65 of 66 checks passed
@krystofwoldrich krystofwoldrich deleted the kw-add-read-ios-options-file branch January 22, 2025 16:55
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.

3 participants