Skip to content

Conversation

@fcuny
Copy link

@fcuny fcuny commented Nov 13, 2012

When the options stdout_logfile or stderr_logfile are set to
"syslog", the children are logging to the syslog service,
but the daemon is still logging to a file.

By letting the possibility to set the value for logfile to
"syslog", we can have the same behavior for the daemon.

When the options ``stdout_logfile`` or ``stderr_logfile`` are set to
"syslog", the children are logging to the syslog service,
but the daemon is still logging to a file.

By letting the possibility to set the value for ``logfile`` to
"syslog", we can have the same behavior for the daemon.
@christopherhesse
Copy link

This seems pretty useful to me, and simple too. I'm not sure why you'd want all children to log to syslog but not the daemon. Any reason it's not merged?

@timbunce
Copy link

+1

@fredrik
Copy link

fredrik commented Apr 4, 2013

+1

I'd love to see this feature on master.

@fbacchella
Copy link

But a little more configuration is needed, as the logname are not very usable:
May 6 16:25:48 hostname python: 2013-05-06 16:25:48,503 INFO supervisord started with pid 27296

So the formatter needs to be changed too.

@fbacchella
Copy link

A patch to full syslog support in supervisord, with better syslog formatting:

diff -u  supervisor-3.0b1/supervisor/options.py.syslog supervisor-3.0b1/supervisor/options.py 
--- supervisor-3.0b1/supervisor/options.py.syslog       2012-08-23 03:45:12.000000000 +0200
+++ supervisor-3.0b1/supervisor/options.py      2013-05-06 19:21:17.396521369 +0200
@@ -447,7 +447,8 @@
         else:
             logfile = section.logfile

-        self.logfile = normalize_path(logfile)
+        if logfile != 'syslog':
+            self.logfile = normalize_path(logfile)

         if self.pidfile:
             pidfile = self.pidfile
diff -u  supervisor-3.0b1/supervisor/loggers.py.syslog supervisor-3.0b1/supervisor/loggers.py 
--- supervisor-3.0b1/supervisor/loggers.py.syslog       2012-08-23 03:45:12.000000000 +0200
+++ supervisor-3.0b1/supervisor/loggers.py      2013-05-06 20:26:24.187266010 +0200
@@ -324,26 +324,32 @@
     def getvalue(self):
         raise NotImplementedError

+level_to_syslog = { LevelsByName.CRIT: syslog.LOG_CRIT,
+                    LevelsByName.ERRO: syslog.LOG_ERR,
+                    LevelsByName.WARN: syslog.LOG_WARNING,
+                    LevelsByName.INFO: syslog.LOG_NOTICE,
+                    LevelsByName.DEBG: syslog.LOG_DEBUG,
+                  }
+ 
 class SyslogHandler(Handler):
     def __init__(self):
         assert 'syslog' in globals(), "Syslog module not present"
+        syslog.openlog('supervisord',syslog.LOG_PID)

     def close(self):
         pass

     def emit(self, record):
-        try:
-            params = record.asdict()
-            message = params['message']
-            for line in message.rstrip('\n').split('\n'):
-                params['message'] = line
-                msg = self.fmt % params
-                try:
-                    syslog.syslog(msg)
-                except UnicodeError:
-                    syslog.syslog(msg.encode("UTF-8"))
-        except:
-            self.handleError(record)
+        if record.level >= LevelsByName.DEBG:
+            try:
+                priority = level_to_syslog[record.level]  | syslog.LOG_DAEMON
+                for line in record.msg.rstrip('\n').split('\n'):
+                    try:
+                        syslog.syslog(priority, line)
+                    except UnicodeError:
+                        syslog.syslog(priority, line.encode("UTF-8"))
+            except:
+                self.handleError(record)

 def getLogger(filename, level, fmt, rotating=False, maxbytes=0, backups=0,
               stdout=False):

@olliebun
Copy link

Is it possible to get this into master? This would be a very useful feature.

@glarrain
Copy link

glarrain commented Aug 6, 2013

As a workaround, if you use rsyslog, from a certain version it is capable of monitoring log files. I set that up to send my logs to Loggly through rsyslog

@samv
Copy link

samv commented Sep 18, 2013

Hey folks, I've knocked together patches which enhance the support and also pass tests. If people want to go have a look at the other pull request (#289) and give feedback that would be lovely! In particular, the way stdout and stderr streams are logged is something I'd be curious to hear people's comments on. Do you think each stream should be sent with a unique tag? Or is an in-message prefix as currently written sufficient?

@Siecje
Copy link

Siecje commented Oct 25, 2015

I'm curious why this has not been accepted. It is a minor change and a desired feature.

