From 55e6f473d9eeb9ecf6a8539d75dbf86fbc2063ef Mon Sep 17 00:00:00 2001 From: HNOONa-0 Date: Fri, 21 Feb 2025 13:30:53 +0200 Subject: [PATCH 1/7] support per-severity overdue settings --- src/logging.cc | 67 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 10 deletions(-) diff --git a/src/logging.cc b/src/logging.cc index 08b2ab387..267dec465 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -337,6 +337,16 @@ const char* GetLogSeverityName(LogSeverity severity) { return LogSeverityNames[severity]; } +int FindFilepathLogSeverity(const std::string& filepath) { + for (int i = GLOG_INFO; i < NUM_SEVERITIES; i++) { + if (filepath.rfind(GetLogSeverityName(static_cast(i))) != std::string::npos) { + return i; + } + } + + return -1; +} + static bool SendEmailInternal(const char* dest, const char* subject, const char* body, bool use_logging); @@ -437,13 +447,15 @@ class LogCleaner { // Setting overdue to 0 days will delete all logs. void Enable(const std::chrono::minutes& overdue); + void Enable(const LogSeverity severity, const std::chrono::minutes& overdue); void Disable(); + void Disable(const LogSeverity severity); void Run(const std::chrono::system_clock::time_point& current_time, bool base_filename_selected, const string& base_filename, const string& filename_extension); - bool enabled() const { return enabled_; } + bool enabled() const; private: vector GetOverdueLogNames( @@ -459,9 +471,7 @@ class LogCleaner { const string& filepath, const std::chrono::system_clock::time_point& current_time) const; - bool enabled_{false}; - std::chrono::minutes overdue_{ - std::chrono::duration>{1}}; + vector overdue_per_severity_; std::chrono::system_clock::time_point next_cleanup_time_; // cycle count at which to clean overdue log }; @@ -1282,19 +1292,38 @@ void LogFileObject::Write( } } -LogCleaner::LogCleaner() = default; +LogCleaner::LogCleaner() { + overdue_per_severity_.resize(NUM_SEVERITIES, std::chrono::minutes::max()); +} + +bool LogCleaner::enabled() const { + // return true if any severity is enabled by iterating over the array + for (int i = 0; i < NUM_SEVERITIES; i++) { + if (overdue_per_severity_[i] != std::chrono::minutes::max()) return true; + } + + return false; +} void LogCleaner::Enable(const std::chrono::minutes& overdue) { - enabled_ = true; - overdue_ = overdue; + // for backward compatability, set all severities to the same value + fill(overdue_per_severity_.begin(), overdue_per_severity_.end(), overdue); +} + +void LogCleaner::Enable(const LogSeverity severity, const std::chrono::minutes& overdue) { + overdue_per_severity_[severity] = overdue; } -void LogCleaner::Disable() { enabled_ = false; } +void LogCleaner::Disable() { fill(overdue_per_severity_.begin(), overdue_per_severity_.end(), std::chrono::minutes::max()); } + +void LogCleaner::Disable(const LogSeverity severity) { + overdue_per_severity_[severity] = std::chrono::minutes::max(); +} 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(enabled()); assert(!base_filename_selected || !base_filename.empty()); // avoid scanning logs too frequently @@ -1471,7 +1500,15 @@ bool LogCleaner::IsLogLastModifiedOver( const auto 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_; + + int severity = FindFilepathLogSeverity(filepath); + // if the filepath does not have a severity, cant clean it + if (severity < 0) { + perror(("Cannot clean the file. No severity found for: " + filepath).c_str()); + return false; + } + + return diff >= overdue_per_severity_[severity]; } // If failed to get file stat, don't return true! @@ -2627,6 +2664,16 @@ void EnableLogCleaner(const std::chrono::minutes& overdue) { log_cleaner.Enable(overdue); } +void EnableLogCleaner(LogSeverity severity, unsigned int overdue_days) { + log_cleaner.Enable(severity, std::chrono::duration_cast( + std::chrono::duration>{ + overdue_days})); +} + +void EnableLogCleaner(LogSeverity severity, const std::chrono::minutes& overdue) { + log_cleaner.Enable(severity, overdue); +} + void DisableLogCleaner() { log_cleaner.Disable(); } LogMessageTime::LogMessageTime() = default; From 73a803b3ac5cf107f91e63e831ed563a83d424b8 Mon Sep 17 00:00:00 2001 From: HNOONa-0 Date: Fri, 21 Feb 2025 14:59:49 +0200 Subject: [PATCH 2/7] use unordered_map instead of vector and rename member --- src/logging.cc | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/src/logging.cc b/src/logging.cc index 267dec465..28f845807 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -72,6 +72,7 @@ #include #include #include +#include #ifdef HAVE__CHSIZE_S # include // for truncate log file @@ -455,7 +456,7 @@ class LogCleaner { bool base_filename_selected, const string& base_filename, const string& filename_extension); - bool enabled() const; + bool enabled() const { return !overdue_.empty(); } private: vector GetOverdueLogNames( @@ -471,7 +472,7 @@ class LogCleaner { const string& filepath, const std::chrono::system_clock::time_point& current_time) const; - vector overdue_per_severity_; + std::unordered_map overdue_; std::chrono::system_clock::time_point next_cleanup_time_; // cycle count at which to clean overdue log }; @@ -1292,32 +1293,23 @@ void LogFileObject::Write( } } -LogCleaner::LogCleaner() { - overdue_per_severity_.resize(NUM_SEVERITIES, std::chrono::minutes::max()); -} - -bool LogCleaner::enabled() const { - // return true if any severity is enabled by iterating over the array - for (int i = 0; i < NUM_SEVERITIES; i++) { - if (overdue_per_severity_[i] != std::chrono::minutes::max()) return true; - } - - return false; -} +LogCleaner::LogCleaner() = default; void LogCleaner::Enable(const std::chrono::minutes& overdue) { // for backward compatability, set all severities to the same value - fill(overdue_per_severity_.begin(), overdue_per_severity_.end(), overdue); + for (int i = GLOG_INFO; i < NUM_SEVERITIES; i++) { + overdue_[static_cast(i)] = overdue; + } } void LogCleaner::Enable(const LogSeverity severity, const std::chrono::minutes& overdue) { - overdue_per_severity_[severity] = overdue; + overdue_[severity] = overdue; } -void LogCleaner::Disable() { fill(overdue_per_severity_.begin(), overdue_per_severity_.end(), std::chrono::minutes::max()); } +void LogCleaner::Disable() { overdue_.clear(); } -void LogCleaner::Disable(const LogSeverity severity) { - overdue_per_severity_[severity] = std::chrono::minutes::max(); +void LogCleaner::Disable(const LogSeverity severity) { + overdue_.erase(severity); } void LogCleaner::Run(const std::chrono::system_clock::time_point& current_time, @@ -1508,7 +1500,12 @@ bool LogCleaner::IsLogLastModifiedOver( return false; } - return diff >= overdue_per_severity_[severity]; + auto severity_it = overdue_.find(static_cast(severity)); + if (severity_it == overdue_.end()) { + return false; + } + + return diff >= severity_it->second; } // If failed to get file stat, don't return true! From 12b2c574450dc80ad585260a686991f9624f2fea Mon Sep 17 00:00:00 2001 From: HNOONa-0 Date: Fri, 21 Feb 2025 15:01:19 +0200 Subject: [PATCH 3/7] remove overloaded cleaner method --- src/logging.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/logging.cc b/src/logging.cc index 28f845807..3c142461a 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -2661,12 +2661,6 @@ void EnableLogCleaner(const std::chrono::minutes& overdue) { log_cleaner.Enable(overdue); } -void EnableLogCleaner(LogSeverity severity, unsigned int overdue_days) { - log_cleaner.Enable(severity, std::chrono::duration_cast( - std::chrono::duration>{ - overdue_days})); -} - void EnableLogCleaner(LogSeverity severity, const std::chrono::minutes& overdue) { log_cleaner.Enable(severity, overdue); } From 8d2979c9a073beb82ac942a66d587792fa1d0694 Mon Sep 17 00:00:00 2001 From: HNOONa-0 Date: Fri, 21 Feb 2025 15:13:09 +0200 Subject: [PATCH 4/7] remove unused variable --- src/logging.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/logging.cc b/src/logging.cc index 3c142461a..68b4506a1 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -356,7 +356,6 @@ base::Logger::~Logger() = default; namespace { constexpr std::intmax_t kSecondsInDay = 60 * 60 * 24; -constexpr std::intmax_t kSecondsInWeek = kSecondsInDay * 7; // Optional user-configured callback to print custom prefixes. class PrefixFormatter { From 6927d55138f100ba09881423348f76715a5eca97 Mon Sep 17 00:00:00 2001 From: HNOONa-0 Date: Fri, 21 Feb 2025 17:42:15 +0200 Subject: [PATCH 5/7] allow cleaning files that have no severity --- src/logging.cc | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/logging.cc b/src/logging.cc index 68b4506a1..a2676eb4f 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -471,7 +471,7 @@ class LogCleaner { const string& filepath, const std::chrono::system_clock::time_point& current_time) const; - std::unordered_map overdue_; + std::unordered_map overdue_; std::chrono::system_clock::time_point next_cleanup_time_; // cycle count at which to clean overdue log }; @@ -1295,9 +1295,11 @@ void LogFileObject::Write( LogCleaner::LogCleaner() = default; void LogCleaner::Enable(const std::chrono::minutes& overdue) { + // for the files that have no severity specified + overdue_[-1] = overdue; // for backward compatability, set all severities to the same value for (int i = GLOG_INFO; i < NUM_SEVERITIES; i++) { - overdue_[static_cast(i)] = overdue; + overdue_[i] = overdue; } } @@ -1492,14 +1494,7 @@ bool LogCleaner::IsLogLastModifiedOver( std::chrono::system_clock::from_time_t(file_stat.st_mtime); const auto diff = current_time - last_modified_time; - int severity = FindFilepathLogSeverity(filepath); - // if the filepath does not have a severity, cant clean it - if (severity < 0) { - perror(("Cannot clean the file. No severity found for: " + filepath).c_str()); - return false; - } - - auto severity_it = overdue_.find(static_cast(severity)); + auto severity_it = overdue_.find(FindFilepathLogSeverity(filepath)); if (severity_it == overdue_.end()) { return false; } @@ -2666,6 +2661,10 @@ void EnableLogCleaner(LogSeverity severity, const std::chrono::minutes& overdue) void DisableLogCleaner() { log_cleaner.Disable(); } +void DisableLogCleaner(LogSeverity severity) { + log_cleaner.Disable(severity); +} + LogMessageTime::LogMessageTime() = default; namespace { From eab31bdd4cc97739e261e2cd1854f806b884cff2 Mon Sep 17 00:00:00 2001 From: HNOONa-0 Date: Sat, 22 Feb 2025 19:18:42 +0200 Subject: [PATCH 6/7] enable, disable generic logs. export the functions to be used --- src/glog/logging.h | 4 ++++ src/logging.cc | 27 +++++++++++++++++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/glog/logging.h b/src/glog/logging.h index 47439667c..ad12a66a1 100644 --- a/src/glog/logging.h +++ b/src/glog/logging.h @@ -485,6 +485,10 @@ InstallFailureFunction(logging_fail_func_t fail_func); GLOG_EXPORT void EnableLogCleaner(const std::chrono::minutes& overdue); GLOG_EXPORT void DisableLogCleaner(); GLOG_EXPORT void SetApplicationFingerprint(const std::string& fingerprint); +GLOG_EXPORT void EnableLogCleaner(LogSeverity severity, const std::chrono::minutes& overdue); +GLOG_EXPORT void DisableLogCleaner(LogSeverity severity); +GLOG_EXPORT void EnableLogCleanerForGenericLogs(const std::chrono::minutes& overdue); +GLOG_EXPORT void DisableLogCleanerForGenericLogs(); class LogSink; // defined below diff --git a/src/logging.cc b/src/logging.cc index a2676eb4f..1ac58b9ee 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -450,6 +450,9 @@ class LogCleaner { void Enable(const LogSeverity severity, const std::chrono::minutes& overdue); void Disable(); void Disable(const LogSeverity severity); + // For files with no severity in their names + void EnableForGenericLogs(const std::chrono::minutes& overdue); + void DisableForGenericLogs(); void Run(const std::chrono::system_clock::time_point& current_time, bool base_filename_selected, const string& base_filename, @@ -1295,11 +1298,11 @@ void LogFileObject::Write( LogCleaner::LogCleaner() = default; void LogCleaner::Enable(const std::chrono::minutes& overdue) { - // for the files that have no severity specified - overdue_[-1] = overdue; - // for backward compatability, set all severities to the same value + // For the files that have no severity specified, we use -1 as the key + EnableForGenericLogs(overdue); + // For backward compatability, set all severities to the same value for (int i = GLOG_INFO; i < NUM_SEVERITIES; i++) { - overdue_[i] = overdue; + Enable(static_cast(i), overdue); } } @@ -1313,6 +1316,14 @@ void LogCleaner::Disable(const LogSeverity severity) { overdue_.erase(severity); } +void LogCleaner::EnableForGenericLogs(const std::chrono::minutes& overdue) { + overdue_[-1] = overdue; +} + +void LogCleaner::DisableForGenericLogs() { + overdue_.erase(-1); +} + void LogCleaner::Run(const std::chrono::system_clock::time_point& current_time, bool base_filename_selected, const string& base_filename, const string& filename_extension) { @@ -2659,12 +2670,20 @@ void EnableLogCleaner(LogSeverity severity, const std::chrono::minutes& overdue) log_cleaner.Enable(severity, overdue); } +void EnableLogCleanerForGenericLogs(const std::chrono::minutes& overdue) { + log_cleaner.EnableForGenericLogs(overdue); +} + void DisableLogCleaner() { log_cleaner.Disable(); } void DisableLogCleaner(LogSeverity severity) { log_cleaner.Disable(severity); } +void DisableLogCleanerForGenericLogs() { + log_cleaner.DisableForGenericLogs(); +} + LogMessageTime::LogMessageTime() = default; namespace { From 314903eac784638e017007c894ca107602227642 Mon Sep 17 00:00:00 2001 From: HNOONa-0 Date: Sat, 22 Feb 2025 19:24:55 +0200 Subject: [PATCH 7/7] add deferred test file --- src/cleanup_deferred_unittest.cc | 100 +++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 src/cleanup_deferred_unittest.cc diff --git a/src/cleanup_deferred_unittest.cc b/src/cleanup_deferred_unittest.cc new file mode 100644 index 000000000..1583543da --- /dev/null +++ b/src/cleanup_deferred_unittest.cc @@ -0,0 +1,100 @@ +// 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(CleanDeferred, logging) { + using namespace std::chrono_literals; + const string dest = + FLAGS_test_tmpdir + "/test_cleanup_enable_generic_logs"; + google::EnableLogCleanerForGenericLogs(1h); + google::SetLogDestination(GLOG_INFO, dest.c_str()); + for (unsigned i = 0; i < 10; ++i) { + LOG(INFO) << "cleanup test"; + } + + google::DisableLogCleanerForGenericLogs(); + // delete the file + CHECK(unlink(dest.c_str()) == 0) << ": " << strerror(errno); +} + +int main(int argc, char** argv) { + FLAGS_colorlogtostderr = false; + FLAGS_timestamp_in_logfile_name = false; + FLAGS_logcleansecs = 1; +#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); +}