Skip to content

feat(rustup-init): set log level to WARN on -q if RUSTUP_LOG is unset #3911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion rustup-init.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Options:
-v, --verbose
Enable verbose output
-q, --quiet
Disable progress output
Disable progress output, limit console logger level to 'WARN' if 'RUSTUP_LOG' is unset
-y
Disable confirmation prompt
--default-host <DEFAULT_HOST>
Expand Down
19 changes: 13 additions & 6 deletions src/bin/rustup-init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use cfg_if::cfg_if;
use rs_tracing::{
close_trace_file, close_trace_file_internal, open_trace_file, trace_to_file_internal,
};
use tracing_subscriber::{reload::Handle, EnvFilter, Registry};

use rustup::cli::common;
use rustup::cli::proxy_mode;
Expand All @@ -44,9 +45,9 @@ async fn main() -> Result<ExitCode> {
opentelemetry::global::set_text_map_propagator(
opentelemetry_sdk::propagation::TraceContextPropagator::new(),
);
let subscriber = rustup::cli::log::tracing_subscriber(&process);
let (subscriber, console_filter) = rustup::cli::log::tracing_subscriber(&process);
tracing::subscriber::set_global_default(subscriber)?;
let result = run_rustup(&process).await;
let result = run_rustup(&process, console_filter).await;
// We're tracing, so block until all spans are exported.
#[cfg(feature = "otel")]
opentelemetry::global::shutdown_tracer_provider();
Expand All @@ -61,19 +62,25 @@ async fn main() -> Result<ExitCode> {
}

#[cfg_attr(feature = "otel", tracing::instrument)]
async fn run_rustup(process: &Process) -> Result<utils::ExitCode> {
async fn run_rustup(
process: &Process,
console_filter: Handle<EnvFilter, Registry>,
) -> Result<utils::ExitCode> {
if let Ok(dir) = process.var("RUSTUP_TRACE_DIR") {
open_trace_file!(dir)?;
}
let result = run_rustup_inner(process).await;
let result = run_rustup_inner(process, console_filter).await;
if process.var("RUSTUP_TRACE_DIR").is_ok() {
close_trace_file!();
}
result
}

#[cfg_attr(feature = "otel", tracing::instrument(err))]
async fn run_rustup_inner(process: &Process) -> Result<utils::ExitCode> {
async fn run_rustup_inner(
process: &Process,
console_filter: Handle<EnvFilter, Registry>,
) -> Result<utils::ExitCode> {
// Guard against infinite proxy recursion. This mostly happens due to
// bugs in rustup.
do_recursion_guard(process)?;
Expand All @@ -92,7 +99,7 @@ async fn run_rustup_inner(process: &Process) -> Result<utils::ExitCode> {
// name. Browsers rename duplicates to
// e.g. rustup-setup(2), and this allows all variations
// to work.
setup_mode::main(current_dir, process).await
setup_mode::main(current_dir, process, console_filter).await
}
Some(n) if n.starts_with("rustup-gc-") => {
// This is the final uninstallation stage on windows where
Expand Down
51 changes: 31 additions & 20 deletions src/cli/log.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,40 @@
use std::{fmt, io::Write};

#[cfg(feature = "otel")]
use opentelemetry_sdk::trace::Tracer;
use termcolor::{Color, ColorSpec, WriteColor};
use tracing::{level_filters::LevelFilter, Event, Subscriber};
use tracing_subscriber::{
fmt::{
format::{self, FormatEvent, FormatFields},
FmtContext,
},
layer::SubscriberExt,
registry::LookupSpan,
EnvFilter, Layer,
reload, EnvFilter, Layer, Registry,
};

#[cfg(feature = "otel")]
use opentelemetry_sdk::trace::Tracer;

use crate::{currentprocess::Process, utils::notify::NotificationLevel};

pub fn tracing_subscriber(process: &Process) -> impl tracing::Subscriber {
use tracing_subscriber::{layer::SubscriberExt, Registry};

pub fn tracing_subscriber(
process: &Process,
) -> (
impl tracing::Subscriber,
reload::Handle<EnvFilter, Registry>,
) {
#[cfg(feature = "otel")]
let telemetry = telemetry(process);
let console_logger = console_logger(process);
let (console_logger, console_filter) = console_logger(process);
#[cfg(feature = "otel")]
{
Registry::default().with(console_logger).with(telemetry)
(
Registry::default().with(console_logger).with(telemetry),
console_filter,
)
}
#[cfg(not(feature = "otel"))]
{
Registry::default().with(console_logger)
(Registry::default().with(console_logger), console_filter)
}
}

Expand All @@ -38,7 +44,7 @@ pub fn tracing_subscriber(process: &Process) -> impl tracing::Subscriber {
/// When the `RUSTUP_LOG` environment variable is present, a standard [`tracing_subscriber`]
/// formatter will be used according to the filtering directives set in its value.
/// Otherwise, this logger will use [`EventFormatter`] to mimic "classic" Rustup `stderr` output.
fn console_logger<S>(process: &Process) -> impl Layer<S>
fn console_logger<S>(process: &Process) -> (impl Layer<S>, reload::Handle<EnvFilter, S>)
where
S: Subscriber + for<'span> LookupSpan<'span>,
{
Expand All @@ -55,17 +61,22 @@ where
.with_writer(move || process.stderr())
.with_ansi(has_ansi);
if let Ok(directives) = maybe_rustup_log_directives {
let env_filter = EnvFilter::builder()
.with_default_directive(LevelFilter::INFO.into())
.parse_lossy(directives);
logger.compact().with_filter(env_filter).boxed()
let (env_filter, handle) = reload::Layer::new(
EnvFilter::builder()
.with_default_directive(LevelFilter::INFO.into())
.parse_lossy(directives),
);
(logger.compact().with_filter(env_filter).boxed(), handle)
} else {
// Receive log lines from Rustup only.
let env_filter = EnvFilter::new("rustup=DEBUG");
logger
.event_format(EventFormatter)
.with_filter(env_filter)
.boxed()
let (env_filter, handle) = reload::Layer::new(EnvFilter::new("rustup=DEBUG"));
(
logger
.event_format(EventFormatter)
.with_filter(env_filter)
.boxed(),
handle,
)
}
}

Expand Down
15 changes: 13 additions & 2 deletions src/cli/setup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::path::PathBuf;
use anyhow::Result;
use clap::Parser;
use tracing::warn;
use tracing_subscriber::{reload::Handle, EnvFilter, Registry};

use crate::{
cli::{
Expand All @@ -28,7 +29,7 @@ struct RustupInit {
#[arg(short, long)]
verbose: bool,

/// Disable progress output
/// Disable progress output, limit console logger level to 'WARN' if 'RUSTUP_LOG' is unset
#[arg(short, long)]
quiet: bool,

Expand Down Expand Up @@ -73,7 +74,11 @@ struct RustupInit {
}

#[cfg_attr(feature = "otel", tracing::instrument)]
pub async fn main(current_dir: PathBuf, process: &Process) -> Result<utils::ExitCode> {
pub async fn main(
current_dir: PathBuf,
process: &Process,
console_filter: Handle<EnvFilter, Registry>,
) -> Result<utils::ExitCode> {
use clap::error::ErrorKind;

let RustupInit {
Expand Down Expand Up @@ -111,6 +116,12 @@ pub async fn main(current_dir: PathBuf, process: &Process) -> Result<utils::Exit
warn!("{}", common::WARN_COMPLETE_PROFILE);
}

if quiet && process.var("RUSTUP_LOG").is_err() {
console_filter
.modify(|it| *it = EnvFilter::new("rustup=WARN"))
.expect("error reloading `EnvFilter` for console_logger");
}

let opts = InstallOpts {
default_host_triple: default_host,
default_toolchain,
Expand Down
2 changes: 1 addition & 1 deletion src/currentprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl TestProcess {
impl From<TestContext> for TestProcess {
fn from(inner: TestContext) -> Self {
let inner = Process::TestProcess(inner);
let guard = crate::cli::log::tracing_subscriber(&inner).set_default();
let guard = crate::cli::log::tracing_subscriber(&inner).0.set_default();
Self {
process: inner,
_guard: guard,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Options:
-v, --verbose
Enable verbose output
-q, --quiet
Disable progress output
Disable progress output, limit console logger level to 'WARN' if 'RUSTUP_LOG' is unset
-y
Disable confirmation prompt
--default-host <DEFAULT_HOST>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Options:
-v, --verbose
Enable verbose output
-q, --quiet
Disable progress output
Disable progress output, limit console logger level to 'WARN' if 'RUSTUP_LOG' is unset
-y
Disable confirmation prompt
--default-host <DEFAULT_HOST>
Expand Down
Loading