Skip to content

Commit

Permalink
feat(console): add --allow flag (#513)
Browse files Browse the repository at this point in the history
## Description

This pull request adds a new flag to `tokio-console` that allows warning
lints to be selectively disabled. Additionally, you can use the value
`all` to disable all warnings.  For example, you can use it like this:

```sh
cargo run -- --warn "self-wakes,lost-waker,never-yielded" --allow "self-wakes,never-yielded"
```

Which will warn on all three of the current lints, but then allow the
`self-wakes` and `never-yielded` lints. The end result will be that only
the `lost-wakers` lint will be "active".

This PR follows from [comments in #493].

## Explanation of Changes

Added an `AllowedWarnings` enum to the command line interface to
represent the allowed warnings. `All` for disable all warnings.
`Explicit` explicitly indicates which warning can be ignored.

[comments in #493]: #493 (comment)
  • Loading branch information
Rustin170506 authored Feb 13, 2024
1 parent f28ba4a commit 8da7037
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 2 deletions.
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,26 @@ Options:
[default: self-wakes lost-waker never-yielded]
[possible values: self-wakes, lost-waker, never-yielded]

-A, --allow <ALLOW_WARNINGS>...
Allow lint warnings.

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

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.

If this is set to `all`, all warnings are allowed.

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

--log-dir <LOG_DIRECTORY>
Path to a directory to write the console's internal logs to.
Expand Down
87 changes: 86 additions & 1 deletion tokio-console/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use clap::{ArgAction, ArgGroup, CommandFactory, Parser as Clap, Subcommand, Valu
use clap_complete::Shell;
use color_eyre::eyre::WrapErr;
use serde::{Deserialize, Serialize};
use std::collections::BTreeSet;
use std::fmt;
use std::fs;
use std::ops::Not;
Expand Down Expand Up @@ -62,11 +63,29 @@ pub struct Config {
/// * `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>,

/// Allow lint warnings.
///
/// This is a comma-separated list of warnings to allow.
///
/// 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.
///
/// If this is set to `all`, all warnings are allowed.
///
/// [possible values: all, self-wakes, lost-waker, never-yielded]
#[clap(long = "allow", short = 'A', num_args = 1..)]
pub(crate) allow_warnings: Option<AllowedWarnings>,

/// Path to a directory to write the console's internal logs to.
///
/// [default: /tmp/tokio-console/logs]
Expand Down Expand Up @@ -126,6 +145,19 @@ pub(crate) enum KnownWarnings {
NeverYielded,
}

impl FromStr for KnownWarnings {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"self-wakes" => Ok(KnownWarnings::SelfWakes),
"lost-waker" => Ok(KnownWarnings::LostWaker),
"never-yielded" => Ok(KnownWarnings::NeverYielded),
_ => Err(format!("unknown warning: {}", s)),
}
}
}

impl From<&KnownWarnings> for warnings::Linter<Task> {
fn from(warning: &KnownWarnings) -> Self {
match warning {
Expand Down Expand Up @@ -155,6 +187,49 @@ impl KnownWarnings {
]
}
}
/// An enum representing the types of warnings that are allowed.
// Note: ValueEnum only supports unit variants, so we have to use a custom
// parser for this.
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)]
pub(crate) enum AllowedWarnings {
/// Represents the case where all warnings are allowed.
All,
/// Represents the case where only some specific warnings are allowed.
/// The allowed warnings are stored in a `BTreeSet` of `KnownWarnings`.
Explicit(BTreeSet<KnownWarnings>),
}

impl FromStr for AllowedWarnings {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"all" => Ok(AllowedWarnings::All),
_ => {
let warnings = s
.split(',')
.map(|s| s.parse::<KnownWarnings>())
.collect::<Result<BTreeSet<_>, _>>()
.map_err(|err| format!("failed to parse warning: {}", err))?;
Ok(AllowedWarnings::Explicit(warnings))
}
}
}
}

