The `CrashedLastRun`, `EnableScopeSync` and `ScopeObserver` options should all be internal

[jamescrosswell]

See #4529 (comment).

These options in particular used to enable the SDK for Unity to be able to overwrite them. These were not necessarily intended to be used by actual users. Since we resolved this by having the internals visible to the SDK for Unity, this is no longer a requirement.

We should make these internal in the next major release.

[linear]

NET-449 Should CrashedLastRun, `EnableScopeSync` and `ScopeObserver` be internal?

[Flash0ver]

We are thinking about removing SentryOptions.EnableScopeSync from the public API surface area,
and instead have it implicitly enabled by user code setting a SentryOptions.ScopeObserver,
or our Native/Android/iOS implementations setting a respective platform IScopeObserver,
or an aggregate of both a User and Sentry IScopeObserver.

[Flash0ver]

Discussed again: We keep all options, but will add wrappers/combiners/aggregators for both CrashedLastRun and ScopeObserver, so that user-provided instances/delegates will not overwrite the ones that we set.

[jamescrosswell]

@Flash0ver after looking at this again, I think we misunderstood things the first time around.

Scope observers

It's not obvious, but the existing CocoaScopeObserver and AndroidScopeObserver implementations do preserve/chain any user defined scope observers here:

sentry-dotnet/src/Sentry/Platforms/Cocoa/CocoaScopeObserver.cs

Lines 16 to 18 in 438ae83

	// Chain any previous observer, but guard against circular reference.
	var observer = options.ScopeObserver;
	_innerObserver = observer is CocoaScopeObserver ? null : observer;

sentry-dotnet/src/Sentry/Platforms/Android/AndroidScopeObserver.cs

Lines 16 to 18 in 438ae83

	// Chain any previous observer, but guard against circular reference.
	var observer = options.ScopeObserver;
	_innerObserver = observer is AndroidScopeObserver ? null : observer;

The NativeScopeObserver doesn't but I'm not sure we would want it to anyway... since this can be active at the same time as either the Cocoa or Android observers (meaning if it also chained, we'd end up with potentially multiple calls to the user defined observer, if they had one). It might be possible to avoid that by removing the responsibility of chaining from the individual observer implementations and putting that instead in a CopositeScopeObserver... which would be what would actually sit on the options. I don't know that we have an actual problem that needs solving here at the moment though, so I'm inclined to leave things as they are for the time being (we probably have more pressing work to do}.

CrashedLastRun

Looking at the implementations that we have for the various native SDKs, I don't think we want the user to be able to replace/override the logic we have for each platform here... and there's no logical way to "chain" these and have both our logic and the user's logic run, since the purpose of this delegate is to return a boolean that indicates whether the last run crashed or not.

That does raise the question of whether this property should be public in the options... I suspect it should not. We could look at marking it obsolete now and making it internal in a future major release.

[bitsandfoxes]

IIRC, these options in particular used to enable the SDK for Unity to be able to overwrite them. These were not necessarily intended to be used by actual users. Since we resolved this by having the internals visible to the SDK for Unity, this is no longer a requirement.

[jamescrosswell]

Cool, thanks @bitsandfoxes . Let's make them internal in a future major release then 👍🏻