Skip to content

Use Apple unified logging API (os_log)#27892

Closed
LeoNatan wants to merge 1 commit into
facebook:masterfrom
LeoNatan:modernize-rn-logger
Closed

Use Apple unified logging API (os_log)#27892
LeoNatan wants to merge 1 commit into
facebook:masterfrom
LeoNatan:modernize-rn-logger

Conversation

@LeoNatan
Copy link
Copy Markdown
Contributor

Summary

As discussed in #27863, the following changes were made to modernize the internal default logging function:

  • RCTDefaultLogThreshold is now set to RCTLogLevelTrace in both release and debug builds—the Apple logging system will discard uncollected log entires, while allowing for collection when needed
  • RCTLogLevel is translated to the appropriate log type
  • The log subsystem is "com.facebook.react.log"
  • RCTLogSource translates to the appropriate category ("native"/"javascript")
  • Log the provided message using os_log_with_type

Closes #27863

Changelog

[iOS] [Changed] - Use Apple unified logging API (os_log)

Test Plan

Ran a test app in the iOS simulator, and verified that logs are correctly displayed in Console.app as well as using the following command:

/usr/bin/xcrun simctl spawn booted log stream --level debug --style compact --predicate 'process=="ReactNativeTesterApp" && subsystem=="com.facebook.react.log"'

@LeoNatan LeoNatan requested a review from shergin as a code owner January 28, 2020 19:16
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 28, 2020
@react-native-bot react-native-bot added the Platform: iOS iOS applications. label Jan 28, 2020
@LeoNatan
Copy link
Copy Markdown
Contributor Author

Not sure what is going on with the tests, but I don't think it's related to my change.

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PeteTheHeat has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @LeoNatan in f501ed6.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Jan 28, 2020
osdnk pushed a commit to osdnk/react-native that referenced this pull request Mar 9, 2020
Summary:
As discussed in facebook#27863, the following changes were made to modernize the internal default logging function:

- `RCTDefaultLogThreshold` is now set to `RCTLogLevelTrace` in both release and debug builds—the Apple logging system will discard uncollected log entires, while allowing for collection when needed
- `RCTLogLevel` is translated to the appropriate log type
- The log subsystem is "com.facebook.react.log"
- `RCTLogSource` translates to the appropriate category ("native"/"javascript")
- Log the provided message using `os_log_with_type`

Closes facebook#27863

## Changelog

[iOS] [Changed] - Use Apple unified logging API (os_log)
Pull Request resolved: facebook#27892

Test Plan:
## From Original PR

Ran a test app in the iOS simulator, and verified that logs are correctly displayed in Console.app as well as using the following command:
```sh
/usr/bin/xcrun simctl spawn booted log stream --level debug --style compact --predicate 'process=="ReactNativeTesterApp" && subsystem=="com.facebook.react.log"'
```

## Peter's Test Plan

1. Apply P125583473
2. Verify log output in Xcode P125583504
3. Apply this diff
4. Verify log output in Xcode P125583597

These appear unchanged, after digging into why, I realized that FB doesn't even use the default log function, we inject a custom one [here](https://fburl.com/diffusion/887a1axs). So this PR shouldn't affect us at all. :)

Differential Revision: D19605414

Pulled By: PeteTheHeat

fbshipit-source-id: 1d70fb702c337a759905d4a65a951a31353ce775
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

It's time to upgrade RCTDefaultLogFunction to use modern os_log API

4 participants