Create User and Breadcrumb data classes from a Map

[marandaneto]

Often Hybrid SDKs have to create data classes out of a Map, See:

https://github.com/getsentry/sentry-dart/blob/b8b9b7b2608c35e4a3f7340ae9d2f0e9c06d984f/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt#L272-L286

This code is repeated for Android and iOS, on Flutter, RN, and others.
Ideally, Android and iOS would expose extension methods for this, such as:

User.fromMap(Map<String, Any> user).
Breadcrumb.fromMap(Map<String, Any> crumb).

Build in Core SDKs

Preview Give feedback

1. Create User and Breadcrumb data classes from a dictionary sentry-cocoa#2687
   Effort: Small Impact: Small Platform: Cocoa Status: Backlog +4
2. Create User and Breadcrumb data classes from a Map sentry-java#2534
   Platform: Android Platform: Java +2

Use in Hybrid SDKs

Preview Give feedback

1. Flutter
2. Create User and Breadcrumb data classes from a Map sentry-react-native#4373
   Platform: React-Native +1

[marandaneto]

The user.geo field is missing on Android and iOS and has to be added in order to be able to sync the scope with this additional data.

[krystofwoldrich]

The hybrid SDKs are blocked because this need to be implemented on the native SDKs first.

[marandaneto]

The hybrid SDKs are blocked because this need to be implemented on the native SDKs first.

Please raise issues on the native SDKs to make it possible and move the status to blocked.

[marandaneto]

@denrase we can add those methods directly in the native SDKs and use them here.

[brustolin]

IMO at first glance this looks nice, to create this once in the cocoa SDK and every other project make use of it.
But tbh, once every other SDK has its own code you don't change it anymore.
And by implementing this on the hybrid SDK, you will receive a compilation error if you trying to access invalid property.
If we do this in the cocoa side, the hybrid SDK will pass a dictionary and any invalid property (removed or renamed) will be swallowed and you wont know about it.

[denrase]

@marandaneto Do we still want to do this, as @brustolin has some reservations, at least for cocoa. On sentry-java we have the unknown dictionary where we store all properties that are not known.

[marandaneto]

@brustolin both ways have trade-offs, the difference is that if something is wrong (something is missing, etc), you fix it only once on Cocoa, rather than multiple Hybrid SDKs, that alone already pays off IMO.
IMO we should still do it.
@brustolin double check with @philipphofmann

[brustolin]

Just to be clear, Im not against doing it. IMO is better for the hybrid SDK to no include this unnecessary extra dependency on the Core SDKs.

If you really think this is the way to go I wont oppose it, @denrase can definitely include this in the cocoa SDK, no problem.

If I remember correctly @philipphofmann has the same opinion, we already talked about this issue.
But he can speak for himself next week.

[marandaneto]

Native SDKs should not swallow unknown properties, that's a feature that is implemented at least on Java/Android and some other SDKs.
See Java
Everything would still be sent, does Cocoa have something similar?

[marandaneto]

Let's put this on hold till @philipphofmann is back, so we can set up a meeting and talk about the pros and cons.

[brustolin]

Native SDKs should not swallow unknown properties

We would need to iterate over every key in the dictionary to see if is a valid key.
Definitely not a good approach.

[krystofwoldrich]

IMO is better for the hybrid SDK to no include this unnecessary extra dependency on the Core SDKs.

I wouldn't say it's extra dependency, we depend on the Core SDKs method anyway and if something changes we need to fix it in the Hybrid SDKs. Like this, it could be fixed only once as it was mentioned.

[marandaneto]

@denrase I've talked to the Native SDK folks and they are fine with it, a few things to point out.

• We can consider adding to PrivateSentrySDKOnly instead of being public to everyone.
• We can consider adding the unknown map to those data classes so unknown fields are not lost during serialziation/deserialization roundtrip, similarly to Android.
• We can consider compiling those methods only if an ifdef is set, such as #if os(Hybrid) (nice to have, most likely not needed).

[brustolin]

Use the "/Sentry/include/HybridPublic" directory to create new headers that expose this new features for hybrid SDKs.

[kahest]

closing as completed, remaining task is tracked in RN repo