Hubs/Scopes Merge 22 - Combine global, isolation and current scope

[adinauer]

#skip-changelog

📜 Description

When creating envelopes/events/transactions/... we now have to use all three scopes (global, isolation and current).

TODO: Breadcrumbs currently do not maintain insertion order across scopes. This will be adressed in a future PR.

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

• 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]

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 1a804ca

[adinauer]

CombinedContextsView isn't totally necessary to have but it avoids constantly creating copies of Contexts.

[adinauer]

We should use CombinedContextsView as ScopeType.COMBINED for cross platform SDKs should get all values and calls setDevice and setOperatingSystem so handing back a copy that isn't mutable (or rather loses updates) isn't ideal. See #3375

[adinauer]

Not quite sure here.

[adinauer]

We're now returning a new CombinedScopeView with forked isolation and current scope. See #3375

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	411.22 ms	497.16 ms	85.94 ms
Size	1.70 MiB	2.28 MiB	595.91 KiB

Previous results on branch: feat/hsm-22-combine-scopes

Startup times

Revision	Plain	With Sentry	Diff
be47198	360.88 ms	426.53 ms	65.66 ms

App size

Revision	Plain	With Sentry	Diff
be47198	1.70 MiB	2.28 MiB	595.91 KiB

[markushi]

I think we need some propagation or copy-on-write mechanism for all Scope related fields.

E.g.

val user = Sentry.getUser() // returns global scope user, as it's unset
Sentry.setUser(user) // now default scope is set to the user

user.email = "[email protected]" // both global and default scope is written to

[adinauer]

Hmm this should only happen if someone calls .setUser on global scope so an actual User instance is returned from CombinedScopeView and then pass that into .setUser that goes to current or isolation scope.

Can we ignore this edge case for now and address it, when it actually happens for a customer?

One solution would be to have something like CombinedContextsView for user and other fields.

[adinauer]

Here's an example that should avoid the problem:

sentry-java/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java

Lines 87 to 98 in ab1c3a6

	@Nullable User user = scope.getUser();
	if (user == null) {
	user = new User();
	scope.setUser(user);
	}
	if (user.getId() == null) {
	try {
	user.setId(Installation.id(context));
	} catch (RuntimeException e) {
	logger.log(SentryLevel.ERROR, "Could not retrieve installation ID", e);
	}
	}

[lbloder]

The two comments within Scopes.getGlobalScope() could be removed:

sentry-java/sentry/src/main/java/io/sentry/Scopes.java

Lines 584 to 588 in a474402

	public @NotNull IScope getGlobalScope() {
	// TODO should be:
	return Sentry.getGlobalScope();
	// return scope;
	}

LGTM otherwise