classification
Title: syslog syscall support for SysLogLogger
Type: Stage:
Components: Library (Lib) Versions:
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: LwarX, josm, luke-jr, vinay.sajip
Priority: normal Keywords: patch

Created on 2007-05-03 00:12 by luke-jr, last changed 2009-06-22 14:56 by LwarX. This issue is now closed.

Files
File name Uploaded Description Edit
SysLogHandler-syslog-0.04.patch luke-jr, 2007-05-06 16:14 syslog support (svn trunk)
Messages (29)
msg52554 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-03 00:12
This allows SysLogLogger to be used via the normal syslog syscall.
msg52555 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-03 00:14
File Added: SysLogHandler-syslog-0.02.patch
msg52556 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-03 00:22
File Added: SysLogHandler-syslog-0.03.patch
msg52557 - (view) Author: John Smith (josm) Date: 2007-05-05 13:25
What advantage does OS's syslog have over the logging module's one?
I've never been familiar with syslog,
So could you please tell me the difference?
msg52558 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-05 17:28
Not all syslog daemons accept socket connections. For example, metalog.
msg52559 - (view) Author: John Smith (josm) Date: 2007-05-06 09:58
Thank you for your reply.

A few more comments and requests.

+    def __init__(self, address=('localhost', SYSLOG_UDP_PORT), facility=LOG_USER, ident=sys.argv[0]):

Why sys.argv[0]? Assuming command line arguments doesn't look reasonable to me.

+        self.unixsocket = 0
+	self.syscall = 0

Please use single-tab indents. (PEP7)


Can I have the patch patch against the svn repository?
(against trunk should be preferable)
I had some problem applying your patch.
msg52560 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-06 16:05
The 'ident' value is normally the name of the program, which should be argv[0]. I prefer and normally use single-tab indents. However, when patching code, I adjust to the existing standard within the source. In this case, my handlers.py was already indented with spaces so I conformed to that. I glanced at svn before posting this, and it doesn't appear to be much different for this file.
msg52561 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-06 16:14
File Added: SysLogHandler-syslog-0.04.patch
msg52562 - (view) Author: John Smith (josm) Date: 2007-05-10 12:31
Sorry, I quoted wrong PEP... PEP 7 is for C code, not Python's one.
PEP 8 is right guide.

I'm still wondering whether using sys.argv[0] for default value is valid or not.
How about changing this as ident = 'syslog'?
doesn't that work?
msg52563 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-10 15:09
No, 'syslog' wouldn't work because the running program's name is not 'syslog'. It should be the same name as appears in the process list, which is argv[0]
msg52564 - (view) Author: John Smith (josm) Date: 2007-05-10 21:52
Now I understand. Thank you for your patience :)

