From 3e7a5225a5bf17b20ce78c3b654445ef55018b34 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 21 Oct 2025 11:24:00 +0200 Subject: [PATCH 1/5] Initial impl --- include/boost/redis/connection.hpp | 5 +- include/boost/redis/detail/connect_fsm.hpp | 6 +- .../boost/redis/detail/connection_logger.hpp | 53 ----- .../boost/redis/detail/connection_state.hpp | 4 +- include/boost/redis/detail/redis_stream.hpp | 4 +- include/boost/redis/detail/run_fsm.hpp | 5 +- include/boost/redis/detail/writer_fsm.hpp | 6 +- include/boost/redis/impl/connect_fsm.ipp | 69 +++++- .../boost/redis/impl/connection_logger.ipp | 196 ------------------ include/boost/redis/impl/log_utils.hpp | 72 +++++++ include/boost/redis/impl/reader_fsm.ipp | 21 +- include/boost/redis/impl/run_fsm.ipp | 20 +- include/boost/redis/impl/writer_fsm.ipp | 17 +- include/boost/redis/logger.hpp | 10 + include/boost/redis/src.hpp | 1 - 15 files changed, 195 insertions(+), 294 deletions(-) delete mode 100644 include/boost/redis/detail/connection_logger.hpp delete mode 100644 include/boost/redis/impl/connection_logger.ipp create mode 100644 include/boost/redis/impl/log_utils.hpp diff --git a/include/boost/redis/connection.hpp b/include/boost/redis/connection.hpp index 7ad01f29..ca973c5f 100644 --- a/include/boost/redis/connection.hpp +++ b/include/boost/redis/connection.hpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -138,7 +137,7 @@ struct connection_impl { , reconnect_timer_{ex} , ping_timer_{ex} , receive_channel_{ex, 256} - , st_{std::move(lgr)} + , st_{{std::move(lgr)}} { set_receive_adapter(any_adapter{ignore}); writer_cv_.expires_at((std::chrono::steady_clock::time_point::max)()); @@ -863,7 +862,7 @@ class basic_connection { // Used by both this class and connection void set_stderr_logger(logger::level lvl, const config& cfg) { - impl_->st_.logger.reset(detail::make_stderr_logger(lvl, cfg.log_prefix)); + impl_->st_.logger.lgr = detail::make_stderr_logger(lvl, cfg.log_prefix); } // Initiation for async_run. This is required because we need access diff --git a/include/boost/redis/detail/connect_fsm.hpp b/include/boost/redis/detail/connect_fsm.hpp index f1d52bbc..17627615 100644 --- a/include/boost/redis/detail/connect_fsm.hpp +++ b/include/boost/redis/detail/connect_fsm.hpp @@ -19,7 +19,7 @@ namespace boost::redis::detail { -class connection_logger; +struct buffered_logger; // What transport is redis_stream using? enum class transport_type @@ -63,10 +63,10 @@ struct connect_action { class connect_fsm { int resume_point_{0}; const config* cfg_{nullptr}; - connection_logger* lgr_{nullptr}; + buffered_logger* lgr_{nullptr}; public: - connect_fsm(const config& cfg, connection_logger& lgr) noexcept + connect_fsm(const config& cfg, buffered_logger& lgr) noexcept : cfg_(&cfg) , lgr_(&lgr) { } diff --git a/include/boost/redis/detail/connection_logger.hpp b/include/boost/redis/detail/connection_logger.hpp deleted file mode 100644 index 7dbb158c..00000000 --- a/include/boost/redis/detail/connection_logger.hpp +++ /dev/null @@ -1,53 +0,0 @@ -/* Copyright (c) 2018-2025 Marcelo Zimbres Silva (mzimbres@gmail.com) - * - * Distributed under the Boost Software License, Version 1.0. (See - * accompanying file LICENSE.txt) - */ - -#ifndef BOOST_REDIS_CONNECTION_LOGGER_HPP -#define BOOST_REDIS_CONNECTION_LOGGER_HPP - -#include - -#include -#include - -#include - -namespace boost::redis::detail { - -// Wraps a logger and a string buffer for re-use, and provides -// utility functions to format the log messages that we use. -// The long-term trend will be moving most of this class to finite state -// machines as we write them -class connection_logger { - logger logger_; - std::string msg_; - -public: - connection_logger(logger&& logger) noexcept - : logger_(std::move(logger)) - { } - - void reset(logger&& logger) { logger_ = std::move(logger); } - - void on_resolve(system::error_code const& ec, asio::ip::tcp::resolver::results_type const& res); - void on_connect(system::error_code const& ec, asio::ip::tcp::endpoint const& ep); - void on_connect(system::error_code const& ec, std::string_view unix_socket_ep); - void on_ssl_handshake(system::error_code const& ec); - void on_write(std::size_t n); - void on_read(system::error_code const& ec, std::size_t n); - void on_setup(system::error_code const& ec, std::string_view diagnostic); - void log(logger::level lvl, std::string_view msg); - void log(logger::level lvl, std::string_view msg1, std::string_view msg2); - void log(logger::level lvl, std::string_view op, system::error_code const& ec); - void trace(std::string_view message) { log(logger::level::debug, message); } - void trace(std::string_view op, system::error_code const& ec) - { - log(logger::level::debug, op, ec); - } -}; - -} // namespace boost::redis::detail - -#endif // BOOST_REDIS_LOGGER_HPP diff --git a/include/boost/redis/detail/connection_state.hpp b/include/boost/redis/detail/connection_state.hpp index addf9f3f..f908c5d1 100644 --- a/include/boost/redis/detail/connection_state.hpp +++ b/include/boost/redis/detail/connection_state.hpp @@ -10,8 +10,8 @@ #define BOOST_REDIS_CONNECTION_STATE_HPP #include -#include #include +#include #include #include @@ -22,7 +22,7 @@ namespace boost::redis::detail { // Contains all the members in connection that don't depend on the Executor. // Makes implementing sans-io algorithms easier struct connection_state { - connection_logger logger; + buffered_logger logger; config cfg{}; multiplexer mpx{}; std::string setup_diagnostic{}; diff --git a/include/boost/redis/detail/redis_stream.hpp b/include/boost/redis/detail/redis_stream.hpp index 9abf243a..ff5ab81b 100644 --- a/include/boost/redis/detail/redis_stream.hpp +++ b/include/boost/redis/detail/redis_stream.hpp @@ -9,8 +9,8 @@ #include #include -#include #include +#include #include #include @@ -170,7 +170,7 @@ class redis_stream { // I/O template - auto async_connect(const config& cfg, connection_logger& l, CompletionToken&& token) + auto async_connect(const config& cfg, buffered_logger& l, CompletionToken&& token) { return asio::async_compose( connect_op{*this, connect_fsm(cfg, l)}, diff --git a/include/boost/redis/detail/run_fsm.hpp b/include/boost/redis/detail/run_fsm.hpp index 77a11e24..c2a17d23 100644 --- a/include/boost/redis/detail/run_fsm.hpp +++ b/include/boost/redis/detail/run_fsm.hpp @@ -9,8 +9,6 @@ #ifndef BOOST_REDIS_RUN_FSM_HPP #define BOOST_REDIS_RUN_FSM_HPP -#include - #include #include @@ -19,8 +17,7 @@ namespace boost::redis::detail { // Forward decls -class connection_logger; -class multiplexer; +struct connection_state; // What should we do next? enum class run_action_type diff --git a/include/boost/redis/detail/writer_fsm.hpp b/include/boost/redis/detail/writer_fsm.hpp index 187e6743..be285834 100644 --- a/include/boost/redis/detail/writer_fsm.hpp +++ b/include/boost/redis/detail/writer_fsm.hpp @@ -9,9 +9,8 @@ #ifndef BOOST_REDIS_WRITER_FSM_HPP #define BOOST_REDIS_WRITER_FSM_HPP -#include - #include +#include #include #include @@ -22,8 +21,7 @@ namespace boost::redis::detail { // Forward decls -class connection_logger; -class multiplexer; +struct connection_state; // What should we do next? enum class writer_action_type diff --git a/include/boost/redis/impl/connect_fsm.ipp b/include/boost/redis/impl/connect_fsm.ipp index ff8cc34c..82724588 100644 --- a/include/boost/redis/impl/connect_fsm.ipp +++ b/include/boost/redis/impl/connect_fsm.ipp @@ -8,16 +8,59 @@ #include #include -#include #include #include +#include #include #include +#include #include +#include + namespace boost::redis::detail { +// Logging +inline void format_tcp_endpoint(const asio::ip::tcp::endpoint& ep, std::string& to) +{ + // This formatting is inspired by Asio's endpoint operator<< + const auto& addr = ep.address(); + if (addr.is_v6()) + to += '['; + to += addr.to_string(); + if (addr.is_v6()) + to += ']'; + to += ':'; + to += std::to_string(ep.port()); +} + +template <> +struct log_traits { + static inline void log(std::string& to, const asio::ip::tcp::endpoint& value) + { + format_tcp_endpoint(value, to); + } +}; + +template <> +struct log_traits { + static inline void log(std::string& to, const asio::ip::tcp::resolver::results_type& value) + { + auto iter = value.cbegin(); + auto end = value.cend(); + + if (iter != end) { + format_tcp_endpoint(iter->endpoint(), to); + ++iter; + for (; iter != end; ++iter) { + to += ", "; + format_tcp_endpoint(iter->endpoint(), to); + } + } + } +}; + inline transport_type transport_from_config(const config& cfg) { if (cfg.unix_socket.empty()) { @@ -61,7 +104,11 @@ connect_action connect_fsm::resume( ec = translate_timeout_error(ec, cancel_state, error::resolve_timeout); // Log it - lgr_->on_resolve(ec, resolver_results); + if (ec) { + log_info(*lgr_, "Error resolving the server hostname: ", ec); + } else { + log_info(*lgr_, "Resolve results: ", resolver_results); + } // Delegate to the regular resume function return resume(ec, st, cancel_state); @@ -77,7 +124,11 @@ connect_action connect_fsm::resume( ec = translate_timeout_error(ec, cancel_state, error::connect_timeout); // Log it - lgr_->on_connect(ec, selected_endpoint); + if (ec) { + log_info(*lgr_, "Failed to connect to the server: ", ec); + } else { + log_info(*lgr_, "Connected to ", selected_endpoint); + } // Delegate to the regular resume function return resume(ec, st, cancel_state); @@ -108,7 +159,11 @@ connect_action connect_fsm::resume( ec = translate_timeout_error(ec, cancel_state, error::connect_timeout); // Log it - lgr_->on_connect(ec, cfg_->unix_socket); + if (ec) { + log_info(*lgr_, "Failed to connect to the server: ", ec); + } else { + log_info(*lgr_, "Connected to ", cfg_->unix_socket); + } // If this failed, we can't continue if (ec) { @@ -156,7 +211,11 @@ connect_action connect_fsm::resume( ec = translate_timeout_error(ec, cancel_state, error::ssl_handshake_timeout); // Log it - lgr_->on_ssl_handshake(ec); + if (ec) { + log_info(*lgr_, "Failed to perform SSL handshake: ", ec); + } else { + log_info(*lgr_, "Successfully performed SSL handshake"); + } // If this failed, we can't continue if (ec) { diff --git a/include/boost/redis/impl/connection_logger.ipp b/include/boost/redis/impl/connection_logger.ipp deleted file mode 100644 index 3ff24e9b..00000000 --- a/include/boost/redis/impl/connection_logger.ipp +++ /dev/null @@ -1,196 +0,0 @@ -/* Copyright (c) 2018-2025 Marcelo Zimbres Silva (mzimbres@gmail.com) - * - * Distributed under the Boost Software License, Version 1.0. (See - * accompanying file LICENSE.txt) - */ - -#include -#include - -#include -#include - -#include -#include - -namespace boost::redis::detail { - -inline void format_tcp_endpoint(const asio::ip::tcp::endpoint& ep, std::string& to) -{ - // This formatting is inspired by Asio's endpoint operator<< - const auto& addr = ep.address(); - if (addr.is_v6()) - to += '['; - to += addr.to_string(); - if (addr.is_v6()) - to += ']'; - to += ':'; - to += std::to_string(ep.port()); -} - -inline void format_error_code(system::error_code ec, std::string& to) -{ - // Using error_code::what() includes any source code info - // that the error may contain, making the messages too long. - // This implementation was taken from error_code::what() - to += ec.message(); - to += " ["; - to += ec.to_string(); - to += ']'; -} - -void connection_logger::on_resolve( - system::error_code const& ec, - asio::ip::tcp::resolver::results_type const& res) -{ - if (logger_.lvl < logger::level::info) - return; - - if (ec) { - msg_ = "Error resolving the server hostname: "; - format_error_code(ec, msg_); - } else { - msg_ = "Resolve results: "; - auto iter = res.cbegin(); - auto end = res.cend(); - - if (iter != end) { - format_tcp_endpoint(iter->endpoint(), msg_); - ++iter; - for (; iter != end; ++iter) { - msg_ += ", "; - format_tcp_endpoint(iter->endpoint(), msg_); - } - } - } - - logger_.fn(logger::level::info, msg_); -} - -void connection_logger::on_connect(system::error_code const& ec, asio::ip::tcp::endpoint const& ep) -{ - if (logger_.lvl < logger::level::info) - return; - - if (ec) { - msg_ = "Failed to connect to the server: "; - format_error_code(ec, msg_); - } else { - msg_ = "Connected to "; - format_tcp_endpoint(ep, msg_); - } - - logger_.fn(logger::level::info, msg_); -} - -void connection_logger::on_connect(system::error_code const& ec, std::string_view unix_socket_ep) -{ - if (logger_.lvl < logger::level::info) - return; - - if (ec) { - msg_ = "Failed to connect to the server: "; - format_error_code(ec, msg_); - } else { - msg_ = "Connected to "; - msg_ += unix_socket_ep; - } - - logger_.fn(logger::level::info, msg_); -} - -void connection_logger::on_ssl_handshake(system::error_code const& ec) -{ - if (logger_.lvl < logger::level::info) - return; - - if (ec) { - msg_ = "Failed to perform SSL handshake: "; - format_error_code(ec, msg_); - } else { - msg_ = "Successfully performed SSL handshake"; - } - - logger_.fn(logger::level::info, msg_); -} - -void connection_logger::on_write(std::size_t n) -{ - if (logger_.lvl < logger::level::debug) - return; - - msg_ = "Writer task: "; - msg_ += std::to_string(n); - msg_ += " bytes written."; - - logger_.fn(logger::level::debug, msg_); -} - -void connection_logger::on_read(system::error_code const& ec, std::size_t bytes_read) -{ - if (logger_.lvl < logger::level::debug) - return; - - msg_ = "Reader task: "; - msg_ += std::to_string(bytes_read); - msg_ += " bytes read"; - if (ec) { - msg_ += ", error: "; - format_error_code(ec, msg_); - } - - logger_.fn(logger::level::debug, msg_); -} - -void connection_logger::on_setup(system::error_code const& ec, std::string_view diagnostic) -{ - if (logger_.lvl < logger::level::info) - return; - - msg_ = "Setup request execution: "; - if (ec) { - format_error_code(ec, msg_); - if (!diagnostic.empty()) { - msg_ += " ("; - msg_ += diagnostic; - msg_ += ')'; - } - } else { - msg_ += "success"; - } - - logger_.fn(logger::level::info, msg_); -} - -void connection_logger::log(logger::level lvl, std::string_view message) -{ - if (logger_.lvl < lvl) - return; - logger_.fn(lvl, message); -} - -void connection_logger::log(logger::level lvl, std::string_view message1, std::string_view message2) -{ - if (logger_.lvl < lvl) - return; - - msg_ = message1; - msg_ += ": "; - msg_ += message2; - - logger_.fn(lvl, msg_); -} - -void connection_logger::log(logger::level lvl, std::string_view op, system::error_code const& ec) -{ - if (logger_.lvl < lvl) - return; - - msg_ = op; - msg_ += ": "; - format_error_code(ec, msg_); - - logger_.fn(lvl, msg_); -} - -} // namespace boost::redis::detail diff --git a/include/boost/redis/impl/log_utils.hpp b/include/boost/redis/impl/log_utils.hpp new file mode 100644 index 00000000..638b29c4 --- /dev/null +++ b/include/boost/redis/impl/log_utils.hpp @@ -0,0 +1,72 @@ +/* Copyright (c) 2018-2025 Marcelo Zimbres Silva (mzimbres@gmail.com) + * + * Distributed under the Boost Software License, Version 1.0. (See + * accompanying file LICENSE.txt) + */ + +#ifndef BOOST_REDIS_LOG_UTILS_HPP +#define BOOST_REDIS_LOG_UTILS_HPP + +#include + +#include + +#include +#include +#include + +namespace boost::redis::detail { + +template +struct log_traits { + static inline void log(std::string& to, std::string_view value) { to += value; } +}; + +template <> +struct log_traits { + static inline void log(std::string& to, std::size_t value) { to += std::to_string(value); } +}; + +template <> +struct log_traits { + static inline void log(std::string& to, system::error_code value) + { + // Using error_code::what() includes any source code info + // that the error may contain, making the messages too long. + // This implementation was taken from error_code::what() + to += value.message(); + to += " ["; + to += value.to_string(); + to += ']'; + } +}; + +template +void log(buffered_logger& to, logger::level lvl, const Args&... args) +{ + if (to.lgr.lvl < lvl) + return; + + to.buffer.clear(); + + auto a = {(log_traits::log(to.buffer, args), 0)...}; + static_cast(a); + + to.lgr.fn(lvl, to.buffer); +} + +template +void log_debug(buffered_logger& to, const Args&... args) +{ + log(to, logger::level::debug, args...); +} + +template +void log_info(buffered_logger& to, const Args&... args) +{ + log(to, logger::level::info, args...); +} + +} // namespace boost::redis::detail + +#endif // BOOST_REDIS_LOGGER_HPP diff --git a/include/boost/redis/impl/reader_fsm.ipp b/include/boost/redis/impl/reader_fsm.ipp index 2ae05a6b..a47eb769 100644 --- a/include/boost/redis/impl/reader_fsm.ipp +++ b/include/boost/redis/impl/reader_fsm.ipp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -28,19 +29,19 @@ reader_fsm::action reader_fsm::resume( // Prepare the buffer for the read operation ec = st.mpx.prepare_read(); if (ec) { - st.logger.trace("Reader task: error in prepare_read", ec); + log_debug(st.logger, "Reader task: error in prepare_read", ec); return {ec}; } // Read. The connection might spend health_check_interval without writing data. // Give it another health_check_interval for the response to arrive. // If we don't get anything in this time, consider the connection as dead - st.logger.trace("Reader task: issuing read"); + log_debug(st.logger, "Reader task: issuing read"); BOOST_REDIS_YIELD(resume_point_, 1, action::read_some(2 * st.cfg.health_check_interval)) // Check for cancellations if (is_terminal_cancel(cancel_state)) { - st.logger.trace("Reader task: cancelled (1)"); + log_debug(st.logger, "Reader task: cancelled (1)"); return system::error_code(asio::error::operation_aborted); } @@ -52,7 +53,11 @@ reader_fsm::action reader_fsm::resume( } // Log what we read - st.logger.on_read(ec, bytes_read); + if (ec) { + log_debug(st.logger, "Reader task: ", bytes_read, " bytes read, error: ", ec); + } else { + log_debug(st.logger, "Reader task: ", bytes_read, " bytes read"); + } // Process the bytes read, even if there was an error st.mpx.commit_read(bytes_read); @@ -72,12 +77,12 @@ reader_fsm::action reader_fsm::resume( if (ec) { // TODO: Perhaps log what has not been consumed to aid // debugging. - st.logger.trace("Reader task: error processing message", ec); + log_debug(st.logger, "Reader task: error processing message", ec); return ec; } if (res_.first == consume_result::needs_more) { - st.logger.trace("Reader task: incomplete message received"); + log_debug(st.logger, "Reader task: incomplete message received"); break; } @@ -85,13 +90,13 @@ reader_fsm::action reader_fsm::resume( BOOST_REDIS_YIELD(resume_point_, 2, action::notify_push_receiver(res_.second)) // Check for cancellations if (is_terminal_cancel(cancel_state)) { - st.logger.trace("Reader task: cancelled (2)"); + log_debug(st.logger, "Reader task: cancelled (2)"); return system::error_code(asio::error::operation_aborted); } // Check for other errors if (ec) { - st.logger.trace("Reader task: error notifying push receiver", ec); + log_debug(st.logger, "Reader task: error notifying push receiver", ec); return ec; } } else { diff --git a/include/boost/redis/impl/run_fsm.ipp b/include/boost/redis/impl/run_fsm.ipp index bdc0dcd7..40114b91 100644 --- a/include/boost/redis/impl/run_fsm.ipp +++ b/include/boost/redis/impl/run_fsm.ipp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -67,7 +68,16 @@ inline any_adapter make_setup_adapter(connection_state& st) inline void on_setup_done(const multiplexer::elem& elm, connection_state& st) { - st.logger.on_setup(elm.get_error(), st.setup_diagnostic); + const auto ec = elm.get_error(); + if (ec) { + if (st.setup_diagnostic.empty()) { + log_info(st.logger, "Setup request execution: ", ec); + } else { + log_info(st.logger, "Setup request execution: ", ec, " (", st.setup_diagnostic, ")"); + } + } else { + log_info(st.logger, "Setup request execution: success"); + } } run_action run_fsm::resume( @@ -81,7 +91,7 @@ run_action run_fsm::resume( // Check config ec = check_config(st.cfg); if (ec) { - st.logger.log(logger::level::err, "Invalid configuration", ec); + log(st.logger, logger::level::err, "Invalid configuration", ec); stored_ec_ = ec; BOOST_REDIS_YIELD(resume_point_, 1, run_action_type::immediate) return stored_ec_; @@ -99,7 +109,7 @@ run_action run_fsm::resume( // Check for cancellations if (is_terminal_cancel(cancel_state)) { - st.logger.trace("Run: cancelled (1)"); + log_debug(st.logger, "Run: cancelled (1)"); return system::error_code(asio::error::operation_aborted); } @@ -139,7 +149,7 @@ run_action run_fsm::resume( // Check for cancellations if (is_terminal_cancel(cancel_state)) { - st.logger.trace("Run: cancelled (2)"); + log_debug(st.logger, "Run: cancelled (2)"); return system::error_code(asio::error::operation_aborted); } @@ -153,7 +163,7 @@ run_action run_fsm::resume( // Check for cancellations if (is_terminal_cancel(cancel_state)) { - st.logger.trace("Run: cancelled (3)"); + log_debug(st.logger, "Run: cancelled (3)"); return system::error_code(asio::error::operation_aborted); } } diff --git a/include/boost/redis/impl/writer_fsm.ipp b/include/boost/redis/impl/writer_fsm.ipp index 8e129dfd..cc2270fc 100644 --- a/include/boost/redis/impl/writer_fsm.ipp +++ b/include/boost/redis/impl/writer_fsm.ipp @@ -10,12 +10,13 @@ #define BOOST_REDIS_WRITER_FSM_IPP #include -#include #include #include #include #include #include +#include +#include #include #include @@ -27,7 +28,7 @@ namespace boost::redis::detail { inline void process_ping_node( - connection_logger& lgr, + buffered_logger& lgr, resp3::basic_node const& nd, system::error_code& ec) { @@ -38,11 +39,11 @@ inline void process_ping_node( } if (ec) { - lgr.log(logger::level::info, "Health checker: server answered ping with an error", nd.value); + log_info(lgr, "Health checker: server answered ping with an error", nd.value); } } -inline any_adapter make_ping_adapter(connection_logger& lgr) +inline any_adapter make_ping_adapter(buffered_logger& lgr) { return any_adapter{ [&lgr](any_adapter::parse_event evt, resp3::node_view const& nd, system::error_code& ec) { @@ -75,7 +76,7 @@ writer_action writer_fsm::resume( // Commit the received bytes. This accounts for partial success bool finished = st.mpx.commit_write(bytes_written); - st.logger.on_write(bytes_written); + log_debug(st.logger, "Writer task: ", bytes_written, " bytes written"); // Check for cancellations and translate error codes if (is_terminal_cancel(cancel_state)) @@ -86,9 +87,9 @@ writer_action writer_fsm::resume( // Check for errors if (ec) { if (ec == asio::error::operation_aborted) { - st.logger.trace("Writer task: cancelled (1)."); + log_debug(st.logger, "Writer task: cancelled (1)."); } else { - st.logger.trace("Writer task error", ec); + log_debug(st.logger, "Writer task error", ec); } return ec; } @@ -104,7 +105,7 @@ writer_action writer_fsm::resume( // Check for cancellations if (is_terminal_cancel(cancel_state)) { - st.logger.trace("Writer task: cancelled (2)."); + log_debug(st.logger, "Writer task: cancelled (2)."); return system::error_code(asio::error::operation_aborted); } diff --git a/include/boost/redis/logger.hpp b/include/boost/redis/logger.hpp index ce540fb5..08e9556c 100644 --- a/include/boost/redis/logger.hpp +++ b/include/boost/redis/logger.hpp @@ -8,6 +8,7 @@ #define BOOST_REDIS_LOGGER_HPP #include +#include #include namespace boost::redis { @@ -92,6 +93,15 @@ struct logger { std::function fn; }; +namespace detail { + +struct buffered_logger { + logger lgr; + std::string buffer{}; +}; + +} // namespace detail + } // namespace boost::redis #endif // BOOST_REDIS_LOGGER_HPP diff --git a/include/boost/redis/src.hpp b/include/boost/redis/src.hpp index a8a9dee2..c9dad234 100644 --- a/include/boost/redis/src.hpp +++ b/include/boost/redis/src.hpp @@ -5,7 +5,6 @@ */ #include -#include #include #include #include From 9cb04deaa39fc08c83d7effa4284bc6326d6b30a Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 21 Oct 2025 11:34:21 +0200 Subject: [PATCH 2/5] Test and format fixes --- include/boost/redis/impl/reader_fsm.ipp | 6 +++--- include/boost/redis/impl/run_fsm.ipp | 2 +- include/boost/redis/impl/writer_fsm.ipp | 6 +++--- test/test_connect_fsm.cpp | 5 ++--- test/test_reader_fsm.cpp | 2 +- test/test_run_fsm.cpp | 3 +-- test/test_writer_fsm.cpp | 4 +--- 7 files changed, 12 insertions(+), 16 deletions(-) diff --git a/include/boost/redis/impl/reader_fsm.ipp b/include/boost/redis/impl/reader_fsm.ipp index a47eb769..5c1539eb 100644 --- a/include/boost/redis/impl/reader_fsm.ipp +++ b/include/boost/redis/impl/reader_fsm.ipp @@ -29,7 +29,7 @@ reader_fsm::action reader_fsm::resume( // Prepare the buffer for the read operation ec = st.mpx.prepare_read(); if (ec) { - log_debug(st.logger, "Reader task: error in prepare_read", ec); + log_debug(st.logger, "Reader task: error in prepare_read: ", ec); return {ec}; } @@ -77,7 +77,7 @@ reader_fsm::action reader_fsm::resume( if (ec) { // TODO: Perhaps log what has not been consumed to aid // debugging. - log_debug(st.logger, "Reader task: error processing message", ec); + log_debug(st.logger, "Reader task: error processing message: ", ec); return ec; } @@ -96,7 +96,7 @@ reader_fsm::action reader_fsm::resume( // Check for other errors if (ec) { - log_debug(st.logger, "Reader task: error notifying push receiver", ec); + log_debug(st.logger, "Reader task: error notifying push receiver: ", ec); return ec; } } else { diff --git a/include/boost/redis/impl/run_fsm.ipp b/include/boost/redis/impl/run_fsm.ipp index 40114b91..a2667cd0 100644 --- a/include/boost/redis/impl/run_fsm.ipp +++ b/include/boost/redis/impl/run_fsm.ipp @@ -91,7 +91,7 @@ run_action run_fsm::resume( // Check config ec = check_config(st.cfg); if (ec) { - log(st.logger, logger::level::err, "Invalid configuration", ec); + log(st.logger, logger::level::err, "Invalid configuration: ", ec); stored_ec_ = ec; BOOST_REDIS_YIELD(resume_point_, 1, run_action_type::immediate) return stored_ec_; diff --git a/include/boost/redis/impl/writer_fsm.ipp b/include/boost/redis/impl/writer_fsm.ipp index cc2270fc..f4d3f779 100644 --- a/include/boost/redis/impl/writer_fsm.ipp +++ b/include/boost/redis/impl/writer_fsm.ipp @@ -39,7 +39,7 @@ inline void process_ping_node( } if (ec) { - log_info(lgr, "Health checker: server answered ping with an error", nd.value); + log_info(lgr, "Health checker: server answered ping with an error: ", nd.value); } } @@ -76,7 +76,7 @@ writer_action writer_fsm::resume( // Commit the received bytes. This accounts for partial success bool finished = st.mpx.commit_write(bytes_written); - log_debug(st.logger, "Writer task: ", bytes_written, " bytes written"); + log_debug(st.logger, "Writer task: ", bytes_written, " bytes written."); // Check for cancellations and translate error codes if (is_terminal_cancel(cancel_state)) @@ -89,7 +89,7 @@ writer_action writer_fsm::resume( if (ec == asio::error::operation_aborted) { log_debug(st.logger, "Writer task: cancelled (1)."); } else { - log_debug(st.logger, "Writer task error", ec); + log_debug(st.logger, "Writer task error: ", ec); } return ec; } diff --git a/test/test_connect_fsm.cpp b/test/test_connect_fsm.cpp index 641f67cf..d3fde091 100644 --- a/test/test_connect_fsm.cpp +++ b/test/test_connect_fsm.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include @@ -29,7 +28,7 @@ namespace asio = boost::asio; using detail::connect_fsm; using detail::connect_action_type; using detail::connect_action; -using detail::connection_logger; +using detail::buffered_logger; using detail::redis_stream_state; using detail::transport_type; using asio::ip::tcp; @@ -105,7 +104,7 @@ auto resolver_data = [] { // Reduce duplication struct fixture : detail::log_fixture { config cfg; - connection_logger lgr{make_logger()}; + buffered_logger lgr{make_logger()}; connect_fsm fsm{cfg, lgr}; redis_stream_state st{}; diff --git a/test/test_reader_fsm.cpp b/test/test_reader_fsm.cpp index f215a54d..75664aee 100644 --- a/test/test_reader_fsm.cpp +++ b/test/test_reader_fsm.cpp @@ -96,7 +96,7 @@ void copy_to(multiplexer& mpx, std::string_view data) } struct fixture : redis::detail::log_fixture { - connection_state st{make_logger()}; + connection_state st{{make_logger()}}; generic_response resp; fixture() diff --git a/test/test_run_fsm.cpp b/test/test_run_fsm.cpp index 117b4e78..b67dc929 100644 --- a/test/test_run_fsm.cpp +++ b/test/test_run_fsm.cpp @@ -31,7 +31,6 @@ using detail::run_action_type; using detail::run_action; using boost::system::error_code; using boost::asio::cancellation_type_t; -using detail::connection_logger; using namespace std::chrono_literals; // Operators @@ -86,7 +85,7 @@ struct fixture : detail::log_fixture { } fixture(config&& cfg = default_config()) - : st{make_logger(), std::move(cfg)} + : st{{make_logger()}, std::move(cfg)} { } }; diff --git a/test/test_writer_fsm.cpp b/test/test_writer_fsm.cpp index f206b2e4..da7f185d 100644 --- a/test/test_writer_fsm.cpp +++ b/test/test_writer_fsm.cpp @@ -6,7 +6,6 @@ // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) // -#include #include #include #include @@ -36,7 +35,6 @@ using detail::writer_action; using detail::connection_state; using boost::system::error_code; using boost::asio::cancellation_type_t; -using detail::connection_logger; using namespace std::chrono_literals; // Operators @@ -110,7 +108,7 @@ struct test_elem { }; struct fixture : detail::log_fixture { - connection_state st{make_logger()}; + connection_state st{{make_logger()}}; writer_fsm fsm; fixture() From 54768f1f32778f8318bcb417d74be5482c81bbc0 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 21 Oct 2025 11:35:13 +0200 Subject: [PATCH 3/5] log_err --- include/boost/redis/impl/log_utils.hpp | 6 ++++++ include/boost/redis/impl/run_fsm.ipp | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/include/boost/redis/impl/log_utils.hpp b/include/boost/redis/impl/log_utils.hpp index 638b29c4..76f1dc31 100644 --- a/include/boost/redis/impl/log_utils.hpp +++ b/include/boost/redis/impl/log_utils.hpp @@ -67,6 +67,12 @@ void log_info(buffered_logger& to, const Args&... args) log(to, logger::level::info, args...); } +template +void log_err(buffered_logger& to, const Args&... args) +{ + log(to, logger::level::err, args...); +} + } // namespace boost::redis::detail #endif // BOOST_REDIS_LOGGER_HPP diff --git a/include/boost/redis/impl/run_fsm.ipp b/include/boost/redis/impl/run_fsm.ipp index a2667cd0..463bc957 100644 --- a/include/boost/redis/impl/run_fsm.ipp +++ b/include/boost/redis/impl/run_fsm.ipp @@ -91,7 +91,7 @@ run_action run_fsm::resume( // Check config ec = check_config(st.cfg); if (ec) { - log(st.logger, logger::level::err, "Invalid configuration: ", ec); + log_err(st.logger, "Invalid configuration: ", ec); stored_ec_ = ec; BOOST_REDIS_YIELD(resume_point_, 1, run_action_type::immediate) return stored_ec_; From 87e1ff2509872a62c07269636251cf438983924a Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 21 Oct 2025 11:44:04 +0200 Subject: [PATCH 4/5] Comments and refactor --- include/boost/redis/impl/log_utils.hpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/include/boost/redis/impl/log_utils.hpp b/include/boost/redis/impl/log_utils.hpp index 76f1dc31..e554d59a 100644 --- a/include/boost/redis/impl/log_utils.hpp +++ b/include/boost/redis/impl/log_utils.hpp @@ -9,6 +9,7 @@ #include +#include #include #include @@ -17,11 +18,16 @@ namespace boost::redis::detail { +// Internal trait that defines how to log different types. +// The base template applies to types convertible to string_view template struct log_traits { + // log should convert the input value to string and append it to the supplied buffer static inline void log(std::string& to, std::string_view value) { to += value; } }; +// Formatting size_t and error codes is shared between almost all FSMs, so it's defined here. +// Support for types used only in one FSM should be added in the relevant FSM file. template <> struct log_traits { static inline void log(std::string& to, std::size_t value) { to += std::to_string(value); } @@ -41,20 +47,28 @@ struct log_traits { } }; +// Logs a message with the specified severity to the logger. +// Formatting won't be performed if the logger's level is inferior to lvl. +// args are stringized using log_traits, and concatenated. template void log(buffered_logger& to, logger::level lvl, const Args&... args) { + // Severity check if (to.lgr.lvl < lvl) return; + // Clear the buffer to.buffer.clear(); - auto a = {(log_traits::log(to.buffer, args), 0)...}; - static_cast(a); + // Format all arguments + auto dummy = {(log_traits::log(to.buffer, args), 0)...}; + ignore_unused(dummy); + // Invoke the function to.lgr.fn(lvl, to.buffer); } +// Shorthand for each log level we use template void log_debug(buffered_logger& to, const Args&... args) { From 8d825f0e0fdc28619a9364e5a7184a8c0be142b3 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 21 Oct 2025 11:48:43 +0200 Subject: [PATCH 5/5] Optimization --- include/boost/redis/impl/log_utils.hpp | 29 ++++++++++++++++---------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/include/boost/redis/impl/log_utils.hpp b/include/boost/redis/impl/log_utils.hpp index e554d59a..e4b7349c 100644 --- a/include/boost/redis/impl/log_utils.hpp +++ b/include/boost/redis/impl/log_utils.hpp @@ -15,6 +15,7 @@ #include #include #include +#include namespace boost::redis::detail { @@ -47,25 +48,31 @@ struct log_traits { } }; +template +void format_log_args(std::string& to, const Args&... args) +{ + auto dummy = {(log_traits::log(to, args), 0)...}; + ignore_unused(dummy); +} + // Logs a message with the specified severity to the logger. // Formatting won't be performed if the logger's level is inferior to lvl. // args are stringized using log_traits, and concatenated. -template -void log(buffered_logger& to, logger::level lvl, const Args&... args) +template +void log(buffered_logger& to, logger::level lvl, const Arg0& arg0, const Rest&... arg_rest) { // Severity check if (to.lgr.lvl < lvl) return; - // Clear the buffer - to.buffer.clear(); - - // Format all arguments - auto dummy = {(log_traits::log(to.buffer, args), 0)...}; - ignore_unused(dummy); - - // Invoke the function - to.lgr.fn(lvl, to.buffer); + // Optimization: if we get passed a single string, don't copy it to the buffer + if constexpr (sizeof...(Rest) == 0u && std::is_convertible_v) { + to.lgr.fn(lvl, arg0); + } else { + to.buffer.clear(); + format_log_args(to.buffer, arg0, arg_rest...); + to.lgr.fn(lvl, to.buffer); + } } // Shorthand for each log level we use