Merge #200: event loop: add LogOptions struct and reduce the log size

8500340 eventloop: add `LogOptions` struct (ismaelsadeeq)

Pull request description:

  This PR fixes #190 by reducing the maximum size of characters to from 1000 to 200.

  This is achieved by creating a new struct `LogOptions`
  The `LogOptions` struct will contain
    i) The log callback function pointer
    ii) The maximum character to pass to `LogEscape`

  The main motivation is to reduce the verbosity of the logs and make
   LogEscape `max_size` configurable.

ACKs for top commit:
  ryanofsky:
    Code review ACK 8500340. Just whitespace cleanup since last review. Nice to have a shrinking PR
  Eunovo:
    ReACK 8500340

Tree-SHA512: 981d8e5957b482e4217d00fcf6f3244e52f491af593e8d16f4f91b7fcf0210645b466ef916205fa23527f6beebb18be2e2a5457f97b546bdf837bdcaa20ad5d2

Author: ryanofsky

include/mp/proxy-io.h

@@ -130,6 +130,16 @@ class Logger
     std::ostringstream m_buffer;
 };
 
+struct LogOptions {
+
+    //! External logging callback.
+    LogFn log_fn;
+
+    //! Maximum number of characters to use when representing
+    //! request and response structs as strings.
+    size_t max_chars{200};
+};
+
 std::string LongThreadName(const char* exe_name);
 
 //! Event loop implementation.
@@ -204,12 +214,12 @@ class EventLoop
 
     Logger log()
     {
-        Logger logger(false, m_log_fn);
+        Logger logger(false, m_log_opts.log_fn);
         logger << "{" << LongThreadName(m_exe_name) << "} ";
         return logger;
     }
-    Logger logPlain() { return {false, m_log_fn}; }
-    Logger raise() { return {true, m_log_fn}; }
+    Logger logPlain() { return {false, m_log_opts.log_fn}; }
+    Logger raise() { return {true, m_log_opts.log_fn}; }
 
     //! Process name included in thread names so combined debug output from
     //! multiple processes is easier to understand.
@@ -255,8 +265,8 @@ class EventLoop
     //! List of connections.
     std::list<Connection> m_incoming_connections;
 
-    //! External logging callback.
-    LogFn m_log_fn;
+    //! Logging options
+    LogOptions m_log_opts;
 
     //! External context pointer.
     void* m_context;

include/mp/proxy-types.h

@@ -631,13 +631,13 @@ void clientInvoke(ProxyClient& proxy_client, const GetRequest& get_request, Fiel
         IterateFields().handleChain(*invoke_context, request, FieldList(), typename FieldObjs::BuildParams{&fields}...);
         proxy_client.m_context.loop->logPlain()
             << "{" << thread_context.thread_name << "} IPC client send "
-            << TypeName<typename Request::Params>() << " " << LogEscape(request.toString());
+            << TypeName<typename Request::Params>() << " " << LogEscape(request.toString(), proxy_client.m_context.loop->m_log_opts.max_chars);
 
         proxy_client.m_context.loop->m_task_set->add(request.send().then(
             [&](::capnp::Response<typename Request::Results>&& response) {
                 proxy_client.m_context.loop->logPlain()
                     << "{" << thread_context.thread_name << "} IPC client recv "
-                    << TypeName<typename Request::Results>() << " " << LogEscape(response.toString());
+                    << TypeName<typename Request::Results>() << " " << LogEscape(response.toString(), proxy_client.m_context.loop->m_log_opts.max_chars);
                 try {
                     IterateFields().handleChain(
                         *invoke_context, response, FieldList(), typename FieldObjs::ReadResults{&fields}...);
@@ -701,7 +701,7 @@ kj::Promise<void> serverInvoke(Server& server, CallContext& call_context, Fn fn)
 
     int req = ++server_reqs;
     server.m_context.loop->log() << "IPC server recv request  #" << req << " "
-                                     << TypeName<typename Params::Reads>() << " " << LogEscape(params.toString());
+                                     << TypeName<typename Params::Reads>() << " " << LogEscape(params.toString(), server.m_context.loop->m_log_opts.max_chars);
 
     try {
         using ServerContext = ServerInvokeContext<Server, CallContext>;
@@ -718,7 +718,7 @@ kj::Promise<void> serverInvoke(Server& server, CallContext& call_context, Fn fn)
             [&]() { return kj::Promise<CallContext>(kj::mv(call_context)); })
             .then([&server, req](CallContext call_context) {
                 server.m_context.loop->log() << "IPC server send response #" << req << " " << TypeName<Results>()
-                                                 << " " << LogEscape(call_context.getResults().toString());
+                                                 << " " << LogEscape(call_context.getResults().toString(), server.m_context.loop->m_log_opts.max_chars);
             });
     } catch (const std::exception& e) {
         server.m_context.loop->log() << "IPC server unhandled exception: " << e.what();

include/mp/util.h

@@ -203,7 +203,7 @@ std::string ThreadName(const char* exe_name);
 
 //! Escape binary string for use in log so it doesn't trigger unicode decode
 //! errors in python unit tests.
-std::string LogEscape(const kj::StringTree& string);
+std::string LogEscape(const kj::StringTree& string, size_t max_size);
 
 //! Callback type used by SpawnProcess below.
 using FdToArgsFn = std::function<std::vector<std::string>(int fd)>;

src/mp/proxy.cpp

@@ -187,9 +187,9 @@ EventLoop::EventLoop(const char* exe_name, LogFn log_fn, void* context)
     : m_exe_name(exe_name),
       m_io_context(kj::setupAsyncIo()),
       m_task_set(new kj::TaskSet(m_error_handler)),
-      m_log_fn(std::move(log_fn)),
       m_context(context)
 {
+    m_log_opts.log_fn = log_fn;
     int fds[2];
     KJ_SYSCALL(socketpair(AF_UNIX, SOCK_STREAM, 0, fds));
     m_wait_fd = fds[0];

src/mp/util.cpp

@@ -76,12 +76,11 @@ std::string ThreadName(const char* exe_name)
     return std::move(buffer).str();
 }
 
-std::string LogEscape(const kj::StringTree& string)
+std::string LogEscape(const kj::StringTree& string, size_t max_size)
 {
-    const int MAX_SIZE = 1000;
     std::string result;
     string.visit([&](const kj::ArrayPtr<const char>& piece) {
-        if (result.size() > MAX_SIZE) return;
+        if (result.size() > max_size) return;
         for (const char c : piece) {
             if (c == '\\') {
                 result.append("\\\\");
@@ -92,7 +91,7 @@ std::string LogEscape(const kj::StringTree& string)
             } else {
                 result.push_back(c);
             }
-            if (result.size() > MAX_SIZE) {
+            if (result.size() > max_size) {
                 result += "...";
                 break;
             }