feat: Distinguish log file names and limit log file count

src/failure_detector/test/run.sh

@@ -40,9 +40,9 @@ while read -r -a line; do
         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 ----"

src/meta/test/config-test.ini

@@ -43,11 +43,9 @@ tool = nativerun
 ;toollets = tracer, profiler
 ;fault_injector
 pause_on_start = false
-
-logging_start_level = LOG_LEVEL_DEBUG
+logging_start_level = LOG_LEVEL_INFO
 logging_factory_name = dsn::tools::simple_logger
 
-
 [tools.simple_logger]
 fast_flush = true
 short_header = false

src/meta/test/meta_state/run.sh

@@ -40,9 +40,9 @@ while read -r -a line; do
         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 ----"

src/meta/test/run.sh

@@ -54,9 +54,9 @@ if [ $? -ne 0 ]; then
     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 ----"

src/replica/backup/test/run.sh

@@ -45,7 +45,7 @@ fi
 ./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

src/replica/bulk_load/test/run.sh

@@ -45,7 +45,7 @@ fi
 ./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

src/replica/duplication/test/run.sh

@@ -45,7 +45,7 @@ fi
 ./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

src/replica/split/test/run.sh

@@ -45,7 +45,7 @@ fi
 ./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

src/replica/storage/simple_kv/run.sh

@@ -62,9 +62,9 @@ if [ -f core ] || ! grep ERR_OK out > /dev/null ; then
     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 ----"

src/replica/storage/simple_kv/test/run.sh

@@ -52,8 +52,8 @@ function run_single()
     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

src/runtime/global_config.h

@@ -169,8 +169,7 @@ struct service_spec
     std::vector<threadpool_spec> threadpool_specs;
     std::vector<service_app_spec> app_specs;
 
-    // auto-set
-    std::string dir_log;
+    std::string log_dir;
 
     service_spec() {}
     bool init();
