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

Introduce BeforeInitNativeSDKCallback #1915

Closed
wants to merge 3 commits into from

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Mar 5, 2024

📜 Description

Provide uses with a callback to mutate arguments passed to native SDKs. This is just an idea to resolve #1877 and we should discuss if this is a viable solution.

We could introduce SentryNativeOptions and pass it instead of the map, so we'd have type safety and maybe also only expose a subset of options.

💡 Motivation and Context

Relates to #1877

💚 How did you test it?

📝 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 Mar 5, 2024

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Introduce `BeforeInitNativeSDKCallback` ([#1915](https://github.com/getsentry/sentry-dart/pull/1915))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 97b6bbf

Copy link
Contributor

github-actions bot commented Mar 5, 2024

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 397.52 ms 468.43 ms 70.91 ms
Size 6.34 MiB 7.28 MiB 966.70 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8cb6557 321.20 ms 370.46 ms 49.26 ms
48adddf 353.96 ms 431.10 ms 77.14 ms
464b4d0 320.71 ms 380.02 ms 59.31 ms
7748b7c 356.37 ms 426.70 ms 70.33 ms
b44db8f 307.77 ms 366.41 ms 58.64 ms
55cc6ef 300.98 ms 351.98 ms 51.00 ms
df16b96 326.08 ms 391.82 ms 65.74 ms
7f2b01d 304.94 ms 345.71 ms 40.78 ms
f4cc744 349.53 ms 394.68 ms 45.15 ms
ef31c7f 311.39 ms 359.33 ms 47.94 ms

App size

Revision Plain With Sentry Diff
8cb6557 6.06 MiB 7.03 MiB 997.01 KiB
48adddf 6.27 MiB 7.20 MiB 959.09 KiB
464b4d0 6.06 MiB 7.03 MiB 990.27 KiB
7748b7c 6.27 MiB 7.20 MiB 959.09 KiB
b44db8f 6.16 MiB 7.14 MiB 1003.72 KiB
55cc6ef 6.06 MiB 7.03 MiB 995.45 KiB
df16b96 6.06 MiB 7.03 MiB 988.94 KiB
7f2b01d 5.94 MiB 6.95 MiB 1.01 MiB
f4cc744 5.94 MiB 6.95 MiB 1.01 MiB
ef31c7f 6.06 MiB 7.09 MiB 1.03 MiB

Copy link
Contributor

github-actions bot commented Mar 5, 2024

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1257.55 ms 1274.24 ms 16.69 ms
Size 8.33 MiB 9.40 MiB 1.07 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
bd37365 1236.22 ms 1243.27 ms 7.04 ms
d883d62 1221.39 ms 1230.18 ms 8.80 ms
42f6e7e 1232.52 ms 1247.86 ms 15.34 ms
b9da046 1256.43 ms 1275.82 ms 19.39 ms
ebfead1 1271.57 ms 1284.48 ms 12.91 ms
014c3ea 1298.73 ms 1351.24 ms 52.51 ms
7f2b01d 1253.92 ms 1266.70 ms 12.78 ms
a817b8f 1261.90 ms 1264.62 ms 2.73 ms
955541a 1230.22 ms 1252.96 ms 22.73 ms
56810ff 1267.59 ms 1293.48 ms 25.89 ms

App size

Revision Plain With Sentry Diff
bd37365 8.28 MiB 9.34 MiB 1.06 MiB
d883d62 8.29 MiB 9.36 MiB 1.07 MiB
42f6e7e 8.28 MiB 9.33 MiB 1.05 MiB
b9da046 8.10 MiB 9.16 MiB 1.07 MiB
ebfead1 8.10 MiB 9.16 MiB 1.07 MiB
014c3ea 8.33 MiB 9.39 MiB 1.06 MiB
7f2b01d 8.16 MiB 9.16 MiB 1.00 MiB
a817b8f 8.29 MiB 9.37 MiB 1.07 MiB
955541a 8.28 MiB 9.34 MiB 1.06 MiB
56810ff 8.15 MiB 9.12 MiB 987.35 KiB

@buenaflor
Copy link
Contributor

Need to check how other sdks do it, e.g https://github.com/getsentry/sentry-capacitor/pull/585/files

@denrase
Copy link
Collaborator Author

denrase commented Mar 11, 2024

Closing in favour of #1931

@denrase denrase closed this Mar 11, 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.

Use different options for native SDK
2 participants