From 04e64d442adb780a126996aa910da84e6939185a Mon Sep 17 00:00:00 2001 From: James Short Date: Tue, 20 Dec 2022 11:27:44 -0800 Subject: [PATCH] Logfile location configurability. Fixes #555 --- .circleci/config.yml | 8 +++++++ etc/et.cfg | 1 + src/base/LogHandler.cpp | 33 ++++++++++++++++++----------- src/base/LogHandler.hpp | 4 ++-- src/terminal/TerminalClientMain.cpp | 16 ++++++++------ src/terminal/TerminalMain.cpp | 8 ++++--- src/terminal/TerminalServerMain.cpp | 22 ++++++++++++++----- 7 files changed, 63 insertions(+), 29 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index fad233f18..25eaea95d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -53,6 +53,10 @@ jobs: - run: name: Kill server command: sudo pkill etserver + - run: + name: Debug info/logs if failed + when: always + command: ls -la /tmp/et*; sudo awk 'FNR==1 {print "XXXXXX", FILENAME, "XXXXXX"}{print}' /tmp/et*.log connect_with_jumphost: machine: @@ -84,6 +88,10 @@ jobs: - run: name: Kill servers command: sudo pkill etserver + - run: + name: Debug info/logs if failed + when: always + command: ls -la /tmp/et*; sudo awk 'FNR==1 {print "XXXXXX", FILENAME, "XXXXXX"}{print}' /tmp/et*.log workflows: version: 2 diff --git a/etc/et.cfg b/etc/et.cfg index 4a7d3443b..b3fc41dfd 100644 --- a/etc/et.cfg +++ b/etc/et.cfg @@ -10,3 +10,4 @@ verbose = 0 silent = 0 logsize = 20971520 telemetry = true +logdirectory = /tmp diff --git a/src/base/LogHandler.cpp b/src/base/LogHandler.cpp index 3c648fe66..33d421845 100644 --- a/src/base/LogHandler.cpp +++ b/src/base/LogHandler.cpp @@ -35,9 +35,8 @@ void LogHandler::setupLogFiles(el::Configurations *defaultConf, timeinfo = localtime(&rawtime); strftime(buffer, sizeof(buffer), "%Y-%m-%d_%H-%M-%S", timeinfo); string current_time(buffer); - string logFilename = path + "/" + filenamePrefix + "-" + current_time; - string stderrFilename = - path + "/" + filenamePrefix + "-stderr-" + current_time; + string logFilename = filenamePrefix + "-" + current_time; + string stderrFilename = filenamePrefix + "-stderr-" + current_time; if (appendPid) { string pid = std::to_string(getpid()); logFilename.append("_" + pid); @@ -45,11 +44,11 @@ void LogHandler::setupLogFiles(el::Configurations *defaultConf, } logFilename.append(".log"); stderrFilename.append(".log"); - createLogFile(logFilename.c_str()); + string fullFname = createLogFile(path, logFilename); // Enable strict log file size check el::Loggers::addFlag(el::LoggingFlag::StrictLogFileSizeCheck); - defaultConf->setGlobally(el::ConfigurationType::Filename, logFilename); + defaultConf->setGlobally(el::ConfigurationType::Filename, fullFname); defaultConf->setGlobally(el::ConfigurationType::ToFile, "true"); defaultConf->setGlobally(el::ConfigurationType::MaxLogFileSize, maxlogsize); @@ -60,7 +59,7 @@ void LogHandler::setupLogFiles(el::Configurations *defaultConf, } if (redirectStderrToFile) { - stderrToFile(stderrFilename); + stderrToFile(path, stderrFilename); } } @@ -82,18 +81,28 @@ void LogHandler::setupStdoutLogger() { el::Loggers::reconfigureLogger(stdoutLogger, stdoutConf); } -void LogHandler::createLogFile(const string &filename) { +string LogHandler::createLogFile(const string &path, const string &filename) { + string fullFname = path + "/" + filename; + try { + fs::create_directories(path); + } catch (const fs::filesystem_error &fse) { + CLOG(ERROR, "stdout") << "Cannot create logfile directory: " << fse.what() + << endl; + exit(1); + } #ifdef WIN32 // O_NOFOLLOW does not exist on windows - FATAL_FAIL(::open(filename.c_str(), O_EXCL | O_CREAT, 0600)); + FATAL_FAIL(::open(fullFname.c_str(), O_EXCL | O_CREAT, 0600)); #else - FATAL_FAIL(::open(filename.c_str(), O_NOFOLLOW | O_EXCL | O_CREAT, 0600)); + FATAL_FAIL(::open(fullFname.c_str(), O_NOFOLLOW | O_EXCL | O_CREAT, 0600)); #endif + return fullFname; } -void LogHandler::stderrToFile(const string &stderrFilename) { - createLogFile(stderrFilename.c_str()); - FILE *stderr_stream = freopen(stderrFilename.c_str(), "w", stderr); +void LogHandler::stderrToFile(const string &path, + const string &stderrFilename) { + string fullFname = createLogFile(path, stderrFilename); + FILE *stderr_stream = freopen(fullFname.c_str(), "w", stderr); if (!stderr_stream) { STFATAL << "Invalid filename " << stderrFilename; } diff --git a/src/base/LogHandler.hpp b/src/base/LogHandler.hpp index de7e3553c..1fa9088cb 100644 --- a/src/base/LogHandler.hpp +++ b/src/base/LogHandler.hpp @@ -17,8 +17,8 @@ class LogHandler { static void setupStdoutLogger(); private: - static void stderrToFile(const string &stderrFilename); - static void createLogFile(const string &filename); + static void stderrToFile(const string &path, const string &stderrFilename); + static string createLogFile(const string &path, const string &filename); }; } // namespace et #endif // __ET_LOG_HANDLER__ diff --git a/src/terminal/TerminalClientMain.cpp b/src/terminal/TerminalClientMain.cpp index bbd716f57..0823a923f 100644 --- a/src/terminal/TerminalClientMain.cpp +++ b/src/terminal/TerminalClientMain.cpp @@ -88,11 +88,13 @@ int main(int argc, char** argv) { ("v,verbose", "Enable verbose logging", cxxopts::value()->default_value("0")) // ("k,keepalive", "Client keepalive duration in seconds", - cxxopts::value()) // - ("logtostdout", "Write log to stdout") // - ("silent", "Disable logging") // - ("N,no-terminal", "Do not create a terminal") // - ("f,forward-ssh-agent", "Forward ssh-agent socket") // + cxxopts::value()) // + ("l,logdir", "Base directory for log files.", + cxxopts::value()->default_value(tmpDir)) // + ("logtostdout", "Write log to stdout") // + ("silent", "Disable logging") // + ("N,no-terminal", "Do not create a terminal") // + ("f,forward-ssh-agent", "Forward ssh-agent socket") // ("ssh-socket", "The ssh-agent socket to forward", cxxopts::value()) // ("telemetry", @@ -124,8 +126,8 @@ int main(int argc, char** argv) { defaultConf.setGlobally(el::ConfigurationType::Enabled, "false"); } - LogHandler::setupLogFiles(&defaultConf, tmpDir, "etclient", - result.count("logtostdout"), + LogHandler::setupLogFiles(&defaultConf, result["logdir"].as(), + "etclient", result.count("logtostdout"), !result.count("logtostdout")); el::Loggers::reconfigureLogger("default", defaultConf); diff --git a/src/terminal/TerminalMain.cpp b/src/terminal/TerminalMain.cpp index 553fbe000..df02ea014 100644 --- a/src/terminal/TerminalMain.cpp +++ b/src/terminal/TerminalMain.cpp @@ -48,7 +48,9 @@ int main(int argc, char** argv) { // Not used by etterminal but easylogging uses this flag under the hood ("v,verbose", "Enable verbose logging", cxxopts::value()->default_value("0")) // - ("logtostdout", "Write log to stdout") // + ("l,logdir", "Base directory for log files.", + cxxopts::value()->default_value(GetTempDirectory())) // + ("logtostdout", "Write log to stdout") // ("serverfifo", "If set, connects to the etserver instance listening on the matching " "fifo name", // @@ -138,7 +140,7 @@ int main(int argc, char** argv) { string username = string(ssh_get_local_username()); if (result.count("jump")) { // etserver with --jump cannot write to the default log file(root) - LogHandler::setupLogFiles(&defaultConf, GetTempDirectory(), + LogHandler::setupLogFiles(&defaultConf, result["logdir"].as(), ("etjump-" + username + "-" + id), result.count("logtostdout"), false); // Reconfigure default logger to apply settings above @@ -167,7 +169,7 @@ int main(int argc, char** argv) { } // etserver with --idpasskey cannot write to the default log file(root) - LogHandler::setupLogFiles(&defaultConf, GetTempDirectory(), + LogHandler::setupLogFiles(&defaultConf, result["logdir"].as(), ("etterminal-" + username + "-" + id), result.count("logtostdout"), false); diff --git a/src/terminal/TerminalServerMain.cpp b/src/terminal/TerminalServerMain.cpp index 9d730a36b..6c62776ad 100644 --- a/src/terminal/TerminalServerMain.cpp +++ b/src/terminal/TerminalServerMain.cpp @@ -37,7 +37,9 @@ int main(int argc, char **argv) { ("daemon", "Daemonize the server") // ("cfgfile", "Location of the config file", cxxopts::value()->default_value("")) // - ("logtostdout", "log to stdout") // + ("l,logdir", "Base directory for log files.", + cxxopts::value()) // + ("logtostdout", "log to stdout") // ("pidfile", "Location of the pid file", cxxopts::value()->default_value( "/var/run/etserver.pid")) // @@ -78,6 +80,7 @@ int main(int argc, char **argv) { int port = 0; string bindIp = ""; bool telemetry = false; + string logDirectory = GetTempDirectory(); if (result.count("cfgfile")) { // Load the config file CSimpleIniA ini(true, false, false); @@ -120,6 +123,7 @@ int main(int argc, char **argv) { if (silent && atoi(silent) != 0) { defaultConf.setGlobally(el::ConfigurationType::Enabled, "false"); } + // read log file size limit const char *logsize = ini.GetValue("Debug", "logsize", NULL); if (logsize && atoi(logsize) != 0) { @@ -127,6 +131,11 @@ int main(int argc, char **argv) { maxlogsize = string(logsize); } + // log file directory (TODO path validation and trailing slash cleanup) + const char *logdir = ini.GetValue("Debug", "logdirectory", NULL); + if (logdir) { + logDirectory = string(logdir); + } } else { STFATAL << "Invalid config file: " << cfgfilename; } @@ -149,6 +158,10 @@ int main(int argc, char **argv) { telemetry = result["telemetry"].as(); } + if (result.count("logdir")) { + logDirectory = result["logdir"].as(); + } + GOOGLE_PROTOBUF_VERIFY_VERSION; srand(1); @@ -157,10 +170,9 @@ int main(int argc, char **argv) { } // Set log file for etserver process here. - LogHandler::setupLogFiles(&defaultConf, GetTempDirectory(), "etserver", - result.count("logtostdout"), - !result.count("logtostdout"), - true /* appendPid */, maxlogsize); + LogHandler::setupLogFiles( + &defaultConf, logDirectory, "etserver", result.count("logtostdout"), + !result.count("logtostdout"), true /* appendPid */, maxlogsize); // Reconfigure default logger to apply settings above el::Loggers::reconfigureLogger("default", defaultConf); // set thread name