tracing icon indicating copy to clipboard operation
tracing copied to clipboard

subscriber: re-introduce `chrono` types and feature flags

Open davidbarsky opened this issue 2 years ago • 2 comments

Feature Request

Crates

  • This proposed change impacts tracing-subscriber.

Motivation

It is impossible to soundly to use tracing_subscriber::fmt::time::LocalTime or [tracing_subscriber::fmt::time::OffsetTime] in any multithreaded environment because any call to setenv will immediately make accessing any environment variable unsound via a use-after-free. On unix systems, the time crate checks whether the binary is running in an single-threaded environment. If the binary has multiple threads running, time does not return the local offset as the platform's libc will call getenv("TZ") to determine the local timezone.

Unfortunately, my employer has patched the Rust toolchain to default to jemalloc as the default, system allocator. On startup, jemalloc spins up background thread(s), which means that time will never return the local time.

Proposal

Chrono has reimplemented the timezone database file parsing https://github.com/chronotope/chrono/issues/674. This removes the need to check whether other threads are running, as time currently does. Once this feature lands, I propose that we reintroduce Chrono-specific timers under a chrono feature flag. These will be similar to the Chrono timers in tracing-subscriber 0.2. I'm not sure how to handle the tracing_subscriber::fmt::time::FormatTime trait, as it is currently tied to time's formatters, not chrono's.

If both time and chrono are activated as feature flags, then time should remain the default formatter for fmt::{Layer, Subscriber}, but this default can be revisited as part of tracing-subscriber 0.4. If time adopts a similar approach to chrono's (as tracked in https://github.com/time-rs/time/issues/193), we can evaluate what formatting is needed or required in tracing-subscriber, but this is out of scope for this issue.

Alternatives

The alternative is that this feature shouldn't be added, but this would pose a non-trivial blocker to using tracing-subscriber in certain environments.

davidbarsky avatar Apr 18 '22 17:04 davidbarsky

FYI, chrono 0.4.20 has now been released (but I assume you'd want to wait for 0.5).

djc avatar Aug 04 '22 14:08 djc

While I think I'd prefer to wait for chrono 0.5 to include chrono-based timestamp time formatters inside of tracing-subscriber (as the format strings are, unfortunately, a public API), I think this feature can be implemented in a separate crate and linked from tracing-subscriber's documentation for the the time being. It should also, hopefully, provide some useful feedback to you for any planned changes to chrono 0.5 :).

I'll need to actually implement this to confirm, but I think the format strings are the only API surface we need to expose via tracing-subscriber.

davidbarsky avatar Aug 06 '22 15:08 davidbarsky