Skip to content

Commit

Permalink
feat(console): add flags and configurations for warnings (#493)
Browse files Browse the repository at this point in the history
This branch adds a new `--warn` command-line argument and `warnings`
field in the config file. These new configs accept a list of warnings to
enable.

Closes #323

Signed-off-by: hi-rustin <rustin.liu@gmail.com>
  • Loading branch information
hi-rustin committed Jan 19, 2024
1 parent ef61081 commit 174883f
Show file tree
Hide file tree
Showing 7 changed files with 121 additions and 15 deletions.
12 changes: 6 additions & 6 deletions Cargo.lock

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

19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,25 @@ Options:
[env: RUST_LOG=]

-W, --warn <WARNINGS>...
Enable lint warnings.
This is a comma-separated list of warnings to enable.
Each warning is specified by its name, which is one of:
* `self-wakes` -- Warns when a task wakes itself more than a
certain percentage of its total wakeups. Default percentage is
50%.
* `lost-waker` -- Warns when a task is dropped without being
woken.
* `never-yielded` -- Warns when a task has never yielded.
[default: self-wakes lost-waker never-yielded]
[possible values: self-wakes, lost-waker, never-yielded]

--log-dir <LOG_DIRECTORY>
Path to a directory to write the console's internal logs to.
Expand Down
4 changes: 2 additions & 2 deletions tokio-console/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,5 @@ toml = "0.5"
dirs = "5"

[dev-dependencies]
# Use 1.64.0 compatible version of `trycmd@0.13.4`.
trycmd = "=0.13.4"
# Use 1.64.0 compatible version of `trycmd@0.13.6`.
trycmd = "=0.13.6"
5 changes: 5 additions & 0 deletions tokio-console/console.example.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
default_target_addr = 'http://127.0.0.1:6669/'
log = 'off'
warnings = [
'self-wakes',
'lost-waker',
'never-yielded',
]
log_directory = '/tmp/tokio-console/logs'
retention = '6s'

Expand Down
69 changes: 69 additions & 0 deletions tokio-console/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::state::tasks::Task;
use crate::view::Palette;
use crate::warnings;
use clap::builder::{PossibleValuesParser, TypedValueParser};
use clap::{ArgAction, ArgGroup, CommandFactory, Parser as Clap, Subcommand, ValueHint};
use clap_complete::Shell;
Expand Down Expand Up @@ -48,6 +50,23 @@ pub struct Config {
#[clap(long = "log", env = "RUST_LOG")]
log_filter: Option<LogFilter>,

/// Enable lint warnings.
///
/// This is a comma-separated list of warnings to enable.
///
/// Each warning is specified by its name, which is one of:
///
/// * `self-wakes` -- Warns when a task wakes itself more than a certain percentage of its total wakeups.
/// Default percentage is 50%.
///
/// * `lost-waker` -- Warns when a task is dropped without being woken.
///
/// * `never-yielded` -- Warns when a task has never yielded.
///
#[clap(long = "warn", short = 'W', value_delimiter = ',', num_args = 1..)]
#[clap(default_values_t = KnownWarnings::default_enabled_warnings())]
pub(crate) warnings: Vec<KnownWarnings>,

/// Path to a directory to write the console's internal logs to.
///
/// [default: /tmp/tokio-console/logs]
Expand Down Expand Up @@ -98,6 +117,45 @@ pub struct Config {
pub subcmd: Option<OptionalCmd>,
}

/// Known warnings that can be enabled or disabled.
#[derive(clap::ValueEnum, Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)]
#[serde(rename_all = "kebab-case")]
pub(crate) enum KnownWarnings {
SelfWakes,
LostWaker,
NeverYielded,
}

impl From<&KnownWarnings> for warnings::Linter<Task> {
fn from(warning: &KnownWarnings) -> Self {
match warning {
KnownWarnings::SelfWakes => warnings::Linter::new(warnings::SelfWakePercent::default()),
KnownWarnings::LostWaker => warnings::Linter::new(warnings::LostWaker),
KnownWarnings::NeverYielded => warnings::Linter::new(warnings::NeverYielded::default()),
}
}
}

impl fmt::Display for KnownWarnings {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
KnownWarnings::SelfWakes => write!(f, "self-wakes"),
KnownWarnings::LostWaker => write!(f, "lost-waker"),
KnownWarnings::NeverYielded => write!(f, "never-yielded"),
}
}
}

