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

Chore: Move SentryUserFeedbackIntegration initialization. #4538

Open
wants to merge 4 commits into
base: fix/load-integrations
Choose a base branch
from

Conversation

brustolin
Copy link
Contributor

Integrations should be initialized in the defaultIntegrations property of the options.

Is there any reason to handle this differently for UserFeedback, @armcknight?

#skip-changelog

Copy link

github-actions bot commented Nov 15, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1220.46 ms 1247.65 ms 27.19 ms
Size 22.30 KiB 730.78 KiB 708.48 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b11cd87 1234.51 ms 1250.52 ms 16.01 ms
5b14b06 1196.33 ms 1227.00 ms 30.67 ms
aa4eddf 1228.21 ms 1236.72 ms 8.51 ms
a4ee85b 1227.57 ms 1232.32 ms 4.75 ms
4d3df92 1245.35 ms 1259.58 ms 14.23 ms
e8b11f8 1233.66 ms 1249.74 ms 16.08 ms
6161814 1216.90 ms 1233.18 ms 16.29 ms
742d4b6 1217.04 ms 1229.38 ms 12.33 ms
2124551 1231.78 ms 1264.94 ms 33.16 ms
ecd9ecd 1215.77 ms 1238.70 ms 22.93 ms

App size

Revision Plain With Sentry Diff
b11cd87 21.58 KiB 631.86 KiB 610.28 KiB
5b14b06 22.84 KiB 402.56 KiB 379.72 KiB
aa4eddf 21.58 KiB 544.86 KiB 523.28 KiB
a4ee85b 20.76 KiB 434.92 KiB 414.16 KiB
4d3df92 22.85 KiB 413.45 KiB 390.60 KiB
e8b11f8 22.85 KiB 411.92 KiB 389.07 KiB
6161814 21.58 KiB 713.97 KiB 692.39 KiB
742d4b6 21.58 KiB 546.19 KiB 524.61 KiB
2124551 22.85 KiB 411.69 KiB 388.84 KiB
ecd9ecd 20.76 KiB 420.23 KiB 399.47 KiB

Previous results on branch: chore/move-sentryfeedback-startup

Startup times

Revision Plain With Sentry Diff
de31158 1214.08 ms 1231.51 ms 17.43 ms

App size

Revision Plain With Sentry Diff
de31158 22.30 KiB 730.79 KiB 708.49 KiB

@brustolin brustolin changed the base branch from main to fix/load-integrations November 15, 2024 14:44
@armcknight
Copy link
Member

Hmm, the reason I put this here is that I saw defaultIntegrations (which calls defaultIntegrationClasses) called in SentryOptions init, and assumed that was the only time that list would be accessed, which is too early to determine if user feedback is configured or not. But I see now that defaultIntegrationClasses is called again in SentrySDK.startWithOptions, which will have the options configured by then.

In what case would the version of defaultIntegrationClasses be used that is accessed here instead of the later result? Is it always overwritten, or just in some cases?

_integrations = sentry_defaultIntegrations();

vs

NSArray<Class> *defaultIntegrations = SentryOptions.defaultIntegrationClasses;

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