[c++] Refine Log Code (ray-project#2816)

* Support setting logging level from env variable

* Remove Env Variable related code

* lint

src/global_scheduler/global_scheduler.cc

@@ -455,8 +455,8 @@ void start_server(const char *node_ip_address,
 
 int main(int argc, char *argv[]) {
   InitShutdownRAII ray_log_shutdown_raii(
-      ray::RayLog::StartRayLog, ray::RayLog::ShutDownRayLog, argv[0], RAY_INFO,
-      /*log_dir=*/"");
+      ray::RayLog::StartRayLog, ray::RayLog::ShutDownRayLog, argv[0],
+      ray::RayLogLevel::INFO, /*log_dir=*/"");
   ray::RayLog::InstallFailureSignalHandler();
   signal(SIGTERM, signal_handler);
   /* IP address and port of the primary redis instance. */

src/local_scheduler/local_scheduler.cc

@@ -1423,8 +1423,8 @@ void start_server(
 #ifndef LOCAL_SCHEDULER_TEST
 int main(int argc, char *argv[]) {
   InitShutdownRAII ray_log_shutdown_raii(
-      ray::RayLog::StartRayLog, ray::RayLog::ShutDownRayLog, argv[0], RAY_INFO,
-      /*log_dir=*/"");
+      ray::RayLog::StartRayLog, ray::RayLog::ShutDownRayLog, argv[0],
+      ray::RayLogLevel::INFO, /*log_dir=*/"");
   ray::RayLog::InstallFailureSignalHandler();
   signal(SIGTERM, signal_handler);
   /* Path of the listening socket of the local scheduler. */

src/plasma/plasma_manager.cc

@@ -1626,8 +1626,8 @@ void signal_handler(int signal) {
 #ifndef PLASMA_TEST
 int main(int argc, char *argv[]) {
   InitShutdownRAII ray_log_shutdown_raii(
-      ray::RayLog::StartRayLog, ray::RayLog::ShutDownRayLog, argv[0], RAY_INFO,
-      /*log_dir=*/"");
+      ray::RayLog::StartRayLog, ray::RayLog::ShutDownRayLog, argv[0],
+      ray::RayLogLevel::INFO, /*log_dir=*/"");
   ray::RayLog::InstallFailureSignalHandler();
   signal(SIGTERM, signal_handler);
   /* Socket name of the plasma store this manager is connected to. */

src/ray/CMakeLists.txt

@@ -70,8 +70,8 @@ set(RAY_LIB_DEPENDENCIES
     gen_common_fbs)
 
 if(RAY_USE_GLOG)
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DRAY_USE_GLOG")
-  set(RAY_LIB_STATIC_LINK_LIBS ${RAY_LIB_STATIC_LINK_LIBS} ${GLOG_STATIC_LIB})
+  add_definitions(-DRAY_USE_GLOG)
+  set(RAY_LIB_STATIC_LINK_LIBS ${RAY_LIB_STATIC_LINK_LIBS} glog)
   set(RAY_LIB_DEPENDENCIES ${RAY_LIB_DEPENDENCIES} glog)
 endif()
 

src/ray/raylet/main.cc

@@ -16,7 +16,8 @@ static std::vector<std::string> parse_worker_command(std::string worker_command)
 
 int main(int argc, char *argv[]) {
   InitShutdownRAII ray_log_shutdown_raii(ray::RayLog::StartRayLog,
-                                         ray::RayLog::ShutDownRayLog, argv[0], RAY_INFO,
+                                         ray::RayLog::ShutDownRayLog, argv[0],
+                                         ray::RayLogLevel::INFO,
                                          /*log_dir=*/"");
   ray::RayLog::InstallFailureSignalHandler();
   RAY_CHECK(argc == 11 || argc == 12);

src/ray/raylet/monitor_main.cc

@@ -5,8 +5,8 @@
 
 int main(int argc, char *argv[]) {
   InitShutdownRAII ray_log_shutdown_raii(ray::RayLog::StartRayLog,
-                                         ray::RayLog::ShutDownRayLog, argv[0], RAY_INFO,
-                                         /*log_dir=*/"");
+                                         ray::RayLog::ShutDownRayLog, argv[0],
+                                         ray::RayLogLevel::INFO, /*log_dir=*/"");
   ray::RayLog::InstallFailureSignalHandler();
   RAY_CHECK(argc == 3 || argc == 4);
 

src/ray/util/logging.cc

@@ -1,8 +1,12 @@
+#include "ray/util/logging.h"
+
+#ifndef _WIN32
+#include <execinfo.h>
+#endif
+#include <stdlib.h>
 #include <cstdlib>
 #include <iostream>
 
-#include "ray/util/logging.h"
-
 #ifdef RAY_USE_GLOG
 #include "glog/logging.h"
 #endif
@@ -13,13 +17,13 @@ namespace ray {
 // which is independent of any libs.
 class CerrLog {
  public:
-  CerrLog(int severity) : severity_(severity), has_logged_(false) {}
+  CerrLog(RayLogLevel severity) : severity_(severity), has_logged_(false) {}
 
   virtual ~CerrLog() {
     if (has_logged_) {
       std::cerr << std::endl;
     }
-    if (severity_ == RAY_FATAL) {
+    if (severity_ == RayLogLevel::FATAL) {
       PrintBackTrace();
       std::abort();
     }
@@ -32,15 +36,15 @@ class CerrLog {
 
   template <class T>
   CerrLog &operator<<(const T &t) {
-    if (severity_ != RAY_DEBUG) {
+    if (severity_ != RayLogLevel::DEBUG) {
       has_logged_ = true;
       std::cerr << t;
     }
     return *this;
   }
 
  protected:
-  const int severity_;
+  const RayLogLevel severity_;
   bool has_logged_;
 
   void PrintBackTrace() {
@@ -52,39 +56,45 @@ class CerrLog {
   }
 };
 
-int RayLog::severity_threshold_ = RAY_INFO;
+#ifdef RAY_USE_GLOG
+typedef google::LogMessage LoggingProvider;
+#else
+typedef ray::CerrLog LoggingProvider;
+#endif
+
+RayLogLevel RayLog::severity_threshold_ = RayLogLevel::INFO;
 std::string RayLog::app_name_ = "";
 
 #ifdef RAY_USE_GLOG
 using namespace google;
 
 // Glog's severity map.
-static int GetMappedSeverity(int severity) {
+static int GetMappedSeverity(RayLogLevel severity) {
   switch (severity) {
-  case RAY_DEBUG:
+  case RayLogLevel::DEBUG:
     return GLOG_INFO;
-  case RAY_INFO:
+  case RayLogLevel::INFO:
     return GLOG_INFO;
-  case RAY_WARNING:
+  case RayLogLevel::WARNING:
     return GLOG_WARNING;
-  case RAY_ERROR:
+  case RayLogLevel::ERROR:
     return GLOG_ERROR;
-  case RAY_FATAL:
+  case RayLogLevel::FATAL:
     return GLOG_FATAL;
   default:
-    RAY_LOG(FATAL) << "Unsupported logging level: " << severity;
+    RAY_LOG(FATAL) << "Unsupported logging level: " << static_cast<int>(severity);
     // This return won't be hit but compiler needs it.
     return GLOG_FATAL;
   }
 }
 
 #endif
 
-void RayLog::StartRayLog(const std::string &app_name, int severity_threshold,
+void RayLog::StartRayLog(const std::string &app_name, RayLogLevel severity_threshold,
                          const std::string &log_dir) {
-#ifdef RAY_USE_GLOG
   severity_threshold_ = severity_threshold;
   app_name_ = app_name;
+#ifdef RAY_USE_GLOG
   int mapped_severity_threshold = GetMappedSeverity(severity_threshold_);
   google::InitGoogleLogging(app_name_.c_str());
   google::SetStderrLogging(mapped_severity_threshold);
@@ -122,34 +132,44 @@ void RayLog::InstallFailureSignalHandler() {
 #endif
 }
 
-bool RayLog::IsLevelEnabled(int log_level) { return log_level >= severity_threshold_; }
+bool RayLog::IsLevelEnabled(RayLogLevel log_level) {
+  return log_level >= severity_threshold_;
+}
 
-RayLog::RayLog(const char *file_name, int line_number, int severity)
-    // glog does not have DEBUG level, we can handle it here.
-    : is_enabled_(severity >= severity_threshold_) {
+RayLog::RayLog(const char *file_name, int line_number, RayLogLevel severity)
+    // glog does not have DEBUG level, we can handle it using is_enabled_.
+    : logging_provider_(nullptr),
+      is_enabled_(severity >= severity_threshold_) {
 #ifdef RAY_USE_GLOG
   if (is_enabled_) {
-    logging_provider_.reset(
-        new google::LogMessage(file_name, line_number, GetMappedSeverity(severity)));
+    logging_provider_ =
+        new google::LogMessage(file_name, line_number, GetMappedSeverity(severity));
   }
 #else
-  logging_provider_.reset(new CerrLog(severity));
-  *logging_provider_ << file_name << ":" << line_number << ": ";
+  auto logging_provider = new CerrLog(severity);
+  *logging_provider << file_name << ":" << line_number << ": ";
+  logging_provider_ = logging_provider;
 #endif
 }
 
 std::ostream &RayLog::Stream() {
+  auto logging_provider = reinterpret_cast<LoggingProvider *>(logging_provider_);
 #ifdef RAY_USE_GLOG
   // Before calling this function, user should check IsEnabled.
   // When IsEnabled == false, logging_provider_ will be empty.
-  return logging_provider_->stream();
+  return logging_provider->stream();
 #else
-  return logging_provider_->Stream();
+  return logging_provider->Stream();
 #endif
 }
 
 bool RayLog::IsEnabled() const { return is_enabled_; }
 
-RayLog::~RayLog() { logging_provider_.reset(); }
+RayLog::~RayLog() {
+  if (logging_provider_ != nullptr) {
+    delete reinterpret_cast<LoggingProvider *>(logging_provider_);
+    logging_provider_ = nullptr;
+  }
+}
 
 }  // namespace ray

src/ray/util/logging.h

@@ -1,49 +1,26 @@
 #ifndef RAY_UTIL_LOGGING_H
 #define RAY_UTIL_LOGGING_H
 
-#ifndef _WIN32
-#include <execinfo.h>
-#endif
-
-#include <cstdlib>
 #include <iostream>
-#include <memory>
-
-#include "ray/util/macros.h"
+#include <string>
 
-// Forward declaration for the log provider.
-#ifdef RAY_USE_GLOG
-namespace google {
-class LogMessage;
-}  // namespace google
-typedef google::LogMessage LoggingProvider;
-#else
 namespace ray {
-class CerrLog;
-}  // namespace ray
-typedef ray::CerrLog LoggingProvider;
-#endif
 
-namespace ray {
-// Log levels. LOG ignores them, so their values are abitrary.
-
-#define RAY_DEBUG (-1)
-#define RAY_INFO 0
-#define RAY_WARNING 1
-#define RAY_ERROR 2
-#define RAY_FATAL 3
+enum class RayLogLevel { DEBUG = -1, INFO = 0, WARNING = 1, ERROR = 2, FATAL = 3 };
 
 #define RAY_LOG_INTERNAL(level) ::ray::RayLog(__FILE__, __LINE__, level)
 
-#define RAY_LOG(level) \
-  if (ray::RayLog::IsLevelEnabled(RAY_##level)) RAY_LOG_INTERNAL(RAY_##level)
+#define RAY_LOG(level)                                      \
+  if (ray::RayLog::IsLevelEnabled(ray::RayLogLevel::level)) \
+  RAY_LOG_INTERNAL(ray::RayLogLevel::level)
 
 #define RAY_IGNORE_EXPR(expr) ((void)(expr))
 
-#define RAY_CHECK(condition)                                                          \
-  (condition) ? RAY_IGNORE_EXPR(0) : ::ray::Voidify() &                               \
-                                         ::ray::RayLog(__FILE__, __LINE__, RAY_FATAL) \
-                                             << " Check failed: " #condition " "
+#define RAY_CHECK(condition)                                                   \
+  (condition) ? RAY_IGNORE_EXPR(0)                                             \
+              : ::ray::Voidify() &                                             \
+                    ::ray::RayLog(__FILE__, __LINE__, ray::RayLogLevel::FATAL) \
+                        << " Check failed: " #condition " "
 
 #ifdef NDEBUG
 
@@ -67,14 +44,13 @@ class RayLogBase {
  public:
   virtual ~RayLogBase(){};
 
+  // By default, this class is a null log because it return false here.
   virtual bool IsEnabled() const { return false; };
 
   template <typename T>
   RayLogBase &operator<<(const T &t) {
     if (IsEnabled()) {
       Stream() << t;
-    } else {
-      RAY_IGNORE_EXPR(t);
     }
     return *this;
   }
@@ -85,7 +61,7 @@ class RayLogBase {
 
 class RayLog : public RayLogBase {
  public:
-  RayLog(const char *file_name, int line_number, int severity);
+  RayLog(const char *file_name, int line_number, RayLogLevel severity);
 
   virtual ~RayLog();
 
@@ -94,29 +70,37 @@ class RayLog : public RayLogBase {
   /// \return True if logging is enabled and false otherwise.
   virtual bool IsEnabled() const;
 
-  // The init function of ray log for a program which should be called only once.
-  // If logDir is empty, the log won't output to file.
-  static void StartRayLog(const std::string &appName, int severity_threshold = RAY_ERROR,
+  /// The init function of ray log for a program which should be called only once.
+  ///
+  /// \parem appName The app name which starts the log.
+  /// \param severity_threshold Logging threshold for the program.
+  /// \param logDir Logging output file name. If empty, the log won't output to file.
+  static void StartRayLog(const std::string &appName,
+                          RayLogLevel severity_threshold = RayLogLevel::INFO,
                           const std::string &logDir = "");
 
-  // The shutdown function of ray log which should be used with StartRayLog as a pair.
+  /// The shutdown function of ray log which should be used with StartRayLog as a pair.
   static void ShutDownRayLog();
 
   /// Return whether or not the log level is enabled in current setting.
   ///
   /// \param log_level The input log level to test.
   /// \return True if input log level is not lower than the threshold.
-  static bool IsLevelEnabled(int log_level);
+  static bool IsLevelEnabled(RayLogLevel log_level);
 
-  // Install the failure signal handler to output call stack when crash.
-  // If glog is not installed, this function won't do anything.
+  /// Install the failure signal handler to output call stack when crash.
+  /// If glog is not installed, this function won't do anything.
   static void InstallFailureSignalHandler();
+  // Get the log level from environment variable.
+  static RayLogLevel GetLogLevelFromEnv();
 
  private:
-  std::unique_ptr<LoggingProvider> logging_provider_;
+  // Hide the implementation of log provider by void *.
+  // Otherwise, lib user may define the same macro to use the correct header file.
+  void *logging_provider_;
   /// True if log messages should be logged and false if they should be ignored.
   bool is_enabled_;
-  static int severity_threshold_;
+  static RayLogLevel severity_threshold_;
   // In InitGoogleLogging, it simply keeps the pointer.
   // We need to make sure the app name passed to InitGoogleLogging exist.
   static std::string app_name_;

src/ray/util/logging_test.cc

@@ -41,14 +41,14 @@ TEST(PrintLogTest, LogTestWithoutInit) {
 
 TEST(PrintLogTest, LogTestWithInit) {
   // Test empty app name.
-  RayLog::StartRayLog("", RAY_DEBUG);
+  RayLog::StartRayLog("", RayLogLevel::DEBUG);
   PrintLog();
   RayLog::ShutDownRayLog();
 }
 
 // This test will output large amount of logs to stderr, should be disabled in travis.
 TEST(LogPerfTest, PerfTest) {
-  RayLog::StartRayLog("/fake/path/to/appdire/LogPerfTest", RAY_ERROR, "/tmp/");
+  RayLog::StartRayLog("/fake/path/to/appdire/LogPerfTest", RayLogLevel::ERROR, "/tmp/");
   int rounds = 100000;
 
   int64_t start_time = current_time_ms();

src/ray/util/signal_test.cc

@@ -82,7 +82,8 @@ TEST(SignalTest, SIGILL_Test) {
 
 int main(int argc, char **argv) {
   InitShutdownRAII ray_log_shutdown_raii(ray::RayLog::StartRayLog,
-                                         ray::RayLog::ShutDownRayLog, argv[0], RAY_INFO,
+                                         ray::RayLog::ShutDownRayLog, argv[0],
+                                         ray::RayLogLevel::INFO,
                                          /*log_dir=*/"");
   ray::RayLog::InstallFailureSignalHandler();
   ::testing::InitGoogleTest(&argc, argv);