diff --git a/CMakeLists.txt b/CMakeLists.txt index f3dba78..9e1d25e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -729,6 +729,11 @@ if (BUILD_TESTING) target_link_libraries (cleanup_with_relative_prefix_unittest PRIVATE glog_test) + add_executable (cleanup_all_log_levels_unittest + src/cleanup_all_log_levels_unittest.cc) + + target_link_libraries (cleanup_all_log_levels_unittest PRIVATE glog_test) + set (CLEANUP_LOG_DIR ${glog_BINARY_DIR}/cleanup_tests) add_test (NAME cleanup_init COMMAND @@ -755,6 +760,13 @@ if (BUILD_TESTING) -DTEST_SUBDIR=test_subdir/ -P ${glog_SOURCE_DIR}/cmake/RunCleanerTest3.cmake WORKING_DIRECTORY ${glog_BINARY_DIR}) + add_test (NAME cleanup_all_log_levels COMMAND + ${CMAKE_COMMAND} + -DLOGCLEANUP=$ + -DTEST_DIR=${glog_BINARY_DIR}/ + -DTEST_SUBDIR=test_subdir/ + -P ${glog_SOURCE_DIR}/cmake/RunCleanerTestAllLogLevels.cmake + WORKING_DIRECTORY ${glog_BINARY_DIR}) # Fixtures setup set_tests_properties (cleanup_init PROPERTIES FIXTURES_SETUP logcleanuptest) @@ -764,6 +776,7 @@ if (BUILD_TESTING) set_tests_properties (cleanup_immediately PROPERTIES FIXTURES_REQUIRED logcleanuptest) set_tests_properties (cleanup_with_absolute_prefix PROPERTIES FIXTURES_REQUIRED logcleanuptest) set_tests_properties (cleanup_with_relative_prefix PROPERTIES FIXTURES_REQUIRED logcleanuptest) + set_tests_properties (cleanup_all_log_levels PROPERTIES FIXTURES_REQUIRED logcleanuptest) add_executable (striplog0_unittest src/striplog_unittest.cc diff --git a/bazel/glog.bzl b/bazel/glog.bzl index ff54e24..3ef12df 100644 --- a/bazel/glog.bzl +++ b/bazel/glog.bzl @@ -233,6 +233,7 @@ def glog_library(with_gflags = 1, **kwargs): "cleanup_immediately", "cleanup_with_absolute_prefix", "cleanup_with_relative_prefix", + "cleanup_all_log_levels", # "demangle", # Broken # "logging", # Broken # "mock-log", # Broken diff --git a/cmake/RunCleanerTestAllLogLevels.cmake b/cmake/RunCleanerTestAllLogLevels.cmake new file mode 100644 index 0000000..9c083bd --- /dev/null +++ b/cmake/RunCleanerTestAllLogLevels.cmake @@ -0,0 +1,34 @@ +file (TOUCH test_cleanup_info_20240312-111111.2222.allloglevels) +file (TOUCH test_cleanup_warning_20240312-111111.2222.allloglevels) +file (TOUCH test_cleanup_error_20240312-111111.2222.allloglevels) +file (TOUCH test_cleanup_info_20240312-111111.2222.allloglevels.notalog) +execute_process (COMMAND ${LOGCLEANUP} RESULT_VARIABLE _RESULT) + +if (NOT _RESULT EQUAL 0) + message (FATAL_ERROR "Failed to run logcleanup_unittest (error: ${_RESULT})") +endif (NOT _RESULT EQUAL 0) + +file (GLOB LOG_FILES ${TEST_DIR}/test_cleanup_*.allloglevels) +list (LENGTH LOG_FILES NUM_LOG_FILES) + +if (WIN32) + # On Windows open files cannot be removed and will result in a permission + # denied error while unlinking such file. Therefore, the last file will be + # retained. + set (_expected 1) + else (WIN32) + set (_expected 0) +endif (WIN32) + +if (NOT NUM_LOG_FILES EQUAL _expected) + message (SEND_ERROR "Expected ${_expected} log file in log directory but found ${NUM_LOG_FILES}") +endif (NOT NUM_LOG_FILES EQUAL _expected) + +file (GLOB NON_LOG_FILES ${TEST_DIR}/test_cleanup_*.notalog) +list (LENGTH NON_LOG_FILES NUM_NON_LOG_FILES) + +if (NOT NUM_NON_LOG_FILES EQUAL 1) + message (SEND_ERROR "Expected 1 non-log file in log directory but found ${NUM_NON_LOG_FILES}") +endif (NOT NUM_NON_LOG_FILES EQUAL 1) + +file (REMOVE test_cleanup_info_20240312-111111.2222.allloglevels.notalog) diff --git a/src/cleanup_all_log_levels_unittest.cc b/src/cleanup_all_log_levels_unittest.cc new file mode 100644 index 0000000..ae05eaf --- /dev/null +++ b/src/cleanup_all_log_levels_unittest.cc @@ -0,0 +1,97 @@ +// Copyright (c) 2024, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include "base/commandlineflags.h" +#include "glog/logging.h" +#include "glog/raw_logging.h" +#include "googletest.h" + +#ifdef GLOG_USE_GFLAGS +# include +using namespace GFLAGS_NAMESPACE; +#endif + +#ifdef HAVE_LIB_GMOCK +# include + +# include "mock-log.h" +// Introduce several symbols from gmock. +using google::glog_testing::ScopedMockLog; +using testing::_; +using testing::AllOf; +using testing::AnyNumber; +using testing::HasSubstr; +using testing::InitGoogleMock; +using testing::StrictMock; +using testing::StrNe; +#endif + +using namespace google; + +TEST(CleanImmediatelyAllLogLevels, logging) { + using namespace std::chrono_literals; + google::EnableLogCleaner(0h); + google::SetLogFilenameExtension(".allloglevels"); + google::SetLogDestination(GLOG_INFO, "test_cleanup_info_"); + google::SetLogDestination(GLOG_WARNING, "test_cleanup_warning_"); + google::SetLogDestination(GLOG_ERROR, "test_cleanup_error_"); + + LOG(INFO) << "cleanup test"; + + google::DisableLogCleaner(); +} + +int main(int argc, char** argv) { + FLAGS_colorlogtostderr = false; + FLAGS_timestamp_in_logfile_name = true; +#ifdef GLOG_USE_GFLAGS + ParseCommandLineFlags(&argc, &argv, true); +#endif + // Make sure stderr is not buffered as stderr seems to be buffered + // on recent windows. + setbuf(stderr, nullptr); + + // Test some basics before InitGoogleLogging: + CaptureTestStderr(); + const string early_stderr = GetCapturedTestStderr(); + + EXPECT_FALSE(IsGoogleLoggingInitialized()); + + InitGoogleLogging(argv[0]); + + EXPECT_TRUE(IsGoogleLoggingInitialized()); + + InitGoogleTest(&argc, argv); +#ifdef HAVE_LIB_GMOCK + InitGoogleMock(&argc, argv); +#endif + + // so that death tests run before we use threads + CHECK_EQ(RUN_ALL_TESTS(), 0); +} diff --git a/src/logging.cc b/src/logging.cc index a452cff..64e26c5 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -430,6 +431,17 @@ class LogFileObject : public base::Logger { bool CreateLogfile(const string& time_pid_string); }; +struct LogCleanerFileInfo { + std::string dir; + std::string base_filename; + std::string filename_extension; +}; + +bool operator<(const LogCleanerFileInfo& l, const LogCleanerFileInfo& r) { + return std::tie(l.dir, l.base_filename, l.filename_extension) < + std::tie(r.dir, r.base_filename, r.filename_extension); +} + // Encapsulate all log cleaner related states class LogCleaner { public: @@ -439,11 +451,11 @@ class LogCleaner { void Enable(const std::chrono::minutes& overdue); void Disable(); - void Run(const std::chrono::system_clock::time_point& current_time, - bool base_filename_selected, const string& base_filename, - const string& filename_extension); + void RegisterLogFile(const std::string& path, const std::string& ext); + void UnregisterLogFile(const std::string& path, const std::string& ext); + void UnregisterAllLogFiles(); - bool enabled() const { return enabled_; } + void Run(const std::chrono::system_clock::time_point& current_time); private: vector GetOverdueLogNames( @@ -459,11 +471,15 @@ class LogCleaner { const string& filepath, const std::chrono::system_clock::time_point& current_time) const; + std::string RemoveDuplicatePathDelimiters(const std::string& path) const; + std::string GetLogDir(const std::string& filepath) const; + bool enabled_{false}; std::chrono::minutes overdue_{ std::chrono::duration>{1}}; - std::chrono::system_clock::time_point - next_cleanup_time_; // cycle count at which to clean overdue log + std::chrono::system_clock::time_point next_cleanup_time_; + std::set log_files_; + std::mutex mutex_; }; LogCleaner log_cleaner; @@ -929,7 +945,11 @@ LogFileObject::LogFileObject(LogSeverity severity, const char* base_filename) filename_extension_(), severity_(severity), rollover_attempt_(kRolloverAttemptFrequency - 1), - start_time_(std::chrono::system_clock::now()) {} + start_time_(std::chrono::system_clock::now()) { + if (!base_filename_.empty()) { + log_cleaner.RegisterLogFile(base_filename_, filename_extension_); + } +} LogFileObject::~LogFileObject() { std::lock_guard l{mutex_}; @@ -940,24 +960,36 @@ void LogFileObject::SetBasename(const char* basename) { std::lock_guard l{mutex_}; base_filename_selected_ = true; if (base_filename_ != basename) { + if (!base_filename_.empty()) { + log_cleaner.UnregisterLogFile(base_filename_, filename_extension_); + } // Get rid of old log file since we are changing names if (file_ != nullptr) { file_ = nullptr; rollover_attempt_ = kRolloverAttemptFrequency - 1; } base_filename_ = basename; + if (!base_filename_.empty()) { + log_cleaner.RegisterLogFile(base_filename_, filename_extension_); + } } } void LogFileObject::SetExtension(const char* ext) { std::lock_guard l{mutex_}; if (filename_extension_ != ext) { + if (!base_filename_.empty()) { + log_cleaner.UnregisterLogFile(base_filename_, filename_extension_); + } // Get rid of old log file since we are changing names if (file_ != nullptr) { file_ = nullptr; rollover_attempt_ = kRolloverAttemptFrequency - 1; } filename_extension_ = ext; + if (!base_filename_.empty()) { + log_cleaner.RegisterLogFile(base_filename_, filename_extension_); + } } } @@ -1112,11 +1144,8 @@ void LogFileObject::Write( return; } - auto cleanupLogs = [this, current_time = timestamp] { - if (log_cleaner.enabled()) { - log_cleaner.Run(current_time, base_filename_selected_, base_filename_, - filename_extension_); - } + auto cleanupLogs = [current_time = timestamp] { + log_cleaner.Run(current_time); }; // Remove old logs @@ -1209,6 +1238,8 @@ void LogFileObject::Write( } } + log_cleaner.RegisterLogFile(base_filename_, filename_extension_); + // Write a header message into the log file if (FLAGS_log_file_header) { ostringstream file_header_stream; @@ -1306,11 +1337,33 @@ void LogCleaner::Enable(const std::chrono::minutes& overdue) { void LogCleaner::Disable() { enabled_ = false; } -void LogCleaner::Run(const std::chrono::system_clock::time_point& current_time, - bool base_filename_selected, const string& base_filename, - const string& filename_extension) { - assert(enabled_); - assert(!base_filename_selected || !base_filename.empty()); +void LogCleaner::RegisterLogFile(const std::string& base_filepath, + const std::string& filename_extension) { + std::lock_guard l{mutex_}; + log_files_.insert({GetLogDir(base_filepath), base_filepath, filename_extension}); + + // Reset the next cleanup time so that the next Run() will clean up the new logs + next_cleanup_time_ = {}; +} + +void LogCleaner::UnregisterLogFile(const std::string& base_filepath, + const std::string& filename_extension) { + std::lock_guard l{mutex_}; + log_files_.erase({GetLogDir(base_filepath), base_filepath, filename_extension}); +} + +void LogCleaner::UnregisterAllLogFiles() +{ + std::lock_guard l{mutex_}; + log_files_.clear(); +} + +void LogCleaner::Run(const std::chrono::system_clock::time_point& current_time) { + if (!enabled_) { + return; + } + + std::lock_guard l{mutex_}; // avoid scanning logs too frequently if (current_time < next_cleanup_time_) { @@ -1322,24 +1375,10 @@ void LogCleaner::Run(const std::chrono::system_clock::time_point& current_time, std::chrono::duration_cast( std::chrono::duration{FLAGS_logcleansecs}); - vector dirs; - - if (!base_filename_selected) { - dirs = GetLoggingDirectories(); - } else { - size_t pos = base_filename.find_last_of(possible_dir_delim, string::npos, - sizeof(possible_dir_delim)); - if (pos != string::npos) { - string dir = base_filename.substr(0, pos + 1); - dirs.push_back(dir); - } else { - dirs.emplace_back("."); - } - } - - for (const std::string& dir : dirs) { - vector logs = GetOverdueLogNames(dir, current_time, base_filename, - filename_extension); + for (const auto& log_file_info : log_files_) { + const auto logs = GetOverdueLogNames(log_file_info.dir, current_time, + log_file_info.base_filename, + log_file_info.filename_extension); for (const std::string& log : logs) { // NOTE May fail on Windows if the file is still open int result = unlink(log.c_str()); @@ -1376,6 +1415,7 @@ vector LogCleaner::GetOverdueLogNames( log_directory[log_directory.size() - 1]) != dir_delim_end) { filepath = log_directory + filepath; } + filepath = RemoveDuplicatePathDelimiters(filepath); if (IsLogFromCurrentProject(filepath, base_filename, filename_extension) && @@ -1395,22 +1435,7 @@ bool LogCleaner::IsLogFromCurrentProject( // We should remove duplicated delimiters from `base_filename`, e.g., // before: "/tmp//.." // after: "/tmp/.." - string cleaned_base_filename; - - const char* const dir_delim_end = - possible_dir_delim + sizeof(possible_dir_delim); - - size_t real_filepath_size = filepath.size(); - for (char c : base_filename) { - if (cleaned_base_filename.empty()) { - cleaned_base_filename += c; - } else if (std::find(possible_dir_delim, dir_delim_end, c) == - dir_delim_end || - (!cleaned_base_filename.empty() && - c != cleaned_base_filename[cleaned_base_filename.size() - 1])) { - cleaned_base_filename += c; - } - } + string cleaned_base_filename = RemoveDuplicatePathDelimiters(base_filename); // Return early if the filename doesn't start with `cleaned_base_filename`. if (filepath.find(cleaned_base_filename) != 0) { @@ -1420,6 +1445,7 @@ bool LogCleaner::IsLogFromCurrentProject( // Check if in the string `filename_extension` is right next to // `cleaned_base_filename` in `filepath` if the user // has set a custom filename extension. + size_t real_filepath_size = filepath.size(); if (!filename_extension.empty()) { if (cleaned_base_filename.size() >= real_filepath_size) { return false; @@ -1493,6 +1519,36 @@ bool LogCleaner::IsLogLastModifiedOver( return false; } +std::string LogCleaner::RemoveDuplicatePathDelimiters(const std::string& path) const { + string cleaned_path; + + const char* const dir_delim_end = + possible_dir_delim + sizeof(possible_dir_delim); + + for (char c : path) { + if (cleaned_path.empty()) { + cleaned_path += c; + } else if (std::find(possible_dir_delim, dir_delim_end, c) == + dir_delim_end || + (!cleaned_path.empty() && + c != cleaned_path[cleaned_path.size() - 1])) { + cleaned_path += c; + } + } + + return cleaned_path; +} + +std::string LogCleaner::GetLogDir(const std::string& filepath) const { + const size_t pos = filepath.find_last_of( + possible_dir_delim, string::npos, sizeof(possible_dir_delim)); + if (pos != std::string::npos) { + return filepath.substr(0, pos + 1); + } else { + return "."; + } +} + } // namespace // Static log data space to avoid alloc failures in a LOG(FATAL) @@ -2628,6 +2684,7 @@ void InstallPrefixFormatter(PrefixFormatterCallback callback, void* data) { void ShutdownGoogleLogging() { ShutdownGoogleLoggingUtilities(); LogDestination::DeleteLogDestinations(); + log_cleaner.UnregisterAllLogFiles(); logging_directories_list = nullptr; g_prefix_formatter = nullptr; }