Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update logging to use log, add command line flag to toggle it #1147

Merged
merged 3 commits into from
Jan 14, 2020

Conversation

MarkMcCaskey
Copy link
Contributor

@MarkMcCaskey MarkMcCaskey commented Jan 14, 2020

This PR:

  • ports our logging to the log crate which is the de-facto standard way to do logging in Rust (it defines a trait which logging backends can implement (we can also implement our own at a later date using these traits)). Another benefit of using the standard logging utilities is that we can now allow users of our libraries to filter and display log messages from Wasmer in a more natural way.
  • adds a command line flag to enable/disable logging
  • updates the debug and trace features to pass the correct static toggles to the log crate; judging by the log documentation these features need to only be set once
  • copies and slightly modifies our fern configuration from wapm
  • updates the makefile so that make release compiles out all log statements
  • TODO: update CI to not print with color (may not be necessary actually)

Review

  • Add a short description of the the change to the CHANGELOG.md file

Here's some example output:

[1579035881.809 DEBUG wasmer_wasi::state] wasi::fs::inodes
[1579035881.809 DEBUG wasmer_wasi::state] wasi::fs::preopen_dirs
[1579035881.809 DEBUG wasmer_wasi::state] wasi::fs::mapped_dirs
[1579035881.809 DEBUG wasmer_wasi::state] wasi::fs::end
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::fd_prestat_get: fd=3
[1579035881.810 DEBUG wasmer_wasi::state] in prestat_fd Fd { rights: 536870911, rights_inheriting: 536870911, flags: 0, offset: 0, open_flags: 1, inode: Index { index: 3, generation: 0 } }
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::fd_prestat_dir_name: fd=3, path_len=2
[1579035881.810 DEBUG wasmer_wasi::syscalls] => result: "/"
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::fd_fdstat_get: fd=3, buf_ptr=1048536
[1579035881.810 DEBUG wasmer_wasi::state] fdstat: Fd { rights: 536870911, rights_inheriting: 536870911, flags: 0, offset: 0, open_flags: 1, inode: Index { index: 3, generation: 0 } }
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::fd_prestat_get: fd=4
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::environ_sizes_get
[1579035881.810 DEBUG wasmer_wasi::syscalls] env_var_count: 0, env_buf_size: 0
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::args_sizes_get
[1579035881.810 DEBUG wasmer_wasi::syscalls] => argc=3, argv_buf_size=92
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::args_get
[1579035881.810 DEBUG wasmer_wasi::syscalls] => args:
                   0: /Users/mark/.wasmer/globals/wapm_packages/mark/[email protected]/wasi-example.wasm
                   1: -e
                   2: HQ+
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::args_sizes_get
[1579035881.811 DEBUG wasmer_wasi::syscalls] => argc=3, argv_buf_size=92
[1579035881.811 DEBUG wasmer_wasi::syscalls] wasi::args_get
[1579035881.811 DEBUG wasmer_wasi::syscalls] => args:
                   0: /Users/mark/.wasmer/globals/wapm_packages/mark/[email protected]/wasi-example.wasm
                   1: -e
                   2: HQ+
[1579035881.811 DEBUG wasmer_wasi::syscalls] wasi::random_get buf_len: 16
[1579035881.811 DEBUG wasmer_wasi::syscalls] wasi::fd_write: fd=1
Hello, world!

[1579035881.811 DEBUG wasmer_wasi::syscalls] wasi::fd_write: fd=1
HQ+

@MarkMcCaskey MarkMcCaskey added 🎉 enhancement New feature! 📦 lib-deprecated About the deprecated crates 📦 lib-cli About wasmer-cli labels Jan 14, 2020
@MarkMcCaskey
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jan 14, 2020
@MarkMcCaskey MarkMcCaskey force-pushed the feature/standardized-logging branch from a56d859 to 12f7416 Compare January 14, 2020 20:42
@MarkMcCaskey
Copy link
Contributor Author

bors try-

@MarkMcCaskey
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jan 14, 2020
@MarkMcCaskey MarkMcCaskey force-pushed the feature/standardized-logging branch from c03f2f1 to 397343e Compare January 14, 2020 21:00
@MarkMcCaskey
Copy link
Contributor Author

bors try-

