Add exception logging function to syslog module

[ericvsmith]

BPO	8214
Nosy	@pitrou, @ericvsmith, @benjaminp, @merwok, @briancurtin
Files	logexception.diff: Implementation of first function, to log exception information. logexception2.patch: Full implementation for Python 2. logexception3.patch: Patch with changes Jack suggested. logexception4.patch: Test fix suggested by Brian. logexception5.patch: Test fix suggested by Bejnamin.

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2010-03-23.22:12:11.813>
labels = ['extension-modules', 'type-feature']
title = 'Add exception logging function to syslog module'
updated_at = <Date 2019-03-15.23:56:07.284>
user = 'https://github.com/ericvsmith'

bugs.python.org fields:

activity = <Date 2019-03-15.23:56:07.284>
actor = 'BreamoreBoy'
assignee = 'jafo'
closed = False
closed_date = None
closer = None
components = ['Extension Modules']
creation = <Date 2010-03-23.22:12:11.813>
creator = 'eric.smith'
dependencies = []
files = ['17089', '17159', '17174', '17177', '17179']
hgrepos = []
issue_num = 8214
keywords = ['patch', 'needs review']
message_count = 20.0
messages = ['101605', '101668', '101681', '101682', '101683', '101685', '101686', '101687', '104211', '104693', '104746', '104754', '104755', '104756', '104757', '104759', '104761', '104762', '105181', '221669']
nosy_count = 6.0
nosy_names = ['jafo', 'pitrou', 'eric.smith', 'benjamin.peterson', 'eric.araujo', 'brian.curtin']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue8214'
versions = ['Python 3.5']

[ericvsmith]

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

[vstinner]

syslog_exception() should be declared outside logexceptions(), so it's possible to call it directly. Example:

try:
...
except Exception:
syslog_exception(*sys.exc_info())

[ericvsmith]

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.

[vstinner]

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?

[ericvsmith]

> 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.

[vstinner]

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.

[ericvsmith]

Ah, I see. Yes, logexceptions needs a better name. Maybe enable_exception_logging.

[ericvsmith]

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

[jafo]

I believe I have the first function implemented. See the attached patch for that code and feel free to review.

[jafo]

I have completed the exception handling code as prototyped in msg101687. Please review.

[jafo]

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.

[pitrou]

Rewriting another implementation of traceback formatting in C is bad.
Just import the "traceback" module and call the desired function in that module.