@@ -186,6 +185,11 @@ CONFIG_FLD_STRING_LIST(toollets,
 CONFIG_FLD_STRING(data_dir,
                   "./data",
                   "The default directory to place the all the data, logs, coredump files, and etc.")
+CONFIG_FLD_STRING_BY_KEY(
+    log_dir,
+    "log_dir",
+    "",
+    "The directory to place the logs especially. 'data_dir' will be used if it's empty.")
 CONFIG_FLD(
     bool,
     bool,

src/runtime/service_api_c.cpp

@@ -44,6 +44,8 @@
 #include <gperftools/malloc_extension.h>
 #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"
@@ -53,8 +55,6 @@
 #include "runtime/rpc/rpc_engine.h"
 #include "runtime/rpc/rpc_host_port.h"
 #include "runtime/rpc/rpc_message.h"
-#include "security/init.h"
-#include "security/negotiation_manager.h"
 #include "runtime/service_app.h"
 #include "runtime/service_engine.h"
 #include "runtime/task/task.h"
@@ -63,6 +63,8 @@
 #include "runtime/task/task_spec.h"
 #include "runtime/task/task_worker.h"
 #include "runtime/tool_api.h"
+#include "security/init.h"
+#include "security/negotiation_manager.h"
 #include "utils/api_utilities.h"
 #include "utils/command_manager.h"
 #include "utils/config_api.h"
@@ -75,6 +77,7 @@
 #include "utils/join_point.h"
 #include "utils/logging_provider.h"
 #include "utils/process_utils.h"
+#include "utils/string_conv.h"
 #include "utils/strings.h"
 #include "utils/sys_exit_hook.h"
 #include "utils/threadpool_spec.h"
@@ -342,45 +345,29 @@ inline void dsn_global_init()
 
 static std::string dsn_log_prefixed_message_func()
 {
-    std::string res;
-    res.resize(100);
-    char *prefixed_message = const_cast<char *>(res.c_str());
-
-    int tid = dsn::utils::get_current_tid();
-    auto t = dsn::task::get_current_task_id();
+    const int tid = dsn::utils::get_current_tid();
+    const auto t = dsn::task::get_current_task_id();
     if (t) {
         if (nullptr != dsn::task::get_current_worker2()) {
-            sprintf(prefixed_message,
-                    "%6s.%7s%d.%016" PRIx64 ": ",
-                    dsn::task::get_current_node_name(),
-                    dsn::task::get_current_worker2()->pool_spec().name.c_str(),
-                    dsn::task::get_current_worker2()->index(),
-                    t);
+            return fmt::format("{}.{}{}.{:016}: ",
+                               dsn::task::get_current_node_name(),
+                               dsn::task::get_current_worker2()->pool_spec().name,
+                               dsn::task::get_current_worker2()->index(),
+                               t);
         } else {
-            sprintf(prefixed_message,
-                    "%6s.%7s.%05d.%016" PRIx64 ": ",
-                    dsn::task::get_current_node_name(),
-                    "io-thrd",
-                    tid,
-                    t);
+            return fmt::format(
+                "{}.io-thrd.{}.{:016}: ", dsn::task::get_current_node_name(), tid, t);
         }
     } else {
         if (nullptr != dsn::task::get_current_worker2()) {
-            sprintf(prefixed_message,
-                    "%6s.%7s%u: ",
-                    dsn::task::get_current_node_name(),
-                    dsn::task::get_current_worker2()->pool_spec().name.c_str(),
-                    dsn::task::get_current_worker2()->index());
+            return fmt::format("{}.{}{}: ",
+                               dsn::task::get_current_node_name(),
+                               dsn::task::get_current_worker2()->pool_spec().name,
+                               dsn::task::get_current_worker2()->index());
         } else {
-            sprintf(prefixed_message,
-                    "%6s.%7s.%05d: ",
-                    dsn::task::get_current_node_name(),
-                    "io-thrd",
-                    tid);
+            return fmt::format("{}.io-thrd.{}: ", dsn::task::get_current_node_name(), tid);
         }
     }
-
-    return res;
 }
 
 bool run(const char *config_file,
@@ -443,40 +430,66 @@ bool run(const char *config_file,
 
     ::dsn::utils::coredump::init();
 
-    // setup log dir
-    spec.dir_log = ::dsn::utils::filesystem::path_combine(cdir, "log");
-    dsn::utils::filesystem::create_directory(spec.dir_log);
+    // Setup log directory.
+    // If log_dir is not set, use data_dir/log instead.
+    if (spec.log_dir.empty()) {
+        spec.log_dir = ::dsn::utils::filesystem::path_combine(spec.data_dir, "log");
+        fmt::print(stdout, "log_dir is not set, use '{}' instead\n", spec.log_dir);
+    }
+    // Validate log_dir.
+    if (!dsn::utils::filesystem::is_absolute_path(spec.log_dir)) {
+        fmt::print(stderr, "log_dir({}) should be set with an absolute path\n", spec.log_dir);
+        return false;
+    }
+    dsn::utils::filesystem::create_directory(spec.log_dir);
 
-    // init tools
+    // Initialize tools.
     dsn_all.tool.reset(::dsn::utils::factory_store<::dsn::tools::tool_app>::create(
         spec.tool.c_str(), ::dsn::PROVIDER_TYPE_MAIN, spec.tool.c_str()));
     dsn_all.tool->install(spec);
 
-    // init app specs
+    // Initialize app specs.
     if (!spec.init_app_specs()) {
-        printf("error in config file %s, exit ...\n", config_file);
+        fmt::print(stderr, "error in config file {}, exit ...\n", config_file);
         return false;
     }
 
 #ifdef DSN_ENABLE_GPERF
     ::MallocExtension::instance()->SetMemoryReleaseRate(FLAGS_tcmalloc_release_rate);
 #endif
 
-    // init logging
-    dsn_log_init(spec.logging_factory_name, spec.dir_log, 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 minimum necessary
+    // Prepare the minimum necessary.
     ::dsn::service_engine::instance().init_before_toollets(spec);
 
     LOG_INFO("process({}) start: {}, date: {}",
              getpid(),
              dsn::utils::process_start_millis(),
              dsn::utils::process_start_date_time_mills());
 
-    // init toollets
-    for (auto it = spec.toollets.begin(); it != spec.toollets.end(); ++it) {
-        auto tlet =
-            dsn::tools::internal_use_only::get_toollet(it->c_str(), ::dsn::PROVIDER_TYPE_MAIN);
+    // Initialize toollets.
+    for (const auto &toollet_name : spec.toollets) {
+        auto tlet = dsn::tools::internal_use_only::get_toollet(toollet_name.c_str(),
+                                                               ::dsn::PROVIDER_TYPE_MAIN);
         CHECK_NOTNULL(tlet, "toolet not found");
         tlet->install(spec);
     }
@@ -507,29 +520,34 @@ bool run(const char *config_file,
         }
     }
 
-    // split app_name and app_index
-    std::list<std::string> applistkvs;
-    ::dsn::utils::split_args(app_list.c_str(), applistkvs, ';');
-
     // init apps
     for (auto &sp : spec.app_specs) {
-        if (!sp.run)
+        if (!sp.run) {
             continue;
+        }
 
         bool create_it = false;
-
         // create all apps
-        if (app_list == "") {
+        if (app_list.empty()) {
             create_it = true;
         } else {
-            for (auto &kv : applistkvs) {
-                std::list<std::string> argskvs;
-                ::dsn::utils::split_args(kv.c_str(), argskvs, '@');
-                if (std::string("apps.") + argskvs.front() == sp.config_section) {
-                    if (argskvs.size() < 2)
+            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, '@');
+                CHECK(!name_and_index.empty(),
+                      "app_name should be specified in '{}'",
+                      app_name_and_index);
+                if (std::string("apps.") + name_and_index.front() == sp.config_section) {
+                    if (name_and_index.size() < 2) {
                         create_it = true;
-                    else
-                        create_it = (std::stoi(argskvs.back()) == sp.index);
+                    } else {
+                        int32_t index = 0;
+                        const auto index_str = name_and_index.back();
+                        CHECK(dsn::buf2int32(index_str, index),
+                              "'{}' is not a valid index",
+                              index_str);
+                        create_it = (index == sp.index);
+                    }
                     break;
                 }
             }
@@ -540,10 +558,11 @@ bool run(const char *config_file,
         }
     }
 
-    if (dsn::service_engine::instance().get_all_nodes().size() == 0) {
-        printf("no app are created, usually because \n"
-               "app_name is not specified correctly, should be 'xxx' in [apps.xxx]\n"
-               "or app_index (1-based) is greater than specified count in config file\n");
+    if (dsn::service_engine::instance().get_all_nodes().empty()) {
+        fmt::print(stderr,
+                   "no app are created, usually because \n"
+                   "app_name is not specified correctly, should be 'xxx' in [apps.xxx]\n"
+                   "or app_index (1-based) is greater than specified count in config file\n");
         exit(1);
     }
 

src/runtime/test/run.sh

@@ -40,9 +40,9 @@ while read -r -a line; do
         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 ----"

src/runtime/tracer.cpp

@@ -408,7 +408,7 @@ void tracer::install(service_spec &spec)
             "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.xx.txt)>",
+            "<log_file_name(e.g., replica.log.yyyyMMdd_hhmmss_SSS)>",
             tracer_log_flow);
     });
 }