Skip to content

Commit

Permalink
subscriber: use ANSI formatting in field formatters (#1702)
Browse files Browse the repository at this point in the history
Depends on #1696

## Motivation

PR #1696 adds a new API for propagating whether or not ANSI formatting
escape codes are enabled to the event formatter and field formatter via
the `Writer` type. This means that it's now quite easy to make field
formatting also include ANSI formatting escape codes when ANSI
formatting is enabled. Currently, `tracing-subscriber`'s default field
formatter does not use ANSI formatting --- ANSI escape codes are
currently only used for parts of the event log line controlled by the
event formatter, not within fields. Adding ANSI colors and formatting in
the fields could make the formatted output nicer looking and easier to
read.

## Solution

This branch adds support for ANSI formatting escapes in the default
field formatter, when ANSI formatting is enabled. Now, field names will
be italicized, and the `=` delimiter is dimmed. This should make it a
little easier to visually scan the field lists, since the names and
values are more clearly separated and should be easier to distinguish.

Additionally, I cleaned up the code for conditionally using ANSI
formatting significantly. Now, the `Writer` type has (crate-private)
methods for returning `Style`s for bold, dimmed, and italicized text,
when ANSI escapes are enabled, or no-op `Style`s when they are not. This
is reused in all the formatter implementations, so it makes sense to
have it in one place. I also added dimmed formatting of delimiters in a
couple other places in the default event format, which I thought
improved readability a bit by bringing the actual *text* to the
forefront.

Example of the default format with ANSI escapes enabled:
![image](https://user-images.githubusercontent.com/2796466/140166750-aaf64bd8-b051-4985-9e7d-168fe8757b11.png)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
hawkw committed Nov 20, 2021
1 parent 99e46e8 commit aa38daa
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 41 deletions.
119 changes: 83 additions & 36 deletions tracing-subscriber/src/fmt/format/mod.rs
Expand Up @@ -376,6 +376,39 @@ impl<'writer> Writer<'writer> {
pub fn has_ansi_escapes(&self) -> bool {
self.is_ansi
}

pub(in crate::fmt::format) fn bold(&self) -> Style {
#[cfg(feature = "ansi")]
{
if self.is_ansi {
return Style::new().bold();
}
}

Style::new()
}

pub(in crate::fmt::format) fn dimmed(&self) -> Style {
#[cfg(feature = "ansi")]
{
if self.is_ansi {
return Style::new().dimmed();
}
}

Style::new()
}

pub(in crate::fmt::format) fn italic(&self) -> Style {
#[cfg(feature = "ansi")]
{
if self.is_ansi {
return Style::new().italic();
}
}

Style::new()
}
}

impl fmt::Write for Writer<'_> {
Expand Down Expand Up @@ -724,12 +757,11 @@ where
write!(writer, "{:0>2?} ", std::thread::current().id())?;
}

let dimmed = writer.dimmed();

if let Some(scope) = ctx.ctx.event_scope(event) {
let bold = if writer.has_ansi_escapes() {
Style::new().bold()
} else {
Style::new()
};
let bold = writer.bold();

let mut seen = false;

for span in scope.from_root() {
Expand All @@ -742,7 +774,7 @@ where
write!(writer, "{}{}{}", bold.paint("{"), fields, bold.paint("}"))?;
}
}
writer.write_char(':')?;
write!(writer, "{}", dimmed.paint(":"))?;
}

if seen {
Expand All @@ -751,7 +783,12 @@ where
};

if self.display_target {
write!(writer, "{}: ", meta.target())?;
write!(
writer,
"{}{} ",
dimmed.paint(meta.target()),
dimmed.paint(":")
)?;
}

ctx.format_fields(writer.by_ref(), event)?;
Expand Down Expand Up @@ -821,15 +858,12 @@ where
}

