classification
Title: Add exception logging function to syslog module
Type: enhancement Stage: patch review
Components: Extension Modules Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: jafo Nosy List: BreamoreBoy, benjamin.peterson, brian.curtin, eric.araujo, eric.smith, jafo, pitrou
Priority: normal Keywords: needs review, patch

Created on 2010-03-23 22:12 by eric.smith, last changed 2014-06-27 00:19 by BreamoreBoy.

Files
File name Uploaded Description Edit
logexception.diff jafo, 2010-04-26 10:38 Implementation of first function, to log exception information.
logexception2.patch jafo, 2010-05-01 06:14 Full implementation for Python 2.
logexception3.patch jafo, 2010-05-01 22:40 Patch with changes Jack suggested.
logexception4.patch jafo, 2010-05-02 00:41 Test fix suggested by Brian.
logexception5.patch jafo, 2010-05-02 01:40 Test fix suggested by Bejnamin.
Messages (20)
msg101605 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-03-23 22:12
Sean Reifschneider proposed [1] adding the ability to log an exception using the syslog module.

My proposed implementation is along the lines of:

def logexceptions(chain=True):
    import sys
    import traceback
    import syslog

    # Should we chain to the existing sys.excepthook?
    current_hook = sys.excepthook if chain else None

    def syslog_exception(etype, evalue, etb):
        if current_hook:
            current_hook(etype, evalue, etb)
        # The result of traceback.format_exception might contain
        # embedded newlines, so we have the nested loops.
        for line in traceback.format_exception(etype, evalue, etb):
            for line in line.rstrip().split('\n'):
                syslog.syslog(line)
    sys.excepthook = syslog_exception

Although it would need to be written in C to work in the existing syslog module, and of course it would call syslog.syslog directly.


[1] http://mail.python.org/pipermail/python-ideas/2010-March/006927.html
msg101668 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-25 01:58
syslog_exception() should be declared outside logexceptions(), so it's possible to call it directly. Example:

try:
  ...
except Exception:
  syslog_exception(*sys.exc_info())
msg101681 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-03-25 09:17
Good point. So that makes the implementation more like:

import traceback
import syslog
import sys

def syslog_exception(etype, evalue, etb):
    # The result of traceback.format_exception might contain
    # embedded newlines, so we have the nested loops.
    for line in traceback.format_exception(etype, evalue, etb):
        for line in line.rstrip().split('\n'):
            syslog.syslog(line)

def logexceptions(chain=True):
    # Should we chain to the existing sys.excepthook?
    current_hook = sys.excepthook if chain else None

    def hook(etype, evalue, etb):
        if current_hook:
            current_hook(etype, evalue, etb)
        syslog_exception(etype, evalue, etb)
    sys.excepthook = hook

We could then make syslog_exception take a tuple instead of the 3 values. I'm not sure which is more convenient, or more widely used in other APIs.

Once it's moved into syslog, maybe syslog_exception named syslog.log_exception.
msg101682 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-25 09:26
> Once it's moved into syslog, maybe syslog_exception named syslog.log_exception.

In that case, how would be called the second function? Can write a patch with an unit test?
msg101683 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-03-25 09:30
>> Once it's moved into syslog, maybe syslog_exception named syslog.log_exception.

That's "should be named", of course. Although on second thought maybe syslog.syslog_exception really is the right name, to mirror syslog.syslog.

> In that case, how would be called the second function? Can write a patch with an unit test?

I'm not sure which second function you mean.

As far as the implementation, Sean's working on it, although I'm willing to help.
msg101685 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-25 09:35
> I'm not sure which second function you mean.

"logexceptions" which replaces sys.excepthook. "log_exception" and "log_exceptions" are very close.

cgitb has a function "enable" which sets sys.excepthook.
msg101686 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-03-25 09:41
Ah, I see. Yes, logexceptions needs a better name. Maybe enable_exception_logging.
msg101687 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2010-03-25 09:56
Here's a version that would be more analogous to what the C implementation would look like. It uses a class instead of a closure to capture the "chain" value. The 2 exposed functions syslog_exception and enable_exception_logging are the new APIs in this proposal, which would be written in C as part of the syslog module. The class is a hidden implementation detail.

########################
import traceback
import syslog
import sys

def syslog_exception(etype, evalue, etb):
    # The result of traceback.format_exception might contain
    # embedded newlines, so we have the nested loops.
    for line in traceback.format_exception(etype, evalue, etb):
        for line in line.rstrip().split('\n'):
            syslog.syslog(line)

class _Hook(object):
    def __init__(self, chain):
        # Should we chain to the existing sys.excepthook?
        self._current_hook = sys.excepthook if chain else None

    def _hook(self, etype, evalue, etb):
        if self._current_hook:
            self._current_hook(etype, evalue, etb)
        syslog_exception(etype, evalue, etb)

