Merge ~epics-core/epics-base/+git/Com:errlog2syslog into ~epics-core/epics-base/+git/epics-base:7.0

Proposed by mdavidsaver
Status: Needs review
Proposed branch: ~epics-core/epics-base/+git/Com:errlog2syslog
Merge into: ~epics-core/epics-base/+git/epics-base:7.0
Diff against target: 174 lines (+77/-6)
7 files modified
configure/CONFIG_ENV (+4/-0)
modules/database/src/ioc/misc/iocInit.c (+2/-0)
modules/libcom/src/env/envDefs.h (+1/-0)
modules/libcom/src/error/errlog.h (+8/-0)
modules/libcom/src/log/Makefile (+1/-0)
modules/libcom/src/log/logClient.c (+12/-6)
modules/libcom/src/log/syslogSink.c (+49/-0)
Reviewer Review Type Date Requested Status
mdavidsaver Needs Information
Andrew Johnson Needs Fixing
Review via email:

Description of the change

Bridge from IOC errlog to syslog

If supported (!WIN32 && !vxWorks), and if $EPICS_IOC_LOG_IDENT is set,
then errlog messages will be passed to syslog().
The value of $EPICS_IOC_LOG_IDENT is used as the syslog identifier.

The idea being that an admin/init system will define EPICS_IOC_LOG_IDENT
appropriately if desired, so there is no need for boilerplate in every st.cmd.

By default this is only enabled for IOCs by calling errlogToSyslog()
from iocBuild(). (in case an IOC indirectly execs eg. caget)

To post a comment you must log in.
Revision history for this message
mdavidsaver (mdavidsaver) wrote :
Revision history for this message
Andrew Johnson (anj) wrote :

Group 10/4: Rename _IDENT to _PREFIX and hook into the existing iocLogPrefix() iocsh command, make that env the default prefix, or the user can call iocLogPrefix() from their st.cmd file.

review: Needs Fixing
Revision history for this message
mdavidsaver (mdavidsaver) wrote :

Updated to interconnect with iocLogPrefix(). This adds a restriction that iocLogPrefix() can't be called after iocInit(). I don't expect this to be a problem, but have no data on usage. Maybe telling that no usage appears in a tech-talk archive.

Should give the originators of iocLogPrefix() (SLAC) an opportunity to comment prior to merging.

review: Needs Information
Revision history for this message
Andrew Johnson (anj) wrote :

iocLogPrefix() can only be called once, and if you're using it that's presumably because you want to be able to identify which IOC generated each log message, so you should be running it before any messages will be logged anyway. Thus I see no problem with that new restriction.

However with this change applied any supported IOC that sets iocLogPrefix() would always pass its log messages to syslog; I don't see any way to set a prefix for logging to an external log-server without also enabling errlogToSyslog(). An IOC currently has to run iocLogInit to connect to an external log-server, so maybe errlogToSyslog should become a similar command that must appear in st.cmd (iocBuild is too late anyway, dbLoadRecords calls errlogPrintf() and its output should be logged). It would probably need protection against being run twice, and maybe emit an error message if the prefix isn't set.

I'm not convinced we have the prefix logic right — sorry! Wouldn't the following be more intuitive: If the _PREFIX environment variable is set (and not blank) it should be used automatically without needing to call iocLogPrefix(). The user can set the variable in the st.cmd file, so the command isn't really necessary anyway. There should be one or more commands in the st.cmd file to enable the desired log mechanisms. The enable commands can check any necessary preconditions (such as requiring _PREFIX to be set, and saving its value) and report errors then.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

> I don't see any way to set a prefix for logging to an external log-server without also enabling errlogToSyslog()

Well yes, I removed this as requested...

> become a similar command that must appear in st.cmd

Unfortunately this is exactly what I'm trying to avoid.

My motivating use case is a sysadmin wishing to setup/change logging for all IOCs on a host. Defining an environment variable is a common way for an init script (or systemd unit) to communicate this. Of course, this can already be accomplished by capturing stdout/err (a la procServ). Though procServ isn't used as often as (I think) it should be.

> I'm not convinced we have the prefix logic right — sorry!

Shall I remove the interconnection with iocLogPrefix() ?

Revision history for this message
Martin Konrad (info-martin-konrad) wrote :

Another thought on environment variables vs. commands in st.cmd: At FRIB our IT administrators are running our log servers. They know best which host/port/protocol should be used. They have the tools to provide these settings consistently as environment variables/config files on all machines. I would thus suggest to make it possible for IT to be in control of these settings while IOC engineers are in control of which database files are loaded etc.

We are currently doing stuff like


