Merge pull request #1875 from Expensify/flo_loggingParams

Update logging macros to accept a map of parameters
Expensify · Oct 10, 2024 · f519c2f · f519c2f
2 parents f440475 + 2cff42a
commit f519c2f
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 17 deletions.
diff --git a/BedrockServer.cpp b/BedrockServer.cpp
@@ -742,9 +742,9 @@ void BedrockServer::runCommand(unique_ptr<BedrockCommand>&& _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}});
         }
     }
 

diff --git a/libstuff/SLog.cpp b/libstuff/SLog.cpp
@@ -38,3 +38,20 @@ void SLogStackTrace(int level) {
         }
     }
 }
+
+string addLogParams(string&& message, const map<string, string>& params) {
+    if (params.empty()) {
+        return message;
+    }
+
+    message += " ~~ ";
+    for (size_t i = 0; i < params.size(); ++i) {
+        if (i > 0) {
+            message += " ";
+        }
+        const auto& param = *next(params.begin(), i);
+        message += param.first + ": '" + param.second + "'";
+    }
+
+    return message;
+}
diff --git a/libstuff/libstuff.cpp b/libstuff/libstuff.cpp
@@ -2823,8 +2823,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);
@@ -2882,8 +2882,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) {

diff --git a/libstuff/libstuff.h b/libstuff/libstuff.h
@@ -230,14 +230,16 @@ 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<void (*)(int priority, const char *format, ...)> SSyslogFunc;
 
+string addLogParams(string&& message, const map<string, string>& 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 + "] "
-#define SSYSLOG(_PRI_, _MSG_)                                                   \
+#define SSYSLOG(_PRI_, _MSG_, ...)                                              \
     do {                                                                        \
         if (_g_SLogMask & (1 << (_PRI_))) {                                     \
             ostringstream __out;                                                \
-            __out << _MSG_ << endl;                                             \
-            const string s = __out.str();                                       \
+            __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()); \
@@ -246,14 +248,14 @@ extern atomic<void (*)(int priority, const char *format, ...)> 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 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 {                                                    \
-        SSYSLOG(LOG_ERR, "[eror] " << SLOGPREFIX << _MSG_); \
+        SSYSLOG(LOG_ERR, "[eror] " << SLOGPREFIX << _MSG_, ##__VA_ARGS__); \
         SLogStackTrace();                                   \
         abort();                                            \
     } while (false)

diff --git a/sqlitecluster/SQLite.cpp b/sqlitecluster/SQLite.cpp
@@ -460,7 +460,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));