On Jan 4, 2018, at 11:07 AM, Svein Seldal <sveinse@seldal.com> wrote:
Hi
I'm currently working on understanding the twister.logger scheme with the purpose of getting proper logging in my end-user application.
https://gist.github.com/sveinse/041c69b6bbdde83cf5b00fecae3dcd50
It took me a while to get my head around the difference in using twisted.logger.globalLogBeginner.beginLoggingTo() vs using twisted.logger.globalLogPublisher.addObserver(). Neither of the methods behave precisely as I'd like them to. See https://gist.github.com/sveinse/041c69b6bbdde83cf5b00fecae3dcd50#file-log-py... <https://gist.github.com/sveinse/041c69b6bbdde83cf5b00fecae3dcd50#file-log-py...>
The comment here:
any errors in the observers will be silently ignored because the observers are no longer run through the LogPublisher()
is incorrect. If you're seeing errors being ignored, it's a bug. However, the specific claim that "the observers are no longer run through the LogPublisher" is definitely wrong. See the implementation here: https://github.com/twisted/twisted/blob/8b5865cda742b7d727f97cdec4b9e83974d5... <https://github.com/twisted/twisted/blob/8b5865cda742b7d727f97cdec4b9e83974d5...> Note in that implementation that the `self._publisher` being replayed to is precisely the LogPublisher, with all the new observers added to it. Errors should be handled as normal. Perhaps this is not happening in the correct order with the LoggingFile replacements for stdin/stdout/stderr, and should be taking place later?
1) What are the intended difference between these two ways of installing a logobserver?
beginLoggingTo is meant to be the thing that you do at program start-up to take over from the boot-time buffering log observer. addObserver is meant to be the thing that you do at any time during a program's run to add an observer for whatever other reason. While the exact difference depends upon your application, in general, if you're setting up the "main" log observer for your process, you should probably be doing it with beginLoggingTo.
2) I'd like to contribute a SyslogObserver for the new twister.logger scheme. See the _syslog.py file in the gist. It has been ported from the old logging scheme. Would this be useful to Twisted?
Absolutely!
I was uncertain what to do with log_failures in the observer. I see that dumping of log_failures has been removed from t.l.FileLogObserver(). What is the appropriate way to deal with log_failures?
Given that syslog is ultimately an unstructured / text-based format, you probably want to do something similar to what formatEventAsClassicLogText does here: https://github.com/twisted/twisted/blob/8b5865cda742b7d727f97cdec4b9e83974d5... <https://github.com/twisted/twisted/blob/8b5865cda742b7d727f97cdec4b9e83974d5...>.
In the proposal of SyslogObserver I addded a dumpTraceback option to the constructor to select if the tb should go to the syslog or not. (My application shall have tb to syslog.)
And, now that I look at your code, I can see that it's almost doing more or less exactly what I suggested with respect to log_failure, so, there you go, you're already doing it right :-). I'd probably default dumpTraceback to true - you probably do want to see tracebacks, if they're happening, and syslog is your primary log observer. Thanks for using Twisted and thanks even more for your interest in improving our log tooling! -glyph