[tracing-subscriber]: add chrono crate implementations of FormatTime #2690

shayne-fletcher · 2023-08-18T19:58:52Z

Motivation

Issue #2080 explains that it's not possible to soundly use tracing_subscriber::fmt::time::LocalTime in a multithreaded context. It proposes adding alternative time formatters that use the chrono crate to workaround which is what this PR offers.

Solution

A new source file 'chrono_crate.rs' is added to the 'tracing-subscriber' package implementing mod chrono_crate providing two new tag types LocalTime and Utc with associated time::FormatTime trait implementations that call chrono::Local::now().to_rfc3339() and chrono::Utc::now().to_rfc3339() respectively. Simple unit-tests of the new functionality accompany the additions.

…(local time & UTC)

davidbarsky · 2023-08-21T14:11:49Z

One note: we've got a few broken doc tests. One failure—which you can see [here](https://app.netlify.com/sites/tracing-rs/deploys/64dfd174e129e00008898be10—is complaining that no module named Chrono is in scope. I think you might need to write [`chrono`], but you should be iron out any issues using cargo doc --no-deps --all-features locally.

davidbarsky

I forgot to add this, but it's probably not unreasonable to copy-and-paste the contents of https://docs.rs/tracing-subscriber/0.2.25/src/tracing_subscriber/fmt/time/mod.rs.html#157 back into into here.

tracing-subscriber/src/fmt/time/chrono_crate.rs

shayne-fletcher · 2023-08-21T18:49:16Z

test plan:

cargo test --features "chrono-time" -p tracing-subscriber chrono_format_time
cargo doc --no-deps --all-features

tracing-subscriber/src/fmt/time/chrono_crate.rs

davidbarsky · 2023-09-21T18:04:59Z

@shayne-fletcher alright, I think that this PR is in a good spot. Because the blast radius of adding a new dependency might be a large, I'd like to get @hawkw's eyes on this PR before we merge.

hawkw

Overall, this looks pretty close to ready --- the API structure seems good to me. I commented on a few things I think we ought to address first.

Thanks for the PR!

tracing-subscriber/src/fmt/time/chrono_crate.rs

tracing-subscriber/src/fmt/time/mod.rs

tracing-subscriber/Cargo.toml

hawkw

looks good to me, modulo one little nit about docs.

hawkw · 2023-09-22T18:48:57Z

tracing-subscriber/src/fmt/time/chrono_crate.rs

+use crate::fmt::format::Writer;
+use crate::fmt::time::FormatTime;
+
+use std::sync::Arc;
+
+/// Formats [local time]s and [UTC time]s with `FormatTime` implementations
+/// that use the [`chrono` crate].
+///
+/// [local time]: [`chrono::offset::Local`]
+/// [UTC time]: [`chrono::offset::Utc`]
+/// [`chrono` crate]: [`chrono`]


I think this was supposed to be a module-level doc comment?

Suggested change

use crate::fmt::format::Writer;

use crate::fmt::time::FormatTime;

use std::sync::Arc;

/// Formats [local time]s and [UTC time]s with `FormatTime` implementations

/// that use the [`chrono` crate].

///

/// [local time]: [`chrono::offset::Local`]

/// [UTC time]: [`chrono::offset::Utc`]

/// [`chrono` crate]: [`chrono`]

//! Formats [local time]s and [UTC time]s with `FormatTime` implementations

//! that use the [`chrono` crate].

//!

//! [local time]: [`chrono::offset::Local`]

//! [UTC time]: [`chrono::offset::Utc`]

//! [`chrono` crate]: [`chrono`]

use crate::fmt::format::Writer;

use crate::fmt::time::FormatTime;

use std::sync::Arc;

...but, i guess having a module-level doc comment isn't super important since the module is private?

…2690) Issue #2080 explains that it's not possible to soundly use [`tracing_subscriber::fmt::time::LocalTime`](https://docs.rs/tracing-subscriber/latest/tracing_subscriber/fmt/time/struct.LocalTime.html) in a multithreaded context. It proposes adding alternative time formatters that use the [chrono crate](https://docs.rs/chrono/latest/chrono/) to workaround which is what this PR offers. A new source file 'chrono_crate.rs' is added to the 'tracing-subscriber' package implementing `mod chrono_crate` providing two new tag types `LocalTime` and `Utc` with associated `time::FormatTime` trait implementations that call `chrono::Local::now().to_rfc3339()` and `chrono::Utc::now().to_rfc3339()` respectively. Simple unit-tests of the new functionality accompany the additions. --------- Co-authored-by: David Barsky <me@davidbarsky.com> Co-authored-by: Shayne Fletcher <shaynefletcher@meta.com>

@shayne-fletcher

# 0.3.18 (November 13, 2023) This release of `tracing-subscriber` adds support for the [`NO_COLOR`] environment variable (an informal standard to disable emitting ANSI color escape codes) in `fmt::Layer`, reintroduces support for the [`chrono`] crate, and increases the minimum supported Rust version (MSRV) to Rust 1.63.0. It also introduces several minor API improvements. ### Added - **chrono**: Add [`chrono`] implementations of `FormatTime` ([#2690]) - **subscriber**: Add support for the [`NO_COLOR`] environment variable in `fmt::Layer` ([#2647]) - **fmt**: make `format::Writer::new()` public ([#2680]) - **filter**: Implement `layer::Filter` for `Option<Filter>` ([#2407]) ### Changed - **log**: bump version of `tracing-log` to 0.2 ([#2772]) - Increased minimum supported Rust version (MSRV) to 1.63.0+. [`chrono`]: https://github.com/chronotope/chrono [`NO_COLOR`]: https://no-color.org/ [#2690]: #2690 [#2647]: #2647 [#2680]: #2680 [#2407]: #2407 [#2772]: #2772 Thanks to @shayne-fletcher, @dmlary, @kaifastromai, and @jsgf for contributing!

