Skip to content

Commit

Permalink
feat: log to base dir (#5197)
Browse files Browse the repository at this point in the history
Description
---
- Change the logging initialization to account for the `-b` option on application startup, and dynamically replace the new `{{log_dir}}` placeholder value inside the log4rs files.
- Update logging directory naming for consistency from `base-node` to `base_node`
- Fix the log4rs config for the tari miner so it will log to stdout as well as the created log file
- Update tari miner log target for consistency

Motivation and Context
---
log4rs considers the cli working directory the root. From there it appends the `path` from the log4rs yaml file as a relative path (unless it's an absolute path). The problem with this is that it completely disregards the standard Tari cli option `-b` for assigning a base directory. This can result in the configuration in the expected assigned location, and the logging ending up in whatever working directory you happened to be in at the time. This has resulted in me finding lots of orphaned logging directories and files spread over my local system.

How Has This Been Tested?
---
Tested locally by using the `-b` option to a location different than the current working directory and ensuring _all_ config and logs end up in the directory provided. Whereas before the `log` dir still would have been created in the working directory.


BREAKING CHANGE: This PR changes the logging configuration which is only generated on the first run. Users may wish to remove log config files:
- `base_dir/config/base_node/log4rs.yml`
- `base_dir/config/miner/log4rs.yml`
- `base_dir/config/wallet/log4rs.yml`
  • Loading branch information
brianp committed Feb 28, 2023
1 parent 8d34e8a commit 5147b5c
Show file tree
Hide file tree
Showing 16 changed files with 89 additions and 37 deletions.
22 changes: 21 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 8 additions & 9 deletions applications/tari_base_node/log4rs_sample.yml
Expand Up @@ -14,7 +14,6 @@ appenders:
# An appender named "stdout" that writes to stdout
stdout:
kind: console

encoder:
pattern: "{d(%H:%M)} {h({l}):5} {m}{n}"
filters:
Expand All @@ -24,7 +23,7 @@ appenders:
# An appender named "network" that writes to a file with a custom pattern encoder
network:
kind: rolling_file
path: "log/base-node/network.log"
path: "{{log_dir}}/log/base_node/network.log"
policy:
kind: compound
trigger:
Expand All @@ -34,13 +33,13 @@ appenders:
kind: fixed_window
base: 1
count: 5
pattern: "log/base-node/network.{}.log"
pattern: "{{log_dir}}/log/base_node/network.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] [Thread:{I}] {l:5} {m} // {f}:{L}{n}"
# An appender named "network" that writes to a file with a custom pattern encoder
message_logging:
kind: rolling_file
path: "log/base-node/messages.log"
path: "{{log_dir}}/log/base_node/messages.log"
policy:
kind: compound
trigger:
Expand All @@ -50,15 +49,15 @@ appenders:
kind: fixed_window
base: 1
count: 5
pattern: "log/base-node/messages.{}.log"
pattern: "{{log_dir}}/log/base_node/messages.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] [Thread:{I}] {l:5} {m} // {f}:{L}{n}"


# An appender named "base_layer" that writes to a file with a custom pattern encoder
base_layer:
kind: rolling_file
path: "log/base-node/base_layer.log"
path: "{{log_dir}}/log/base_node/base_layer.log"
policy:
kind: compound
trigger:
Expand All @@ -68,14 +67,14 @@ appenders:
kind: fixed_window
base: 1
count: 5
pattern: "log/base-node/base_layer.{}.log"
pattern: "{{log_dir}}/log/base_node/base_layer.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] [{X(node-public-key)},{X(node-id)}] {l:5} {m} // {f}:{L}{n}"

# An appender named "other" that writes to a file with a custom pattern encoder
other:
kind: rolling_file
path: "log/base-node/other.log"
path: "{{log_dir}}/log/base_node/other.log"
policy:
kind: compound
trigger:
Expand All @@ -85,7 +84,7 @@ appenders:
kind: fixed_window
base: 1
count: 5
pattern: "log/base-node/other.{}.log"
pattern: "{{log_dir}}/log/base_node/other.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] [Thread:{I}] {l:5} {m}{n} // {f}:{L} "

Expand Down
1 change: 1 addition & 0 deletions applications/tari_base_node/src/main.rs
Expand Up @@ -109,6 +109,7 @@ fn main_inner() -> Result<(), ExitError> {

initialize_logging(
&cli.common.log_config_path("base_node"),
&cli.common.get_base_path(),
include_str!("../log4rs_sample.yml"),
)?;
info!(
Expand Down
14 changes: 7 additions & 7 deletions applications/tari_console_wallet/log4rs_sample.yml
Expand Up @@ -14,7 +14,7 @@ appenders:
# An appender named "stdout" that writes to file.
stdout:
kind: rolling_file
path: "log/wallet/stdout.log"
path: "{{log_dir}}/log/wallet/stdout.log"
append: false
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] {h({l}):5} {m}{n}"
Expand All @@ -32,7 +32,7 @@ appenders:
# An appender named "network" that writes to a file with a custom pattern encoder
network:
kind: rolling_file
path: "log/wallet/network.log"
path: "{{log_dir}}/log/wallet/network.log"
policy:
kind: compound
trigger:
Expand All @@ -42,14 +42,14 @@ appenders:
kind: fixed_window
base: 1
count: 5
pattern: "log/wallet/network.{}.log"
pattern: "{{log_dir}}/log/wallet/network.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] [Thread:{I}] {l:5} {m}{n}"

# An appender named "base_layer" that writes to a file with a custom pattern encoder
base_layer:
kind: rolling_file
path: "log/wallet/base_layer.log"
path: "{{log_dir}}/log/wallet/base_layer.log"
policy:
kind: compound
trigger:
Expand All @@ -59,14 +59,14 @@ appenders:
kind: fixed_window
base: 1
count: 5
pattern: "log/wallet/base_layer.{}.log"
pattern: "{{log_dir}}/log/wallet/base_layer.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] [Thread:{I}] {l:5} {m}{n}"

# An appender named "base_layer" that writes to a file with a custom pattern encoder
other:
kind: rolling_file
path: "log/wallet/other.log"
path: "{{log_dir}}/log/wallet/other.log"
policy:
kind: compound
trigger:
Expand All @@ -76,7 +76,7 @@ appenders:
kind: fixed_window
base: 1
count: 5
pattern: "log/wallet/other.{}.log"
pattern: "{{log_dir}}/log/wallet/other.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] [Thread:{I}] {l:5} {m}{n}"

Expand Down
3 changes: 2 additions & 1 deletion applications/tari_console_wallet/src/main.rs
Expand Up @@ -73,9 +73,10 @@ fn main() {
fn main_inner() -> Result<(), ExitError> {
let cli = Cli::parse();

let cfg = load_configuration(cli.common.config_path().as_path(), true, &cli)?;
let cfg = load_configuration(cli.common.config_path(), true, &cli)?;
initialize_logging(
&cli.common.log_config_path("wallet"),
&cli.common.get_base_path(),
include_str!("../log4rs_sample.yml"),
)?;

Expand Down
4 changes: 2 additions & 2 deletions applications/tari_merge_mining_proxy/log4rs_sample.yml
Expand Up @@ -21,7 +21,7 @@ appenders:
# An appender named "proxy" that writes to a file with a custom pattern encoder
proxy:
kind: rolling_file
path: "log/proxy/proxy.log"
path: "{{log_dir}}/log/proxy/proxy.log"
policy:
kind: compound
trigger:
Expand All @@ -31,7 +31,7 @@ appenders:
kind: fixed_window
base: 1
count: 50
pattern: "log/proxy/proxy.{}.log"
pattern: "{{log_dir}}/log/proxy/proxy.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] {l:5} {m}{n}"

Expand Down
1 change: 1 addition & 0 deletions applications/tari_merge_mining_proxy/src/main.rs
Expand Up @@ -52,6 +52,7 @@ async fn main() -> Result<(), anyhow::Error> {

initialize_logging(
&cli.common.log_config_path("proxy"),
&cli.common.get_base_path(),
include_str!("../log4rs_sample.yml"),
)?;
run_merge_miner::start_merge_miner(cli).await
Expand Down
11 changes: 7 additions & 4 deletions applications/tari_miner/log4rs_sample.yml
Expand Up @@ -20,7 +20,7 @@ appenders:
# An appender named "base_layer" that writes to a file with a custom pattern encoder
miner:
kind: rolling_file
path: "log/miner/miner.log"
path: "{{log_dir}}/log/miner/miner.log"
policy:
kind: compound
trigger:
Expand All @@ -30,7 +30,7 @@ appenders:
kind: fixed_window
base: 1
count: 5
pattern: "log/miner/miner.{}.log"
pattern: "{{log_dir}}/log/miner/miner.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] [Thread:{I}] {l:5} {m}{n}"

Expand All @@ -46,14 +46,17 @@ loggers:
level: debug
appenders:
- miner
- stdout
additive: false
tari_miner:
tari::miner:
level: debug
appenders:
- miner
- stdout
additive: false
tari_miner:
level: info
level: debug
appenders:
- miner
- stdout
additive: false
5 changes: 3 additions & 2 deletions applications/tari_miner/src/main.rs
Expand Up @@ -32,8 +32,8 @@ use tokio::runtime::Runtime;

use crate::cli::Cli;

pub const LOG_TARGET: &str = "tari_miner::miner::main";
pub const LOG_TARGET_FILE: &str = "tari_miner::logging::miner::main";
pub const LOG_TARGET: &str = "tari::miner::main";
pub const LOG_TARGET_FILE: &str = "tari::logging::miner::main";

mod cli;
mod config;
Expand Down Expand Up @@ -67,6 +67,7 @@ async fn main_inner() -> Result<(), ExitError> {
let cli = Cli::parse();
initialize_logging(
&cli.common.log_config_path("miner"),
&cli.common.get_base_path(),
include_str!("../log4rs_sample.yml"),
)?;
start_miner(cli).await
Expand Down
2 changes: 1 addition & 1 deletion applications/tari_miner/src/miner.rs
Expand Up @@ -35,7 +35,7 @@ use thread::JoinHandle;

use super::difficulty::BlockHeaderSha3;

pub const LOG_TARGET: &str = "tari_miner::miner::standalone";
pub const LOG_TARGET: &str = "tari::miner::standalone";

// Identify how often mining thread is reporting / checking context
// ~400_000 hashes per second
Expand Down
4 changes: 2 additions & 2 deletions applications/tari_miner/src/run_miner.rs
Expand Up @@ -52,8 +52,8 @@ use crate::{
utils::{coinbase_request, extract_outputs_and_kernels},
};

pub const LOG_TARGET: &str = "tari_miner::miner::main";
pub const LOG_TARGET_FILE: &str = "tari_miner::logging::miner::main";
pub const LOG_TARGET: &str = "tari::miner::main";
pub const LOG_TARGET_FILE: &str = "tari::logging::miner::main";

type WalletGrpcClient = WalletClient<InterceptedService<Channel, ClientAuthenticationInterceptor>>;

Expand Down
4 changes: 2 additions & 2 deletions applications/tari_miner/src/stratum/controller.rs
Expand Up @@ -32,8 +32,8 @@ use log::*;

use crate::stratum::{error::Error, stratum_types as types, stream::Stream};

pub const LOG_TARGET: &str = "tari_miner::miner::stratum::controller";
pub const LOG_TARGET_FILE: &str = "tari_miner::logging::miner::stratum::controller";
pub const LOG_TARGET: &str = "tari::miner::stratum::controller";
pub const LOG_TARGET_FILE: &str = "tari::logging::miner::stratum::controller";

pub struct Controller {
server_url: String,
Expand Down
Expand Up @@ -34,8 +34,8 @@ use crate::{
stratum::{error::Error, stratum_types as types},
};

pub const LOG_TARGET: &str = "tari_miner::miner::stratum::controller";
pub const LOG_TARGET_FILE: &str = "tari_miner::logging::miner::stratum::controller";
pub const LOG_TARGET: &str = "tari::miner::stratum::controller";
pub const LOG_TARGET_FILE: &str = "tari::logging::miner::stratum::controller";

pub struct Controller {
rx: mpsc::Receiver<types::miner_message::MinerMessage>,
Expand Down
1 change: 1 addition & 0 deletions common/Cargo.toml
Expand Up @@ -27,6 +27,7 @@ path-clean = "0.1.0"
prost-build = { version = "0.9.0", optional = true }
serde = { version = "1.0.106", default_features = false }
serde_json = "1.0.51"
serde_yaml = "0.9.17"
sha2 = "0.9.5"
structopt = { version = "0.3.13", default_features = false }
tempfile = "3.1.0"
Expand Down
32 changes: 28 additions & 4 deletions common/src/logging.rs
Expand Up @@ -21,12 +21,19 @@
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
//

use std::{fs, fs::File, io::Write, path::Path};
use std::{
fs,
fs::File,
io::{Read, Write},
path::Path,
};

use log4rs::config::RawConfig;

use crate::ConfigError;

/// Set up application-level logging using the Log4rs configuration file specified in
pub fn initialize_logging(config_file: &Path, default: &str) -> Result<(), ConfigError> {
pub fn initialize_logging(config_file: &Path, base_path: &Path, default: &str) -> Result<(), ConfigError> {
println!(
"Initializing logging according to {:?}",
config_file.to_str().unwrap_or("[??]")
Expand All @@ -43,8 +50,25 @@ pub fn initialize_logging(config_file: &Path, default: &str) -> Result<(), Confi
.map_err(|e| ConfigError::new("Could not create default log file", Some(e.to_string())))?;
}

log4rs::init_file(config_file, Default::default())
.map_err(|e| ConfigError::new("Could not initialize logging", Some(e.to_string())))
let mut file =
File::open(config_file).map_err(|e| ConfigError::new("Could not locate file: {}", Some(e.to_string())))?;
let mut contents = String::new();

file.read_to_string(&mut contents)
.map_err(|e| ConfigError::new("Could not read file: {}", Some(e.to_string())))?;

let contents = contents.replace(
"{{log_dir}}",
base_path
.to_str()
.expect("Could not replace {{log_dir}} variable from the log4rs config"),
);

let config: RawConfig =
serde_yaml::from_str(&contents).expect("Could not parse the contents of the log file as yaml");
log4rs::init_raw_config(config).expect("Could not initialize logging");

Ok(())
}

/// Log an error if an `Err` is returned from the `$expr`. If the given expression is `Ok(v)`,
Expand Down
1 change: 1 addition & 0 deletions integration_tests/tests/cucumber.rs
Expand Up @@ -4973,6 +4973,7 @@ fn flush_stdout(buffer: &Arc<Mutex<Vec<u8>>>) {
fn main() {
initialize_logging(
&PathBuf::from("log4rs/cucumber.yml"),
&PathBuf::from("./"),
include_str!("../log4rs/cucumber.yml"),
)
.expect("logging not configured");
Expand Down

0 comments on commit 5147b5c

Please sign in to comment.