impl KnownWarnings {
fn default_enabled_warnings() -> Vec<Self> {
vec![
KnownWarnings::SelfWakes,
KnownWarnings::LostWaker,
KnownWarnings::NeverYielded,
]
}
}

#[derive(Debug, Subcommand, PartialEq, Eq)]
pub enum OptionalCmd {
/// Generate a `console.toml` config file with the default configuration
Expand Down Expand Up @@ -240,6 +298,7 @@ impl FromStr for LogFilter {
struct ConfigFile {
default_target_addr: Option<String>,
log: Option<String>,
warnings: Vec<KnownWarnings>,
log_directory: Option<PathBuf>,
retention: Option<RetainFor>,
charset: Option<CharsetConfig>,
Expand Down Expand Up @@ -428,6 +487,13 @@ impl Config {
log_directory: other.log_directory.or(self.log_directory),
target_addr: other.target_addr.or(self.target_addr),
log_filter: other.log_filter.or(self.log_filter),
warnings: {
let mut warns: Vec<KnownWarnings> = other.warnings;
warns.extend(self.warnings);
warns.sort_unstable();
warns.dedup();
warns
},
retain_for: other.retain_for.or(self.retain_for),
view_options: self.view_options.merge_with(other.view_options),
subcmd: other.subcmd.or(self.subcmd),
Expand All @@ -442,6 +508,7 @@ impl Default for Config {
log_filter: Some(LogFilter(
filter::Targets::new().with_default(filter::LevelFilter::OFF),
)),
warnings: KnownWarnings::default_enabled_warnings(),
log_directory: Some(default_log_directory()),
retain_for: Some(RetainFor::default()),
view_options: ViewOptions::default(),
Expand Down Expand Up @@ -677,6 +744,7 @@ impl From<Config> for ConfigFile {
default_target_addr: config.target_addr.map(|addr| addr.to_string()),
log: config.log_filter.map(|filter| filter.to_string()),
log_directory: config.log_directory,
warnings: config.warnings,
retention: config.retain_for,
charset: Some(CharsetConfig {
lang: config.view_options.lang,
Expand All @@ -699,6 +767,7 @@ impl TryFrom<ConfigFile> for Config {
Ok(Config {
target_addr: value.target_addr()?,
log_filter: value.log_filter()?,
warnings: value.warnings.clone(),
log_directory: value.log_directory.take(),
retain_for: value.retain_for(),
view_options: ViewOptions {
Expand Down
8 changes: 1 addition & 7 deletions tokio-console/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,7 @@ async fn main() -> color_eyre::Result<()> {
let (details_tx, mut details_rx) = mpsc::channel::<TaskDetails>(2);

let mut state = State::default()
// TODO(eliza): allow configuring the list of linters via the
// CLI/possibly a config file?
.with_task_linters(vec![
warnings::Linter::new(warnings::SelfWakePercent::default()),
warnings::Linter::new(warnings::LostWaker),
warnings::Linter::new(warnings::NeverYielded::default()),
])
.with_task_linters(args.warnings.iter().map(|lint| lint.into()))
.with_retain_for(retain_for);
let mut input = Box::pin(input::EventStream::new().try_filter(|event| {
future::ready(!matches!(
Expand Down
19 changes: 19 additions & 0 deletions tokio-console/tests/cli-ui.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,25 @@ Options:

[env: RUST_LOG=]

-W, --warn <WARNINGS>...
Enable lint warnings.

This is a comma-separated list of warnings to enable.

Each warning is specified by its name, which is one of:

* `self-wakes` -- Warns when a task wakes itself more than a
certain percentage of its total wakeups. Default percentage is
50%.

* `lost-waker` -- Warns when a task is dropped without being
woken.

* `never-yielded` -- Warns when a task has never yielded.

[default: self-wakes lost-waker never-yielded]
[possible values: self-wakes, lost-waker, never-yielded]

--log-dir <LOG_DIRECTORY>
Path to a directory to write the console's internal logs to.

Expand Down

0 comments on commit 174883f

Please sign in to comment.