to ensure our sysadmins are in control. This works but it would be easier if the IOC would pick up logging settings directly from environment variables.

Revision history for this message
Andrew Johnson (anj) wrote :

Every site wants to configure their IOCs and startup slightly differently, and I don't have a problem with allowing that. I don't see why you guys don't like having to include commands in your startup scripts to implement the behavior you want though; that caPutLogInit command above doesn't have to appear in the st.cmd file itself, and it shows that you can still use environment variables for configuration parameters along with a startup line.

At APS we collect all our common commands into scripts in a site-specific support module that the st.cmd files executes at the appropriate time. I am working on makeBaseApp templates for the APS Upgrade to make it easy for engineers to generate IOCs that follow our standards. The st.cmd file generated for a soft IOC looks like this:


< "envPaths"
< "${IOCSTD}/bin/${ARCH}/envParams.iocsh"

## Connect to log server

cd "${TOP}"

## Register support components
dbLoadDatabase "dbd/first.dbd"
first_registerRecordDeviceDriver pdbbase

## Load record instances here
#dbLoadRecords "db/first.db", "IOC=${IOC_NAME=${IOC}}"

## Do iocStd pre-initialization
< "${IOCSTD}/bin/${ARCH}/preInit.iocsh"

## Start the IOC running

## Start sequence programs here
#seq firstSeq, "IOC=${IOC_NAME=${IOC}}"

## Do iocStd post-initialization
< "${IOCSTD}/bin/${ARCH}/postInit.iocsh"

## Script st.cmd done.

