Clean up / consolidate config logging logic

llarp/config/config.cpp

@@ -73,6 +73,16 @@ namespace llarp
     return val;
   }
 
+  LoggingConfig::LogType LoggingConfig::LogTypeFromString(const std::string& str)
+  {
+    if (str == "unknown") return LogType::Unknown;
+    else if (str == "file") return LogType::File;
+    else if (str == "json") return LogType::Json;
+    else if (str == "syslog") return LogType::Syslog;
+
+    return LogType::Unknown;
+  }
+
   void
   RouterConfig::defineConfigOptions(Configuration& conf)
   {
@@ -388,63 +398,28 @@ namespace llarp
   void
   LoggingConfig::defineConfigOptions(Configuration& conf)
   {
+    conf.defineOption<std::string>("logging", "type", false, "file",
+      [this](std::string arg) {
+      LoggingConfig::LogType type = LogTypeFromString(arg);
+        if (type == LogType::Unknown)
+          throw std::invalid_argument(stringify("invalid log type: ", arg));
+
+        m_logType = type;
+      });
 
-    conf.defineOption<std::string>("logging", "type", false, "",
+    conf.defineOption<std::string>("logging", "level", false, "info",
       [this](std::string arg) {
-        (void)arg;
-        // TODO: this whole thing needs a rewrite, eww eww eww
+        nonstd::optional<LogLevel> level = LogLevelFromString(arg);
+        if (not level.has_value())
+          throw std::invalid_argument(stringify( "invalid log level value: ", arg));
+
+        m_logLevel = level.value();
       });
 
-    /*
-    if(key == "type" && val == "syslog")
-    {
-      // TODO(despair): write event log syslog class
-#if defined(_WIN32)
-      LogError("syslog not supported on win32");
-#else
-      LogInfo("Switching to syslog");
-      LogContext::Instance().logStream = std::make_unique<SysLogStream>();
-#endif
-    }
-    if (key == "level")
-    {
-      const auto maybe = LogLevelFromString(str(val));
-      if (not maybe.has_value())
-      {
-        LogError("bad log level: ", val);
-        return;
-      }
-      const LogLevel lvl = maybe.value();
-      LogContext::Instance().runtimeLevel = lvl;
-      LogInfo("Log level set to ", LogLevelToName(lvl));
-    }
-    if (key == "type" && val == "json")
-    {
-      m_LogJSON = true;
-    }
-    if (key == "file")
-    {
-      LogInfo("open log file: ", val);
-      std::string fname{val};
-      FILE* const logfile = ::fopen(fname.c_str(), "a");
-      if (logfile)
-      {
-        m_LogFile = logfile;
-        LogInfo("will log to file ", val);
-      }
-      else if (errno)
-      {
-        LogError("could not open log file at '", val, "': ", strerror(errno));
-        errno = 0;
-      }
-      else
-      {
-        LogError(
-            "failed to open log file at '", val, "' for an unknown reason, bailing tf out kbai");
-        ::abort();
-      }
-    }
-    */
+    conf.defineOption<std::string>("logging", "file", false, "stdout",
+      [this](std::string arg) {
+        m_logFile = arg;
+      });
   }
 
   bool

llarp/config/config.hpp

@@ -227,8 +227,18 @@ namespace llarp
 
   struct LoggingConfig
   {
-    bool m_LogJSON = false;
-    FILE* m_LogFile = stdout;
+    enum class LogType
+    {
+      Unknown = 0,
+      File,
+      Json,
+      Syslog,
+    };
+    static LogType LogTypeFromString(const std::string&);
+
+    LogType m_logType;
+    LogLevel m_logLevel;
+    std::string m_logFile;
 
     void
     defineConfigOptions(Configuration& conf);

llarp/router/router.cpp

@@ -614,17 +614,51 @@ namespace llarp
 
     // Logging config
 
-    auto logfile = conf->logging.m_LogFile;
-
-    if (conf->logging.m_LogJSON)
+    FILE* logfile = nullptr;
+    if (conf->logging.m_logFile == "stdout")
+    {
+      logfile = stdout;
+    }
+    else
     {
-      LogContext::Instance().logStream =
-          std::make_unique<JSONLogStream>(diskworker(), logfile, 100ms, logfile != stdout);
+      logfile = ::fopen(conf->logging.m_logFile.c_str(), "a");
+      if (not logfile)
+      {
+        LogError("could not open logfile ", conf->logging.m_logFile, ", errno: ", strerror(errno));
+        return false;
+      }
     }
-    else if (logfile != stdout)
+
+    switch (conf->logging.m_logType)
     {
-      LogContext::Instance().logStream =
-          std::make_unique<FileLogStream>(diskworker(), logfile, 100ms, true);
+      case LoggingConfig::LogType::Unknown:
+        // Config shouldn't allow this to happen, so we die swiftly if we get here
+        assert(conf->logging.m_logType != LoggingConfig::LogType::Unknown);
+        break;
+      case LoggingConfig::LogType::File:
+        LogContext::Instance().logStream =
+            std::make_unique< FileLogStream >(diskworker(), logfile, 100ms, true);
+        break;
+      case LoggingConfig::LogType::Json:
+        LogContext::Instance().logStream = std::make_unique< JSONLogStream >(
+            diskworker(), logfile, 100ms, logfile != stdout);
+        break;
+      case LoggingConfig::LogType::Syslog:
+        if (logfile)
+        {
+          // TODO: this logic should be handled in Config
+          LogError("Cannot mix log type=syslog and file=*");
+          ::fclose(logfile);
+          return false;
+        }
+#if defined(_WIN32)
+        LogError("syslog not supported on win32");
+        return false;
+#else
+        LogContext::Instance().logStream = std::make_unique< SysLogStream >();
+#endif
+        break;
+
     }
 
     netConfig.insert(conf->dns.netConfig.begin(), conf->dns.netConfig.end());