server: separate logger instance for connection/access logging

This introduces the ability to supply a separate logger instance for logging connection activities.
By default, a null logger is assigned for connection logging.

Fixes #77

examples/example.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <malloy/server/routing_context.hpp>
 #include <spdlog/logger.h>
 #include <spdlog/sinks/stdout_color_sinks.h>
 
@@ -32,3 +33,18 @@ create_example_logger()
 
     return logger;
 }
+
+/**
+ * Setup the malloy config with values suitable for examples.
+ *
+ * @param cfg The config.
+ */
+static
+void
+setup_example_config(malloy::server::routing_context::config& cfg)
+{
+    cfg.doc_root          = examples_doc_root;
+    cfg.num_threads       = 5;
+    cfg.logger            = create_example_logger();
+    cfg.connection_logger = cfg.logger;
+}

examples/server/basic/main.cpp

@@ -12,11 +12,9 @@ int main()
 {
     // Create malloy controller config
     malloy::server::routing_context::config cfg;
+    setup_example_config(cfg);
     cfg.interface   = "127.0.0.1";
     cfg.port        = 8080;
-    cfg.doc_root    = examples_doc_root;
-    cfg.num_threads = 5;
-    cfg.logger      = create_example_logger();
 
     // Create malloy controller
     malloy::server::routing_context c{cfg};

examples/server/custom_request_filter/main.cpp

@@ -1,3 +1,5 @@
+#include "../../example.hpp"
+
 #include <boost/beast/http/file_body.hpp>
 #include <malloy/core/http/request.hpp>
 #include <malloy/server/routing_context.hpp>
@@ -29,11 +31,10 @@ int
 main()
 {
 	malloy::server::routing_context::config cfg;
-	cfg.num_threads = 2;
+    setup_example_config(cfg);
 	cfg.interface = "127.0.0.1";
 	cfg.port = 8080;
 	cfg.doc_root = "/";
-    cfg.logger = spdlog::default_logger();
 
 	malloy::server::routing_context ctrl{cfg};
 

examples/server/html_form/main.cpp

@@ -14,11 +14,9 @@ int main()
 
     // Create malloy controller config
     server::routing_context::config cfg;
+    setup_example_config(cfg);
     cfg.interface   = "127.0.0.1";
     cfg.port        = 8080;
-    cfg.doc_root    = examples_doc_root;
-    cfg.num_threads = 1;
-    cfg.logger      = create_example_logger();
 
     // Create malloy controller
     server::routing_context c{cfg};

examples/server/routing/files/main.cpp

@@ -10,11 +10,9 @@ int main(int argc, char* argv[])
 {
     // Create malloy controller config
     malloy::server::routing_context::config cfg;
+    setup_example_config(cfg);
     cfg.interface   = "127.0.0.1";
     cfg.port        = 8080;
-    cfg.doc_root    = examples_doc_root;
-    cfg.num_threads = 5;
-    cfg.logger      = create_example_logger();
 
     // Create malloy controller
     malloy::server::routing_context c{cfg};

examples/server/routing/policies/main.cpp

@@ -62,11 +62,9 @@ int main()
 {
     // Create malloy controller config
     malloy::server::routing_context::config cfg;
+    setup_example_config(cfg);
     cfg.interface   = "127.0.0.1";
     cfg.port        = 8080;
-    cfg.doc_root    = examples_doc_root;
-    cfg.num_threads = 5;
-    cfg.logger      = create_example_logger();
 
     // Create malloy controller
     malloy::server::routing_context c{cfg};

examples/server/routing/preflights/main.cpp

@@ -9,11 +9,9 @@ int main()
 {
     // Create malloy controller config
     malloy::server::routing_context::config cfg;
+    setup_example_config(cfg);
     cfg.interface   = "127.0.0.1";
     cfg.port        = 8080;
-    cfg.doc_root    = examples_doc_root;
-    cfg.num_threads = 5;
-    cfg.logger      = create_example_logger();
 
     // Create malloy controller
     malloy::server::routing_context c{cfg};

examples/server/routing/regex/main.cpp

@@ -9,11 +9,9 @@ int main()
 {
     // Create malloy controller config
     malloy::server::routing_context::config cfg;
+    setup_example_config(cfg);
     cfg.interface   = "127.0.0.1";
     cfg.port        = 8080;
-    cfg.doc_root    = examples_doc_root;
-    cfg.num_threads = 5;
-    cfg.logger      = create_example_logger();
 
     // Create malloy controller
     malloy::server::routing_context c{cfg};

examples/server/routing/subrouters/main.cpp

@@ -10,11 +10,9 @@ int main(int argc, char* argv[])
 {
     // Create malloy controller config
     malloy::server::routing_context::config cfg;
+    setup_example_config(cfg);
     cfg.interface   = "127.0.0.1";
     cfg.port        = 8080;
-    cfg.doc_root    = examples_doc_root;
-    cfg.num_threads = 5;
-    cfg.logger      = create_example_logger();
 
     // Create malloy controller
     malloy::server::routing_context c{cfg};

examples/server/session/main.cpp

@@ -15,11 +15,9 @@ int main()
 
     // Create malloy controller config
     malloy::server::routing_context::config cfg;
+    setup_example_config(cfg);
     cfg.interface   = "127.0.0.1";
     cfg.port        = 8080;
-    cfg.doc_root    = examples_doc_root;
-    cfg.num_threads = 5;
-    cfg.logger      = create_example_logger();
 
     // Create malloy controller
     malloy::server::routing_context c{cfg};

examples/server/ssl/main.cpp

@@ -10,11 +10,9 @@ int main()
 {
     // Create malloy controller config
     malloy::server::routing_context::config cfg;
+    setup_example_config(cfg);
     cfg.interface   = "127.0.0.1";
     cfg.port        = 8080;
-    cfg.doc_root    = examples_doc_root;
-    cfg.num_threads = 5;
-    cfg.logger      = create_example_logger();
 
     // Create malloy controller
     malloy::server::routing_context c{cfg};

examples/server/websocket/main.cpp

@@ -10,11 +10,9 @@ int main()
 {
     // Create malloy controller config
     malloy::server::routing_context::config cfg;
+    setup_example_config(cfg);
     cfg.interface   = "127.0.0.1";
     cfg.port        = 8080;
-    cfg.doc_root    = examples_doc_root;
-    cfg.num_threads = 5;
-    cfg.logger      = create_example_logger();
 
     // Create malloy controller
     malloy::server::routing_context c{cfg};

lib/malloy/server/http/connection.hpp

@@ -279,15 +279,18 @@ namespace malloy::server::http
 
             // Check if this is a WS request
             if (boost::beast::websocket::is_upgrade(gen->header())) {
-                m_logger->debug("upgrading HTTP connection to WS connection.");
+                m_logger->info("upgrading HTTP connection to WS connection");
 
                 // Create a websocket connection, transferring ownership
                 // of both the socket and the HTTP request.
                 boost::beast::get_lowest_layer(derived().stream()).expires_never();
-
                 auto ws_connection = server::websocket::connection::make(
-                    m_logger->clone("websocket_connection"),
-                    malloy::websocket::stream{derived().release_stream()}, cfg.agent_string);
+                    m_logger,
+                    malloy::websocket::stream{derived().release_stream()},
+                    cfg.agent_string
+                );
+
+                // Hand over to router
                 m_router->websocket(*m_doc_root, gen, ws_connection);
             }
 
@@ -334,7 +337,7 @@ namespace malloy::server::http
             derived().do_close();
 
             // At this point the connection is closed gracefully
-            m_logger->trace("closed HTTP connection gracefully.");
+            m_logger->info("HTTP connection closed gracefully");
         }
     };
 

lib/malloy/server/http/connection_detector.cpp

@@ -17,6 +17,8 @@ using namespace malloy::server::http;
  *
  * @sa connection
  */
+// ToDo: Consider adding public interface(s) to the connection class to facilitate logging of requests and repsonses
+//       instead of passing the logger into here.
 template<typename Derived>
 class router_adaptor :
     public connection<Derived>::handler
@@ -27,32 +29,50 @@ class router_adaptor :
     using http_conn_t = const connection_t&;
     using req_t = std::shared_ptr<typename connection<Derived>::request_generator>;
 
-    router_adaptor(router_t router) :
+    /**
+     * Constructor.
+     *
+     * @param logger The logger instance. This should be the same instance passed to the connection constructor.
+     * @param router The router.
+     */
+    router_adaptor(std::shared_ptr<spdlog::logger> logger, router_t router) :
+        m_logger{ std::move(logger) },
         m_router{ std::move(router) }
     {
     }
 
     void
     websocket(const std::filesystem::path& root, const req_t& req, const std::shared_ptr<malloy::server::websocket::connection>& conn) override
     {
-        send_msg<true>(root, req, conn);
+        m_logger->info("WS request: {} {}",
+           boost::beast::http::to_string(req->header().method()),
+           req->header().target()
+       );
+
+        handle<true>(root, req, conn);
     }
 
     void
     http(const std::filesystem::path& root, const req_t& req, http_conn_t conn) override
     {
-        send_msg<false>(root, req, conn);
+        m_logger->info("HTTP request: {} {}",
+           boost::beast::http::to_string(req->header().method()),
+           req->header().target()
+       );
+
+        handle<false>(root, req, conn);
     }
 
 private:
-    template<bool isWs>
+    std::shared_ptr<spdlog::logger> m_logger;   // This should be the same instance as the logger used by the connection
+    router_t m_router;
+
+    template<bool isWebsocket>
     void
-    send_msg(const std::filesystem::path& root, const req_t& req, auto conn)
+    handle(const std::filesystem::path& root, const req_t& req, auto conn)
     {
-        m_router->handle_request<isWs, Derived>(root, req, conn);
+        m_router->handle_request<isWebsocket, Derived>(root, req, conn);
     }
-
-    router_t m_router;
 };
 
 connection_detector::connection_detector(
@@ -114,7 +134,7 @@ connection_detector::on_detect(boost::beast::error_code ec, [[maybe_unused]] boo
                     m_ctx,
                     std::move(m_buffer),
                     m_doc_root,
-                    std::make_shared<router_adaptor<connection_tls>>(m_router)
+                    std::make_shared<router_adaptor<connection_tls>>(m_logger, m_router)
                 )
             );
         }
@@ -126,7 +146,7 @@ connection_detector::on_detect(boost::beast::error_code ec, [[maybe_unused]] boo
                 m_stream.release_socket(),
                 std::move(m_buffer),
                 m_doc_root,
-                std::make_shared<router_adaptor<connection_plain>>(m_router)
+                std::make_shared<router_adaptor<connection_plain>>(m_logger, m_router)
             )
         );
     }([this](auto&& conn) {

lib/malloy/server/listener.cpp

@@ -9,6 +9,7 @@ using namespace malloy::server;
 
 listener::listener(
     std::shared_ptr<spdlog::logger> logger,
+    std::shared_ptr<spdlog::logger> connection_logger,
     boost::asio::io_context& ioc,
     std::shared_ptr<boost::asio::ssl::context> tls_ctx,
     const boost::asio::ip::tcp::endpoint& endpoint,
@@ -17,6 +18,7 @@ listener::listener(
     std::string agent_string
 ) :
     m_logger(std::move(logger)),
+    m_connection_logger(std::move(connection_logger)),
     m_io_ctx(ioc),
     m_tls_ctx(std::move(tls_ctx)),
     m_acceptor(boost::asio::make_strand(ioc)),
@@ -29,6 +31,8 @@ listener::listener(
     // Sanity check on logger
     if (!m_logger)
         throw std::runtime_error("did not receive a valid logger instance.");
+    if (!m_connection_logger)
+        throw std::runtime_error("did not receive a valid connection logger instance.");
 
     // Open the acceptor
     m_acceptor.open(endpoint.protocol(), ec);
@@ -104,17 +108,20 @@ listener::on_accept(boost::beast::error_code ec, boost::asio::ip::tcp::socket so
         return;
     }
 
-    // Log
-    m_logger->info(
-        "accepting incoming connection from {}:{}",
-        socket.remote_endpoint().address().to_string(),
-        socket.remote_endpoint().port()
+    // Setup connection logger
+    auto connection_logger = m_connection_logger->clone(
+        fmt::format(
+            "{}:{}",
+            socket.remote_endpoint().address().to_string(),
+            socket.remote_endpoint().port()
+        )
     );
+    connection_logger->info("accepting incoming connection");
 
     // Create the http detector connection
     // This will automatically spawn the correct type of connection (eg. plain or TLS).
     auto connection = std::make_shared<http::connection_detector>(
-        m_logger->clone("http_connection"),
+        connection_logger,
         std::move(socket),
         m_tls_ctx,
         m_doc_root,

lib/malloy/server/listener.hpp

@@ -37,6 +37,7 @@ namespace malloy::server
          * Constructor
          *
          * @param logger The logger instance to use.
+         * @param connection_logger The logger instance for connections.
          * @param ioc The I/O context to use.
          * @param tls_ctx The TLS context to use.
          * @param endpoint The enpoint to use.
@@ -46,6 +47,7 @@ namespace malloy::server
          */
         listener(
             std::shared_ptr<spdlog::logger> logger,
+            std::shared_ptr<spdlog::logger> connection_logger,
             boost::asio::io_context& ioc,
             std::shared_ptr<boost::asio::ssl::context> tls_ctx,
             const boost::asio::ip::tcp::endpoint& endpoint,
@@ -108,6 +110,7 @@ namespace malloy::server
 
     private:
         std::shared_ptr<spdlog::logger> m_logger;
+        std::shared_ptr<spdlog::logger> m_connection_logger;
         boost::asio::io_context& m_io_ctx;
         std::shared_ptr<boost::asio::ssl::context> m_tls_ctx;
         boost::asio::ip::tcp::acceptor m_acceptor;

lib/malloy/server/routing_context.cpp

@@ -6,6 +6,7 @@
 #endif
 
 #include <spdlog/logger.h>
+#include <spdlog/sinks/null_sink.h>
 
 #include <memory>
 
@@ -16,6 +17,10 @@ routing_context::routing_context(config cfg) :
     m_router{m_cfg.logger != nullptr ? m_cfg.logger->clone("router") : nullptr, m_cfg.agent_string}
 {
     m_cfg.validate();
+
+    // If no connection logger is provided, use the null logger.
+    if (!m_cfg.connection_logger)
+        m_cfg.connection_logger = spdlog::null_logger_mt("connection");
 }
 
 #if MALLOY_FEATURE_TLS