Merge pull request #51 from morphis/f/logger-rework

Rework logger integration with the emugl component a bit

external/android-emugl/host/libs/Translator/GLES_CM/GLEScmImp.cpp

@@ -71,7 +71,6 @@ static GLESiface  s_glesIface = {
 extern "C" {
 
 static void initGLESx() {
-    DBG("No special initialization necessary for GLES_CM\n");
     return;
 }
 

external/android-emugl/host/libs/Translator/GLES_V2/GLESv2Imp.cpp

@@ -70,7 +70,6 @@ static GLESiface  s_glesIface = {
 extern "C" {
 
 static void initGLESx() {
-    DBG("No special initialization necessary for GLES_V2\n");
     return;
 }
 

external/android-emugl/host/tools/emugen/ApiGen.cpp

@@ -823,12 +823,7 @@ int ApiGen::genDecoderImpl(const std::string &filename)
     fprintf(fp, "typedef unsigned int tsize_t; // Target \"size_t\", which is 32-bit for now. It may or may not be the same as host's size_t when emugen is compiled.\n\n");
 
     // helper macros
-    fprintf(fp,
+    fprintf(fp, "#  define DEBUG(...) do { if (emugl_cxt_logger) { emugl_cxt_logger(LogLevel::TRACE, __VA_ARGS__); } } while(0)\n\n");
-            "#ifdef OPENGL_DEBUG_PRINTOUT\n"
-            "#  define DEBUG(...) do { if (emugl_cxt_logger) { emugl_cxt_logger(__VA_ARGS__); } } while(0)\n"
-            "#else\n"
-            "#  define DEBUG(...)  ((void)0)\n"
-            "#endif\n\n");
 
     fprintf(fp,
             "#ifdef CHECK_GLERROR\n"
@@ -913,7 +908,7 @@ int ApiGen::genDecoderImpl(const std::string &filename)
                 }
             } else if (pass == PASS_DebugPrint) {
                 fprintf(fp,
-                        "\t\t\tDEBUG(\"%s(%%p): %s(%s)\\n\", stream",
+                        "\t\t\tDEBUG(\"%s(%%p): %s(%s)\", stream",
                         m_basename.c_str(),
                         e->name().c_str(),
                         printString.c_str());

external/android-emugl/shared/OpenglCodecCommon/ChecksumCalculatorThreadInfo.cpp

@@ -98,6 +98,6 @@ void ChecksumCalculatorThreadInfo::validOrDie(void* buf,
     // We should actually call crashhandler_die(message), but I don't think we
     // can link to that library from here
     if (!validate(buf, bufLen, checksum, checksumLen)) {
-        emugl_crash_reporter(message);
+        emugl_crash_reporter(emugl::LogLevel::FATAL, message);
     }
 }

external/android-emugl/shared/emugl/common/crash_reporter.cpp

@@ -18,13 +18,13 @@
 
 #include <cstdlib>
 
-void default_crash_reporter(const char* format, ...) {
+void default_crash_reporter(const emugl::LogLevel &level, const char* format, ...) {
     abort();
 }
 
-crash_reporter_t emugl_crash_reporter = default_crash_reporter;
+logger_t emugl_crash_reporter = default_crash_reporter;
 
-void set_emugl_crash_reporter(crash_reporter_t crash_reporter) {
+void set_emugl_crash_reporter(logger_t crash_reporter) {
     if (crash_reporter) {
         emugl_crash_reporter = crash_reporter;
     } else {

external/android-emugl/shared/emugl/common/crash_reporter.h

@@ -16,7 +16,7 @@
 
 #pragma once
 
-typedef void (*crash_reporter_t)(const char* format, ...);
+#include "emugl/common/logging.h"
 
-extern crash_reporter_t emugl_crash_reporter;
+extern logger_t emugl_crash_reporter;
-void set_emugl_crash_reporter(crash_reporter_t crash_reporter);
+void set_emugl_crash_reporter(logger_t crash_reporter);

external/android-emugl/shared/emugl/common/logging.cpp

@@ -16,7 +16,7 @@
 
 #include "emugl/common/logging.h"
 
-void default_logger(const char* fmt, ...) { }
+void default_logger(const emugl::LogLevel &level, const char* fmt, ...) { }
 
 logger_t emugl_logger = default_logger;
 logger_t emugl_cxt_logger = default_logger;

external/android-emugl/shared/emugl/common/logging.h

@@ -14,10 +14,24 @@
 * limitations under the License.
 */
 
+#ifndef EMUGL_COMMON_LOGGING_H_
+#define EMUGL_COMMON_LOGGING_H_
+
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wredundant-decls"
 
-typedef void (*logger_t)(const char* fmt, ...);
+namespace emugl {
+enum class LogLevel {
+  TRACE,
+  DEBUG,
+  INFO,
+  WARNING,
+  ERROR,
+  FATAL,
+};
+} // namespace emugl
+
+typedef void (*logger_t)(const emugl::LogLevel &level, const char* fmt, ...);
 extern logger_t emugl_logger;
 extern logger_t emugl_cxt_logger;
 void set_emugl_logger(logger_t f);
@@ -40,3 +54,5 @@ void set_emugl_cxt_logger(logger_t f);
 #endif
 
 #pragma GCC diagnostic pop
+
+#endif

src/anbox/daemon.cpp

@@ -39,6 +39,12 @@ Daemon::Daemon()
      .command(std::make_shared<cmds::SessionManager>())
      .command(std::make_shared<cmds::Launch>())
      .command(std::make_shared<cmds::ContainerManager>());
+
+  Log().Init(anbox::Logger::Severity::kWarning);
+
+  const auto log_level = utils::get_env_value("ANBOX_LOG_LEVEL", "");
+  if (!log_level.empty() || !Log().SetSeverityFromString(log_level))
+    WARNING("Failed to set logging severity to '%s'", log_level);
 }
 
 int Daemon::Run(const std::vector<std::string> &arguments) try {

src/anbox/graphics/emugl/RenderApi.cpp

@@ -23,7 +23,6 @@
 #include "OpenGLESDispatch/GLESv2Dispatch.h"
 
 #include "emugl/common/crash_reporter.h"
-#include "emugl/common/logging.h"
 
 #include <string.h>
 
@@ -50,7 +49,7 @@ std::vector<GLLibrary> default_gl_libraries(bool no_glesv1) {
   };
 }
 
-bool initialize(const std::vector<GLLibrary> &libs, emugl_logger_struct log_funcs, emugl_crash_func_t crash_func) {
+bool initialize(const std::vector<GLLibrary> &libs, emugl_logger_struct log_funcs, logger_t crash_func) {
   set_emugl_crash_reporter(crash_func);
   set_emugl_logger(log_funcs.coarse);
   set_emugl_cxt_logger(log_funcs.fine);

src/anbox/graphics/emugl/RenderApi.h

@@ -21,12 +21,11 @@
 
 #include <boost/filesystem/path.hpp>
 
-typedef void (*emugl_logger_func_t)(const char* fmt, ...);
+#include "emugl/common/logging.h"
-typedef void (*emugl_crash_func_t)(const char* format, ...);
 
 typedef struct {
-  emugl_logger_func_t coarse;
+  logger_t coarse;
-  emugl_logger_func_t fine;
+  logger_t fine;
 } emugl_logger_struct;
 
 namespace anbox {
@@ -40,7 +39,7 @@ struct GLLibrary {
 
 std::vector<GLLibrary> default_gl_libraries(bool no_glesv1 = false);
 
-bool initialize(const std::vector<GLLibrary> &libs, emugl_logger_struct log_funcs, emugl_crash_func_t crash_func);
+bool initialize(const std::vector<GLLibrary> &libs, emugl_logger_struct log_funcs, logger_t crash_func);
 }  // namespace emugl
 }  // namespace graphics
 }  // namespace anbox

src/anbox/graphics/gl_renderer_server.cpp

@@ -29,7 +29,9 @@
 #include <stdexcept>
 
 namespace {
-void logger_write(const char *format, ...) {
+void logger_write(const emugl::LogLevel &level, const char *format, ...) {
+  (void)level;
+
   char message[2048];
   va_list args;
 
@@ -37,7 +39,25 @@ void logger_write(const char *format, ...) {
   vsnprintf(message, sizeof(message) - 1, format, args);
   va_end(args);
 
-  DEBUG("%s", message);
+  switch (level) {
+  case emugl::LogLevel::WARNING:
+    WARNING("%s", message);
+    break;
+  case emugl::LogLevel::ERROR:
+    ERROR("%s", message);
+    break;
+  case emugl::LogLevel::FATAL:
+    FATAL("%s", message);
+    break;
+  case emugl::LogLevel::DEBUG:
+    DEBUG("%s", message);
+    break;
+  case emugl::LogLevel::TRACE:
+    TRACE("%s", message);
+    break;
+  default:
+    break;
+  }
 }
 }
 

src/anbox/logger.cpp

@@ -30,8 +30,7 @@
 
 namespace {
 namespace attrs {
-BOOST_LOG_ATTRIBUTE_KEYWORD(Severity, "anbox::Severity",
+BOOST_LOG_ATTRIBUTE_KEYWORD(Severity, "anbox::Severity", anbox::Logger::Severity)
-                            anbox::Logger::Severity)
 BOOST_LOG_ATTRIBUTE_KEYWORD(Location, "Location", anbox::Logger::Location)
 BOOST_LOG_ATTRIBUTE_KEYWORD(Timestamp, "Timestamp", boost::posix_time::ptime)
 }
@@ -39,8 +38,7 @@ BOOST_LOG_ATTRIBUTE_KEYWORD(Timestamp, "Timestamp", boost::posix_time::ptime)
 struct BoostLogLogger : public anbox::Logger {
   BoostLogLogger() : initialized_(false) {}
 
-  void Init(const anbox::Logger::Severity& severity =
+  void Init(const anbox::Logger::Severity& severity = anbox::Logger::Severity::kWarning) override {
-                anbox::Logger::Severity::kWarning) override {
     if (initialized_) return;
 
     boost::log::formatter formatter =
@@ -58,25 +56,28 @@ struct BoostLogLogger : public anbox::Logger {
     auto logger = boost::log::add_console_log(std::cout);
     logger->set_formatter(formatter);
 
-    // FIXME need to enable this once we found how we wrap this
+    severity_ = severity;
-    // properly into our service architecture. For now left as
-    // it is.
-    boost::ignore_unused_variable_warning(severity);
-    // logger->set_filter(attrs::Severity < severity);
-
     initialized_ = true;
   }
 
-  void Log(Severity severity, const std::string& message,
+  void SetSeverity(const Severity& severity) override {
-           const boost::optional<Location>& loc) {
+    severity_ = severity;
+  }
+
+  void Log(Severity severity, const std::string& message, const boost::optional<Location>& loc) override {
     if (!initialized_) Init();
 
+    // FIXME somehow set_filter doesn't work with the trivial logger. If
+    // we set a filter based on the severity attribute open_record will
+    // not return a new record. Because of that we do a poor man filtering
+    // here until we have a proper way to do this via boost.
+    if (severity < severity_)
+      return;
+
     if (auto rec = boost::log::trivial::logger::get().open_record()) {
       boost::log::record_ostream out{rec};
       out << boost::log::add_value(attrs::Severity, severity)
-          << boost::log::add_value(
+          << boost::log::add_value(attrs::Timestamp, boost::posix_time::microsec_clock::universal_time())
-                 attrs::Timestamp,
-                 boost::posix_time::microsec_clock::universal_time())
           << message;
 
       if (loc) {
@@ -91,6 +92,7 @@ struct BoostLogLogger : public anbox::Logger {
   }
 
  private:
+  Severity severity_;
   bool initialized_;
 };
 
@@ -105,6 +107,24 @@ void SetInstance(const std::shared_ptr<anbox::Logger>& logger) {
 }
 namespace anbox {
 
+bool Logger::SetSeverityFromString(const std::string& severity) {
+  if (severity == "trace")
+    SetSeverity(Severity::kTrace);
+  else if (severity == "debug")
+    SetSeverity(Severity::kDebug);
+  else if (severity == "info")
+    SetSeverity(Severity::kInfo);
+  else if (severity == "warning")
+    SetSeverity(Severity::kWarning);
+  else if (severity == "error")
+    SetSeverity(Severity::kError);
+  else if (severity == "fatal")
+    SetSeverity(Severity::kFatal);
+  else
+    return false;
+  return true;
+}
+
 void Logger::Trace(const std::string& message,
                    const boost::optional<Location>& location) {
   Log(Severity::kTrace, message, location);

src/anbox/logger.h

@@ -49,6 +49,9 @@ class Logger : public DoNotCopyOrMove {
 
   virtual void Init(const Severity& severity = Severity::kWarning) = 0;
 
+  bool SetSeverityFromString(const std::string &severity);
+  virtual void SetSeverity(const Severity& severity) = 0;
+
   virtual void Log(Severity severity, const std::string& message,
                    const boost::optional<Location>& location) = 0;