Skip to content
This repository was archived by the owner on Jan 11, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
a5117db
skip null round retry
Nov 14, 2023
43bf8fe
rearrange logging
Nov 14, 2023
d485555
Merge branch 'main' of protocol-github:consensus-shipyard/fendermint …
Nov 15, 2023
ed5ab9d
specify import
Nov 15, 2023
5796d5c
better null error handling
Nov 15, 2023
961b6be
format code
Nov 15, 2023
a205b2e
add tracing logs
Nov 15, 2023
b83dd8e
expand tilda
Nov 15, 2023
bba832b
prettier logging
Nov 15, 2023
3d11185
prettier logs
Nov 15, 2023
5a5c134
update logging
Nov 15, 2023
d6b0f0d
prettier log
Nov 15, 2023
9a585f1
Merge branch 'main' of protocol-github:consensus-shipyard/fendermint …
Nov 15, 2023
e4873a6
use ipc user instead of root
Nov 15, 2023
884a5a2
specify log path
Nov 15, 2023
c83dde3
remove unused logs
Nov 15, 2023
87445a6
Update fendermint/vm/topdown/src/finality/null.rs
cryptoAtwill Nov 15, 2023
2fa6c84
Update fendermint/vm/interpreter/src/chain.rs
cryptoAtwill Nov 15, 2023
6bb0bf0
Update fendermint/vm/topdown/src/sync.rs
cryptoAtwill Nov 15, 2023
ae9aed1
structured logging and log dir optional
Nov 15, 2023
8e73396
add debug logger
Nov 15, 2023
5d3113f
add debug logs
Nov 15, 2023
0e1bcc3
log fendermint only
Nov 15, 2023
f79f7f3
Look ahead limit use finalized height (#433)
cryptoAtwill Nov 15, 2023
7a9e257
comments
Nov 15, 2023
6810ed9
Merge branch 'tracing-logs' of protocol-github:consensus-shipyard/fen…
Nov 15, 2023
dde324d
Update fendermint/vm/topdown/src/sync.rs
cryptoAtwill Nov 15, 2023
636bdc5
rename makes everything clearer
Nov 15, 2023
d2d2fa8
Merge branch 'tracing-logs' of protocol-github:consensus-shipyard/fen…
Nov 15, 2023
0ba8f9c
Update fendermint/app/src/main.rs
cryptoAtwill Nov 15, 2023
46022c9
clean logs
Nov 15, 2023
2e374f0
Merge branch 'tracing-logs' of protocol-github:consensus-shipyard/fen…
Nov 15, 2023
bbbb8a5
log to console
Nov 15, 2023
6d94af3
update docker commands
Nov 15, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 113 additions & 24 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ tracing = "0.1"
tracing-subscriber = "0.3"
url = "2.4.1"
zeroize = "1.6"
trace4rs = "0.5.1"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you know if this is available on Github somewhere? I can't find a link at https://crates.io/crates/trace4rs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

literally = "0.1.3"

# Vendored for cross-compilation, see https://github.com/cross-rs/cross/wiki/Recipes#openssl
openssl = { version = "0.10", features = ["vendored"] }
Expand Down
4 changes: 4 additions & 0 deletions docker/runner.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,7 @@ COPY docker/.artifacts/bundle.car $FM_HOME_DIR/bundle.car
COPY docker/.artifacts/contracts $FM_HOME_DIR/contracts
COPY --from=builder /app/fendermint/app/config $FM_HOME_DIR/config
COPY --from=builder /app/output/bin/fendermint /usr/local/bin/fendermint

RUN useradd ipc && chown -R ipc $FM_HOME_DIR

USER ipc
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need an ipc user instead of root here? Also, I don't think it is the case but I am wondering if this may break the execution of the fendermint container considering the directory structure that we use. Have you tried it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I never quite understood how to use users inside containers properly. I made images where I used add-user to create users with specific IDs and groups, but unless those IDs aligned with what exists on the host, I still had to end up setting the -u option to make sure I can remove files created into mounted volumes without sudo. Since this is a container, I never quite got why we shouldn't use root 🤷 I'm sure I'm just ignorant, but if it works...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed as it's not needed anymore. But I think one day we should not use root to execute docker

2 changes: 2 additions & 0 deletions fendermint/app/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ tokio = { workspace = true }
tower-abci = { workspace = true }
tracing = { workspace = true }
tracing-subscriber = { workspace = true }
trace4rs = { workspace = true }
literally = { workspace = true }

fendermint_abci = { path = "../abci" }
fendermint_app_options = { path = "./options" }
Expand Down
73 changes: 64 additions & 9 deletions fendermint/app/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
// Copyright 2022-2023 Protocol Labs
// SPDX-License-Identifier: Apache-2.0, MIT

use tracing_subscriber::FmtSubscriber;
use anyhow::{anyhow, Context};
use std::path::Path;
use std::str::FromStr;
use std::sync::Arc;
use trace4rs::config::{AppenderId, Policy, Target};
use trace4rs::{
config::{self, Config, Format},
Handle,
};

pub use fendermint_app_options as options;
pub use fendermint_app_settings as settings;
use fendermint_app_settings::expand_tilde;

mod cmd;

Expand All @@ -14,14 +23,7 @@ async fn main() {

// Log events to stdout.
if let Some(level) = opts.tracing_level() {
// Writing to stderr so if we have output like JSON then we can pipe it to something else.
let subscriber = FmtSubscriber::builder()
.with_max_level(level)
.with_writer(std::io::stderr)
.finish();

tracing::subscriber::set_global_default(subscriber)
.expect("setting default subscriber failed");
create_log(level, &opts.home_dir).expect("cannot create logging");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should add a setting to configure the output path, and if we want the log to be output on std err/std output, or to a file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's right to do both. I had a similar thing in my Scala programs except I was able to configure them through XML, so we can enable the debug level on a package-by-package basis (third parties as well) without recompiling.

}

if let Err(e) = cmd::exec(&opts).await {
Expand All @@ -30,6 +32,59 @@ async fn main() {
}
}

fn create_log(level: tracing::Level, home_dir: &Path) -> anyhow::Result<()> {
let log_folder = expand_tilde(home_dir)
.join("logs")
.to_str()
.ok_or_else(|| anyhow!("cannot parse log folder"))?
.to_string();
std::fs::create_dir_all(&log_folder).context("cannot create log folder")?;

let console_appender = config::Appender::Console;
let topdown_appender = config::Appender::RollingFile {
path: format!("{log_folder}/topdown.log"),
policy: Policy {
maximum_file_size: "10mb".to_string(),
// we keep the last 5 log files, older files will be deleted
max_size_roll_backups: 5,
pattern: None,
},
};

// debug level logger
let topdown_logger = config::Logger {
level: config::LevelFilter::DEBUG,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we make this configurable too? Now we want this in debug mode for debugging purpose, but we may want to keep the same level for all in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would say we just keep this top down logs just in case for something unexpected. The logs will be 50mb in total, it's not taking a lot of space. But we always have some logs to refer back to.

appenders: literally::hset! { AppenderId::from("topdown") },
format: Format::default(),
};

// default logging output info log to console
let default = config::Logger {
level: config::LevelFilter::from_str(level.as_str())?,
appenders: literally::hset! { AppenderId::from("console") },
format: Format::default(),
};

let config = Config {
default,
loggers: literally::hmap! {
Target::from("fendermint_vm_topdown") => topdown_logger.clone(),
Target::from("fendermint_vm_interpreter::chain") => topdown_logger,
},
appenders: literally::hmap! {
AppenderId::from("console") => console_appender,
AppenderId::from("topdown") => topdown_appender
},
};

let handle = Arc::new(Handle::try_from(config).unwrap());

tracing::subscriber::set_global_default(handle.subscriber())
.context("setting default subscriber failed")?;

Ok(())
}

#[cfg(test)]
mod tests {
use cid::Cid;
Expand Down
Loading