forcedotcom / SalesforceMobileSDK-Android

Android SDK for Salesforce
Other
341 stars 388 forks source link

@W-16580498: [Android] Hook to propagate MSDK logs to other systems #2635

Closed JohnsonEricAtSalesforce closed 1 week ago

JohnsonEricAtSalesforce commented 1 week ago

🎸 Ready For Review! 🥁

This adds a new concept of "log receivers" to the SDK's existing logging infrastructure, plus a "factory" which can be registered by the app to create and register these receivers to named components in the logging flow. The factory object could be a bridge to the app's dependency injection framework, such as Hilt.

I tried this out in the template apps and the overall pattern gave the app what I expected. I marked this ready for review with the anticipation we might have to discuss the naming convention a bit. I thought over names such as "adapter", "destination", "target" or "handler" before settling on "receiver". I though this was a good semantic since the object only has to "receive" the log entry - What happens after that is entirely up to the implementation and our interface names don't provide implementation bias.

I added some logic to ensure the app can also choose their own handling for our notion of component names. In my template app, I tried a few patterns such a using a single logger that didn't use the provided component name or concatenated it to the message by mapping instances of receiver objects to component name. We can add some sample code for this into the MSDK docs.

As always, thanks for reading!

mobilesdk-bot commented 1 week ago
1 Warning
:warning: No Lint Results.

Tests results for SalesforceAnalytics

Generated by :no_entry_sign: Danger

JohnsonEricAtSalesforce commented 1 week ago

@wmathurin - Take a look at https://github.com/forcedotcom/SalesforceMobileSDK-Android/pull/2635/commits/42de28d146acafeead83ccde45a12e174c3139d9 . Thanks!

mobilesdk-bot commented 1 week ago
1 Warning
:warning: No Lint Results.

Tests results for SalesforceHybrid

Generated by :no_entry_sign: Danger

mobilesdk-bot commented 1 week ago
1 Warning
:warning: No Lint Results.

Tests results for SalesforceSDK

Generated by :no_entry_sign: Danger

mobilesdk-bot commented 1 week ago
1 Warning
:warning: No Lint Results.

Tests results for SalesforceReact

Generated by :no_entry_sign: Danger

JohnsonEricAtSalesforce commented 1 week ago

There's a lot of ways the app could use this new logging feature, but for a quick example I used a couple of anonymous adapters and ensured I could work with both level and component name. Obviously, this is a pretty trivial example and wouldn't add value in reality, but it shows where the app can inject any and all the logic they wish. Here's a screenshot!

Screenshot 2024-11-08 at 11 39 13
wmathurin commented 1 week ago

Maybe instead of a screenshot, you could have the example code in the description of the PR

setLogReceiverFactory(object:SalesforceLogReceiverFactory {
...
})
brandonpage commented 1 week ago

There's a lot of ways the app could use this new logging feature, but for a quick example I used a couple of anonymous adapters and ensured I could work with both level and component name. Obviously, this is a pretty trivial example and wouldn't add value in reality, but it shows where the app can inject any and all the logic they wish. Here's a screenshot!

Screenshot 2024-11-08 at 11 39 13

This could also be implemented with a single code block (like if interfaces supported JVM overloads) by assigning one to the other:

override fun receive(level: Level, tag: String, message: String) = receive(level, tag, message, throwable = null)
override fun receive(level: Level, tag: String, message: String, throwable: Throwable?) {
   // single impl
}