Add device and OS attributes to logs #4493
Conversation
adinauer
requested review from
lcian,
markushi,
romtsn and
stefanosiano
as code owners
|
Performance metrics 🚀
|
| return event; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
An alternative to going through EventProcessor would be something very similar to it, like a LogProcessor so I opted for simply extending EventProcessor instead.
We could also offer hooks but I didn't want to hack that in, so I went this route and we can transform it into a better hook API in the future.
| * @return the event itself, a mutated SentryLogEvent or null | ||
| */ | ||
| @Nullable | ||
| default SentryLogEvent process(@NotNull SentryLogEvent event) { |
There was a problem hiding this comment.
Decided not to add Hint here since we decided to not have it for logs.beforeSend either. LMK if you think it's needed.
| event.getContexts().put(osNameKey, currentOS); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
so I'm thinking querying DeviceInfoUtil is probably too much work for this, shall we maybe just directly use the const values for brand, model and family from android.os.Build? Same for the OS name and version, those are also const values. Just trying to minimize impact since logs can happen frequent and originate from the main thread.
There was a problem hiding this comment.
Updated, introduced a LazyEvaluator so we don't run a regex for the family on each log emitted.
| * @return the event itself, a mutated SentryLogEvent or null | ||
| */ | ||
| @Nullable | ||
| default SentryLogEvent process(@NotNull SentryLogEvent event) { |
There was a problem hiding this comment.
Can we also add an empty override to MainEventProcessor? I recall there used to be problems in dotnet/unity due to absence of proper desugaring in some versions, so the default interface methods have caused crashes.
romtsn
left a comment
romtsn
left a comment
There was a problem hiding this comment.
a couple of things to address, but LGTM otherwise!
|
@romtsn What's the best way for HybridSDKs to use this feature when capturing logs on the hybrid layer? |
📜 Description
Add device and OS attributes to logs:
os.nameos.devicedevice.branddevice.modeldevice.family💡 Motivation and Context
Closes #4460
💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps