Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subscriber: add lifetime parameter to MakeWriter #781

Merged
merged 6 commits into from
Nov 21, 2020
Merged

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Jul 6, 2020

Motivation

Currently, the tracing-subscriber crate has the MakeWriter trait for
customizing the io writer used by fmt. This trait is necessary (rather
than simply using a Write instance) because the default implementation
performs the IO on the thread where an event was recorded, meaning that
a separate writer needs to be acquired by each thread (either by calling
a function like io::stdout, by locking a shared Write instance,
etc).

Right now there is a blanket impl for Fn() -> T where T: Write. This
works fine with functions like io::stdout. However, the other common
case for this trait is locking a shared writer.

Therefore, it makes sense to see an implementation like this:

impl<'a, W: io::Write> MakeWriter for Mutex<W>
where
    W: io::Write,
{
    type Writer = MutexWriter<'a, W>;
    fn make_writer(&self) -> Self::Writer {
        MutexWriter(self.lock().unwrap())
    }
}

pub struct MutexWriter<'a, W>(MutexGuard<'a, W>);

impl<W: io::Write> io::Write for MutexWriter<'_, W> {
    // write to the shared writer in the `MutexGuard`...
}

Unfortunately, it's impossible to write this. Since MakeWriter always
takes an &self parameter and returns Self::Writer, the generic
parameter is unbounded:

    Checking tracing-subscriber v0.2.4 (/home/eliza/code/tracing/tracing-subscriber)
error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
  --> tracing-subscriber/src/fmt/writer.rs:61:6
   |
61 | impl<'a, W: io::Write> MakeWriter for Mutex<W>
   |      ^^ unconstrained lifetime parameter

error: aborting due to previous error

This essentially precludes any MakeWriter impl where the writer is
borrowed from the type implementing MakeWriter. This is a significant
blow to the usefulness of the trait. For example, it prevented the use
of MakeWriter in tracing-flame as suggested in
#631 (comment).

Proposal

This PR changes MakeWriter to be generic over a lifetime 'a:

pub trait MakeWriter<'a> {
    type Writer: io::Write;

    fn make_writer(&'a self) -> Self::Writer;
}

The self parameter is now borrowed for the &'a lifetime, so it is
okay to return a writer borrowed from self, such as in the Mutex
case.

I've also added an impl of MakeWriter for Mutex<T> where T: Writer.

Unfortunately, this is a breaking change and will need to wait until we
release tracing-subscriber 0.3.

Fixes #675.

Signed-off-by: Eliza Weisman eliza@buoyant.io

@hawkw
Copy link
Member Author

hawkw commented Jul 6, 2020

(this is currently a draft because it's part of the breaking changes planned for tracing-subscriber 0.3)

Copy link
Member

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment about direction. The new docs on MakeWriter are great.

@hawkw hawkw added this to the tracing-subscriber 0.3 milestone Jul 8, 2020
@hawkw hawkw added crate/subscriber Related to the `tracing-subscriber` crate meta/breaking This is a breaking change, and should wait until the next breaking release. labels Jul 8, 2020
@hawkw hawkw changed the base branch from master to subscriber-0.3-dev August 6, 2020 21:51
@hawkw hawkw changed the title [DRAFT] subscriber: add lifetime parameter to MakeWriter subscriber: add lifetime parameter to MakeWriter Aug 6, 2020
@hawkw hawkw marked this pull request as ready for review August 6, 2020 21:51
@hawkw hawkw requested review from jtescher, yaahc and a team as code owners August 6, 2020 21:51
@hawkw
Copy link
Member Author

hawkw commented Aug 6, 2020

I've rebased this onto a new dev branch for all the tracing-subscriber 0.3 breaking changes, and marked it as ready for review.

Copy link
Collaborator

@jtescher jtescher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me 👍

@hawkw hawkw requested a review from davidbarsky August 7, 2020 17:58
Copy link
Member

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two requested changes.

Should this be gated on the test framework changes to avoid an annoying rebase down the line?

tracing-subscriber/src/fmt/writer.rs Outdated Show resolved Hide resolved
tracing-subscriber/src/fmt/writer.rs Outdated Show resolved Hide resolved
@yaahc yaahc removed their request for review October 9, 2020 20:20
Motivation
----------

Currently, the `tracing-subscriber` crate has the `MakeWriter` trait for
customizing the io writer used by `fmt`. This trait is necessary (rather
than simply using a `Write` instance) because the default implementation
performs the IO on the thread where an event was recorded, meaning that
a separate writer needs to be acquired by each thread (either by calling
a function like `io::stdout`, by locking a shared `Write` instance,
etc).

Right now there is a blanket impl for `Fn() -> T where T: Write`. This
works fine with functions like `io::stdout`. However, the _other_ common
case for this trait is locking a shared writer.

Therefore, it makes sense to see an implementation like this:

``` rust
impl<'a, W: io::Write> MakeWriter for Mutex<W>
where
    W: io::Write,
{
    type Writer = MutexWriter<'a, W>;
    fn make_writer(&self) -> Self::Writer {
        MutexWriter(self.lock().unwrap())
    }
}

pub struct MutexWriter<'a, W>(MutexGuard<'a, W>);

impl<W: io::Write> io::Write for MutexWriter<'_, W> {
    // write to the shared writer in the `MutexGuard`...
}
```

Unfortunately, it's impossible to write this. Since `MakeWriter` always
takes an `&self` parameter and returns `Self::Writer`, the generic
parameter is unbounded:
```
    Checking tracing-subscriber v0.2.4 (/home/eliza/code/tracing/tracing-subscriber)
error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
  --> tracing-subscriber/src/fmt/writer.rs:61:6
   |
61 | impl<'a, W: io::Write> MakeWriter for Mutex<W>
   |      ^^ unconstrained lifetime parameter

error: aborting due to previous error
```

This essentially precludes any `MakeWriter` impl where the writer is
borrowed from the type implementing `MakeWriter`. This is a significant
blow to the usefulness of the trait. For example, it prevented the use
of `MakeWriter` in `tracing-flame` as suggested in
#631 (comment).

Proposal
--------

This PR changes `MakeWriter` to be generic over a lifetime `'a`:

```rust
pub trait MakeWriter<'a> {
    type Writer: io::Write;

    fn make_writer(&'a self) -> Self::Writer;
}
```
The `self` parameter is now borrowed for the `&'a` lifetime, so it is
okay to return a writer borrowed from `self`, such as in the `Mutex`
case.

I've also added an impl of `MakeWriter` for `Mutex<T> where T: Writer`.

Unfortunately, this is a breaking change and will need to wait until we
release `tracing-subscriber` 0.3.

Fixes #675.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw changed the base branch from subscriber-0.3-dev to master October 26, 2020 23:12
@hawkw hawkw force-pushed the eliza/fix-makewriter branch 2 times, most recently from 254a2f0 to 348e8d3 Compare October 27, 2020 22:29
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw
Copy link
Member Author

hawkw commented Oct 27, 2020

@davidbarsky this should be good to ship whenever?

@hawkw hawkw merged commit b1baa6c into master Nov 21, 2020
hawkw added a commit that referenced this pull request Oct 19, 2021
This backports PR #781 from `master`.

## Motivation

Currently, the `tracing-subscriber` crate has the `MakeWriter` trait for
customizing the io writer used by `fmt`. This trait is necessary (rather
than simply using a `Write` instance) because the default implementation
performs the IO on the thread where an event was recorded, meaning that
a separate writer needs to be acquired by each thread (either by calling
a function like `io::stdout`, by locking a shared `Write` instance,
etc).

Right now there is a blanket impl for `Fn() -> T where T: Write`. This
works fine with functions like `io::stdout`. However, the _other_ common
case for this trait is locking a shared writer.

Therefore, it makes sense to see an implementation like this:

``` rust
impl<'a, W: io::Write> MakeWriter for Mutex<W>
where
    W: io::Write,
{
    type Writer = MutexWriter<'a, W>;
    fn make_writer(&self) -> Self::Writer {
        MutexWriter(self.lock().unwrap())
    }
}

pub struct MutexWriter<'a, W>(MutexGuard<'a, W>);

impl<W: io::Write> io::Write for MutexWriter<'_, W> {
    // write to the shared writer in the `MutexGuard`...
}
```

Unfortunately, it's impossible to write this. Since `MakeWriter` always
takes an `&self` parameter and returns `Self::Writer`, the generic
parameter is unbounded:
```
    Checking tracing-subscriber v0.2.4 (/home/eliza/code/tracing/tracing-subscriber)
error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
  --> tracing-subscriber/src/fmt/writer.rs:61:6
   |
61 | impl<'a, W: io::Write> MakeWriter for Mutex<W>
   |      ^^ unconstrained lifetime parameter

error: aborting due to previous error
```

This essentially precludes any `MakeWriter` impl where the writer is
borrowed from the type implementing `MakeWriter`. This is a significant
blow to the usefulness of the trait. For example, it prevented the use
of `MakeWriter` in `tracing-flame` as suggested in
#631 (comment).

## Proposal

This PR changes `MakeWriter` to be generic over a lifetime `'a`:

```rust
pub trait MakeWriter<'a> {
    type Writer: io::Write;

    fn make_writer(&'a self) -> Self::Writer;
}
```
The `self` parameter is now borrowed for the `&'a` lifetime, so it is
okay to return a writer borrowed from `self`, such as in the `Mutex`
case.

I've also added an impl of `MakeWriter` for `Mutex<T> where T: Writer`.

Unfortunately, this is a breaking change and will need to wait until we
release `tracing-subscriber` 0.3.

Fixes #675.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Oct 19, 2021
This backports PR #781 from `master`.

## Motivation

Currently, the `tracing-subscriber` crate has the `MakeWriter` trait for
customizing the io writer used by `fmt`. This trait is necessary (rather
than simply using a `Write` instance) because the default implementation
performs the IO on the thread where an event was recorded, meaning that
a separate writer needs to be acquired by each thread (either by calling
a function like `io::stdout`, by locking a shared `Write` instance,
etc).

Right now there is a blanket impl for `Fn() -> T where T: Write`. This
works fine with functions like `io::stdout`. However, the _other_ common
case for this trait is locking a shared writer.

Therefore, it makes sense to see an implementation like this:

``` rust
impl<'a, W: io::Write> MakeWriter for Mutex<W>
where
    W: io::Write,
{
    type Writer = MutexWriter<'a, W>;
    fn make_writer(&self) -> Self::Writer {
        MutexWriter(self.lock().unwrap())
    }
}

pub struct MutexWriter<'a, W>(MutexGuard<'a, W>);

impl<W: io::Write> io::Write for MutexWriter<'_, W> {
    // write to the shared writer in the `MutexGuard`...
}
```

Unfortunately, it's impossible to write this. Since `MakeWriter` always
takes an `&self` parameter and returns `Self::Writer`, the generic
parameter is unbounded:
```
    Checking tracing-subscriber v0.2.4 (/home/eliza/code/tracing/tracing-subscriber)
error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
  --> tracing-subscriber/src/fmt/writer.rs:61:6
   |
61 | impl<'a, W: io::Write> MakeWriter for Mutex<W>
   |      ^^ unconstrained lifetime parameter

error: aborting due to previous error
```

This essentially precludes any `MakeWriter` impl where the writer is
borrowed from the type implementing `MakeWriter`. This is a significant
blow to the usefulness of the trait. For example, it prevented the use
of `MakeWriter` in `tracing-flame` as suggested in
#631 (comment).

## Proposal

This PR changes `MakeWriter` to be generic over a lifetime `'a`:

```rust
pub trait MakeWriter<'a> {
    type Writer: io::Write;

    fn make_writer(&'a self) -> Self::Writer;
}
```
The `self` parameter is now borrowed for the `&'a` lifetime, so it is
okay to return a writer borrowed from `self`, such as in the `Mutex`
case.

I've also added an impl of `MakeWriter` for `Mutex<T> where T: Writer`.

Unfortunately, this is a breaking change and will need to wait until we
release `tracing-subscriber` 0.3.

Fixes #675.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Oct 22, 2021
# 0.3.0 (Oct 22, 2021)

This is a breaking release of `tracing-subscriber`. The primary breaking
change in this release is the removal of the dependency on the [`chrono`
crate], due to [RUSTSEC-2020-0159]. To replace `chrono`, support is
added for formatting timestamps using the [`time` crate] instead.

In addition, this release includes a number of other breaking API
changes, such as adding (limited) support for `#![no_std]` targets,
removing previously deprecated APIs, and more.

### Breaking Changes

- Removed APIs deprecated in the v0.2.x release series.
- Renamed `Layer::new_span` to `Layer::on_new_span` ([#1674])
- Removed `Layer` impl for `Arc<L: Layer<S>>` and `Arc<dyn Layer<S> +
  ...>` ([#1649])
- Replaced the [`chrono` crate] with the [`time` crate] for timestamp
  formatting, to resolve [RUSTSEC-2020-0159] ([#1646])
- Removed `json` and `env-filter` from default features. They must now
  be enabled explictly ([#1647])
- Changed `FormatEvent::format_event` and `FormatFields::format_fields`
  trait methods to take a `Writer` type, rather than a `&mut dyn
  fmt::Write` trait object ([#1661])
- Changed the signature of the `MakeWriter` trait by adding a lifetime
  parameter ([#781])
  ### Changed

- **layer**: Renamed `Layer::new_span` to `Layer::on_new_span` ([#1674])
- **fmt**: Changed `FormatEvent::format_event` and
  `FormatFields::format_fields` trait methods to take a `Writer` type,
  rather than a `&mut dyn fmt::Write` trait object ([#1661])
- **json**, **env-filter**: `json` and `env-filter` feature flags are no
  longer enabled by default ([#1647])
  ### Removed

- Removed deprecated `CurrentSpan` type ([#1320])
- **registry**: Removed deprecated `SpanRef::parents` iterator, replaced
  by `SpanRef::scope` in [#1431] ([#1648)])
- **layer**: Removed deprecated `Context::scope` iterator, replaced by
  `Context::span_scope` and `Context::event_scope` in [#1431] and
  [#1434] ([#1648)])
- **layer**: Removed `Layer` impl for `Arc<L: Layer<S>>` and `Arc<dyn
  Layer<S> + ...>`. These interfere with per-layer filtering. ([#1649])
- **fmt**: Removed deprecated `LayerBuilder` type ([#1673])
- **fmt**: Removed `fmt::Layer::on_event` (renamed to
  `fmt::Layer::fmt_event`) ([#1673])
- **fmt**, **chrono**: Removed the `chrono` feature flag and APIs for
  using the [`chrono` crate] for timestamp formatting ([#1646])
  ### Added

- **fmt**, **time**: `LocalTime` and `UtcTime` types for formatting
  timestamps using the [`time` crate] ([#1646])
- **fmt**: Added a lifetime parameter to the `MakeWriter` trait,
  allowing it to return a borrowed writer. This enables implementations
  of `MakeWriter` for types such as `Mutex<T: io::Write>` and
  `std::fs::File`. ([#781])
- **env-filter**: Documentation improvements ([#1637])
- Support for some APIs on `#![no_std]` targets, by disabling the `std`
  feature flag ([#1660])

Thanks to @Folyd and @nmathewson for contributing to this release!

[#1320]: #1320
[#1673]: #1673
[#1674]: #1674
[#1646]: #1646
[#1647]: #1647
[#1648]: #1648
[#1649]: #1649
[#1660]: #1660
[#1661]: #1661
[#1431]: #1431
[#1434]: #1434
[#781]: #781

[`chrono` crate]: https://crates.io/crates/chrono
[`time` crate]: https://crates.io/crates/time
[RUSTSEC-2020-0159]: https://rustsec.org/advisories/RUSTSEC-2020-0159.html
hawkw added a commit that referenced this pull request Oct 22, 2021
# 0.3.0 (Oct 22, 2021)

This is a breaking release of `tracing-subscriber`. The primary breaking
change in this release is the removal of the dependency on the [`chrono`
crate], due to [RUSTSEC-2020-0159]. To replace `chrono`, support is
added for formatting timestamps using the [`time` crate] instead.

In addition, this release includes a number of other breaking API
changes, such as adding (limited) support for `#![no_std]` targets,
removing previously deprecated APIs, and more.

### Breaking Changes

- Removed APIs deprecated in the v0.2.x release series.
- Renamed `Layer::new_span` to `Layer::on_new_span` ([#1674])
- Removed `Layer` impl for `Arc<L: Layer<S>>` and `Arc<dyn Layer<S> +
  ...>` ([#1649])
- Replaced the [`chrono` crate] with the [`time` crate] for timestamp
  formatting, to resolve [RUSTSEC-2020-0159] ([#1646])
- Removed `json` and `env-filter` from default features. They must now
  be enabled explictly ([#1647])
- Changed `FormatEvent::format_event` and `FormatFields::format_fields`
  trait methods to take a `Writer` type, rather than a `&mut dyn
  fmt::Write` trait object ([#1661])
- Changed the signature of the `MakeWriter` trait by adding a lifetime
  parameter ([#781])
  ### Changed

- **layer**: Renamed `Layer::new_span` to `Layer::on_new_span` ([#1674])
- **fmt**: Changed `FormatEvent::format_event` and
  `FormatFields::format_fields` trait methods to take a `Writer` type,
  rather than a `&mut dyn fmt::Write` trait object ([#1661])
- **json**, **env-filter**: `json` and `env-filter` feature flags are no
  longer enabled by default ([#1647])
  ### Removed

- Removed deprecated `CurrentSpan` type ([#1320])
- **registry**: Removed deprecated `SpanRef::parents` iterator, replaced
  by `SpanRef::scope` in [#1431] ([#1648)])
- **layer**: Removed deprecated `Context::scope` iterator, replaced by
  `Context::span_scope` and `Context::event_scope` in [#1431] and
  [#1434] ([#1648)])
- **layer**: Removed `Layer` impl for `Arc<L: Layer<S>>` and `Arc<dyn
  Layer<S> + ...>`. These interfere with per-layer filtering. ([#1649])
- **fmt**: Removed deprecated `LayerBuilder` type ([#1673])
- **fmt**: Removed `fmt::Layer::on_event` (renamed to
  `fmt::Layer::fmt_event`) ([#1673])
- **fmt**, **chrono**: Removed the `chrono` feature flag and APIs for
  using the [`chrono` crate] for timestamp formatting ([#1646])
  ### Added

- **fmt**, **time**: `LocalTime` and `UtcTime` types for formatting
  timestamps using the [`time` crate] ([#1646])
- **fmt**: Added a lifetime parameter to the `MakeWriter` trait,
  allowing it to return a borrowed writer. This enables implementations
  of `MakeWriter` for types such as `Mutex<T: io::Write>` and
  `std::fs::File`. ([#781])
- **env-filter**: Documentation improvements ([#1637])
- Support for some APIs on `#![no_std]` targets, by disabling the `std`
  feature flag ([#1660])

Thanks to @Folyd and @nmathewson for contributing to this release!

[#1320]: #1320
[#1673]: #1673
[#1674]: #1674
[#1646]: #1646
[#1647]: #1647
[#1648]: #1648
[#1649]: #1649
[#1660]: #1660
[#1661]: #1661
[#1431]: #1431
[#1434]: #1434
[#781]: #781

[`chrono` crate]: https://crates.io/crates/chrono
[`time` crate]: https://crates.io/crates/time
[RUSTSEC-2020-0159]: https://rustsec.org/advisories/RUSTSEC-2020-0159.html

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Oct 29, 2021
## Motivation

In `tracing-subscriber` 0.3.x, `MakeWriter` filtering seems to have
stopped working when using the `BoxMakeWriter` wrapper to erase the type
of a `MakeWriter` implementation. It looks like what happened is that
commit 6cc6c47, which backported the
change to add a lifetime parameter to `MakeWriter` (#781), I
accidentally clobbered the `make_writer_for` method on the inner `Boxed`
type, so that it only has `make_writer`:
6cc6c47#diff-c5dc275b15a60c1a2d4694da3797f4247c4f2e1e0978fd210dd14452d6746283L737-L739

This meant that any filtering performed by the `MakeWriter` inside the
box is now ignored. My bad!

## Solution

This commit puts back the missing `make_writer_for` method. Whoops!

Fixes #1694
hawkw added a commit that referenced this pull request Oct 29, 2021
## Motivation

In `tracing-subscriber` 0.3.x, `MakeWriter` filtering seems to have
stopped working when using the `BoxMakeWriter` wrapper to erase the type
of a `MakeWriter` implementation. It looks like what happened is that
commit 6cc6c47, which backported the
change to add a lifetime parameter to `MakeWriter` (#781), I
accidentally clobbered the `make_writer_for` method on the inner `Boxed`
type, so that it only has `make_writer`:
6cc6c47#diff-c5dc275b15a60c1a2d4694da3797f4247c4f2e1e0978fd210dd14452d6746283L737-L739

This meant that any filtering performed by the `MakeWriter` inside the
box is now ignored. My bad!

## Solution

This commit puts back the missing `make_writer_for` method. Whoops!

Fixes #1694
davidbarsky pushed a commit to tokio-rs/tracing-opentelemetry that referenced this pull request Mar 21, 2023
# 0.3.0 (Oct 22, 2021)

This is a breaking release of `tracing-subscriber`. The primary breaking
change in this release is the removal of the dependency on the [`chrono`
crate], due to [RUSTSEC-2020-0159]. To replace `chrono`, support is
added for formatting timestamps using the [`time` crate] instead.

In addition, this release includes a number of other breaking API
changes, such as adding (limited) support for `#![no_std]` targets,
removing previously deprecated APIs, and more.

### Breaking Changes

- Removed APIs deprecated in the v0.2.x release series.
- Renamed `Layer::new_span` to `Layer::on_new_span` ([#1674])
- Removed `Layer` impl for `Arc<L: Layer<S>>` and `Arc<dyn Layer<S> +
  ...>` ([#1649])
- Replaced the [`chrono` crate] with the [`time` crate] for timestamp
  formatting, to resolve [RUSTSEC-2020-0159] ([#1646])
- Removed `json` and `env-filter` from default features. They must now
  be enabled explictly ([#1647])
- Changed `FormatEvent::format_event` and `FormatFields::format_fields`
  trait methods to take a `Writer` type, rather than a `&mut dyn
  fmt::Write` trait object ([#1661])
- Changed the signature of the `MakeWriter` trait by adding a lifetime
  parameter ([#781])
  ### Changed

- **layer**: Renamed `Layer::new_span` to `Layer::on_new_span` ([#1674])
- **fmt**: Changed `FormatEvent::format_event` and
  `FormatFields::format_fields` trait methods to take a `Writer` type,
  rather than a `&mut dyn fmt::Write` trait object ([#1661])
- **json**, **env-filter**: `json` and `env-filter` feature flags are no
  longer enabled by default ([#1647])
  ### Removed

- Removed deprecated `CurrentSpan` type ([#1320])
- **registry**: Removed deprecated `SpanRef::parents` iterator, replaced
  by `SpanRef::scope` in [#1431] ([#1648)])
- **layer**: Removed deprecated `Context::scope` iterator, replaced by
  `Context::span_scope` and `Context::event_scope` in [#1431] and
  [#1434] ([#1648)])
- **layer**: Removed `Layer` impl for `Arc<L: Layer<S>>` and `Arc<dyn
  Layer<S> + ...>`. These interfere with per-layer filtering. ([#1649])
- **fmt**: Removed deprecated `LayerBuilder` type ([#1673])
- **fmt**: Removed `fmt::Layer::on_event` (renamed to
  `fmt::Layer::fmt_event`) ([#1673])
- **fmt**, **chrono**: Removed the `chrono` feature flag and APIs for
  using the [`chrono` crate] for timestamp formatting ([#1646])
  ### Added

- **fmt**, **time**: `LocalTime` and `UtcTime` types for formatting
  timestamps using the [`time` crate] ([#1646])
- **fmt**: Added a lifetime parameter to the `MakeWriter` trait,
  allowing it to return a borrowed writer. This enables implementations
  of `MakeWriter` for types such as `Mutex<T: io::Write>` and
  `std::fs::File`. ([#781])
- **env-filter**: Documentation improvements ([#1637])
- Support for some APIs on `#![no_std]` targets, by disabling the `std`
  feature flag ([#1660])

Thanks to @Folyd and @nmathewson for contributing to this release!

[#1320]: tokio-rs/tracing#1320
[#1673]: tokio-rs/tracing#1673
[#1674]: tokio-rs/tracing#1674
[#1646]: tokio-rs/tracing#1646
[#1647]: tokio-rs/tracing#1647
[#1648]: tokio-rs/tracing#1648
[#1649]: tokio-rs/tracing#1649
[#1660]: tokio-rs/tracing#1660
[#1661]: tokio-rs/tracing#1661
[#1431]: tokio-rs/tracing#1431
[#1434]: tokio-rs/tracing#1434
[#781]: tokio-rs/tracing#781

[`chrono` crate]: https://crates.io/crates/chrono
[`time` crate]: https://crates.io/crates/time
[RUSTSEC-2020-0159]: https://rustsec.org/advisories/RUSTSEC-2020-0159.html

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
…kio-rs#1654)

This backports PR tokio-rs#781 from `master`.

## Motivation

Currently, the `tracing-subscriber` crate has the `MakeWriter` trait for
customizing the io writer used by `fmt`. This trait is necessary (rather
than simply using a `Write` instance) because the default implementation
performs the IO on the thread where an event was recorded, meaning that
a separate writer needs to be acquired by each thread (either by calling
a function like `io::stdout`, by locking a shared `Write` instance,
etc).

Right now there is a blanket impl for `Fn() -> T where T: Write`. This
works fine with functions like `io::stdout`. However, the _other_ common
case for this trait is locking a shared writer.

Therefore, it makes sense to see an implementation like this:

``` rust
impl<'a, W: io::Write> MakeWriter for Mutex<W>
where
    W: io::Write,
{
    type Writer = MutexWriter<'a, W>;
    fn make_writer(&self) -> Self::Writer {
        MutexWriter(self.lock().unwrap())
    }
}

pub struct MutexWriter<'a, W>(MutexGuard<'a, W>);

impl<W: io::Write> io::Write for MutexWriter<'_, W> {
    // write to the shared writer in the `MutexGuard`...
}
```

Unfortunately, it's impossible to write this. Since `MakeWriter` always
takes an `&self` parameter and returns `Self::Writer`, the generic
parameter is unbounded:
```
    Checking tracing-subscriber v0.2.4 (/home/eliza/code/tracing/tracing-subscriber)
error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
  --> tracing-subscriber/src/fmt/writer.rs:61:6
   |
61 | impl<'a, W: io::Write> MakeWriter for Mutex<W>
   |      ^^ unconstrained lifetime parameter

error: aborting due to previous error
```

This essentially precludes any `MakeWriter` impl where the writer is
borrowed from the type implementing `MakeWriter`. This is a significant
blow to the usefulness of the trait. For example, it prevented the use
of `MakeWriter` in `tracing-flame` as suggested in
tokio-rs#631 (comment).

## Proposal

This PR changes `MakeWriter` to be generic over a lifetime `'a`:

```rust
pub trait MakeWriter<'a> {
    type Writer: io::Write;

    fn make_writer(&'a self) -> Self::Writer;
}
```
The `self` parameter is now borrowed for the `&'a` lifetime, so it is
okay to return a writer borrowed from `self`, such as in the `Mutex`
case.

I've also added an impl of `MakeWriter` for `Mutex<T> where T: Writer`.

Unfortunately, this is a breaking change and will need to wait until we
release `tracing-subscriber` 0.3.

Fixes tokio-rs#675.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
# 0.3.0 (Oct 22, 2021)

This is a breaking release of `tracing-subscriber`. The primary breaking
change in this release is the removal of the dependency on the [`chrono`
crate], due to [RUSTSEC-2020-0159]. To replace `chrono`, support is
added for formatting timestamps using the [`time` crate] instead.

In addition, this release includes a number of other breaking API
changes, such as adding (limited) support for `#![no_std]` targets,
removing previously deprecated APIs, and more.

### Breaking Changes

- Removed APIs deprecated in the v0.2.x release series.
- Renamed `Layer::new_span` to `Layer::on_new_span` ([tokio-rs#1674])
- Removed `Layer` impl for `Arc<L: Layer<S>>` and `Arc<dyn Layer<S> +
  ...>` ([tokio-rs#1649])
- Replaced the [`chrono` crate] with the [`time` crate] for timestamp
  formatting, to resolve [RUSTSEC-2020-0159] ([tokio-rs#1646])
- Removed `json` and `env-filter` from default features. They must now
  be enabled explictly ([tokio-rs#1647])
- Changed `FormatEvent::format_event` and `FormatFields::format_fields`
  trait methods to take a `Writer` type, rather than a `&mut dyn
  fmt::Write` trait object ([tokio-rs#1661])
- Changed the signature of the `MakeWriter` trait by adding a lifetime
  parameter ([tokio-rs#781])
  ### Changed

- **layer**: Renamed `Layer::new_span` to `Layer::on_new_span` ([tokio-rs#1674])
- **fmt**: Changed `FormatEvent::format_event` and
  `FormatFields::format_fields` trait methods to take a `Writer` type,
  rather than a `&mut dyn fmt::Write` trait object ([tokio-rs#1661])
- **json**, **env-filter**: `json` and `env-filter` feature flags are no
  longer enabled by default ([tokio-rs#1647])
  ### Removed

- Removed deprecated `CurrentSpan` type ([tokio-rs#1320])
- **registry**: Removed deprecated `SpanRef::parents` iterator, replaced
  by `SpanRef::scope` in [tokio-rs#1431] ([tokio-rs#1648)])
- **layer**: Removed deprecated `Context::scope` iterator, replaced by
  `Context::span_scope` and `Context::event_scope` in [tokio-rs#1431] and
  [tokio-rs#1434] ([tokio-rs#1648)])
- **layer**: Removed `Layer` impl for `Arc<L: Layer<S>>` and `Arc<dyn
  Layer<S> + ...>`. These interfere with per-layer filtering. ([tokio-rs#1649])
- **fmt**: Removed deprecated `LayerBuilder` type ([tokio-rs#1673])
- **fmt**: Removed `fmt::Layer::on_event` (renamed to
  `fmt::Layer::fmt_event`) ([tokio-rs#1673])
- **fmt**, **chrono**: Removed the `chrono` feature flag and APIs for
  using the [`chrono` crate] for timestamp formatting ([tokio-rs#1646])
  ### Added

- **fmt**, **time**: `LocalTime` and `UtcTime` types for formatting
  timestamps using the [`time` crate] ([tokio-rs#1646])
- **fmt**: Added a lifetime parameter to the `MakeWriter` trait,
  allowing it to return a borrowed writer. This enables implementations
  of `MakeWriter` for types such as `Mutex<T: io::Write>` and
  `std::fs::File`. ([tokio-rs#781])
- **env-filter**: Documentation improvements ([tokio-rs#1637])
- Support for some APIs on `#![no_std]` targets, by disabling the `std`
  feature flag ([tokio-rs#1660])

Thanks to @Folyd and @nmathewson for contributing to this release!

[tokio-rs#1320]: tokio-rs#1320
[tokio-rs#1673]: tokio-rs#1673
[tokio-rs#1674]: tokio-rs#1674
[tokio-rs#1646]: tokio-rs#1646
[tokio-rs#1647]: tokio-rs#1647
[tokio-rs#1648]: tokio-rs#1648
[tokio-rs#1649]: tokio-rs#1649
[tokio-rs#1660]: tokio-rs#1660
[tokio-rs#1661]: tokio-rs#1661
[tokio-rs#1431]: tokio-rs#1431
[tokio-rs#1434]: tokio-rs#1434
[tokio-rs#781]: tokio-rs#781

[`chrono` crate]: https://crates.io/crates/chrono
[`time` crate]: https://crates.io/crates/time
[RUSTSEC-2020-0159]: https://rustsec.org/advisories/RUSTSEC-2020-0159.html

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
…s#1695)

## Motivation

In `tracing-subscriber` 0.3.x, `MakeWriter` filtering seems to have
stopped working when using the `BoxMakeWriter` wrapper to erase the type
of a `MakeWriter` implementation. It looks like what happened is that
commit 6cc6c47, which backported the
change to add a lifetime parameter to `MakeWriter` (tokio-rs#781), I
accidentally clobbered the `make_writer_for` method on the inner `Boxed`
type, so that it only has `make_writer`:
tokio-rs@6cc6c47#diff-c5dc275b15a60c1a2d4694da3797f4247c4f2e1e0978fd210dd14452d6746283L737-L739

This meant that any filtering performed by the `MakeWriter` inside the
box is now ignored. My bad!

## Solution

This commit puts back the missing `make_writer_for` method. Whoops!

Fixes tokio-rs#1694
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/subscriber Related to the `tracing-subscriber` crate meta/breaking This is a breaking change, and should wait until the next breaking release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

subscriber: MakeWriter probably needs to be generic over a lifetime
3 participants