Opened 12 years ago

Last modified 10 years ago

#3363 enhancement reopened

twistd should support external log rotation facilities

Reported by: radix Owned by:
Priority: normal Milestone:
Component: core Keywords: twistd
Cc: Thijs Triemstra Branch:
Author:

Description

This basically means that it should be possible to get twistd to handle a signal to re-open its logfiles.

Attachments (1)

ticket-3363.patch (4.4 KB) - added by henninge 10 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 11 years ago by Glyph

Owner: changed from Glyph to radix

comment:2 Changed 10 years ago by Jason J. W. Williams

Isn't this implemented already? Issuing SIGUSR1 forces a reload?

comment:3 Changed 10 years ago by Jean-Paul Calderone

Resolution: invalid
Status: newclosed

Yes.

comment:4 Changed 10 years ago by Jean-Paul Calderone

Resolution: invalid
Status: closedreopened

Actually, as it turns out, no. SIGUSR1 forces a rotation, not a re-open. This is not quite the same.

comment:5 Changed 10 years ago by Jean-Paul Calderone

#4557 was a duplicate of this.

Changed 10 years ago by henninge

Attachment: ticket-3363.patch added

comment:6 Changed 10 years ago by henninge

Keywords: review added

comment:7 Changed 10 years ago by henninge

The patch I submitted adds an "--nologrotation" option that deactivates log rotation and makes twistd reopen its log file on SIGUSR1 instead. Without this option the old behavior is retained.

comment:8 Changed 10 years ago by Jean-Paul Calderone

Keywords: twistd added

comment:9 Changed 10 years ago by Jean-Paul Calderone

There was some discussion of this issue on IRC. One factor to consider is the proliferation of logging configuration options accepted by twistd. So far, we've been avoiding introducing new options because we have no general plan for twistd. I'd personally like to avoid having ten (or twenty or fifty) command line flags just for configuring LogFileObserver. Not just because this kind of mess makes it hard for users to find what they want, but also because LogFileObserver is just one possible observer. It would be great if we could come up with a plan which allowed arbitrary configuration of a (or several) log observer without having to extend twistd for every option.

I'm not sure if I should leave this ticket in review or not, so I'm going to for now. It's possible that in a few days I'll come back and say something more. :)

Also, I went through and marked all the twistd-related issues that I could find with the "twistd" keyword, so you can now look at them all with this easy query:

http://twistedmatrix.com/trac/query?status=assigned&status=new&status=reopened&order=priority&col=id&col=summary&col=status&col=type&col=priority&col=milestone&col=component&keywords=~twistd

comment:10 Changed 10 years ago by Glyph

I think for now --nologrotation may be the way to go for now, since it is a small fix. The point about a long-term plan is well-taken though.

Two other areas which have faced similar option proliferation are ICredentialsChecker construction, for which we now have strcred, and IListeningPort construction, for which we now have strports (and soon, I hope, "strpoints").

The general pattern of the solution is simple: instead of --foo-a --foo-b --foo-c, we have a (pluggable) --thunk foo:a,b,c option. strports and endpoints are both fairly simple, and we could probably cram most of a log configuration system into something similar.

If we did this for logging, not only could we support different loggers, we could also support multiple loggers of the same type.

This is the basic idea behind the way the 'logging' module configures itself; you can set up any number of loggers and configure them in various ways. Unfortunately loggers are probably going to push the boundaries of what's comfortable to type on the command line. Take, for example, the "simple" example logging.conf file in the "configuring logging" section of the logging module's documentation. Of course, we could address this with --log-observer logging:/path/to/my/logging.conf.

As a general high-level plan for all of these systems that take command-line options to construct something, we might want to consider having a 'python' plugin which execfiles a Python source file and pulls a variable out of it (or maybe even just evaluates a Python statement provided directly on the command line, for simple scripts) to satisfy whatever interface is required. This would serve a dual purpose. First, it would give you a way to do quick-and-dirty customization via something like a .tac file, but for small portions of the twistd application service structure instead of needing to build the whole thing yourself. Second, it would provide a straightforward way to integrate things for which there are too many options to stick into a single command-line string in a consistent way.

To sum up, this particular use-case would be addressed with --log-observer simple:/path/to/my/log. The behavior we've got right now could be requested with --log-observer rotating:/path/to/my/log. Each of these would still want to grab SIGUSR1, so we may want to add a cooperative system for listening to that signal, or for requesting a signal to listen to, and then logging the allocation of each signal ("send SIGUSR1 to rotate the log at ...", "send SIGUSR2 to close the log at ..."). Even POSIX guarantees us up to 8 signals between SIGRTMIN and SIGRTMAX.

Should we file a new ticket for doing this for logging?

comment:11 Changed 10 years ago by khorn

Would it be possible to have something like apache's -D flag, where you have one option that can be used multiple times and that takes key/value pairs?

so if -x is the "override logging flag", you might do:

-xoption1=value1 -xoption2=value2

or something like that.

Something like that could even be more general than just logging...you could pass these values to the .tac file or plugin or whatever and let it decide what to do.

I don't know whether this is a good idea, mind you...

comment:12 Changed 10 years ago by henninge

I solved this for now by defining my own log observer and applying it in the tac file. Luckily twistd checks for a component in the application that provides ILogObserver. http://paste.ubuntu.com/464982/

comment:13 Changed 10 years ago by Thijs Triemstra

Cc: Thijs Triemstra added

Many comments since the keyword review was added, should this ticket still be in review or..

comment:14 Changed 10 years ago by henninge

Keywords: review removed

No, I think not.

comment:15 Changed 9 years ago by <automation>

Owner: radix deleted
Note: See TracTickets for help on using tickets.