Skip to content

Commit

Permalink
subscriber: propagate ANSI config via Writer (#1696)
Browse files Browse the repository at this point in the history
## Motivation

Currently, whether `tracing-subscriber`'s `fmt` subscriber will ANSI
formatting escape codes use is configured on the `Format` type. This
means that the configuration is honored by the event formatters
implemented in `tracing-subscriber`, but is not easily exposed to those
in other crates. Additionally, it's not currently easy to expose the
configuration to the field formatter, so it's difficult to implement
field formatters that use ANSI escape codes conditionally.

Issue #1651 suggested a new API for this, where the writer that's passed
in to the event and field formatters provides a method for checking if
ANSI escape codes are supported.

## Solution

This branch adds a new method to the `Writer` type added in #1661. The
`FormatEvent` and `FormatFields` implementations can call
`Writer::has_ansi_escapes` to determine whether ANSI escape codes are
supported. This is also propagated to `FormattedFields`, so that it can
be determined when adding new fields to a preexisting set of formatted
fields.

Fixes #1661

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
hawkw committed Nov 8, 2021
1 parent 8d4d5ac commit 8a7d0c5
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 68 deletions.
49 changes: 35 additions & 14 deletions tracing-subscriber/src/fmt/fmt_subscriber.rs
Expand Up @@ -67,6 +67,7 @@ pub struct Subscriber<C, N = format::DefaultFields, E = format::Format, W = fn()
fmt_fields: N,
fmt_event: E,
fmt_span: format::FmtSpanConfig,
is_ansi: bool,
_inner: PhantomData<C>,
}

Expand Down Expand Up @@ -114,6 +115,7 @@ where
fmt_event: e,
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -148,6 +150,7 @@ impl<C, N, E, W> Subscriber<C, N, E, W> {
fmt_fields: self.fmt_fields,
fmt_event: self.fmt_event,
fmt_span: self.fmt_span,
is_ansi: self.is_ansi,
make_writer,
_inner: self._inner,
}
Expand Down Expand Up @@ -180,10 +183,21 @@ impl<C, N, E, W> Subscriber<C, N, E, W> {
fmt_fields: self.fmt_fields,
fmt_event: self.fmt_event,
fmt_span: self.fmt_span,
is_ansi: self.is_ansi,
make_writer: TestWriter::default(),
_inner: self._inner,
}
}

/// Enable ANSI terminal colors for formatted output.
#[cfg(feature = "ansi")]
#[cfg_attr(docsrs, doc(cfg(feature = "ansi")))]
pub fn with_ansi(self, ansi: bool) -> Self {
Subscriber {
is_ansi: ansi,
..self
}
}
}

impl<C, N, L, T, W> Subscriber<C, N, format::Format<L, T>, W>
Expand All @@ -210,6 +224,7 @@ where
fmt_fields: self.fmt_fields,
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
_inner: self._inner,
}
}
Expand All @@ -221,6 +236,7 @@ where
fmt_fields: self.fmt_fields,
fmt_span: self.fmt_span.without_time(),
make_writer: self.make_writer,
is_ansi: self.is_ansi,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -273,16 +289,6 @@ where
}
}

/// Enable ANSI terminal colors for formatted output.
#[cfg(feature = "ansi")]
#[cfg_attr(docsrs, doc(cfg(feature = "ansi")))]
pub fn with_ansi(self, ansi: bool) -> Subscriber<C, N, format::Format<L, T>, W> {
Subscriber {
fmt_event: self.fmt_event.with_ansi(ansi),
..self
}
}

/// Sets whether or not an event's target is displayed.
pub fn with_target(self, display_target: bool) -> Subscriber<C, N, format::Format<L, T>, W> {
Subscriber {
Expand Down Expand Up @@ -337,6 +343,7 @@ where
fmt_fields: self.fmt_fields,
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
_inner: self._inner,
}
}
Expand All @@ -350,6 +357,7 @@ where
fmt_fields: format::Pretty::default(),
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -377,6 +385,8 @@ where
fmt_fields: format::JsonFields::new(),
fmt_span: self.fmt_span,
make_writer: self.make_writer,
// always disable ANSI escapes in JSON mode!
is_ansi: false,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -442,6 +452,7 @@ impl<C, N, E, W> Subscriber<C, N, E, W> {
fmt_fields,
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
_inner: self._inner,
}
}
Expand All @@ -454,6 +465,7 @@ impl<C> Default for Subscriber<C> {
fmt_event: format::Format::default(),
fmt_span: format::FmtSpanConfig::default(),
make_writer: io::stdout,
is_ansi: cfg!(feature = "ansi"),
_inner: PhantomData,
}
}
Expand Down Expand Up @@ -487,6 +499,7 @@ where
#[derive(Default)]
pub struct FormattedFields<E: ?Sized> {
_format_fields: PhantomData<fn(E)>,
was_ansi: bool,
/// The formatted fields of a span.
pub fields: String,
}
Expand All @@ -496,6 +509,7 @@ impl<E: ?Sized> FormattedFields<E> {
pub fn new(fields: String) -> Self {
Self {
fields,
was_ansi: false,
_format_fields: PhantomData,
}
}
Expand All @@ -505,7 +519,7 @@ impl<E: ?Sized> FormattedFields<E> {
/// The returned [`format::Writer`] can be used with the
/// [`FormatFields::format_fields`] method.
pub fn as_writer(&mut self) -> format::Writer<'_> {
format::Writer::new(&mut self.fields)
format::Writer::new(&mut self.fields).with_ansi(self.was_ansi)
}
}

Expand All @@ -514,6 +528,7 @@ impl<E: ?Sized> fmt::Debug for FormattedFields<E> {
f.debug_struct("FormattedFields")
.field("fields", &self.fields)
.field("formatter", &format_args!("{}", std::any::type_name::<E>()))
.field("was_ansi", &self.was_ansi)
.finish()
}
}
Expand Down Expand Up @@ -564,9 +579,10 @@ where
let mut fields = FormattedFields::<N>::new(String::new());
if self
.fmt_fields
.format_fields(fields.as_writer(), attrs)
.format_fields(fields.as_writer().with_ansi(self.is_ansi), attrs)
.is_ok()
{
fields.was_ansi = self.is_ansi;
extensions.insert(fields);
}
}
Expand Down Expand Up @@ -598,9 +614,10 @@ where
let mut fields = FormattedFields::<N>::new(String::new());
if self
.fmt_fields
.format_fields(fields.as_writer(), values)
.format_fields(fields.as_writer().with_ansi(self.is_ansi), values)
.is_ok()
{
fields.was_ansi = self.is_ansi;
extensions.insert(fields);
}
}
Expand Down Expand Up @@ -705,7 +722,11 @@ where
let ctx = self.make_ctx(ctx);
if self
.fmt_event
.format_event(&ctx, format::Writer::new(&mut buf), event)
.format_event(
&ctx,
format::Writer::new(&mut buf).with_ansi(self.is_ansi),
event,
)
.is_ok()
{
let mut writer = self.make_writer.make_writer_for(event.metadata());
Expand Down
57 changes: 44 additions & 13 deletions tracing-subscriber/src/fmt/format/mod.rs
Expand Up @@ -234,6 +234,7 @@ where
pub struct Writer<'writer> {
writer: &'writer mut dyn fmt::Write,
// TODO(eliza): add ANSI support
is_ansi: bool,
}

/// A [`FormatFields`] implementation that formats fields by calling a function
Expand Down Expand Up @@ -273,7 +274,7 @@ pub struct Full;
pub struct Format<F = Full, T = SystemTime> {
format: F,
pub(crate) timer: T,
pub(crate) ansi: bool,
pub(crate) ansi: Option<bool>,
pub(crate) display_timestamp: bool,
pub(crate) display_target: bool,
pub(crate) display_level: bool,
Expand All @@ -291,16 +292,26 @@ impl<'writer> Writer<'writer> {
pub(crate) fn new(writer: &'writer mut impl fmt::Write) -> Self {
Self {
writer: writer as &mut dyn fmt::Write,
is_ansi: false,
}
}

// TODO(eliza): consider making this a public API?
pub(crate) fn with_ansi(self, is_ansi: bool) -> Self {
Self { is_ansi, ..self }
}

/// Return a new `Writer` that mutably borrows `self`.
///
/// This can be used to temporarily borrow a `Writer` to pass a new `Writer`
/// to a function that takes a `Writer` by value, allowing the original writer
/// to still be used once that function returns.
pub fn by_ref(&mut self) -> Writer<'_> {
Writer::new(self)
let is_ansi = self.is_ansi;
Writer {
writer: self as &mut dyn fmt::Write,
is_ansi,
}
}

/// Writes a string slice into this `Writer`, returning whether the write succeeded.
Expand Down Expand Up @@ -346,7 +357,7 @@ impl<'writer> Writer<'writer> {
self.writer.write_char(c)
}

/// Glue for usage of the [`write!`] macro with `Wrriter`s.
/// Glue for usage of the [`write!`] macro with `Writer`s.
///
/// This method should generally not be invoked manually, but rather through
/// the [`write!`] macro itself.
Expand All @@ -361,6 +372,14 @@ impl<'writer> Writer<'writer> {
pub fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> fmt::Result {
self.writer.write_fmt(args)
}

/// Returns `true` if ANSI escape codes may be used to add colors
/// and other formatting when writing to this `Writer`.
///
/// If this returns `false`, formatters should not emit ANSI escape codes.
pub fn has_ansi_escapes(&self) -> bool {
self.is_ansi
}
}

impl fmt::Write for Writer<'_> {
Expand All @@ -384,6 +403,7 @@ impl fmt::Debug for Writer<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Writer")
.field("writer", &format_args!("<&mut dyn fmt::Write>"))
.field("is_ansi", &self.is_ansi)
.finish()
}
}
Expand All @@ -395,7 +415,7 @@ impl Default for Format<Full, SystemTime> {
Format {
format: Full,
timer: SystemTime,
ansi: true,
ansi: None,
display_timestamp: true,
display_target: true,
display_level: true,
Expand Down Expand Up @@ -532,7 +552,10 @@ impl<F, T> Format<F, T> {

/// Enable ANSI terminal colors for formatted output.
pub fn with_ansi(self, ansi: bool) -> Format<F, T> {
Format { ansi, ..self }
Format {
ansi: Some(ansi),
..self
}
}

/// Sets whether or not an event's target is displayed.
Expand Down Expand Up @@ -573,15 +596,15 @@ impl<F, T> Format<F, T> {
}
}

fn format_level(&self, level: Level, writer: &mut dyn fmt::Write) -> fmt::Result
fn format_level(&self, level: Level, writer: &mut Writer<'_>) -> fmt::Result
where
F: LevelNames,
{
if self.display_level {
let fmt_level = {
#[cfg(feature = "ansi")]
{
F::format_level(level, self.ansi)
F::format_level(level, writer.has_ansi_escapes())
}
#[cfg(not(feature = "ansi"))]
{
Expand All @@ -608,7 +631,7 @@ impl<F, T> Format<F, T> {
// colors.
#[cfg(feature = "ansi")]
{
if self.ansi {
if writer.has_ansi_escapes() {
let style = Style::new().dimmed();
write!(writer, "{}", style.prefix())?;

Expand All @@ -632,10 +655,10 @@ impl<F, T> Format<F, T> {
writer.write_char(' ')
}

fn bold(&self) -> Style {
fn bold(&self, is_ansi: bool) -> Style {
#[cfg(feature = "ansi")]
{
if self.ansi {
if self.ansi.unwrap_or(true) && is_ansi {
return Style::new().bold();
}
}
Expand Down Expand Up @@ -704,6 +727,14 @@ where
#[cfg(not(feature = "tracing-log"))]
let meta = event.metadata();

// if the `Format` struct *also* has an ANSI color configuration,
// override the writer...the API for configuring ANSI color codes on the
// `Format` struct is deprecated, but we still need to honor those
// configurations.
if let Some(ansi) = self.ansi {
writer = writer.with_ansi(ansi);
}

self.format_timestamp(&mut writer)?;
self.format_level(*meta.level(), &mut writer)?;

Expand All @@ -726,7 +757,7 @@ where
}

if let Some(scope) = ctx.ctx.event_scope(event) {
let bold = self.bold();
let bold = self.bold(writer.has_ansi_escapes());
let mut seen = false;

for span in scope.from_root() {
Expand Down Expand Up @@ -799,7 +830,7 @@ where
if self.display_target {
let target = meta.target();
#[cfg(feature = "ansi")]
let target = if self.ansi {
let target = if writer.has_ansi_escapes() {
Style::new().bold().paint(target)
} else {
Style::new().paint(target)
Expand All @@ -811,7 +842,7 @@ where
ctx.format_fields(writer.by_ref(), event)?;

#[cfg(feature = "ansi")]
let dimmed = if self.ansi {
let dimmed = if writer.has_ansi_escapes() {
Style::new().dimmed()
} else {
Style::new()
Expand Down

0 comments on commit 8a7d0c5

Please sign in to comment.