Skip to content
This repository was archived by the owner on Oct 10, 2023. It is now read-only.

Add --debug flag which sets the log level to debug#146

Merged
cole-h merged 2 commits intomainfrom
cole/ds-325-make-it-easier-to-debug-riff
Sep 28, 2022
Merged

Add --debug flag which sets the log level to debug#146
cole-h merged 2 commits intomainfrom
cole/ds-325-make-it-easier-to-debug-riff

Conversation

@cole-h
Copy link
Copy Markdown
Member

@cole-h cole-h commented Sep 28, 2022

This will print out additional debug logging. It also displays the telemetry that was sent (if it was sent).


--debug takes precedence over the RUST_LOG env var, but I don't know of a good way to communicate this happens because, well, tracing hasn't been set up yet... Just gotta add a directive for us (and only us) that will override the "default" of info! Thanks, Ana!

This will print out additional debug logging. It also displays the
telemetry that was sent (if it was sent).
@cole-h cole-h requested review from Hoverbear and grahamc September 28, 2022 17:36
Copy link
Copy Markdown
Contributor

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

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

I would prefer it if --debug added a Directive (https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html#directives) of riff=debug without harming other portions of the RUST_LOG which may exist.

This is so a user could still do like RUST_LOG=mio=trace riff --debug.

I used to do this like

    pub(crate) fn filter_layer(&self) -> eyre::Result<EnvFilter> {
        let mut filter_layer = match EnvFilter::try_from_default_env() {
            Ok(layer) => layer,
            Err(e) => {
                // Catch a parse error and report it, ignore a missing env.
                if let Some(source) = e.source() {
                    match source.downcast_ref::<std::env::VarError>() {
                        Some(std::env::VarError::NotPresent) => (),
                        _ => return Err(e).wrap_err_with(|| "parsing RUST_LOG directives"),
                    }
                }
                EnvFilter::try_new(&format!("{}={}", env!("CARGO_PKG_NAME"), self.log_level()))?
            },
        };
        for directive in &self.log_level {
            // 'clone' the directive. (PR: https://github.com/tokio-rs/tracing/pull/1985)
            let directive_str = directive.to_string();
            let directive = Directive::from_str(&directive_str)?;
            filter_layer = filter_layer.add_directive(directive);
        }

        Ok(filter_layer)
    }

Where this was some arg in the clap struct:

    /// Tracing directives
    ///
    /// This application also understands the `RUST_LOG` environment variable.
    ///
    /// See https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html#directives
    #[clap(long, global = true, group = "verbosity", multiple_occurrences = true)]
    pub(crate) log_level: Vec<Directive>,

However that was for random hobby stuff, so we may want some different behavior.

This allows other RUST_LOG directives to take effect (e.g.
`RUST_LOG=mio=trace`).
Copy link
Copy Markdown
Contributor

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

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

This seems good. :)

@cole-h cole-h merged commit c47ff5c into main Sep 28, 2022
@cole-h cole-h deleted the cole/ds-325-make-it-easier-to-debug-riff branch September 28, 2022 18:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants