Make the loglevel persistent by moving the LOGGER table from the LOGL…

…EVEL DB to the CONFIG DB (#687)

- What I did

Add the ability to the user to save the loglevel and make it persistent to reboot.

- How I did it

Move the logger tables from the LOGLEVEL DB to the CONFIG DB

- How to verify it

Change the orchagent loglevel (for example) -> swssloglevel -c orchagent -l DEBUG
Save the loglevel -> run config save
Reboot
Verify that the orchagent log level is still DEBUG ->run run redis-cli -n 4 hgetall "LOGGER|orchagent"

BUILD

@@ -4,7 +4,7 @@ exports_files(["LICENSE"])
 
 cc_library(
     name = "common",
-    srcs = glob(["common/*.cpp"], exclude=["common/loglevel.cpp"]),
+    srcs = glob(["common/*.cpp"], exclude=["common/loglevel.cpp", "common/loglevel_util.cpp"]),
     hdrs = glob([
         "common/*.h",
         "common/*.hpp",

common/Makefile.am

@@ -79,7 +79,9 @@ libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CF
 libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CPPFLAGS) $(CODE_COVERAGE_CPPFLAGS)
 libswsscommon_la_LIBADD = -lpthread $(LIBNL_LIBS) $(CODE_COVERAGE_LIBS) -lzmq -lboost_serialization -luuid -lyang
 
-swssloglevel_SOURCES = loglevel.cpp
+swssloglevel_SOURCES =  \
+	loglevel.cpp        \
+	loglevel_util.cpp	
 
 swssloglevel_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS)
 swssloglevel_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CODE_COVERAGE_CPPFLAGS)

common/logger.cpp

@@ -11,7 +11,7 @@
 #include "schema.h"
 #include "select.h"
 #include "dbconnector.h"
-#include "consumerstatetable.h"
+#include "subscriberstatetable.h"
 #include "producerstatetable.h"
 
 using namespace swss;
@@ -83,6 +83,7 @@ void Logger::swssPrioNotify(const std::string& component, const std::string& pri
     }
     else
     {
+        SWSS_LOG_DEBUG("Changing logger minPrio to %s", prioStr.c_str());
         logger.m_minPrio = priorityStringMap.at(prioStr);
     }
 }
