React-native: It's time to upgrade RCTDefaultLogFunction to use modern os_log API

[LeoNatan]

Currently, the default logger outputs directly to stderr. This renders log collection using Apple's log stream API broken, as stderr output is not collected by the Apple logging system. For example, in Detox, we collect the device logs when running a test, but anything logged from the JS is not collected.

It's time to upgrade this behavior by using the Apple Unified Logging API. As an added bonus, the system logging API has log levels similar to those of RCTLogLevel, which can be mapped to the system log levels.

I am willing to write a PR for this once there is an agreement on the solution.

Thanks

React Native version: 0.61.2 -> master

Steps To Reproduce

1. console.log() in JavaScript
2. Look for the log in the Console.app or using log stream command

Describe what you expected to happen:
The logged text should appear in the Console.app

[LeoNatan]

The log source input variable can be used to output native and JavaScript logs into different log categories, for better filtering capabilities.

https://developer.apple.com/documentation/os/1643744-os_log_create?language=objc

[PeteTheHeat]

Migrating to Apple's unified os_log API seems like a great idea. It's iOS 10+, which won't be an issue at FB, as we've stopped supporting iOS9, is that also true of OSS?

My only concerns:

• The privacy "feature" which considers dynamic strings and objects private. Will this be an issue? It seems everything we'd want to log is technically dynamic.
• This appears to change the behaviour of what is being logged in production builds, correct? fprintf doesn't log in prod builds, while os_log will? My concern here is that some apps are logging a bunch of garbage which is currently stripped from prod builds, but now wouldn't be.

[LeoNatan]

Thank you for your reply.

The public podspec is also iOS 10.0 and above:

s.platforms              = { :ios => "10.0", :tvos => "10.0" }

To answer your concerns:

• The issue of public and private strings can be sidestepped completely by using a %{public}s modifier, and then printing the log line as is. This is how we log to the system: https://github.com/wix/DTXLoggingInfra/blob/fdce2f996a40682f69464cd0893d734e862f48cd/DTXLogging.m#L24
• The modern Apple logging system is designed to be performant in production scenarios. It automatically discards log entries depending on configuration and buffers. More information here: https://developer.apple.com/documentation/os/os_log_type_t?language=objc

If we set the default log level to debug, there would be no impact on performance.

In the code, I already see

#if RCT_DEBUG
static const RCTLogLevel RCTDefaultLogThreshold = (RCTLogLevel)(RCTLogLevelInfo - 1);
#else
static const RCTLogLevel RCTDefaultLogThreshold = RCTLogLevelError;
#endif

But I think this is unnecessary, or at least should be set to pass all RN logs to the system and let it decide what to do. As an example, Detox configures the logging system to capture debug and info messages; if a Detox user tests a release build of their app, the log entries should be allowed to reach the logging system, which will a. in case of a Detox test, collect all log entries, b. throw the non-error log entries on a production device.

[PeteTheHeat]

Sounds great, thanks for the detailed explanation!

Is there anything else you'd want to know from our side? If not, you can go ahead and draft up a PR and I'll review/merge it.

[LeoNatan]

Great, since we have an agreement, I'll write and submit a PR soon. Thanks

[LeoNatan]

27892

Thanks

[LeoNatan]

Thank you!