Add command-line logging configuration#4928
Open
RAOF wants to merge 10 commits into
Open
Conversation
26bc659 to
6de4b1f
Compare
3d14247 to
2667f7b
Compare
6de4b1f to
ea33b1e
Compare
84da01b to
8c4d98f
Compare
ea33b1e to
31a56f5
Compare
…more API This lets us drop the `logging_internal.h` header and instead fully drive the logging configuration from the main `mir::options` system, avoiding a dependency on boost::program_option in mircore (and, later, avoiding a dependency on libmiral in mircore)
8c4d98f to
05ee4f8
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces tag-based log filtering that can be configured from the command line via a new --log-level option, integrating with Mir’s existing logging/tagging system and exposing the new functionality through the mircore ABI.
Changes:
- Add a new
mir::logging::Tagsubsystem with severity thresholds, tag enumeration, and severity parsing. - Wire
--log-level=tag=severityinto the default server configuration so tags can be filtered at startup. - Update unit/acceptance tests and documentation to cover the new command-line behavior.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit-tests/logging/test_logging.cpp | Updates logging unit tests to account for tag severity filtering. |
| tests/unit-tests/CMakeLists.txt | Adjusts unit-test include paths. |
| tests/acceptance-tests/test_command_line_handling.cpp | Adds acceptance coverage for --log-level parsing and behavior. |
| src/platform/options/default_configuration.cpp | Implements the --log-level option and applies tag filters during option parsing. |
| src/platform/options/CMakeLists.txt | Updates include directories for the options object library. |
| src/core/symbols.map | Exports newly added logging/tag-related symbols (and formatter) in the mircore ABI map. |
| src/core/logging/tag.cpp | Introduces the new tag registry, severity parsing, and tag filtering logic. |
| src/core/logging/logger.cpp | Applies tag severity filtering when logging via tag lists. |
| src/core/CMakeLists.txt | Adds the new tag implementation and header to the mircore build. |
| include/core/mir/logging/tag.h | Adds the public tag/severity/filtering API and formatter declaration. |
| include/core/mir/logging/logger.h | Refactors to include tag definitions from the new header. |
| doc/sphinx/configuring/reference/options.md.include | Documents the new log-level option and tag list. |
| debian/libmircore2.symbols | Updates Debian symbols to reflect newly exported ABI. |
Comments suppressed due to low confidence (1)
src/core/logging/tag.cpp:144
lookup_tag()matches tags only bytag.name(leaf name), butlist_known_tags()returns full hierarchical paths (e.g.core/input). As a result, tags shown to users (and potentially passed from configuration/CLI) won't be found.
Consider storing/looking up tags by their full path (or accepting both full path and leaf name) to keep list_known_tags() and set_severity() consistent and avoid name collisions.
auto lookup_tag(decltype(known_tags)::Locked const& tag_list, std::string_view tag_name) -> ml::Tag&
{
for (auto& tag : *tag_list)
{
if (tag.name == tag_name)
{
return tag;
}
}
BOOST_THROW_EXCEPTION((std::out_of_range{std::format("Looking up nonexistent tag: {}", tag_name)}));
}
Comment on lines
+136
to
+144
| auto split_assignment(std::string_view value) | ||
| -> std::pair<std::string_view, std::string_view> | ||
| { | ||
| auto equals_pos = value.find('='); | ||
| if (equals_pos == value.length() || equals_pos == value.npos) | ||
| { | ||
| BOOST_THROW_EXCEPTION((std::runtime_error{ | ||
| std::format("Failed to parse log-level: {}", value)})); | ||
| } |
Comment on lines
+255
to
+276
| std::string tags; | ||
| std::ranges::copy( | ||
| mir::logging::list_known_tags() | std::views::transform([](std::string const& tag) { return " - " + tag; }) | std::views::join_with('\n'), | ||
| std::back_inserter(tags) | ||
| ); | ||
|
|
||
| add_options() | ||
| ("log-level", | ||
| po::value<std::vector<std::string>>()->notifier(update_tag_filtering), | ||
| ("Minimum severity of a log message required for it to be printed.\n" | ||
| "Valid severities are: critical, error, warning, informational, debug (“warn” and “info” can be used as short forms of “warning” and “informational”)\n" | ||
| "Must be specified in the form “tag=severity”\n" | ||
| "\n" | ||
| "Tags are heirarchical. Setting a tag's severity implicitly sets the severity of all its children.\n" | ||
| "The root of the tag hierarchy is “core”, so --log-level=core=debug will enable all tags at debug level.\n" | ||
| "\n" | ||
| "This option can be specified multiple times, and filters are applied in the order they are encountered.\n" | ||
| "For example “--log-level core=warning --log-level graphics=debug” will enable all tags at the warning level\n" | ||
| "except for graphics (and its children), which will be enabled at the debug level.\n" | ||
| "\n" | ||
| "Possible tags to filter on are:\n" | ||
| + tags).c_str()); |
| "Valid severities are: critical, error, warning, informational, debug (“warn” and “info” can be used as short forms of “warning” and “informational”)\n" | ||
| "Must be specified in the form “tag=severity”\n" | ||
| "\n" | ||
| "Tags are heirarchical. Setting a tag's severity implicitly sets the severity of all its children.\n" |
Comment on lines
+196
to
+206
| try | ||
| { | ||
| auto locked_tags = known_tags.lock(); | ||
| auto& tag = lookup_tag(locked_tags, name); | ||
| tag.logging_severity = sev; | ||
| for_each_child(tag, locked_tags, [sev](ml::Tag& tag) { tag.logging_severity = sev; }); | ||
| } | ||
| catch(std::exception const& err) | ||
| { | ||
| mir::log(ml::Severity::critical, {ml::core()}, "Failed to set tag severity to {}: {}", sev, err.what()); | ||
| } |
Comment on lines
+54
to
+60
| * | ||
| * Global initialisation is unsequeneced between different translation units, | ||
| * so we don't want to expose these globals directly, but by exposing the | ||
| * functions that statically initialise these tags we can ensure that any static | ||
| * initialisation dependencies are correctly ordered, and these hidden globals | ||
| * ensure that the tags are registered by the time any function from this | ||
| * translation unit is called. |
Comment on lines
17
to
29
| #include <mir/logging/logger.h> | ||
|
|
||
| #include <mir/synchronised.h> | ||
| #include <mir/logging/dumb_console_logger.h> | ||
| #include <mir/logging/logger.h> | ||
| #include <boost/throw_exception.hpp> | ||
| #include <concepts> | ||
| #include <mir/logging/logger.h> | ||
|
|
||
| #include <mir/logging/tag.h> | ||
| #include <mir/synchronised.h> | ||
| #include <mir/logging/dumb_console_logger.h> | ||
| #include <mir/fatal.h> |
Comment on lines
+18
to
+24
| #include "mir/options/program_option.h" | ||
| #include <boost/program_options/detail/cmdline.hpp> | ||
| #include <boost/program_options/options_description.hpp> | ||
| #include <mir/logging/logger.h> | ||
| #include <mir/log.h> | ||
|
|
||
| #include <boost/program_options.hpp> |
| Valid severities are: critical, error, warning, informational, debug (“warn” and “info” can be used as short forms of “warning” and “informational”) | ||
| Must be specified in the form “tag=severity” | ||
|
|
||
| Tags are heirarchical. Setting a tag's severity implicitly sets the severity of all its children. |
Comment on lines
+135
to
+143
| For example “--log-level core=warning --log-level graphics=debug” will enable all tags at the warning level | ||
| except for graphics (and its children), which will be enabled at the debug level. | ||
|
|
||
| Possible tags to filter on are: | ||
| - core | ||
| - core/input | ||
| - core/wayland | ||
| - core/graphics | ||
| - core/window-management |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist