From 5a2cb2a1b658646f2d15920a343629961a316d4f Mon Sep 17 00:00:00 2001 From: mturnock Date: Mon, 11 Mar 2024 20:37:46 +0000 Subject: [PATCH 1/2] Enable log cleaner to clean files of all verbosities Add OverrideLogCleanerFilePathPrefix to override the files the log cleaner removes --- src/glog/logging.h | 1 + src/logging.cc | 44 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/glog/logging.h b/src/glog/logging.h index 6e36eaadf..f6acb3169 100644 --- a/src/glog/logging.h +++ b/src/glog/logging.h @@ -484,6 +484,7 @@ InstallFailureFunction(logging_fail_func_t fail_func); // Enable/Disable old log cleaner. GLOG_EXPORT void EnableLogCleaner(const std::chrono::minutes& overdue); GLOG_EXPORT void DisableLogCleaner(); +GLOG_EXPORT void OverrideLogCleanerFilePathPrefix(const std::string& prefix); GLOG_EXPORT void SetApplicationFingerprint(const std::string& fingerprint); class LogSink; // defined below diff --git a/src/logging.cc b/src/logging.cc index 08b2ab387..595f08bcc 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -439,6 +439,8 @@ class LogCleaner { void Enable(const std::chrono::minutes& overdue); void Disable(); + void OverrideFilePathPrefix(const std::string& prefix); + void Run(const std::chrono::system_clock::time_point& current_time, bool base_filename_selected, const string& base_filename, const string& filename_extension); @@ -464,6 +466,8 @@ class LogCleaner { std::chrono::duration>{1}}; std::chrono::system_clock::time_point next_cleanup_time_; // cycle count at which to clean overdue log + bool override_required_file_path_prefix_{false}; + std::string required_file_path_prefix_; }; LogCleaner log_cleaner; @@ -1291,6 +1295,11 @@ void LogCleaner::Enable(const std::chrono::minutes& overdue) { void LogCleaner::Disable() { enabled_ = false; } +void LogCleaner::OverrideFilePathPrefix(const std::string& prefix) { + override_required_file_path_prefix_ = true; + required_file_path_prefix_ = prefix; +} + void LogCleaner::Run(const std::chrono::system_clock::time_point& current_time, bool base_filename_selected, const string& base_filename, const string& filename_extension) { @@ -1309,17 +1318,20 @@ void LogCleaner::Run(const std::chrono::system_clock::time_point& current_time, 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 (base_filename_selected || override_required_file_path_prefix_) { + const string base_path = override_required_file_path_prefix_ + ? required_file_path_prefix_ + : base_filename; + size_t pos = base_path.find_last_of(possible_dir_delim, string::npos, + sizeof(possible_dir_delim)); if (pos != string::npos) { - string dir = base_filename.substr(0, pos + 1); + string dir = base_path.substr(0, pos + 1); dirs.push_back(dir); } else { dirs.emplace_back("."); } + } else { + dirs = GetLoggingDirectories(); } for (const std::string& dir : dirs) { @@ -1377,6 +1389,22 @@ vector LogCleaner::GetOverdueLogNames( bool LogCleaner::IsLogFromCurrentProject( const string& filepath, const string& base_filename, const string& filename_extension) const { + // If overriding log file path prefix, check if the file path starts with + // 'required_file_path_prefix_' and ends with `filename_extension`. + if (override_required_file_path_prefix_) { + if (filepath.find(required_file_path_prefix_) != 0) { + return false; + } + if (filename_extension.empty()) { + return true; + } + if (filepath.size() < filename_extension.size()) { + return false; + } + return filename_extension == + filepath.substr(filepath.size() - filename_extension.size()); + } + // We should remove duplicated delimiters from `base_filename`, e.g., // before: "/tmp//.." // after: "/tmp/.." @@ -2629,6 +2657,10 @@ void EnableLogCleaner(const std::chrono::minutes& overdue) { void DisableLogCleaner() { log_cleaner.Disable(); } +void OverrideLogCleanerFilePathPrefix(const std::string& prefix) { + log_cleaner.OverrideFilePathPrefix(prefix); +} + LogMessageTime::LogMessageTime() = default; namespace { From 2d1ffb96d893ddc329b2b88740957c79083dc52f Mon Sep 17 00:00:00 2001 From: mturnock Date: Mon, 11 Mar 2024 23:21:57 +0000 Subject: [PATCH 2/2] Test for OverrideLogCleanerFilePathPrefix --- CMakeLists.txt | 13 +++ bazel/glog.bzl | 1 + cmake/RunCleanerTestFilePathOverride.cmake | 32 +++++++ ...leanup_with_file_path_override_unittest.cc | 96 +++++++++++++++++++ 4 files changed, 142 insertions(+) create mode 100644 cmake/RunCleanerTestFilePathOverride.cmake create mode 100644 src/cleanup_with_file_path_override_unittest.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 847530698..c239ae1dd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -728,6 +728,11 @@ if (BUILD_TESTING) target_link_libraries (cleanup_with_relative_prefix_unittest PRIVATE glog_test) + add_executable (cleanup_with_file_path_override_unittest + src/cleanup_with_file_path_override_unittest.cc) + + target_link_libraries (cleanup_with_file_path_override_unittest PRIVATE glog_test) + set (CLEANUP_LOG_DIR ${glog_BINARY_DIR}/cleanup_tests) add_test (NAME cleanup_init COMMAND @@ -754,6 +759,13 @@ if (BUILD_TESTING) -DTEST_SUBDIR=test_subdir/ -P ${glog_SOURCE_DIR}/cmake/RunCleanerTest3.cmake WORKING_DIRECTORY ${glog_BINARY_DIR}) + add_test (NAME cleanup_with_file_path_override COMMAND + ${CMAKE_COMMAND} + -DLOGCLEANUP=$ + -DTEST_DIR=${glog_BINARY_DIR}/ + -DTEST_SUBDIR=test_subdir/ + -P ${glog_SOURCE_DIR}/cmake/RunCleanerTestFilePathOverride.cmake + WORKING_DIRECTORY ${glog_BINARY_DIR}) # Fixtures setup set_tests_properties (cleanup_init PROPERTIES FIXTURES_SETUP logcleanuptest) @@ -763,6 +775,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_with_file_path_override PROPERTIES FIXTURES_REQUIRED logcleanuptest) add_executable (striplog0_unittest src/striplog_unittest.cc diff --git a/bazel/glog.bzl b/bazel/glog.bzl index a311d0d60..e932e9ae7 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_with_file_path_override", # "demangle", # Broken # "logging", # Broken # "mock-log", # Broken diff --git a/cmake/RunCleanerTestFilePathOverride.cmake b/cmake/RunCleanerTestFilePathOverride.cmake new file mode 100644 index 000000000..7c8ef1594 --- /dev/null +++ b/cmake/RunCleanerTestFilePathOverride.cmake @@ -0,0 +1,32 @@ +file (TOUCH test_cleanup_SHOULD_BE_CLEANED.logfileoverride) +file (TOUCH test_cleanup_SHOULD_NOT_BE_CLEANED.logfileoverride.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_*.logfileoverride) +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_SHOULD_NOT_BE_CLEANED.logfileoverride.notalog) diff --git a/src/cleanup_with_file_path_override_unittest.cc b/src/cleanup_with_file_path_override_unittest.cc new file mode 100644 index 000000000..c640a6b29 --- /dev/null +++ b/src/cleanup_with_file_path_override_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(CleanImmediatelyWithFilePathOverride, logging) { + using namespace std::chrono_literals; + google::EnableLogCleaner(0h); + google::SetLogFilenameExtension(".logfileoverride"); + google::SetLogDestination(GLOG_ERROR, "test_cleanup_error_"); + google::OverrideLogCleanerFilePathPrefix("test_cleanup_"); + + LOG(ERROR) << "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); +}