Those three ${IOCSTD}/bin/${ARCH}/*.iocsh scripts implement all the common initialization tasks, and if I need to add or modify something for all our IOCs that's where I make the necessary changes. I suspect ITER uses Ralph's snippets feature to do something similar. You could also have the st.cmd file execute scripts whose path comes from the environment, giving you even more control if that's what you really want.

In my case above I'm putting the iocLogInit command in the st.cmd file instead of a script to allow the engineers to turn logging off if/when they want to because that's something they're used to, but I could put it in the envParams.ioch script instead.

Unmerged commits

13854ad... by mdavidsaver

Copy errlog to syslog()

If supported (!WIN32 && !vxWorks), and if $EPICS_IOC_LOG_PREFIX is set,
then errlog messages will be passed to syslog().
The value of $EPICS_IOC_LOG_PREFIX is used as the syslog identifier.

errlogToSyslog() connect w/ iocLogPrefix()

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/configure/CONFIG_ENV b/configure/CONFIG_ENV
2index 6d0d52a..e47236a 100644
3--- a/configure/CONFIG_ENV
4+++ b/configure/CONFIG_ENV
5@@ -53,3 +53,7 @@ EPICS_IOC_IGNORE_SERVERS=""
6 # EPICS_IOC_LOG_PORT Log server port number etc.
9+# Log Client:
10+# If set, log to local syslog() with this identifier
13diff --git a/modules/database/src/ioc/misc/iocInit.c b/modules/database/src/ioc/misc/iocInit.c
14index b8aa438..90b69ef 100644
15--- a/modules/database/src/ioc/misc/iocInit.c
16+++ b/modules/database/src/ioc/misc/iocInit.c
17@@ -194,6 +194,8 @@ int iocBuild(void)
18 {
19 int status;
21+ errlogToSyslog();
23 status = iocBuild_1();
24 if (status) return status;
26diff --git a/modules/libcom/src/env/envDefs.h b/modules/libcom/src/env/envDefs.h
27index 2490702..bbb37ff 100644
28--- a/modules/libcom/src/env/envDefs.h
29+++ b/modules/libcom/src/env/envDefs.h
30@@ -72,6 +72,7 @@ epicsShareExtern const ENV_PARAM EPICS_IOC_LOG_FILE_LIMIT;
31 epicsShareExtern const ENV_PARAM EPICS_IOC_LOG_FILE_NAME;
33 epicsShareExtern const ENV_PARAM IOCSH_PS1;
34+epicsShareExtern const ENV_PARAM EPICS_IOC_LOG_PREFIX;
35 epicsShareExtern const ENV_PARAM IOCSH_HISTSIZE;
38diff --git a/modules/libcom/src/error/errlog.h b/modules/libcom/src/error/errlog.h
39index e8a4fb5..66108f2 100644
40--- a/modules/libcom/src/error/errlog.h
41+++ b/modules/libcom/src/error/errlog.h
42@@ -84,6 +84,14 @@ epicsShareFunc int errlogVprintfNoConsole(const char *pformat,va_list pvar);
44 epicsShareFunc void errSymLookup(long status, char *pBuf, size_t bufLength);
46+/** @brief (maybe) send errlog message to syslog.
47+ *
48+ * If supported (!WIN32 && !vxWorks), and if $EPICS_IOC_LOG_PREFIX is set,
49+ * then errlog messages will be passed to syslog().
50+ * The value of $EPICS_IOC_LOG_PREFIX is used as the syslog identifier.
51+ */
52+epicsShareFunc void errlogToSyslog(void);
54 #ifdef __cplusplus
55 }
56 #endif
57diff --git a/modules/libcom/src/log/Makefile b/modules/libcom/src/log/Makefile
58index 2dcfbc5..eddfc05 100644
59--- a/modules/libcom/src/log/Makefile
60+++ b/modules/libcom/src/log/Makefile
61@@ -12,6 +12,7 @@ INC += iocLog.h
62 INC += logClient.h
63 Com_SRCS += iocLog.c
64 Com_SRCS += logClient.c
65+Com_SRCS += syslogSink.c
67 PROD_HOST += iocLogServer
69diff --git a/modules/libcom/src/log/logClient.c b/modules/libcom/src/log/logClient.c
70index 44883e3..a1370ce 100644
71--- a/modules/libcom/src/log/logClient.c
72+++ b/modules/libcom/src/log/logClient.c
73@@ -34,6 +34,7 @@
74 #include "epicsExit.h"
75 #include "epicsSignal.h"
76 #include "epicsExport.h"
77+#include "envDefs.h"
79 #include "logClient.h"
81@@ -61,10 +62,10 @@ typedef struct {
82 static const double LOG_RESTART_DELAY = 5.0; /* sec */
83 static const double LOG_SERVER_SHUTDOWN_TIMEOUT = 30.0; /* sec */
86- * If set using iocLogPrefix() this string is prepended to all log messages:
87+/* If set using iocLogPrefix() this string is prepended to all log messages.
88+ * Also referenced from syslogSink.c
89 */
90-static char* logClientPrefix = NULL;
91+char* logClientPrefix = NULL;
93 /*
94 * logClientClose ()
95@@ -553,16 +554,21 @@ void epicsShareAPI iocLogPrefix(const char * prefix)
96 */
98 if (logClientPrefix) {
99- printf ("iocLogPrefix: The prefix was already set to \"%s\" "
100- "and can't be changed.\n", logClientPrefix);
101+ if(prefix)
102+ printf ("iocLogPrefix: The prefix was already set to \"%s\" "
103+ "and can't be changed.\n", logClientPrefix);
104 return;
105 }
106+ if(!prefix) {
107+ prefix = envGetConfigParamPtr(&EPICS_IOC_LOG_PREFIX);
108+ }
110 if (prefix) {
111 unsigned prefixLen = strlen(prefix);
112 if (prefixLen > 0) {
113 char * localCopy = malloc(prefixLen+1);
114- strcpy(localCopy, prefix);
115+ if(localCopy)
116+ strcpy(localCopy, prefix);
117 logClientPrefix = localCopy;
118 }
119 }
120diff --git a/modules/libcom/src/log/syslogSink.c b/modules/libcom/src/log/syslogSink.c
121new file mode 100644
122index 0000000..1ee5c46
123--- /dev/null
124+++ b/modules/libcom/src/log/syslogSink.c
125@@ -0,0 +1,49 @@
127+* Copyright (c) 2019 Michael Davidsaver
128+* EPICS BASE Versions 3.13.7
129+* and higher are distributed subject to a Software License Agreement found
130+* in file LICENSE that is included with this distribution.
133+#if !defined(_WIN32) && !defined(vxWorks)
135+#include <syslog.h>
137+#define epicsExportSharedSymbols
138+#include <errlog.h>
139+#include <logClient.h>
140+#include <envDefs.h>
142+/* logClient.c */
143+extern char* logClientPrefix;
146+void err2sys(void *unused, const char *message)
148+ (void)unused;
149+ syslog(LOG_INFO, "%s", message);
152+void errlogToSyslog(void)
154+ const char *ident;
155+ iocLogPrefix(NULL);
156+ ident = logClientPrefix;
158+ if(!ident || !ident[0])
159+ return;
161+ openlog(ident, 0, LOG_USER);
163+ errlogAddListener(err2sys, 0);
164+ syslog(LOG_DEBUG, "Syslog connected");
167+#else /* !_WIN32 && !vxWorks */
169+#define epicsExportSharedSymbols
170+#include <errlog.h>
172+void errlogToSyslog(void) {}
174+#endif /* _WIN32 && !vxWorks */


People subscribed via source and target branches