Add OffsetTime formatter as an alternative to LocalTime without unsoundness

[marienz]

Feature Request

Crates

tracing-subscriber v0.3.3

Motivation

Provide a workaround for LocalTime typically requiring --cfg unsound_local_offset (as described in #1688) for programs that can accept their tracing output not following timezone changes while the program runs.

Proposal

Add an OffsetTime formatter that is initialized with a format and a timezone offset. This formats time the same way LocalTime does, but with a fixed offset. Make this easy to initialize with the local time's UTC offset. Document that this will typically fail unless called early at startup.

Even if initializing OffsetTime fails, that failure is easier to handle than it is for LocalTime: failure occurs when the formatter is initialized (typically at program startup, in application code), not when it is used (when outputting an event, in tracing code).

I've implemented a working proof of concept. It needs tests, docstring polish, and some API polish, but I've used this to confirm this approach actually works without unsound_local_offset (in a program where LocalTime did not).

I should be able to turn that code into a proper PR if this proposal is accepted.

Alternatives

• Replace LocalTime with this approach. Keeps the API simpler, but not following DST changes is not ideal.
• The status quo: require unsound_local_offset or single-threadedness. unsound_local_offset is (more or less deliberately) inconvenient to use, and enables unsound behavior. Although the circumstances under which programs are actually unsound are rare, proving they will not occur for any program is difficult, particularly if they link to any non-Rust code.
• Wait and see if the time crate comes up with a way of avoiding the unsound behavior (Better solution for getting local offset on unix time-rs/time#380). This is not trivial: the most straightforward approach involves reimplementing parsing zoneinfo files, and as far as I know their format and location are both implementation-defined. They are considering other approaches involving forking, but I don't see how those will work without then exec-ing a helper program that tells us our local timezone offset, which does not seem very practical for a low-dependency library like time.
• This proposal, but with our own mechanism for automatically updating the local offset. I don't think this is really any easier for tracing to do than it is for time, but for tokio users we might be able to implement something involving a long-running (single-threaded) helper process communicating timezone changes back to the main program. This seems like a lot of complexity (for a feature I personally do not need), but possible.

[hawkw]

This proposal seems reasonable to me, and I'd happily review a PR for it if you open one. 👍

I don't think we should remove the LocalTime formatter, since removing it would be a breaking change. I think we should probably add an OffsetTime formatter separately from LocalTime, like you suggested. If the time crate provides a better solution in the future, we can internally change the LocalTime formatter to use that, but in the meantime, I think this approach is probably the best one for now.

[marienz]

Thanks, I'll send a PR (probably this weekend).

I didn't explain the "Replace" alternative properly. I did not mean to remove LocalTime and add OffsetTime: I meant replace the implementation of LocalTime with OffsetTime, with the offset always the local offset at initialization. That's a non-breaking change as far as the API is concerned, and unlike my proposal it's a change that could be rolled back in the future without a breaking change. But I don't like the behavioral change, and I don't like that the reimplemented LocalTime would not be able to fail to initialize if it cannot get the local offset (because the current LocalTime::new returns Self, not a Result).

I think adding a separate OffsetTime is the best approach for now, but if time does manage to find a better solution OffsetTime is no longer very useful (but would have to stick around as discouraged API until the next breaking change).