Skip to content

Commit

Permalink
HPCC-31775 Changes following review
Browse files Browse the repository at this point in the history 
Signed-off-by: Shamser Ahmed <[email protected]>
  • Loading branch information
@shamser
shamser committed Jan 13, 2025
1 parent 2861a84 commit b0f5701
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 63 deletions.
2 changes: 1 addition & 1 deletion dali/base/daclient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ bool initClientProcess(IGroup *servergrp, DaliClientRole role, unsigned mpport,
covengrp->Release();
queryLogMsgManager()->setSession(myProcessSession());

if (getComponentConfigSP()->getPropBool("logging/@enableGlobalSysLog") || getGlobalConfigSP()->getPropBool("logging/@enableGlobalSysLog"))
if (getComponentConfigSP()->getPropBool("logging/@enableGlobalSysLog", getGlobalConfigSP()->getPropBool("logging/@enableGlobalSysLog")))
UseDaliForOperatorMessages(true);

if (!isContainerized()) // The Environment is bare-metal only
Expand Down
84 changes: 56 additions & 28 deletions dali/base/sysinfologger.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
############################################################################## */

#include "sysinfologger.hpp"
#include "daclient.hpp"
#include "jutil.hpp"

#define SDS_LOCK_TIMEOUT (5*60*1000) // 5 minutes
Expand Down Expand Up @@ -144,12 +143,12 @@ class CSysInfoLoggerMsg : implements ISysInfoLoggerMsg
const char *msg = msgPtree->queryProp(nullptr);
return msg ? msg : "";
}
void setHidden(bool _hidden)
virtual void setHidden(bool _hidden) override
{
ensureUpdateable();
msgPtree->setPropBool(ATTR_HIDDEN, _hidden);
}
StringBuffer & getXpath(StringBuffer & xpath)
virtual StringBuffer & getXpath(StringBuffer & xpath) override
{
unsigned year, month, day;
extractDate(queryTimeStamp(), year, month, day);
Expand Down Expand Up @@ -422,10 +421,10 @@ ISysInfoLoggerMsgFilter * createSysInfoLoggerMsgFilter(unsigned __int64 msgId, c

class CSysInfoLoggerMsgIterator : public CSimpleInterfaceOf<ISysInfoLoggerMsgIterator>
{
Linked<IConstSysInfoLoggerMsgFilter> filter;
// N.b. IRemoteConnection exists for the duration of the iterator so if this iterator exists for too long, it could cause
// performance issues for other clients: consider caching some messages and releasing connection (and reopening as necessary).
Owned<IRemoteConnection> conn;
Linked<IRemoteConnection> conn;
Linked<IConstSysInfoLoggerMsgFilter> filter;
bool updateable = false;
Owned<IPropertyTreeIterator> msgIter;
CSysInfoLoggerMsg infoMsg;
Expand All @@ -446,15 +445,8 @@ class CSysInfoLoggerMsgIterator : public CSimpleInterfaceOf<ISysInfoLoggerMsgIte
}

public:
CSysInfoLoggerMsgIterator(IConstSysInfoLoggerMsgFilter * _filter, bool _updateable=false, IRemoteConnection *_conn=nullptr) : filter(_filter), conn(_conn), updateable(_updateable)
CSysInfoLoggerMsgIterator(IRemoteConnection *_conn, IConstSysInfoLoggerMsgFilter * _filter, bool _updateable = false) : conn(_conn), filter(_filter), updateable(_updateable)
{
if (!conn)
{
unsigned mode = updateable ? RTM_LOCK_WRITE : RTM_LOCK_READ;
conn.setown(querySDS().connect(SYS_INFO_ROOT, myProcessSession(), mode, SDS_LOCK_TIMEOUT));
if (!conn)
throw makeStringExceptionV(-1, "CSysInfoLoggerMsgIterator: unable to create connection to '%s'", SYS_INFO_ROOT);
}
}
CSysInfoLoggerMsg & queryInfoLoggerMsg()
{
Expand Down Expand Up @@ -485,18 +477,54 @@ class CSysInfoLoggerMsgIterator : public CSimpleInterfaceOf<ISysInfoLoggerMsgIte
return msgIter ? msgIter->isValid() : false;
}
};
class NullSysInfoLoggerMsgIterator : public CSimpleInterfaceOf<ISysInfoLoggerMsgIterator>
{
public:
virtual ISysInfoLoggerMsg & query() override
{
UNIMPLEMENTED;
}
virtual bool first() override
{
return false;
}
virtual bool next() override
{
return false;
}
virtual bool isValid() override
{
return false;
}
};

ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(bool visibleOnly, bool hiddenOnly, unsigned year, unsigned month, unsigned day, const char *source)
ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(IRemoteConnection * conn, IConstSysInfoLoggerMsgFilter * msgFilter, bool updateable)
{
return new CSysInfoLoggerMsgIterator(conn, msgFilter, updateable);
}

ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(IConstSysInfoLoggerMsgFilter * msgFilter, bool updateable)
{
unsigned mode = updateable ? RTM_LOCK_WRITE : RTM_LOCK_READ;
Owned<IRemoteConnection> conn = querySDS().connect(SYS_INFO_ROOT, myProcessSession(), mode, SDS_LOCK_TIMEOUT);
if (!conn)
return new NullSysInfoLoggerMsgIterator();
return createSysInfoLoggerMsgIterator(conn, msgFilter, updateable);
}

ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(IRemoteConnection * conn, bool visibleOnly, bool hiddenOnly, unsigned year, unsigned month, unsigned day, const char *source, bool updateable)
{
Owned<CSysInfoLoggerMsgFilter> filter = new CSysInfoLoggerMsgFilter(visibleOnly, hiddenOnly, year, month, day, source);
return new CSysInfoLoggerMsgIterator(filter, false);
return createSysInfoLoggerMsgIterator(conn, filter, updateable);
}

ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(IConstSysInfoLoggerMsgFilter * msgFilter)
ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(bool visibleOnly, bool hiddenOnly, unsigned year, unsigned month, unsigned day, const char *source, bool updateable)
{
return new CSysInfoLoggerMsgIterator(msgFilter, false);
Owned<CSysInfoLoggerMsgFilter> filter = new CSysInfoLoggerMsgFilter(visibleOnly, hiddenOnly, year, month, day, source);
return createSysInfoLoggerMsgIterator(filter, updateable);
}


// returns messageId
unsigned __int64 logSysInfoError(const LogMsgCategory & cat, LogMsgCode code, const char *source, const char * msg, timestamp_type ts)
{
Expand Down Expand Up @@ -530,54 +558,54 @@ unsigned __int64 logSysInfoError(const LogMsgCategory & cat, LogMsgCode code, co
return makeMessageId(ts, seqn);
}

unsigned updateMessage(IConstSysInfoLoggerMsgFilter * msgFilter, std::function<void (CSysInfoLoggerMsg &)> updateOp)
unsigned updateMessage(IConstSysInfoLoggerMsgFilter * msgFilter, std::function<void (ISysInfoLoggerMsg &)> updateOp)
{
unsigned count = 0;
Owned<CSysInfoLoggerMsgIterator> iter = new CSysInfoLoggerMsgIterator(msgFilter, true);
Owned<ISysInfoLoggerMsgIterator> iter = createSysInfoLoggerMsgIterator(msgFilter, true);
ForEach(*iter)
{
CSysInfoLoggerMsg & sysInfoMsg = iter->queryInfoLoggerMsg();
ISysInfoLoggerMsg & sysInfoMsg = iter->query();
updateOp(sysInfoMsg);
++count;
}
return count;
}

unsigned updateMessage(unsigned __int64 msgId, const char *source, std::function<void (CSysInfoLoggerMsg &)> updateOp)
unsigned updateMessage(unsigned __int64 msgId, const char *source, std::function<void (ISysInfoLoggerMsg &)> updateOp)
{
Owned<IConstSysInfoLoggerMsgFilter> msgFilter = createSysInfoLoggerMsgFilter(msgId, source);
return updateMessage(msgFilter, updateOp);
}

unsigned hideLogSysInfoMsg(IConstSysInfoLoggerMsgFilter * msgFilter)
{
return updateMessage(msgFilter, [](CSysInfoLoggerMsg & sysInfoMsg){sysInfoMsg.setHidden(true);});
return updateMessage(msgFilter, [](ISysInfoLoggerMsg & sysInfoMsg){sysInfoMsg.setHidden(true);});
}

bool hideLogSysInfoMsg(unsigned __int64 msgId, const char *source)
{
return updateMessage(msgId, source, [](CSysInfoLoggerMsg & sysInfoMsg){sysInfoMsg.setHidden(true);})==1;
return updateMessage(msgId, source, [](ISysInfoLoggerMsg & sysInfoMsg){sysInfoMsg.setHidden(true);})==1;
}

unsigned unhideLogSysInfoMsg(IConstSysInfoLoggerMsgFilter * msgFilter)
{
return updateMessage(msgFilter, [](CSysInfoLoggerMsg & sysInfoMsg){sysInfoMsg.setHidden(false);});
return updateMessage(msgFilter, [](ISysInfoLoggerMsg & sysInfoMsg){sysInfoMsg.setHidden(false);});
}

bool unhideLogSysInfoMsg(unsigned __int64 msgId, const char *source)
{
return updateMessage(msgId, source, [](CSysInfoLoggerMsg & sysInfoMsg){sysInfoMsg.setHidden(false);})==1;
return updateMessage(msgId, source, [](ISysInfoLoggerMsg & sysInfoMsg){sysInfoMsg.setHidden(false);})==1;
}

unsigned deleteLogSysInfoMsg(IConstSysInfoLoggerMsgFilter * msgFilter)
{
std::vector<std::string> deleteXpathList;
Owned<IRemoteConnection> conn = querySDS().connect(SYS_INFO_ROOT, myProcessSession(), RTM_LOCK_WRITE, SDS_LOCK_TIMEOUT);
{
Owned<CSysInfoLoggerMsgIterator> iter = new CSysInfoLoggerMsgIterator(msgFilter, false, conn.getLink());
Owned<ISysInfoLoggerMsgIterator> iter = createSysInfoLoggerMsgIterator(conn, msgFilter, false);
ForEach(*iter)
{
CSysInfoLoggerMsg & sysInfoMsg = iter->queryInfoLoggerMsg();
ISysInfoLoggerMsg & sysInfoMsg = iter->query();
StringBuffer xpath;
sysInfoMsg.getXpath(xpath);
deleteXpathList.push_back(xpath.str());
Expand Down Expand Up @@ -764,7 +792,7 @@ void UseDaliForOperatorMessages(bool use)
if (!msgHandler)
{
msgHandler.setown(getDaliMsgLoggerHandler());
ILogMsgFilter * operatorFilter = getCategoryLogMsgFilter(MSGAUD_operator,
Owned<ILogMsgFilter> operatorFilter = getCategoryLogMsgFilter(MSGAUD_operator,
MSGCLS_disaster|MSGCLS_error|MSGCLS_warning,
WarnMsgThreshold);
queryLogMsgManager()->addMonitor(msgHandler, operatorFilter);
Expand Down
9 changes: 7 additions & 2 deletions dali/base/sysinfologger.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include "jlog.hpp"
#include "jutil.hpp"
#include "daclient.hpp"

#ifdef DALI_EXPORT
#define SYSINFO_API DECL_EXPORT
Expand All @@ -37,6 +38,8 @@ interface ISysInfoLoggerMsg
virtual LogMsgClass queryClass() const = 0;
virtual unsigned __int64 queryLogMsgId() const = 0;
virtual const char * queryMsg() const = 0;
virtual void setHidden(bool _hidden) = 0;
virtual StringBuffer & getXpath(StringBuffer & xpath) = 0;
};

interface IConstSysInfoLoggerMsgFilter : public IInterface
Expand Down Expand Up @@ -77,8 +80,10 @@ typedef IIteratorOf<ISysInfoLoggerMsg> ISysInfoLoggerMsgIterator;

SYSINFO_API ISysInfoLoggerMsgFilter * createSysInfoLoggerMsgFilter(const char *source=nullptr);
SYSINFO_API ISysInfoLoggerMsgFilter * createSysInfoLoggerMsgFilter(unsigned __int64 msgId, const char *source=nullptr);
SYSINFO_API ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(bool _visibleOnly, bool _hiddenOnly, unsigned _year, unsigned _month, unsigned _day, const char *source=nullptr);
SYSINFO_API ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(IConstSysInfoLoggerMsgFilter * msgFilter);
SYSINFO_API ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(IRemoteConnection * conn, IConstSysInfoLoggerMsgFilter * msgFilter, bool updateable = false);
SYSINFO_API ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(IConstSysInfoLoggerMsgFilter * msgFilter, bool updateable = false);
SYSINFO_API ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(IRemoteConnection * conn, bool visibleOnly, bool hiddenOnly, unsigned year, unsigned month, unsigned day, const char *source, bool updateable = false);
SYSINFO_API ISysInfoLoggerMsgIterator * createSysInfoLoggerMsgIterator(bool visibleOnly, bool hiddenOnly, unsigned year, unsigned month, unsigned day, const char *source, bool updateable = false);

SYSINFO_API unsigned __int64 logSysInfoError(const LogMsgCategory & cat, LogMsgCode code, const char *source, const char * msg, unsigned __int64 ts);
SYSINFO_API unsigned hideLogSysInfoMsg(IConstSysInfoLoggerMsgFilter * msgFilter);
Expand Down
46 changes: 20 additions & 26 deletions esp/services/ws_workunits/ws_wudetails.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,10 @@ void WUDetails::processRequest(IEspWUDetailsRequest &req, IEspWUDetailsResponse
// 2) dynamic global messages generated from executing components
// Static component messages will have COMPONENT_NOTE_MASK.
Owned<IPropertyTreeIterator> iter = componentConfig->getElements("warnings");
unsigned __int64 id=1;
unsigned __int64 entry_num=1;
CDateTime dt;
dt.setNow();
timestamp_type ts = dt.getTimeStamp();
ForEach(*iter)
{
IPropertyTree & cur = iter->query();
Expand All @@ -510,37 +513,28 @@ void WUDetails::processRequest(IEspWUDetailsRequest &req, IEspWUDetailsResponse
espWuResponseNote->setErrorCode_null();
espWuResponseNote->setSeverity(cur.queryProp("@severity"));
espWuResponseNote->setCost(0);
VStringBuffer idstr("%" I64F "u", id|COMPONENT_NOTE_MASK);
VStringBuffer idstr("%" I64F "u", makeMessageId(ts, entry_num, true));
espWuResponseNote->setId(idstr.str());
espWuResponseNotes.append(*espWuResponseNote.getClear());
id++;
entry_num++;
}
Owned<ISysInfoLoggerMsgFilter> msgFilter = createSysInfoLoggerMsgFilter();
msgFilter->setVisibleOnly();
try
Owned<ISysInfoLoggerMsgIterator> msgIter = createSysInfoLoggerMsgIterator(msgFilter);
ForEach(*msgIter)
{
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();
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<IEspWUResponseScope> respScope = createWUResponseScope("","");
respScope->setScopeName("");
Expand Down
8 changes: 4 additions & 4 deletions helm/hpcc/values.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1137,10 +1137,10 @@
"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"
}
"enableGlobalSysLog": {
"type": "boolean",
"description" : "Enable centralized reporting of important user/operator logging messages"
}
},
"additionalProperties": { "type": ["integer", "string", "boolean"] }
},
Expand Down
3 changes: 1 addition & 2 deletions helm/hpcc/values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ 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)
# Enable centralized reporting of important user/operator logging messages (disabled by default)
#enableGlobalSysLog: true

# tracing sets the default tracing information for all components. Can be overridden locally
Expand Down

0 comments on commit b0f5701

Please sign in to comment.