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: add an option to configure native backend on windows & linux #2522

Merged
merged 5 commits into from
Dec 19, 2024

Conversation

vaind
Copy link
Collaborator

@vaind vaind commented Dec 19, 2024

📜 Description

Since v8.11.0 was released, there were a couple of issues reported with builds breaking. All of these were about a crashpad backend (which is the default one).

Because the change was introduced in a minor release, we shouldn't have a breaking change and we're making a native crash handler an opt-in.
Also, because we the reported issues were about building the backend, we're only changing that part of the native integration, while still leaving obfuscation support untouched.

What this PR effectively does:

  • add SENTRY_NATIVE_BACKEND env var that is picked up during build (can be one of crashpad, breakpad, inproc, none
  • sets the default value of SENTRY_NATIVE_BACKEND to none - this will change in v9
  • adds future default values to windows & linux cmakelists, although they're currently overwritten.

💡 Motivation and Context

💚 How did you test it?

Manually

📝 Checklist

  • I reviewed submitted code
  • 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
  • All tests passing
  • No breaking changes

🔮 Next steps

Copy link
Contributor

github-actions bot commented Dec 19, 2024

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

Generated by 🚫 dangerJS against fc5cc91

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.17%. Comparing base (c3aeb1f) to head (fc5cc91).
Report is 1 commits behind head on rel/8.11.2.

Additional details and impacted files
@@              Coverage Diff               @@
##           rel/8.11.2    #2522      +/-   ##
==============================================
+ Coverage       86.85%   92.17%   +5.32%     
==============================================
  Files             260       84     -176     
  Lines            9240     2902    -6338     
==============================================
- Hits             8025     2675    -5350     
+ Misses           1215      227     -988     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Dec 19, 2024

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1256.49 ms 1285.31 ms 28.82 ms
Size 8.42 MiB 9.87 MiB 1.44 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
9fe67d5 1242.33 ms 1268.77 ms 26.44 ms
a775d3f 1241.29 ms 1241.53 ms 0.24 ms
103eb14 1254.11 ms 1280.65 ms 26.55 ms
754cdbe 1261.67 ms 1280.88 ms 19.20 ms
408bdab 1239.63 ms 1257.61 ms 17.98 ms
fbf42af 1253.76 ms 1269.51 ms 15.76 ms
c73ab67 1267.73 ms 1279.36 ms 11.63 ms
99704d2 1241.74 ms 1258.00 ms 16.26 ms
bd1b990 1209.43 ms 1226.96 ms 17.53 ms
4c13d97 1265.41 ms 1287.95 ms 22.55 ms

App size

Revision Plain With Sentry Diff
9fe67d5 8.32 MiB 9.50 MiB 1.18 MiB
a775d3f 8.28 MiB 9.34 MiB 1.06 MiB
103eb14 8.38 MiB 9.71 MiB 1.34 MiB
754cdbe 8.29 MiB 9.37 MiB 1.08 MiB
408bdab 8.28 MiB 9.34 MiB 1.06 MiB
fbf42af 8.16 MiB 9.17 MiB 1.01 MiB
c73ab67 8.29 MiB 9.36 MiB 1.07 MiB
99704d2 8.42 MiB 9.87 MiB 1.44 MiB
bd1b990 8.32 MiB 9.38 MiB 1.06 MiB
4c13d97 8.38 MiB 9.78 MiB 1.40 MiB

Previous results on branch: feat/native-backend-config

Startup times

Revision Plain With Sentry Diff
d660887 1270.15 ms 1279.88 ms 9.73 ms

App size

Revision Plain With Sentry Diff
d660887 8.42 MiB 9.87 MiB 1.44 MiB

Copy link
Contributor

github-actions bot commented Dec 19, 2024

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 518.85 ms 636.24 ms 117.39 ms
Size 6.46 MiB 7.48 MiB 1.02 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
1cdcacf 389.94 ms 463.53 ms 73.59 ms
be08ed1 361.37 ms 414.84 ms 53.47 ms
3ad66e4 406.90 ms 481.48 ms 74.58 ms
ad69abc 297.35 ms 385.89 ms 88.54 ms
ef6466d 373.96 ms 443.17 ms 69.21 ms
e893df5 310.60 ms 380.58 ms 69.98 ms
f25f207 341.40 ms 427.73 ms 86.34 ms
61e71ec 343.94 ms 410.59 ms 66.66 ms
1131914 317.90 ms 361.58 ms 43.69 ms
fe4aa56 356.06 ms 428.67 ms 72.61 ms

App size

Revision Plain With Sentry Diff
1cdcacf 6.33 MiB 7.26 MiB 949.77 KiB
be08ed1 6.33 MiB 7.26 MiB 946.42 KiB
3ad66e4 6.33 MiB 7.26 MiB 947.04 KiB
ad69abc 6.06 MiB 7.09 MiB 1.03 MiB
ef6466d 6.34 MiB 7.28 MiB 967.79 KiB
e893df5 6.06 MiB 7.09 MiB 1.03 MiB
f25f207 6.33 MiB 7.26 MiB 943.41 KiB
61e71ec 6.34 MiB 7.28 MiB 966.26 KiB
1131914 5.94 MiB 6.96 MiB 1.02 MiB
fe4aa56 6.06 MiB 7.10 MiB 1.04 MiB

Previous results on branch: feat/native-backend-config

Startup times

Revision Plain With Sentry Diff
d660887 451.04 ms 524.22 ms 73.18 ms

App size

Revision Plain With Sentry Diff
d660887 6.46 MiB 7.48 MiB 1.02 MiB

@vaind vaind mentioned this pull request Dec 19, 2024
8 tasks
Copy link
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

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

👍 like we talked about, we will merge this into a new branch from the previous patch release and make a release there

@vaind vaind changed the base branch from main to rel/8.11.2 December 19, 2024 16:07
@vaind vaind force-pushed the feat/native-backend-config branch from 388ea91 to a42d4a2 Compare December 19, 2024 16:22
Copy link
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

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

I added only small changes but looks good

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@vaind vaind marked this pull request as ready for review December 19, 2024 16:37
Co-authored-by: Giancarlo Buenaflor <giancarlo_buenaflor@yahoo.com>
@vaind vaind merged commit 517ad7b into rel/8.11.2 Dec 19, 2024
41 of 43 checks passed
@vaind vaind deleted the feat/native-backend-config branch December 19, 2024 16:38
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.

2 participants