Skip to content

Commit ca893c0

Browse files
streeterdstreeterd
committed
HPCC-33885 WIP
Signed-off-by: Dave Streeter <[email protected]>
1 parent 6522db7 commit ca893c0

File tree

3 files changed

+131
-90
lines changed

3 files changed

+131
-90
lines changed

‎system/jlib/jlog.cpp

Lines changed: 128 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -2377,98 +2377,85 @@ static constexpr unsigned queueLenDefault = 512;
23772377
static constexpr unsigned queueDropDefault = 0; // disabled by default
23782378
static constexpr bool useSysLogDefault = false;
23792379

2380-
static Singleton<bool> firstUpdateConfigHookCall;
2381-
static bool firstUpdateConfigHookCallValue{true};
2382-
bool queryFirstUpdateConfigHookCall()
2380+
struct LogFormatChangedDetails
23832381
{
2384-
return *firstUpdateConfigHookCall.query([] () { return &firstUpdateConfigHookCallValue; });
2385-
}
2386-
2387-
auto updateConfigFunc = [](const IPropertyTree *oldComponentConfiguration, const IPropertyTree *oldGlobalConfiguration)
2388-
{
2389-
setupContainerizedLogMsgHandler(true);
2382+
StringBuffer logFormat{""};
2383+
bool formatChangeDetected{false};
23902384
};
2391-
static CConfigUpdateHook configUpdateHook;
2392-
// NOTE: The parameter updateConfigOnTheFly value of true signifies that the thread
2393-
// safe caller has completed setting up the logging configuration (normally
2394-
// from a main call).
2395-
// Any further configuration changes are from non-thread safe callers.
2396-
// EXCEPT: There are 2 calls to setupContainerizedLogMsgHandler that are
2397-
// thread-safe. In order to allow them to change the log format (if required)
2398-
// etc we need to allow the first config update hook call to update the config
2399-
// as if it was not called from the config update hook.
2400-
void setupContainerizedLogMsgHandler(bool updateConfigOnTheFly)
2385+
LogFormatChangedDetails hasLogFormatChanged(const Owned<IPropertyTree> &logConfig)
24012386
{
2402-
// Allow the first config update hook call to update the config as if it was not
2403-
// called from the config update hook to allow log configuration changes.
2404-
if(updateConfigOnTheFly && queryFirstUpdateConfigHookCall())
2387+
UWARNLOG("DJPS hasLogFormatChanged() called");
2388+
LogFormatChangedDetails logFormatChanged{};
2389+
2390+
if (logConfig->hasProp(logFormatAtt))
24052391
{
2406-
firstUpdateConfigHookCallValue = false;
2407-
updateConfigOnTheFly = false;
2392+
StringAttr logConfigFormat(logConfig->queryProp(logFormatAtt));
2393+
UWARNLOG("DJPS hasLogFormatChanged() logConfigFormat=%s", logConfigFormat.str());
2394+
logFormatChanged.logFormat.set(logConfigFormat.str());
2395+
if (streq(logConfigFormat, "xml"))
2396+
{
2397+
UWARNLOG("DJPS hasLogFormatChanged() xml");
2398+
if (theStderrHandler->queryFormatType() != LOGFORMAT_xml)
2399+
{
2400+
UWARNLOG("DJPS hasLogFormatChanged() logFormatChanged.formatChangeDetected = true");
2401+
logFormatChanged.formatChangeDetected = true;
2402+
}
2403+
}
2404+
else if (streq(logConfigFormat, "json"))
2405+
{
2406+
UWARNLOG("DJPS hasLogFormatChanged() json");
2407+
if (theStderrHandler->queryFormatType() != LOGFORMAT_json)
2408+
{
2409+
UWARNLOG("DJPS hasLogFormatChanged() logFormatChanged.formatChangeDetected = true");
2410+
logFormatChanged.formatChangeDetected = true;
2411+
}
2412+
}
2413+
else if (streq(logConfigFormat, "table"))
2414+
{
2415+
UWARNLOG("DJPS hasLogFormatChanged() table");
2416+
if (theStderrHandler->queryFormatType() != LOGFORMAT_table)
2417+
{
2418+
UWARNLOG("DJPS hasLogFormatChanged() logFormatChanged.formatChangeDetected = true");
2419+
logFormatChanged.formatChangeDetected = true;
2420+
}
2421+
}
2422+
else
2423+
LOG(MCoperatorWarning, "JLog: Invalid log format configuration detected '%s'!", logConfigFormat.str());
24082424
}
24092425

2426+
return logFormatChanged;
2427+
}
2428+
2429+
bool configureHandlers(bool rejectOnTheFlyChanges)
2430+
{
2431+
UWARNLOG("DJPS configureHandlers(%s)", boolToStr(rejectOnTheFlyChanges));
24102432
Owned<IPropertyTree> logConfig = getComponentConfigSP()->getPropTree("logging");
24112433
if (logConfig)
24122434
{
24132435
if (logConfig->getPropBool(logDisabledAtt, false))
24142436
{
24152437
removeLog();
2416-
2417-
configUpdateHook.installOnce(updateConfigFunc, false);
2418-
return;
2438+
UWARNLOG("DJPS logging disabled");
2439+
return false;
24192440
}
24202441

2421-
if (logConfig->hasProp(logFormatAtt))
2442+
if (rejectOnTheFlyChanges)
24222443
{
2423-
if (updateConfigOnTheFly)
2424-
{
2444+
UWARNLOG("DJPS checking logformat change");
2445+
LogFormatChangedDetails logFormatChanged = hasLogFormatChanged(logConfig);
2446+
if (logFormatChanged.formatChangeDetected)
24252447
UWARNLOG("JLog: Ignoring log format configuration change on the fly is not supported in a containerized environment");
2426-
}
2427-
else
2428-
{
2429-
const char *logFormat = logConfig->queryProp(logFormatAtt);
2430-
bool newFormatDetected = false;
2431-
if (streq(logFormat, "xml"))
2432-
{
2433-
if (theStderrHandler->queryFormatType() != LOGFORMAT_xml)
2434-
{
2435-
newFormatDetected = true;
2436-
theStderrHandler = new HandleLogMsgHandlerXML(stderr, MSGFIELD_STANDARD);
2437-
}
2438-
}
2439-
else if (streq(logFormat, "json"))
2440-
{
2441-
if (theStderrHandler->queryFormatType() != LOGFORMAT_json)
2442-
{
2443-
newFormatDetected = true;
2444-
theStderrHandler = new HandleLogMsgHandlerJSON(stderr, MSGFIELD_STANDARD);
2445-
}
2446-
}
2447-
else if (streq(logFormat, "table"))
2448-
{
2449-
if (theStderrHandler->queryFormatType() != LOGFORMAT_table)
2450-
{
2451-
newFormatDetected = true;
2452-
theStderrHandler = new HandleLogMsgHandlerTable(stderr, MSGFIELD_STANDARD);
2453-
}
2454-
}
2455-
else
2456-
LOG(MCoperatorWarning, "JLog: Invalid log format configuration detected '%s'!", logFormat);
2457-
2458-
if (newFormatDetected)
2459-
theManager->resetMonitors();
2460-
}
24612448
}
24622449

24632450
if (logConfig->hasProp(logFieldsAtt))
24642451
{
2465-
//Supported logging fields: TRC,SPN,AUD,CLS,DET,MID,TIM,DAT,PID,TID,NOD,JOB,USE,SES,COD,MLT,MCT,NNT,COM,QUO,PFX,ALL,STD
2452+
// Supported logging fields: TRC,SPN,AUD,CLS,DET,MID,TIM,DAT,PID,TID,NOD,JOB,USE,SES,COD,MLT,MCT,NNT,COM,QUO,PFX,ALL,STD
24662453
const char *logFields = logConfig->queryProp(logFieldsAtt);
24672454
if (!isEmptyString(logFields))
24682455
theStderrHandler->setMessageFields(logMsgFieldsFromAbbrevs(logFields));
24692456
}
24702457

2471-
//Only recreate filter if at least one filter attribute configured
2458+
// Only recreate filter if at least one filter attribute configured
24722459
if (logConfig->hasProp(logMsgDetailAtt) || logConfig->hasProp(logMsgAudiencesAtt) || logConfig->hasProp(logMsgClassesAtt))
24732460
{
24742461
LogMsgDetail logDetail = logConfig->getPropInt(logMsgDetailAtt, DefaultDetail);
@@ -2491,6 +2478,73 @@ static CConfigUpdateHook configUpdateHook;
24912478
Owned<ILogMsgFilter> filter = getCategoryLogMsgFilter(msgAudiences, msgClasses, logDetail, local);
24922479
theManager->changeMonitorFilter(theStderrHandler, filter);
24932480
}
2481+
}
2482+
2483+
return true;
2484+
}
2485+
2486+
auto updateConfigFunc = [](const IPropertyTree *oldComponentConfiguration, const IPropertyTree *oldGlobalConfiguration)
2487+
{
2488+
configureHandlers(true);
2489+
};
2490+
static CConfigUpdateHook configUpdateHook;
2491+
void setupContainerizedLogMsgHandler()
2492+
{
2493+
UWARNLOG("DJPS setupContainerizedLogMsgHandler() called");
2494+
configUpdateHook.installOnce(updateConfigFunc, false);
2495+
2496+
if (!configureHandlers(false)) // false return means that logging is disabled
2497+
{
2498+
UWARNLOG("DJPS !configureHandlers(false)");
2499+
return;
2500+
}
2501+
2502+
Owned<IPropertyTree> logConfig = getComponentConfigSP()->getPropTree("logging");
2503+
if (logConfig)
2504+
{
2505+
LogFormatChangedDetails logFormatChanged = hasLogFormatChanged(logConfig);
2506+
if (logFormatChanged.formatChangeDetected)
2507+
{
2508+
bool newFormatDetected = false;
2509+
if (streq(logFormatChanged.logFormat.str(), "xml"))
2510+
{
2511+
UWARNLOG("DJPS existing log format is xml");
2512+
if (theStderrHandler->queryFormatType() != LOGFORMAT_xml)
2513+
{
2514+
UWARNLOG("DJPS set newFormatDetected = true");
2515+
newFormatDetected = true;
2516+
theStderrHandler = new HandleLogMsgHandlerXML(stderr, MSGFIELD_STANDARD);
2517+
}
2518+
}
2519+
else if (streq(logFormatChanged.logFormat.str(), "json"))
2520+
{
2521+
UWARNLOG("DJPS existing log format is json");
2522+
if (theStderrHandler->queryFormatType() != LOGFORMAT_json)
2523+
{
2524+
UWARNLOG("DJPS set newFormatDetected = true");
2525+
newFormatDetected = true;
2526+
theStderrHandler = new HandleLogMsgHandlerJSON(stderr, MSGFIELD_STANDARD);
2527+
}
2528+
}
2529+
else if (streq(logFormatChanged.logFormat.str(), "table"))
2530+
{
2531+
UWARNLOG("DJPS existing log format is table");
2532+
if (theStderrHandler->queryFormatType() != LOGFORMAT_table)
2533+
{
2534+
UWARNLOG("DJPS set newFormatDetected = true");
2535+
newFormatDetected = true;
2536+
theStderrHandler = new HandleLogMsgHandlerTable(stderr, MSGFIELD_STANDARD);
2537+
}
2538+
}
2539+
else
2540+
LOG(MCoperatorWarning, "JLog: Invalid log format configuration detected '%s'!", logFormatChanged.logFormat.str());
2541+
2542+
if (newFormatDetected)
2543+
{
2544+
UWARNLOG("DJPS processing newFormatDetected = true");
2545+
theManager->resetMonitors();
2546+
}
2547+
}
24942548

24952549
unsigned queueLen = logConfig->getPropInt(logQueueLenAtt, queueLenDefault);
24962550
if (queueLen)
@@ -2513,29 +2567,17 @@ static CConfigUpdateHook configUpdateHook;
25132567
UseSysLogForOperatorMessages();
25142568

25152569
unsigned postMortemLines = logConfig->getPropInt(capturePostMortemAtt, 0);
2516-
if (postMortemLines)
2570+
if (postMortemLines && !thePostMortemHandler)
25172571
{
2518-
if (!thePostMortemHandler)
2519-
{
2520-
// augment postmortem files with <pid> to avoid clashes where multiple processes are running within
2521-
// same process space, e.g. hthor processes running in same k8s container
2522-
unsigned pid = GetCurrentProcessId();
2523-
VStringBuffer portMortemFileBase("/tmp/postmortem.%u.log", pid);
2572+
// augment postmortem files with <pid> to avoid clashes where multiple processes are running within
2573+
// same process space, e.g. hthor processes running in same k8s container
2574+
unsigned pid = GetCurrentProcessId();
2575+
VStringBuffer portMortemFileBase("/tmp/postmortem.%u.log", pid);
25242576

2525-
thePostMortemHandler = new PostMortemLogMsgHandler(portMortemFileBase, postMortemLines, MSGFIELD_STANDARD);
2526-
queryLogMsgManager()->addMonitor(thePostMortemHandler, getCategoryLogMsgFilter(MSGAUD_all, MSGCLS_all, TopDetail));
2527-
}
2528-
else
2529-
{
2530-
if (thePostMortemHandler->queryMaxLinesToKeep() != postMortemLines)
2531-
{
2532-
thePostMortemHandler->setMaxLinesToKeep(postMortemLines);
2533-
}
2534-
}
2577+
thePostMortemHandler = new PostMortemLogMsgHandler(portMortemFileBase, postMortemLines, MSGFIELD_STANDARD);
2578+
queryLogMsgManager()->addMonitor(thePostMortemHandler, getCategoryLogMsgFilter(MSGAUD_all, MSGCLS_all, TopDetail));
25352579
}
25362580
}
2537-
2538-
configUpdateHook.installOnce(updateConfigFunc, false);
25392581
}
25402582

