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

Proposed by mdavidsaver
Status: Needs review
Proposed branch: ~epics-core/epics-base/+git/Com:errlog2syslog
Merge into: 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
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=""
53# EPICS_IOC_LOG_PORT Log server port number etc.53# EPICS_IOC_LOG_PORT Log server port number etc.
56# Log Client:
57# If set, log to local syslog() with this identifier
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)
195 int status;195 int status;
197 errlogToSyslog();
197 status = iocBuild_1();199 status = iocBuild_1();
198 if (status) return status;200 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;
72epicsShareExtern const ENV_PARAM EPICS_IOC_LOG_FILE_NAME;72epicsShareExtern const ENV_PARAM EPICS_IOC_LOG_FILE_NAME;
74epicsShareExtern const ENV_PARAM IOCSH_PS1;74epicsShareExtern const ENV_PARAM IOCSH_PS1;
75epicsShareExtern const ENV_PARAM EPICS_IOC_LOG_PREFIX;
75epicsShareExtern const ENV_PARAM IOCSH_HISTSIZE;76epicsShareExtern const ENV_PARAM IOCSH_HISTSIZE;
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);
85epicsShareFunc void errSymLookup(long status, char *pBuf, size_t bufLength);85epicsShareFunc void errSymLookup(long status, char *pBuf, size_t bufLength);
87/** @brief (maybe) send errlog message to syslog.
88 *
89 * If supported (!WIN32 && !vxWorks), and if $EPICS_IOC_LOG_PREFIX is set,
90 * then errlog messages will be passed to syslog().
91 * The value of $EPICS_IOC_LOG_PREFIX is used as the syslog identifier.
92 */
93epicsShareFunc void errlogToSyslog(void);
87#ifdef __cplusplus95#ifdef __cplusplus
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
12INC += logClient.h12INC += logClient.h
13Com_SRCS += iocLog.c13Com_SRCS += iocLog.c
14Com_SRCS += logClient.c14Com_SRCS += logClient.c
15Com_SRCS += syslogSink.c
16PROD_HOST += iocLogServer17PROD_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 @@
34#include "epicsExit.h"34#include "epicsExit.h"
35#include "epicsSignal.h"35#include "epicsSignal.h"
36#include "epicsExport.h"36#include "epicsExport.h"
37#include "envDefs.h"
38#include "logClient.h"39#include "logClient.h"
@@ -61,10 +62,10 @@ typedef struct {
61static const double LOG_RESTART_DELAY = 5.0; /* sec */62static const double LOG_RESTART_DELAY = 5.0; /* sec */
62static const double LOG_SERVER_SHUTDOWN_TIMEOUT = 30.0; /* sec */63static const double LOG_SERVER_SHUTDOWN_TIMEOUT = 30.0; /* sec */
64/*65/* If set using iocLogPrefix() this string is prepended to all log messages.
65 * If set using iocLogPrefix() this string is prepended to all log messages:66 * Also referenced from syslogSink.c
66 */67 */
67static char* logClientPrefix = NULL;68char* logClientPrefix = NULL;
70 * logClientClose ()71 * logClientClose ()
@@ -553,16 +554,21 @@ void epicsShareAPI iocLogPrefix(const char * prefix)
553 */554 */
555 if (logClientPrefix) {556 if (logClientPrefix) {
556 printf ("iocLogPrefix: The prefix was already set to \"%s\" "557 if(prefix)
557 "and can't be changed.\n", logClientPrefix);558 printf ("iocLogPrefix: The prefix was already set to \"%s\" "
559 "and can't be changed.\n", logClientPrefix);
558 return;560 return;
559 }561 }
562 if(!prefix) {
563 prefix = envGetConfigParamPtr(&EPICS_IOC_LOG_PREFIX);
564 }
561 if (prefix) {566 if (prefix) {
562 unsigned prefixLen = strlen(prefix);567 unsigned prefixLen = strlen(prefix);
563 if (prefixLen > 0) {568 if (prefixLen > 0) {
564 char * localCopy = malloc(prefixLen+1);569 char * localCopy = malloc(prefixLen+1);
565 strcpy(localCopy, prefix);570 if(localCopy)
571 strcpy(localCopy, prefix);
566 logClientPrefix = localCopy;572 logClientPrefix = localCopy;
567 }573 }
568 }574 }
diff --git a/modules/libcom/src/log/syslogSink.c b/modules/libcom/src/log/syslogSink.c
569new file mode 100644575new file mode 100644
index 0000000..1ee5c46
--- /dev/null
+++ b/modules/libcom/src/log/syslogSink.c
@@ -0,0 +1,49 @@
2* Copyright (c) 2019 Michael Davidsaver
3* EPICS BASE Versions 3.13.7
4* and higher are distributed subject to a Software License Agreement found
5* in file LICENSE that is included with this distribution.
8#if !defined(_WIN32) && !defined(vxWorks)
10#include <syslog.h>
12#define epicsExportSharedSymbols
13#include <errlog.h>
14#include <logClient.h>
15#include <envDefs.h>
17/* logClient.c */
18extern char* logClientPrefix;
21void err2sys(void *unused, const char *message)
23 (void)unused;
24 syslog(LOG_INFO, "%s", message);
27void errlogToSyslog(void)
29 const char *ident;
30 iocLogPrefix(NULL);
31 ident = logClientPrefix;
33 if(!ident || !ident[0])
34 return;
36 openlog(ident, 0, LOG_USER);
38 errlogAddListener(err2sys, 0);
39 syslog(LOG_DEBUG, "Syslog connected");
42#else /* !_WIN32 && !vxWorks */
44#define epicsExportSharedSymbols
45#include <errlog.h>
47void errlogToSyslog(void) {}
49#endif /* _WIN32 && !vxWorks */


People subscribed via source and target branches