From 99940aeeb478e1c96959aed69561336ab067d0d3 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 23 Oct 2019 11:04:14 -0700 Subject: [PATCH] chore: remove tracing. (#1680) Historically, logging has been added haphazardly. Here, we entirely remove logging as none of it is particularly useful. In the future, we will add tracing back in order to expose useful data to the user of Tokio. --- tokio-executor/Cargo.toml | 2 - tokio-executor/src/lib.rs | 4 -- tokio-executor/src/tracing.rs | 84 ------------------------- tokio-net/Cargo.toml | 6 -- tokio-net/src/driver/reactor.rs | 15 ----- tokio-net/src/driver/registration.rs | 1 - tokio-net/src/lib.rs | 2 - tokio-net/src/process/unix/orphan.rs | 11 ++-- tokio-net/src/tcp/stream.rs | 1 - tokio-net/src/tracing.rs | 78 ----------------------- tokio-net/tests/process_stdio.rs | 13 ---- tokio/Cargo.toml | 7 --- tokio/src/runtime/threadpool/builder.rs | 25 +++----- tokio/src/runtime/threadpool/mod.rs | 10 +-- 14 files changed, 13 insertions(+), 246 deletions(-) delete mode 100644 tokio-executor/src/tracing.rs delete mode 100644 tokio-net/src/tracing.rs diff --git a/tokio-executor/Cargo.toml b/tokio-executor/Cargo.toml index 7a1d15d5e..4fb77c5f3 100644 --- a/tokio-executor/Cargo.toml +++ b/tokio-executor/Cargo.toml @@ -29,8 +29,6 @@ thread-pool = ["num_cpus"] futures-util-preview = { version = "=0.3.0-alpha.19", features = ["channel"] } tokio-sync = { version = "=0.2.0-alpha.6", optional = true, path = "../tokio-sync" } -tracing = { version = "0.1.5", optional = true } - # current-thread dependencies crossbeam-channel = { version = "0.3.8", optional = true } diff --git a/tokio-executor/src/lib.rs b/tokio-executor/src/lib.rs index b5e0bf471..0b0cd3822 100644 --- a/tokio-executor/src/lib.rs +++ b/tokio-executor/src/lib.rs @@ -71,10 +71,6 @@ macro_rules! thread_local { #[macro_use] mod tests; -#[cfg(any(feature = "current-thread", feature = "threadpool"))] -#[macro_use] -mod tracing; - mod enter; mod error; mod executor; diff --git a/tokio-executor/src/tracing.rs b/tokio-executor/src/tracing.rs deleted file mode 100644 index 0cf5832d6..000000000 --- a/tokio-executor/src/tracing.rs +++ /dev/null @@ -1,84 +0,0 @@ -#![allow(unused_macros)] - -//! This module provides a small facade that wraps the `tracing` APIs we use, so -//! that when the `tracing` dependency is disabled, `tracing`'s macros expand to -//! no-ops. -//! -//! This means we don't have to put a `#[cfg(feature = "tracing")]` on every -//! individual use of a `tracing` macro. -#[cfg(not(feature = "tracing"))] -#[derive(Clone, Debug)] -pub(crate) struct Span {} - -#[cfg(feature = "tracing")] -macro_rules! trace { - ($($arg:tt)+) => { - tracing::trace!($($arg)+) - }; -} - -#[cfg(not(feature = "tracing"))] -macro_rules! trace { - ($($arg:tt)+) => {}; -} - -#[cfg(feature = "tracing")] -macro_rules! debug { - ($($arg:tt)+) => { - tracing::debug!($($arg)+) - }; -} - -#[cfg(not(feature = "tracing"))] -macro_rules! debug { - ($($arg:tt)+) => {}; -} - -#[cfg(feature = "tracing")] -macro_rules! warn { - ($($arg:tt)+) => { - tracing::warn!($($arg)+) - }; -} - -#[cfg(not(feature = "tracing"))] -macro_rules! warn { - ($($arg:tt)+) => {}; -} - -#[cfg(feature = "tracing")] -macro_rules! error { - ($($arg:tt)+) => { - tracing::error!($($arg)+) - }; -} - -#[cfg(not(feature = "tracing"))] -macro_rules! error { - ($($arg:tt)+) => {}; -} - -#[cfg(feature = "tracing")] -macro_rules! trace_span { - ($($arg:tt)+) => { - tracing::trace_span!($($arg)+) - }; -} - -#[cfg(not(feature = "tracing"))] -macro_rules! trace_span { - ($($arg:tt)+) => { - crate::tracing::Span::new() - }; -} - -#[cfg(not(feature = "tracing"))] -impl Span { - pub(crate) fn new() -> Self { - Span {} - } - - pub(crate) fn enter(&self) -> Span { - Span {} - } -} diff --git a/tokio-net/Cargo.toml b/tokio-net/Cargo.toml index f19fd7598..c5946ae17 100644 --- a/tokio-net/Cargo.toml +++ b/tokio-net/Cargo.toml @@ -59,15 +59,12 @@ uds = [ "iovec", "libc", ] -log = ["tracing/log"] [dependencies] tokio-executor = { version = "=0.2.0-alpha.6", features = ["blocking"], path = "../tokio-executor" } tokio-io = { version = "=0.2.0-alpha.6", path = "../tokio-io" } tokio-sync = { version = "=0.2.0-alpha.6", path = "../tokio-sync" } -tracing = { version = "0.1.5", optional = true } - # driver implementation crossbeam-utils = "0.6.0" futures-core-preview = "=0.3.0-alpha.19" @@ -98,9 +95,6 @@ version = "0.3" default-features = false optional = true -[target.'cfg(test)'.dependencies] -tracing = { version = "0.1.5", features = ["log"] } - [dev-dependencies] tokio = { version = "=0.2.0-alpha.6", path = "../tokio" } tokio-test = { version = "=0.2.0-alpha.6", path = "../tokio-test" } diff --git a/tokio-net/src/driver/reactor.rs b/tokio-net/src/driver/reactor.rs index c83ce8d5e..60ed552f0 100644 --- a/tokio-net/src/driver/reactor.rs +++ b/tokio-net/src/driver/reactor.rs @@ -210,7 +210,6 @@ impl Reactor { self.inner.io_dispatch.read().is_empty() } - #[cfg_attr(feature = "tracing", tracing::instrument(level = "debug"))] fn poll(&mut self, max_wait: Option) -> io::Result<()> { // Block waiting for an event to happen, peeling out how many events // happened. @@ -221,17 +220,8 @@ impl Reactor { // Process all the events that came in, dispatching appropriately - // event count is only used for tracing instrumentation. - #[cfg(feature = "tracing")] - let mut events = 0; - for event in self.events.iter() { - #[cfg(feature = "tracing")] - { - events += 1; - } let token = event.token(); - trace!(event.readiness = ?event.readiness(), event.token = ?token); if token == TOKEN_WAKEUP { self.inner @@ -243,8 +233,6 @@ impl Reactor { } } - trace!(message = "loop process", events); - Ok(()) } @@ -400,7 +388,6 @@ impl Inner { }; let token = aba_guard | key; - debug!(message = "adding I/O source", token); self.io.register( source, @@ -418,13 +405,11 @@ impl Inner { } pub(super) fn drop_source(&self, token: usize) { - debug!(message = "dropping I/O source", token); self.io_dispatch.write().remove(token); } /// Registers interest in the I/O resource associated with `token`. pub(super) fn register(&self, token: usize, dir: Direction, w: Waker) { - debug!(message = "scheduling", direction = ?dir, token); let io_dispatch = self.io_dispatch.read(); let sched = io_dispatch.get(token).unwrap(); diff --git a/tokio-net/src/driver/registration.rs b/tokio-net/src/driver/registration.rs index ed53b84eb..a5f24a35e 100644 --- a/tokio-net/src/driver/registration.rs +++ b/tokio-net/src/driver/registration.rs @@ -235,7 +235,6 @@ impl Registration { if ready.is_empty() { if let Some(cx) = cx { - debug!(message = "scheduling", ?direction, token = self.token); // Update the task info match direction { Direction::Read => sched.reader.register_by_ref(cx.waker()), diff --git a/tokio-net/src/lib.rs b/tokio-net/src/lib.rs index b9e27cec5..978048b07 100644 --- a/tokio-net/src/lib.rs +++ b/tokio-net/src/lib.rs @@ -39,8 +39,6 @@ //! [`Registration`]: struct.Registration.html //! [`PollEvented`]: struct.PollEvented.html //! [reactor module]: https://docs.rs/tokio/0.1/tokio/reactor/index.html -#[macro_use] -mod tracing; mod addr; pub use addr::ToSocketAddrs; diff --git a/tokio-net/src/process/unix/orphan.rs b/tokio-net/src/process/unix/orphan.rs index 69adbbdb3..c02c903fa 100644 --- a/tokio-net/src/process/unix/orphan.rs +++ b/tokio-net/src/process/unix/orphan.rs @@ -69,12 +69,11 @@ impl OrphanQueue for AtomicOrphanQueue { while let Ok(mut orphan) = self.queue.pop() { match orphan.try_wait() { Ok(Some(_)) => {} - Err(e) => error!( - message = "leaking orphaned process due to try_wait() error", - orphan.id =orphan.id(), - error = %e, - ), - + Err(_) => { + // TODO: bubble up error some how. Is this an internal bug? + // Shoudl we panic? Is it OK for this to be silently + // dropped? + } // Still not done yet, we need to put it back in the queue // when were done draining it, so that we don't get stuck // in an infinite loop here diff --git a/tokio-net/src/tcp/stream.rs b/tokio-net/src/tcp/stream.rs index 2634e6905..35e6f8063 100644 --- a/tokio-net/src/tcp/stream.rs +++ b/tokio-net/src/tcp/stream.rs @@ -659,7 +659,6 @@ impl TcpStream { unsafe { buf.advance_mut(n); } - trace!(tcp.written.bytes = n); Poll::Ready(Ok(n)) } Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => { diff --git a/tokio-net/src/tracing.rs b/tokio-net/src/tracing.rs deleted file mode 100644 index 0a01d23f6..000000000 --- a/tokio-net/src/tracing.rs +++ /dev/null @@ -1,78 +0,0 @@ -//! This module provides a small facade that wraps the `tracing` APIs we use, so -//! that when the `tracing` dependency is disabled, `tracing`'s macros expand to -//! no-ops. -//! -//! This means we don't have to put a `#[cfg(feature = "tracing")]` on every -//! individual use of a `tracing` macro. - -// The macros in this module may or may not be used depending on the combination -// of feature flags enabled. Rather than feature-flagging each individual macro -// to only be defined when the features that use it are enabled, just allow -// unused macros in some cases. -#![allow(unused_macros)] -#![allow(dead_code)] - -#[cfg(not(feature = "tracing"))] -#[derive(Clone, Debug)] -pub(crate) struct Span {} - -#[cfg(feature = "tracing")] -macro_rules! trace { - ($($arg:tt)+) => { - tracing::trace!($($arg)+) - }; -} - -#[cfg(not(feature = "tracing"))] -macro_rules! trace { - ($($arg:tt)+) => {{}}; -} - -#[cfg(feature = "tracing")] -macro_rules! debug { - ($($arg:tt)+) => { - tracing::debug!($($arg)+) - }; -} - -#[cfg(not(feature = "tracing"))] -macro_rules! debug { - ($($arg:tt)+) => {{}}; -} - -#[cfg(feature = "tracing")] -macro_rules! error { - ($($arg:tt)+) => { - tracing::error!($($arg)+) - }; -} - -#[cfg(not(feature = "tracing"))] -macro_rules! error { - ($($arg:tt)+) => {{}}; -} - -#[cfg(feature = "tracing")] -macro_rules! trace_span { - ($($arg:tt)+) => { - tracing::trace_span!($($arg)+) - }; -} - -#[cfg(not(feature = "tracing"))] -macro_rules! trace_span { - ($($arg:tt)+) => { - crate::tracing::Span::new() - }; -} - -#[cfg(not(feature = "tracing"))] -impl Span { - pub(crate) fn new() -> Self { - Span {} - } - - pub(crate) fn enter(&self) -> Span { - Span {} - } -} diff --git a/tokio-net/tests/process_stdio.rs b/tokio-net/tests/process_stdio.rs index e98162d4d..2e139b773 100644 --- a/tokio-net/tests/process_stdio.rs +++ b/tokio-net/tests/process_stdio.rs @@ -1,9 +1,6 @@ #![cfg(feature = "process")] #![warn(rust_2018_idioms)] -#[macro_use] -extern crate tracing; - use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio_net::process::{Child, Command}; @@ -38,10 +35,7 @@ async fn feed_cat(mut cat: Child, n: usize) -> io::Result { // Produce n lines on the child's stdout. let write = async { - debug!("starting to feed"); - for i in 0..n { - debug!("sending line {} to child", i); let bytes = format!("line {}\n", i).into_bytes(); stdin.write_all(&bytes).await.unwrap(); } @@ -56,8 +50,6 @@ async fn feed_cat(mut cat: Child, n: usize) -> io::Result { // Try to read `n + 1` lines, ensuring the last one is empty // (i.e. EOF is reached after `n` lines. loop { - debug!("starting read from child"); - let data = reader .next() .await @@ -67,11 +59,6 @@ async fn feed_cat(mut cat: Child, n: usize) -> io::Result { let num_read = data.len(); let done = num_lines >= n; - debug!( - "read line {} from child ({} bytes, done: {})", - num_lines, num_read, done - ); - match (done, num_read) { (false, 0) => panic!("broken pipe"), (true, n) if n != 0 => panic!("extraneous data"), diff --git a/tokio/Cargo.toml b/tokio/Cargo.toml index ac78708e2..ba6e0af02 100644 --- a/tokio/Cargo.toml +++ b/tokio/Cargo.toml @@ -52,13 +52,11 @@ rt-full = [ "timer", "tokio-executor/current-thread", "tokio-executor/thread-pool", - "tracing-core", ] signal = ["tokio-net/signal"] sync = ["tokio-sync"] tcp = ["io", "tokio-net/tcp"] timer = ["crossbeam-utils", "slab"] -tracing = ["tracing-core"] udp = ["io", "tokio-net/udp"] uds = ["io", "tokio-net/uds"] process = ["io", "tokio-net/process"] @@ -78,11 +76,6 @@ tokio-executor = { version = "=0.2.0-alpha.6", optional = true, path = "../tokio tokio-macros = { version = "=0.2.0-alpha.6", optional = true, path = "../tokio-macros" } tokio-net = { version = "=0.2.0-alpha.6", optional = true, features = ["async-traits"], path = "../tokio-net" } tokio-sync = { version = "=0.2.0-alpha.6", optional = true, path = "../tokio-sync", features = ["async-traits"] } -tracing-core = { version = "0.1", optional = true } - -[target.'cfg(feature = "tracing")'.dependencies] -tokio-net = { version = "=0.2.0-alpha.6", optional = true, path = "../tokio-net", features = ["tracing", "async-traits"] } -tokio-executor = { version = "=0.2.0-alpha.6", optional = true, path = "../tokio-executor", features = ["tracing"] } [dev-dependencies] tokio-test = { version = "=0.2.0-alpha.6", path = "../tokio-test" } diff --git a/tokio/src/runtime/threadpool/builder.rs b/tokio/src/runtime/threadpool/builder.rs index 7f6f27f46..df0ff337c 100644 --- a/tokio/src/runtime/threadpool/builder.rs +++ b/tokio/src/runtime/threadpool/builder.rs @@ -5,7 +5,6 @@ use crate::timer::timer::{self, Timer}; use tokio_executor::thread_pool; use tokio_net::driver::{self, Reactor}; -use tracing_core as trace; use std::{fmt, io}; use std::sync::{Arc, Mutex}; @@ -241,13 +240,6 @@ impl Builder { // Get a handle to the clock for the runtime. let clock = self.clock.clone(); - // Get the current trace dispatcher. - // TODO(eliza): when `tokio-trace-core` is stable enough to take a - // public API dependency, we should allow users to set a custom - // subscriber for the runtime. - let dispatch = trace::dispatcher::get_default(trace::Dispatch::clone); - let trace = dispatch.clone(); - let around_reactor_handles = reactor_handles.clone(); let around_timer_handles = timer_handles.clone(); @@ -260,17 +252,15 @@ impl Builder { let _reactor = driver::set_default(&around_reactor_handles[index]); clock::with_default(&clock, || { let _timer = timer::set_default(&around_timer_handles[index]); - trace::dispatcher::with_default(&dispatch, || { - if let Some(after_start) = after_start.as_ref() { - after_start(); - } + if let Some(after_start) = after_start.as_ref() { + after_start(); + } - next(); + next(); - if let Some(before_stop) = before_stop.as_ref() { - before_stop(); - } - }) + if let Some(before_stop) = before_stop.as_ref() { + before_stop(); + } }) }) .build_with_park(move |index| { @@ -286,7 +276,6 @@ impl Builder { pool, reactor_handles, timer_handles, - trace, }), }) } diff --git a/tokio/src/runtime/threadpool/mod.rs b/tokio/src/runtime/threadpool/mod.rs index e23cda4d6..441b2059a 100644 --- a/tokio/src/runtime/threadpool/mod.rs +++ b/tokio/src/runtime/threadpool/mod.rs @@ -14,7 +14,6 @@ use crate::timer::timer; use tokio_executor::thread_pool::ThreadPool; use tokio_net::driver; -use tracing_core as trace; use std::future::Future; use std::io; @@ -47,9 +46,6 @@ struct Inner { /// Timer handles timer_handles: Vec, - - /// Tracing dispatcher - trace: trace::Dispatch, } // ===== impl Runtime ===== @@ -137,14 +133,10 @@ impl Runtime { where F: Future, { - let trace = &self.inner().trace; - let _reactor = driver::set_default(&self.inner().reactor_handles[0]); let _timer = timer::set_default(&self.inner().timer_handles[0]); - trace::dispatcher::with_default(trace, || { - self.inner().pool.block_on(future) - }) + self.inner().pool.block_on(future) } /// Return a handle to the runtime's spawner.