Merge pull request #292 from microsoft/apilogerrorsnocond

Process plog severity correctly from CLI11 flag

Author: abeltrano

src/common/server/NetRemoteServerConfiguration.cxx

@@ -27,9 +27,10 @@ ConfigureCliAppOptions(CLI::App& app, NetRemoteServerConfiguration& config)
         "The address to listen on for incoming connections");
 
     app.add_flag(
-        "-v,--verbosity",
-        config.LogVerbosity,
-        "The log verbosity level. Supply multiple times to increase verbosity (0=warnings, errors, and fatal messages, 1=info messages, 2=debug messages, 3=verbose messages)");
+           "-v,--verbosity",
+           config.LogVerbosity,
+           "The log verbosity level. Supply multiple times to increase verbosity (0=fatal, 1=errors, 2=warnings, 3=info, 4=debug, 5+=verbose)")
+        ->default_val(NetRemoteServerConfiguration::LogVerbosityDefault);
 
     app.add_flag(
         "--enable-file-logging",

src/common/server/include/microsoft/net/remote/service/NetRemoteServerConfiguration.hxx

@@ -18,6 +18,11 @@ namespace Microsoft::Net::Remote::Service
  */
 struct NetRemoteServerConfiguration
 {
+    /**
+     * @brief Default log verbosity.
+     */
+    static constexpr auto LogVerbosityDefault = 3;
+
     /**
      * @brief Create a NetRemoteServerConfiguration object from command-line
      * arguments.
@@ -52,12 +57,21 @@ struct NetRemoteServerConfiguration
     bool RunInBackground{ false };
 
     /**
-     * @brief How verbose log output should be. The default level is 0, which
-     * will show all warnings, errors, and fatal messages. A level of 1 will
-     * show all info messages, and a level of 2 will show all debug messages,
-     * and a level of 3 or above will show all verbose messages.
+     * @brief Log verbosity threshold. The default level is 'info' (3) which will print all log messages with verbosity
+     * level info, warning, error, and fatal.
+     *
+     * --------------------
+     * | level | severity |
+     * | ----- | -------- |
+     * |   0   |  fatal   |
+     * |   1   |  error   |
+     * |   2   |  warning |
+     * |   3   |  info    |
+     * |   4   |  debug   |
+     * |   5+  |  verbose |
+     * --------------------
      */
-    uint32_t LogVerbosity{ 0 };
+    uint32_t LogVerbosity{ LogVerbosityDefault };
 
     /**
      * @brief Whether to enable logging to file or not.

src/common/shared/logging/LogUtils.cxx

@@ -31,12 +31,16 @@ logging::LogVerbosityToPlogSeverity(uint32_t verbosity) noexcept
 {
     switch (verbosity) {
     case 0:
-        return plog::warning;
+        return plog::fatal;
     case 1:
-        return plog::info;
+        return plog::error;
     case 2:
-        return plog::debug;
+        return plog::warning;
     case 3:
+        return plog::info;
+    case 4:
+        return plog::debug;
+    case 5:
     default:
         return plog::verbose;
     }

src/linux/server/Main.cxx

@@ -9,6 +9,7 @@
 #include <utility>
 
 #include <logging/LogUtils.hxx>
+#include <magic_enum.hpp>
 #include <microsoft/net/NetworkManager.hxx>
 #include <microsoft/net/NetworkOperationsLinux.hxx>
 #include <microsoft/net/remote/service/NetRemoteServer.hxx>
@@ -102,6 +103,8 @@ main(int argc, char *argv[])
         plog::init(logSeverity).addAppender(plog::get<std::to_underlying(LogInstanceId::File)>());
     }
 
+    LOGN << std::format("Netremote server starting (log level={})", magic_enum::enum_name(logSeverity));
+
     // Create an access point manager and discovery agent.
     auto accessPointManager = AccessPointManager::Create();
     auto accessPointControllerFactory = std::make_unique<AccessPointControllerLinuxFactory>();
@@ -125,7 +128,6 @@ main(int argc, char *argv[])
     NetRemoteServer server{ configuration };
 
     // Start the server.
-    LOGI << "Netremote server starting";
     server.Run();
 
     // If running in the background, daemonize the process.
@@ -149,13 +151,13 @@ main(int argc, char *argv[])
             });
         }
 
-        LOGI << "Netremote server stopping";
+        LOGN << "Netremote server stopping";
         auto &grpcServer = server.GetGrpcServer();
         grpcServer->Shutdown();
         grpcServer->Wait();
     }
 
-    LOGI << "Netremote server stopped";
+    LOGN << "Netremote server stopped";
 
     return 0;
 }

src/linux/wifi/apmanager/AccessPointDiscoveryAgentOperationsNetlink.cxx

@@ -272,6 +272,11 @@ void
 AccessPointDiscoveryAgentOperationsNetlink::ProcessNetlinkMessagesThread(NetlinkSocket netlinkSocket, std::stop_token stopToken)
 // NOLINTEND(performance-unnecessary-value-param)
 {
+    LOGD << "Netlink message processing thread started";
+    auto logOnExit = notstd::ScopeExit([] {
+        LOGD << "Netlink message processing thread stopped";
+    });
+
     // Disable sequence number checking since it is not required for event notifications.
     nl_socket_disable_seq_check(netlinkSocket);
 
@@ -394,7 +399,6 @@ AccessPointDiscoveryAgentOperationsNetlink::ProcessNetlinkMessagesThread(Netlink
         }
     }
 
-    LOGI << "Netlink message processing thread has exited";
 }
 
 /* static */