Replace chrono with time 0.3

[vilgotf]

Feature Request

Crates

tracing-subscriber

Motivation

to reduce the amount of dependencies: chrono pulls in libc and num-integer whereas time has zero deps.

Proposal

Replace chrono with time. chrono hasn't seen a commit in 8 months whereas time is actively developed (although time has a much higher MSRV, 0.3.0 has 1.48 and 0.3.3 has 1.51).

Alternatives

humantime is another zero dep crate that could be pulled in, but it doesn't offer alternatives for ChronoLocal. ChronoLocal could be removed in an API rewrite of the time module but that's a larger effort than what I'm proposing.

[hawkw]

Hmm, we currently have a constructor for a time format that takes a chrono strftime format string and formats timestamps using that format:

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

Lines 222 to 231 in 0fa74b9

	/// Format the time using the given format string.
	///
	/// See [`chrono::format::strftime`]
	/// for details on the supported syntax.
	///
	pub fn with_format(format_string: String) -> Self {
	ChronoLocal {
	format: ChronoFmtType::Custom(format_string),
	}
	}

I see that the time also has an API for formatting timestamps with a formatting string, but it's not clear to me whether it will accept the same syntax. So, it would be nice to know if replacing that function with the time crate's alternative would be breaking or not.

Another thing that's worth noting, though, is that the chrono dependency is feature flagged. Thus, rather than replacing the existing formatters with the time crate, we could simply add a new feature flag for the time crate, and add timestamp formatting implementations that use time. This way, it would be fine if time's formatting string syntax is different. This also has the advantage that if users are already using chrono or time in their project, they can pick the dependency that's already in their dependency tree. We could add a feature for humantime as well. We would have separate APIs for each of these crates' timestamp formatting implementations that are enabled based on their various feature flags, so the fact that humantime can't implement all the same APIs as chrono wouldn't be an issue in this case.

This also has the nice advantage of avoiding having APIs with names like ChronoLocal and ChronoUtc that actually don't use the chrono crate at all, which seems potentially confusing...

The one issue is that, while chrono is feature-flagged, it is currently enabled by default:

tracing/tracing-subscriber/Cargo.toml

Line 26 in 0fa74b9

default = ["env-filter", "smallvec", "fmt", "ansi", "chrono", "tracing-log", "json"]

so, if users want to use humantime or time instead, and avoid depending on chrono, they'd have to add default-features = false to their tracing-subscriber dependency.

In the future, it would probably be better to change the default features so that no external timestamp formatting dependency is on by default, since tracing-subscriber has its own simple formatting implementation that's used when no external timestamp formatter is enabled. However, removing a default feature is a breakign change, so that would have to wait until tracing-subscriber 0.3.

What do you think of the proposal of adding time (and humantime) as separate, feature-flagged APIs, rather than replacing the implementation of the currently provided chrono formatters?

[vilgotf]

To qualify I'd like to say that I have little experience with tracing-subscribers API (having only used it in simple setups) and I wasn't even aware that tracing-subscriber had an internal (human readable)¹ time formatting implementation.

I like your proposal of disabling custom timestamp formatting by default, since most users don't need it. As time and chrono are ~even in popularity I think it makes sense to support both for the time being and users can, as you're saying, use what they're already using.

Also, having looked more into humantime I'm not sure that it's appropriate here: my understanding is that it's for predefined formats. On another note, enabling the formatting feature of time pulls in itoa so it wouldn't have zero deps :(

Footnotes

1. when I say human readable I include YYYY-MM-DD etc. (essentially everything that's not a unix timestamp) ↩

[hawkw]

Also, having looked more into humantime I'm not sure that it's appropriate here: my understanding is that it's for predefined formats. On another note, enabling the formatting feature of time pulls in itoa so it wouldn't have zero deps :(

humantime has RFC 3339 formatting, so I think it might be worth providing it as an option --- it might be the lightest-weight way to get an RFC 3339 formatter, if you don't need the other functionality provided by time or chrono?

[vilgotf]

humantime has RFC 3339 formatting, so I think it might be worth providing it as an option --- it might be the lightest-weight way to get an RFC 3339 formatter, if you don't need the other functionality provided by time or chrono?

I suppose so

[jplatte]

This just a lot more urgent because time has (had) a vulnerability that doesn't seem like it will be fixed in 0.1.x, and chrono builds on that old version. There's a PR that would bump the time version used by chrono but it's not at all clear whether it will receive maintainer attention soon-ish.

[hawkw]

This just a lot more urgent because time has (had) a vulnerability that doesn't seem like it will be fixed in 0.1.x, and chrono builds on that old version. There's a PR that would bump the time version used by chrono but it's not at all clear whether it will receive maintainer attention soon-ish.

Yeah, we will probably be publishing a v0.3 of tracing-subscriber today that replaces the chrono dependency with a dependency on time 0.2. We can re-add a chrono API in v0.3 (under a default-off feature flag) once the vulnerability is fixed.

[jplatte]

with a dependency on time 0.2.

Do you really mean 0.2, or 0.3 (latest)?

[hawkw]

with a dependency on time 0.2.

Do you really mean 0.2, or 0.3 (latest)?

Sorry, I meant 0.3.