From 7206d0f6a23f435fd41d1f0447e94a7703f20a24 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Thu, 11 Aug 2022 09:15:40 +0000 Subject: [PATCH 01/23] Start update swsssloglevel script, Add -d option to return all component to default, change LOGLEVEL DB to CONFIG DB --- common/loglevel.cpp | 109 ++++++++++++++++++++++++++++++-------------- 1 file changed, 75 insertions(+), 34 deletions(-) diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 1be85c099..897014378 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -26,21 +26,26 @@ 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 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::vectorfieldValues = { fv }; - table.set(component, fieldValues); + + SWSS_LOG_NOTICE("EDEN start set log level function"); + SWSS_LOG_NOTICE("EDEN component: %s, loglevel: %s", component.c_str(), loglevel.c_str()); + + logger_tbl.hset(component, "LOGLEVEL",loglevel); + SWSS_LOG_NOTICE("EDEN end setLoglevel"); + } bool validateSaiLoglevel(const std::string &prioStr) @@ -57,11 +62,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 +72,40 @@ bool filterSaiKeys(const std::string& key) return key.find("SAI_API_") == std::string::npos; } +std::vector get_sai_keys(std::vector keys) +{ + keys.erase(std::remove_if(keys.begin(), keys.end(), filterSaiKeys), keys.end()); + return keys; +} + +std::vector get_no_sai_keys(std::vector keys) +{ + keys.erase(std::remove_if(keys.begin(), keys.end(), filterOutSaiKeys), keys.end()); + return keys; +} +//TODO change to MSET command +void setAllLoglevel(swss::Table& logger_tbl, std::vector components,std::string loglevel) +{ + SWSS_LOG_NOTICE("EDEN start setAllLoglevel"); + for (const auto& component : components) + { + SWSS_LOG_NOTICE("EDEN in setAllLoglevel component: %s, loglevel: %s", component.c_str(), loglevel.c_str()); + setLoglevel(logger_tbl, component, loglevel); + } + SWSS_LOG_NOTICE("EDEN end setAllLoglevel"); +} + int main(int argc, char **argv) { + SWSS_LOG_NOTICE("EDEN start main"); + std::string key_prefix = "LOGGER|"; + int opt; - bool applyToAll = false, print = false; + bool applyToAll = false, print = false, default_loglevel = 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) + //todo what is th : in the thired variable? + while ((opt = getopt (argc, argv, "c:l:sapdh")) != -1) { switch(opt) { @@ -98,6 +124,9 @@ int main(int argc, char **argv) case 'p': print = true; break; + case 'd': + default_loglevel = true; + break; case 'h': exitWithUsage(EXIT_SUCCESS, ""); break; @@ -105,21 +134,14 @@ int main(int argc, char **argv) exitWithUsage(EXIT_FAILURE, "Invalid option"); } } + SWSS_LOG_NOTICE("EDEN cearting config db connector"); - 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); + //TODO: change the "logger" to CFG_LOGGER_TABLE_NAME + std::string table_name = "LOGGER"; + swss::Table logger_tbl(&config_db, table_name); + std::vector keys; + logger_tbl.getKeys(keys); if (print) { @@ -133,8 +155,9 @@ 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); + //todo: remove key_prefix and replace with CFG_LOGGER_TABLE_NAME+ "|" + const auto redis_key = key_prefix.append(key); + auto level = config_db.hget(redis_key, DAEMON_LOGLEVEL); if (nullptr == level) { std::cerr << std::left << std::setw(30) << key << "Unknown log level" << std::endl; @@ -152,6 +175,23 @@ int main(int argc, char **argv) return (EXIT_SUCCESS); } + if(default_loglevel) + { + //check if the command right I mean there is no loglevel or somthing like this + //todo change the apply all??? + std::vector sai_keys = get_sai_keys(keys); + std::vector no_sai_keys = get_no_sai_keys(keys); + + + setAllLoglevel(logger_tbl,no_sai_keys, "NOTICE"); + //todo add the default as constant + + setAllLoglevel(logger_tbl,sai_keys, "SAI_LOG_LEVEL_NOTICE"); + + return (EXIT_SUCCESS); + + } + if ((prefix == "SAI_API_") && !validateSaiLoglevel(loglevel)) { exitWithUsage(EXIT_FAILURE, "Invalid SAI loglevel value"); @@ -176,10 +216,10 @@ int main(int argc, char **argv) { keys.erase(std::remove_if(keys.begin(), keys.end(), filterOutSaiKeys), keys.end()); } - +//todo change to setAllloglevel ? for (const auto& key : keys) { - setLoglevel(db, key, loglevel); + setLoglevel(logger_tbl, key, loglevel); } exit(EXIT_SUCCESS); @@ -190,8 +230,9 @@ int main(int argc, char **argv) { exitWithUsage(EXIT_FAILURE, "Component not present in DB"); } + SWSS_LOG_NOTICE("EDEN calling set log level"); - setLoglevel(db, component, loglevel); + setLoglevel(logger_tbl, component, loglevel); return EXIT_SUCCESS; } From cdf055e9aaf7a3576603e5cab889f01eb093011b Mon Sep 17 00:00:00 2001 From: EdenGri Date: Wed, 17 Aug 2022 08:12:25 +0000 Subject: [PATCH 02/23] change the logger class to use CONFIG DB instead of LOGLEVEL DB --- common/logger.cpp | 51 ++++++++++++++++++++------------------- common/loglevel.cpp | 15 +++--------- common/schema.h | 5 +++- common/sonicv2connector.h | 3 +++ 4 files changed, 37 insertions(+), 37 deletions(-) diff --git a/common/logger.cpp b/common/logger.cpp index de3e955de..82c9405f9 100644 --- a/common/logger.cpp +++ b/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; @@ -119,10 +119,10 @@ 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); - - std::string key = dbName + ":" + dbName; + SWSS_LOG_NOTICE("EDEN in linkToDBNative func dbName:%s", dbName.c_str()); + DBConnector db("CONFIG_DB", 0); + //TODO: change to table name from schema + std::string key = "LOGGER|" + dbName; std::string prio, output; bool doUpdate = false; auto prioPtr = db.hget(key, DAEMON_LOGLEVEL); @@ -130,12 +130,14 @@ void Logger::linkToDbWithOutput( if (prioPtr == nullptr) { + SWSS_LOG_NOTICE("EDEN dbName:%s, prioPtr is NULL!!!!", dbName.c_str()); prio = defPrio; doUpdate = true; } else { prio = *prioPtr; + SWSS_LOG_NOTICE("EDEN dbName:%s, prioPtr:%s", dbName.c_str(), prio.c_str()); } if (outputPtr == nullptr) @@ -149,9 +151,17 @@ void Logger::linkToDbWithOutput( output = *outputPtr; } + int intDoUpdate = 0; + if (doUpdate) + { + intDoUpdate = 1; + } + SWSS_LOG_NOTICE("EDEN dbName:%s, doUpdate:%d. if 1, doupdate=true", dbName.c_str(), intDoUpdate); if (doUpdate) { - ProducerStateTable table(&db, dbName); + //todo: change to table name + SWSS_LOG_NOTICE("EDEN dbName:%s, inside do update", dbName.c_str()); + swss::Table table(&db, CFG_LOGGER_TABLE_NAME); FieldValueTuple fvLevel(DAEMON_LOGLEVEL, prio); FieldValueTuple fvOutput(DAEMON_LOGOUTPUT, output); std::vectorfieldValues = { fvLevel, fvOutput }; @@ -193,27 +203,18 @@ Logger::Priority Logger::getMinPrio() return getInstance().m_minPrio; } + void Logger::settingThread() { Select select; - DBConnector db("LOGLEVEL_DB", 0); - std::map> selectables; + DBConnector db("CONFIG_DB", 0); + std::map> selectables; + auto table = std::make_shared(&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(&db, dbName); - selectables.emplace(dbName, table); - select.addSelectable(table.get()); - } - } - } Selectable *selectable = nullptr; @@ -233,14 +234,14 @@ void Logger::settingThread() } KeyOpFieldsValuesTuple koValues; - ConsumerStateTable *consumerStateTable = NULL; - consumerStateTable = dynamic_cast(selectable); - if (consumerStateTable == NULL) + SubscriberStateTable *subscriberStateTable = NULL; + subscriberStateTable = dynamic_cast(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)) diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 897014378..4e14e62fb 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -44,6 +44,7 @@ void setLoglevel(swss::Table& logger_tbl, const std::string& component, const st SWSS_LOG_NOTICE("EDEN component: %s, loglevel: %s", component.c_str(), loglevel.c_str()); logger_tbl.hset(component, "LOGLEVEL",loglevel); + SWSS_LOG_NOTICE("EDEN end setLoglevel"); } @@ -98,7 +99,6 @@ void setAllLoglevel(swss::Table& logger_tbl, std::vector components int main(int argc, char **argv) { SWSS_LOG_NOTICE("EDEN start main"); - std::string key_prefix = "LOGGER|"; int opt; bool applyToAll = false, print = false, default_loglevel = false; @@ -137,9 +137,7 @@ int main(int argc, char **argv) SWSS_LOG_NOTICE("EDEN cearting config db connector"); DBConnector config_db("CONFIG_DB", 0); - //TODO: change the "logger" to CFG_LOGGER_TABLE_NAME - std::string table_name = "LOGGER"; - swss::Table logger_tbl(&config_db, table_name); + swss::Table logger_tbl(&config_db, CFG_LOGGER_TABLE_NAME); std::vector keys; logger_tbl.getKeys(keys); @@ -155,8 +153,7 @@ int main(int argc, char **argv) std::sort(keys.begin(), keys.end()); for (const auto& key : keys) { - //todo: remove key_prefix and replace with CFG_LOGGER_TABLE_NAME+ "|" - const auto redis_key = key_prefix.append(key); + const auto redis_key = (std::string(CFG_LOGGER_TABLE_NAME).append(std::string("|"))).append(key); auto level = config_db.hget(redis_key, DAEMON_LOGLEVEL); if (nullptr == level) { @@ -216,12 +213,8 @@ int main(int argc, char **argv) { keys.erase(std::remove_if(keys.begin(), keys.end(), filterOutSaiKeys), keys.end()); } -//todo change to setAllloglevel ? - for (const auto& key : keys) - { - setLoglevel(logger_tbl, key, loglevel); - } + setAllLoglevel(logger_tbl,keys, loglevel); exit(EXIT_SUCCESS); } diff --git a/common/schema.h b/common/schema.h index 73fb3b94f..8d996e6a7 100644 --- a/common/schema.h +++ b/common/schema.h @@ -217,7 +217,7 @@ namespace swss { #define COUNTERS_EVENTS_LATENCY "latency_in_ms" /***** LOGLEVEL DATABASE *****/ - +//TODO: what to do with this fileds? #define DAEMON_TABLE_NAME "DAEMON_TABLE" #define DAEMON_LOGLEVEL "LOGLEVEL" #define DAEMON_LOGOUTPUT "LOGOUTPUT" @@ -421,6 +421,9 @@ namespace swss { #define CFG_FLOW_COUNTER_ROUTE_PATTERN_TABLE_NAME "FLOW_COUNTER_ROUTE_PATTERN" +#define CFG_LOGGER_TABLE_NAME "LOGGER" + + /***** STATE DATABASE *****/ #define STATE_SWITCH_CAPABILITY_TABLE_NAME "SWITCH_CAPABILITY_TABLE" diff --git a/common/sonicv2connector.h b/common/sonicv2connector.h index d700d926f..27d80bf3f 100644 --- a/common/sonicv2connector.h +++ b/common/sonicv2connector.h @@ -98,6 +98,9 @@ class SonicV2Connector_Native return super(SonicV2Connector, self).set(db_name, _hash, key, value, blocking) return super(SonicV2Connector, self).set(db_name, _hash, key, str(value), blocking) + + def del(self, db_name, key, blocking=False): + return super(SonicV2Connector, self).del(db_name,key, blocking) %} #endif } From 870fe0938ce3a891a41692822744acebdddfc444 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Wed, 17 Aug 2022 08:59:33 +0000 Subject: [PATCH 03/23] changing LOGGER string to constant --- common/logger.cpp | 3 ++- common/logger.h | 4 ++++ common/loglevel.cpp | 3 ++- common/schema.h | 5 ----- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/common/logger.cpp b/common/logger.cpp index 82c9405f9..43a222778 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -122,7 +122,8 @@ void Logger::linkToDbWithOutput( SWSS_LOG_NOTICE("EDEN in linkToDBNative func dbName:%s", dbName.c_str()); DBConnector db("CONFIG_DB", 0); //TODO: change to table name from schema - std::string key = "LOGGER|" + dbName; + + std::string key = CFG_LOGGER_TABLE_NAME + "|" + dbName; std::string prio, output; bool doUpdate = false; auto prioPtr = db.hget(key, DAEMON_LOGLEVEL); diff --git a/common/logger.h b/common/logger.h index 79fdec47f..07ffa9dd2 100644 --- a/common/logger.h +++ b/common/logger.h @@ -46,6 +46,10 @@ void err_exit(const char *fn, int ln, int e, const char *fmt, ...) class Logger { public: + +#define DAEMON_LOGLEVEL "LOGLEVEL" +#define DAEMON_LOGOUTPUT "LOGOUTPUT" + enum Priority { SWSS_EMERG, diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 4e14e62fb..7bdbb0f4b 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -151,9 +151,10 @@ int main(int argc, char **argv) } std::sort(keys.begin(), keys.end()); + std::string redis_key_prefix = CFG_LOGGER_TABLE_NAME + "|"; for (const auto& key : keys) { - const auto redis_key = (std::string(CFG_LOGGER_TABLE_NAME).append(std::string("|"))).append(key); + const auto redis_key = redis_key_prefix.append(key); auto level = config_db.hget(redis_key, DAEMON_LOGLEVEL); if (nullptr == level) { diff --git a/common/schema.h b/common/schema.h index 8d996e6a7..3c3524301 100644 --- a/common/schema.h +++ b/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 *****/ -//TODO: what to do with this fileds? -#define DAEMON_TABLE_NAME "DAEMON_TABLE" -#define DAEMON_LOGLEVEL "LOGLEVEL" -#define DAEMON_LOGOUTPUT "LOGOUTPUT" /***** FLEX COUNTER DATABASE *****/ From a055e5e23f435cd7061b9370215913e26c6584d0 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Sun, 21 Aug 2022 15:01:17 +0000 Subject: [PATCH 04/23] update --- common/logger.cpp | 18 ++++-------------- common/loglevel.cpp | 3 ++- common/sonicv2connector.cpp | 4 ++++ common/sonicv2connector.h | 5 +++-- 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/common/logger.cpp b/common/logger.cpp index 43a222778..0060563e1 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -119,11 +119,11 @@ void Logger::linkToDbWithOutput( // Initialize internal DB with observer logger.m_settingChangeObservers.insert(std::make_pair(dbName, std::make_pair(prioNotify, outputNotify))); - SWSS_LOG_NOTICE("EDEN in linkToDBNative func dbName:%s", dbName.c_str()); - DBConnector db("CONFIG_DB", 0); - //TODO: change to table name from schema - std::string key = CFG_LOGGER_TABLE_NAME + "|" + dbName; + DBConnector db("CONFIG_DB", 0); + //TODO: change to be in h file + std::string key_prefix(strcat(CFG_LOGGER_TABLE_NAME, "|")); + std::string key = key_prefix + dbName; std::string prio, output; bool doUpdate = false; auto prioPtr = db.hget(key, DAEMON_LOGLEVEL); @@ -131,16 +131,13 @@ void Logger::linkToDbWithOutput( if (prioPtr == nullptr) { - SWSS_LOG_NOTICE("EDEN dbName:%s, prioPtr is NULL!!!!", dbName.c_str()); prio = defPrio; doUpdate = true; } else { prio = *prioPtr; - SWSS_LOG_NOTICE("EDEN dbName:%s, prioPtr:%s", dbName.c_str(), prio.c_str()); } - if (outputPtr == nullptr) { output = defOutput; @@ -152,16 +149,9 @@ void Logger::linkToDbWithOutput( output = *outputPtr; } - int intDoUpdate = 0; - if (doUpdate) - { - intDoUpdate = 1; - } - SWSS_LOG_NOTICE("EDEN dbName:%s, doUpdate:%d. if 1, doupdate=true", dbName.c_str(), intDoUpdate); if (doUpdate) { //todo: change to table name - SWSS_LOG_NOTICE("EDEN dbName:%s, inside do update", dbName.c_str()); swss::Table table(&db, CFG_LOGGER_TABLE_NAME); FieldValueTuple fvLevel(DAEMON_LOGLEVEL, prio); FieldValueTuple fvOutput(DAEMON_LOGOUTPUT, output); diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 7bdbb0f4b..315b58916 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -151,7 +151,8 @@ int main(int argc, char **argv) } std::sort(keys.begin(), keys.end()); - std::string redis_key_prefix = CFG_LOGGER_TABLE_NAME + "|"; + //TODO: change to be in h file + std::string redis_key_prefix = strcat(CFG_LOGGER_TABLE_NAME, "|"); for (const auto& key : keys) { const auto redis_key = redis_key_prefix.append(key); diff --git a/common/sonicv2connector.cpp b/common/sonicv2connector.cpp index 59552cde2..8857d25be 100644 --- a/common/sonicv2connector.cpp +++ b/common/sonicv2connector.cpp @@ -103,6 +103,10 @@ int64_t SonicV2Connector_Native::del(const std::string& db_name, const std::stri { return m_dbintf.del(db_name, key, blocking); } +int64_t SonicV2Connector_Native::del2(const std::string& db_name, const std::string& key, bool blocking) +{ + return m_dbintf.del(db_name, key, blocking); +} void SonicV2Connector_Native::delete_all_by_pattern(const std::string& db_name, const std::string& pattern) { diff --git a/common/sonicv2connector.h b/common/sonicv2connector.h index 27d80bf3f..c565881bb 100644 --- a/common/sonicv2connector.h +++ b/common/sonicv2connector.h @@ -47,6 +47,7 @@ class SonicV2Connector_Native int64_t set(const std::string& db_name, const std::string& _hash, const std::string& key, const std::string& val, bool blocking=false); int64_t del(const std::string& db_name, const std::string& key, bool blocking=false); + int64_t del2(const std::string& db_name, const std::string& key, bool blocking=false); void delete_all_by_pattern(const std::string& db_name, const std::string& pattern); @@ -99,8 +100,8 @@ class SonicV2Connector_Native return super(SonicV2Connector, self).set(db_name, _hash, key, str(value), blocking) - def del(self, db_name, key, blocking=False): - return super(SonicV2Connector, self).del(db_name,key, blocking) + def del_table(self, db_name, key): + return super(SonicV2Connector, self).del2(db_name,key) %} #endif } From ba27e7d5062bd376f6c863ae6b26029945ce0a7d Mon Sep 17 00:00:00 2001 From: EdenGri Date: Tue, 23 Aug 2022 13:49:16 +0000 Subject: [PATCH 05/23] update unit tests logger of moving the Logger tables to the CONFIG DB --- tests/logger_ut.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/logger_ut.cpp b/tests/logger_ut.cpp index c17c9c262..34cc175d2 100644 --- a/tests/logger_ut.cpp +++ b/tests/logger_ut.cpp @@ -11,7 +11,7 @@ using namespace swss; void setLoglevel(DBConnector& db, const string& key, const string& loglevel) { - ProducerStateTable table(&db, key); + swss::Table table(&db, key); FieldValueTuple fv(DAEMON_LOGLEVEL, loglevel); std::vectorfieldValues = { fv }; table.set(key, fieldValues); @@ -19,7 +19,7 @@ void setLoglevel(DBConnector& db, const string& key, const string& loglevel) void clearLoglevelDB() { - DBConnector db("LOGLEVEL_DB", 0); + DBConnector db("CONFIG_DB", 0); RedisReply r(&db, "FLUSHALL", REDIS_REPLY_STATUS); r.checkStatusOK(); } @@ -31,7 +31,7 @@ void prioNotify(const string &component, const string &prioStr) void checkLoglevel(DBConnector& db, const string& key, const string& loglevel) { - string redis_key = key + ":" + key; + string redis_key = "LOGGER|" + key; auto level = db.hget(redis_key, DAEMON_LOGLEVEL); EXPECT_FALSE(level == nullptr); if (level != nullptr) @@ -42,7 +42,7 @@ void checkLoglevel(DBConnector& db, const string& key, const string& loglevel) TEST(LOGGER, loglevel) { - DBConnector db("LOGLEVEL_DB", 0); + DBConnector db("CONFIG_DB", 0); clearLoglevelDB(); string key1 = "table1", key2 = "table2", key3 = "table3"; From f7581182ab8a802ccc8195e77ad41e371438c253 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Wed, 24 Aug 2022 12:54:08 +0000 Subject: [PATCH 06/23] bug fix: change the key_prefix --- common/logger.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/logger.cpp b/common/logger.cpp index 0060563e1..211b35f0b 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -122,8 +122,10 @@ void Logger::linkToDbWithOutput( DBConnector db("CONFIG_DB", 0); //TODO: change to be in h file - std::string key_prefix(strcat(CFG_LOGGER_TABLE_NAME, "|")); + std::string key_prefix(CFG_LOGGER_TABLE_NAME); + key_prefix=+"|"; std::string key = key_prefix + dbName; + std::string prio, output; bool doUpdate = false; auto prioPtr = db.hget(key, DAEMON_LOGLEVEL); From 3d1e0f1cc79d0bfa04cf749680c96397102e2377 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Sun, 28 Aug 2022 15:21:48 +0000 Subject: [PATCH 07/23] add unit test for the swssloglevel script, update logger_ut to use CONFIG DB and not LOGLEVEL DB --- common/loglevel.cpp | 77 ++++++++++++++++++++++++------------------- tests/logger_ut.cpp | 4 +-- tests/loglevel_ut.cpp | 61 ++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 36 deletions(-) create mode 100644 tests/loglevel_ut.cpp diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 315b58916..a5a6d82ef 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -96,44 +96,45 @@ void setAllLoglevel(swss::Table& logger_tbl, std::vector components SWSS_LOG_NOTICE("EDEN end setAllLoglevel"); } -int main(int argc, char **argv) +int swssloglevel(int argc, char **argv) { SWSS_LOG_NOTICE("EDEN start main"); - int opt; - bool applyToAll = false, print = false, default_loglevel = false; - std::string prefix = "", component, loglevel; - auto exitWithUsage = std::bind(usage, argv[0], std::placeholders::_1, std::placeholders::_2); - //todo what is th : in the thired variable? - while ((opt = getopt (argc, argv, "c:l:sapdh")) != -1) - { - switch(opt) + int opt; + bool applyToAll = false, print = false, default_loglevel = false; + std::string prefix = "", component, loglevel; + auto exitWithUsage = std::bind(usage, argv[0], std::placeholders::_1, std::placeholders::_2); + //todo what is th : in the thired variable? + while ((opt = getopt (argc, argv, "c:l:sapdh")) != -1) { - case 'c': - component = optarg; - break; - case 'l': - loglevel = optarg; - break; - case 's': - prefix = "SAI_API_"; - break; - case 'a': - applyToAll = true; - break; - case 'p': - print = true; - break; - case 'd': - default_loglevel = true; - break; - case 'h': - exitWithUsage(EXIT_SUCCESS, ""); - break; - default: - exitWithUsage(EXIT_FAILURE, "Invalid option"); + switch(opt) + { + case 'c': + component = optarg; + break; + case 'l': + loglevel = optarg; + break; + case 's': + prefix = "SAI_API_"; + break; + case 'a': + applyToAll = true; + break; + case 'p': + print = true; + break; + case 'd': + default_loglevel = true; + break; + case 'h': + exitWithUsage(EXIT_SUCCESS, ""); + break; + default: + exitWithUsage(EXIT_FAILURE, "Invalid option"); + } } - } + SWSS_LOG_NOTICE("EDEN cearting config db connector"); DBConnector config_db("CONFIG_DB", 0); @@ -152,7 +153,8 @@ int main(int argc, char **argv) std::sort(keys.begin(), keys.end()); //TODO: change to be in h file - std::string redis_key_prefix = strcat(CFG_LOGGER_TABLE_NAME, "|"); + std::string key_prefix(CFG_LOGGER_TABLE_NAME); + key_prefix=+"|"; for (const auto& key : keys) { const auto redis_key = redis_key_prefix.append(key); @@ -231,3 +233,10 @@ int main(int argc, char **argv) return EXIT_SUCCESS; } + +int main(int argc, char **argv) +{ + + + return swssloglevel(argc, argv); +} diff --git a/tests/logger_ut.cpp b/tests/logger_ut.cpp index 34cc175d2..a4ccd6954 100644 --- a/tests/logger_ut.cpp +++ b/tests/logger_ut.cpp @@ -17,7 +17,7 @@ void setLoglevel(DBConnector& db, const string& key, const string& loglevel) table.set(key, fieldValues); } -void clearLoglevelDB() +void clearConfigDB() { DBConnector db("CONFIG_DB", 0); RedisReply r(&db, "FLUSHALL", REDIS_REPLY_STATUS); @@ -43,7 +43,7 @@ void checkLoglevel(DBConnector& db, const string& key, const string& loglevel) TEST(LOGGER, loglevel) { DBConnector db("CONFIG_DB", 0); - clearLoglevelDB(); + clearConfigDB(); string key1 = "table1", key2 = "table2", key3 = "table3"; diff --git a/tests/loglevel_ut.cpp b/tests/loglevel_ut.cpp new file mode 100644 index 000000000..bc4a785f3 --- /dev/null +++ b/tests/loglevel_ut.cpp @@ -0,0 +1,61 @@ +#include "common/dbconnector.h" +#include "common/producerstatetable.h" +#include "common/consumerstatetable.h" +#include "common/select.h" +#include "common/schema.h" +#include "gtest/gtest.h" +#include + +using namespace std; +using namespace swss; + +void setLoglevel(DBConnector& db, const string& key, const string& loglevel) +{ + swss::Table table(&db, key); + FieldValueTuple fv(DAEMON_LOGLEVEL, loglevel); + std::vectorfieldValues = { fv }; + table.set(key, fieldValues); +} + +void clearConfigDB() +{ + DBConnector db("CONFIG_DB", 0); + RedisReply r(&db, "FLUSHALL", REDIS_REPLY_STATUS); + r.checkStatusOK(); +} + +void checkLoglevel(DBConnector& db, const string& key, const string& loglevel) +{ + string redis_key = "LOGGER|" + key; + auto level = db.hget(redis_key, DAEMON_LOGLEVEL); + EXPECT_FALSE(level == nullptr); + if (level != nullptr) + { + EXPECT_EQ(*level, loglevel); + } +} + +TEST(LOGGER, loglevel) +{ + DBConnector db("CONFIG_DB", 0); + clearConfigDB(); + + string key1 = "table1", key2 = "table2", key3 = "table3"; + + cout << "Setting log level NOTICE for table1." << endl; + setLoglevel(db, key1, "NOTICE"); + cout << "Setting log level DEBUG for table1." << endl; + setLoglevel(db, key1, "DEBUG"); + cout << "Setting log level SAI_LOG_LEVEL_ERROR for table1." << endl; + setLoglevel(db, key1, "SAI_LOG_LEVEL_ERROR"); + + sleep(1); + swssloglevel("-d"); + sleep(1); + cout << "Checking log level for tables." << endl; + checkLoglevel(db, key1, "NOTICE"); + checkLoglevel(db, key2, "NOTICE"); + checkLoglevel(db, key3, "SAI_LOG_LEVEL_NOTICE"); + + cout << "Done." << endl; +} From 6952679bc6eb95cbcff77a82b8fe4bc21eaf4b37 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Sun, 28 Aug 2022 15:25:13 +0000 Subject: [PATCH 08/23] change prefix string name --- common/loglevel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/loglevel.cpp b/common/loglevel.cpp index a5a6d82ef..27b707b86 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -153,8 +153,8 @@ int swssloglevel(int argc, char **argv) std::sort(keys.begin(), keys.end()); //TODO: change to be in h file - std::string key_prefix(CFG_LOGGER_TABLE_NAME); - key_prefix=+"|"; + std::string redis_key_prefix(CFG_LOGGER_TABLE_NAME); + redis_key_prefix=+"|"; for (const auto& key : keys) { const auto redis_key = redis_key_prefix.append(key); From 1c1ec15121a3e091b6fcd2a62d42718e26944333 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Wed, 31 Aug 2022 07:27:22 +0000 Subject: [PATCH 09/23] add the loglevel unit test to the Makefile --- tests/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index 2c3e8aaf9..8c07535fd 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -28,6 +28,7 @@ tests_SOURCES = redis_ut.cpp \ warm_restart_ut.cpp \ redis_multi_db_ut.cpp \ logger_ut.cpp \ + loglevel_ut.cpp \ redis_multi_ns_ut.cpp \ fdb_flush.cpp \ stringutility_ut.cpp \ From fe9019ac42b3415dfcc5f48ac9c38f2fdb88c01c Mon Sep 17 00:00:00 2001 From: EdenGri Date: Wed, 31 Aug 2022 10:32:19 +0000 Subject: [PATCH 10/23] add include of loglevel.cpp --- tests/loglevel_ut.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/loglevel_ut.cpp b/tests/loglevel_ut.cpp index bc4a785f3..c9e37fc79 100644 --- a/tests/loglevel_ut.cpp +++ b/tests/loglevel_ut.cpp @@ -5,7 +5,7 @@ #include "common/schema.h" #include "gtest/gtest.h" #include - +#include "common/loglevel.cpp" using namespace std; using namespace swss; @@ -50,7 +50,8 @@ TEST(LOGGER, loglevel) setLoglevel(db, key1, "SAI_LOG_LEVEL_ERROR"); sleep(1); - swssloglevel("-d"); + char* argv[1] = {"-d"}; + swssloglevel(1,argv); sleep(1); cout << "Checking log level for tables." << endl; checkLoglevel(db, key1, "NOTICE"); From 7a8aa6104aa42fb657434f4911e759ff7b466feb Mon Sep 17 00:00:00 2001 From: EdenGri Date: Mon, 5 Sep 2022 09:56:01 +0000 Subject: [PATCH 11/23] change the select table to LOGGER --- common/logger.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/common/logger.cpp b/common/logger.cpp index 211b35f0b..22832821d 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -74,6 +74,8 @@ const Logger::PriorityStringMap Logger::priorityStringMap = { void Logger::swssPrioNotify(const std::string& component, const std::string& prioStr) { + SWSS_LOG_NOTICE("EDEN start swssPrioNotify"); + auto& logger = getInstance(); if (priorityStringMap.find(prioStr) == priorityStringMap.end()) @@ -83,7 +85,10 @@ void Logger::swssPrioNotify(const std::string& component, const std::string& pri } else { + SWSS_LOG_NOTICE("EDEN changing min prio of logger"); logger.m_minPrio = priorityStringMap.at(prioStr); + SWSS_LOG_NOTICE("EDEN changing min prio of logger end"); + } } @@ -125,7 +130,7 @@ void Logger::linkToDbWithOutput( std::string key_prefix(CFG_LOGGER_TABLE_NAME); key_prefix=+"|"; std::string key = key_prefix + dbName; - + std::string prio, output; bool doUpdate = false; auto prioPtr = db.hget(key, DAEMON_LOGLEVEL); @@ -265,9 +270,11 @@ void Logger::settingThread() void Logger::write(Priority prio, const char *fmt, ...) { + if (prio > m_minPrio) return; + // TODO // + add thread id using std::thread::id this_id = std::this_thread::get_id(); // + add timestmap with millisecond resolution From c6011383735ad378ea0462ce99da104a02f42160 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Mon, 5 Sep 2022 19:10:09 +0000 Subject: [PATCH 12/23] loglevel unit tests --- common/loglevel.cpp | 7 +- common/loglevel.h | 18 ++++ common/loglevel_util.cpp | 6 ++ sonic-db-cli/sonic-db-cli | 210 ++++++++++++++++++++++++++++++++++++++ tests/loglevel_ut.cpp | 24 ++--- 5 files changed, 248 insertions(+), 17 deletions(-) create mode 100644 common/loglevel.h create mode 100644 common/loglevel_util.cpp create mode 100755 sonic-db-cli/sonic-db-cli diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 27b707b86..72c4dc37f 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -5,6 +5,7 @@ #include #include #include "schema.h" +#include "loglevel.h" #include "logger.h" #include "dbconnector.h" #include "producerstatetable.h" @@ -196,6 +197,7 @@ int swssloglevel(int argc, char **argv) if ((prefix == "SAI_API_") && !validateSaiLoglevel(loglevel)) { exitWithUsage(EXIT_FAILURE, "Invalid SAI loglevel value"); + } else if ((prefix == "") && (Logger::priorityStringMap.find(loglevel) == Logger::priorityStringMap.end())) { @@ -234,9 +236,4 @@ int swssloglevel(int argc, char **argv) return EXIT_SUCCESS; } -int main(int argc, char **argv) -{ - - return swssloglevel(argc, argv); -} diff --git a/common/loglevel.h b/common/loglevel.h new file mode 100644 index 000000000..21b4c73e4 --- /dev/null +++ b/common/loglevel.h @@ -0,0 +1,18 @@ +/* +void usage(const std::string &program, int status, const std::string &message); + +void setLoglevel(swss::Table& logger_tbl, const std::string& component, const std::string& loglevel); + +bool validateSaiLoglevel(const std::string &prioStr); + +bool filterOutSaiKeys(const std::string& key); + +bool filterSaiKeys(const std::string& key); + +std::vector get_sai_keys(std::vector keys); + +std::vector get_no_sai_keys(std::vector keys); + +void setAllLoglevel(swss::Table& logger_tbl, std::vector components,std::string loglevel); +*/ +int swssloglevel(int argc, char **argv); diff --git a/common/loglevel_util.cpp b/common/loglevel_util.cpp new file mode 100644 index 000000000..fcd89144e --- /dev/null +++ b/common/loglevel_util.cpp @@ -0,0 +1,6 @@ +#include "loglevel.h" + +int main(int argc, char **argv) +{ + return swssloglevel(argc, argv); +} diff --git a/sonic-db-cli/sonic-db-cli b/sonic-db-cli/sonic-db-cli new file mode 100755 index 000000000..664dfdd43 --- /dev/null +++ b/sonic-db-cli/sonic-db-cli @@ -0,0 +1,210 @@ +#! /bin/bash + +# sonic-db-cli - temporary wrapper script for .libs/sonic-db-cli +# Generated by libtool (GNU libtool) 2.4.6 Debian-2.4.6-15 +# +# The sonic-db-cli program cannot be directly executed until all the libtool +# libraries that it depends on are installed. +# +# This wrapper script should never be moved out of the build directory. +# If it is, it will not operate correctly. + +# Sed substitution that helps us do robust quoting. It backslashifies +# metacharacters that are still active within double-quoted strings. +sed_quote_subst='s|\([`"$\\]\)|\\\1|g' + +# Be Bourne compatible +if test -n "${ZSH_VERSION+set}" && (emulate sh) >/dev/null 2>&1; then + emulate sh + NULLCMD=: + # Zsh 3.x and 4.x performs word splitting on ${1+"$@"}, which + # is contrary to our usage. Disable this feature. + alias -g '${1+"$@"}'='"$@"' + setopt NO_GLOB_SUBST +else + case `(set -o) 2>/dev/null` in *posix*) set -o posix;; esac +fi +BIN_SH=xpg4; export BIN_SH # for Tru64 +DUALCASE=1; export DUALCASE # for MKS sh + +# The HP-UX ksh and POSIX shell print the target directory to stdout +# if CDPATH is set. +(unset CDPATH) >/dev/null 2>&1 && unset CDPATH + +relink_command="" + +# This environment variable determines our operation mode. +if test "$libtool_install_magic" = "%%%MAGIC variable%%%"; then + # install mode needs the following variables: + generated_by_libtool_version='2.4.6' + notinst_deplibs=' /sonic/src/sonic-swss-common/common/libswsscommon.la' +else + # When we are sourced in execute mode, $file and $ECHO are already set. + if test "$libtool_execute_magic" != "%%%MAGIC variable%%%"; then + file="$0" + +# A function that is used when there is no print builtin or printf. +func_fallback_echo () +{ + eval 'cat <<_LTECHO_EOF +$1 +_LTECHO_EOF' +} + ECHO="printf %s\\n" + fi + +# Very basic option parsing. These options are (a) specific to +# the libtool wrapper, (b) are identical between the wrapper +# /script/ and the wrapper /executable/ that is used only on +# windows platforms, and (c) all begin with the string --lt- +# (application programs are unlikely to have options that match +# this pattern). +# +# There are only two supported options: --lt-debug and +# --lt-dump-script. There is, deliberately, no --lt-help. +# +# The first argument to this parsing function should be the +# script's ../libtool value, followed by no. +lt_option_debug= +func_parse_lt_options () +{ + lt_script_arg0=$0 + shift + for lt_opt + do + case "$lt_opt" in + --lt-debug) lt_option_debug=1 ;; + --lt-dump-script) + lt_dump_D=`$ECHO "X$lt_script_arg0" | /bin/sed -e 's/^X//' -e 's%/[^/]*$%%'` + test "X$lt_dump_D" = "X$lt_script_arg0" && lt_dump_D=. + lt_dump_F=`$ECHO "X$lt_script_arg0" | /bin/sed -e 's/^X//' -e 's%^.*/%%'` + cat "$lt_dump_D/$lt_dump_F" + exit 0 + ;; + --lt-*) + $ECHO "Unrecognized --lt- option: '$lt_opt'" 1>&2 + exit 1 + ;; + esac + done + + # Print the debug banner immediately: + if test -n "$lt_option_debug"; then + echo "sonic-db-cli:sonic-db-cli:$LINENO: libtool wrapper (GNU libtool) 2.4.6 Debian-2.4.6-15" 1>&2 + fi +} + +# Used when --lt-debug. Prints its arguments to stdout +# (redirection is the responsibility of the caller) +func_lt_dump_args () +{ + lt_dump_args_N=1; + for lt_arg + do + $ECHO "sonic-db-cli:sonic-db-cli:$LINENO: newargv[$lt_dump_args_N]: $lt_arg" + lt_dump_args_N=`expr $lt_dump_args_N + 1` + done +} + +# Core function for launching the target application +func_exec_program_core () +{ + + if test -n "$lt_option_debug"; then + $ECHO "sonic-db-cli:sonic-db-cli:$LINENO: newargv[0]: $progdir/$program" 1>&2 + func_lt_dump_args ${1+"$@"} 1>&2 + fi + exec "$progdir/$program" ${1+"$@"} + + $ECHO "$0: cannot exec $program $*" 1>&2 + exit 1 +} + +# A function to encapsulate launching the target application +# Strips options in the --lt-* namespace from $@ and +# launches target application with the remaining arguments. +func_exec_program () +{ + case " $* " in + *\ --lt-*) + for lt_wr_arg + do + case $lt_wr_arg in + --lt-*) ;; + *) set x "$@" "$lt_wr_arg"; shift;; + esac + shift + done ;; + esac + func_exec_program_core ${1+"$@"} +} + + # Parse options + func_parse_lt_options "$0" ${1+"$@"} + + # Find the directory that this script lives in. + thisdir=`$ECHO "$file" | /bin/sed 's%/[^/]*$%%'` + test "x$thisdir" = "x$file" && thisdir=. + + # Follow symbolic links until we get to the real thisdir. + file=`ls -ld "$file" | /bin/sed -n 's/.*-> //p'` + while test -n "$file"; do + destdir=`$ECHO "$file" | /bin/sed 's%/[^/]*$%%'` + + # If there was a directory component, then change thisdir. + if test "x$destdir" != "x$file"; then + case "$destdir" in + [\\/]* | [A-Za-z]:[\\/]*) thisdir="$destdir" ;; + *) thisdir="$thisdir/$destdir" ;; + esac + fi + + file=`$ECHO "$file" | /bin/sed 's%^.*/%%'` + file=`ls -ld "$thisdir/$file" | /bin/sed -n 's/.*-> //p'` + done + + # Usually 'no', except on cygwin/mingw when embedded into + # the cwrapper. + WRAPPER_SCRIPT_BELONGS_IN_OBJDIR=no + if test "$WRAPPER_SCRIPT_BELONGS_IN_OBJDIR" = "yes"; then + # special case for '.' + if test "$thisdir" = "."; then + thisdir=`pwd` + fi + # remove .libs from thisdir + case "$thisdir" in + *[\\/].libs ) thisdir=`$ECHO "$thisdir" | /bin/sed 's%[\\/][^\\/]*$%%'` ;; + .libs ) thisdir=. ;; + esac + fi + + # Try to get the absolute directory name. + absdir=`cd "$thisdir" && pwd` + test -n "$absdir" && thisdir="$absdir" + + program='sonic-db-cli' + progdir="$thisdir/.libs" + + + if test -f "$progdir/$program"; then + # Add our own library path to LD_LIBRARY_PATH + LD_LIBRARY_PATH="/sonic/src/sonic-swss-common/common/.libs:$LD_LIBRARY_PATH" + + # Some systems cannot cope with colon-terminated LD_LIBRARY_PATH + # The second colon is a workaround for a bug in BeOS R4 sed + LD_LIBRARY_PATH=`$ECHO "$LD_LIBRARY_PATH" | /bin/sed 's/::*$//'` + + export LD_LIBRARY_PATH + + if test "$libtool_execute_magic" != "%%%MAGIC variable%%%"; then + # Run the actual program with our arguments. + func_exec_program ${1+"$@"} + fi + else + # The program doesn't exist. + $ECHO "$0: error: '$progdir/$program' does not exist" 1>&2 + $ECHO "This script is just a wrapper for $program." 1>&2 + $ECHO "See the libtool documentation for more information." 1>&2 + exit 1 + fi +fi diff --git a/tests/loglevel_ut.cpp b/tests/loglevel_ut.cpp index c9e37fc79..9cf3eeb2e 100644 --- a/tests/loglevel_ut.cpp +++ b/tests/loglevel_ut.cpp @@ -3,13 +3,13 @@ #include "common/consumerstatetable.h" #include "common/select.h" #include "common/schema.h" +#include "common/loglevel.h" #include "gtest/gtest.h" #include -#include "common/loglevel.cpp" using namespace std; using namespace swss; -void setLoglevel(DBConnector& db, const string& key, const string& loglevel) +void setLoglevel2(DBConnector& db, const string& key, const string& loglevel) { swss::Table table(&db, key); FieldValueTuple fv(DAEMON_LOGLEVEL, loglevel); @@ -17,14 +17,14 @@ void setLoglevel(DBConnector& db, const string& key, const string& loglevel) table.set(key, fieldValues); } -void clearConfigDB() +void clearConfigDB2() { DBConnector db("CONFIG_DB", 0); RedisReply r(&db, "FLUSHALL", REDIS_REPLY_STATUS); r.checkStatusOK(); } -void checkLoglevel(DBConnector& db, const string& key, const string& loglevel) +void checkLoglevel2(DBConnector& db, const string& key, const string& loglevel) { string redis_key = "LOGGER|" + key; auto level = db.hget(redis_key, DAEMON_LOGLEVEL); @@ -35,28 +35,28 @@ void checkLoglevel(DBConnector& db, const string& key, const string& loglevel) } } -TEST(LOGGER, loglevel) +TEST(LOGLEVEL, loglevel) { DBConnector db("CONFIG_DB", 0); - clearConfigDB(); + clearConfigDB2(); string key1 = "table1", key2 = "table2", key3 = "table3"; cout << "Setting log level NOTICE for table1." << endl; - setLoglevel(db, key1, "NOTICE"); + setLoglevel2(db, key1, "NOTICE"); cout << "Setting log level DEBUG for table1." << endl; - setLoglevel(db, key1, "DEBUG"); + setLoglevel2(db, key1, "DEBUG"); cout << "Setting log level SAI_LOG_LEVEL_ERROR for table1." << endl; - setLoglevel(db, key1, "SAI_LOG_LEVEL_ERROR"); + setLoglevel2(db, key1, "SAI_LOG_LEVEL_ERROR"); sleep(1); char* argv[1] = {"-d"}; swssloglevel(1,argv); sleep(1); cout << "Checking log level for tables." << endl; - checkLoglevel(db, key1, "NOTICE"); - checkLoglevel(db, key2, "NOTICE"); - checkLoglevel(db, key3, "SAI_LOG_LEVEL_NOTICE"); + checkLoglevel2(db, key1, "NOTICE"); + checkLoglevel2(db, key2, "NOTICE"); + checkLoglevel2(db, key3, "SAI_LOG_LEVEL_NOTICE"); cout << "Done." << endl; } From a4e1ae2f94cc0975e0118ff1d0dd0e56c2e3da96 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Tue, 6 Sep 2022 15:29:28 +0000 Subject: [PATCH 13/23] removin UT, bug fix changing the key in logger.cpp file --- common/logger.cpp | 10 +- common/loglevel.cpp | 3 +- common/loglevel.h | 18 ---- common/loglevel_util.cpp | 6 -- sonic-db-cli/sonic-db-cli | 210 -------------------------------------- tests/Makefile.am | 1 - tests/loglevel_ut.cpp | 62 ----------- 7 files changed, 9 insertions(+), 301 deletions(-) delete mode 100644 common/loglevel.h delete mode 100644 common/loglevel_util.cpp delete mode 100755 sonic-db-cli/sonic-db-cli delete mode 100644 tests/loglevel_ut.cpp diff --git a/common/logger.cpp b/common/logger.cpp index 22832821d..90c0adc10 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -85,7 +85,7 @@ void Logger::swssPrioNotify(const std::string& component, const std::string& pri } else { - SWSS_LOG_NOTICE("EDEN changing min prio of logger"); + SWSS_LOG_NOTICE("EDEN changing min prio of logger to %s", prioStr.c_str()); logger.m_minPrio = priorityStringMap.at(prioStr); SWSS_LOG_NOTICE("EDEN changing min prio of logger end"); @@ -128,22 +128,26 @@ void Logger::linkToDbWithOutput( DBConnector db("CONFIG_DB", 0); //TODO: change to be in h file std::string key_prefix(CFG_LOGGER_TABLE_NAME); - key_prefix=+"|"; + key_prefix+="|"; std::string key = key_prefix + dbName; std::string prio, output; bool doUpdate = false; auto prioPtr = db.hget(key, DAEMON_LOGLEVEL); auto outputPtr = db.hget(key, DAEMON_LOGOUTPUT); + SWSS_LOG_NOTICE("EDEN hget for key: %s", key.c_str()); if (prioPtr == nullptr) { + SWSS_LOG_NOTICE("EDEN hget for loglevel got nullptr"); prio = defPrio; doUpdate = true; } else { prio = *prioPtr; + SWSS_LOG_NOTICE("EDEN hget for loglevel got: %s", prio.c_str()); + } if (outputPtr == nullptr) { @@ -163,11 +167,13 @@ void Logger::linkToDbWithOutput( FieldValueTuple fvLevel(DAEMON_LOGLEVEL, prio); FieldValueTuple fvOutput(DAEMON_LOGOUTPUT, output); std::vectorfieldValues = { fvLevel, fvOutput }; + SWSS_LOG_NOTICE("EDEN setting loglevel to: %s in linkToDb", prio.c_str()); table.set(dbName, fieldValues); } logger.m_currentPrios.set(dbName, prio); logger.m_currentOutputs.set(dbName, output); + SWSS_LOG_NOTICE("EDEN calling to swssnotify from linktodb witj loglevel %s", prio.c_str()); prioNotify(dbName, prio); outputNotify(dbName, output); diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 72c4dc37f..648b2a0c2 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -5,7 +5,6 @@ #include #include #include "schema.h" -#include "loglevel.h" #include "logger.h" #include "dbconnector.h" #include "producerstatetable.h" @@ -97,7 +96,7 @@ void setAllLoglevel(swss::Table& logger_tbl, std::vector components SWSS_LOG_NOTICE("EDEN end setAllLoglevel"); } -int swssloglevel(int argc, char **argv) +int main(int argc, char **argv) { SWSS_LOG_NOTICE("EDEN start main"); diff --git a/common/loglevel.h b/common/loglevel.h deleted file mode 100644 index 21b4c73e4..000000000 --- a/common/loglevel.h +++ /dev/null @@ -1,18 +0,0 @@ -/* -void usage(const std::string &program, int status, const std::string &message); - -void setLoglevel(swss::Table& logger_tbl, const std::string& component, const std::string& loglevel); - -bool validateSaiLoglevel(const std::string &prioStr); - -bool filterOutSaiKeys(const std::string& key); - -bool filterSaiKeys(const std::string& key); - -std::vector get_sai_keys(std::vector keys); - -std::vector get_no_sai_keys(std::vector keys); - -void setAllLoglevel(swss::Table& logger_tbl, std::vector components,std::string loglevel); -*/ -int swssloglevel(int argc, char **argv); diff --git a/common/loglevel_util.cpp b/common/loglevel_util.cpp deleted file mode 100644 index fcd89144e..000000000 --- a/common/loglevel_util.cpp +++ /dev/null @@ -1,6 +0,0 @@ -#include "loglevel.h" - -int main(int argc, char **argv) -{ - return swssloglevel(argc, argv); -} diff --git a/sonic-db-cli/sonic-db-cli b/sonic-db-cli/sonic-db-cli deleted file mode 100755 index 664dfdd43..000000000 --- a/sonic-db-cli/sonic-db-cli +++ /dev/null @@ -1,210 +0,0 @@ -#! /bin/bash - -# sonic-db-cli - temporary wrapper script for .libs/sonic-db-cli -# Generated by libtool (GNU libtool) 2.4.6 Debian-2.4.6-15 -# -# The sonic-db-cli program cannot be directly executed until all the libtool -# libraries that it depends on are installed. -# -# This wrapper script should never be moved out of the build directory. -# If it is, it will not operate correctly. - -# Sed substitution that helps us do robust quoting. It backslashifies -# metacharacters that are still active within double-quoted strings. -sed_quote_subst='s|\([`"$\\]\)|\\\1|g' - -# Be Bourne compatible -if test -n "${ZSH_VERSION+set}" && (emulate sh) >/dev/null 2>&1; then - emulate sh - NULLCMD=: - # Zsh 3.x and 4.x performs word splitting on ${1+"$@"}, which - # is contrary to our usage. Disable this feature. - alias -g '${1+"$@"}'='"$@"' - setopt NO_GLOB_SUBST -else - case `(set -o) 2>/dev/null` in *posix*) set -o posix;; esac -fi -BIN_SH=xpg4; export BIN_SH # for Tru64 -DUALCASE=1; export DUALCASE # for MKS sh - -# The HP-UX ksh and POSIX shell print the target directory to stdout -# if CDPATH is set. -(unset CDPATH) >/dev/null 2>&1 && unset CDPATH - -relink_command="" - -# This environment variable determines our operation mode. -if test "$libtool_install_magic" = "%%%MAGIC variable%%%"; then - # install mode needs the following variables: - generated_by_libtool_version='2.4.6' - notinst_deplibs=' /sonic/src/sonic-swss-common/common/libswsscommon.la' -else - # When we are sourced in execute mode, $file and $ECHO are already set. - if test "$libtool_execute_magic" != "%%%MAGIC variable%%%"; then - file="$0" - -# A function that is used when there is no print builtin or printf. -func_fallback_echo () -{ - eval 'cat <<_LTECHO_EOF -$1 -_LTECHO_EOF' -} - ECHO="printf %s\\n" - fi - -# Very basic option parsing. These options are (a) specific to -# the libtool wrapper, (b) are identical between the wrapper -# /script/ and the wrapper /executable/ that is used only on -# windows platforms, and (c) all begin with the string --lt- -# (application programs are unlikely to have options that match -# this pattern). -# -# There are only two supported options: --lt-debug and -# --lt-dump-script. There is, deliberately, no --lt-help. -# -# The first argument to this parsing function should be the -# script's ../libtool value, followed by no. -lt_option_debug= -func_parse_lt_options () -{ - lt_script_arg0=$0 - shift - for lt_opt - do - case "$lt_opt" in - --lt-debug) lt_option_debug=1 ;; - --lt-dump-script) - lt_dump_D=`$ECHO "X$lt_script_arg0" | /bin/sed -e 's/^X//' -e 's%/[^/]*$%%'` - test "X$lt_dump_D" = "X$lt_script_arg0" && lt_dump_D=. - lt_dump_F=`$ECHO "X$lt_script_arg0" | /bin/sed -e 's/^X//' -e 's%^.*/%%'` - cat "$lt_dump_D/$lt_dump_F" - exit 0 - ;; - --lt-*) - $ECHO "Unrecognized --lt- option: '$lt_opt'" 1>&2 - exit 1 - ;; - esac - done - - # Print the debug banner immediately: - if test -n "$lt_option_debug"; then - echo "sonic-db-cli:sonic-db-cli:$LINENO: libtool wrapper (GNU libtool) 2.4.6 Debian-2.4.6-15" 1>&2 - fi -} - -# Used when --lt-debug. Prints its arguments to stdout -# (redirection is the responsibility of the caller) -func_lt_dump_args () -{ - lt_dump_args_N=1; - for lt_arg - do - $ECHO "sonic-db-cli:sonic-db-cli:$LINENO: newargv[$lt_dump_args_N]: $lt_arg" - lt_dump_args_N=`expr $lt_dump_args_N + 1` - done -} - -# Core function for launching the target application -func_exec_program_core () -{ - - if test -n "$lt_option_debug"; then - $ECHO "sonic-db-cli:sonic-db-cli:$LINENO: newargv[0]: $progdir/$program" 1>&2 - func_lt_dump_args ${1+"$@"} 1>&2 - fi - exec "$progdir/$program" ${1+"$@"} - - $ECHO "$0: cannot exec $program $*" 1>&2 - exit 1 -} - -# A function to encapsulate launching the target application -# Strips options in the --lt-* namespace from $@ and -# launches target application with the remaining arguments. -func_exec_program () -{ - case " $* " in - *\ --lt-*) - for lt_wr_arg - do - case $lt_wr_arg in - --lt-*) ;; - *) set x "$@" "$lt_wr_arg"; shift;; - esac - shift - done ;; - esac - func_exec_program_core ${1+"$@"} -} - - # Parse options - func_parse_lt_options "$0" ${1+"$@"} - - # Find the directory that this script lives in. - thisdir=`$ECHO "$file" | /bin/sed 's%/[^/]*$%%'` - test "x$thisdir" = "x$file" && thisdir=. - - # Follow symbolic links until we get to the real thisdir. - file=`ls -ld "$file" | /bin/sed -n 's/.*-> //p'` - while test -n "$file"; do - destdir=`$ECHO "$file" | /bin/sed 's%/[^/]*$%%'` - - # If there was a directory component, then change thisdir. - if test "x$destdir" != "x$file"; then - case "$destdir" in - [\\/]* | [A-Za-z]:[\\/]*) thisdir="$destdir" ;; - *) thisdir="$thisdir/$destdir" ;; - esac - fi - - file=`$ECHO "$file" | /bin/sed 's%^.*/%%'` - file=`ls -ld "$thisdir/$file" | /bin/sed -n 's/.*-> //p'` - done - - # Usually 'no', except on cygwin/mingw when embedded into - # the cwrapper. - WRAPPER_SCRIPT_BELONGS_IN_OBJDIR=no - if test "$WRAPPER_SCRIPT_BELONGS_IN_OBJDIR" = "yes"; then - # special case for '.' - if test "$thisdir" = "."; then - thisdir=`pwd` - fi - # remove .libs from thisdir - case "$thisdir" in - *[\\/].libs ) thisdir=`$ECHO "$thisdir" | /bin/sed 's%[\\/][^\\/]*$%%'` ;; - .libs ) thisdir=. ;; - esac - fi - - # Try to get the absolute directory name. - absdir=`cd "$thisdir" && pwd` - test -n "$absdir" && thisdir="$absdir" - - program='sonic-db-cli' - progdir="$thisdir/.libs" - - - if test -f "$progdir/$program"; then - # Add our own library path to LD_LIBRARY_PATH - LD_LIBRARY_PATH="/sonic/src/sonic-swss-common/common/.libs:$LD_LIBRARY_PATH" - - # Some systems cannot cope with colon-terminated LD_LIBRARY_PATH - # The second colon is a workaround for a bug in BeOS R4 sed - LD_LIBRARY_PATH=`$ECHO "$LD_LIBRARY_PATH" | /bin/sed 's/::*$//'` - - export LD_LIBRARY_PATH - - if test "$libtool_execute_magic" != "%%%MAGIC variable%%%"; then - # Run the actual program with our arguments. - func_exec_program ${1+"$@"} - fi - else - # The program doesn't exist. - $ECHO "$0: error: '$progdir/$program' does not exist" 1>&2 - $ECHO "This script is just a wrapper for $program." 1>&2 - $ECHO "See the libtool documentation for more information." 1>&2 - exit 1 - fi -fi diff --git a/tests/Makefile.am b/tests/Makefile.am index 8c07535fd..2c3e8aaf9 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -28,7 +28,6 @@ tests_SOURCES = redis_ut.cpp \ warm_restart_ut.cpp \ redis_multi_db_ut.cpp \ logger_ut.cpp \ - loglevel_ut.cpp \ redis_multi_ns_ut.cpp \ fdb_flush.cpp \ stringutility_ut.cpp \ diff --git a/tests/loglevel_ut.cpp b/tests/loglevel_ut.cpp deleted file mode 100644 index 9cf3eeb2e..000000000 --- a/tests/loglevel_ut.cpp +++ /dev/null @@ -1,62 +0,0 @@ -#include "common/dbconnector.h" -#include "common/producerstatetable.h" -#include "common/consumerstatetable.h" -#include "common/select.h" -#include "common/schema.h" -#include "common/loglevel.h" -#include "gtest/gtest.h" -#include -using namespace std; -using namespace swss; - -void setLoglevel2(DBConnector& db, const string& key, const string& loglevel) -{ - swss::Table table(&db, key); - FieldValueTuple fv(DAEMON_LOGLEVEL, loglevel); - std::vectorfieldValues = { fv }; - table.set(key, fieldValues); -} - -void clearConfigDB2() -{ - DBConnector db("CONFIG_DB", 0); - RedisReply r(&db, "FLUSHALL", REDIS_REPLY_STATUS); - r.checkStatusOK(); -} - -void checkLoglevel2(DBConnector& db, const string& key, const string& loglevel) -{ - string redis_key = "LOGGER|" + key; - auto level = db.hget(redis_key, DAEMON_LOGLEVEL); - EXPECT_FALSE(level == nullptr); - if (level != nullptr) - { - EXPECT_EQ(*level, loglevel); - } -} - -TEST(LOGLEVEL, loglevel) -{ - DBConnector db("CONFIG_DB", 0); - clearConfigDB2(); - - string key1 = "table1", key2 = "table2", key3 = "table3"; - - cout << "Setting log level NOTICE for table1." << endl; - setLoglevel2(db, key1, "NOTICE"); - cout << "Setting log level DEBUG for table1." << endl; - setLoglevel2(db, key1, "DEBUG"); - cout << "Setting log level SAI_LOG_LEVEL_ERROR for table1." << endl; - setLoglevel2(db, key1, "SAI_LOG_LEVEL_ERROR"); - - sleep(1); - char* argv[1] = {"-d"}; - swssloglevel(1,argv); - sleep(1); - cout << "Checking log level for tables." << endl; - checkLoglevel2(db, key1, "NOTICE"); - checkLoglevel2(db, key2, "NOTICE"); - checkLoglevel2(db, key3, "SAI_LOG_LEVEL_NOTICE"); - - cout << "Done." << endl; -} From ec0d20dfff897d781bb8a8e009c5bf847e2a30a0 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Thu, 8 Sep 2022 14:03:19 +0000 Subject: [PATCH 14/23] remove unused del_table function --- common/sonicv2connector.cpp | 4 ---- common/sonicv2connector.h | 3 --- 2 files changed, 7 deletions(-) diff --git a/common/sonicv2connector.cpp b/common/sonicv2connector.cpp index 8857d25be..59552cde2 100644 --- a/common/sonicv2connector.cpp +++ b/common/sonicv2connector.cpp @@ -103,10 +103,6 @@ int64_t SonicV2Connector_Native::del(const std::string& db_name, const std::stri { return m_dbintf.del(db_name, key, blocking); } -int64_t SonicV2Connector_Native::del2(const std::string& db_name, const std::string& key, bool blocking) -{ - return m_dbintf.del(db_name, key, blocking); -} void SonicV2Connector_Native::delete_all_by_pattern(const std::string& db_name, const std::string& pattern) { diff --git a/common/sonicv2connector.h b/common/sonicv2connector.h index c565881bb..9210137be 100644 --- a/common/sonicv2connector.h +++ b/common/sonicv2connector.h @@ -47,7 +47,6 @@ class SonicV2Connector_Native int64_t set(const std::string& db_name, const std::string& _hash, const std::string& key, const std::string& val, bool blocking=false); int64_t del(const std::string& db_name, const std::string& key, bool blocking=false); - int64_t del2(const std::string& db_name, const std::string& key, bool blocking=false); void delete_all_by_pattern(const std::string& db_name, const std::string& pattern); @@ -100,8 +99,6 @@ class SonicV2Connector_Native return super(SonicV2Connector, self).set(db_name, _hash, key, str(value), blocking) - def del_table(self, db_name, key): - return super(SonicV2Connector, self).del2(db_name,key) %} #endif } From 77abc836a38512f96d8fb1ace4cf3fdc802552ca Mon Sep 17 00:00:00 2001 From: EdenGri Date: Thu, 8 Sep 2022 14:07:27 +0000 Subject: [PATCH 15/23] add example in the help option for changing all components loglevel --- common/loglevel.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 648b2a0c2..e445831ec 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -31,6 +31,7 @@ using namespace swss; << "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 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; From 815e440b4457742c115a3b5c08a49e098648c9a7 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Sun, 11 Sep 2022 14:12:57 +0000 Subject: [PATCH 16/23] add UT for persistent loglevel --- BUILD | 2 +- common/Makefile.am | 4 ++- common/loglevel.cpp | 75 +++++++++++++++++++++------------------- common/loglevel.h | 6 ++++ common/loglevel_util.cpp | 7 ++++ tests/Makefile.am | 2 ++ tests/logger_ut.cpp | 7 ++-- tests/logger_ut.h | 13 +++++++ tests/loglevel_ut.cpp | 45 ++++++++++++++++++++++++ 9 files changed, 122 insertions(+), 39 deletions(-) create mode 100644 common/loglevel.h create mode 100644 common/loglevel_util.cpp create mode 100644 tests/logger_ut.h create mode 100644 tests/loglevel_ut.cpp diff --git a/BUILD b/BUILD index a66a57a60..15b443b08 100644 --- a/BUILD +++ b/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", diff --git a/common/Makefile.am b/common/Makefile.am index e9e9d1a19..fc9a9c10c 100644 --- a/common/Makefile.am +++ b/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) diff --git a/common/loglevel.cpp b/common/loglevel.cpp index e445831ec..fb2d92a2d 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -97,44 +97,47 @@ void setAllLoglevel(swss::Table& logger_tbl, std::vector components SWSS_LOG_NOTICE("EDEN end setAllLoglevel"); } -int main(int argc, char **argv) + +int swssloglevel(int argc, char** argv) { + SWSS_LOG_NOTICE("EDEN start main"); - int opt; - bool applyToAll = false, print = false, default_loglevel = false; - std::string prefix = "", component, loglevel; - auto exitWithUsage = std::bind(usage, argv[0], std::placeholders::_1, std::placeholders::_2); - //todo what is th : in the thired variable? - while ((opt = getopt (argc, argv, "c:l:sapdh")) != -1) + int opt; + bool applyToAll = false, print = false, default_loglevel = false; + std::string prefix = "", component, loglevel; + auto exitWithUsage = std::bind(usage, argv[0], std::placeholders::_1, std::placeholders::_2); + + //todo what is th : in the thired variable? + while ( (opt = getopt (argc, argv, "c:l:sapdh")) != -1) + { + switch(opt) { - switch(opt) - { - case 'c': - component = optarg; - break; - case 'l': - loglevel = optarg; - break; - case 's': - prefix = "SAI_API_"; - break; - case 'a': - applyToAll = true; - break; - case 'p': - print = true; - break; - case 'd': - default_loglevel = true; - break; - case 'h': - exitWithUsage(EXIT_SUCCESS, ""); - break; - default: - exitWithUsage(EXIT_FAILURE, "Invalid option"); - } + case 'c': + component = optarg; + break; + case 'l': + loglevel = optarg; + break; + case 's': + prefix = "SAI_API_"; + break; + case 'a': + applyToAll = true; + break; + case 'p': + print = true; + break; + case 'd': + default_loglevel = true; + break; + case 'h': + exitWithUsage(EXIT_SUCCESS, ""); + break; + default: + exitWithUsage(EXIT_FAILURE, "Invalid option"); } + } SWSS_LOG_NOTICE("EDEN cearting config db connector"); @@ -179,6 +182,8 @@ int main(int argc, char **argv) if(default_loglevel) { + std::cout <<"EDEN in the default opt"<< std::endl; + //check if the command right I mean there is no loglevel or somthing like this //todo change the apply all??? std::vector sai_keys = get_sai_keys(keys); @@ -191,9 +196,7 @@ int main(int argc, char **argv) setAllLoglevel(logger_tbl,sai_keys, "SAI_LOG_LEVEL_NOTICE"); return (EXIT_SUCCESS); - } - if ((prefix == "SAI_API_") && !validateSaiLoglevel(loglevel)) { exitWithUsage(EXIT_FAILURE, "Invalid SAI loglevel value"); @@ -237,3 +240,5 @@ int main(int argc, char **argv) } + + diff --git a/common/loglevel.h b/common/loglevel.h new file mode 100644 index 000000000..626a73f20 --- /dev/null +++ b/common/loglevel.h @@ -0,0 +1,6 @@ +#ifndef __LOGLEVEL_H__ +#define __LOGLEVEL_H__ + +int swssloglevel(int argc, char** argv); + +#endif /* __LOGLEVEL_H__ */ diff --git a/common/loglevel_util.cpp b/common/loglevel_util.cpp new file mode 100644 index 000000000..cdcbbf770 --- /dev/null +++ b/common/loglevel_util.cpp @@ -0,0 +1,7 @@ +#include "loglevel.h" + +int main(int argc, char** argv) +{ + swssloglevel(argc,argv); + +} diff --git a/tests/Makefile.am b/tests/Makefile.am index 2c3e8aaf9..8c82a9769 100644 --- a/tests/Makefile.am +++ b/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 \ diff --git a/tests/logger_ut.cpp b/tests/logger_ut.cpp index a4ccd6954..2810d794c 100644 --- a/tests/logger_ut.cpp +++ b/tests/logger_ut.cpp @@ -3,6 +3,7 @@ #include "common/consumerstatetable.h" #include "common/select.h" #include "common/schema.h" +#include "logger_ut.h" #include "gtest/gtest.h" #include @@ -11,7 +12,7 @@ using namespace swss; void setLoglevel(DBConnector& db, const string& key, const string& loglevel) { - swss::Table table(&db, key); + swss::Table table(&db, CFG_LOGGER_TABLE_NAME); FieldValueTuple fv(DAEMON_LOGLEVEL, loglevel); std::vectorfieldValues = { fv }; table.set(key, fieldValues); @@ -31,7 +32,9 @@ void prioNotify(const string &component, const string &prioStr) void checkLoglevel(DBConnector& db, const string& key, const string& loglevel) { - string redis_key = "LOGGER|" + key; + std::string key_prefix(CFG_LOGGER_TABLE_NAME); + key_prefix+="|"; + string redis_key = key_prefix + key; auto level = db.hget(redis_key, DAEMON_LOGLEVEL); EXPECT_FALSE(level == nullptr); if (level != nullptr) diff --git a/tests/logger_ut.h b/tests/logger_ut.h new file mode 100644 index 000000000..14e3a201d --- /dev/null +++ b/tests/logger_ut.h @@ -0,0 +1,13 @@ +#ifndef __LOGGER_UT_H__ +#define __LOGGER_UT_H__ + +void setLoglevel(swss::DBConnector& db, const std::string& key, const std::string& loglevel); + +void clearConfigDB(); + +void prioNotify(const std::string &component, const std::string &prioStr); + +void checkLoglevel(swss::DBConnector& db, const std::string& key, const std::string& loglevel); + + +#endif /* __LOGGER_UT_H__ */ diff --git a/tests/loglevel_ut.cpp b/tests/loglevel_ut.cpp new file mode 100644 index 000000000..a4e7cef01 --- /dev/null +++ b/tests/loglevel_ut.cpp @@ -0,0 +1,45 @@ +#include "common/dbconnector.h" +#include "common/producerstatetable.h" +#include "common/consumerstatetable.h" +#include "common/select.h" +#include "common/schema.h" +#include "common/loglevel.h" +#include "logger_ut.h" +#include "gtest/gtest.h" +#include +using namespace std; +using namespace swss; + + + +TEST(LOGLEVEL, loglevel) +{ + DBConnector db("CONFIG_DB", 0); + clearConfigDB(); + + string key1 = "table1"; + //string key2 = "table2", key3 = "SAI_table3"; + + cout << "Setting log level NOTICE for table1." << endl; + setLoglevel(db, key1, "NOTICE"); + //cout << "Setting log level DEBUG for table1." << endl; + //setLoglevel(db, key2, "DEBUG"); + //cout << "Setting log level SAI_LOG_LEVEL_ERROR for table1." << endl; + //setLoglevel(db, key3, "SAI_LOG_LEVEL_ERROR"); + + sleep(1); + + + char* argv[] = {"loglevel", "-d"}; + int argc = sizeof(argv) / sizeof(argv[0]); + + swssloglevel(argc, argv); + //sleep(1); + + cout << "Checking log level for tables." << endl; + //checkLoglevel(db, key1, "NOTICE"); + //checkLoglevel(db, key2, "NOTICE"); + //checkLoglevel(db, key3, "SAI_LOG_LEVEL_NOTICE"); + + cout << "Done." << endl; +} From 915e574428f7e45180738101c80c2f4c721f7f04 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Sun, 11 Sep 2022 15:02:34 +0000 Subject: [PATCH 17/23] remove/add debug logs --- common/logger.cpp | 16 +++++----------- common/loglevel.cpp | 20 +------------------- 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/common/logger.cpp b/common/logger.cpp index 90c0adc10..9550ac877 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -74,8 +74,6 @@ const Logger::PriorityStringMap Logger::priorityStringMap = { void Logger::swssPrioNotify(const std::string& component, const std::string& prioStr) { - SWSS_LOG_NOTICE("EDEN start swssPrioNotify"); - auto& logger = getInstance(); if (priorityStringMap.find(prioStr) == priorityStringMap.end()) @@ -85,10 +83,8 @@ void Logger::swssPrioNotify(const std::string& component, const std::string& pri } else { - SWSS_LOG_NOTICE("EDEN changing min prio of logger to %s", prioStr.c_str()); + SWSS_LOG_DEBUG("Changing logger minPrio to %s", prioStr.c_str()); logger.m_minPrio = priorityStringMap.at(prioStr); - SWSS_LOG_NOTICE("EDEN changing min prio of logger end"); - } } @@ -130,24 +126,21 @@ void Logger::linkToDbWithOutput( std::string key_prefix(CFG_LOGGER_TABLE_NAME); key_prefix+="|"; std::string key = key_prefix + dbName; + SWSS_LOG_DEBUG("Component %s register to logger with the key: %s", dbName.c_str(), key.c_str()); std::string prio, output; bool doUpdate = false; auto prioPtr = db.hget(key, DAEMON_LOGLEVEL); auto outputPtr = db.hget(key, DAEMON_LOGOUTPUT); - SWSS_LOG_NOTICE("EDEN hget for key: %s", key.c_str()); if (prioPtr == nullptr) { - SWSS_LOG_NOTICE("EDEN hget for loglevel got nullptr"); prio = defPrio; doUpdate = true; } else { prio = *prioPtr; - SWSS_LOG_NOTICE("EDEN hget for loglevel got: %s", prio.c_str()); - } if (outputPtr == nullptr) { @@ -167,13 +160,14 @@ void Logger::linkToDbWithOutput( FieldValueTuple fvLevel(DAEMON_LOGLEVEL, prio); FieldValueTuple fvOutput(DAEMON_LOGOUTPUT, output); std::vectorfieldValues = { fvLevel, fvOutput }; - SWSS_LOG_NOTICE("EDEN setting loglevel to: %s in linkToDb", prio.c_str()); + + SWSS_LOG_DEBUG("Set %s loglevel to %s", dbName, prio); + table.set(dbName, fieldValues); } logger.m_currentPrios.set(dbName, prio); logger.m_currentOutputs.set(dbName, output); - SWSS_LOG_NOTICE("EDEN calling to swssnotify from linktodb witj loglevel %s", prio.c_str()); prioNotify(dbName, prio); outputNotify(dbName, output); diff --git a/common/loglevel.cpp b/common/loglevel.cpp index fb2d92a2d..5fed6fb11 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -40,14 +40,7 @@ using namespace swss; void setLoglevel(swss::Table& logger_tbl, const std::string& component, const std::string& loglevel) { - - SWSS_LOG_NOTICE("EDEN start set log level function"); - SWSS_LOG_NOTICE("EDEN component: %s, loglevel: %s", component.c_str(), loglevel.c_str()); - logger_tbl.hset(component, "LOGLEVEL",loglevel); - - SWSS_LOG_NOTICE("EDEN end setLoglevel"); - } bool validateSaiLoglevel(const std::string &prioStr) @@ -88,21 +81,16 @@ std::vector get_no_sai_keys(std::vector keys) //TODO change to MSET command void setAllLoglevel(swss::Table& logger_tbl, std::vector components,std::string loglevel) { - SWSS_LOG_NOTICE("EDEN start setAllLoglevel"); for (const auto& component : components) { - SWSS_LOG_NOTICE("EDEN in setAllLoglevel component: %s, loglevel: %s", component.c_str(), loglevel.c_str()); setLoglevel(logger_tbl, component, loglevel); } - SWSS_LOG_NOTICE("EDEN end setAllLoglevel"); + SWSS_LOG_DEBUG("All components are with default loglevel"); } int swssloglevel(int argc, char** argv) { - - SWSS_LOG_NOTICE("EDEN start main"); - int opt; bool applyToAll = false, print = false, default_loglevel = false; std::string prefix = "", component, loglevel; @@ -139,8 +127,6 @@ int swssloglevel(int argc, char** argv) } } - SWSS_LOG_NOTICE("EDEN cearting config db connector"); - DBConnector config_db("CONFIG_DB", 0); swss::Table logger_tbl(&config_db, CFG_LOGGER_TABLE_NAME); std::vector keys; @@ -182,8 +168,6 @@ int swssloglevel(int argc, char** argv) if(default_loglevel) { - std::cout <<"EDEN in the default opt"<< std::endl; - //check if the command right I mean there is no loglevel or somthing like this //todo change the apply all??? std::vector sai_keys = get_sai_keys(keys); @@ -232,8 +216,6 @@ int swssloglevel(int argc, char** argv) { exitWithUsage(EXIT_FAILURE, "Component not present in DB"); } - SWSS_LOG_NOTICE("EDEN calling set log level"); - setLoglevel(logger_tbl, component, loglevel); return EXIT_SUCCESS; From a90f80c73257a417280a40e6c1b59e47c1b17b9c Mon Sep 17 00:00:00 2001 From: EdenGri Date: Sun, 11 Sep 2022 16:45:09 +0000 Subject: [PATCH 18/23] add default loglevel as constant --- common/logger.cpp | 5 +---- common/loglevel.cpp | 21 ++++++++------------- common/loglevel.h | 2 ++ common/sonicv2connector.h | 1 - tests/loglevel_ut.cpp | 20 +++++++++----------- 5 files changed, 20 insertions(+), 29 deletions(-) diff --git a/common/logger.cpp b/common/logger.cpp index 9550ac877..8ff332ea2 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -122,7 +122,6 @@ void Logger::linkToDbWithOutput( logger.m_settingChangeObservers.insert(std::make_pair(dbName, std::make_pair(prioNotify, outputNotify))); DBConnector db("CONFIG_DB", 0); - //TODO: change to be in h file std::string key_prefix(CFG_LOGGER_TABLE_NAME); key_prefix+="|"; std::string key = key_prefix + dbName; @@ -155,13 +154,12 @@ void Logger::linkToDbWithOutput( if (doUpdate) { - //todo: change to table name swss::Table table(&db, CFG_LOGGER_TABLE_NAME); FieldValueTuple fvLevel(DAEMON_LOGLEVEL, prio); FieldValueTuple fvOutput(DAEMON_LOGOUTPUT, output); std::vectorfieldValues = { fvLevel, fvOutput }; - SWSS_LOG_DEBUG("Set %s loglevel to %s", dbName, prio); + SWSS_LOG_DEBUG("Set %s loglevel to %s", dbName.c_str() , prio.c_str()); table.set(dbName, fieldValues); } @@ -274,7 +272,6 @@ void Logger::write(Priority prio, const char *fmt, ...) if (prio > m_minPrio) return; - // TODO // + add thread id using std::thread::id this_id = std::this_thread::get_id(); // + add timestmap with millisecond resolution diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 5fed6fb11..82db86bbc 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -5,6 +5,7 @@ #include #include #include "schema.h" +#include "loglevel.h" #include "logger.h" #include "dbconnector.h" #include "producerstatetable.h" @@ -78,7 +79,7 @@ std::vector get_no_sai_keys(std::vector keys) keys.erase(std::remove_if(keys.begin(), keys.end(), filterOutSaiKeys), keys.end()); return keys; } -//TODO change to MSET command + void setAllLoglevel(swss::Table& logger_tbl, std::vector components,std::string loglevel) { for (const auto& component : components) @@ -92,11 +93,10 @@ void setAllLoglevel(swss::Table& logger_tbl, std::vector components int swssloglevel(int argc, char** argv) { int opt; - bool applyToAll = false, print = false, default_loglevel = 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); - //todo what is th : in the thired variable? while ( (opt = getopt (argc, argv, "c:l:sapdh")) != -1) { switch(opt) @@ -117,7 +117,7 @@ int swssloglevel(int argc, char** argv) print = true; break; case 'd': - default_loglevel = true; + default_loglevel_opt = true; break; case 'h': exitWithUsage(EXIT_SUCCESS, ""); @@ -142,7 +142,7 @@ int swssloglevel(int argc, char** argv) } std::sort(keys.begin(), keys.end()); - //TODO: change to be in h file + std::string redis_key_prefix(CFG_LOGGER_TABLE_NAME); redis_key_prefix=+"|"; for (const auto& key : keys) @@ -166,18 +166,13 @@ int swssloglevel(int argc, char** argv) return (EXIT_SUCCESS); } - if(default_loglevel) + if(default_loglevel_opt) { - //check if the command right I mean there is no loglevel or somthing like this - //todo change the apply all??? std::vector sai_keys = get_sai_keys(keys); std::vector no_sai_keys = get_no_sai_keys(keys); - - setAllLoglevel(logger_tbl,no_sai_keys, "NOTICE"); - //todo add the default as constant - - setAllLoglevel(logger_tbl,sai_keys, "SAI_LOG_LEVEL_NOTICE"); + setAllLoglevel(logger_tbl,no_sai_keys, std::string(DEFAULT_LOGLEVEL)); + setAllLoglevel(logger_tbl,sai_keys, std::string(SAI_DEFAULT_LOGLEVEL)); return (EXIT_SUCCESS); } diff --git a/common/loglevel.h b/common/loglevel.h index 626a73f20..dc772f03e 100644 --- a/common/loglevel.h +++ b/common/loglevel.h @@ -1,6 +1,8 @@ #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__ */ diff --git a/common/sonicv2connector.h b/common/sonicv2connector.h index 9210137be..d700d926f 100644 --- a/common/sonicv2connector.h +++ b/common/sonicv2connector.h @@ -98,7 +98,6 @@ class SonicV2Connector_Native return super(SonicV2Connector, self).set(db_name, _hash, key, value, blocking) return super(SonicV2Connector, self).set(db_name, _hash, key, str(value), blocking) - %} #endif } diff --git a/tests/loglevel_ut.cpp b/tests/loglevel_ut.cpp index a4e7cef01..26ddab762 100644 --- a/tests/loglevel_ut.cpp +++ b/tests/loglevel_ut.cpp @@ -17,29 +17,27 @@ TEST(LOGLEVEL, loglevel) DBConnector db("CONFIG_DB", 0); clearConfigDB(); - string key1 = "table1"; - //string key2 = "table2", key3 = "SAI_table3"; + string key1 = "table1", key2 = "table2", key3 = "SAI_API_table3"; cout << "Setting log level NOTICE for table1." << endl; setLoglevel(db, key1, "NOTICE"); - //cout << "Setting log level DEBUG for table1." << endl; - //setLoglevel(db, key2, "DEBUG"); - //cout << "Setting log level SAI_LOG_LEVEL_ERROR for table1." << endl; - //setLoglevel(db, key3, "SAI_LOG_LEVEL_ERROR"); + cout << "Setting log level DEBUG for table1." << endl; + setLoglevel(db, key2, "DEBUG"); + cout << "Setting log level SAI_LOG_LEVEL_ERROR for table1." << endl; + setLoglevel(db, key3, "SAI_LOG_LEVEL_ERROR"); sleep(1); - char* argv[] = {"loglevel", "-d"}; int argc = sizeof(argv) / sizeof(argv[0]); swssloglevel(argc, argv); - //sleep(1); + sleep(1); cout << "Checking log level for tables." << endl; - //checkLoglevel(db, key1, "NOTICE"); - //checkLoglevel(db, key2, "NOTICE"); - //checkLoglevel(db, key3, "SAI_LOG_LEVEL_NOTICE"); + checkLoglevel(db, key1, "NOTICE"); + checkLoglevel(db, key2, "NOTICE"); + checkLoglevel(db, key3, "SAI_LOG_LEVEL_NOTICE"); cout << "Done." << endl; } From 4fd96d7283772189988839af6fdf7f285594ac79 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Tue, 13 Sep 2022 10:55:00 +0000 Subject: [PATCH 19/23] fixes after code review --- common/Makefile.am | 2 +- common/logger.cpp | 5 +---- common/loglevel.cpp | 9 --------- common/loglevel.h | 1 + common/loglevel_util.cpp | 1 - common/schema.h | 2 -- tests/Makefile.am | 4 ++-- tests/loglevel_ut.cpp | 15 ++++++--------- 8 files changed, 11 insertions(+), 28 deletions(-) diff --git a/common/Makefile.am b/common/Makefile.am index fc9a9c10c..e2d122e37 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -80,7 +80,7 @@ libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CP libswsscommon_la_LIBADD = -lpthread $(LIBNL_LIBS) $(CODE_COVERAGE_LIBS) -lzmq -lboost_serialization -luuid -lyang swssloglevel_SOURCES = \ - loglevel.cpp \ + loglevel.cpp \ loglevel_util.cpp swssloglevel_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS) diff --git a/common/logger.cpp b/common/logger.cpp index 8ff332ea2..fc6f4dbd5 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -141,6 +141,7 @@ void Logger::linkToDbWithOutput( { prio = *prioPtr; } + if (outputPtr == nullptr) { output = defOutput; @@ -158,9 +159,7 @@ void Logger::linkToDbWithOutput( FieldValueTuple fvLevel(DAEMON_LOGLEVEL, prio); FieldValueTuple fvOutput(DAEMON_LOGOUTPUT, output); std::vectorfieldValues = { fvLevel, fvOutput }; - SWSS_LOG_DEBUG("Set %s loglevel to %s", dbName.c_str() , prio.c_str()); - table.set(dbName, fieldValues); } @@ -199,7 +198,6 @@ Logger::Priority Logger::getMinPrio() return getInstance().m_minPrio; } - void Logger::settingThread() { Select select; @@ -268,7 +266,6 @@ void Logger::settingThread() void Logger::write(Priority prio, const char *fmt, ...) { - if (prio > m_minPrio) return; diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 82db86bbc..84e8f3f05 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -89,7 +89,6 @@ void setAllLoglevel(swss::Table& logger_tbl, std::vector components SWSS_LOG_DEBUG("All components are with default loglevel"); } - int swssloglevel(int argc, char** argv) { int opt; @@ -142,7 +141,6 @@ int swssloglevel(int argc, char** argv) } std::sort(keys.begin(), keys.end()); - std::string redis_key_prefix(CFG_LOGGER_TABLE_NAME); redis_key_prefix=+"|"; for (const auto& key : keys) @@ -170,16 +168,13 @@ int swssloglevel(int argc, char** argv) { std::vector sai_keys = get_sai_keys(keys); std::vector 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"); - } else if ((prefix == "") && (Logger::priorityStringMap.find(loglevel) == Logger::priorityStringMap.end())) { @@ -215,7 +210,3 @@ int swssloglevel(int argc, char** argv) return EXIT_SUCCESS; } - - - - diff --git a/common/loglevel.h b/common/loglevel.h index dc772f03e..a9d86b8d3 100644 --- a/common/loglevel.h +++ b/common/loglevel.h @@ -3,6 +3,7 @@ #define DEFAULT_LOGLEVEL "NOTICE" #define SAI_DEFAULT_LOGLEVEL "SAI_LOG_LEVEL_NOTICE" + int swssloglevel(int argc, char** argv); #endif /* __LOGLEVEL_H__ */ diff --git a/common/loglevel_util.cpp b/common/loglevel_util.cpp index cdcbbf770..9580d329f 100644 --- a/common/loglevel_util.cpp +++ b/common/loglevel_util.cpp @@ -3,5 +3,4 @@ int main(int argc, char** argv) { swssloglevel(argc,argv); - } diff --git a/common/schema.h b/common/schema.h index 3c3524301..3601f3146 100644 --- a/common/schema.h +++ b/common/schema.h @@ -415,10 +415,8 @@ 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 *****/ #define STATE_SWITCH_CAPABILITY_TABLE_NAME "SWITCH_CAPABILITY_TABLE" diff --git a/tests/Makefile.am b/tests/Makefile.am index 8c82a9769..55f2dc62a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -28,8 +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 \ + ../common/loglevel.cpp \ + loglevel_ut.cpp \ redis_multi_ns_ut.cpp \ fdb_flush.cpp \ stringutility_ut.cpp \ diff --git a/tests/loglevel_ut.cpp b/tests/loglevel_ut.cpp index 26ddab762..ceb2e6cb0 100644 --- a/tests/loglevel_ut.cpp +++ b/tests/loglevel_ut.cpp @@ -7,23 +7,20 @@ #include "logger_ut.h" #include "gtest/gtest.h" #include -using namespace std; using namespace swss; - - TEST(LOGLEVEL, loglevel) { DBConnector db("CONFIG_DB", 0); clearConfigDB(); - string key1 = "table1", key2 = "table2", key3 = "SAI_API_table3"; + std::string key1 = "table1", key2 = "table2", key3 = "SAI_API_table3"; - cout << "Setting log level NOTICE for table1." << endl; + std::cout << "Setting log level NOTICE for table1." << std::endl; setLoglevel(db, key1, "NOTICE"); - cout << "Setting log level DEBUG for table1." << endl; + std::cout << "Setting log level DEBUG for table1." << std::endl; setLoglevel(db, key2, "DEBUG"); - cout << "Setting log level SAI_LOG_LEVEL_ERROR for table1." << endl; + std::cout << "Setting log level SAI_LOG_LEVEL_ERROR for table1." << std::endl; setLoglevel(db, key3, "SAI_LOG_LEVEL_ERROR"); sleep(1); @@ -34,10 +31,10 @@ TEST(LOGLEVEL, loglevel) swssloglevel(argc, argv); sleep(1); - cout << "Checking log level for tables." << endl; + std::cout << "Checking log level for tables." << std::endl; checkLoglevel(db, key1, "NOTICE"); checkLoglevel(db, key2, "NOTICE"); checkLoglevel(db, key3, "SAI_LOG_LEVEL_NOTICE"); - cout << "Done." << endl; + std::cout << "Done." << std::endl; } From 7a57fec3a31c0def4425f48ab8fc35ff0be38e03 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Tue, 13 Sep 2022 12:27:07 +0000 Subject: [PATCH 20/23] change DEBUG log message --- common/loglevel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 84e8f3f05..c717dd788 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -86,7 +86,7 @@ void setAllLoglevel(swss::Table& logger_tbl, std::vector components { setLoglevel(logger_tbl, component, loglevel); } - SWSS_LOG_DEBUG("All components are with default loglevel"); + SWSS_LOG_DEBUG("All components are with %s loglevel", loglevel.c_str()); } int swssloglevel(int argc, char** argv) From 4b3e39d02370a5f324f07cf088069c5414e84ace Mon Sep 17 00:00:00 2001 From: EdenGri Date: Wed, 14 Sep 2022 11:14:58 +0000 Subject: [PATCH 21/23] bug fix: return code 1 in swssloglevel -p --- common/loglevel.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/common/loglevel.cpp b/common/loglevel.cpp index c717dd788..6073cbf2c 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -141,20 +141,17 @@ int swssloglevel(int argc, char** argv) } std::sort(keys.begin(), keys.end()); - std::string redis_key_prefix(CFG_LOGGER_TABLE_NAME); - redis_key_prefix=+"|"; for (const auto& key : keys) { - const auto redis_key = redis_key_prefix.append(key); - auto level = config_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; } } From 12fa97a156114db0958a81a77160725504462c1a Mon Sep 17 00:00:00 2001 From: EdenGri Date: Mon, 3 Oct 2022 12:21:41 +0000 Subject: [PATCH 22/23] Fix after public code review: use a table instead dbConnector when accessing the table, use static const instead define, and add white space where needed --- common/logger.cpp | 24 ++++-------------------- common/logger.h | 6 +++--- common/loglevel.cpp | 4 ++-- 3 files changed, 9 insertions(+), 25 deletions(-) diff --git a/common/logger.cpp b/common/logger.cpp index fc6f4dbd5..3eb503df4 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -122,40 +122,24 @@ void Logger::linkToDbWithOutput( logger.m_settingChangeObservers.insert(std::make_pair(dbName, std::make_pair(prioNotify, outputNotify))); DBConnector db("CONFIG_DB", 0); - std::string key_prefix(CFG_LOGGER_TABLE_NAME); - key_prefix+="|"; - std::string key = key_prefix + dbName; - SWSS_LOG_DEBUG("Component %s register to logger with the key: %s", dbName.c_str(), key.c_str()); + swss::Table table(&db, CFG_LOGGER_TABLE_NAME); + SWSS_LOG_DEBUG("Component %s register to logger", dbName.c_str()); 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_LOGLEVEL, output)) { output = defOutput; doUpdate = true; - - } - else - { - output = *outputPtr; } if (doUpdate) { - swss::Table table(&db, CFG_LOGGER_TABLE_NAME); FieldValueTuple fvLevel(DAEMON_LOGLEVEL, prio); FieldValueTuple fvOutput(DAEMON_LOGOUTPUT, output); std::vectorfieldValues = { fvLevel, fvOutput }; diff --git a/common/logger.h b/common/logger.h index 07ffa9dd2..135879c96 100644 --- a/common/logger.h +++ b/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))) @@ -47,9 +50,6 @@ class Logger { public: -#define DAEMON_LOGLEVEL "LOGLEVEL" -#define DAEMON_LOGOUTPUT "LOGOUTPUT" - enum Priority { SWSS_EMERG, diff --git a/common/loglevel.cpp b/common/loglevel.cpp index 6073cbf2c..fee3230fe 100644 --- a/common/loglevel.cpp +++ b/common/loglevel.cpp @@ -80,7 +80,7 @@ std::vector get_no_sai_keys(std::vector keys) return keys; } -void setAllLoglevel(swss::Table& logger_tbl, std::vector components,std::string loglevel) +void setAllLoglevel(swss::Table& logger_tbl, std::vector components, std::string loglevel) { for (const auto& component : components) { @@ -194,7 +194,7 @@ int swssloglevel(int argc, char** argv) keys.erase(std::remove_if(keys.begin(), keys.end(), filterOutSaiKeys), keys.end()); } - setAllLoglevel(logger_tbl,keys, loglevel); + setAllLoglevel(logger_tbl, keys, loglevel); exit(EXIT_SUCCESS); } From 25d3848c241f90620ee9a785a0056b40654658e2 Mon Sep 17 00:00:00 2001 From: EdenGri Date: Wed, 26 Oct 2022 20:46:15 +0000 Subject: [PATCH 23/23] bug fix: change key from LOGLEVEL to LOGOUTPUT --- common/logger.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/logger.cpp b/common/logger.cpp index 3eb503df4..4dc583265 100644 --- a/common/logger.cpp +++ b/common/logger.cpp @@ -132,7 +132,7 @@ void Logger::linkToDbWithOutput( prio = defPrio; doUpdate = true; } - if(!table.hget(dbName, DAEMON_LOGLEVEL, output)) + if(!table.hget(dbName, DAEMON_LOGOUTPUT, output)) { output = defOutput; doUpdate = true;