DST calculation problem in log.py/FileLogObserver.getTimezoneOffset

[thor]

getTimezoneOffset uses Python's "time.daylight" to determine whether or not localtime currently needs to be adjusted by DST rules, but time.daylight indicates just that the local time zone uses DST at all, not that it should be used at the moment, thus getTimezoneOffset always returns the DST offset even in winter (i.e. now in the northern hemisphere) resulting in a local time that is off by 1 hour for me at the moment (CEST instead of CET is used).

Cf.  http://docs.python.org/lib/module-time.html , the Python src code (Modules/timemodule.c) and the following from log.py

def getTimezoneOffset(self):

""" Return the current local timezone offset from UTC.

@rtype: C{int} @return: The number of seconds offset from UTC. West is positive, east is negative. """

if time.daylight:

return time.altzone

return time.timezone

[exarkun]

Added a test and a fix in logging-timezone-2404

Someone please review!

[exarkun]

It occurs to me that the code in the branch does not explicitly account for the possibility of tm_isdst being -1. I don't know when it takes on this value, apparently never on Linux with glibc 2.3.6 in EST/EDT. Maybe other platforms/timezones have it, though. It's not really clear what can be done differently to handle this case. Maybe we should just log in UTC always.

[zseil]

There's been a recent bug report with a similar content in the Python bugtracker:  http://www.python.org/sf/1647654

The reporter suggests the following solution:

def getTimezoneOffset(when):
    offset = datetime.fromtimestamp(when) - datetime.utcfromtimestamp(when)
    return offset.days * 86400 + offset.seconds

[therve]

What do you think ? It seems a bit cleaner than the current solution ?

[exarkun]

It does push some logic out into a lower level library. It's also more correct (although maybe not in any real-life situation). Unfortunately, it's harder to test. In fact, I can't think of any correct way to test any implementation of this method.

I think I'm now convinced that the default log observer should log in UTC, not local time.

[jknight]

To test: Set the TZ env var to something well-known, call time.tzset(), check results, put back TZ and call tzset() again.

[exarkun]

Hoorj. Okay, thanks. :)

utcfromtimestamp/fromtimestamp/tzset based implementation in the branch now.

[thor]

I don't think logging that time in UTC is such a great idea, as I guess most people interested in the actual value of that timestamp would want to know that time in relation to their normal real life timescales or in relation to other log files, which are mostly in local time as well. Sure you can adjust that time in your head, but who really wants to do that every time..

Having to customize the logger in every twistd 10 liner just to be able to read the time when you're still half asleep is cumbersome....

[itamarst]

calendar.timegm might be useful based on one of the comments you made in the test.

thor: I'm pretty sure log display will not be in UTC, but exarkun can correct me.

[exarkun]

It does look like timegm would be useful in replacing the mktime() line, but since that was there already I'm not going to touch it here.

Indeed, the log display will still be in local time. Since James pointed out a way to actually test this, I preserved the existing behavior. I would still probably prefer UTC, but I'm not actually going to implement that until I have a more compelling reason to.

[therve]

Sorry, but tzset is only available on Unix.

[thor]

OK, sorry about that then, I seem to have missed the "+ tzOffset" bit in line 325

[jknight]

+1 for test.skip = "STBWindows" on Windows.

[exarkun]

Okay, skipped if tzset is missing.

[therve]

Good :). Please merge.

[exarkun]

(In [19561]) Merge logging-timezone-2404

Author: exarkun Reviewer: therve Fixes #2404

Correct FileLogObserver.getTimezoneOffset to properly handle the difference between daylight saving and standard time, by delegating to a lower level (something inside the datetime type) rather than trying to determine whether daylight saving time is on effect or not itself.