Opened 6 years ago

Closed 20 months ago

#3513 enhancement closed fixed (fixed)

Added microsecond to formatTime

Reported by: wangchun Owned by: exarkun
Priority: normal Milestone:
Component: core Keywords: twistd
Cc: exarkun, zseil Branch: branches/datetime-strftime-3513
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

The current formatTime implementation cannot format some time zone string correctly.

For busy servers, timestamp with microsecond is useful to administrators understand what was happened from log files.

Currently, user cannot give format string to reproduce the default format. We should make the time format more flexible and customizable by the user. Any ideas?

Attachments (2)

log.diff (3.1 KB) - added by wangchun 6 years ago.
log-tzinfo-3513.diff (14.1 KB) - added by zseil 6 years ago.
add timezone info support to LogFileObservers

Download all attachments as: .zip

Change History (17)

Changed 6 years ago by wangchun

comment:1 Changed 6 years ago by exarkun

  • Cc exarkun added

You can subclass FileLogObserver and override formatTime. That wouldn't be much harder than replacing the time format with a new string.

Most cases won't benefit from having microseconds in the log, so I don't think that data should be included by default.

Maybe time.strftime should gain a new format character for this value?

comment:2 Changed 6 years ago by wangchun

You are right, but the code would look a bit hacking, since FileLogObserver is hard coded in startLogging.

Perhaps we can find some way to do it without having to subclass the observer. There is a Formatter class in Python's logging module.

comment:3 Changed 6 years ago by exarkun

#638 is a ticket for making it possible to customize the logging configuration used by twistd. That would address the problem of FileLogObserver being hard coded in startLogging.

comment:4 Changed 6 years ago by exarkun

  • Summary changed from Fixed bug that log.py incorrectly handles UTC-00:30; Added microsecond to formatTime to Added microsecond to formatTime

I filed #3515 for the small negative tzoffset bug.

Changed 6 years ago by zseil

add timezone info support to LogFileObservers

comment:5 Changed 6 years ago by zseil

  • Cc zseil added
  • Keywords review added

datetime.datetime.strftime() in Python 2.6 accepts a new %f format character, which expands to the number of microseconds. So I used this as an excuse to convert all time manipulation in FileLogObserver to datetime module objects. The patch adds a new method getTimezoneInfo to FileLogObserver, which controls the current local time. It also always uses the datetime.strftime() method for time formating. The default time format is unchanged, but now that #3534 has landed it should be easier to change timeFormat used by twistd.

comment:6 Changed 6 years ago by glyph

  • Owner glyph deleted

comment:7 Changed 6 years ago by therve

The patch attached as little to do with the current ticket. It would be better to open a new one.

comment:8 Changed 6 years ago by zseil

  • Keywords review removed

Ok, I opened #3546.

comment:9 Changed 4 years ago by exarkun

  • Keywords twistd added

comment:10 Changed 4 years ago by <automation>

comment:11 Changed 20 months ago by exarkun

#6219 was a duplicate of this.

comment:12 Changed 20 months ago by exarkun

  • Author set to exarkun
  • Branch set to branches/datetime-strftime-3513

(In [36644]) Branching to 'datetime-strftime-3513'

comment:13 Changed 20 months ago by exarkun

  • Keywords review added

Just switched to using datetime.datetime.strftime instead of time.strftime. The former seems to be a superset of the latter, just adding %f (microseconds) and %z (timezone offset). It's probably a bit slower too.

Build results

comment:14 Changed 20 months ago by tom.prince

  • Keywords review removed
  • Owner set to exarkun

Apparently timedelta.total_seconds is only available in python 2.7:

exceptions.AttributeError: 'datetime.timedelta' object has no attribute 'total_seconds'

so that hunk should be reverted.

Also, the documentation of formatTime needs to updated to reflect the changed underlying function.

Please commit after those changes and a green buildbot run.

comment:15 Changed 20 months ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(In [36699]) Merge datetime-strftime-3513

Author: exarkun
Reviewer: tom.prince
Fixes: #3513

Switch from time.strftime to datetime.datetime.strftime for formatting
log message timestamps, adding support for microseconds and timezone offsets.

Note: See TracTickets for help on using tickets.