Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arbtt-stats --dump-samples uses wrong timezone #155

Closed
martinvonwittich opened this issue May 9, 2022 · 5 comments · Fixed by #156
Closed

arbtt-stats --dump-samples uses wrong timezone #155

martinvonwittich opened this issue May 9, 2022 · 5 comments · Fixed by #156

Comments

@martinvonwittich
Copy link

I'm using the Europe/Berlin timezone which observes DST - it uses CET (UTC+1) in winter, and CEST (UTC+2) in summer.

Unfortunately, arbtt-stats --dump-samples always seems to use the current offset, and not the correct offset for the respective timestamp. E.g. this sample:

martin@martin ~ % arbtt-dump -t json | grep -m1 '2022-01-'  
{"desktop":"Arbeitsfläche 1","inactive":884,"date":"2022-01-03T22:37:16.294875171Z","windows":[],"rate":60000},

should be displayed as 2022-01-03 23:37:16, because in January the offset of Europe/Berlin was CET (UTC+1). date does it correctly:

martin@martin ~ % date -d '2022-01-03T22:37:16.294875171Z'
Mo 3. Jan 23:37:16 CET 2022

But arbtt-stats --dump-samples instead uses the offset CEST (UTC+2) because it is the offset of today:

martin@martin ~ % date
Di 10. Mai 00:03:39 CEST 2022

and therefore incorrectly displays it as 2022-01-04 00:37:16:

martin@martin ~ % arbtt-stats --dump-samples 2>/dev/null | grep -m1 -A3 '2022-01-'   
2022-01-04 00:37:16 (884ms inactive):
    Current Desktop: Arbeitsfläche 1
    Desktop:Arbeitsfläche_1
    Type:Unknown
@martinvonwittich
Copy link
Author

I believe this might be related to #82 which seems to have been the change that introduced timezone support to arbtt-stats.

@nomeata
Copy link
Owner

nomeata commented May 10, 2022

Yes, unfortunately timezone handling is insufficient in arbtt in general; I was young and would have needed more education to have done it properly from the start.

In particular, all samples are stored without a timezone (but at least, I believe, all in UTC, rather than some unknown timezone).

For this particular issue, though, I think the solution is to not use getCurrentTimeZone in

dumpHeader :: UTCTime -> Integer -> IO ()
dumpHeader time lastActivity = do
    tz <- getCurrentTimeZone
    printf "%s (%dms inactive):\n"
        (formatTime defaultTimeLocale "%F %X" (utcToLocalTime tz time))
        lastActivity

but rather use the functions from the tz package.

I’ll look into it at some point in the future, unless someone beats me to it :-)

@nomeata
Copy link
Owner

nomeata commented May 10, 2022

Ah, but you return two different issues; one with the json dump, and one with the human-readable dump.

Isn’t it the case that (currently) the JSON dump uses UTC consistently? (hence the Z in the date strings)? That’s at least not wrong, is it?

@martinvonwittich
Copy link
Author

Ah, but you return two different issues; one with the json dump, and one with the human-readable dump.

Yes, I intentionally compared the same event from both arbtt-stats --dump-samples and arbtt-dump -t json to illustrate the issue, so that I could actually prove that the timestamp from arbtt-stats --dump-samples is wrong. The JSON dump serves as a reference that isn't affected by timezone bugs because it just prints the raw UTC timestamp.

(I'm reasonably sure that it is the same event I'm comparing because it is the first event from 2022.)

Isn’t it the case that (currently) the JSON dump uses UTC consistently? (hence the Z in the date strings)? That’s at least not wrong, is it?

Yes, the JSON dump does it correctly because it doesn't bother with timezones at all, as far as I can tell.

@nomeata
Copy link
Owner

nomeata commented May 10, 2022

Ok, thanks for the clarification. I think I can fix that easily; I only very recently learned about this dark corner of the Haskell timezone handling…

nomeata added a commit that referenced this issue May 10, 2022
by using the `tz` library instead of just `time`. Fixes #155.
nomeata added a commit that referenced this issue May 10, 2022
by using the `tz` library instead of just `time`. Fixes #155.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants