Skip to content

Commit b7d0b49

Browse files
authored
feat: Reduce logging for tests with --quiet (#7656)
The test driver by default is configured to run at `Trace`, configured through the shared `slog` [dependency](https://github.com/dfinity/ic/blob/23d89f18b6e0ff711931b17fe8d115e2694fcae5/bazel/external_crates.bzl#L1298-L1305). This adds a new `--quiet` flag that reduces the log level to `Info`, and suppresses some of the more verbose logs from background tasks.
1 parent 018288c commit b7d0b49

File tree

6 files changed

+59
-14
lines changed

6 files changed

+59
-14
lines changed

rs/tests/driver/src/driver/bootstrap.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ use ic_registry_canister_api::IPv4Config;
4141
use ic_registry_provisional_whitelist::ProvisionalWhitelist;
4242
use ic_registry_subnet_type::SubnetType;
4343
use ic_types::malicious_behavior::MaliciousBehavior;
44-
use slog::{Logger, info, warn};
44+
use slog::{Logger, debug, info, warn};
4545
use std::{
4646
collections::BTreeMap,
4747
convert::Into,
@@ -213,7 +213,7 @@ pub fn init_ic(
213213
ic_config.skip_unassigned_record();
214214
}
215215

216-
info!(test_env.logger(), "Initializing via {:?}", &ic_config);
216+
debug!(test_env.logger(), "Initializing via {:?}", &ic_config);
217217

218218
Ok(ic_config.initialize()?)
219219
}

rs/tests/driver/src/driver/context.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ pub struct GroupContext {
2626
pub group_base_name: String,
2727
pub logs_enabled: bool,
2828
pub exclude_logs: Vec<Regex>,
29+
pub quiet: bool,
2930
}
3031

3132
impl GroupContext {
@@ -43,11 +44,12 @@ impl GroupContext {
4344
group_base_name: String,
4445
logs_enabled: bool,
4546
exclude_logs: Vec<Regex>,
47+
quiet: bool,
4648
) -> Result<Self> {
4749
let task_id = subproc_info.as_ref().map(|t| t.0.clone());
4850
let sock_id = subproc_info.map(|t| t.1).unwrap_or_default();
4951
let socket_path = Self::log_socket_path(sock_id);
50-
let logger = Self::create_logger(socket_path, task_id)?;
52+
let logger = Self::create_logger(socket_path, task_id, quiet)?;
5153

5254
let exec_path = std::env::current_exe().expect("could not acquire parent process path");
5355
if !exec_path.is_file() {
@@ -68,6 +70,7 @@ impl GroupContext {
6870
group_base_name,
6971
logs_enabled,
7072
exclude_logs,
73+
quiet,
7174
})
7275
}
7376

@@ -188,14 +191,18 @@ impl GroupContext {
188191
}
189192

190193
/// Create a logger for this process.
191-
fn create_logger(sock_path: PathBuf, subproc_id: Option<TaskId>) -> Result<Logger> {
194+
fn create_logger(
195+
sock_path: PathBuf,
196+
subproc_id: Option<TaskId>,
197+
quiet: bool,
198+
) -> Result<Logger> {
192199
if let Some(task_id) = subproc_id {
193200
let sender = LogSender::new(task_id, sock_path.clone())
194201
.with_context(|| format!("Log socket path: {sock_path:?}"))?;
195202
let logger = Logger::root(sender, slog::o!());
196203
Ok(logger)
197204
} else {
198-
let logger = crate::driver::logger::new_stdout_logger();
205+
let logger = crate::driver::logger::new_stdout_logger(quiet);
199206
Ok(logger)
200207
}
201208
}

rs/tests/driver/src/driver/group.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,9 @@ pub struct CliArgs {
141141
help = "The list of regexes which will be skipped from the streaming."
142142
)]
143143
pub exclude_logs: Vec<Regex>,
144+
145+
#[clap(long, short, help = "Reduce terminal logging to mostly test output.")]
146+
pub quiet: bool,
144147
}
145148

146149
impl CliArgs {
@@ -218,9 +221,16 @@ fn subproc(
218221
task_id: TaskId,
219222
target_fn: impl SubprocessFn,
220223
ctx: &mut ComposeContext,
224+
quiet: bool,
221225
) -> SubprocessTask {
222226
trace!(ctx.group_ctx.log(), "subproc(task_name={})", &task_id);
223-
SubprocessTask::new(task_id, ctx.rh.clone(), target_fn, ctx.group_ctx.clone())
227+
SubprocessTask::new(
228+
task_id,
229+
ctx.rh.clone(),
230+
target_fn,
231+
ctx.group_ctx.clone(),
232+
quiet,
233+
)
224234
}
225235

226236
fn timed(
@@ -393,7 +403,7 @@ impl SystemTestSubGroup {
393403
};
394404
timed(
395405
Plan::Leaf {
396-
task: Box::from(subproc(task_id, closure, ctx)),
406+
task: Box::from(subproc(task_id, closure, ctx, false)),
397407
},
398408
ctx.timeout_per_test,
399409
None,
@@ -567,6 +577,8 @@ impl SystemTestGroup {
567577
debug!(group_ctx.log(), "SystemTestGroup.make_plan");
568578
let start_time = Utc::now();
569579

580+
let quiet = group_ctx.quiet;
581+
570582
let mut compose_ctx = ComposeContext {
571583
rh,
572584
group_ctx: group_ctx.clone(),
@@ -645,6 +657,7 @@ impl SystemTestGroup {
645657
}
646658
},
647659
&mut compose_ctx,
660+
false,
648661
)) as Box<dyn Task>;
649662

650663
// The ID of the root task is needed outside this function for awaiting when the plan execution finishes.
@@ -699,6 +712,7 @@ impl SystemTestGroup {
699712
}
700713
},
701714
&mut compose_ctx,
715+
quiet,
702716
)) as Box<dyn Task>
703717
} else {
704718
Box::from(EmptyTask::new(keepalive_task_id)) as Box<dyn Task>
@@ -734,6 +748,7 @@ impl SystemTestGroup {
734748
}
735749
},
736750
&mut compose_ctx,
751+
quiet,
737752
);
738753

