Skip to content

Commit

Permalink
tracing: simplify subscriber construction with Boxed layers (#1304)
Browse files Browse the repository at this point in the history
Currently, the way the `tracing` subscriber is constructed is somewhat
convoluted, due to potentially constructing formatting layers with
different types (either plaintext logging or JSON). Because of this, we
can't simply have methods that return the `fmt` layer. Instead, we have
to pass the `Registry` into the methods that construct the `fmt` layers
and return a type-erased `Dispatch` after adding the `fmt` layers. This
is kind of not great. In particular, it makes adding additional layers
difficult, such as access logging (#601) and (eventually)
`tokio-console` support.

This branch refactors the subscriber construction to use the
(recently-added) `impl Layer<S> for Box<dyn Layer<S>>` in
`tracing-subscriber`. Now, we can erase the types of the JSON and
plaintext `fmt` layers and return them from the methods that construct
them, and layer them onto the `Registry` in
`tracing::Settings::build()`. This makes the `tracing` setup
significantly easier, and will enable changes I want to make in #601.

Boxing these layers does add slight overhead of dynamic dispatch + a
pointer dereference. However, I doubt this has a huge performance impact
in practice...

(cherry picked from commit 3bb7ec4)
Signed-off-by: Oliver Gould <ver@buoyant.io>
  • Loading branch information
hawkw authored and olix0r committed Mar 30, 2022
1 parent fdba26c commit e1b6fd7
Showing 1 changed file with 27 additions and 22 deletions.
49 changes: 27 additions & 22 deletions linkerd/tracing/src/lib.rs
Expand Up @@ -8,11 +8,8 @@ mod uptime;
use self::uptime::Uptime;
use linkerd_error::Error;
use std::{env, str};
use tracing::Dispatch;
use tracing_subscriber::{fmt::format, layer::Layered, prelude::*, reload, EnvFilter};

type Registry =
Layered<reload::Layer<EnvFilter, tracing_subscriber::Registry>, tracing_subscriber::Registry>;
use tracing::{Dispatch, Subscriber};
use tracing_subscriber::{fmt::format, prelude::*, registry::LookupSpan, reload, EnvFilter, Layer};

const ENV_LOG_LEVEL: &str = "LINKERD2_PROXY_LOG";
const ENV_LOG_FORMAT: &str = "LINKERD2_PROXY_LOG_FORMAT";
Expand Down Expand Up @@ -94,14 +91,11 @@ impl Settings {
.to_uppercase()
}

fn mk_registry(&self) -> (Registry, level::Handle) {
let log_level = self.filter.as_deref().unwrap_or(DEFAULT_LOG_LEVEL);
let (filter, level) = reload::Layer::new(EnvFilter::new(log_level));
let reg = tracing_subscriber::registry().with(filter);
(reg, level::Handle::new(level))
}

fn mk_json(&self, registry: Registry) -> Dispatch {
fn mk_json<S>(&self) -> Box<dyn Layer<S> + Send + Sync + 'static>
where
S: Subscriber + for<'span> LookupSpan<'span>,
S: Send + Sync,
{
let fmt = tracing_subscriber::fmt::format()
.with_timer(Uptime::starting_now())
.with_thread_ids(!self.is_test)
Expand All @@ -121,32 +115,43 @@ impl Settings {
.fmt_fields(format::JsonFields::default());

if self.is_test {
registry.with(fmt.with_test_writer()).into()
Box::new(fmt.with_test_writer())
} else {
registry.with(fmt).into()
Box::new(fmt)
}
}

fn mk_plain(&self, registry: Registry) -> Dispatch {
fn mk_plain<S>(&self) -> Box<dyn Layer<S> + Send + Sync + 'static>
where
S: Subscriber + for<'span> LookupSpan<'span>,
S: Send + Sync,
{
let fmt = tracing_subscriber::fmt::format()
.with_timer(Uptime::starting_now())
.with_thread_ids(!self.is_test);
let fmt = tracing_subscriber::fmt::layer().event_format(fmt);
if self.is_test {
registry.with(fmt.with_test_writer()).into()
Box::new(fmt.with_test_writer())
} else {
registry.with(fmt).into()
Box::new(fmt)
}
}

pub fn build(self) -> (Dispatch, Handle) {
let (registry, level) = self.mk_registry();
let log_level = self.filter.as_deref().unwrap_or(DEFAULT_LOG_LEVEL);
let (filter, level) = reload::Layer::new(EnvFilter::new(log_level));
let level = level::Handle::new(level);

let dispatch = match self.format().as_ref() {
"JSON" => self.mk_json(registry),
_ => self.mk_plain(registry),
let logger = match self.format().as_ref() {
"JSON" => self.mk_json(),
_ => self.mk_plain(),
};

let dispatch = tracing_subscriber::registry()
.with(filter)
.with(logger)
.into();

(dispatch, Handle(Some(level)))
}
}
Expand Down

0 comments on commit e1b6fd7

Please sign in to comment.