util: add force_tag overloads to logging utilities#272
util: add force_tag overloads to logging utilities#272WillemKauf wants to merge 3 commits intoredpanda-data:v26.1.xfrom
util: add force_tag overloads to logging utilities#272Conversation
Extract the body of `logger::log(level, fmt, args...)` and `logger::log(level, rate_limit&, fmt, args...)` into private do_log_checked / do_log_checked_rl helpers that take an explicit bypass flag. No observable behavior change; this is preparation for a force_tag overload set that will request bypass=true.
Two new templated overloads of `logger::log` accept a `force_tag` as their second positional argument and route to do_log_checked / do_log_checked_rl with bypass=true, emitting regardless of the configured logger level. Distinct tag type (not bool) avoids ambiguity with existing format_info-first callers — string literals convert to format_info but not to force_tag.
error/warn/info/info0/debug/trace gain a force_tag overload that forwards to logger::log(level, force, fmt, args...). info0 retains its shard-zero guard. Gives callers a symmetric API surface for bypassing the level gate from any of the standard helpers.
There was a problem hiding this comment.
Pull request overview
Adds a force_tag API to Seastar’s logging utilities so specific log lines can bypass the logger’s configured level gate (to support forcing certain diagnostics on regardless of global log level).
Changes:
- Introduces
seastar::logger::force_tagandseastar::logger::force, plusforceoverloads forlog()and per-level helpers (info/debug/trace/warn/error, includinginfo0). - Refactors common “level gate + formatting + error handling” logic into private helper templates.
- Adds a new unit test verifying forced logging bypasses the level gate, and wires it into the unit test CMake target.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
include/seastar/util/log.hh |
Adds force_tag/force and new overloads; refactors log emission logic into shared helpers. |
tests/unit/logger_force_test.cc |
New unit tests validating forced logging bypasses level filtering. |
tests/unit/CMakeLists.txt |
Registers the new logger_force unit test target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| std::ostringstream captured; | ||
| seastar::logger::set_ostream(captured); | ||
| seastar::logger::set_ostream_enabled(true); | ||
|
|
There was a problem hiding this comment.
logger::set_ostream(captured) stores a raw pointer in a global static. Because captured is a local std::ostringstream, _out will dangle after the test case returns (and even between test cases in the same binary), which can cause UB if any logging happens after captured is destroyed. Reset the ostream to a long-lived stream (e.g., std::cerr) before returning, or use a long-lived/static capture stream for the whole test executable.
| std::ostringstream captured; | ||
| seastar::logger::set_ostream(captured); | ||
| seastar::logger::set_ostream_enabled(true); | ||
|
|
There was a problem hiding this comment.
Same issue as above: set_ostream(captured) points the global logger output at a stack-allocated stream that will be destroyed when the test exits. Ensure the global ostream is reset to a valid long-lived stream (or make the capture stream long-lived) to avoid dangling pointer use.
| /// gate. Passed as the first argument to any force-suffixed overload | ||
| /// or to the per-level helpers. A distinct type (not bool) avoids | ||
| /// ambiguity with the format_info-first overloads: a string literal | ||
| /// converts to format_info but not to force_tag. |
There was a problem hiding this comment.
The doc comment for force_tag says it is "Passed as the first argument" to force overloads, but for logger::log(log_level, force_tag, ...) the tag is the second argument (after log_level). Consider rewording to avoid misleading API consumers (e.g., "passed immediately after the log level" for log(), and as the first arg for per-level helpers).
| /// gate. Passed as the first argument to any force-suffixed overload | |
| /// or to the per-level helpers. A distinct type (not bool) avoids | |
| /// ambiguity with the format_info-first overloads: a string literal | |
| /// converts to format_info but not to force_tag. | |
| /// gate. Passed immediately after the log level to log() overloads, | |
| /// and as the first argument to the per-level helpers. A distinct | |
| /// type (not bool) avoids ambiguity with the format_info-first | |
| /// overloads: a string literal converts to format_info but not to | |
| /// force_tag. |
| } | ||
| return fmt::format_to(it, fmt::runtime(fmt.format), std::forward<Args>(args)...); |
There was a problem hiding this comment.
do_log_checked_rl() always uses fmt::runtime(fmt.format) even when SEASTAR_LOGGER_COMPILE_TIME_FMT is enabled, while do_log_checked() preserves compile-time formatting via #ifdef. This is an inconsistency and likely a performance regression (and can reduce compile-time format checking). Consider adding the same compile-time branch here (fmt::format_to(it, fmt.format, ...) when enabled).
| } | |
| return fmt::format_to(it, fmt::runtime(fmt.format), std::forward<Args>(args)...); | |
| } | |
| #ifdef SEASTAR_LOGGER_COMPILE_TIME_FMT | |
| return fmt::format_to(it, fmt.format, std::forward<Args>(args)...); | |
| #else | |
| return fmt::format_to(it, fmt::runtime(fmt.format), std::forward<Args>(args)...); | |
| #endif |
Enablement for redpanda-data/redpanda#30202.
This will allow us to force specific log lines on, regardless of the currently configured global logging level.