HPCC-31775 Changes following review

Signed-off-by: Shamser Ahmed <[email protected]>
hpcc-systems · Dec 9, 2024 · acb2cc7 · acb2cc7
1 parent 0ffdd7f
commit acb2cc7
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 32 deletions.
diff --git a/dali/base/daclient.cpp b/dali/base/daclient.cpp
@@ -142,8 +142,9 @@ bool initClientProcess(IGroup *servergrp, DaliClientRole role, unsigned mpport,
     covengrp->Release();
     queryLogMsgManager()->setSession(myProcessSession());
 
-    if (getGlobalConfigSP()->getPropBool("@enableGlobalSysLog", true))
-        UseDaliForOperatorMessages();
+    if (getComponentConfigSP()->getPropBool("logging/@enableGlobalSysLog") || getGlobalConfigSP()->getPropBool("logging/@enableGlobalSysLog"))
+        UseDaliForOperatorMessages(true);
+
     if (!isContainerized()) // The Environment is bare-metal only
     {
         // auto install environment monitor for server roles

diff --git a/dali/base/sysinfologger.cpp b/dali/base/sysinfologger.cpp
@@ -695,7 +695,7 @@ unsigned deleteOlderThanLogSysInfoMsg(bool visibleOnly, bool hiddenOnly, unsigne
     return count;
 }
 
-class DaliMsgLoggerHandler : public CInterfaceOf<ILogMsgHandler>
+class DaliMsgLoggerHandler : public CSimpleInterfaceOf<ILogMsgHandler>
 {
 public:
     DaliMsgLoggerHandler(unsigned _messageFields=MSGFIELD_all) : messageFields(_messageFields)
@@ -708,7 +708,7 @@ class DaliMsgLoggerHandler : public CInterfaceOf<ILogMsgHandler>
         unsigned __int64 ts = sysInfo.queryTime() * 1000000 + sysInfo.queryUSecs();
         logSysInfoError(msg.queryCategory(), msg.queryCode(), queryComponentName(), msg.queryText(), ts);
     }
-    virtual bool needsPrep() const  override
+    virtual bool needsPrep() const override
     {
         return false;
     }
@@ -746,23 +746,25 @@ class DaliMsgLoggerHandler : public CInterfaceOf<ILogMsgHandler>
     unsigned messageFields = MSGFIELD_all;
 };
 
+static Owned<ILogMsgHandler> msgHandler;
+
 void UseDaliForOperatorMessages(bool use)
 {
-    static ILogMsgHandler *msgHandler=NULL;
-    if (use==(msgHandler!=NULL))
-        return;
     if (use)
     {
-        msgHandler = getDaliMsgLoggerHandler();
-        ILogMsgFilter * operatorFilter = getCategoryLogMsgFilter(MSGAUD_operator,
-                                                                 MSGCLS_disaster|MSGCLS_error|MSGCLS_warning,
-                                                                 WarnMsgThreshold);
-        queryLogMsgManager()->addMonitorOwn(msgHandler, operatorFilter);
+        if (!msgHandler)
+        {
+            msgHandler.setown(getDaliMsgLoggerHandler());
+            ILogMsgFilter * operatorFilter = getCategoryLogMsgFilter(MSGAUD_operator,
+                                                                    MSGCLS_disaster|MSGCLS_error|MSGCLS_warning,
+                                                                    WarnMsgThreshold);
+            queryLogMsgManager()->addMonitor(msgHandler, operatorFilter);
+        }
     }
-    else
+    else if (msgHandler)
     {
         queryLogMsgManager()->removeMonitor(msgHandler);
-        msgHandler = NULL;
+        msgHandler.clear();
     }
 }
 

diff --git a/dali/base/sysinfologger.hpp b/dali/base/sysinfologger.hpp
@@ -90,5 +90,5 @@ SYSINFO_API bool deleteLogSysInfoMsg(unsigned __int64 msgId, const char *source=
 SYSINFO_API unsigned deleteOlderThanLogSysInfoMsg(bool visibleOnly, bool hiddenOnly, unsigned year, unsigned month, unsigned day, const char *source=nullptr);
 
 SYSINFO_API ILogMsgHandler * getDaliMsgLoggerHandler();
-SYSINFO_API void UseDaliForOperatorMessages(bool use=true);
+SYSINFO_API void UseDaliForOperatorMessages(bool use);
 #endif
diff --git a/esp/services/ws_workunits/ws_wudetails.cpp b/esp/services/ws_workunits/ws_wudetails.cpp
@@ -24,7 +24,7 @@
 #include "jstatcodes.h"
 #include "sysinfologger.hpp"
 
-#define COMPONENT_NOTE_MASK 0xFFFFFC0
+#define COMPONENT_NOTE_MASK 0x80000000
 
 typedef std::pair<WuAttr, StringBuffer> AttribValuePair;
 bool operator==(const AttribValuePair & p1, const AttribValuePair & p2)
@@ -517,21 +517,30 @@ void WUDetails::processRequest(IEspWUDetailsRequest &req, IEspWUDetailsResponse
             }
             Owned<ISysInfoLoggerMsgFilter> msgFilter = createSysInfoLoggerMsgFilter();
             msgFilter->setVisibleOnly();
-            Owned<ISysInfoLoggerMsgIterator> msgIter = createSysInfoLoggerMsgIterator(msgFilter);
-            ForEach(*msgIter)
+            try
             {
-                ISysInfoLoggerMsg & sysInfoMsg = msgIter->query();
-                Owned<IEspWUResponseNote> espWuResponseNote = createWUResponseNote("","");
-                StringBuffer tmpbuf;
-                encodeXML(sysInfoMsg.queryMsg(), tmpbuf, ENCODE_NEWLINES, strlen(sysInfoMsg.queryMsg()), true);
-                espWuResponseNote->setSource(sysInfoMsg.querySource());
-                espWuResponseNote->setMessage(tmpbuf.str());
-                espWuResponseNote->setErrorCode(sysInfoMsg.queryLogMsgCode());
-                espWuResponseNote->setSeverity(LogMsgClassToVarString(sysInfoMsg.queryClass()));
-                espWuResponseNote->setCost(0);
-                VStringBuffer idstr("%" I64F "u", sysInfoMsg.queryLogMsgId());
-                espWuResponseNote->setId(idstr.str());
-                espWuResponseNotes.append(*espWuResponseNote.getClear());
+                Owned<ISysInfoLoggerMsgIterator> msgIter = createSysInfoLoggerMsgIterator(msgFilter);
+                ForEach(*msgIter)
+                {
+                    ISysInfoLoggerMsg & sysInfoMsg = msgIter->query();
+                    Owned<IEspWUResponseNote> espWuResponseNote = createWUResponseNote("","");
+                    StringBuffer tmpbuf;
+                    encodeXML(sysInfoMsg.queryMsg(), tmpbuf, ENCODE_NEWLINES, strlen(sysInfoMsg.queryMsg()), true);
+                    espWuResponseNote->setSource(sysInfoMsg.querySource());
+                    espWuResponseNote->setMessage(tmpbuf.str());
+                    espWuResponseNote->setErrorCode(sysInfoMsg.queryLogMsgCode());
+                    espWuResponseNote->setSeverity(LogMsgClassToVarString(sysInfoMsg.queryClass()));
+                    espWuResponseNote->setCost(0);
+                    VStringBuffer idstr("%" I64F "u", sysInfoMsg.queryLogMsgId());
+                    espWuResponseNote->setId(idstr.str());
+                    espWuResponseNotes.append(*espWuResponseNote.getClear());
+                }
+            }
+            catch(IException *e)
+            {
+                // Non-existant global messages may mean that /SysInfoLogs hasn't been created
+                // so catch and ignore the exception
+                e->Release();
             }
             Owned<IEspWUResponseScope> respScope = createWUResponseScope("","");
             respScope->setScopeName("");

diff --git a/helm/hpcc/values.schema.json b/helm/hpcc/values.schema.json
@@ -1137,7 +1137,11 @@
           "type": "string",
           "description": "The data format used to report logging: xml|json|table(space delimited values)",
           "enum": [ "xml", "json", "table"]
-        }
+        },
+	"enableGlobalSysLog": {
+	  "type": "boolean",
+          "description" : "Enable the reporting of important user/operator messages in ECL Watch"
+	}
       },
       "additionalProperties": { "type": ["integer", "string", "boolean"] }
     },

diff --git a/helm/hpcc/values.yaml b/helm/hpcc/values.yaml
@@ -24,6 +24,9 @@ global:
   # logging sets the default logging information for all components. Can be overridden locally
   logging:
     detail: 80
+    # GlobalSysLog records user/operator messages to dali so that messages are reported in ECL Watch
+    # The following enables GlobalSysLog (default: disabled)
+    #enableGlobalSysLog: true
 
   # tracing sets the default tracing information for all components. Can be overridden locally
   tracing:

diff --git a/testing/unittests/dalitests.cpp b/testing/unittests/dalitests.cpp
@@ -3182,7 +3182,6 @@ class DaliSysInfoLoggerTester : public CppUnit::TestFixture
     void testInit()
     {
         daliClientInit();
-        UseDaliForOperatorMessages(false);
     }
     void testWrite()
     {