impl AllowedWarnings {
fn merge(&self, allowed: &Self) -> Self {
match (self, allowed) {
(AllowedWarnings::All, _) => AllowedWarnings::All,
(_, AllowedWarnings::All) => AllowedWarnings::All,
(AllowedWarnings::Explicit(a), AllowedWarnings::Explicit(b)) => {
let mut warnings = a.clone();
warnings.extend(b.clone());
AllowedWarnings::Explicit(warnings)
}
}
}
}

#[derive(Debug, Subcommand, PartialEq, Eq)]
pub enum OptionalCmd {
Expand Down Expand Up @@ -299,6 +374,7 @@ struct ConfigFile {
default_target_addr: Option<String>,
log: Option<String>,
warnings: Vec<KnownWarnings>,
allow_warnings: Option<AllowedWarnings>,
log_directory: Option<PathBuf>,
retention: Option<RetainFor>,
charset: Option<CharsetConfig>,
Expand Down Expand Up @@ -494,6 +570,12 @@ impl Config {
warns.dedup();
warns
},
allow_warnings: {
match (self.allow_warnings, other.allow_warnings) {
(Some(a), Some(b)) => Some(a.merge(&b)),
(a, b) => a.or(b),
}
},
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 @@ -509,6 +591,7 @@ impl Default for Config {
filter::Targets::new().with_default(filter::LevelFilter::OFF),
)),
warnings: KnownWarnings::default_enabled_warnings(),
allow_warnings: None,
log_directory: Some(default_log_directory()),
retain_for: Some(RetainFor::default()),
view_options: ViewOptions::default(),
Expand Down Expand Up @@ -745,6 +828,7 @@ impl From<Config> for ConfigFile {
log: config.log_filter.map(|filter| filter.to_string()),
log_directory: config.log_directory,
warnings: config.warnings,
allow_warnings: config.allow_warnings,
retention: config.retain_for,
charset: Some(CharsetConfig {
lang: config.view_options.lang,
Expand All @@ -768,6 +852,7 @@ impl TryFrom<ConfigFile> for Config {
target_addr: value.target_addr()?,
log_filter: value.log_filter()?,
warnings: value.warnings.clone(),
allow_warnings: value.allow_warnings.clone(),
log_directory: value.log_directory.take(),
retain_for: value.retain_for(),
view_options: ViewOptions {
Expand Down
12 changes: 11 additions & 1 deletion tokio-console/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use ratatui::{
use tokio::sync::{mpsc, watch};

use crate::{
config::AllowedWarnings,
input::{Event, KeyEvent, KeyEventKind},
view::{bold, UpdateKind},
};
Expand Down Expand Up @@ -64,9 +65,18 @@ async fn main() -> color_eyre::Result<()> {
let (update_tx, update_rx) = watch::channel(UpdateKind::Other);
// A channel to send the task details update stream (no need to keep outdated details in the memory)
let (details_tx, mut details_rx) = mpsc::channel::<TaskDetails>(2);
let warnings = match args.allow_warnings {
Some(AllowedWarnings::All) => vec![],
Some(AllowedWarnings::Explicit(allow_warnings)) => args
.warnings
.iter()
.filter(|lint| !allow_warnings.contains(lint))
.collect::<Vec<_>>(),
None => args.warnings.iter().collect::<Vec<_>>(),
};

let mut state = State::default()
.with_task_linters(args.warnings.iter().map(|lint| lint.into()))
.with_task_linters(warnings.into_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
20 changes: 20 additions & 0 deletions tokio-console/tests/cli-ui.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,26 @@ Options:
[default: self-wakes lost-waker never-yielded]
[possible values: self-wakes, lost-waker, never-yielded]

-A, --allow <ALLOW_WARNINGS>...
Allow lint warnings.

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

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.

If this is set to `all`, all warnings are allowed.

[possible values: all, 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 8da7037

Please sign in to comment.