You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pegasus.apache.org by wa...@apache.org on 2023/12/22 17:41:15 UTC
(incubator-pegasus) branch master updated: refactor(logging): drop the unnecessary interface, use `log` instead of `dsn_log` and improve the tests (#1809)
This is an automated email from the ASF dual-hosted git repository.
wangdan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git
The following commit(s) were added to refs/heads/master by this push:
new 2f551c7ed refactor(logging): drop the unnecessary interface, use `log` instead of `dsn_log` and improve the tests (#1809)
2f551c7ed is described below
commit 2f551c7ed0df596feb42de1722eb7dc4d9f896a8
Author: Dan Wang <wa...@apache.org>
AuthorDate: Sat Dec 23 01:41:09 2023 +0800
refactor(logging): drop the unnecessary interface, use `log` instead of `dsn_log` and improve the tests (#1809)
Refactor the logging system, including:
- drop the unnecessary interface `logging_provider::dsn_logv()`
- use `log` instead of `dsn_log` or `dlog` for the naming of
logging-related macros, functions, classes, types and variables
- improve the tests for the logging and fix the bug that the whole
utils test directory is removed after utils tests are finished
---
src/replica/replica_2pc.cpp | 4 +-
src/runtime/security/sasl_init.cpp | 4 +-
src/runtime/task/task_spec.h | 9 ---
src/utils/api_utilities.h | 56 ++++++----------
src/utils/fmt_logging.h | 17 ++---
src/utils/logging.cpp | 49 +++-----------
src/utils/logging_provider.h | 19 ++----
src/utils/simple_logger.cpp | 133 +++++++++----------------------------
src/utils/simple_logger.h | 41 ++++--------
src/utils/test/endian_test.cpp | 1 +
src/utils/test/logger.cpp | 101 ++++++++++++++--------------
src/utils/test/logging.cpp | 44 +++++-------
12 files changed, 159 insertions(+), 319 deletions(-)
diff --git a/src/replica/replica_2pc.cpp b/src/replica/replica_2pc.cpp
index 3acb42a19..ebba712cc 100644
--- a/src/replica/replica_2pc.cpp
+++ b/src/replica/replica_2pc.cpp
@@ -245,7 +245,7 @@ void replica::init_prepare(mutation_ptr &mu, bool reconciliation, bool pop_all_c
const auto request_count = mu->client_requests.size();
mu->data.header.last_committed_decree = last_committed_decree();
- dsn_log_level_t level = LOG_LEVEL_DEBUG;
+ log_level_t level = LOG_LEVEL_DEBUG;
if (mu->data.header.decree == invalid_decree) {
mu->set_id(get_ballot(), _prepare_list->max_decree() + 1);
// print a debug log if necessary
@@ -258,7 +258,7 @@ void replica::init_prepare(mutation_ptr &mu, bool reconciliation, bool pop_all_c
}
mu->_tracer->set_name(fmt::format("mutation[{}]", mu->name()));
- dlog_f(level, "{}: mutation {} init_prepare, mutation_tid={}", name(), mu->name(), mu->tid());
+ LOG(level, "{}: mutation {} init_prepare, mutation_tid={}", name(), mu->name(), mu->tid());
// child should prepare mutation synchronously
mu->set_is_sync_to_child(_primary_states.sync_send_write_request);
diff --git a/src/runtime/security/sasl_init.cpp b/src/runtime/security/sasl_init.cpp
index 2d3c19dba..651491ff0 100644
--- a/src/runtime/security/sasl_init.cpp
+++ b/src/runtime/security/sasl_init.cpp
@@ -32,7 +32,7 @@ namespace dsn {
namespace security {
DSN_DEFINE_string(security, sasl_plugin_path, "/usr/lib/sasl2", "path to search sasl plugins");
-dsn_log_level_t get_dsn_log_level(int level)
+log_level_t get_log_level(int level)
{
switch (level) {
case SASL_LOG_ERR:
@@ -53,7 +53,7 @@ int sasl_simple_logger(void *context, int level, const char *msg)
return SASL_OK;
}
- dlog_f(get_dsn_log_level(level), "sasl log info: {}", msg);
+ LOG(get_log_level(level), "sasl log info: {}", msg);
return SASL_OK;
}
diff --git a/src/runtime/task/task_spec.h b/src/runtime/task/task_spec.h
index 9ce31bd22..77beb9e90 100644
--- a/src/runtime/task/task_spec.h
+++ b/src/runtime/task/task_spec.h
@@ -32,7 +32,6 @@
#include <vector>
#include "runtime/task/task_code.h"
-#include "utils/api_utilities.h"
#include "utils/config_api.h"
#include "utils/config_helper.h"
#include "utils/customizable_id.h"
@@ -43,14 +42,6 @@
#include "utils/join_point.h"
#include "utils/threadpool_code.h"
-ENUM_BEGIN(dsn_log_level_t, LOG_LEVEL_INVALID)
-ENUM_REG(LOG_LEVEL_DEBUG)
-ENUM_REG(LOG_LEVEL_INFO)
-ENUM_REG(LOG_LEVEL_WARNING)
-ENUM_REG(LOG_LEVEL_ERROR)
-ENUM_REG(LOG_LEVEL_FATAL)
-ENUM_END(dsn_log_level_t)
-
namespace dsn {
enum task_state
diff --git a/src/utils/api_utilities.h b/src/utils/api_utilities.h
index 4e1d664ed..29cf232a4 100644
--- a/src/utils/api_utilities.h
+++ b/src/utils/api_utilities.h
@@ -31,10 +31,9 @@
#pragma once
-#include <stdarg.h>
-
-#include "ports.h"
+#include "utils/enum_helper.h"
#include "utils/fmt_utils.h"
+#include "utils/ports.h"
/*!
@defgroup logging Logging Service
@@ -48,7 +47,8 @@
@{
*/
-typedef enum dsn_log_level_t {
+enum log_level_t
+{
LOG_LEVEL_DEBUG,
LOG_LEVEL_INFO,
LOG_LEVEL_WARNING,
@@ -56,40 +56,26 @@ typedef enum dsn_log_level_t {
LOG_LEVEL_FATAL,
LOG_LEVEL_COUNT,
LOG_LEVEL_INVALID
-} dsn_log_level_t;
+};
+
+ENUM_BEGIN(log_level_t, LOG_LEVEL_INVALID)
+ENUM_REG(LOG_LEVEL_DEBUG)
+ENUM_REG(LOG_LEVEL_INFO)
+ENUM_REG(LOG_LEVEL_WARNING)
+ENUM_REG(LOG_LEVEL_ERROR)
+ENUM_REG(LOG_LEVEL_FATAL)
+ENUM_END(log_level_t)
-USER_DEFINED_ENUM_FORMATTER(dsn_log_level_t)
+USER_DEFINED_ENUM_FORMATTER(log_level_t)
// logs with level smaller than this start_level will not be logged
-extern dsn_log_level_t dsn_log_start_level;
-extern dsn_log_level_t dsn_log_get_start_level();
-extern void dsn_log_set_start_level(dsn_log_level_t level);
-extern void dsn_logv(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *fmt,
- va_list args);
-extern void dsn_logf(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *fmt,
- ...);
-extern void dsn_log(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *str);
+extern log_level_t log_start_level;
+extern log_level_t get_log_start_level();
+extern void set_log_start_level(log_level_t level);
+extern void global_log(
+ const char *file, const char *function, const int line, log_level_t log_level, const char *str);
extern void dsn_coredump();
-// __FILENAME__ macro comes from the cmake, in which we calculate a filename without path.
-#define dlog(level, ...) \
- do { \
- if (level >= dsn_log_start_level) \
- dsn_logf(__FILENAME__, __FUNCTION__, __LINE__, level, __VA_ARGS__); \
- } while (false)
-
#define dreturn_not_ok_logged(err, ...) \
do { \
if (dsn_unlikely((err) != dsn::ERR_OK)) { \
@@ -124,7 +110,7 @@ extern void dsn_coredump();
#define dverify_logged(exp, level, ...) \
do { \
if (dsn_unlikely(!(exp))) { \
- dlog(level, __VA_ARGS__); \
+ LOG(level, __VA_ARGS__); \
return false; \
} \
} while (0)
@@ -135,7 +121,7 @@ extern void dsn_coredump();
#define dstop_on_false_logged(exp, level, ...) \
do { \
if (dsn_unlikely(!(exp))) { \
- dlog(level, __VA_ARGS__); \
+ LOG(level, __VA_ARGS__); \
return; \
} \
} while (0)
diff --git a/src/utils/fmt_logging.h b/src/utils/fmt_logging.h
index 0a6ac64d9..2e40c8279 100644
--- a/src/utils/fmt_logging.h
+++ b/src/utils/fmt_logging.h
@@ -28,18 +28,19 @@
// instead we use fmt::format.
// TODO(wutao1): prevent construction of std::string for each log.
-#define dlog_f(level, ...) \
+// __FILENAME__ macro comes from the cmake, in which we calculate a filename without path.
+#define LOG(level, ...) \
do { \
- if (level >= dsn_log_start_level) \
- dsn_log( \
+ if (level >= log_start_level) \
+ global_log( \
__FILENAME__, __FUNCTION__, __LINE__, level, fmt::format(__VA_ARGS__).c_str()); \
} while (false)
-#define LOG_DEBUG(...) dlog_f(LOG_LEVEL_DEBUG, __VA_ARGS__)
-#define LOG_INFO(...) dlog_f(LOG_LEVEL_INFO, __VA_ARGS__)
-#define LOG_WARNING(...) dlog_f(LOG_LEVEL_WARNING, __VA_ARGS__)
-#define LOG_ERROR(...) dlog_f(LOG_LEVEL_ERROR, __VA_ARGS__)
-#define LOG_FATAL(...) dlog_f(LOG_LEVEL_FATAL, __VA_ARGS__)
+#define LOG_DEBUG(...) LOG(LOG_LEVEL_DEBUG, __VA_ARGS__)
+#define LOG_INFO(...) LOG(LOG_LEVEL_INFO, __VA_ARGS__)
+#define LOG_WARNING(...) LOG(LOG_LEVEL_WARNING, __VA_ARGS__)
+#define LOG_ERROR(...) LOG(LOG_LEVEL_ERROR, __VA_ARGS__)
+#define LOG_FATAL(...) LOG(LOG_LEVEL_FATAL, __VA_ARGS__)
#define LOG_WARNING_IF(x, ...) \
do { \
diff --git a/src/utils/logging.cpp b/src/utils/logging.cpp
index e34b3bfd3..5b55b9d4c 100644
--- a/src/utils/logging.cpp
+++ b/src/utils/logging.cpp
@@ -24,13 +24,11 @@
* THE SOFTWARE.
*/
-#include <stdarg.h>
#include <algorithm>
#include <functional>
#include <memory>
#include <string>
-#include "runtime/task/task_spec.h"
#include "runtime/tool_api.h"
#include "simple_logger.h"
#include "utils/api_utilities.h"
@@ -41,7 +39,7 @@
#include "utils/logging_provider.h"
#include "utils/sys_exit_hook.h"
-dsn_log_level_t dsn_log_start_level = dsn_log_level_t::LOG_LEVEL_INFO;
+log_level_t log_start_level = LOG_LEVEL_INFO;
DSN_DEFINE_string(core,
logging_start_level,
"LOG_LEVEL_INFO",
@@ -73,12 +71,10 @@ void dsn_log_init(const std::string &logging_factory_name,
const std::string &dir_log,
std::function<std::string()> dsn_log_prefixed_message_func)
{
- dsn_log_start_level =
- enum_from_string(FLAGS_logging_start_level, dsn_log_level_t::LOG_LEVEL_INVALID);
+ log_start_level = enum_from_string(FLAGS_logging_start_level, LOG_LEVEL_INVALID);
- CHECK_NE_MSG(dsn_log_start_level,
- dsn_log_level_t::LOG_LEVEL_INVALID,
- "invalid [core] logging_start_level specified");
+ CHECK_NE_MSG(
+ log_start_level, LOG_LEVEL_INVALID, "invalid [core] logging_start_level specified");
// register log flush on exit
if (FLAGS_logging_flush_on_exit) {
@@ -94,42 +90,15 @@ void dsn_log_init(const std::string &logging_factory_name,
}
}
-dsn_log_level_t dsn_log_get_start_level() { return dsn_log_start_level; }
+log_level_t get_log_start_level() { return log_start_level; }
-void dsn_log_set_start_level(dsn_log_level_t level) { dsn_log_start_level = level; }
+void set_log_start_level(log_level_t level) { log_start_level = level; }
-void dsn_logv(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *fmt,
- va_list args)
+void global_log(
+ const char *file, const char *function, const int line, log_level_t log_level, const char *str)
{
dsn::logging_provider *logger = dsn::logging_provider::instance();
- logger->dsn_logv(file, function, line, log_level, fmt, args);
-}
-
-void dsn_logf(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *fmt,
- ...)
-{
- va_list ap;
- va_start(ap, fmt);
- dsn_logv(file, function, line, log_level, fmt, ap);
- va_end(ap);
-}
-
-void dsn_log(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *str)
-{
- dsn::logging_provider *logger = dsn::logging_provider::instance();
- logger->dsn_log(file, function, line, log_level, str);
+ logger->log(file, function, line, log_level, str);
}
namespace dsn {
diff --git a/src/utils/logging_provider.h b/src/utils/logging_provider.h
index d668e0c7c..5061cee49 100644
--- a/src/utils/logging_provider.h
+++ b/src/utils/logging_provider.h
@@ -26,8 +26,6 @@
#pragma once
-#include <stdarg.h>
-
#include "utils/api_utilities.h"
#include "utils/command_manager.h"
#include "utils/factory_store.h"
@@ -58,18 +56,11 @@ public:
// not thread-safe
static void set_logger(logging_provider *logger);
- virtual void dsn_logv(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *fmt,
- va_list args) = 0;
-
- virtual void dsn_log(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *str) = 0;
+ virtual void log(const char *file,
+ const char *function,
+ const int line,
+ log_level_t log_level,
+ const char *str) = 0;
virtual void flush() = 0;
diff --git a/src/utils/simple_logger.cpp b/src/utils/simple_logger.cpp
index a555fd12f..f1464aa26 100644
--- a/src/utils/simple_logger.cpp
+++ b/src/utils/simple_logger.cpp
@@ -34,8 +34,8 @@
#include <sstream>
#include <vector>
+#include "absl/strings/string_view.h"
#include "runtime/api_layer1.h"
-#include "runtime/task/task_spec.h"
#include "utils/command_manager.h"
#include "utils/fail_point.h"
#include "utils/filesystem.h"
@@ -44,7 +44,6 @@
#include "utils/ports.h"
#include "utils/process_utils.h"
#include "utils/string_conv.h"
-#include "absl/strings/string_view.h"
#include "utils/strings.h"
#include "utils/time_utils.h"
@@ -72,7 +71,7 @@ DSN_DEFINE_validator(stderr_start_level, [](const char *level) -> bool {
return !utils::equals(level, "LOG_LEVEL_INVALID");
});
-static void print_header(FILE *fp, dsn_log_level_t log_level)
+static void print_header(FILE *fp, log_level_t log_level)
{
// The leading character of each log lines, corresponding to the log level
// D: Debug
@@ -98,7 +97,7 @@ static void print_header(FILE *fp, dsn_log_level_t log_level)
namespace {
-inline void process_fatal_log(dsn_log_level_t log_level)
+inline void process_fatal_log(log_level_t log_level)
{
if (dsn_likely(log_level < LOG_LEVEL_FATAL)) {
return;
@@ -118,38 +117,12 @@ inline void process_fatal_log(dsn_log_level_t log_level)
} // anonymous namespace
-screen_logger::screen_logger(bool short_header) { _short_header = short_header; }
+screen_logger::screen_logger(bool short_header) : _short_header(short_header) {}
screen_logger::~screen_logger(void) {}
-void screen_logger::dsn_logv(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *fmt,
- va_list args)
-{
- utils::auto_lock<::dsn::utils::ex_lock_nr> l(_lock);
-
- print_header(stdout, log_level);
- if (!_short_header) {
- printf("%s:%d:%s(): ", file, line, function);
- }
- vprintf(fmt, args);
- printf("\n");
-
- if (log_level >= LOG_LEVEL_ERROR) {
- ::fflush(stdout);
- }
-
- process_fatal_log(log_level);
-}
-
-void screen_logger::dsn_log(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *str)
+void screen_logger::log(
+ const char *file, const char *function, const int line, log_level_t log_level, const char *str)
{
utils::auto_lock<::dsn::utils::ex_lock_nr> l(_lock);
@@ -169,15 +142,14 @@ void screen_logger::dsn_log(const char *file,
void screen_logger::flush() { ::fflush(stdout); }
simple_logger::simple_logger(const char *log_dir)
+ : _log_dir(std::string(log_dir)),
+ _log(nullptr),
+ // we assume all valid entries are positive
+ _start_index(0),
+ _index(1),
+ _lines(0),
+ _stderr_start_level(enum_from_string(FLAGS_stderr_start_level, LOG_LEVEL_INVALID))
{
- _log_dir = std::string(log_dir);
- // we assume all valid entries are positive
- _start_index = 0;
- _index = 1;
- _lines = 0;
- _log = nullptr;
- _stderr_start_level = enum_from_string(FLAGS_stderr_start_level, LOG_LEVEL_INVALID);
-
// check existing log files
std::vector<std::string> sub_list;
CHECK(dsn::utils::filesystem::get_subfiles(_log_dir, sub_list, false),
@@ -185,25 +157,30 @@ simple_logger::simple_logger(const char *log_dir)
_log_dir);
for (auto &fpath : sub_list) {
auto &&name = dsn::utils::filesystem::get_file_name(fpath);
- if (name.length() <= 8 || name.substr(0, 4) != "log.")
+ if (name.length() <= 8 || name.substr(0, 4) != "log.") {
continue;
+ }
int index;
- if (1 != sscanf(name.c_str(), "log.%d.txt", &index) || index <= 0)
+ if (1 != sscanf(name.c_str(), "log.%d.txt", &index) || index <= 0) {
continue;
+ }
- if (index > _index)
+ if (index > _index) {
_index = index;
+ }
- if (_start_index == 0 || index < _start_index)
+ if (_start_index == 0 || index < _start_index) {
_start_index = index;
+ }
}
sub_list.clear();
- if (_start_index == 0)
+ if (_start_index == 0) {
_start_index = _index;
- else
+ } else {
++_index;
+ }
create_log_file();
@@ -225,27 +202,26 @@ simple_logger::simple_logger(const char *log_dir)
"reset-log-start-level - reset the log start level",
"reset-log-start-level [DEBUG | INFO | WARNING | ERROR | FATAL]",
[](const std::vector<std::string> &args) {
- dsn_log_level_t start_level;
+ log_level_t start_level;
if (args.size() == 0) {
- start_level =
- enum_from_string(FLAGS_logging_start_level, dsn_log_level_t::LOG_LEVEL_INVALID);
+ start_level = enum_from_string(FLAGS_logging_start_level, LOG_LEVEL_INVALID);
} else {
std::string level_str = "LOG_LEVEL_" + args[0];
- start_level =
- enum_from_string(level_str.c_str(), dsn_log_level_t::LOG_LEVEL_INVALID);
- if (start_level == dsn_log_level_t::LOG_LEVEL_INVALID) {
+ start_level = enum_from_string(level_str.c_str(), LOG_LEVEL_INVALID);
+ if (start_level == LOG_LEVEL_INVALID) {
return "ERROR: invalid level '" + args[0] + "'";
}
}
- dsn_log_set_start_level(start_level);
+ set_log_start_level(start_level);
return std::string("OK, current level is ") + enum_to_string(start_level);
}));
}
void simple_logger::create_log_file()
{
- if (_log != nullptr)
+ if (_log != nullptr) {
::fclose(_log);
+ }
_lines = 0;
@@ -280,51 +256,8 @@ void simple_logger::flush()
::fflush(stdout);
}
-void simple_logger::dsn_logv(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *fmt,
- va_list args)
-{
- va_list args2;
- if (log_level >= _stderr_start_level) {
- va_copy(args2, args);
- }
-
- utils::auto_lock<::dsn::utils::ex_lock> l(_lock);
-
- print_header(_log, log_level);
- if (!FLAGS_short_header) {
- fprintf(_log, "%s:%d:%s(): ", file, line, function);
- }
- vfprintf(_log, fmt, args);
- fprintf(_log, "\n");
- if (FLAGS_fast_flush || log_level >= LOG_LEVEL_ERROR) {
- ::fflush(_log);
- }
-
- if (log_level >= _stderr_start_level) {
- print_header(stdout, log_level);
- if (!FLAGS_short_header) {
- printf("%s:%d:%s(): ", file, line, function);
- }
- vprintf(fmt, args2);
- printf("\n");
- }
-
- process_fatal_log(log_level);
-
- if (++_lines >= 200000) {
- create_log_file();
- }
-}
-
-void simple_logger::dsn_log(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *str)
+void simple_logger::log(
+ const char *file, const char *function, const int line, log_level_t log_level, const char *str)
{
utils::auto_lock<::dsn::utils::ex_lock> l(_lock);
diff --git a/src/utils/simple_logger.h b/src/utils/simple_logger.h
index 9e502ac44..e9657a801 100644
--- a/src/utils/simple_logger.h
+++ b/src/utils/simple_logger.h
@@ -26,7 +26,6 @@
#pragma once
-#include <stdarg.h>
#include <cstdio>
#include <string>
@@ -46,24 +45,17 @@ public:
explicit screen_logger(bool short_header);
~screen_logger() override;
- void dsn_logv(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *fmt,
- va_list args) override;
-
- void dsn_log(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *str) override;
+ void log(const char *file,
+ const char *function,
+ const int line,
+ log_level_t log_level,
+ const char *str) override;
virtual void flush();
private:
::dsn::utils::ex_lock_nr _lock;
- bool _short_header;
+ const bool _short_header;
};
/*
@@ -76,18 +68,11 @@ public:
simple_logger(const char *log_dir);
~simple_logger() override;
- void dsn_logv(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *fmt,
- va_list args) override;
-
- void dsn_log(const char *file,
- const char *function,
- const int line,
- dsn_log_level_t log_level,
- const char *str) override;
+ void log(const char *file,
+ const char *function,
+ const int line,
+ log_level_t log_level,
+ const char *str) override;
void flush() override;
@@ -95,14 +80,14 @@ private:
void create_log_file();
private:
- std::string _log_dir;
::dsn::utils::ex_lock _lock; // use recursive lock to avoid dead lock when flush() is called
// in signal handler if cored for bad logging format reason.
+ const std::string _log_dir;
FILE *_log;
int _start_index;
int _index;
int _lines;
- dsn_log_level_t _stderr_start_level;
+ log_level_t _stderr_start_level;
};
} // namespace tools
} // namespace dsn
diff --git a/src/utils/test/endian_test.cpp b/src/utils/test/endian_test.cpp
index 6dc846270..dd8a807c6 100644
--- a/src/utils/test/endian_test.cpp
+++ b/src/utils/test/endian_test.cpp
@@ -21,6 +21,7 @@
#include "gtest/gtest.h"
#include "utils/endians.h"
+#include "utils/enum_helper.h"
using namespace dsn;
diff --git a/src/utils/test/logger.cpp b/src/utils/test/logger.cpp
index f8b213f6f..2b6e8a697 100644
--- a/src/utils/test/logger.cpp
+++ b/src/utils/test/logger.cpp
@@ -24,8 +24,9 @@
* THE SOFTWARE.
*/
+#include <boost/algorithm/string/predicate.hpp>
#include <errno.h>
-#include <stdarg.h>
+#include <fmt/core.h>
#include <stdio.h>
#include <unistd.h>
#include <algorithm>
@@ -36,99 +37,95 @@
#include "gtest/gtest.h"
#include "utils/api_utilities.h"
-#include "utils/error_code.h"
#include "utils/filesystem.h"
+#include "utils/flags.h"
#include "utils/logging_provider.h"
-#include "utils/ports.h"
#include "utils/safe_strerror_posix.h"
#include "utils/simple_logger.h"
-using std::vector;
-using std::string;
+namespace dsn {
+namespace tools {
-using namespace dsn;
-using namespace dsn::tools;
+DSN_DECLARE_uint64(max_number_of_log_files_on_disk);
-static const int simple_logger_gc_gap = 20;
+namespace {
-static void get_log_file_index(vector<int> &log_index)
+void get_log_file_index(std::vector<int> &log_index)
{
- vector<string> sub_list;
- string path = "./";
- ASSERT_TRUE(utils::filesystem::get_subfiles(path, sub_list, false));
+ std::vector<std::string> sub_list;
+ ASSERT_TRUE(dsn::utils::filesystem::get_subfiles("./", sub_list, false));
- for (auto &ptr : sub_list) {
- auto &&name = utils::filesystem::get_file_name(ptr);
- if (name.length() <= 8 || name.substr(0, 4) != "log.")
+ for (const auto &path : sub_list) {
+ const auto &name = dsn::utils::filesystem::get_file_name(path);
+ if (!boost::algorithm::starts_with(name, "log.")) {
continue;
+ }
+ if (!boost::algorithm::ends_with(name, ".txt")) {
+ continue;
+ }
+
int index;
- if (1 != sscanf(name.c_str(), "log.%d.txt", &index))
+ if (1 != sscanf(name.c_str(), "log.%d.txt", &index)) {
continue;
+ }
log_index.push_back(index);
}
}
-static void clear_files(vector<int> &log_index)
-{
- char file[256] = {};
- for (auto i : log_index) {
- snprintf_p(file, 256, "log.%d.txt", i);
- dsn::utils::filesystem::remove_path(string(file));
- }
-}
+// Don't name the dir with "./test", otherwise the whole utils test dir would be removed.
+const std::string kTestDir("./test_logger");
-static void prepare_test_dir()
+void prepare_test_dir()
{
- const char *dir = "./test";
- string dr(dir);
- ASSERT_TRUE(dsn::utils::filesystem::create_directory(dr));
- ASSERT_EQ(0, ::chdir(dir));
+ ASSERT_TRUE(dsn::utils::filesystem::create_directory(kTestDir));
+ ASSERT_EQ(0, ::chdir(kTestDir.c_str()));
}
-static void finish_test_dir()
+void remove_test_dir()
{
- const char *dir = "./test";
- ASSERT_EQ(0, ::chdir("..")) << "chdir failed, err = " << utils::safe_strerror(errno);
- ASSERT_TRUE(utils::filesystem::remove_path(dir)) << "remove_directory " << dir << " failed";
+ ASSERT_EQ(0, ::chdir("..")) << "chdir failed, err = " << dsn::utils::safe_strerror(errno);
+ ASSERT_TRUE(dsn::utils::filesystem::remove_path(kTestDir)) << "remove_directory " << kTestDir
+ << " failed";
}
-void log_print(logging_provider *logger, const char *fmt, ...)
-{
- va_list vl;
- va_start(vl, fmt);
- logger->dsn_logv(__FILE__, __FUNCTION__, __LINE__, LOG_LEVEL_DEBUG, fmt, vl);
- va_end(vl);
-}
+} // anonymous namespace
+
+#define LOG_PRINT(logger, ...) \
+ (logger)->log( \
+ __FILE__, __FUNCTION__, __LINE__, LOG_LEVEL_DEBUG, fmt::format(__VA_ARGS__).c_str())
-TEST(tools_common, simple_logger)
+TEST(LoggerTest, SimpleLogger)
{
// Deregister commands to avoid re-register error.
dsn::logging_provider::instance()->deregister_commands();
{
auto logger = std::make_unique<screen_logger>(true);
- log_print(logger.get(), "%s", "test_print");
- std::thread t([](screen_logger *lg) { log_print(lg, "%s", "test_print"); }, logger.get());
+ LOG_PRINT(logger.get(), "{}", "test_print");
+ std::thread t([](screen_logger *lg) { LOG_PRINT(lg, "{}", "test_print"); }, logger.get());
t.join();
logger->flush();
}
prepare_test_dir();
- // create multiple files
- for (unsigned int i = 0; i < simple_logger_gc_gap + 10; ++i) {
+
+ // Create redundant log files to test if their number could be restricted.
+ for (unsigned int i = 0; i < FLAGS_max_number_of_log_files_on_disk + 10; ++i) {
auto logger = std::make_unique<simple_logger>("./");
for (unsigned int i = 0; i != 1000; ++i) {
- log_print(logger.get(), "%s", "test_print");
+ LOG_PRINT(logger.get(), "{}", "test_print");
}
logger->flush();
}
- vector<int> index;
+ std::vector<int> index;
get_log_file_index(index);
- ASSERT_TRUE(!index.empty());
- sort(index.begin(), index.end());
- ASSERT_EQ(simple_logger_gc_gap, index.size());
- clear_files(index);
- finish_test_dir();
+ ASSERT_FALSE(index.empty());
+ ASSERT_EQ(FLAGS_max_number_of_log_files_on_disk, index.size());
+
+ remove_test_dir();
}
+
+} // namespace tools
+} // namespace dsn
diff --git a/src/utils/test/logging.cpp b/src/utils/test/logging.cpp
index 9ac1d3235..b1dd6ed22 100644
--- a/src/utils/test/logging.cpp
+++ b/src/utils/test/logging.cpp
@@ -32,51 +32,37 @@
#include "utils/fail_point.h"
#include "utils/fmt_logging.h"
-TEST(core, logging)
+TEST(LoggingTest, GlobalLog)
{
- dsn_log_level_t level = dsn_log_get_start_level();
- std::cout << "logging start level = " << level << std::endl;
- dsn_logf(__FILENAME__,
- __FUNCTION__,
- __LINE__,
- dsn_log_level_t::LOG_LEVEL_INFO,
- "in TEST(core, logging)");
- dsn_log(__FILENAME__, __FUNCTION__, __LINE__, dsn_log_level_t::LOG_LEVEL_INFO, "");
+ std::cout << "logging start level = " << enum_to_string(get_log_start_level()) << std::endl;
+ global_log(__FILENAME__, __FUNCTION__, __LINE__, LOG_LEVEL_INFO, "in TEST(core, logging)");
}
-TEST(core, logging_big_log)
+TEST(LoggingTest, GlobalLogBig)
{
std::string big_str(128000, 'x');
- dsn_logf(__FILENAME__,
- __FUNCTION__,
- __LINE__,
- dsn_log_level_t::LOG_LEVEL_INFO,
- "write big str %s",
- big_str.c_str());
+ global_log(__FILENAME__, __FUNCTION__, __LINE__, LOG_LEVEL_INFO, big_str.c_str());
}
-TEST(core, dlog)
+TEST(LoggingTest, LogMacro)
{
struct test_case
{
- enum dsn_log_level_t level;
+ log_level_t level;
std::string str;
- } tests[] = {{dsn_log_level_t::LOG_LEVEL_DEBUG, "This is a test"},
- {dsn_log_level_t::LOG_LEVEL_DEBUG, "\\x00%d\\x00\\x01%n/nm"},
- {dsn_log_level_t::LOG_LEVEL_INFO, "\\x00%d\\x00\\x01%n/nm"},
- {dsn_log_level_t::LOG_LEVEL_WARNING, "\\x00%d\\x00\\x01%n/nm"},
- {dsn_log_level_t::LOG_LEVEL_ERROR, "\\x00%d\\x00\\x01%n/nm"},
- {dsn_log_level_t::LOG_LEVEL_FATAL, "\\x00%d\\x00\\x01%n/nm"}};
+ } tests[] = {{LOG_LEVEL_DEBUG, "This is a test"},
+ {LOG_LEVEL_DEBUG, "\\x00%d\\x00\\x01%n/nm"},
+ {LOG_LEVEL_INFO, "\\x00%d\\x00\\x01%n/nm"},
+ {LOG_LEVEL_WARNING, "\\x00%d\\x00\\x01%n/nm"},
+ {LOG_LEVEL_ERROR, "\\x00%d\\x00\\x01%n/nm"},
+ {LOG_LEVEL_FATAL, "\\x00%d\\x00\\x01%n/nm"}};
dsn::fail::setup();
dsn::fail::cfg("coredump_for_fatal_log", "void(false)");
for (auto test : tests) {
- // Test logging_provider::dsn_log
- dlog_f(test.level, "dlog_f: sortkey = {}", test.str);
-
- // Test logging_provider::dsn_logv
- dlog(test.level, "dlog: sortkey = %s", test.str.c_str());
+ // Test logging_provider::log.
+ LOG(test.level, "LOG: sortkey = {}", test.str);
}
dsn::fail::teardown();
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pegasus.apache.org
For additional commands, e-mail: commits-help@pegasus.apache.org