logs: making the admin 'set log level' function a util (envoyproxy#29196

) Risk Level: low (minor refactor) Testing: existing admin tests, new E-M test Docs Changes: n/a Release Notes: n/a Signed-off-by: Alyssa Wilk <[email protected]>
jmarantz · Aug 24, 2023 · d269666 · d269666
1 parent 5cf06ec
commit d269666
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 14 deletions.
diff --git a/mobile/test/common/common/lambda_logger_delegate_test.cc b/mobile/test/common/common/lambda_logger_delegate_test.cc
@@ -6,6 +6,7 @@
 
 using testing::_;
 using testing::HasSubstr;
+using testing::Not;
 
 namespace Envoy {
 namespace Logger {
@@ -38,6 +39,36 @@ TEST_F(LambdaDelegateTest, LogCb) {
   EXPECT_THAT(actual_msg, HasSubstr(expected_msg));
 }
 
+TEST_F(LambdaDelegateTest, LogCbWithLevels) {
+  std::string unexpected_msg = "Hello NoLambdaDelegate";
+  std::string expected_msg = "Hello LambdaDelegate";
+  std::string actual_msg;
+
+  LambdaDelegate delegate({[](envoy_data data, const void* context) -> void {
+                             auto* actual_msg =
+                                 static_cast<std::string*>(const_cast<void*>(context));
+                             *actual_msg = Data::Utility::copyToString(data);
+                             release_envoy_data(data);
+                           },
+                           [](const void*) -> void {}, &actual_msg},
+                          Registry::getSink());
+
+  // Set the log to critical. The message should not be logged.
+  Context::changeAllLogLevels(spdlog::level::critical);
+  ENVOY_LOG_MISC(error, unexpected_msg);
+  EXPECT_THAT(actual_msg, Not(HasSubstr(unexpected_msg)));
+
+  // Change to error. The message should be logged.
+  Context::changeAllLogLevels(spdlog::level::err);
+  ENVOY_LOG_MISC(error, expected_msg);
+  EXPECT_THAT(actual_msg, HasSubstr(expected_msg));
+
+  // Change back to critical and test one more time.
+  Context::changeAllLogLevels(spdlog::level::critical);
+  ENVOY_LOG_MISC(error, expected_msg);
+  EXPECT_THAT(actual_msg, Not(HasSubstr(unexpected_msg)));
+}
+
 TEST_F(LambdaDelegateTest, ReleaseCb) {
   bool released = false;
 

diff --git a/source/common/common/logger.cc b/source/common/common/logger.cc
@@ -202,6 +202,19 @@ bool Context::useFineGrainLogger() {
   return false;
 }
 
+void Context::changeAllLogLevels(spdlog::level::level_enum level) {
+  if (!useFineGrainLogger()) {
+    ENVOY_LOG_MISC(info, "change all log levels: level='{}'",
+                   spdlog::level::level_string_views[level]);
+    Registry::setLogLevel(level);
+  } else {
+    // Level setting with Fine-Grain Logger.
+    FINE_GRAIN_LOG(info, "change all log levels: level='{}'",
+                   spdlog::level::level_string_views[level]);
+    getFineGrainLogContext().setAllFineGrainLoggers(level);
+  }
+}
+
 void Context::enableFineGrainLogger() {
   if (current_context) {
     current_context->enable_fine_grain_logging_ = true;

diff --git a/source/common/common/logger.h b/source/common/common/logger.h
@@ -304,6 +304,9 @@ class Context {
    */
   static bool useFineGrainLogger();
 
+  // Change the log level for all loggers (fine grained or otherwise) to the level provided.
+  static void changeAllLogLevels(spdlog::level::level_enum level);
+
   static void enableFineGrainLogger();
   static void disableFineGrainLogger();
 

diff --git a/source/server/admin/logs_handler.cc b/source/server/admin/logs_handler.cc
@@ -111,7 +111,7 @@ absl::Status LogsHandler::changeLogLevel(Http::Utility::QueryParams& params) {
       return level_to_use.status();
     }
 
-    changeAllLogLevels(*level_to_use);
+    Logger::Context::changeAllLogLevels(*level_to_use);
     return absl::OkStatus();
   }
 
@@ -163,18 +163,6 @@ absl::Status LogsHandler::changeLogLevel(Http::Utility::QueryParams& params) {
   return changeLogLevels(name_levels);
 }
 
-void LogsHandler::changeAllLogLevels(spdlog::level::level_enum level) {
-  if (!Logger::Context::useFineGrainLogger()) {
-    ENVOY_LOG(info, "change all log levels: level='{}'", spdlog::level::level_string_views[level]);
-    Logger::Registry::setLogLevel(level);
-  } else {
-    // Level setting with Fine-Grain Logger.
-    FINE_GRAIN_LOG(info, "change all log levels: level='{}'",
-                   spdlog::level::level_string_views[level]);
-    getFineGrainLogContext().setAllFineGrainLoggers(level);
-  }
-}
-
 absl::Status LogsHandler::changeLogLevels(
     const absl::flat_hash_map<absl::string_view, spdlog::level::level_enum>& changes) {
   if (!Logger::Context::useFineGrainLogger()) {

diff --git a/source/server/admin/logs_handler.h b/source/server/admin/logs_handler.h
@@ -43,7 +43,6 @@ class LogsHandler : public HandlerContextBase, Logger::Loggable<Logger::Id::admi
    * @param params supplies the incoming endpoint query or post params.
    */
   absl::Status changeLogLevel(Http::Utility::QueryParams& params);
-  void changeAllLogLevels(spdlog::level::level_enum level);
   absl::Status
   changeLogLevels(const absl::flat_hash_map<absl::string_view, spdlog::level::level_enum>& changes);