From 8de7505055bdafe3a327c0accb952f7caf24d81f Mon Sep 17 00:00:00 2001 From: Florent De Neve Date: Fri, 20 Sep 2024 15:29:53 +0200 Subject: [PATCH 1/7] Update logging macros to accept a map of parameters --- libstuff/SLog.cpp | 21 ++++++++++++++++ libstuff/libstuff.h | 52 ++++++++++++++++++++++++++++++++-------- sqlitecluster/SQLite.cpp | 2 +- 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/libstuff/SLog.cpp b/libstuff/SLog.cpp index 9e26af9c6..630604575 100644 --- a/libstuff/SLog.cpp +++ b/libstuff/SLog.cpp @@ -38,3 +38,24 @@ void SLogStackTrace(int level) { } } } + +string addLogParams(const string& message) { + return message; +} + +string addLogParams(const string& message, const map& params) { + std::string result = message; + + if (!params.empty()) { + result += " ~~ "; + for (size_t i = 0; i < params.size(); ++i) { + if (i > 0) { + result += " "; + } + const auto& param = *std::next(params.begin(), i); + result += param.first + ": '" + param.second + "'"; + } + } + + return result; +} diff --git a/libstuff/libstuff.h b/libstuff/libstuff.h index e05efd4d6..f71013be8 100644 --- a/libstuff/libstuff.h +++ b/libstuff/libstuff.h @@ -230,14 +230,18 @@ void SSyslogSocketDirect(int priority, const char* format, ...); // Atomic pointer to the syslog function that we'll actually use. Easy to change to `syslog` or `SSyslogSocketDirect`. extern atomic SSyslogFunc; +//template +//string addLogParams(const string& message, const map params); + +string addLogParams(const string& message, const map& params); +string addLogParams(const string& message); + // **NOTE: rsyslog default max line size is 8k bytes. We split on 7k byte boundaries in order to fit the syslog line prefix and the expanded \r\n to #015#012 #define SWHEREAMI SThreadLogPrefix + "(" + basename((char*)__FILE__) + ":" + SToStr(__LINE__) + ") " + __FUNCTION__ + " [" + SThreadLogName + "] " -#define SSYSLOG(_PRI_, _MSG_) \ +#define SSYSLOG(_PRI_, _MSG_, ...) \ do { \ if (_g_SLogMask & (1 << (_PRI_))) { \ - ostringstream __out; \ - __out << _MSG_ << endl; \ - const string s = __out.str(); \ + const string s = addLogParams(_MSG_, ##__VA_ARGS__); \ const string prefix = SWHEREAMI; \ for (size_t i = 0; i < s.size(); i += 7168) { \ (*SSyslogFunc)(_PRI_, "%s", (prefix + s.substr(i, 7168)).c_str()); \ @@ -246,14 +250,42 @@ extern atomic SSyslogFunc; } while (false) #define SLOGPREFIX "" -#define SDEBUG(_MSG_) SSYSLOG(LOG_DEBUG, "[dbug] " << SLOGPREFIX << _MSG_) -#define SINFO(_MSG_) SSYSLOG(LOG_INFO, "[info] " << SLOGPREFIX << _MSG_) -#define SHMMM(_MSG_) SSYSLOG(LOG_NOTICE, "[hmmm] " << SLOGPREFIX << _MSG_) -#define SWARN(_MSG_) SSYSLOG(LOG_WARNING, "[warn] " << SLOGPREFIX << _MSG_) -#define SALERT(_MSG_) SSYSLOG(LOG_ALERT, "[alrt] " << SLOGPREFIX << _MSG_) +#define SDEBUG(_MSG_, ...) \ + do { \ + stringstream ss; \ + ss << "[dbug] " << SLOGPREFIX << _MSG_; \ + SSYSLOG(LOG_INFO, ss.str(), ##__VA_ARGS__); \ + } while (false) +#define SINFO(_MSG_, ...) \ + do { \ + stringstream ss; \ + ss << "[info] " << SLOGPREFIX << _MSG_; \ + SSYSLOG(LOG_INFO, ss.str(), ##__VA_ARGS__); \ + } while (false) +#define SHMMM(_MSG_, ...) \ + do { \ + stringstream ss; \ + ss << "[hmmm] " << SLOGPREFIX << _MSG_; \ + SSYSLOG(LOG_INFO, ss.str(), ##__VA_ARGS__); \ + } while (false) +#define SWARN(_MSG_, ...) \ + do { \ + stringstream ss; \ + ss << "[warn] " << SLOGPREFIX << _MSG_; \ + SSYSLOG(LOG_INFO, ss.str(), ##__VA_ARGS__); \ + } while (false) +#define SALERT(_MSG_, ...) \ + do { \ + stringstream ss; \ + ss << "[alrt] " << SLOGPREFIX << _MSG_; \ + SSYSLOG(LOG_INFO, ss.str(), ##__VA_ARGS__); \ + } while (false) + #define SERROR(_MSG_) \ do { \ - SSYSLOG(LOG_ERR, "[eror] " << SLOGPREFIX << _MSG_); \ + stringstream ss; \ + ss << "[eror] " << SLOGPREFIX << _MSG_; \ + SSYSLOG(LOG_ERR, ss.str(), map()); \ SLogStackTrace(); \ abort(); \ } while (false) diff --git a/sqlitecluster/SQLite.cpp b/sqlitecluster/SQLite.cpp index 7fc77fbfc..d1a89060e 100644 --- a/sqlitecluster/SQLite.cpp +++ b/sqlitecluster/SQLite.cpp @@ -438,7 +438,7 @@ bool SQLite::verifyTable(const string& tableName, const string& sql, bool& creat } bool SQLite::verifyIndex(const string& indexName, const string& tableName, const string& indexSQLDefinition, bool isUnique, bool createIfNotExists) { - SINFO("Verifying index '" << indexName << "'. isUnique? " << to_string(isUnique)); + SINFO("Verifying index", {{"indexName", indexName}, {"isUnique?", to_string(isUnique)}}); SQResult result; SASSERT(read("SELECT sql FROM sqlite_master WHERE type='index' AND tbl_name=" + SQ(tableName) + " AND name=" + SQ(indexName) + ";", result)); From d75211cf398431d46d083095f353ec37a2db7736 Mon Sep 17 00:00:00 2001 From: Florent De Neve Date: Fri, 27 Sep 2024 15:55:42 +0200 Subject: [PATCH 2/7] Cleanup addLogParams function --- libstuff/SLog.cpp | 8 ++------ libstuff/libstuff.h | 3 +-- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/libstuff/SLog.cpp b/libstuff/SLog.cpp index 630604575..f17ccdb6e 100644 --- a/libstuff/SLog.cpp +++ b/libstuff/SLog.cpp @@ -39,12 +39,8 @@ void SLogStackTrace(int level) { } } -string addLogParams(const string& message) { - return message; -} - string addLogParams(const string& message, const map& params) { - std::string result = message; + string result = message; if (!params.empty()) { result += " ~~ "; @@ -52,7 +48,7 @@ string addLogParams(const string& message, const map& params) { if (i > 0) { result += " "; } - const auto& param = *std::next(params.begin(), i); + const auto& param = *next(params.begin(), i); result += param.first + ": '" + param.second + "'"; } } diff --git a/libstuff/libstuff.h b/libstuff/libstuff.h index f71013be8..c0aec3038 100644 --- a/libstuff/libstuff.h +++ b/libstuff/libstuff.h @@ -233,8 +233,7 @@ extern atomic SSyslogFunc; //template //string addLogParams(const string& message, const map params); -string addLogParams(const string& message, const map& params); -string addLogParams(const string& message); +string addLogParams(const string& message, const map& params = {}); // **NOTE: rsyslog default max line size is 8k bytes. We split on 7k byte boundaries in order to fit the syslog line prefix and the expanded \r\n to #015#012 #define SWHEREAMI SThreadLogPrefix + "(" + basename((char*)__FILE__) + ":" + SToStr(__LINE__) + ") " + __FUNCTION__ + " [" + SThreadLogName + "] " From 8f0e66ff006ad593c3fcc9f039537afe1b676fb3 Mon Sep 17 00:00:00 2001 From: Florent De Neve Date: Fri, 27 Sep 2024 16:10:51 +0200 Subject: [PATCH 3/7] Dry macros --- libstuff/libstuff.cpp | 9 ++++----- libstuff/libstuff.h | 46 ++++++++++--------------------------------- 2 files changed, 14 insertions(+), 41 deletions(-) diff --git a/libstuff/libstuff.cpp b/libstuff/libstuff.cpp index b6a79aa5d..d192f2f40 100644 --- a/libstuff/libstuff.cpp +++ b/libstuff/libstuff.cpp @@ -2821,8 +2821,8 @@ bool SREMatch(const string& regExp, const string& input, bool caseSensitive, boo STHROW("Bad regex: " + regExp); } - pcre2_match_context* matchContext = pcre2_match_context_create(0); - pcre2_set_depth_limit(matchContext, 1000); + pcre2_match_context* matchContext = pcre2_match_context_create(0); + pcre2_set_depth_limit(matchContext, 1000); pcre2_match_data* matchData = pcre2_match_data_create_from_pattern(re, 0); int result = pcre2_match(re, (PCRE2_SPTR8)input.c_str() + startOffset, input.size() - startOffset, 0, matchFlags, matchData, matchContext); @@ -2880,8 +2880,8 @@ string SREReplace(const string& regExp, const string& input, const string& repla if (!re) { STHROW("Bad regex: " + regExp); } - pcre2_match_context* matchContext = pcre2_match_context_create(0); - pcre2_set_depth_limit(matchContext, 1000); + pcre2_match_context* matchContext = pcre2_match_context_create(0); + pcre2_set_depth_limit(matchContext, 1000); for (int i = 0; i < 2; i++) { int result = pcre2_substitute(re, (PCRE2_SPTR8)input.c_str(), input.size(), 0, substituteFlags, 0, matchContext, (PCRE2_SPTR8)replacement.c_str(), replacement.size(), (PCRE2_UCHAR*)output, &outSize); if (i == 0 && result == PCRE2_ERROR_NOMEMORY) { @@ -3197,4 +3197,3 @@ SString& SString::operator=(const bool from) { string::operator=(from ? "true" : "false"); return *this; } - diff --git a/libstuff/libstuff.h b/libstuff/libstuff.h index c0aec3038..b2e246d30 100644 --- a/libstuff/libstuff.h +++ b/libstuff/libstuff.h @@ -240,7 +240,9 @@ string addLogParams(const string& message, const map& params = { #define SSYSLOG(_PRI_, _MSG_, ...) \ do { \ if (_g_SLogMask & (1 << (_PRI_))) { \ - const string s = addLogParams(_MSG_, ##__VA_ARGS__); \ + ostringstream __out; \ + __out << _MSG_; \ + const string s = addLogParams(__out.str(), ##__VA_ARGS__); \ const string prefix = SWHEREAMI; \ for (size_t i = 0; i < s.size(); i += 7168) { \ (*SSyslogFunc)(_PRI_, "%s", (prefix + s.substr(i, 7168)).c_str()); \ @@ -249,42 +251,14 @@ string addLogParams(const string& message, const map& params = { } while (false) #define SLOGPREFIX "" -#define SDEBUG(_MSG_, ...) \ - do { \ - stringstream ss; \ - ss << "[dbug] " << SLOGPREFIX << _MSG_; \ - SSYSLOG(LOG_INFO, ss.str(), ##__VA_ARGS__); \ - } while (false) -#define SINFO(_MSG_, ...) \ - do { \ - stringstream ss; \ - ss << "[info] " << SLOGPREFIX << _MSG_; \ - SSYSLOG(LOG_INFO, ss.str(), ##__VA_ARGS__); \ - } while (false) -#define SHMMM(_MSG_, ...) \ - do { \ - stringstream ss; \ - ss << "[hmmm] " << SLOGPREFIX << _MSG_; \ - SSYSLOG(LOG_INFO, ss.str(), ##__VA_ARGS__); \ - } while (false) -#define SWARN(_MSG_, ...) \ - do { \ - stringstream ss; \ - ss << "[warn] " << SLOGPREFIX << _MSG_; \ - SSYSLOG(LOG_INFO, ss.str(), ##__VA_ARGS__); \ - } while (false) -#define SALERT(_MSG_, ...) \ - do { \ - stringstream ss; \ - ss << "[alrt] " << SLOGPREFIX << _MSG_; \ - SSYSLOG(LOG_INFO, ss.str(), ##__VA_ARGS__); \ - } while (false) - -#define SERROR(_MSG_) \ +#define SDEBUG(_MSG_, ...) SSYSLOG(LOG_DEBUG, "[dbug] " << SLOGPREFIX << _MSG_, ##__VA_ARGS__) +#define SINFO(_MSG_, ...) SSYSLOG(LOG_INFO, "[info] " << SLOGPREFIX << _MSG_, ##__VA_ARGS__) +#define SHMMM(_MSG_, ...) SSYSLOG(LOG_NOTICE, "[hmmm] " << SLOGPREFIX << _MSG_, ##__VA_ARGS__) +#define SWARN(_MSG_, ...) SSYSLOG(LOG_WARNING, "[warn] " << SLOGPREFIX << _MSG_, ##__VA_ARGS__) +#define SALERT(_MSG_, ...) SSYSLOG(LOG_ALERT, "[alrt] " << SLOGPREFIX << _MSG_, ##__VA_ARGS__) +#define SERROR(_MSG_, ...) \ do { \ - stringstream ss; \ - ss << "[eror] " << SLOGPREFIX << _MSG_; \ - SSYSLOG(LOG_ERR, ss.str(), map()); \ + SSYSLOG(LOG_ERR, "[eror] " << SLOGPREFIX << _MSG_, ##__VA_ARGS__); \ SLogStackTrace(); \ abort(); \ } while (false) From 6698dcec17aec702fe57607328b4ee438f460dca Mon Sep 17 00:00:00 2001 From: Florent De Neve Date: Fri, 27 Sep 2024 16:29:39 +0200 Subject: [PATCH 4/7] remove comment --- libstuff/libstuff.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/libstuff/libstuff.h b/libstuff/libstuff.h index b2e246d30..523981bdc 100644 --- a/libstuff/libstuff.h +++ b/libstuff/libstuff.h @@ -230,9 +230,6 @@ void SSyslogSocketDirect(int priority, const char* format, ...); // Atomic pointer to the syslog function that we'll actually use. Easy to change to `syslog` or `SSyslogSocketDirect`. extern atomic SSyslogFunc; -//template -//string addLogParams(const string& message, const map params); - string addLogParams(const string& message, const map& params = {}); // **NOTE: rsyslog default max line size is 8k bytes. We split on 7k byte boundaries in order to fit the syslog line prefix and the expanded \r\n to #015#012 From 6c9d76ab4c14626cce490d0ae72227628a29a4e2 Mon Sep 17 00:00:00 2001 From: Florent De Neve Date: Mon, 30 Sep 2024 11:25:25 +0100 Subject: [PATCH 5/7] Early return --- libstuff/SLog.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/libstuff/SLog.cpp b/libstuff/SLog.cpp index f17ccdb6e..b9d13ee30 100644 --- a/libstuff/SLog.cpp +++ b/libstuff/SLog.cpp @@ -41,16 +41,17 @@ void SLogStackTrace(int level) { string addLogParams(const string& message, const map& params) { string result = message; + if (params.empty()) { + return result; + } - if (!params.empty()) { - result += " ~~ "; - for (size_t i = 0; i < params.size(); ++i) { - if (i > 0) { - result += " "; - } - const auto& param = *next(params.begin(), i); - result += param.first + ": '" + param.second + "'"; + result += " ~~ "; + for (size_t i = 0; i < params.size(); ++i) { + if (i > 0) { + result += " "; } + const auto& param = *next(params.begin(), i); + result += param.first + ": '" + param.second + "'"; } return result; From 8f7879065f798ecca5d0093ba1469986182d8381 Mon Sep 17 00:00:00 2001 From: Florent De Neve Date: Thu, 10 Oct 2024 10:45:20 -0400 Subject: [PATCH 6/7] Make 'message' an rvalue reference --- libstuff/SLog.cpp | 13 ++++++------- libstuff/libstuff.h | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/libstuff/SLog.cpp b/libstuff/SLog.cpp index b9d13ee30..99ce1da20 100644 --- a/libstuff/SLog.cpp +++ b/libstuff/SLog.cpp @@ -39,20 +39,19 @@ void SLogStackTrace(int level) { } } -string addLogParams(const string& message, const map& params) { - string result = message; +string addLogParams(string&& message, const map& params) { if (params.empty()) { - return result; + return message; } - result += " ~~ "; + message += " ~~ "; for (size_t i = 0; i < params.size(); ++i) { if (i > 0) { - result += " "; + message += " "; } const auto& param = *next(params.begin(), i); - result += param.first + ": '" + param.second + "'"; + message += param.first + ": '" + param.second + "'"; } - return result; + return message; } diff --git a/libstuff/libstuff.h b/libstuff/libstuff.h index ca1c04815..eb507bf7e 100644 --- a/libstuff/libstuff.h +++ b/libstuff/libstuff.h @@ -230,7 +230,7 @@ void SSyslogSocketDirect(int priority, const char* format, ...); // Atomic pointer to the syslog function that we'll actually use. Easy to change to `syslog` or `SSyslogSocketDirect`. extern atomic SSyslogFunc; -string addLogParams(const string& message, const map& params = {}); +string addLogParams(string&& message, const map& params = {}); // **NOTE: rsyslog default max line size is 8k bytes. We split on 7k byte boundaries in order to fit the syslog line prefix and the expanded \r\n to #015#012 #define SWHEREAMI SThreadLogPrefix + "(" + basename((char*)__FILE__) + ":" + SToStr(__LINE__) + ") " + __FUNCTION__ + " [" + SThreadLogName + "] " From 2cff42a9dd79132862889185bda1383a157d5d9e Mon Sep 17 00:00:00 2001 From: Florent De Neve Date: Thu, 10 Oct 2024 10:48:03 -0400 Subject: [PATCH 7/7] Unique out a warn that needs to be unique --- BedrockServer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 13b6fd3b4..8fbd245a9 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -742,9 +742,9 @@ void BedrockServer::runCommand(unique_ptr&& _command, bool isBlo auto _clusterMessengerCopy = _clusterMessenger; bool result = _clusterMessengerCopy->runOnPeer(*command, false); if (result) { - SINFO("Synchronizing while accepting commands, so forwarded " << command->request.methodLine << " to peer successfully"); + SINFO("Synchronizing while accepting commands; successfully forwarded the command to peer", {{"command", command->request.methodLine}}); } else { - SWARN("Synchronizing while accepting commands, so forwarded " << command->request.methodLine << " to peer, but failed."); + SWARN("Synchronizing while accepting commands, but failed to forward the command to peer.", {{"command", command->request.methodLine}}); } }