refactor(liblogging): use path type instead of string

dynatrace-oss · Oct 5, 2023 · 3665726 · 3665726
1 parent 9bc36f1
commit 3665726
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 12 deletions.
diff --git a/ebpfdiscoverysrv/src/main.cpp b/ebpfdiscoverysrv/src/main.cpp
@@ -5,9 +5,9 @@
 #include "logging/Global.h"
 
 #include <boost/program_options.hpp>
-#include <fmt/format.h>
 
 #include <condition_variable>
+#include <filesystem>
 #include <iostream>
 #include <memory>
 #include <signal.h>
@@ -36,7 +36,7 @@ static boost::program_options::options_description getProgramOptions() {
 	desc.add_options()
       ("log-level", po::value<logging::LogLevel>()->default_value(logging::LogLevel::Err, "error"), "Set log level {trace,debug,info,warning,error,critical,off}")
       ("help,h", "Display available options")
-      ("log-dir", po::value<std::string>()->default_value(""), "Log files directory")
+      ("log-dir", po::value<std::filesystem::path>()->default_value(""), "Log files directory")
       ("log-no-stdout", po::value<bool>()->default_value(false), "Disable logging to stdout")
       ("version", "Display program version")
   ;
@@ -55,10 +55,10 @@ static std::string getProgramVersion() {
  * Logging setup
  */
 
-static void setupLogging(logging::LogLevel logLevel, bool enableStdout, std::string_view logDir) {
+static void setupLogging(logging::LogLevel logLevel, bool enableStdout, const std::filesystem::path& logDir) {
 	logging::Global::getInstance().setup("eBPF-Discovery", enableStdout, logDir);
 	logging::Global::getInstance().setLevel(logLevel);
-	LOG_TRACE("Logging has been set up. (logDir: {})", logDir);
+	LOG_TRACE("Logging has been set up. (logDir: {})", logDir.string());
 }
 
 /*
@@ -139,7 +139,7 @@ int main(int argc, char** argv) {
 
 	logging::LogLevel logLevel{vm["log-level"].as<logging::LogLevel>()};
 	bool isStdoutLogDisabled{vm["log-no-stdout"].as<bool>()};
-	std::string logDir{vm["log-dir"].as<std::string>()};
+	std::filesystem::path logDir{vm["log-dir"].as<std::filesystem::path>()};
 
 	try {
 		setupLogging(logLevel, !isStdoutLogDisabled, logDir);

diff --git a/liblogging/headers/logging/Logger.h b/liblogging/headers/logging/Logger.h
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <cctype>
+#include <filesystem>
 #include <istream>
 #include <optional>
 #include <string>
@@ -37,7 +38,7 @@ class Logger {
 	Logger& operator=(const Logger&) = delete;
 
 	void setLevel(enum LogLevel level);
-	void setup(std::string name = {}, bool logToStdout = true, const std::string_view& logDir = {});
+	void setup(std::string name, bool logToStdout = true, std::filesystem::path logDir = {});
 
 	template <typename... Args>
 	void log(enum LogLevel level, const char* fmt, const Args&... args) {

diff --git a/liblogging/src/Logger.cpp b/liblogging/src/Logger.cpp
@@ -47,13 +47,9 @@ void Logger::setLevel(enum LogLevel level) {
 	spdLogger.set_level(static_cast<spdlog::level::level_enum>(level));
 }
 
-void Logger::setup(std::string name, bool logToStdout, const std::string_view& logDir) {
+void Logger::setup(std::string name, bool logToStdout, std::filesystem::path logDir) {
 	namespace fs = std::filesystem;
 
-	if (name.empty()) {
-		name = "default";
-	}
-
 	std::vector<spdlog::sink_ptr> sinks;
 	if (logToStdout) {
 		sinks.push_back(std::make_shared<spdlog::sinks::stdout_color_sink_mt>());
@@ -64,7 +60,8 @@ void Logger::setup(std::string name, bool logToStdout, const std::string_view& l
 			throw std::runtime_error("Log directory doesn't exist or is not a directory");
 		}
 		// TODO: Check for permissions
-		std::string logFile = fmt::format("{}/{}.log", logDir, name);
+
+		fs::path logFile = logDir / (name + ".log");
 		sinks.push_back(std::make_shared<spdlog::sinks::rotating_file_sink_mt>(logFile, max_size, max_files));
 	}
 
@@ -75,6 +72,7 @@ void Logger::vlogf(enum LogLevel level, const char* format, va_list args) {
 	if (!spdLogger.should_log(static_cast<spdlog::level::level_enum>(level))) {
 		return;
 	}
+
 	const int staticBufSize = 512;
 	static char staticBuf[staticBufSize]{};