Fix: Load integration from same binary

[brustolin]

📜 Description

We use the class name to load integrations, and this could lead to a wrong integration being loaded if sentry is duplicated inside a project.

💡 Motivation and Context

💚 How did you test it?

📝 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

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1235.13 ms	1259.86 ms	24.73 ms
Size	22.30 KiB	747.70 KiB	725.40 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
f0ce81c	1231.06 ms	1244.79 ms	13.73 ms
075a044	1237.26 ms	1255.36 ms	18.10 ms
750f4d7	1213.53 ms	1231.71 ms	18.18 ms
d61b939	1238.61 ms	1240.08 ms	1.47 ms
1ce939e	1216.79 ms	1242.38 ms	25.59 ms
4386045	1225.67 ms	1244.90 ms	19.23 ms
e072ad1	1221.35 ms	1237.48 ms	16.13 ms
adca747	1250.82 ms	1267.57 ms	16.76 ms
46c6025	1213.87 ms	1253.06 ms	39.19 ms
d3abae0	1200.36 ms	1224.22 ms	23.87 ms

App size

Revision	Plain	With Sentry	Diff
f0ce81c	21.58 KiB	418.33 KiB	396.75 KiB
075a044	20.76 KiB	420.41 KiB	399.65 KiB
750f4d7	21.90 KiB	708.34 KiB	686.44 KiB
d61b939	22.85 KiB	407.63 KiB	384.78 KiB
1ce939e	22.85 KiB	412.98 KiB	390.13 KiB
4386045	22.84 KiB	403.07 KiB	380.23 KiB
e072ad1	21.58 KiB	625.82 KiB	604.24 KiB
adca747	20.76 KiB	401.36 KiB	380.60 KiB
46c6025	22.85 KiB	408.84 KiB	385.99 KiB
d3abae0	20.76 KiB	434.92 KiB	414.16 KiB

Previous results on branch: fix/load-integrations

Startup times

Revision	Plain	With Sentry	Diff
d47fa9b	1227.55 ms	1241.30 ms	13.75 ms
74f63e4	1241.04 ms	1260.04 ms	19.00 ms
b1311ae	1244.96 ms	1263.10 ms	18.14 ms
bbd4c55	1237.27 ms	1253.81 ms	16.55 ms
44cf191	1230.69 ms	1247.63 ms	16.94 ms

App size

Revision	Plain	With Sentry	Diff
d47fa9b	22.30 KiB	747.62 KiB	725.31 KiB
74f63e4	22.30 KiB	747.71 KiB	725.41 KiB
b1311ae	22.30 KiB	747.68 KiB	725.37 KiB
bbd4c55	22.30 KiB	730.95 KiB	708.64 KiB
44cf191	22.30 KiB	730.92 KiB	708.62 KiB

[codecov]

Codecov Report

Attention: Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 90.940%. Comparing base (37fd7a2) to head (15c4a4e).
Report is 15 commits behind head on main.

Files with missing lines	Patch %	Lines
Sources/Sentry/SentrySDK.m	80.000%	2 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #4541       +/-   ##
=============================================
- Coverage   91.587%   90.940%   -0.648%     
=============================================
  Files          615       617        +2     
  Lines        69933     70600      +667     
  Branches     25058     25237      +179     
=============================================
+ Hits         64050     64204      +154     
- Misses        5790      6304      +514     
+ Partials        93        92        -1     

Files with missing lines	Coverage Δ
Sources/Sentry/SentryOptions.m	98.698% <100.000%> (ø)
Sources/Sentry/SentrySDK.m	87.650% <80.000%> (-0.276%)	⬇️

... and 39 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 37fd7a2...15c4a4e. Read the comment docs.

---- 🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[armcknight]

Thanks for removing the string-based logic, that has bugged me for a long time. I have a comment on how the method is being declared.

[philipphofmann]

It would be great to have a test failing if we change the code in the SentrySDK.m back to the old implementation.

[brustolin]

It would be great to have a test failing if we change the code in the SentrySDK.m back to the old implementation.

I considered it, but I’m not sure how to enforce it. It seems like a sketch test.

[philipphofmann]

It would be great to have a test failing if we change the code in the SentrySDK.m back to the old implementation.

I considered it, but I’m not sure how to enforce it. It seems like a sketch test.

What about using the sample app you created and adding a UI or integration test?

[philipphofmann]

Thanks a lot for adding the sample to validate that this doesn't happen again anymore. I added a few comments mostly for improving the readability of the code.

[philipphofmann]

m: I would be a bit more specific and explain roughly what happened with SPM and the prebuilt binary.

[brustolin]

// We cannot load the class itself with NSClassFromString because doing so may load a class
// that was duplicated in another module, leading to undefined behavior.

IMO this is descriptive enough.

[brustolin]

Feel free to add a ```suggestion for an alternative

[philipphofmann]

m: gameOnly is a bit confusing to me. Ideally this would use the artifact built here

sentry-cocoa/.github/workflows/build.yml

Lines 86 to 87 in 166d67a

	build-xcframework:
	name: Build XCFramework

but if I'm not mistaken this workflow would need to wait until the build workflow is finished, and that would slow down all UI tests. We could add another workflow called something ui-tests-with-xcframework, and move the duplication test there.

[brustolin]

Let's wait to see if this will become a recurring thing, which I dont think so.
For now having the SDK compiled for the test itself works fine.