search

rust-vmm / vmm-reference

A VMM implementation based of rust-vmm components
Apache License 2.0
147 stars 61 forks source link

RFC: add logging functionality #191

Closed dodsonmg closed 2 years ago

dodsonmg commented 2 years ago

Checking to see if this is the kind of logging implementation you're looking for to address #79.

It uses the log crate and implements a simple logger that I mostly copied from cloud-hypervisor: https://github.com/cloud-hypervisor/cloud-hypervisor/blob/f151a8602c1e8b2b17258aef99abd33140622839/src/main.rs#L74-L117

I left most of the println! and eprintln! statements in main.rs and the api crates alone because these provide immediate feedback to the CLI user (e.g., printing a help message) and it didn't seem appropriate to redirect these to stderr with a timestamp.

I replaced remaining statements in all the crates with appropriate info!, warn!, or error! messages. I also replaced usage of the debug! macro from the utils crate with the debug! macro provided by the log crate.

The implementation defaults to the Debug level, meaning only log entries initiated with the trace! macro are filtered out. Alternately, I could implement a command line argument to set the log level, similar to the cloud-hypervisor implementation.

Also, I would appreciate feedback on how I might go about writing unit tests for the logger module. The tests I've found elsewhere (e.g., for firecracker) support a much more complex logger implementation.

andreeaflorescu commented 2 years ago

Once we add a configuration for the logger we might find a few things worth testing. Until that point I am not sure we can do anything to test the logger that would not be a "coverage test" (i.e. a test that is not checking anything useful, and it's written just so that the coverage does not drop).

andreeaflorescu commented 2 years ago

Closing this PR as it is stale. @dodsonmg feel free to re-open if you want to continue working on it.