@@ -120,41 +121,29 @@ void Logger::linkToDbWithOutput(
     // Initialize internal DB with observer
     logger.m_settingChangeObservers.insert(std::make_pair(dbName, std::make_pair(prioNotify, outputNotify)));
 
-    DBConnector db("LOGLEVEL_DB", 0);
+    DBConnector db("CONFIG_DB", 0);
+    swss::Table table(&db, CFG_LOGGER_TABLE_NAME);
+    SWSS_LOG_DEBUG("Component %s register to logger", dbName.c_str());
 
-    std::string key = dbName + ":" + dbName;
     std::string prio, output;
     bool doUpdate = false;
-    auto prioPtr = db.hget(key, DAEMON_LOGLEVEL);
-    auto outputPtr = db.hget(key, DAEMON_LOGOUTPUT);
-
-    if (prioPtr == nullptr)
+    if(!table.hget(dbName, DAEMON_LOGLEVEL, prio))
     {
         prio = defPrio;
         doUpdate = true;
     }
-    else
-    {
-        prio = *prioPtr;
-    }
-
-    if (outputPtr == nullptr)
+    if(!table.hget(dbName, DAEMON_LOGOUTPUT, output))
     {
         output = defOutput;
         doUpdate = true;
-
-    }
-    else
-    {
-        output = *outputPtr;
     }
 
     if (doUpdate)
     {
-        ProducerStateTable table(&db, dbName);
         FieldValueTuple fvLevel(DAEMON_LOGLEVEL, prio);
         FieldValueTuple fvOutput(DAEMON_LOGOUTPUT, output);
         std::vector<FieldValueTuple>fieldValues = { fvLevel, fvOutput };
+        SWSS_LOG_DEBUG("Set %s loglevel to %s", dbName.c_str() , prio.c_str());
         table.set(dbName, fieldValues);
     }
 
@@ -196,24 +185,14 @@ Logger::Priority Logger::getMinPrio()
 void Logger::settingThread()
 {
     Select select;
-    DBConnector db("LOGLEVEL_DB", 0);
-    std::map<std::string, std::shared_ptr<ConsumerStateTable>> selectables;
+    DBConnector db("CONFIG_DB", 0);
+    std::map<std::string, std::shared_ptr<SubscriberStateTable>> selectables;
+    auto table = std::make_shared<SubscriberStateTable>(&db, CFG_LOGGER_TABLE_NAME);
+    selectables.emplace(CFG_LOGGER_TABLE_NAME, table);
+    select.addSelectable(table.get());
 
     while (m_runSettingThread)
     {
-        if (selectables.size() < m_settingChangeObservers.size())
-        {
-            for (const auto& i : m_settingChangeObservers.getCopy())
-            {
-                const std::string& dbName = i.first;
-                if (selectables.find(dbName) == selectables.end())
-                {
-                    auto table = std::make_shared<ConsumerStateTable>(&db, dbName);
-                    selectables.emplace(dbName, table);
-                    select.addSelectable(table.get());
-                }
-            }
-        }
 
         Selectable *selectable = nullptr;
 
@@ -233,14 +212,14 @@ void Logger::settingThread()
         }
 
         KeyOpFieldsValuesTuple koValues;
-        ConsumerStateTable *consumerStateTable = NULL;
-        consumerStateTable = dynamic_cast<ConsumerStateTable *>(selectable);
-        if (consumerStateTable == NULL)
+        SubscriberStateTable *subscriberStateTable = NULL;
+        subscriberStateTable = dynamic_cast<SubscriberStateTable *>(selectable);
+        if (subscriberStateTable == NULL)
         {
             SWSS_LOG_ERROR("dynamic_cast returned NULL");
             break;
         }
-        consumerStateTable->pop(koValues);
+        subscriberStateTable->pop(koValues);
         std::string key = kfvKey(koValues), op = kfvOp(koValues);
 
         if (op != SET_COMMAND || !m_settingChangeObservers.contains(key))

common/logger.h

@@ -24,6 +24,9 @@ namespace swss {
 
 #define SWSS_LOG_THROW(MSG, ...)       swss::Logger::getInstance().wthrow(swss::Logger::SWSS_ERROR,  ":- %s: " MSG, __FUNCTION__, ##__VA_ARGS__)
 
+static constexpr const char * const DAEMON_LOGLEVEL = "LOGLEVEL";
+static constexpr const char * const DAEMON_LOGOUTPUT = "LOGOUTPUT";
+
 void err_exit(const char *fn, int ln, int e, const char *fmt, ...)
 #ifdef __GNUC__
         __attribute__ ((format (printf, 4, 5)))
@@ -46,6 +49,7 @@ void err_exit(const char *fn, int ln, int e, const char *fmt, ...)
 class Logger
 {
 public:
+
     enum Priority
     {
         SWSS_EMERG,

common/loglevel.cpp

@@ -5,6 +5,7 @@
 #include <algorithm>
 #include <unistd.h>
 #include "schema.h"
+#include "loglevel.h"
 #include "logger.h"
 #include "dbconnector.h"
 #include "producerstatetable.h"
@@ -26,21 +27,21 @@ using namespace swss;
               << "\t -c\tcomponent name in DB for which loglevel is applied (provided with -l)" << std::endl
               << "\t -a\tapply loglevel to all components (provided with -l)" << std::endl
               << "\t -s\tapply loglevel for SAI api component (equivalent to adding prefix \"SAI_API_\" to component)" << std::endl
-              << "\t -p\tprint components registered in DB for which setting can be applied" << std::endl << std::endl
+              << "\t -p\tprint components registered in DB for which setting can be applied" << std::endl
+              << "\t -d\treturn all components to default loglevel" << std::endl<< std::endl
               << "Examples:" << std::endl
               << "\t" << program << " -l NOTICE -c orchagent # set orchagent severity level to NOTICE" << std::endl
               << "\t" << program << " -l SAI_LOG_LEVEL_ERROR -s -c SWITCH # set SAI_API_SWITCH severity to ERROR" << std::endl
-              << "\t" << program << " -l SAI_LOG_LEVEL_DEBUG -s -a # set all SAI_API_* severity to DEBUG" << std::endl;
+              << "\t" << program << " -l DEBUG -a # set all not SAI components severity to DEBUG" << std::endl
+              << "\t" << program << " -l SAI_LOG_LEVEL_DEBUG -s -a # set all SAI_API_* severity to DEBUG" << std::endl
+              << "\t" << program << " -d # return all components to default loglevel" << std::endl;
 
     exit(status);
 }
 
-void setLoglevel(DBConnector& db, const std::string& component, const std::string& loglevel)
+void setLoglevel(swss::Table& logger_tbl, const std::string& component, const std::string& loglevel)
 {
-    ProducerStateTable table(&db, component);
-    FieldValueTuple fv(DAEMON_LOGLEVEL, loglevel);
-    std::vector<FieldValueTuple>fieldValues = { fv };
-    table.set(component, fieldValues);
+    logger_tbl.hset(component, "LOGLEVEL",loglevel);
 }
 
 bool validateSaiLoglevel(const std::string &prioStr)
@@ -57,11 +58,6 @@ bool validateSaiLoglevel(const std::string &prioStr)
     return std::find(saiPrios.begin(), saiPrios.end(), prioStr) != saiPrios.end();
 }
 
-bool filterOutKeysets(const std::string& key)
-{
-    return key.find("_KEY_SET") != std::string::npos;
-}
-
 bool filterOutSaiKeys(const std::string& key)
 {
     return key.find("SAI_API_") != std::string::npos;
@@ -72,14 +68,35 @@ bool filterSaiKeys(const std::string& key)
     return key.find("SAI_API_") == std::string::npos;
 }
 
-int main(int argc, char **argv)
+std::vector<std::string> get_sai_keys(std::vector<std::string> keys)
+{
+    keys.erase(std::remove_if(keys.begin(), keys.end(), filterSaiKeys), keys.end());
+    return keys;
+}
+
+std::vector<std::string> get_no_sai_keys(std::vector<std::string> keys)
+{
+    keys.erase(std::remove_if(keys.begin(), keys.end(), filterOutSaiKeys), keys.end());
+    return keys;
+}
+
+void setAllLoglevel(swss::Table& logger_tbl, std::vector<std::string> components, std::string loglevel)
+{
+    for (const auto& component : components)
+    {
+        setLoglevel(logger_tbl, component, loglevel);
+    }
+    SWSS_LOG_DEBUG("All components are with %s loglevel", loglevel.c_str());
+}
+
+int swssloglevel(int argc, char** argv)
 {
     int opt;
-    bool applyToAll = false, print = false;
+    bool applyToAll = false, print = false, default_loglevel_opt = false;
     std::string prefix = "", component, loglevel;
     auto exitWithUsage = std::bind(usage, argv[0], std::placeholders::_1, std::placeholders::_2);
 
-    while ((opt = getopt (argc, argv, "c:l:saph")) != -1)
+    while ( (opt = getopt (argc, argv, "c:l:sapdh")) != -1)
     {
         switch(opt)
         {
@@ -98,6 +115,9 @@ int main(int argc, char **argv)
             case 'p':
                 print = true;
                 break;
+            case 'd':
+                default_loglevel_opt = true;
+                break;
             case 'h':
                 exitWithUsage(EXIT_SUCCESS, "");
                 break;
@@ -106,20 +126,10 @@ int main(int argc, char **argv)
         }
     }
 
-    DBConnector db("LOGLEVEL_DB", 0);
-    auto keys = db.keys("*");
-    for (auto& key : keys)
-    {
-        size_t colonPos = key.find(':');
-        if (colonPos == std::string::npos)
-        {
-            continue;
-        }
-
-        key = key.substr(0, colonPos);
-    }
-    // Ignore autogenerated keysets
-    keys.erase(std::remove_if(keys.begin(), keys.end(), filterOutKeysets), keys.end());
+    DBConnector config_db("CONFIG_DB", 0);
+    swss::Table logger_tbl(&config_db, CFG_LOGGER_TABLE_NAME);
+    std::vector<std::string> keys;
+    logger_tbl.getKeys(keys);
 
     if (print)
     {
@@ -133,16 +143,15 @@ int main(int argc, char **argv)
         std::sort(keys.begin(), keys.end());
         for (const auto& key : keys)
         {
-            const auto redis_key = std::string(key).append(":").append(key);
-            auto level = db.hget(redis_key, DAEMON_LOGLEVEL);
-            if (nullptr == level)
+            std::string level;
+            if (!(logger_tbl.hget(key, DAEMON_LOGLEVEL, level)))
             {
                 std::cerr << std::left << std::setw(30) << key << "Unknown log level" << std::endl;
                 errorCount ++;
             }
             else
             {
-                std::cout << std::left << std::setw(30) << key << *level << std::endl;
+                std::cout << std::left << std::setw(30) << key << level << std::endl;
             }
         }
 
@@ -152,6 +161,14 @@ int main(int argc, char **argv)
         return (EXIT_SUCCESS);
     }
 
+    if(default_loglevel_opt)
+    {
+        std::vector<std::string> sai_keys = get_sai_keys(keys);
+        std::vector<std::string> no_sai_keys = get_no_sai_keys(keys);
+        setAllLoglevel(logger_tbl,no_sai_keys, std::string(DEFAULT_LOGLEVEL));
+        setAllLoglevel(logger_tbl,sai_keys, std::string(SAI_DEFAULT_LOGLEVEL));
+        return (EXIT_SUCCESS);
+    }
     if ((prefix == "SAI_API_") && !validateSaiLoglevel(loglevel))
     {
         exitWithUsage(EXIT_FAILURE, "Invalid SAI loglevel value");
@@ -177,11 +194,7 @@ int main(int argc, char **argv)
             keys.erase(std::remove_if(keys.begin(), keys.end(), filterOutSaiKeys), keys.end());
         }
 
-        for (const auto& key : keys)
-        {
-            setLoglevel(db, key, loglevel);
-        }
-
+        setAllLoglevel(logger_tbl, keys, loglevel);
         exit(EXIT_SUCCESS);
     }
 
@@ -190,8 +203,7 @@ int main(int argc, char **argv)
     {
         exitWithUsage(EXIT_FAILURE, "Component not present in DB");
     }
-
-    setLoglevel(db, component, loglevel);
+    setLoglevel(logger_tbl, component, loglevel);
 
     return EXIT_SUCCESS;
 }

common/loglevel.h

@@ -0,0 +1,9 @@
+#ifndef __LOGLEVEL_H__
+#define __LOGLEVEL_H__
+
+#define DEFAULT_LOGLEVEL "NOTICE"
+#define SAI_DEFAULT_LOGLEVEL "SAI_LOG_LEVEL_NOTICE"
+
+int swssloglevel(int argc, char** argv);
+
+#endif /* __LOGLEVEL_H__ */

common/loglevel_util.cpp

@@ -0,0 +1,6 @@
+#include "loglevel.h"
+
+int main(int argc, char** argv)
+{
+    swssloglevel(argc,argv);
+}

common/schema.h

@@ -216,11 +216,6 @@ namespace swss {
 #define COUNTERS_EVENTS_MISSED_CACHE        "missed_to_cache"
 #define COUNTERS_EVENTS_LATENCY             "latency_in_ms"
 
-/***** LOGLEVEL DATABASE *****/
-
-#define DAEMON_TABLE_NAME "DAEMON_TABLE"
-#define DAEMON_LOGLEVEL "LOGLEVEL"
-#define DAEMON_LOGOUTPUT "LOGOUTPUT"
 
 /***** FLEX COUNTER DATABASE *****/
 
@@ -420,6 +415,7 @@ namespace swss {
 #define CFG_DHCP_TABLE                              "DHCP_RELAY"
 
 #define CFG_FLOW_COUNTER_ROUTE_PATTERN_TABLE_NAME   "FLOW_COUNTER_ROUTE_PATTERN"
+#define CFG_LOGGER_TABLE_NAME                       "LOGGER"
 
 /***** STATE DATABASE *****/
 

tests/Makefile.am

@@ -28,6 +28,8 @@ tests_SOURCES = redis_ut.cpp                \
                 warm_restart_ut.cpp         \
                 redis_multi_db_ut.cpp       \
                 logger_ut.cpp               \
+                ../common/loglevel.cpp      \
+                loglevel_ut.cpp             \
                 redis_multi_ns_ut.cpp       \
                 fdb_flush.cpp               \
                 stringutility_ut.cpp        \