From cfb872951c94dd869f4531929599e1de830660d2 Mon Sep 17 00:00:00 2001 From: Julius Athenstaedt Date: Mon, 5 Feb 2024 12:35:50 +0100 Subject: [PATCH 1/3] LIBGKFS_LOG_PER_PROCESS environment variable can be used to create logs in separate files for each process of the client. --- include/client/env.hpp | 1 + include/client/logging.hpp | 7 ++++--- src/client/logging.cpp | 15 +++++++++++++-- src/client/preload_context.cpp | 5 ++++- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/include/client/env.hpp b/include/client/env.hpp index c35ae1ad3..2a58ecf81 100644 --- a/include/client/env.hpp +++ b/include/client/env.hpp @@ -45,6 +45,7 @@ static constexpr auto LOG_SYSCALL_FILTER = ADD_PREFIX("LOG_SYSCALL_FILTER"); #endif static constexpr auto LOG_OUTPUT = ADD_PREFIX("LOG_OUTPUT"); +static constexpr auto LOG_PER_PROCESS = ADD_PREFIX("LOG_PER_PROCESS"); static constexpr auto LOG_OUTPUT_TRUNC = ADD_PREFIX("LOG_OUTPUT_TRUNC"); static constexpr auto CWD = ADD_PREFIX("CWD"); static constexpr auto HOSTS_FILE = ADD_PREFIX("HOSTS_FILE"); diff --git a/include/client/logging.hpp b/include/client/logging.hpp index ee570542f..95a79e043 100644 --- a/include/client/logging.hpp +++ b/include/client/logging.hpp @@ -273,7 +273,7 @@ protected: struct logger { - logger(const std::string& opts, const std::string& path, bool trunc + logger(const std::string& opts, const std::string& path, int log_per_process, bool trunc #ifdef GKFS_DEBUG_BUILD , const std::string& filter, int verbosity @@ -293,7 +293,7 @@ struct logger { static_buffer buffer; detail::format_timestamp_to(buffer, timezone_); - fmt::format_to(buffer, "[{}] [{}] ", ::syscall_no_intercept(SYS_gettid), + fmt::format_to(buffer, "[{}] [{}] ", log_process_id_, lookup_level_name(level)); if(!!(level & log::debug)) { @@ -336,7 +336,7 @@ struct logger { static_buffer prefix; detail::format_timestamp_to(prefix); - fmt::format_to(prefix, "[{}] [{}] ", ::syscall_no_intercept(SYS_gettid), + fmt::format_to(prefix, "[{}] [{}] ", log_process_id_, lookup_level_name(level)); char buffer[max_buffer_size]; @@ -402,6 +402,7 @@ struct logger { } int log_fd_; + int log_process_id_; log_level log_mask_; #ifdef GKFS_DEBUG_BUILD diff --git a/src/client/logging.cpp b/src/client/logging.cpp index 2f060053c..cd9c47ee6 100644 --- a/src/client/logging.cpp +++ b/src/client/logging.cpp @@ -31,6 +31,7 @@ #include #include #include +#include extern "C" { #include @@ -43,6 +44,8 @@ extern "C" { #endif +namespace fs = std::filesystem; + namespace { enum class split_str_mode { is_any_of, @@ -280,7 +283,8 @@ process_log_filter(const std::string& log_filter) { #endif // GKFS_DEBUG_BUILD -logger::logger(const std::string& opts, const std::string& path, bool trunc +logger::logger(const std::string& opts, const std::string& path, + int log_per_process, bool trunc #ifdef GKFS_DEBUG_BUILD , const std::string& filter, int verbosity @@ -290,6 +294,7 @@ logger::logger(const std::string& opts, const std::string& path, bool trunc /* use stderr by default */ log_fd_ = 2; + log_process_id_ = ::syscall_no_intercept(SYS_gettid); log_mask_ = process_log_options(opts); #ifdef GKFS_DEBUG_BUILD @@ -304,11 +309,17 @@ logger::logger(const std::string& opts, const std::string& path, bool trunc flags &= ~O_TRUNC; } + std::string file_path = path; + if(log_per_process) { + fs::create_directories(path); + file_path += "/" + std::to_string(log_process_id_); + } + // we use ::open() here rather than ::syscall_no_intercept(SYS_open) // because we want the call to be intercepted by our hooks, which // allows us to categorize the resulting fd as 'internal' and // relocate it to our private range - int fd = ::open(path.c_str(), flags, 0600); + int fd = ::open(file_path.c_str(), flags, 0600); if(fd == -1) { log(gkfs::log::error, __func__, __LINE__, diff --git a/src/client/preload_context.cpp b/src/client/preload_context.cpp index f8ac782fe..fb6e61c6d 100644 --- a/src/client/preload_context.cpp +++ b/src/client/preload_context.cpp @@ -76,6 +76,9 @@ PreloadContext::init_logging() { const std::string log_output = gkfs::env::get_var( gkfs::env::LOG_OUTPUT, gkfs::config::log::client_log_path); + const int log_per_process = std::atoi( + gkfs::env::get_var(gkfs::env::LOG_PER_PROCESS, "0").c_str()); + #ifdef GKFS_DEBUG_BUILD // atoi returns 0 if no int conversion can be performed, which works // for us since if the user provides a non-numeric value we can just treat @@ -92,7 +95,7 @@ PreloadContext::init_logging() { const bool log_trunc = (!trunc_val.empty() && trunc_val[0] != '0'); - gkfs::log::create_global_logger(log_opts, log_output, log_trunc + gkfs::log::create_global_logger(log_opts, log_output, log_per_process, log_trunc #ifdef GKFS_DEBUG_BUILD , log_filter, log_verbosity -- GitLab From b45e7f0a482ce27f41e1b42943496811e08cf1ad Mon Sep 17 00:00:00 2001 From: Julius Athenstaedt Date: Tue, 6 Feb 2024 11:50:44 +0100 Subject: [PATCH 2/3] update CHANGELOG, README and docs. use fmt --- CHANGELOG.md | 3 +++ README.md | 3 ++- docs/sphinx/users/running.md | 5 +++-- include/client/logging.hpp | 3 ++- src/client/logging.cpp | 4 ++-- src/client/preload_context.cpp | 8 +++++--- 6 files changed, 17 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e945f6bf..ad098181c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### New +- Support mtime with option gkfs::config::metadata::use_mtime ([!225](https://storage.bsc.es/gitlab/hpc/gekkofs/-/issues/225)) +- Unify -d and -p flags in scripts/compile_dep.sh and dl_dep.sh ([!188](https://storage.bsc.es/gitlab/hpc/gekkofs/-/issues/188)). +- Logfile per process for the client, activated with LIBGKFS_LOG_PER_PROCESS ([!278](https://storage.bsc.es/gitlab/hpc/gekkofs/-/issues/278)). - Additional tests to increase code coverage ([!141](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/141)). - GKFS_ENABLE_UNUSED_FUNCTIONS added to disable code to increase code coverage. ([!141](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/141)). diff --git a/README.md b/README.md index 6f571eff6..d3175f2e8 100644 --- a/README.md +++ b/README.md @@ -200,7 +200,8 @@ usage: gkfs [-h/--help] [-r/--rootdir ] [-m/--mountdir ] [-a/--args The following environment variables can be used to enable logging in the client library: `LIBGKFS_LOG=` and `LIBGKFS_LOG_OUTPUT=` to configure the output module and set the path to the log file of the client library. If not path is specified in `LIBGKFS_LOG_OUTPUT`, the client library will send log messages -to `/tmp/gkfs_client.log`. +to `/tmp/gkfs_client.log`. With `LIBGKFS_LOG_PER_PROCESS=1` it is possible to write separate logs per client process. +If so, the path specified in `LIBGKFS_LOG_OUTPUT` is a directory but should not end with a `/`. The following modules are available: diff --git a/docs/sphinx/users/running.md b/docs/sphinx/users/running.md index dff987d91..855f4067d 100644 --- a/docs/sphinx/users/running.md +++ b/docs/sphinx/users/running.md @@ -180,7 +180,8 @@ usage: gkfs [-h/--help] [-r/--rootdir ] [-m/--mountdir ] [-a/--args The following environment variables can be used to enable logging in the client library: `LIBGKFS_LOG=` and `LIBGKFS_LOG_OUTPUT=` to configure the output module and set the path to the log file of the client library. If not path is specified in `LIBGKFS_LOG_OUTPUT`, the client library will send log messages -to `/tmp/gkfs_client.log`. +to `/tmp/gkfs_client.log`. With `LIBGKFS_LOG_PER_PROCESS=1` it is possible to write separate logs per client process. +If so, the path specified in `LIBGKFS_LOG_OUTPUT` is a directory but should not end with a `/`. The following modules are available: @@ -216,4 +217,4 @@ the logging subsystem to truncate the file used for logging, rather than append For the daemon, the `GKFS_DAEMON_LOG_PATH=` environment variable can be provided to set the path to the log file, and the log module can be selected with the `GKFS_DAEMON_LOG_LEVEL={off,critical,err,warn,info,debug,trace}` -environment variable whereas `trace` produces the most trace records while `info` is the default value. \ No newline at end of file +environment variable whereas `trace` produces the most trace records while `info` is the default value. diff --git a/include/client/logging.hpp b/include/client/logging.hpp index 95a79e043..8a7196a56 100644 --- a/include/client/logging.hpp +++ b/include/client/logging.hpp @@ -273,7 +273,8 @@ protected: struct logger { - logger(const std::string& opts, const std::string& path, int log_per_process, bool trunc + logger(const std::string& opts, const std::string& path, + bool log_per_process, bool trunc #ifdef GKFS_DEBUG_BUILD , const std::string& filter, int verbosity diff --git a/src/client/logging.cpp b/src/client/logging.cpp index cd9c47ee6..32f0fe7d2 100644 --- a/src/client/logging.cpp +++ b/src/client/logging.cpp @@ -284,7 +284,7 @@ process_log_filter(const std::string& log_filter) { #endif // GKFS_DEBUG_BUILD logger::logger(const std::string& opts, const std::string& path, - int log_per_process, bool trunc + bool log_per_process, bool trunc #ifdef GKFS_DEBUG_BUILD , const std::string& filter, int verbosity @@ -312,7 +312,7 @@ logger::logger(const std::string& opts, const std::string& path, std::string file_path = path; if(log_per_process) { fs::create_directories(path); - file_path += "/" + std::to_string(log_process_id_); + file_path = fmt::format("{}/{}", path, log_process_id_); } // we use ::open() here rather than ::syscall_no_intercept(SYS_open) diff --git a/src/client/preload_context.cpp b/src/client/preload_context.cpp index fb6e61c6d..0cc523f9e 100644 --- a/src/client/preload_context.cpp +++ b/src/client/preload_context.cpp @@ -76,8 +76,9 @@ PreloadContext::init_logging() { const std::string log_output = gkfs::env::get_var( gkfs::env::LOG_OUTPUT, gkfs::config::log::client_log_path); - const int log_per_process = std::atoi( - gkfs::env::get_var(gkfs::env::LOG_PER_PROCESS, "0").c_str()); + const bool log_per_process = + gkfs::env::get_var(gkfs::env::LOG_PER_PROCESS).empty() ? false + : true; #ifdef GKFS_DEBUG_BUILD // atoi returns 0 if no int conversion can be performed, which works @@ -95,7 +96,8 @@ PreloadContext::init_logging() { const bool log_trunc = (!trunc_val.empty() && trunc_val[0] != '0'); - gkfs::log::create_global_logger(log_opts, log_output, log_per_process, log_trunc + gkfs::log::create_global_logger(log_opts, log_output, log_per_process, + log_trunc #ifdef GKFS_DEBUG_BUILD , log_filter, log_verbosity -- GitLab From 4f16eb71a4abf71a1056ad6e2f45ea837345b77d Mon Sep 17 00:00:00 2001 From: Marc Vef Date: Wed, 14 Feb 2024 14:53:43 +0100 Subject: [PATCH 3/3] Review before merge. Minor changes. --- CHANGELOG.md | 7 ++++--- README.md | 4 ++-- docs/sphinx/users/running.md | 4 ++-- src/client/logging.cpp | 3 +++ 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad098181c..d5116a896 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### New -- Support mtime with option gkfs::config::metadata::use_mtime ([!225](https://storage.bsc.es/gitlab/hpc/gekkofs/-/issues/225)) -- Unify -d and -p flags in scripts/compile_dep.sh and dl_dep.sh ([!188](https://storage.bsc.es/gitlab/hpc/gekkofs/-/issues/188)). -- Logfile per process for the client, activated with LIBGKFS_LOG_PER_PROCESS ([!278](https://storage.bsc.es/gitlab/hpc/gekkofs/-/issues/278)). +- Support for client-side per process logging, activated + with `LIBGKFS_LOG_PER_PROCESS` ([!179](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/179)). +- Support mtime with option gkfs::config::metadata:: + use_mtime ([!178](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/178)) - Additional tests to increase code coverage ([!141](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/141)). - GKFS_ENABLE_UNUSED_FUNCTIONS added to disable code to increase code coverage. ([!141](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/141)). diff --git a/README.md b/README.md index d3175f2e8..4efc85273 100644 --- a/README.md +++ b/README.md @@ -200,8 +200,8 @@ usage: gkfs [-h/--help] [-r/--rootdir ] [-m/--mountdir ] [-a/--args The following environment variables can be used to enable logging in the client library: `LIBGKFS_LOG=` and `LIBGKFS_LOG_OUTPUT=` to configure the output module and set the path to the log file of the client library. If not path is specified in `LIBGKFS_LOG_OUTPUT`, the client library will send log messages -to `/tmp/gkfs_client.log`. With `LIBGKFS_LOG_PER_PROCESS=1` it is possible to write separate logs per client process. -If so, the path specified in `LIBGKFS_LOG_OUTPUT` is a directory but should not end with a `/`. +to `/tmp/gkfs_client.log`. Use `LIBGKFS_LOG_PER_PROCESS=ON` to write separate logs per client process. +When enabled, the path specified with `LIBGKFS_LOG_OUTPUT=` is used as a directory. The following modules are available: diff --git a/docs/sphinx/users/running.md b/docs/sphinx/users/running.md index 855f4067d..fa52e02dd 100644 --- a/docs/sphinx/users/running.md +++ b/docs/sphinx/users/running.md @@ -180,8 +180,8 @@ usage: gkfs [-h/--help] [-r/--rootdir ] [-m/--mountdir ] [-a/--args The following environment variables can be used to enable logging in the client library: `LIBGKFS_LOG=` and `LIBGKFS_LOG_OUTPUT=` to configure the output module and set the path to the log file of the client library. If not path is specified in `LIBGKFS_LOG_OUTPUT`, the client library will send log messages -to `/tmp/gkfs_client.log`. With `LIBGKFS_LOG_PER_PROCESS=1` it is possible to write separate logs per client process. -If so, the path specified in `LIBGKFS_LOG_OUTPUT` is a directory but should not end with a `/`. +to `/tmp/gkfs_client.log`. Use `LIBGKFS_LOG_PER_PROCESS=ON` to write separate logs per client process. +When enabled, the path specified with `LIBGKFS_LOG_OUTPUT=` is used as a directory. The following modules are available: diff --git a/src/client/logging.cpp b/src/client/logging.cpp index 32f0fe7d2..274844b78 100644 --- a/src/client/logging.cpp +++ b/src/client/logging.cpp @@ -311,6 +311,9 @@ logger::logger(const std::string& opts, const std::string& path, std::string file_path = path; if(log_per_process) { + if(fs::is_regular_file(file_path) && fs::exists(file_path)) { + fs::remove(file_path); + } fs::create_directories(path); file_path = fmt::format("{}/{}", path, log_process_id_); } -- GitLab