Thanks to the new `chrono` support in `tracing-subscriber`. - https://docs.rs/tracing-subscriber/latest/tracing_subscriber/fmt/time/struct.ChronoLocal.html - tokio-rs/tracing#2690

@shayne-fletcher

# 0.3.18 (November 13, 2023) This release of `tracing-subscriber` adds support for the [`NO_COLOR`] environment variable (an informal standard to disable emitting ANSI color escape codes) in `fmt::Layer`, reintroduces support for the [`chrono`] crate, and increases the minimum supported Rust version (MSRV) to Rust 1.63.0. It also introduces several minor API improvements. ### Added - **chrono**: Add [`chrono`] implementations of `FormatTime` ([tokio-rs#2690]) - **subscriber**: Add support for the [`NO_COLOR`] environment variable in `fmt::Layer` ([tokio-rs#2647]) - **fmt**: make `format::Writer::new()` public ([tokio-rs#2680]) - **filter**: Implement `layer::Filter` for `Option<Filter>` ([tokio-rs#2407]) ### Changed - **log**: bump version of `tracing-log` to 0.2 ([tokio-rs#2772]) - Increased minimum supported Rust version (MSRV) to 1.63.0+. [`chrono`]: https://github.com/chronotope/chrono [`NO_COLOR`]: https://no-color.org/ [tokio-rs#2690]: tokio-rs#2690 [tokio-rs#2647]: tokio-rs#2647 [tokio-rs#2680]: tokio-rs#2680 [tokio-rs#2407]: tokio-rs#2407 [tokio-rs#2772]: tokio-rs#2772 Thanks to @shayne-fletcher, @dmlary, @kaifastromai, and @jsgf for contributing!

shayne-fletcher requested review from hawkw, davidbarsky and a team as code owners August 18, 2023 19:58

[tracing-subscriber]: add chrono crate implementations of FormatTime …

09f209b

…(local time & UTC)

shayne-fletcher force-pushed the chrono-subscriber branch from db86c5a to 09f209b Compare August 18, 2023 20:15

davidbarsky requested changes Aug 21, 2023

View reviewed changes

shayne-fletcher requested a review from davidbarsky August 21, 2023 18:54

shayne-fletcher force-pushed the chrono-subscriber branch from dbb35e0 to 5576e08 Compare August 21, 2023 20:28

[tracing-subscriber]: add chrono feature

f4817a5

shayne-fletcher force-pushed the chrono-subscriber branch from 5576e08 to f4817a5 Compare August 21, 2023 21:10

allow for custom datetime formattters w/chrono_crate

f802334

davidbarsky reviewed Sep 6, 2023

View reviewed changes

davidbarsky and others added 2 commits September 6, 2023 14:48

Merge branch 'master' into chrono-subscriber

e2b09eb

[tracing-subscriber]: remove redundant cfg

897cfeb

shayne-fletcher requested a review from davidbarsky September 7, 2023 18:04

davidbarsky added 2 commits September 7, 2023 17:32

Merge branch 'master' into chrono-subscriber

3e9aa30

edit docs, add constructors for ChronoLocal and ChronoUtc

ae007f9

davidbarsky force-pushed the chrono-subscriber branch from a42b9de to ae007f9 Compare September 21, 2023 16:13

fix invalid doc(cfg(...))

e2c7bec

hawkw requested changes Sep 21, 2023

View reviewed changes

implement hawkw's review suggestions

269695a

shayne-fletcher requested a review from hawkw September 22, 2023 15:18

fix remaining nits

56f3577

hawkw reviewed Sep 22, 2023

View reviewed changes

hawkw approved these changes Sep 22, 2023

View reviewed changes

davidbarsky approved these changes Sep 25, 2023

View reviewed changes

davidbarsky merged commit 2f8f249 into tokio-rs:master Sep 25, 2023
55 checks passed

davidbarsky mentioned this pull request Sep 26, 2023

chore: backport roughly a year's worth of changes #2728

Merged

azzamsa mentioned this pull request Oct 17, 2023

Is there a way to avoid passing the UTC offset? azzamsa/tin#2

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tracing-subscriber]: add chrono crate implementations of FormatTime #2690

[tracing-subscriber]: add chrono crate implementations of FormatTime #2690

shayne-fletcher commented Aug 18, 2023

davidbarsky commented Aug 21, 2023

davidbarsky left a comment

shayne-fletcher commented Aug 21, 2023 •

edited

davidbarsky commented Sep 21, 2023

hawkw left a comment

hawkw left a comment

hawkw Sep 22, 2023

hawkw Sep 22, 2023

[tracing-subscriber]: add chrono crate implementations of FormatTime #2690

[tracing-subscriber]: add chrono crate implementations of FormatTime #2690

Conversation

shayne-fletcher commented Aug 18, 2023

Motivation

Solution

davidbarsky commented Aug 21, 2023

davidbarsky left a comment

Choose a reason for hiding this comment

shayne-fletcher commented Aug 21, 2023 • edited

davidbarsky commented Sep 21, 2023

hawkw left a comment

Choose a reason for hiding this comment

hawkw left a comment

Choose a reason for hiding this comment

hawkw Sep 22, 2023

Choose a reason for hiding this comment

hawkw Sep 22, 2023

Choose a reason for hiding this comment

shayne-fletcher commented Aug 21, 2023 •

edited