@mnaberez
Copy link
Member

@Siecje There are a number of issues and patches tagged syslog with overlap between them. They need to be reviewed together.

@Siecje
Copy link

Siecje commented Oct 25, 2015

@mnaberez There doesn't seem to be another pull request with this feature. This has sat for 3 years and the functionality is already there for programs just not supervisord itself.

@mnaberez
Copy link
Member

@Siecje There's an alternate one right above and I've seen this in at least one other patch that includes this if not more. The syslog patches probably need to be reviewed together.

@samv
Copy link

samv commented Oct 25, 2015

Sure, here's your summary. I'm including it on this one because it's the oldest with that tag, and this is where the discussion is happening.

This one: #162 - created by an ex-colleague of mine, @franckcuny - is where I started with my own PR.
The included patch above as well as #286, #334 and #563 I believe all deliver features included in my PR: mostly relating to configuring the syslog tag (application name), as well as syslog facility and priority.

#446 is genuinely different and relates to remote syslog logging. This is especially important for docker users who are using supervisor as the top-level process in a container, because they don't have a 'syslog' daemon running locally. If you merge this PR, I'll submit a new one which cleans this up and allows it. Personally I think to be consistent you should either support syslog properly in core, or clearly deprecate it and instead point people to the plug-in.

@mnaberez
Copy link
Member

This one: #162 - created by an ex-colleague of mine, @franckcuny - is where I started with my own PR. The included patch above as well as #286, #334 and #563 I believe all deliver features included in my PR: mostly relating to configuring the syslog tag (application name), as well as syslog facility and priority.

Note for future review: "my own PR" that @samv references above is #289.

@jaraco
Copy link
Contributor

jaraco commented Jan 5, 2016

Also, don't forget #241, which supersedes #162 and was accepted and sits in master but has never been released (and whose configuration options now suggest that 4.0.0 will include the changes). #241 addresses the shortcoming of #162 indicated in #182.

@jaraco
Copy link
Contributor

jaraco commented Jan 5, 2016

My recommendation - close #162, (optionally) release 4.0.0 with the changes in master, accept #289 which fixes #286, #334, and #563, and release that as a backward-incompatible release (4.0 or 5.0). Address #446 subsequently.

The important thing here is to make releases on which people may rely and iterate and refine. Instead, we have lots of private forks that organizations are running because the functionality has not been released.

@mnaberez
Copy link
Member

mnaberez commented Jan 5, 2016

(optionally) release 4.0.0 with the changes in master,

It is unfortunate that the syslog changes are tied up with the changes on master. The master branch also has Python 3 support which is pretty broken right now (see issues tagged python 3) and not suitable to be released. An alternative might be to make a new 3.x release with the syslog changes.

@jaraco
Copy link
Contributor

jaraco commented Feb 1, 2016

@mnaberez: A quick browse through those issues, some of them are specific to Python 3 (which is currently unsupported, so a release note stating that Python 3 is still not perfect should suffice), but others seem to be regressions brought about by the attempt at Python 3 support. I suggest we bisect those two failure modes and address the latter, bringing the master back to a stable state independent of supporting Python 3. I'd flag those issues as 'blocker' or similar.

@mnaberez
Copy link
Member

mnaberez commented Feb 1, 2016

so a release note stating that Python 3 is still not perfect should suffice

I'm not willing to release version of Supervisor that I know will crash hard with something as simple as a subprocess outputting a byte that is not valid UTF-8. That's far from an edge case. Companies run this software in production and don't always read all the details in the release notes.

@Siecje
Copy link

Siecje commented Feb 1, 2016

Merging this pull request is backward compatible.

@jaraco
Copy link
Contributor

jaraco commented Feb 1, 2016

That's far from an edge case. Companies run this software in production and don't always read all the details in the release notes.

Fair enough. So hard fail when sys.version_info > (3,) but let's not let the Python 3 support be an unnecessarily high barrier to stability of master.

@jerryhebert
Copy link

Hey folks, is there any update on this and all of the related syslog PRs? I'm trying to understand the best approach to output to syslog with my own preferred local facility but that functionality seems to have been locked up in one PR or another for several years now. I am not sure how to proceed, but it certainly seems like making yet another PR to add this functionality would not help matters.

Any advice would be appreciated, thanks!

@mnaberez mnaberez mentioned this pull request Jun 21, 2017
@xiva-wgt
Copy link

+1

@xiva-wgt
Copy link

Docker container set log to nsyslog on host. Only supervisord write log files into container.
Need log to syslog without filehandler.

@nordnotor
Copy link

+1

@mnaberez mnaberez merged commit fe00b32 into Supervisor:master Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.