fix(sentry-java): Contexts belong on the Scope

[bruno-garcia]

This is work to get sentry-core where it can be the base of sentry-java and its integrations.
The long term goal is to merge this repo into sentry-java instead on a orphan branch and keep the 1.7 work on a branch for bug fixing.

Still WIP. Contexts needs a deep copy of the known types.
If it's App, Device etc, we need to call clone() instead of just bringing the reference over.

This was done here:

https://github.com/getsentry/sentry-dotnet-protocol/blob/afd65b92ed7122a628dc3631711b407f56e9a8a8/src/Sentry.Protocol/Context/Contexts.cs#L60

[metlos]

Also, if I'm not mistaken, this still doesn't make this feature-equivalent to Sentry .NET where the scope also includes the Request as a separate field from the Contexts.

[bruno-garcia]

In dotnet and the docs we mention requests is part of the scope, not the contexts.
So this fix is unrelated to requests. We still need to add the class and a property to scope and event.

[marandaneto]

No description provided. :(

Why do we need that? is it for GA as well?

[bruno-garcia]

There's no description because I thought the title was self explanatory.
It's a fix because the Contexts belong on the scope. Right not it's only on the event so we can't do Sentry.configureScope(s -> s.Context["bla"] = ...)

Here's python:

https://github.com/getsentry/sentry-python/blob/e1b55a1b8f44e626309b33a9f1561a0c6808a214/sentry_sdk/scope.py#L183-L197

C#:

https://github.com/getsentry/sentry-dotnet-protocol/blob/7cde43e194acab89ccc8c23b76a72b7a72d99c4b/src/Sentry.Protocol/BaseScope.cs#L90

[marandaneto]

LGTM, but CI is not happy, need to fix it.

[marandaneto]

There's no description because I thought the title was self explanatory.
It's a fix because the Contexts belong on the scope. Right not it's only on the event so we can't do Sentry.configureScope(s -> s.Context["bla"] = ...)

Here's python:

https://github.com/getsentry/sentry-python/blob/e1b55a1b8f44e626309b33a9f1561a0c6808a214/sentry_sdk/scope.py#L183-L197

C#:

https://github.com/getsentry/sentry-dotnet-protocol/blob/7cde43e194acab89ccc8c23b76a72b7a72d99c4b/src/Sentry.Protocol/BaseScope.cs#L90

ok got it, but we already talked about it a few times in the past and also recently with @HazAT, it was never needed, at least for GA, why do we need now?

Another thing, if Contexts may be cloned, App/Browser/Device/OperatingSystem/SentryRuntime/Gpu need to implements Cloneable and override the clone method and do the magic as well, otherwise, it will be a shallow copy and we don't want that, right?

[bruno-garcia]

ok got it, but we already talked about it a few times in the past and also recently with @HazAT, it was never needed, at least for GA, why do we need now?

It's not needed for GA, this can stay on a branch that we use for the sentry-java experiments of @metlos then to avoid breaking something now that we're trying to go GA

Another thing, if Contexts may be cloned, App/Browser/Device/OperatingSystem/SentryRuntime/Gpu need to implements Cloneable and override the clone method and do the magic as well, otherwise, it will be a shallow copy and we don't want that, right?

Yeah we need to do clone on all of them, since those might be changed on the new scope and it's typed stuff, known types.

So this PR can't be merged until we do that. Also doesn't need to be merged at all until sentry-core gets closer to becoming new sentry-java dependency. The branch can be used instead.

[marandaneto]

ok got it, but we already talked about it a few times in the past and also recently with @HazAT, it was never needed, at least for GA, why do we need now?

It's not needed for GA, this can stay on a branch that we use for the sentry-java experiments of @metlos then to avoid breaking something now that we're trying to go GA

Another thing, if Contexts may be cloned, App/Browser/Device/OperatingSystem/SentryRuntime/Gpu need to implements Cloneable and override the clone method and do the magic as well, otherwise, it will be a shallow copy and we don't want that, right?

Yeah we need to do clone on all of them, since those might be changed on the new scope and it's typed stuff, known types.

So this PR can't be merged until we do that. Also doesn't need to be merged at all until sentry-core gets closer to becoming new sentry-java dependency. The branch can be used instead.

agreed, cool stuff, we can work on it later on then!

[bruno-garcia]

Not sure we want to expose the whole Contexts, but rather just add items to it