From 4f35095c382b3a700dc4a189f4c080698fdf595e Mon Sep 17 00:00:00 2001
From: Jun Wu <quark@fb.com>
Date: Tue, 23 Feb 2021 15:06:26 -0800
Subject: [PATCH] hgcommands: set tracing level to TRACE if EDENSCM_LOG is set

Summary:
Previously, one has to set both EDENSCM_LOG=trace and EDENSCM_TRACE_LEVEL=trace
to enable all logging. That is because the filtering is global - one subscriber
(or layer) filtering out a span or event, then the span or event is gone
forever.

For now, let's just set the TracingCollector Subscriber's filter level
(EDENSCM_TRACE_LEVEL) to TRACE so it solely depends on the EnvFilter
(EDENSCM_LOG) if EDENSCM_LOG is set. That's more friendly.

See `impl<L: Layer<S>, S: Subscriber> Subscriber for Layered<L, S>`:

    fn enabled(&self, metadata: &Metadata<'_>) -> bool {
        if self.layer.enabled(metadata, self.ctx()) {
            // if the outer layer enables the callsite metadata, ask the subscriber.
            self.inner.enabled(metadata)
        } else {
            // otherwise, the callsite is disabled by the layer
            false
        }
    }

See also: https://github.com/tokio-rs/tracing/issues/302

Reviewed By: sfilipco

Differential Revision: D26518017

fbshipit-source-id: 9016b940621fc9a883e6ca3f00eaaeccc051ae74
---
 eden/scm/lib/hgcommands/src/run.rs | 40 ++++++++++++++----------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/eden/scm/lib/hgcommands/src/run.rs b/eden/scm/lib/hgcommands/src/run.rs
index e5760b145de71..3e9e49ce2ccc3 100644
--- a/eden/scm/lib/hgcommands/src/run.rs
+++ b/eden/scm/lib/hgcommands/src/run.rs
@@ -50,12 +50,12 @@ pub fn run_command(args: Vec<String>, io: &IO) -> i32 {
 
     // Setup tracing early since "log_start" will use it immediately.
     // The tracing clock starts ticking from here.
-    let (_tracing_level, tracing_data) = match setup_tracing(&global_opts, io) {
+    let tracing_data = match setup_tracing(&global_opts, io) {
         Err(_) => {
             // With our current architecture it is common to see this path in our tests due to
             // trying to set a global collector a second time. Ignore the error and return some
             // dummy values. FIXME!
-            (Level::INFO, Arc::new(Mutex::new(TracingData::new())))
+            Arc::new(Mutex::new(TracingData::new()))
         }
         Ok(res) => res,
     };
@@ -168,10 +168,7 @@ fn current_dir(io: &IO) -> io::Result<PathBuf> {
     result
 }
 
-fn setup_tracing(
-    global_opts: &Option<HgGlobalOpts>,
-    io: &IO,
-) -> Result<(Level, Arc<Mutex<TracingData>>)> {
+fn setup_tracing(global_opts: &Option<HgGlobalOpts>, io: &IO) -> Result<Arc<Mutex<TracingData>>> {
     // Setup TracingData singleton (currently owned by pytracing).
     {
         let mut data = pytracing::DATA.lock();
@@ -185,26 +182,14 @@ fn setup_tracing(
     }
     let data = pytracing::DATA.clone();
 
-    let level = std::env::var("EDENSCM_TRACE_LEVEL")
-        .ok()
-        .and_then(|s| Level::from_str(&s).ok())
-        .unwrap_or_else(|| {
-            if let Some(opts) = global_opts {
-                if opts.trace {
-                    return Level::DEBUG;
-                }
-            }
-            Level::INFO
-        });
-
-
-    let collector = tracing_collector::default_collector(data.clone(), level.clone());
     let is_test = is_inside_test();
     let log_env_name = ["EDENSCM_LOG", "LOG"]
         .iter()
         .take(if is_test { 2 } else { 1 }) /* Only consider $LOG in tests */
         .find(|s| std::env::var_os(s).is_some());
     if let Some(env_name) = log_env_name {
+        // The env_filter does the actual filtering. No need to filter by level.
+        let collector = tracing_collector::default_collector(data.clone(), Level::TRACE);
         let env_filter = EnvFilter::from_env(env_name);
         let error = io.error();
         let env_logger = FmtLayer::new()
@@ -220,10 +205,23 @@ fn setup_tracing(
             tracing::subscriber::set_global_default(collector)?;
         }
     } else {
+        let level = std::env::var("EDENSCM_TRACE_LEVEL")
+            .ok()
+            .and_then(|s| Level::from_str(&s).ok())
+            .unwrap_or_else(|| {
+                if let Some(opts) = global_opts {
+                    if opts.trace {
+                        return Level::DEBUG;
+                    }
+                }
+                Level::INFO
+            });
+
+        let collector = tracing_collector::default_collector(data.clone(), level);
         tracing::subscriber::set_global_default(collector)?;
     }
 
-    Ok((level, data))
+    Ok(data)
 }
 
 fn maybe_write_trace(