Another thing I have in mind if we should implement Cloneable interface and use clone method at all. If we go instead with our own custom Cloneable<T> interface we could let library users to implement it in their custom objects that are put to Contexts and we wouldn't be limited to clone only our well known objects. It's also generally not recommended to use implement Java Cloneable interfaces.

@marandaneto regarding actually using Scope#contexts - your and @bruno-garcia guidance is welcome here. I believe we should use it in SentryClient#applyScope like:

if (event.getContexts().isEmpty()) {
  event.setContexts(scope.getContexts());
}

but I might have missed something from the bigger picture.

Another thing I have in mind if we should implement Cloneable interface and use clone method at all. If we go instead with our own custom Cloneable<T> interface we could let library users to implement it in their custom objects that are put to Contexts and we wouldn't be limited to clone only our well known objects. It's also generally not recommended to use implement Java Cloneable interfaces.

that sounds a good idea, as I said, the Cloneable is just a marker interface anyway, as we're cloning the list/map etc manually.
if you create a Cloneable<T>, we'd need to fix all the clone methods and copy field by field.

@marandaneto regarding actually using Scope#contexts - your and @bruno-garcia guidance is welcome here. I believe we should use it in SentryClient#applyScope like:

if (event.getContexts().isEmpty()) {
  event.setContexts(scope.getContexts());
}

but I might have missed something from the bigger picture.

yep, this would be moved either to SentryClient or MainEventProcessor, but a note here is, in order to read some device context, we need to have the Android Context and that's why we have this DefaultAndroidEventProcessor.
So right now DefaultAndroidEventProcessor takes an event and mutates it in there, I believe this should change now for something like DefaultAndroidEventProcessor returns a Map with the things we need from Android and then either SentryClient or MainEventProcessor set to the event respecting the precedence of the event or scope

Please be aware:

if (event.getContexts().isEmpty()) {
  event.setContexts(scope.getContexts());
}

The values copied must be cloned. Otherwise while the event is queued for submission, the scope by be changed, affecting that event. The only references they'd share is any user-defined one, which we were not able to clone with our own deep cloning strategy (i.e Reference types in Extra and Contexts)

There's a PR to this PR: maciejwalkowiak#1

@marandaneto regarding actually using Scope#contexts - your and @bruno-garcia guidance is welcome here. I believe we should use it in SentryClient#applyScope like:

if (event.getContexts().isEmpty()) {
  event.setContexts(scope.getContexts());
}

but I might have missed something from the bigger picture.

yep, this would be moved either to SentryClient or MainEventProcessor, but a note here is, in order to read some device context, we need to have the Android Context and that's why we have this DefaultAndroidEventProcessor.
So right now DefaultAndroidEventProcessor takes an event and mutates it in there, I believe this should change now for something like DefaultAndroidEventProcessor returns a Map with the things we need from Android and then either SentryClient or MainEventProcessor set to the event respecting the precedence of the event or scope

What if we add this to SentryClient#applyScope before preprocessors do the work:

for (Map.Entry<String, Object> entry : scope.getContexts().clone().entrySet()) {
  if (!event.getContexts().containsKey(entry.getKey())) {
    event.getContexts().put(entry.getKey(), entry.getValue());
  }
}

DefaultAndroidEventProcessor will set Device and Operating system only if these properties are missing. This would mean that the contexts on SentryEvent take precedence over the contexts on Scope which takes precedence over what is applied in DefaultAndroidEventProcessor. What do you think about this logic @marandaneto?

DefaultAndroidEventProcessor will set Device and Operating system only if these properties are missing. This would mean that the contexts on SentryEvent take precedence over the contexts on Scope which takes precedence over what is applied in DefaultAndroidEventProcessor. What do you think about this logic @marandaneto?

Makes sense to me.

Approving but let's see what @marandaneto has to say

its LGTM for me too, well done @maciejwalkowiak