@MarkMcCaskey
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jan 14, 2020
@MarkMcCaskey MarkMcCaskey force-pushed the feature/standardized-logging branch from 397343e to 286e5db Compare January 14, 2020 21:03
Cargo.toml Outdated
@@ -22,6 +22,9 @@ include = [
[dependencies]
byteorder = "1.3"
errno = "0.2"
fern = { version = "0.5", features = ["colored"], optional = true }
# statically turn off logging for wasmer by default, fetaures override this
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# statically turn off logging for wasmer by default, fetaures override this
# statically turn off logging for wasmer by default, features override this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I deleted this comment because it's no longer true!

src/utils.rs Outdated
pub fn wasmer_should_print_color() -> bool {
std::env::var("WASMER_DISABLE_COLOR")
.map(|_| false)
.unwrap_or(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

.unwrap_or(stdout_isatty()) from the isatty crate?

Which also implies that it shouldn't be WASMER_DISABLE_COLOR because we might also want to forcibly enable color when isatty is false. Perhaps WASMER_COLORS which may be true or false.

@@ -247,6 +248,11 @@ struct Run {
#[structopt(long = "enable-experimental-io-devices", hidden = true)]
enable_experimental_io_devices: bool,

/// Enable debug output
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a canonical naming for the logging flags? I would expect that --debug would map precisely to enabling debug!() and above. Something like --log-level=... which corresponds to the enum in Log::LevelFilter, defaulting to Off given that we conditionally initialize logging?

Or maybe we should set it to Error or Warn and always initialize logging? I like that even more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, so currently I'm using Cargo features for this!

@bors
Copy link
Contributor

bors bot commented Jan 14, 2020

try

Build failed

  • wasmerio.wasmer

@MarkMcCaskey
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Jan 14, 2020
1147: Update logging to use `log`, add command line flag to toggle it r=MarkMcCaskey a=MarkMcCaskey

This PR:
- ports our logging to the [`log`](https://crates.io/crates/log) crate which is the de-facto standard way to do logging in Rust (it defines a trait which logging backends can implement (we can also implement our own at a later date using these traits)).  Another benefit of using the standard logging utilities is that we can now allow users of our libraries to filter and display log messages from Wasmer in a more natural way.
- adds a command line flag to enable/disable logging
- updates the `debug` and `trace` features to pass the correct static toggles to the `log` crate; judging by the `log` documentation these features need to only be set once
- copies and slightly modifies our `fern` configuration from wapm
- updates the makefile so that `make release` compiles out all log statements
- TODO: update CI to not print with color (may not be necessary actually)

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Here's some example output:

```
[1579035881.809 DEBUG wasmer_wasi::state] wasi::fs::inodes
[1579035881.809 DEBUG wasmer_wasi::state] wasi::fs::preopen_dirs
[1579035881.809 DEBUG wasmer_wasi::state] wasi::fs::mapped_dirs
[1579035881.809 DEBUG wasmer_wasi::state] wasi::fs::end
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::fd_prestat_get: fd=3
[1579035881.810 DEBUG wasmer_wasi::state] in prestat_fd Fd { rights: 536870911, rights_inheriting: 536870911, flags: 0, offset: 0, open_flags: 1, inode: Index { index: 3, generation: 0 } }
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::fd_prestat_dir_name: fd=3, path_len=2
[1579035881.810 DEBUG wasmer_wasi::syscalls] => result: "/"
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::fd_fdstat_get: fd=3, buf_ptr=1048536
[1579035881.810 DEBUG wasmer_wasi::state] fdstat: Fd { rights: 536870911, rights_inheriting: 536870911, flags: 0, offset: 0, open_flags: 1, inode: Index { index: 3, generation: 0 } }
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::fd_prestat_get: fd=4
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::environ_sizes_get
[1579035881.810 DEBUG wasmer_wasi::syscalls] env_var_count: 0, env_buf_size: 0
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::args_sizes_get
[1579035881.810 DEBUG wasmer_wasi::syscalls] => argc=3, argv_buf_size=92
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::args_get
[1579035881.810 DEBUG wasmer_wasi::syscalls] => args:
                   0: /Users/mark/.wasmer/globals/wapm_packages/mark/[email protected]/wasi-example.wasm
                   1: -e
                   2: HQ+
[1579035881.810 DEBUG wasmer_wasi::syscalls] wasi::args_sizes_get
[1579035881.811 DEBUG wasmer_wasi::syscalls] => argc=3, argv_buf_size=92
[1579035881.811 DEBUG wasmer_wasi::syscalls] wasi::args_get
[1579035881.811 DEBUG wasmer_wasi::syscalls] => args:
                   0: /Users/mark/.wasmer/globals/wapm_packages/mark/[email protected]/wasi-example.wasm
                   1: -e
                   2: HQ+
[1579035881.811 DEBUG wasmer_wasi::syscalls] wasi::random_get buf_len: 16
[1579035881.811 DEBUG wasmer_wasi::syscalls] wasi::fd_write: fd=1
Hello, world!

[1579035881.811 DEBUG wasmer_wasi::syscalls] wasi::fd_write: fd=1
HQ+
```


Co-authored-by: Mark McCaskey <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jan 14, 2020

Build succeeded

@bors bors bot merged commit 39025d0 into master Jan 14, 2020
@bors bors bot deleted the feature/standardized-logging branch January 14, 2020 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature! 📦 lib-cli About wasmer-cli 📦 lib-deprecated About the deprecated crates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants