Skip to content

Commit

Permalink
Revert 1 commits
Browse files Browse the repository at this point in the history
0cf0c6b 'feat(log): unhide `tracing::instrument` from behind `feature = "otel"`'
  • Loading branch information
rami3l committed Jun 14, 2024
1 parent 0cf0c6b commit 1b7662e
Show file tree
Hide file tree
Showing 18 changed files with 38 additions and 37 deletions.
2 changes: 1 addition & 1 deletion doc/dev-guide/src/tracing.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ when enabled. Instrumenting a currently uninstrumented function is mostly simply
done like so:

```rust
#[tracing::instrument(level = "trace", err, skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(err, skip_all))]
```

`skip_all` is not required, but some core structs don't implement Debug yet, and
Expand Down
2 changes: 1 addition & 1 deletion rustup-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ fn test_inner(mod_path: String, mut input: ItemFn) -> syn::Result<TokenStream> {
#before_ident().await;
// Define a function with same name we can instrument inside the
// tracing enablement logic.
#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
async fn #name() { #inner }
// Thunk through a new thread to permit catching the panic
// without grabbing the entire state machine defined by the
Expand Down
5 changes: 3 additions & 2 deletions src/bin/rustup-init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ async fn maybe_trace_rustup() -> Result<utils::ExitCode> {
result
}

#[tracing::instrument(level = "trace")]
// FIXME: Make `tracing::instrument` always run
#[cfg_attr(feature = "otel", tracing::instrument)]
async fn run_rustup() -> Result<utils::ExitCode> {
if let Ok(dir) = process().var("RUSTUP_TRACE_DIR") {
open_trace_file!(dir)?;
Expand All @@ -79,7 +80,7 @@ async fn run_rustup() -> Result<utils::ExitCode> {
result
}

#[tracing::instrument(level = "trace", err)]
#[cfg_attr(feature = "otel", tracing::instrument(err))]
async fn run_rustup_inner() -> Result<utils::ExitCode> {
// Guard against infinite proxy recursion. This mostly happens due to
// bugs in rustup.
Expand Down
2 changes: 1 addition & 1 deletion src/cli/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ impl Notifier {
}
}

#[tracing::instrument(level = "trace")]
#[cfg_attr(feature = "otel", tracing::instrument)]
pub(crate) fn set_globals(current_dir: PathBuf, verbose: bool, quiet: bool) -> Result<Cfg> {
let notifier = Notifier::new(verbose, quiet);
Cfg::from_env(current_dir, Arc::new(move |n| notifier.handle(n)))
Expand Down
2 changes: 1 addition & 1 deletion src/cli/proxy_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
toolchain::names::ResolvableLocalToolchainName,
};

#[tracing::instrument(level = "trace")]
#[cfg_attr(feature = "otel", tracing::instrument)]
pub async fn main(arg0: &str, current_dir: PathBuf) -> Result<ExitStatus> {
self_update::cleanup_self_updater()?;

Expand Down
10 changes: 5 additions & 5 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ enum SetSubcmd {
},
}

#[tracing::instrument(level = "trace", fields(args = format!("{:?}", process().args_os().collect::<Vec<_>>())))]
#[cfg_attr(feature = "otel", tracing::instrument(fields(args = format!("{:?}", process().args_os().collect::<Vec<_>>()))))]
pub async fn main(current_dir: PathBuf) -> Result<utils::ExitCode> {
self_update::cleanup_self_updater()?;

Expand All @@ -550,7 +550,7 @@ pub async fn main(current_dir: PathBuf) -> Result<utils::ExitCode> {
write!(process().stdout().lock(), "{err}")?;
info!("This is the version for the rustup toolchain manager, not the rustc compiler.");

#[tracing::instrument(level = "trace")]
#[cfg_attr(feature = "otel", tracing::instrument)]
async fn rustc_version(
current_dir: PathBuf,
) -> std::result::Result<String, Box<dyn std::error::Error>> {
Expand Down Expand Up @@ -938,7 +938,7 @@ async fn which(
Ok(utils::ExitCode(0))
}

#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
fn show(cfg: &Cfg, verbose: bool) -> Result<utils::ExitCode> {
common::warn_if_host_is_emulated();

Expand Down Expand Up @@ -1075,7 +1075,7 @@ fn show(cfg: &Cfg, verbose: bool) -> Result<utils::ExitCode> {
Ok(utils::ExitCode(0))
}

#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
fn show_active_toolchain(cfg: &Cfg, verbose: bool) -> Result<utils::ExitCode> {
match cfg.find_active_toolchain()? {
Some((toolchain_name, reason)) => {
Expand All @@ -1099,7 +1099,7 @@ fn show_active_toolchain(cfg: &Cfg, verbose: bool) -> Result<utils::ExitCode> {
Ok(utils::ExitCode(0))
}

#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
fn show_rustup_home(cfg: &Cfg) -> Result<utils::ExitCode> {
writeln!(process().stdout().lock(), "{}", cfg.rustup_dir.display())?;
Ok(utils::ExitCode(0))
Expand Down
2 changes: 1 addition & 1 deletion src/cli/self_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1321,7 +1321,7 @@ pub(crate) async fn check_rustup_update() -> Result<()> {
Ok(())
}

#[tracing::instrument(level = "trace")]
#[cfg_attr(feature = "otel", tracing::instrument)]
pub(crate) fn cleanup_self_updater() -> Result<()> {
let cargo_home = utils::cargo_home()?;
let setup = cargo_home.join(format!("bin/rustup-init{EXE_SUFFIX}"));
Expand Down
2 changes: 1 addition & 1 deletion src/cli/self_update/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ fn has_windows_sdk_libs() -> bool {
}

/// Run by rustup-gc-$num.exe to delete CARGO_HOME
#[tracing::instrument(level = "trace")]
#[cfg_attr(feature = "otel", tracing::instrument)]
pub fn complete_windows_uninstall() -> Result<utils::ExitCode> {
use std::process::Stdio;

Expand Down
2 changes: 1 addition & 1 deletion src/cli/setup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ struct RustupInit {
dump_testament: bool,
}

#[tracing::instrument(level = "trace")]
#[cfg_attr(feature = "otel", tracing::instrument)]
pub async fn main(current_dir: PathBuf) -> Result<utils::ExitCode> {
use clap::error::ErrorKind;

Expand Down
2 changes: 1 addition & 1 deletion src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use anyhow::{Context, Result};

use crate::errors::*;

#[tracing::instrument(level = "trace", err)]
#[cfg_attr(feature = "otel", tracing::instrument(err))]
pub(crate) fn run_command_for_dir<S: AsRef<OsStr> + Debug>(
mut cmd: Command,
arg0: &str,
Expand Down
10 changes: 5 additions & 5 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ impl Cfg {
Ok(self.update_hash_dir.join(toolchain.to_string()))
}

#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
pub(crate) fn upgrade_data(&self) -> Result<()> {
let current_version = self.settings_file.with(|s| Ok(s.version))?;
if current_version == MetadataVersion::default() {
Expand Down Expand Up @@ -693,7 +693,7 @@ impl Cfg {
.ok_or_else(no_toolchain_error)
}

#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
pub(crate) async fn maybe_find_or_install_active_toolchain(
&self,
path: &Path,
Expand Down Expand Up @@ -801,7 +801,7 @@ impl Cfg {
/// - not files
/// - named with a valid resolved toolchain name
/// Currently no notification of incorrect names or entry type is done.
#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
pub(crate) fn list_toolchains(&self) -> Result<Vec<ToolchainName>> {
if utils::is_directory(&self.toolchains_dir) {
let mut toolchains: Vec<_> = utils::read_dir("toolchains", &self.toolchains_dir)?
Expand Down Expand Up @@ -871,7 +871,7 @@ impl Cfg {
Ok(channels.collect().await)
}

#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
pub(crate) fn check_metadata_version(&self) -> Result<()> {
utils::assert_is_directory(&self.rustup_dir)?;

Expand Down Expand Up @@ -899,7 +899,7 @@ impl Cfg {
})
}

#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
pub(crate) fn get_default_host_triple(&self) -> Result<dist::TargetTriple> {
self.settings_file.with(|s| Ok(get_default_host_triple(s)))
}
Expand Down
2 changes: 1 addition & 1 deletion src/dist/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ pub(crate) fn valid_profile_names() -> String {
// an upgrade then all the existing components will be upgraded.
//
// Returns the manifest's hash if anything changed.
#[tracing::instrument(level = "trace", err, skip_all, fields(profile=format!("{profile:?}"), prefix=prefix.path().to_string_lossy().to_string()))]
#[cfg_attr(feature = "otel", tracing::instrument(err, skip_all, fields(profile=format!("{profile:?}"), prefix=prefix.path().to_string_lossy().to_string())))]
pub(crate) async fn update_from_dist(
download: DownloadCfg<'_>,
update_hash: Option<&Path>,
Expand Down
2 changes: 1 addition & 1 deletion src/dist/manifestation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ impl Manifestation {
}
}

#[tracing::instrument(level = "trace")]
#[cfg_attr(feature = "otel", tracing::instrument)]
pub fn load_manifest(&self) -> Result<Option<Manifest>> {
let prefix = self.installation.prefix();
let old_manifest_path = prefix.manifest_file(DIST_MANIFEST);
Expand Down
2 changes: 1 addition & 1 deletion src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub(crate) enum InstallMethod<'a> {

impl<'a> InstallMethod<'a> {
// Install a toolchain
#[tracing::instrument(level = "trace", err, skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(err, skip_all))]
pub(crate) async fn install(&self) -> Result<UpdateStatus> {
let nh = self.cfg().notify_handler.clone();
match self {
Expand Down
4 changes: 2 additions & 2 deletions src/test/mock/clitools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,7 @@ impl Release {
}
}

#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
fn link(&self, path: &Path) {
// Also create the manifests for releases by version
let _ = hard_link(
Expand Down Expand Up @@ -955,7 +955,7 @@ impl Release {
}

// Creates a mock dist server populated with some test data
#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
fn create_mock_dist_server(path: &Path, s: Scenario) {
let chans = match s {
Scenario::None => return,
Expand Down
10 changes: 5 additions & 5 deletions src/test/mock/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ pub enum MockManifestVersion {
}

impl MockDistServer {
#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
pub fn write(&self, vs: &[MockManifestVersion], enable_xz: bool, enable_zst: bool) {
fs::create_dir_all(&self.path).unwrap();

Expand All @@ -149,7 +149,7 @@ impl MockDistServer {
}
}

#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
fn build_package(
&self,
channel: &MockChannel,
Expand Down Expand Up @@ -190,7 +190,7 @@ impl MockDistServer {
}

// Returns the hash of the tarball
#[tracing::instrument(level = "trace", skip_all, fields(format=%format))]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all, fields(format=%format)))]
fn build_target_package(
&self,
channel: &MockChannel,
Expand Down Expand Up @@ -277,7 +277,7 @@ impl MockDistServer {
}

// The v1 manifest is just the directory listing of the rust tarballs
#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
fn write_manifest_v1(&self, channel: &MockChannel) {
let mut buf = String::new();
let package = channel.packages.iter().find(|p| p.name == "rust").unwrap();
Expand Down Expand Up @@ -306,7 +306,7 @@ impl MockDistServer {
hard_link(&hash_path, archive_hash_path).unwrap();
}

#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
fn write_manifest_v2(
&self,
channel: &MockChannel,
Expand Down
12 changes: 6 additions & 6 deletions src/toolchain/distributable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,14 +306,14 @@ impl<'a> DistributableToolchain<'a> {
}
}

#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
pub(crate) fn get_manifestation(&self) -> anyhow::Result<Manifestation> {
let prefix = InstallPrefix::from(self.toolchain.path());
Manifestation::open(prefix, self.desc.target.clone())
}

/// Get the manifest associated with this distribution
#[tracing::instrument(level = "trace", skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))]
pub(crate) fn get_manifest(&self) -> anyhow::Result<Manifest> {
self.get_manifestation()?
.load_manifest()
Expand All @@ -329,7 +329,7 @@ impl<'a> DistributableToolchain<'a> {
InstallPrefix::from(self.toolchain.path().to_owned()).guess_v1_manifest()
}

#[tracing::instrument(level = "trace", err, skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(err, skip_all))]
pub(crate) async fn install(
cfg: &'a Cfg,
desc: &'_ ToolchainDesc,
Expand Down Expand Up @@ -359,7 +359,7 @@ impl<'a> DistributableToolchain<'a> {
Ok((status, Self::new(cfg, desc.clone())?))
}

#[tracing::instrument(level = "trace", err, skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(err, skip_all))]
pub async fn install_if_not_installed(
cfg: &'a Cfg,
desc: &'a ToolchainDesc,
Expand All @@ -377,7 +377,7 @@ impl<'a> DistributableToolchain<'a> {
}
}

#[tracing::instrument(level = "trace", err, skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(err, skip_all))]
pub(crate) async fn update(
&mut self,
components: &[&str],
Expand All @@ -389,7 +389,7 @@ impl<'a> DistributableToolchain<'a> {
}

/// Update a toolchain with control over the channel behaviour
#[tracing::instrument(level = "trace", err, skip_all)]
#[cfg_attr(feature = "otel", tracing::instrument(err, skip_all))]
pub(crate) async fn update_extra(
&mut self,
components: &[&str],
Expand Down
2 changes: 1 addition & 1 deletion src/toolchain/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ impl<'a> Toolchain<'a> {
}

/// Infallible function that describes the version of rustc in an installed distribution
#[tracing::instrument(level = "trace")]
#[cfg_attr(feature = "otel", tracing::instrument)]
pub fn rustc_version(&self) -> String {
// TODO: use create_command instead of manual construction!
let rustc_path = self.binary_file("rustc");
Expand Down

0 comments on commit 1b7662e

Please sign in to comment.