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

[mdavidsaver]

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)

[mdavidsaver (mdavidsaver)]

cf. discussion at https://code.launchpad.net/~dirk.zimoch/epics-base/+git/epics-base/+merge/372925

[Andrew Johnson (anj)]

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

[mdavidsaver (mdavidsaver)]

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

[Andrew Johnson (anj)]

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.

[mdavidsaver (mdavidsaver)]

> 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() ?

[Martin Konrad (info-martin-konrad)]

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

caPutLogInit("${EPICS_PUT_LOG_INET}:${EPICS_PUT_LOG_PORT}", 1)

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.

[Andrew Johnson (anj)]

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:

#!../../bin/linux-x86_64/first

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

## Connect to log server
iocLogInit

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
iocInit

## 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 on 2019-09-24

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()

diff --git a/configure/CONFIG_ENV b/configure/CONFIG_ENV
index 6d0d52a..e47236a 100644
--- a/configure/CONFIG_ENV
+++ b/configure/CONFIG_ENV
@@ -53,3 +53,7 @@ EPICS_IOC_IGNORE_SERVERS=""
 # EPICS_IOC_LOG_PORT Log server port number etc.
 EPICS_IOC_LOG_PORT=7004
 
+# Log Client:
+# If set, log to local syslog() with this identifier
+EPICS_IOC_LOG_PREFIX=
+
diff --git a/modules/database/src/ioc/misc/iocInit.c b/modules/database/src/ioc/misc/iocInit.c
index b8aa438..90b69ef 100644
--- a/modules/database/src/ioc/misc/iocInit.c
+++ b/modules/database/src/ioc/misc/iocInit.c
@@ -194,6 +194,8 @@ int iocBuild(void)
 {
     int status;
 
+    errlogToSyslog();
+
     status = iocBuild_1();
     if (status) return status;
 
diff --git a/modules/libcom/src/env/envDefs.h b/modules/libcom/src/env/envDefs.h
index 2490702..bbb37ff 100644
--- a/modules/libcom/src/env/envDefs.h
+++ b/modules/libcom/src/env/envDefs.h
@@ -72,6 +72,7 @@ epicsShareExtern const ENV_PARAM EPICS_IOC_LOG_FILE_LIMIT;
 epicsShareExtern const ENV_PARAM EPICS_IOC_LOG_FILE_NAME;
 epicsShareExtern const ENV_PARAM EPICS_IOC_LOG_FILE_COMMAND;
 epicsShareExtern const ENV_PARAM IOCSH_PS1;
+epicsShareExtern const ENV_PARAM EPICS_IOC_LOG_PREFIX;
 epicsShareExtern const ENV_PARAM IOCSH_HISTSIZE;
 epicsShareExtern const ENV_PARAM IOCSH_HISTEDIT_DISABLE;
 
diff --git a/modules/libcom/src/error/errlog.h b/modules/libcom/src/error/errlog.h
index e8a4fb5..66108f2 100644
--- a/modules/libcom/src/error/errlog.h
+++ b/modules/libcom/src/error/errlog.h
@@ -84,6 +84,14 @@ epicsShareFunc int errlogVprintfNoConsole(const char *pformat,va_list pvar);
 
 epicsShareFunc void errSymLookup(long status, char *pBuf, size_t bufLength);
 
+/** @brief (maybe) send errlog message 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.
+ */
+epicsShareFunc void errlogToSyslog(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/modules/libcom/src/log/Makefile b/modules/libcom/src/log/Makefile
index 2dcfbc5..eddfc05 100644
--- a/modules/libcom/src/log/Makefile
+++ b/modules/libcom/src/log/Makefile
@@ -12,6 +12,7 @@ INC += iocLog.h
 INC += logClient.h
 Com_SRCS += iocLog.c
 Com_SRCS += logClient.c
+Com_SRCS += syslogSink.c
 
 PROD_HOST += iocLogServer
 
diff --git a/modules/libcom/src/log/logClient.c b/modules/libcom/src/log/logClient.c
index 44883e3..a1370ce 100644
--- a/modules/libcom/src/log/logClient.c
+++ b/modules/libcom/src/log/logClient.c
@@ -34,6 +34,7 @@
 #include "epicsExit.h"
 #include "epicsSignal.h"
 #include "epicsExport.h"
+#include "envDefs.h"
 
 #include "logClient.h"
 
@@ -61,10 +62,10 @@ typedef struct {
 static const double      LOG_RESTART_DELAY = 5.0; /* sec */
 static const double      LOG_SERVER_SHUTDOWN_TIMEOUT = 30.0; /* sec */
 
-/*
- * If set using iocLogPrefix() this string is prepended to all log messages:
+/* If set using iocLogPrefix() this string is prepended to all log messages.
+ * Also referenced from syslogSink.c
  */
-static char* logClientPrefix = NULL;
+char* logClientPrefix = NULL;
 
 /*
  * logClientClose ()
@@ -553,16 +554,21 @@ void epicsShareAPI iocLogPrefix(const char * prefix)
      */
 
     if (logClientPrefix) {
-        printf ("iocLogPrefix: The prefix was already set to \"%s\" "
-            "and can't be changed.\n", logClientPrefix);
+        if(prefix)
+            printf ("iocLogPrefix: The prefix was already set to \"%s\" "
+                "and can't be changed.\n", logClientPrefix);
         return;
     }
+    if(!prefix) {
+        prefix = envGetConfigParamPtr(&EPICS_IOC_LOG_PREFIX);
+    }
 
     if (prefix) {
         unsigned prefixLen = strlen(prefix);
         if (prefixLen > 0) {
             char * localCopy = malloc(prefixLen+1);
-            strcpy(localCopy, prefix);
+            if(localCopy)
+                strcpy(localCopy, prefix);
             logClientPrefix = localCopy;
         }
     }
diff --git a/modules/libcom/src/log/syslogSink.c b/modules/libcom/src/log/syslogSink.c
new file mode 100644
index 0000000..1ee5c46
--- /dev/null
+++ b/modules/libcom/src/log/syslogSink.c
@@ -0,0 +1,49 @@
+/*************************************************************************\
+* Copyright (c) 2019 Michael Davidsaver
+* EPICS BASE Versions 3.13.7
+* and higher are distributed subject to a Software License Agreement found
+* in file LICENSE that is included with this distribution.
+\*************************************************************************/
+
+#if !defined(_WIN32) && !defined(vxWorks)
+
+#include <syslog.h>
+
+#define epicsExportSharedSymbols
+#include <errlog.h>
+#include <logClient.h>
+#include <envDefs.h>
+
+/* logClient.c */
+extern char* logClientPrefix;
+
+static
+void err2sys(void *unused, const char *message)
+{
+    (void)unused;
+    syslog(LOG_INFO, "%s", message);
+}
+
+void errlogToSyslog(void)
+{
+    const char *ident;
+    iocLogPrefix(NULL);
+    ident = logClientPrefix;
+
+    if(!ident || !ident[0])
+        return;
+
+    openlog(ident, 0, LOG_USER);
+
+    errlogAddListener(err2sys, 0);
+    syslog(LOG_DEBUG, "Syslog connected");
+}
+
+#else /* !_WIN32 && !vxWorks */
+
+#define epicsExportSharedSymbols
+#include <errlog.h>
+
+void errlogToSyslog(void) {}
+
+#endif /* _WIN32 && !vxWorks */