feat: Distinguish log file names (#2010)
This patch fills the log file name with the role name. For example, the replica
server log file name is in the form of `replica.log.<yyyyMMdd_hhmmss_SSS>`,
while the meta server log file name is in the form of `meta.log.<yyyyMMdd_hhmmss_SSS>`.
If the role name is empty (typically in unit tests), it will fall back to use
a new configudation `base_name` in `tools.simple_logger` section, it's default
as `pegasus`. Then, the log file is in the form of `pegasus.log.<yyyyMMdd_hhmmss_SSS>`.
```diff
[tools.simple_logger]
+base_name = pegasus
```
diff --git a/src/failure_detector/test/run.sh b/src/failure_detector/test/run.sh
index be8861e..067eee1 100755
--- a/src/failure_detector/test/run.sh
+++ b/src/failure_detector/test/run.sh
@@ -40,9 +40,9 @@
echo "run dsn.failure_detector.tests $test_case failed"
echo "---- ls ----"
ls -l
- if find . -name log.1.txt; then
- echo "---- tail -n 100 log.1.txt ----"
- tail -n 100 `find . -name log.1.txt`
+ if [ `find . -name pegasus.log.* | wc -l` -ne 0 ]; then
+ echo "---- tail -n 100 pegasus.log.* ----"
+ tail -n 100 `find . -name pegasus.log.*`
fi
if [ -f core ]; then
echo "---- gdb ./dsn.failure_detector.tests core ----"
diff --git a/src/meta/test/meta_state/run.sh b/src/meta/test/meta_state/run.sh
index 62db6b8..cb8ad50 100755
--- a/src/meta/test/meta_state/run.sh
+++ b/src/meta/test/meta_state/run.sh
@@ -40,9 +40,9 @@
echo "run dsn_meta_state_tests $test_case failed"
echo "---- ls ----"
ls -l
- if find . -name log.1.txt; then
- echo "---- tail -n 100 log.1.txt ----"
- tail -n 100 `find . -name log.1.txt`
+ if [ `find . -name pegasus.log.* | wc -l` -ne 0 ]; then
+ echo "---- tail -n 100 pegasus.log.* ----"
+ tail -n 100 `find . -name pegasus.log.*`
fi
if [ -f core ]; then
echo "---- gdb ./dsn_meta_state_tests core ----"
diff --git a/src/meta/test/run.sh b/src/meta/test/run.sh
index fe9a116..a27f485 100755
--- a/src/meta/test/run.sh
+++ b/src/meta/test/run.sh
@@ -54,9 +54,9 @@
echo "run dsn.meta.test failed"
echo "---- ls ----"
ls -l
- if find . -name log.1.txt; then
- echo "---- tail -n 100 log.1.txt ----"
- tail -n 100 `find . -name log.1.txt`
+ if [ `find . -name pegasus.log.* | wc -l` -ne 0 ]; then
+ echo "---- tail -n 100 pegasus.log.* ----"
+ tail -n 100 `find . -name pegasus.log.*`
fi
if [ -f core ]; then
echo "---- gdb ./dsn.meta.test core ----"
diff --git a/src/replica/backup/test/run.sh b/src/replica/backup/test/run.sh
index eb88404..87d08b7 100755
--- a/src/replica/backup/test/run.sh
+++ b/src/replica/backup/test/run.sh
@@ -45,7 +45,7 @@
./dsn_replica_backup_test
if [ $? -ne 0 ]; then
- tail -n 100 data/log/log.1.txt
+ tail -n 100 `find . -name pegasus.log.*`
if [ -f core ]; then
gdb ./dsn_replica_backup_test core -ex "bt"
fi
diff --git a/src/replica/bulk_load/test/run.sh b/src/replica/bulk_load/test/run.sh
index 7689036..fff86cd 100755
--- a/src/replica/bulk_load/test/run.sh
+++ b/src/replica/bulk_load/test/run.sh
@@ -45,7 +45,7 @@
./dsn_replica_bulk_load_test
if [ $? -ne 0 ]; then
- tail -n 100 data/log/log.1.txt
+ tail -n 100 `find . -name pegasus.log.*`
if [ -f core ]; then
gdb ./dsn_replica_bulk_load_test core -ex "bt"
fi
diff --git a/src/replica/duplication/test/run.sh b/src/replica/duplication/test/run.sh
index 90dee78..bab5b57 100755
--- a/src/replica/duplication/test/run.sh
+++ b/src/replica/duplication/test/run.sh
@@ -45,7 +45,7 @@
./dsn_replica_dup_test
if [ $? -ne 0 ]; then
- tail -n 100 data/log/log.1.txt
+ tail -n 100 `find . -name pegasus.log.*`
if [ -f core ]; then
gdb ./dsn_replica_dup_test core -ex "bt"
fi
diff --git a/src/replica/split/test/run.sh b/src/replica/split/test/run.sh
index bd75b96..66d4340 100755
--- a/src/replica/split/test/run.sh
+++ b/src/replica/split/test/run.sh
@@ -45,7 +45,7 @@
./dsn_replica_split_test
if [ $? -ne 0 ]; then
- tail -n 100 data/log/log.1.txt
+ tail -n 100 `find . -name pegasus.log.*`
if [ -f core ]; then
gdb ./dsn_replica_split_test core -ex "bt"
fi
diff --git a/src/replica/storage/simple_kv/run.sh b/src/replica/storage/simple_kv/run.sh
index cf7c038..debaf5b 100755
--- a/src/replica/storage/simple_kv/run.sh
+++ b/src/replica/storage/simple_kv/run.sh
@@ -62,9 +62,9 @@
ls -l
echo "---- head -n 100 out ----"
head -n 100 out
- if [ -f data/logs/log.1.txt ]; then
- echo "---- tail -n 100 log.1.txt ----"
- tail -n 100 data/logs/log.1.txt
+ if [ `find data/logs -name pegasus.log.* | wc -l` -ne 0 ]; then
+ echo "---- tail -n 100 pegasus.log.* ----"
+ tail -n 100 `find data/logs -name pegasus.log.*`
fi
if [ -f core ]; then
echo "---- gdb ./dsn.replication.simple_kv core ----"
diff --git a/src/replica/storage/simple_kv/test/run.sh b/src/replica/storage/simple_kv/test/run.sh
index 9c8d073..d09bf71 100755
--- a/src/replica/storage/simple_kv/test/run.sh
+++ b/src/replica/storage/simple_kv/test/run.sh
@@ -52,8 +52,8 @@
echo "${bin} ${prefix}.ini ${prefix}.act"
${bin} ${prefix}.ini ${prefix}.act
ret=$?
- if find . -name log.1.txt &>/dev/null; then
- log=`find . -name log.1.txt`
+ if [ `find . -name pegasus.log.* | wc -l` -ne 0 ]; then
+ log=`find . -name pegasus.log.*`
cat ${log} | grep -v FAILURE_DETECT | grep -v BEACON | grep -v beacon | grep -v THREAD_POOL_FD >${prefix}.log
rm ${log}
fi
diff --git a/src/runtime/service_api_c.cpp b/src/runtime/service_api_c.cpp
index fb4e515..276fec4 100644
--- a/src/runtime/service_api_c.cpp
+++ b/src/runtime/service_api_c.cpp
@@ -45,6 +45,7 @@
#endif
#include "fmt/core.h"
+#include "fmt/format.h"
#include "perf_counter/perf_counters.h"
#include "runtime/api_layer1.h"
#include "runtime/api_task.h"
@@ -457,8 +458,25 @@
::MallocExtension::instance()->SetMemoryReleaseRate(FLAGS_tcmalloc_release_rate);
#endif
- // init logging
- dsn_log_init(spec.logging_factory_name, spec.log_dir, dsn_log_prefixed_message_func);
+ // Extract app_names.
+ std::list<std::string> app_names_and_indexes;
+ ::dsn::utils::split_args(app_list.c_str(), app_names_and_indexes, ';');
+ std::vector<std::string> app_names;
+ for (const auto &app_name_and_index : app_names_and_indexes) {
+ std::vector<std::string> name_and_index;
+ ::dsn::utils::split_args(app_name_and_index.c_str(), name_and_index, '@');
+ if (name_and_index.empty()) {
+ fmt::print(stderr, "app_name should be specified in '{}'", app_name_and_index);
+ return false;
+ }
+ app_names.push_back(name_and_index[0]);
+ }
+
+ // Initialize logging.
+ dsn_log_init(spec.logging_factory_name,
+ spec.log_dir,
+ fmt::format("{}", fmt::join(app_names, ".")),
+ dsn_log_prefixed_message_func);
// Prepare the minimum necessary.
::dsn::service_engine::instance().init_before_toollets(spec);
@@ -513,9 +531,6 @@
if (app_list.empty()) {
create_it = true;
} else {
- // Extract app_names.
- std::list<std::string> app_names_and_indexes;
- ::dsn::utils::split_args(app_list.c_str(), app_names_and_indexes, ';');
for (const auto &app_name_and_index : app_names_and_indexes) {
std::vector<std::string> name_and_index;
::dsn::utils::split_args(app_name_and_index.c_str(), name_and_index, '@');
diff --git a/src/runtime/test/run.sh b/src/runtime/test/run.sh
index 648ccb4..ebb7925 100755
--- a/src/runtime/test/run.sh
+++ b/src/runtime/test/run.sh
@@ -40,9 +40,9 @@
echo "run dsn_runtime_tests $test_case failed"
echo "---- ls ----"
ls -l
- if find . -name log.1.txt; then
- echo "---- tail -n 100 log.1.txt ----"
- tail -n 100 `find . -name log.1.txt`
+ if [ `find . -name pegasus.log.* | wc -l` -ne 0 ]; then
+ echo "---- tail -n 100 pegasus.log.* ----"
+ tail -n 100 `find . -name pegasus.log.*`
fi
if [ -f core ]; then
echo "---- gdb ./dsn_runtime_tests core ----"
diff --git a/src/runtime/tracer.cpp b/src/runtime/tracer.cpp
index 2c1d24e..e85d88e 100644
--- a/src/runtime/tracer.cpp
+++ b/src/runtime/tracer.cpp
@@ -408,7 +408,7 @@
"tracer.find",
"Find related logs",
"[forward|f|backward|b] [rpc|r|task|t] [trace_id|task_id(e.g., a023003920302390)] "
- "<log_file_name(e.g., log.yyyyMMdd_hhmmss_SSS)>",
+ "<log_file_name(e.g., replica.log.yyyyMMdd_hhmmss_SSS)>",
tracer_log_flow);
});
}
diff --git a/src/utils/logging.cpp b/src/utils/logging.cpp
index 0f0484c..66aeeb8 100644
--- a/src/utils/logging.cpp
+++ b/src/utils/logging.cpp
@@ -28,6 +28,7 @@
#include <functional>
#include <memory>
#include <string>
+#include <utility>
#include "runtime/tool_api.h"
#include "simple_logger.h"
@@ -61,7 +62,7 @@
void set_log_prefixed_message_func(std::function<std::string()> func)
{
- log_prefixed_message_func = func;
+ log_prefixed_message_func = std::move(func);
}
} // namespace dsn
@@ -73,7 +74,8 @@
void dsn_log_init(const std::string &logging_factory_name,
const std::string &log_dir,
- std::function<std::string()> dsn_log_prefixed_message_func)
+ const std::string &role_name,
+ const std::function<std::string()> &dsn_log_prefixed_message_func)
{
log_start_level = enum_from_string(FLAGS_logging_start_level, LOG_LEVEL_INVALID);
@@ -86,7 +88,7 @@
}
dsn::logging_provider *logger = dsn::utils::factory_store<dsn::logging_provider>::create(
- logging_factory_name.c_str(), dsn::PROVIDER_TYPE_MAIN, log_dir.c_str());
+ logging_factory_name.c_str(), dsn::PROVIDER_TYPE_MAIN, log_dir.c_str(), role_name.c_str());
dsn::logging_provider::set_logger(logger);
if (dsn_log_prefixed_message_func != nullptr) {
@@ -117,7 +119,7 @@
logging_provider *logging_provider::create_default_instance()
{
- return new tools::screen_logger(true);
+ return new tools::screen_logger(nullptr, nullptr);
}
void logging_provider::set_logger(logging_provider *logger) { _logger.reset(logger); }
diff --git a/src/utils/logging_provider.h b/src/utils/logging_provider.h
index 910e418..8adec47 100644
--- a/src/utils/logging_provider.h
+++ b/src/utils/logging_provider.h
@@ -40,12 +40,12 @@
{
public:
template <typename T>
- static logging_provider *create(const char *log_dir)
+ static logging_provider *create(const char *log_dir, const char *role_name)
{
- return new T(log_dir);
+ return new T(log_dir, role_name);
}
- typedef logging_provider *(*factory)(const char *);
+ typedef logging_provider *(*factory)(const char *, const char *);
public:
virtual ~logging_provider() = default;
@@ -88,4 +88,5 @@
extern void dsn_log_init(const std::string &logging_factory_name,
const std::string &log_dir,
- std::function<std::string()> dsn_log_prefixed_message_func);
+ const std::string &role_name,
+ const std::function<std::string()> &dsn_log_prefixed_message_func);
diff --git a/src/utils/simple_logger.cpp b/src/utils/simple_logger.cpp
index 2765f15..db0e95b 100644
--- a/src/utils/simple_logger.cpp
+++ b/src/utils/simple_logger.cpp
@@ -53,6 +53,7 @@
#include "utils/process_utils.h"
#include "utils/safe_strerror_posix.h"
#include "utils/string_conv.h"
+#include "utils/strings.h"
#include "utils/time_utils.h"
DSN_DEFINE_uint64(tools.simple_logger,
@@ -88,6 +89,8 @@
return LOG_LEVEL_DEBUG <= level && level <= LOG_LEVEL_FATAL;
});
+DSN_DEFINE_string(tools.simple_logger, base_name, "pegasus", "The default base name for log file");
+
DSN_DECLARE_string(logging_start_level);
namespace dsn {
@@ -166,8 +169,6 @@
} // anonymous namespace
-screen_logger::screen_logger(bool short_header) : _short_header(short_header) {}
-
void screen_logger::print_header(log_level_t log_level)
{
::dsn::tools::print_header(stdout, LOG_LEVEL_COUNT, log_level);
@@ -204,14 +205,15 @@
void screen_logger::flush() { ::fflush(stdout); }
-simple_logger::simple_logger(const char *log_dir)
+simple_logger::simple_logger(const char *log_dir, const char *role_name)
: _log_dir(std::string(log_dir)),
_log(nullptr),
_file_bytes(0),
_stderr_start_level(enum_from_string(FLAGS_stderr_start_level, LOG_LEVEL_INVALID))
{
// Use 'role_name' if it is specified, otherwise use 'base_name'.
- const std::string symlink_name("log");
+ const std::string symlink_name(
+ fmt::format("{}.log", utils::is_empty(role_name) ? FLAGS_base_name : role_name));
_file_name_prefix = fmt::format("{}.", symlink_name);
_symlink_path = utils::filesystem::path_combine(_log_dir, symlink_name);
diff --git a/src/utils/simple_logger.h b/src/utils/simple_logger.h
index 3a8b001..7fe8d79 100644
--- a/src/utils/simple_logger.h
+++ b/src/utils/simple_logger.h
@@ -44,7 +44,7 @@
class screen_logger : public logging_provider
{
public:
- explicit screen_logger(bool short_header);
+ explicit screen_logger(const char *, const char *) : _short_header(true) {}
~screen_logger() override = default;
void log(const char *file,
@@ -74,7 +74,9 @@
class simple_logger : public logging_provider
{
public:
- simple_logger(const char *log_dir);
+ // 'log_dir' is the directory to store log files, 'role_name' is the name of the process,
+ // such as 'replica_server', 'meta_server' in Pegasus.
+ simple_logger(const char *log_dir, const char *role_name);
~simple_logger() override;
void log(const char *file,
diff --git a/src/utils/test/clear.sh b/src/utils/test/clear.sh
index d113af3..cc0c873 100755
--- a/src/utils/test/clear.sh
+++ b/src/utils/test/clear.sh
@@ -24,4 +24,4 @@
# THE SOFTWARE.
-rm -rf dsn.utils.tests.xml log*.txt
+rm -rf dsn.utils.tests.xml pegasus*.txt
diff --git a/src/utils/test/logger.cpp b/src/utils/test/logger.cpp
index 5e0f59a..0ef7bcb 100644
--- a/src/utils/test/logger.cpp
+++ b/src/utils/test/logger.cpp
@@ -81,7 +81,7 @@
ASSERT_TRUE(utils::filesystem::get_subfiles(test_dir, sub_list, false));
file_names.clear();
- std::regex pattern(R"(log\.[0-9]{8}_[0-9]{6}_[0-9]{3})");
+ std::regex pattern(R"(SimpleLogger\.log\.[0-9]{8}_[0-9]{6}_[0-9]{3})");
for (const auto &path : sub_list) {
std::string name(utils::filesystem::get_file_name(path));
if (std::regex_match(name, pattern)) {
@@ -147,7 +147,7 @@
TEST_F(logger_test, screen_logger_test)
{
- auto logger = std::make_unique<screen_logger>(true);
+ auto logger = std::make_unique<screen_logger>(nullptr, nullptr);
LOG_PRINT(logger.get(), "{}", "test_print");
std::thread t([](screen_logger *lg) { LOG_PRINT(lg, "{}", "test_print"); }, logger.get());
t.join();
@@ -161,7 +161,7 @@
std::set<std::string> before_files;
NO_FATALS(get_log_files(before_files));
- auto logger = std::make_unique<simple_logger>(test_dir.c_str());
+ auto logger = std::make_unique<simple_logger>(test_dir.c_str(), "SimpleLogger");
for (unsigned int i = 0; i != 1000; ++i) {
LOG_PRINT(logger.get(), "{}", "test_print");
}
diff --git a/src/utils/test/main.cpp b/src/utils/test/main.cpp
index 2be6302..0bdfb7f 100644
--- a/src/utils/test/main.cpp
+++ b/src/utils/test/main.cpp
@@ -25,7 +25,7 @@
{
testing::InitGoogleTest(&argc, argv);
- dsn_log_init("dsn::tools::simple_logger", "./", nullptr);
+ dsn_log_init("dsn::tools::simple_logger", "./", "test", nullptr);
dsn::flags_initialize();
diff --git a/src/utils/test/run.sh b/src/utils/test/run.sh
index 90698a7..06c8b72 100755
--- a/src/utils/test/run.sh
+++ b/src/utils/test/run.sh
@@ -36,9 +36,9 @@
echo "run dsn_utils_tests failed"
echo "---- ls ----"
ls -l
- if find . -name log.1.txt; then
- echo "---- tail -n 100 log.1.txt ----"
- tail -n 100 `find . -name log.1.txt`
+ if [ `find . -name pegasus.log.* | wc -l` -ne 0 ]; then
+ echo "---- tail -n 100 pegasus.log.* ----"
+ tail -n 100 `find . -name pegasus.log.*`
fi
if [ -f core ]; then
echo "---- gdb ./dsn_utils_tests core ----"
diff --git a/src/zookeeper/test/run.sh b/src/zookeeper/test/run.sh
index 38608a2..820dbda 100755
--- a/src/zookeeper/test/run.sh
+++ b/src/zookeeper/test/run.sh
@@ -37,9 +37,9 @@
echo "run dsn.zookeeper.tests failed"
echo "---- ls ----"
ls -l
- if find . -name log.1.txt; then
- echo "---- tail -n 100 log.1.txt ----"
- tail -n 100 `find . -name log.1.txt`
+ if [ `find . -name pegasus.log.* | wc -l` -ne 0 ]; then
+ echo "---- tail -n 100 pegasus.log.* ----"
+ tail -n 100 `find . -name pegasus.log.*`
fi
if [ -f core ]; then
echo "---- gdb ./dsn.zookeeper.tests core ----"
