diff --git a/CMakeLists.txt b/CMakeLists.txt index 59bac783e..476aee9af 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_too_many_logs_unittest + src/cleanup_too_many_logs_unittest.cc) + + target_link_libraries (cleanup_too_many_logs_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_too_many_logs COMMAND + ${CMAKE_COMMAND} + -DLOGCLEANUP=$ + -DTEST_DIR=${glog_BINARY_DIR}/ + -DTEST_SUBDIR=test_subdir/ + -P ${glog_SOURCE_DIR}/cmake/RunCleanerTestTooManyLogs.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_too_many_logs PROPERTIES FIXTURES_REQUIRED logcleanuptest) add_executable (striplog0_unittest src/striplog_unittest.cc diff --git a/bazel/glog.bzl b/bazel/glog.bzl index 286346b3a..85b4fef1b 100644 --- a/bazel/glog.bzl +++ b/bazel/glog.bzl @@ -231,6 +231,7 @@ def glog_library(with_gflags = 1, **kwargs): "cleanup_immediately", "cleanup_with_absolute_prefix", "cleanup_with_relative_prefix", + "cleanup_too_many_logs", # "demangle", # Broken # "logging", # Broken # "mock-log", # Broken diff --git a/cmake/RunCleanerTestTooManyLogs.cmake b/cmake/RunCleanerTestTooManyLogs.cmake new file mode 100644 index 000000000..79f06ee68 --- /dev/null +++ b/cmake/RunCleanerTestTooManyLogs.cmake @@ -0,0 +1,27 @@ +file (TOUCH test_cleanup_info_20240730-111111.2222.toomanylogs) +file (TOUCH test_cleanup_info_20240730-111112.2222.toomanylogs) +file (TOUCH test_cleanup_info_20240730-111113.2222.toomanylogs) +file (TOUCH test_cleanup_info_20240730-111114.2222.toomanylogs) +execute_process (COMMAND ${LOGCLEANUP} RESULT_VARIABLE _RESULT) + +if (NOT _RESULT EQUAL 0) + message (FATAL_ERROR "Failed to run logcleanup_unittest (error: ${_RESULT})") +endif () + +file (GLOB LOG_FILES ${TEST_DIR}/test_cleanup_*.toomanylogs) + +if ("test_cleanup_info_20240731-111111.2222.toomanylogs" IN_LIST LOG_FILES) + message (SEND_ERROR "Expected old log file to be deleted") +endif () + +if ("test_cleanup_info_20240731-111112.2222.toomanylogs" IN_LIST LOG_FILES) + message (SEND_ERROR "Expected old log file to be deleted") +endif () + +if ("test_cleanup_info_20240731-111113.2222.toomanylogs" IN_LIST LOG_FILES) + message (SEND_ERROR "Expected old log file to be deleted") +endif () + +if (NOT "test_cleanup_info_20240731-111111.2222.toomanylogs" IN_LIST LOG_FILES) + message (SEND_ERROR "Expected newest log file to be retained") +endif () diff --git a/src/cleanup_too_many_logs_unittest.cc b/src/cleanup_too_many_logs_unittest.cc new file mode 100644 index 000000000..b92d797b4 --- /dev/null +++ b/src/cleanup_too_many_logs_unittest.cc @@ -0,0 +1,96 @@ +// 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(CleanTooManyLogs, logging) { + using namespace std::chrono_literals; + google::EnableLogCleaner(0h); + google::SetLogDestination(GLOG_INFO, "test_cleanup_info_"); + google::SetLogFilenameExtension(".toomanylogs"); + FLAGS_max_num_log_files = 2; + + 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/flags.cc b/src/flags.cc index c4c2aa4b2..9b712bb68 100644 --- a/src/flags.cc +++ b/src/flags.cc @@ -114,6 +114,11 @@ GLOG_DEFINE_int32(logbufsecs, 30, GLOG_DEFINE_int32(logcleansecs, 60 * 5, // every 5 minutes "Clean overdue logs every this many seconds"); +GLOG_DEFINE_uint32(max_num_log_files, 0, + "Maximum number of log files retained per log level when " + "using the log cleaner. Oldest logs are deleted first. " + "If 0, all log files are kept."); + GLOG_DEFINE_int32(logemaillevel, 999, "Email log messages logged at this level or higher" " (0 means email all; 3 means email FATAL only;" diff --git a/src/glog/flags.h b/src/glog/flags.h index cb542b930..cd5fbccba 100644 --- a/src/glog/flags.h +++ b/src/glog/flags.h @@ -104,6 +104,7 @@ DECLARE_int32(logemaillevel); DECLARE_int32(logcleansecs); +DECLARE_uint32(max_num_log_files); #ifdef GLOG_OS_LINUX DECLARE_bool(drop_log_memory); diff --git a/src/logging.cc b/src/logging.cc index 08b2ab387..a3e7552e0 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -446,7 +446,7 @@ class LogCleaner { bool enabled() const { return enabled_; } private: - vector GetOverdueLogNames( + vector GetLogNamesToDelete( string log_directory, const std::chrono::system_clock::time_point& current_time, const string& base_filename, const string& filename_extension) const; @@ -455,9 +455,9 @@ class LogCleaner { const string& base_filename, const string& filename_extension) const; - bool IsLogLastModifiedOver( + bool GetLastModifiedTime( const string& filepath, - const std::chrono::system_clock::time_point& current_time) const; + std::chrono::system_clock::time_point& last_modified_time) const; bool enabled_{false}; std::chrono::minutes overdue_{ @@ -1323,8 +1323,8 @@ void LogCleaner::Run(const std::chrono::system_clock::time_point& current_time, } for (const std::string& dir : dirs) { - vector logs = GetOverdueLogNames(dir, current_time, base_filename, - filename_extension); + vector logs = GetLogNamesToDelete(dir, current_time, base_filename, + 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()); @@ -1335,14 +1335,13 @@ void LogCleaner::Run(const std::chrono::system_clock::time_point& current_time, } } -vector LogCleaner::GetOverdueLogNames( +vector LogCleaner::GetLogNamesToDelete( string log_directory, const std::chrono::system_clock::time_point& current_time, const string& base_filename, const string& filename_extension) const { - // The names of overdue logs. - vector overdue_log_names; - // Try to get all files within log_directory. + using LogFileInfo = std::pair; + vector log_file_info; DIR* dir; struct dirent* ent; @@ -1362,16 +1361,39 @@ vector LogCleaner::GetOverdueLogNames( filepath = log_directory + filepath; } + std::chrono::system_clock::time_point last_modified_time; if (IsLogFromCurrentProject(filepath, base_filename, filename_extension) && - IsLogLastModifiedOver(filepath, current_time)) { - overdue_log_names.push_back(filepath); + GetLastModifiedTime(filepath, last_modified_time)) { + log_file_info.push_back(std::make_pair(last_modified_time, filepath)); } } closedir(dir); } - return overdue_log_names; + // Sort the log files by last modified time, oldest first. + std::sort(log_file_info.begin(), log_file_info.end()); + + const auto max_num_log_files_set = FLAGS_max_num_log_files != 0; + const auto too_many_log_files = + max_num_log_files_set && log_file_info.size() > FLAGS_max_num_log_files; + const auto min_num_logs_to_del = + too_many_log_files ? log_file_info.size() - FLAGS_max_num_log_files : 0; + + // The names of logs to delete. + vector logs_to_delete; + for (const auto& info : log_file_info) { + const auto& last_modified_time = info.first; + const auto& filepath = info.second; + + const auto is_overdue = current_time - last_modified_time >= overdue_; + const auto need_to_delete = logs_to_delete.size() < min_num_logs_to_del; + + if (is_overdue || need_to_delete) { + logs_to_delete.push_back(filepath); + } + } + return logs_to_delete; } bool LogCleaner::IsLogFromCurrentProject( @@ -1461,17 +1483,16 @@ bool LogCleaner::IsLogFromCurrentProject( return true; } -bool LogCleaner::IsLogLastModifiedOver( +bool LogCleaner::GetLastModifiedTime( const string& filepath, - const std::chrono::system_clock::time_point& current_time) const { + std::chrono::system_clock::time_point& last_modified_time) const { // Try to get the last modified time of this file. struct stat file_stat; if (stat(filepath.c_str(), &file_stat) == 0) { - const auto last_modified_time = + last_modified_time = std::chrono::system_clock::from_time_t(file_stat.st_mtime); - const auto diff = current_time - last_modified_time; - return diff >= overdue_; + return true; } // If failed to get file stat, don't return true!