if self.display_target {
let target = meta.target();
#[cfg(feature = "ansi")]
let target = if writer.has_ansi_escapes() {
Style::new().bold().paint(target)
} else {
Style::new().paint(target)
};

write!(writer, "{}:", target)?;
write!(
writer,
"{}{}",
writer.bold().paint(meta.target()),
writer.dimmed().paint(":")
)?;
}

let fmt_ctx = {
Expand All @@ -844,25 +878,18 @@ where
};
write!(writer, "{}", fmt_ctx)?;

let span = event
.parent()
.and_then(|id| ctx.ctx.span(id))
.or_else(|| ctx.ctx.lookup_current());

let scope = span.into_iter().flat_map(|span| span.scope());
#[cfg(feature = "ansi")]
let dimmed = if writer.has_ansi_escapes() {
Style::new().dimmed()
} else {
Style::new()
};
for span in scope {
let dimmed = writer.dimmed();
for span in ctx
.ctx
.event_scope(event)
.into_iter()
.map(crate::registry::Scope::from_root)
.flatten()
{
let exts = span.extensions();
if let Some(fields) = exts.get::<FormattedFields<N>>() {
if !fields.is_empty() {
#[cfg(feature = "ansi")]
let fields = dimmed.paint(fields.as_str());
write!(writer, " {}", fields)?;
write!(writer, " {}", dimmed.paint(&fields.fields))?;
}
}
}
Expand Down Expand Up @@ -969,9 +996,17 @@ impl<'a> field::Visit for DefaultVisitor<'a> {

fn record_error(&mut self, field: &Field, value: &(dyn std::error::Error + 'static)) {
if let Some(source) = value.source() {
let italic = self.writer.italic();
self.record_debug(
field,
&format_args!("{} {}.sources={}", value, field, ErrorSourceList(source)),
&format_args!(
"{} {}{}{}{}",
value,
italic.paint(field.name()),
italic.paint(".sources"),
self.writer.dimmed().paint("="),
ErrorSourceList(source)
),
)
} else {
self.record_debug(field, &format_args!("{}", value))
Expand All @@ -989,8 +1024,20 @@ impl<'a> field::Visit for DefaultVisitor<'a> {
// Skip fields that are actually log metadata that have already been handled
#[cfg(feature = "tracing-log")]
name if name.starts_with("log.") => Ok(()),
name if name.starts_with("r#") => write!(self.writer, "{}={:?}", &name[2..], value),
name => write!(self.writer, "{}={:?}", name, value),
name if name.starts_with("r#") => write!(
self.writer,
"{}{}{:?}",
self.writer.italic().paint(&name[2..]),
self.writer.dimmed().paint("="),
value
),
name => write!(
self.writer,
"{}{}{:?}",
self.writer.italic().paint(name),
self.writer.dimmed().paint("="),
value
),
};
}
}
Expand Down Expand Up @@ -1459,7 +1506,7 @@ pub(super) mod test {
#[cfg(feature = "ansi")]
#[test]
fn with_ansi_true() {
let expected = "\u{1b}[2mfake time\u{1b}[0m \u{1b}[32m INFO\u{1b}[0m tracing_subscriber::fmt::format::test: hello\n";
let expected = "\u{1b}[2mfake time\u{1b}[0m \u{1b}[32m INFO\u{1b}[0m \u{1b}[2mtracing_subscriber::fmt::format::test\u{1b}[0m\u{1b}[2m:\u{1b}[0m hello\n";
test_ansi(true, expected);
}

Expand Down
6 changes: 1 addition & 5 deletions tracing-subscriber/src/fmt/format/pretty.rs
Expand Up @@ -187,11 +187,7 @@ where
writer.write_char('\n')?;
}

let bold = if writer.has_ansi_escapes() {
Style::new().bold()
} else {
Style::new()
};
let bold = writer.bold();
let span = event
.parent()
.and_then(|id| ctx.span(id))
Expand Down

0 comments on commit aa38daa

Please sign in to comment.