25412583
ILogMsgManager * queryLogMsgManager()

‎system/jlib/jlog.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,8 @@ extern jlib_decl ILogMsgManager * queryLogMsgManager();
913913
extern jlib_decl ILogMsgHandler * queryStderrLogMsgHandler();
914914
extern jlib_decl ILogMsgHandler * queryPostMortemLogMsgHandler();
915915
extern jlib_decl bool copyPostMortemLogging(const char *target, bool clear);
916-
extern jlib_decl void setupContainerizedLogMsgHandler(bool updateConfigOnTheFly = false);
916+
extern jlib_decl void setupContainerizedLogMsgHandler();
917+
bool configureHandlers(bool rejectOnTheFlyChanges);
917918

918919
//extern jlib_decl ILogMsgManager * createLogMsgManager(); // use with care! (needed by mplog listener facility)
919920

‎system/jlib/jlog.ipp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -553,9 +553,7 @@ public:
553553
virtual void handleMessage(const LogMsg & msg) override;
554554
virtual bool needsPrep() const override { return false; }
555555
virtual void prep() override {}
556-
virtual unsigned queryMaxLinesToKeep() const override { return maxLinesToKeep; }
557556
virtual unsigned queryMessageFields() const override { return messageFields; }
558-
virtual void setMaxLinesToKeep(unsigned _maxLinesToKeep) override { maxLinesToKeep = _maxLinesToKeep; }
559557
virtual void setMessageFields(unsigned _fields) override { messageFields = _fields; }
560558
virtual void addToPTree(IPropertyTree * tree) const override;
561559
virtual int flush() override { CriticalBlock block(crit); return fflush(handle); }
@@ -571,7 +569,7 @@ protected:
571569
mutable StringBuffer filename;
572570
mutable CriticalSection crit;
573571
StringBuffer curMsgText;
574-
unsigned maxLinesToKeep = 0;
572+
const unsigned maxLinesToKeep = 0;
575573
unsigned messageFields = MSGFIELD_all;
576574
unsigned linesInCurrent = 0;
577575
unsigned sequence = 0;

0 commit comments

Comments
 (0)