def enable_exception_logging(chain=True):
    sys.excepthook = _Hook(chain)._hook
msg104211 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2010-04-26 10:38
I believe I have the first function implemented.  See the attached patch for that code and feel free to review.
msg104693 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2010-05-01 06:14
I have completed the exception handling code as prototyped in msg101687.  Please review.
msg104746 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2010-05-01 20:19
Jack Diederich commented:

I don't have my tracker login on this computer so I'll post here.
I'd +1 on making the module python with just the core functionality
imported from C (it releases the GIL when doing IO).  Then you
could replace the few hundred lines of C with just the few lines of
python from the prototype.  That said...    The parens in "return
(NULL)" are extra and against PEP 7 (though there are already a
bunch in syslogmodule.c).    You need to NULL check the saved_hook
in newhookobject() before INCREF'ing it.    Should the saved hook
be called after the syslog call?  It might do anything.    The
patch needs unit tests.
msg104754 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-01 22:04
Rewriting another implementation of traceback formatting in C is bad.
Just import the "traceback" module and call the desired function in that module.
msg104755 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-01 22:05
Oh, well, sorry. That's what you are already doing. Forget me :)
msg104756 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2010-05-01 22:40
Thanks for the review Jack.

I was very tempted to split it into C and Python components, but I decided
against it because it's so close to the 2.7 release.  I think it would be
best to defer that for the Python 3 release, because of potential packaging
issues.  I'm open to discussion on that though.

I've changed all the return(NULL)s in the package.

NULL check on saved_hook is done.

I also had the same thought about the saved_hook/syslog ordering, so I've
changed it.

I've added soe unit tests.  I tried getting fancy and testing the exception
handling, but had to fork to do it and then unittests were still catching
the exception, so I just left it the minimal set of tests I put in there.

Thanks.
msg104757 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-05-01 23:40
The test file should use test_support.import_module("syslog") instead of the try/except, which would then make the skip decorators unnecessary.
msg104759 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2010-05-02 00:41
Changed to use import_module.
msg104761 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2010-05-02 01:15
You shouln't use fail*. They're deprecated.
msg104762 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2010-05-02 01:40
Switched to using assertTrue/assertFalse.
msg105181 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2010-05-07 04:28
This won't go into 2.7.0.  I will do a patch for 3.x, and wait for 2.7.0 to see if it can go into 2.7.1.
msg221669 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-27 00:19
@Sean is this something you can pick up again, it seems a shame to let your past efforts gather dust?
History
Date User Action Args
2014-06-27 00:19:53BreamoreBoysetnosy: + BreamoreBoy

messages: + msg221669
versions: + Python 3.5, - Python 3.2
2010-11-28 05:08:03eric.araujosetnosy: + eric.araujo
2010-05-07 04:29:46jafosetmessages: - msg105182
2010-05-07 04:29:14jafosetmessages: + msg105182
2010-05-07 04:28:58jafosetmessages: + msg105181
2010-05-02 01:40:57jafosetfiles: + logexception5.patch

messages: + msg104762
2010-05-02 01:15:58benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg104761
2010-05-02 00:41:14jafosetfiles: + logexception4.patch

messages: + msg104759
2010-05-01 23:40:48brian.curtinsetnosy: + brian.curtin
messages: + msg104757
2010-05-01 22:40:25jafosetfiles: + logexception3.patch

messages: + msg104756
2010-05-01 22:05:18pitrousetmessages: + msg104755
2010-05-01 22:04:11pitrousetnosy: + pitrou

messages: + msg104754
versions: - Python 2.7
2010-05-01 20:19:07jafosetmessages: + msg104746
2010-05-01 06:14:51jafosetfiles: + logexception2.patch
messages: + msg104693

assignee: jafo
keywords: + needs review
stage: needs patch -> patch review
2010-04-26 10:38:38jafosetfiles: + logexception.diff

nosy: - haypo
messages: + msg104211

keywords: + patch
2010-03-25 09:56:06eric.smithsetmessages: + msg101687
2010-03-25 09:41:38eric.smithsetmessages: + msg101686
2010-03-25 09:35:35hayposetmessages: + msg101685
2010-03-25 09:30:25eric.smithsetmessages: + msg101683
2010-03-25 09:26:15hayposetmessages: + msg101682
2010-03-25 09:17:48eric.smithsetmessages: + msg101681
2010-03-25 01:58:26hayposetnosy: + haypo
messages: + msg101668
2010-03-23 22:13:14eric.smithsetnosy: jafo, eric.smith
components: + Extension Modules
2010-03-23 22:12:11eric.smithcreate