From 28c5c5f6e0bc569e17be9e87c1659f30f98d3aee Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 3 Sep 2019 12:19:53 -0700 Subject: [PATCH] log: fix cyclic dependency (#315) Currently, the `tracing-log` crate uses the `CurrentSpan` utility in `tracing-subscriber`. However, now that `tracing-fmt` has been moved into `tracing-subscriber`, its `log` feature (which depends on `tracing-log`) creates a cyclic dependency between these two crates. This branch solves the cyclic dependency by removing the `tracing-subscriber` dependency from `tracing-log`. Instead, the current span is tracked manually. Signed-off-by: Eliza Weisman --- tracing-env-logger/Cargo.toml | 2 +- tracing-log/Cargo.toml | 3 +-- tracing-log/src/trace_logger.rs | 42 ++++++++++++++++++++++++++------- tracing-subscriber/Cargo.toml | 2 +- 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/tracing-env-logger/Cargo.toml b/tracing-env-logger/Cargo.toml index b8f313ed..5c7997fb 100644 --- a/tracing-env-logger/Cargo.toml +++ b/tracing-env-logger/Cargo.toml @@ -17,7 +17,7 @@ license = "MIT" edition = "2018" [dependencies] -tracing-log = "0.0.1-alpha.1" +tracing-log = { version = "0.0.1-alpha.2", path = "../tracing-log" } env_logger = "0.5" log = "0.4" diff --git a/tracing-log/Cargo.toml b/tracing-log/Cargo.toml index 62555d20..8d21bd84 100644 --- a/tracing-log/Cargo.toml +++ b/tracing-log/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "tracing-log" -version = "0.0.1-alpha.1" +version = "0.0.1-alpha.2" authors = ["Tokio Contributors "] edition = "2018" repository = "https://github.com/tokio-rs/tracing" @@ -19,7 +19,6 @@ readme = "README.md" [dependencies] tracing-core = "0.1.2" -tracing-subscriber = "0.0.1-alpha.2" log = { version = "0.4", features = ["std"] } lazy_static = "1.3.0" diff --git a/tracing-log/src/trace_logger.rs b/tracing-log/src/trace_logger.rs index 0532aebc..9af3a8bf 100644 --- a/tracing-log/src/trace_logger.rs +++ b/tracing-log/src/trace_logger.rs @@ -1,5 +1,6 @@ use crate::AsLog; use std::{ + cell::RefCell, collections::HashMap, fmt::{self, Write}, sync::{ @@ -18,10 +19,13 @@ use tracing_core::{ pub struct TraceLogger { settings: Builder, spans: Mutex>, - current: tracing_subscriber::CurrentSpan, next_id: AtomicUsize, } +thread_local! { + static CURRENT: RefCell> = RefCell::new(Vec::new()); +} + #[derive(Debug)] pub struct Builder { log_span_closes: bool, @@ -111,7 +115,6 @@ impl Default for TraceLogger { TraceLogger { settings: Default::default(), spans: Default::default(), - current: Default::default(), next_id: AtomicUsize::new(1), } } @@ -186,7 +189,7 @@ impl Subscriber for TraceLogger { let id = self.next_id(); let mut spans = self.spans.lock().unwrap(); let mut fields = String::new(); - let parent = self.current.id(); + let parent = self.current_id(); if self.settings.parent_fields { let mut next_parent = parent.as_ref(); while let Some(ref parent) = next_parent.and_then(|p| spans.get(&p)) { @@ -218,14 +221,21 @@ impl Subscriber for TraceLogger { } fn enter(&self, id: &Id) { - self.current.enter(id.clone()); + let _ = CURRENT.try_with(|current| { + let mut current = current.borrow_mut(); + if current.contains(id) { + // Ignore duplicate enters. + return; + } + current.push(id.clone()); + }); let spans = self.spans.lock().unwrap(); if self.settings.log_enters { if let Some(span) = spans.get(id) { let log_meta = span.log_meta(); let logger = log::logger(); if logger.enabled(&log_meta) { - let current_id = self.current.id(); + let current_id = self.current_id(); let current_fields = current_id .as_ref() .and_then(|id| spans.get(&id)) @@ -263,7 +273,14 @@ impl Subscriber for TraceLogger { } fn exit(&self, id: &Id) { - self.current.exit(); + let _ = CURRENT.try_with(|current| { + let mut current = current.borrow_mut(); + if current.last() == Some(id) { + current.pop() + } else { + None + } + }); if self.settings.log_exits { let spans = self.spans.lock().unwrap(); if let Some(span) = spans.get(id) { @@ -291,7 +308,7 @@ impl Subscriber for TraceLogger { let logger = log::logger(); if logger.enabled(&log_meta) { let spans = self.spans.lock().unwrap(); - let current = self.current.id().and_then(|id| spans.get(&id)); + let current = self.current_id().and_then(|id| spans.get(&id)); let (current_fields, parent) = current .map(|span| { let fields = span.fields.as_ref(); @@ -347,6 +364,15 @@ impl Subscriber for TraceLogger { } } +impl TraceLogger { + #[inline] + fn current_id(&self) -> Option { + CURRENT + .try_with(|current| current.borrow().last().map(|span| self.clone_span(span))) + .ok()? + } +} + struct LogEvent<'a>(&'a Event<'a>); impl<'a> fmt::Display for LogEvent<'a> { @@ -374,7 +400,7 @@ impl fmt::Debug for TraceLogger { f.debug_struct("TraceLogger") .field("settings", &self.settings) .field("spans", &self.spans) - .field("current", &self.current.id()) + .field("current", &self.current_id()) .field("next_id", &self.next_id) .finish() } diff --git a/tracing-subscriber/Cargo.toml b/tracing-subscriber/Cargo.toml index 77ecdd47..17572c50 100644 --- a/tracing-subscriber/Cargo.toml +++ b/tracing-subscriber/Cargo.toml @@ -36,7 +36,7 @@ smallvec = { optional = true, version = "0.6.10"} lazy_static = { optional = true, version = "1" } # fmt -tracing-log = { version = "0.0.1-alpha.1", optional = true } +tracing-log = { version = "0.0.1-alpha.2", path = "../tracing-log", optional = true } ansi_term = { version = "0.11", optional = true } owning_ref = { version = "0.4.0", optional = true } parking_lot = { version = ">= 0.7, < 0.10", features = ["owning_ref"], optional = true }