739754
Box::from(log_task) as Box<dyn Task>
@@ -765,6 +780,7 @@ impl SystemTestGroup {
765780
SetupResult {}.write_attribute(&env);
766781
},
767782
&mut compose_ctx,
783+
false,
768784
);
769785
timed(
770786
Plan::Leaf {
@@ -787,6 +803,7 @@ impl SystemTestGroup {
787803
teardown_fn(env);
788804
},
789805
&mut compose_ctx,
806+
false,
790807
);
791808
timed(
792809
Plan::Leaf {
@@ -927,6 +944,7 @@ impl SystemTestGroup {
927944
args.group_base_name,
928945
!args.no_logs,
929946
args.exclude_logs,
947+
args.quiet,
930948
)?;
931949

932950
let with_farm = self.with_farm;

rs/tests/driver/src/driver/logger.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
use crate::driver::constants;
44
use anyhow::Result;
5-
use slog::{Drain, KV, Key, Logger, OwnedKVList, Record, o};
5+
use slog::{Drain, KV, Key, Level, Logger, OwnedKVList, Record, o};
66
use slog_term::Decorator;
77
use std::{fmt, fs::File, io};
88
use std::{os::unix::prelude::AsRawFd, path::Path};
@@ -39,15 +39,23 @@ fn new_file_logger<P: AsRef<Path>>(p: P) -> Result<Logger> {
3939
Ok(slog::Logger::root(async_drain(file_drain), o!()))
4040
}
4141

42+
pub fn new_discard_logger() -> Logger {
43+
slog::Logger::root(async_drain(slog::Discard), o!())
44+
}
45+
4246
fn multiplex_logger(l1: Logger, l2: Logger) -> Logger {
4347
slog::Logger::root(slog::Duplicate(l1, l2).fuse(), o!())
4448
}
4549

4650
/// creates a slog::Logger that prints to standard out using an asynchronous drain
47-
pub fn new_stdout_logger() -> Logger {
48-
let decorator = slog_term::TermDecorator::new().build();
49-
let drain = SysTestLogFormatter::new(decorator).fuse();
50-
slog::Logger::root(async_drain(drain), o!())
51+
pub fn new_stdout_logger(quiet: bool) -> Logger {
52+
let decorator = slog_term::TermDecorator::new().force_color().build();
53+
let drain = SysTestLogFormatter::new(decorator);
54+
if quiet {
55+
slog::Logger::root(async_drain(drain.filter_level(Level::Info).fuse()), o!())
56+
} else {
57+
slog::Logger::root(async_drain(drain.fuse()), o!())
58+
}
5159
}
5260

5361
struct SysTestLogFormatter<D> {

rs/tests/driver/src/driver/subprocess_task.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::driver::{
77
context::GroupContext,
88
dsl::SubprocessFn,
99
event::TaskId,
10+
logger::new_discard_logger,
1011
process::{KillFn, Process},
1112
subprocess_ipc::{LogReceiver, ReportOrFailure, log_panic_event},
1213
task::{Task, TaskHandle},
@@ -33,6 +34,7 @@ pub struct SubprocessTask {
3334
f: Arc<Mutex<Option<Box<dyn SubprocessFn>>>>,
3435
spawned: AtomicBool,
3536
group_ctx: GroupContext,
37+
quiet: bool,
3638
}
3739

3840
impl SubprocessTask {
@@ -41,13 +43,15 @@ impl SubprocessTask {
4143
rt: RtHandle,
4244
f: F,
4345
group_ctx: GroupContext,
46+
quiet: bool,
4447
) -> Self {
4548
Self {
4649
task_id,
4750
rt,
4851
f: shared_mutex(Some(Box::new(f))),
4952
spawned: Default::default(),
5053
group_ctx,
54+
quiet,
5155
}
5256
}
5357
}
@@ -85,7 +89,15 @@ impl Task for SubprocessTask {
8589

8690
info!(self.group_ctx.log(), "Spawning {:?} ...", child_cmd);
8791

88-
let log = self.group_ctx.logger();
92+
// When this subtask is `quiet`, have the receiver discard logs.
93+
// NOTE: We discard on the receiver rather than the sender, so that no
94+
// information is lost. Eventually, we may want to split the logfile
95+
// and stdout, and will leave this to the receiver.
96+
let log = if !self.quiet {
97+
self.group_ctx.logger()
98+
} else {
99+
new_discard_logger()
100+
};
89101
let log_rcvr = self
90102
.rt
91103
.block_on(LogReceiver::new(sock_path, log.clone()))

rs/tests/driver/src/driver/test_env.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ impl TestEnv {
4646
pub fn new<P: AsRef<Path>>(path: P, logger: Logger) -> Result<TestEnv> {
4747
let base_path = PathBuf::from(path.as_ref());
4848
let log_file = append_and_lock_exclusive(base_path.join("test.log"))?;
49-
let file_drain = slog_term::FullFormat::new(slog_term::PlainSyncDecorator::new(log_file))
49+
let file_drain = slog_term::FullFormat::new(slog_term::PlainDecorator::new(log_file))
5050
.build()
5151
.fuse();
5252
let file_drain = slog_async::Async::new(file_drain)

0 commit comments

Comments
 (0)