BTW, wouldn't it be nice to do os.path.basename() on ident?
msg52565 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-10 22:16
Yes, that might be a sane thing to add :)
That exact problem bit me a few days ago, actually (I'm using this code already).
msg52566 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2007-05-16 19:13
The use of syslog is system-specific - there is no syslog syscall on Windows.
msg52567 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-16 20:40
There is no syslog on Windows period. I guess you'd better remove chmod support from Python as well-- Windows doesn't use UNIX permissions! The whole purpose of this module is to log to syslog; the standard way for this is via a syscall. Not supporting the syscall kindof defeats the purpose.
msg52568 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2007-05-24 12:36
The whole purpose of the module is not to log to syslog - it logs to a variety of destinations. If you meant the SysLogHandler class - why exactly do you need the syslogging to be done via a syscall? You have two options already - log using a hostname/port for local or remote daemons, or use e.g. address="/dev/log" to use a Unix domain socket for access to the local syslog daemon. So I'm not sure how the purpose is defeated.

The patch as it stands will not import cleanly in Windows (since there is no "syslog" module there), so it cannot be accepted as it stands. Ordinarily I would make any small amendments myself, but at the moment I don't have the time.

You should also be aware that the syslog module may not be thread-safe on all platforms - for instance, see the first Google hit I got for the query "syslog openlog":

http://www.unet.univie.ac.at/aix/libs/basetrf2/syslog.htm

Now logging does provide an I/O lock, but that's only around the emit() call. Your syslog calls have a broader scope (handler open/close) and I'm not sure whether thread-safety could be guaranteed in all cases.

If you want to work up a patch which works across platforms and can allay my worries about multi-threading, I'll certainly look at it again.
msg52569 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-24 12:49
The purpose of the SysLogHandler is to log to the system logger. The standard method for doing this is via the syscall. UDP and UNIX sockets are *additional* methods that are not necessarily supported by all system logging daemons.

If the patch needs work, that's no reason to close it altogether.
msg52570 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2007-05-24 17:55
I didn't close the patch, I just marked it as "Pending" - not the same thing as closing, but giving an opportunity for an improved patch to be uploaded, and putting the ball in your court. And opinions differ about the "standard method" - in fact the UDP/domain socket SysLogHandler implementation was contributed by someone who was using it in a real-world scenario. The syslog call method doesn't appear to provide the ability to log to remote daemons - so the UDP/domain socket route would appear to cover all bases. Are you saying that you are not able to use UDP or Unix sockets in your environment? Can you please give more details about this environment - operating system, version etc.? What about my comment about thread safety of syslog, which you have not addressed?

Marking this item's status as Pending (i.e. ball in your court) and resolution as Rejected (indicating that the patch cannot be accepted as is due to breakage on Windows).
msg52571 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-24 18:10
Not all system loggers support sockets. In my real-world situation, this is Metalog (on Gentoo). Thread safety is a concern, but single-thread support is better than none for now.

How would I go about doing a conditional import for Windows compatibility?
msg52572 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2007-05-25 07:44
Unfortunately, while thread safety remains a concern, I won't be able to put in a patch - I don't have time to handle any additional support requests or bug reports. Since you have a patch which works for you, I suggest you stick with it. Any further updates to the module should be fairly easy for you to merge with e.g. Meld.

A conditional import for Windows compatibility would be something like this:

try:
    import syslog # Will fail where unavailable, e.g. Windows
except ImportError:
    syslog = None

and then, later, in the relevant code:

    if syslog is None:
        # Code avoiding use of syslog module
    else:
        # Code using syslog module

msg52573 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-25 10:35
What logic allows you to reject/postpone this bugfix, but not reject/postpone other patchless bug reports to fix more specifics? Better to have this work as non-thread-safe than not at all, even if the eventual ideal is thread-safe. I find it hard to believe that thread-safety is consistently present throughout the rest of the Python library.
msg52574 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2007-05-25 13:45
It's not a bugfix - it's an enhancement, which no one else has asked for. Given that there's a possible thread safety issue, I see no strong reason to commit a patch to call syslog. After all, you have a patch which works for you; and there is no strong demand in the community for a patch to use syslog. Keep using your patch, where's the problem with that?

I look at bug reports and patches and fix/apply after assessing whether or not the change is an improvement. I don't accept all patches or bug reports as valid - but I don't reject things out of hand, either. This particular patch might make things worse if thread safefy issues aren't thought through properly - then it would be me, not you, who would have to take on any additional support burden. While I can't speak for the Python library as a whole, I have certainly taken a fair amount of trouble to make the logging package thread-safe.

Take a look at the patches list for Python - there are quite a lot of patches which aren't accepted and haven't been accepted for a long time because that's the committer's judgement. Some may get accepted in the future, others will not.
msg52575 - (view) Author: Luke-Jr (luke-jr) Date: 2007-05-25 15:10
This would be the equivalent to having a EmailLogger that supports XMPP and IMAP, but not SMTP. I don't see how you can argue it as not being a bug.

I have a patch that works, yes, but to make use of it, I need to patch Python every time I upgrade, as would all my users. Seeing as we are all users of the Python language, not developers of it, manual patching should *not* be necessary. The SyslogLogger could easily contain a documented note that syscall support may not be thread-safe. Note that in practice, I haven't had any problems-- the application I am writing is highly threaded.
msg56157 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2007-09-27 06:19
It's only a bug when it doesn't work according to design. The present
design seems adequate in that it allows syslogging via UDP or domain
sockets. No one else has asked for the functionality of using system
calls. BTW I note that Metalog's home page says it's a modern
replacement for syslogd and klogd - so one might have reasonably
expected socket support.

You can avoid having to patch Python each time by the simple expedient
of creating a SysLogHandler subclass (a one-time operation) and using it
in place of the included SysLogHandler.
msg56160 - (view) Author: Luke-Jr (luke-jr) Date: 2007-09-27 06:52
So label it a "design flaw" if not a bug. Syscalls are the primary and 
only guaranteed method of writing to the system log. Very few 
applications or users use sockets for syslog, and socket support 
should only be required when logging to a remote system.

Even if this is treated as a 'feature' (which it clearly is more 
than), it should still be merged into the current development branch.
msg89594 - (view) Author: Max Arnold (LwarX) Date: 2009-06-22 07:33
Can I vote for this issue?  Many systems with syslog aren't configured
to listen on UDP socket and thus out of the box SysLogHandler does not work.
msg89595 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2009-06-22 08:22
As the docstring and documentation says, you can use
SysLogHandler("/dev/log") or similar to connect to a local syslog using
Unix domain sockets rather than UDP. Doesn't this work for you?
msg89598 - (view) Author: Max Arnold (LwarX) Date: 2009-06-22 14:10
Is it safe to use single handler instance in multiple loggers or single
stream in multiple handlers?
msg89600 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2009-06-22 14:42
Why would you want to use a single handler instance against multiple
loggers? It's safe to do so, but you could get duplicated messages
appearing. I presume you have reviewed the documentation and are aware
that loggers are organised in a hierarchy and that in the normal case,
handlers of all parent loggers are allowed to handle events logged with
a particular logger.

What do you mean by "single stream in multiple handlers"? In general
this could result in garbled output, if you have multiple threads in
your environment.

Are these questions relevant to this SysLogHandler issue? I couldn't see
a connection with your earlier comment. If not relevant, please post
them on comp.lang.python where you will probably get more people looking
at them, so that the quality of answers is likely to be more helpful to you.
msg89601 - (view) Author: Max Arnold (LwarX) Date: 2009-06-22 14:56
Sorry, I've read your first reply too fast and incorrectly interpreted
it as recommendation to use stream handler with /dev/log.

Anyway, thank you for clarification.
History
Date User Action Args
2009-06-22 14:56:55LwarXsetmessages: + msg89601
2009-06-22 14:42:30vinay.sajipsetmessages: + msg89600
2009-06-22 14:10:10LwarXsetmessages: + msg89598
2009-06-22 08:22:28vinay.sajipsetmessages: + msg89595
2009-06-22 07:33:09LwarXsetnosy: + LwarX
messages: + msg89594
2009-05-20 18:45:47dandrzejewskisettitle: syslog syscall support for SysLogLogger -> logging -> syslog syscall support for SysLogLogger
2009-05-20 18:45:33dandrzejewskisettitle: logging -> syslog syscall support for SysLogLogger -> logging
versions: - Python 2.5
2009-05-20 18:44:39dandrzejewskisettitle: syslog syscall support for SysLogLogger -> logging
versions: + Python 2.5
2007-09-27 06:52:25luke-jrsetmessages: + msg56160
2007-09-27 06:19:25vinay.sajipsetstatus: open -> closed
messages: + msg56157
2007-05-03 00:12:14luke-jrcreate