borntyping / rust-simple_logger

A rust logger that prints all messages with a readable output format.
https://crates.io/crates/simple_logger
MIT License
220 stars 48 forks source link

Add a timestamps_utc configuration #45

Closed uggla closed 2 years ago

uggla commented 2 years ago

Display timestamps in utc instead of local time.

This is a workaround of #44. As OffsetDateTime::now_utc() is safe, it can be used in non single-threaded program.

borntyping commented 2 years ago

Closing this with my sincere apologies - I've implemented this myself in https://github.com/borntyping/rust-simple_logger/pull/46 as I don't want to have this configured by compile time feature flags when timestamps already exists, and there's been enough issues opened on this topic that I wanted to get a release out quickly.

uggla commented 2 years ago

No worries @borntyping. I'm happy this is implemented, now I can use the new version. :+1:

A248 commented 2 years ago

@borntyping Would you be open to re-considering this as a feature flag? It adds unnecessary boilerplate to initialize with the utc_timestamps() method across many executables.

I would further suggest making UTC the default time display, so that users are not surprised by error messages arising from failed attempts to retrieve the local offset. Using time-rs's local offset feature seems fragile across different situations.

borntyping commented 2 years ago

I'm not sure I understand how a feature in Cargo.toml would be less boilerplate than the utc_timestamps() method?

Much as I'm frustrated with this entire situation, changing the default from local time to UTC time is a breaking change I'm not willing to make - it's too easy for users to miss when upgrading, and it may not be possible to identify that something has changed from logs that cover multiple invovations of a program using this.

If there was a way to make it the default for new users but not existing users I'd take it, but I can't think of any good solution here beyond waiting for Rust to stablilze a safer environment variable API and time to update to use it.