ref: only declare conformance on SentryBaseIntegration superclass

[armcknight]

I am working on converting SentryCrashWrapper from a singleton to fully dependency-injected (#2942). This involves changing SentryBaseIntegration.init to accept it as an argument, since it needs to use it in its implementation.

I noticed that all of our integration classes both subclass SentryBaseIntegration and conform to SentryIntegrationProtocol at their declarations. I lifted the protocol conformance declaration out of the subclasses and to the superclass, where it applies to any future subclass for free.

I also converted all the generic id<SentryIntegrationProtocol> to polymorphic declarations of SentryBaseIntegration * instead, like in array types. It would only really make sense to use the type-erased version if customers are able to create their own integrations conforming to SentryIntegrationProtocol, but as far as I can tell we don't support that kind of plug-in architecture. Let me know if I've missed anything here and am wrong about this.

Finally, I changed the implicit header import of SentryOptions from SentryIntegrationProtocol.h to a forward class declaration, and added the necessary missing imports of SentryOptions in the necessary places.

#skip-changelog

[armcknight]

This was an especially important change. Because the init is defined in SentryBaseIntegration and not SentryIntegrationProtocol, when I changed it from SentryBaseIntegration.init to SentryBaseIntegration.initWithCrashWrapper:, this line didn't break compilation. It would have gone on to crash at runtime. This is an unsafe code sector in general, so this makes it a little better.

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1248.06 ms	1259.40 ms	11.34 ms
Size	20.76 KiB	432.78 KiB	412.02 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
83d2d84	1211.31 ms	1227.34 ms	16.03 ms
455619d	1231.40 ms	1237.70 ms	6.30 ms
11ccc16	1206.45 ms	1225.60 ms	19.15 ms
b2f82fa	1236.94 ms	1262.86 ms	25.92 ms
ecd9ecd	1191.76 ms	1216.92 ms	25.16 ms
ea73af6	1241.69 ms	1252.96 ms	11.27 ms
aa27ac0	1214.02 ms	1227.42 ms	13.40 ms
cf724da	1226.61 ms	1235.70 ms	9.09 ms
ecd9ecd	1214.16 ms	1232.59 ms	18.43 ms
46f5eb8	1212.27 ms	1231.42 ms	19.15 ms

App size

Revision	Plain	With Sentry	Diff
83d2d84	20.76 KiB	419.66 KiB	398.90 KiB
455619d	20.76 KiB	432.87 KiB	412.11 KiB
11ccc16	20.76 KiB	431.71 KiB	410.94 KiB
b2f82fa	20.76 KiB	419.62 KiB	398.86 KiB
ecd9ecd	20.76 KiB	420.23 KiB	399.47 KiB
ea73af6	20.76 KiB	425.76 KiB	404.99 KiB
aa27ac0	20.76 KiB	432.21 KiB	411.45 KiB
cf724da	20.76 KiB	430.52 KiB	409.76 KiB
ecd9ecd	20.76 KiB	420.23 KiB	399.47 KiB
46f5eb8	20.76 KiB	432.37 KiB	411.61 KiB

Previous results on branch: armcknight/ref/centralize-SentryIntegrationProtocol-conformance

Startup times

Revision	Plain	With Sentry	Diff
24bf3bd	1211.39 ms	1231.12 ms	19.73 ms

App size

Revision	Plain	With Sentry	Diff
24bf3bd	20.76 KiB	432.79 KiB	412.03 KiB

[brustolin]

I think is nice to make SentryBaseIntegration implements SentryIntegrationProtocol and remove the protocol from our integrations, but I think we should continue to use id in the arrays and when interacting with integrations.
And Im pretty sure this is a breaking change, we dont see many customers using their own integrations, but its easy to do it.

[armcknight]

After talking with @brustolin , it sounds like it is indeed a breaking change if we don't allow customers to supply the names of custom integrations they might create.

My question is, how do custom integrations work? Is it actually a supported feature? I couldn't find documentation about it.

If it is not supposed to be supported, we should consider removing the ability. While we're at it, we should convert the SentryOptions.integrations string array into something that can supply a nonarbitrary set of integrations; something like an enum or option set.

[brustolin]

something like an enum or option set

I believe SentryOptions flags should be enough to enable/disable integrations. If we have an integration not controlled by a flag we should add it.

[philipphofmann]

Custom integrations are an undocumented public feature. The mobile SDKs inherited enabling and disabling features via the list of integrations from languages like JavaScript and Python. On Cocoa, the code for removing one single integration is tedious, so we have simple feature flags for almost all features. I have no clue if somebody is writing custom integration, but our users could, and therefore changing them would be a breaking change. I don't have anything making them internal in the next major.

[armcknight]

It sounds like this PR will not be merged until we are preparing the next major version. I'll close it and link to it from #2946 so we can revive it later.