Skip to content

Commit

Permalink
refactor(logging): drop the unnecessary interface, use log instead …
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@empiredan
empiredan authored Dec 22, 2023
1 parent 05466d3 commit 2f551c7
Show file tree
Hide file tree
Showing 12 changed files with 159 additions and 319 deletions.
4 changes: 2 additions & 2 deletions src/replica/replica_2pc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/security/sasl_init.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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;
}

Expand Down
9 changes: 0 additions & 9 deletions src/runtime/task/task_spec.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
56 changes: 21 additions & 35 deletions src/utils/api_utilities.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -48,48 +47,35 @@
@{
*/

typedef enum dsn_log_level_t {
enum log_level_t
{
LOG_LEVEL_DEBUG,
LOG_LEVEL_INFO,
LOG_LEVEL_WARNING,
LOG_LEVEL_ERROR,
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)) { \
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
17 changes: 9 additions & 8 deletions src/utils/fmt_logging.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 { \
Expand Down
49 changes: 9 additions & 40 deletions src/utils/logging.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
Expand Down Expand Up @@ -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) {
Expand All @@ -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 {
Expand Down
19 changes: 5 additions & 14 deletions src/utils/logging_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@

#pragma once

#include <stdarg.h>

#include "utils/api_utilities.h"
#include "utils/command_manager.h"
#include "utils/factory_store.h"
Expand Down Expand Up @@ -58,18 +56,11 @@ class logging_provider
// 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;

Expand Down
Loading

0 comments on commit 2f551c7

Please sign in to comment.