samwho / spacer

CLI tool to insert spacers when command output stops
https://github.com/samwho/spacer
MIT License
853 stars 9 forks source link

Timezone for timestamps #12

Closed simonwiles closed 11 months ago

simonwiles commented 11 months ago

Thank you for this tool -- it's so useful!

Bug report/feature request:

Unless I've missed something, the timestamps produced by spacer are in UTC regardless of the timezone of the system (or, indeed, anything else)? It would be great to have them follow the system, or, even better have them be configurable (I regularly work on servers in other timezones -- the logs and output I might be interested in following all use timestamps that match the timezone of the server, as they should, but it would be awesome to be able to pipe output to spacer and see spacer's timestamps in my own local timezone).

% while true; do date; sleep 2; done | spacer
Wed Aug 30 01:54:40 PM PDT 2023
2023-08-30 20:54:41 1.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Wed Aug 30 01:54:42 PM PDT 2023
2023-08-30 20:54:43 2.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Wed Aug 30 01:54:44 PM PDT 2023
2023-08-30 20:54:45 2.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Wed Aug 30 01:54:46 PM PDT 2023
2023-08-30 20:54:47 2.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
samwho commented 11 months ago

You're welcome!

We do try to get the local timezone, then fall back to UTC if we can't: https://github.com/samwho/spacer/blob/main/src/main.rs#L94.

I can add the timezone to the date output and that'll tell us for sure what the timezone is. I'm not actually sure what situations would cause time-rs to not know the local timezone 🤔

simonwiles commented 11 months ago

Thanks :) After posting the issue I took a look at the source code and then at the rust docs and realized it was falling back to UTC, but I couldn't work out why the local timezone wasn't available to rust on any of the systems I tested on.

Then I started looking at other ways to get the local timezone (which might also support passing an arbitrary timezone as a command-line argument), but it looks like it'd have to be platform dependent. I reckon it would be possible to get something working straightforwardly enough on my Linux systems by parsing /usr/share/zoneinfo (though I'm not much of a rustacean). Presumably something similar would work for darwin, but I haven't the first idea how to approach that for Windows :)

edit: I did find this for timezone conversion: https://docs.rs/crate/tzdata/0.4.1. Doesn't address getting the local timezone, though (and yeah, it's parsing /usr/share/zoneinfo which I suppose means it doesn't work on Windows?).

samwho commented 11 months ago

I just pushed a commit to the main branch with a potential fix. Would you be able to test it? You'll need to cargo install --git https://github.com/samwho/spacer to get the latest code.

simonwiles commented 11 months ago

Thanks so much for the speedy update.

% while true; do date; sleep 2; done | spacer
Thu Aug 31 09:59:16 AM PDT 2023
2023-08-31 08:59:17 -08:00 1.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Thu Aug 31 09:59:18 AM PDT 2023
2023-08-31 08:59:19 -08:00 2.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Thu Aug 31 09:59:20 AM PDT 2023
2023-08-31 08:59:21 -08:00 2.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Thu Aug 31 09:59:22 AM PDT 2023
$ while true; do date; sleep 2; done | spacer
Thu 31 Aug 19:09:14 CEST 2023
2023-08-31 18:09:15 +01:00 1.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Thu 31 Aug 19:09:16 CEST 2023
2023-08-31 18:09:17 +01:00 2.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Thu 31 Aug 19:09:18 CEST 2023
2023-08-31 18:09:19 +01:00 2.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

It works, and it's an improvement I think, but it hasn't picked up the daylight savings on my Arch workstation or on my Debian and Centos servers. I'm not immediately sure what to make of that (apart from the observation that daylight savings time is an obstructive anachronism at this point -- but that's not really very helpful :)).

samwho commented 11 months ago

Why is time so hard.

So what I think was causing it to fall back to UTC all the time was this: https://github.com/time-rs/time/blob/18cabd12bd4258622028ffa2d9a7cb935f549b8e/time/src/sys/local_offset_at/unix.rs#L144. Seems if you have multiple threads running, it will always fall back to UTC. The comment about it being conservative wasn't lying. To get around it, I fetch the local timezone prior to spawning the background thread spacer relies on.

I have no good theories about DST, though.

samwho commented 11 months ago

I've put out https://github.com/samwho/spacer/releases/tag/v0.2 so you don't have to run from head anymore.

simonwiles commented 11 months ago

Thanks! Tbh I'm not sure if being an hour out is better or worse than just doing UTC regardless. It's confusing either way.

I have limited experience with rust, but would something like this be suitable? https://docs.rs/chrono/latest/chrono/

What do you feel about the possibility of adding the ability to specify a timezone? Would be a killer feature (for me, at least).

simonwiles commented 11 months ago

Just as a quick update to this -- what I really want for myself is something like this:

$ while true; do date; sleep 2; done | spacer --timezone America/Los_Angeles
Wed 13 Sep 21:02:40 CEST 2023
2023-09-13 12:02:41 PDT 1.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Wed 13 Sep 21:02:42 CEST 2023
2023-09-13 12:02:43 PDT 2.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Wed 13 Sep 21:02:44 CEST 2023
2023-09-13 12:02:45 PDT 2.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Wed 13 Sep 21:02:46 CEST 2023
2023-09-13 12:02:47 PDT 2.0s ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
...

I have a branch that does this and works on {Arch,Debian,Centos} Linux and Mac. I've not tried it on Windows (it does build, though, so I presume it's good), and it doesn't have proper tests (the accuracy of timestamps isn't currently tested, but test could be added to do some snapshot-style testings, perhaps -- e.g. that passing --timezone America/Los_Angeles outputs a timestamp that contains either PDT or PST etc?).

I haven't PR'd it until I know how you feel about #13 -- if you'd rather not have anything to do with this that's cool (I'll use my own fork!) -- but if this is a feature you'd be interested in having and this approach looks reasonable to you, I'd be pleased to help with anything else that needs doing to make it mergeable (code could very likely be improved -- I'm not much of a rustacean!).

samwho commented 11 months ago

@simonwiles #13 has been merged, happy to review a PR for the --timezone flag 🙂

For testing, you could add an enum variant to enum Out called SpacerWithTimezone or some such, then in the match out within fn test_output you could have an arm that asserts a timezone is present.

samwho commented 11 months ago

@simonwiles is it everything you hoped for? 🙂

simonwiles commented 11 months ago

Yes! I am an even happier spacerer :) 🎉