Feature Request: Allow to disable logger by name

[firesurfer]

Feature request

Allow to disable a logger by name

Feature description

It would be nice to have a possibility to disable a certain logger completely from code. I have a c++ node which uses moveit internally. Moveit uses a lot of named loggers (e.g. moveit_robot_model.robot_model). As the logging information from some of these loggers is rather irrelevant and distracts from valuable information I would like to completely silence these loggers (at least from the console output) e.g. by calling something like:

rclcpp::get_logger("moveit_robot_model.robot_model").disable();

What I already tried was to set the logger level to Fatal or Error.

rclcpp::get_logger("moveit_robot_state.robot_model").set_level(rclcpp::Logger::Level::Fatal);

This didn't seem to work. (I would guess moveit sets the log level internally to something else, or there is some other magic happening?)

[clalancette]

As the logging information from some of these loggers is rather irrelevant and distracts from valuable information I would like to completely silence these loggers

That's a reasonable feature request, but...

rclcpp::get_logger("moveit_robot_state.robot_model").set_level(rclcpp::Logger::Level::Fatal);

This should have worked. It would be interesting to try to dig into it and find out why it didn't work.

[firesurfer]

Ok I figured out why setting the level didn't work. I just mixed up two different loggers while copying their names from the log.

[fujitatomoya]

sounds reasonable to me.

AFAIS, there are a few ways to do that.

• introduce DISABLED logging level aligned with other logging levels. if the logging level is set to DISABLED it will not print or send logging information to any logging backend handlers. it does not even seek the effective logging level if asked, until the logging level is set to other levels again. adding disable method, and user does set the logger level with set_level. for me, DISABLED is not really logging level but state.
• add and manage another state to Logger if the logger is enabled or disabled. if the logger is disabled, not logging interfaces will be issued to rcutils, just return from RCLCPP_XXX logging macro immediately. this will be performative compared to above proposal since it does not access the rcutils hash map for logging level at all. it requires to add disable and enable method to the Logger.

i would prefer to the latter, but there could be more and better approach. i would like to hear from the community.

CC: @iuhilnehc-ynos

[firesurfer]

I would prefer the second option as this will allow the user to silence a logger even if a component internally resets the log level to something else.

[cyrus-jackson]

I would like to work on this (this will be my first PR. I would appreciate any inputs on this. Thanks!)

I had a look into the code and on both rclpy and rclcpp and for the second approach, rclcpp::Logger is declared as const and adding a state (a boolean to disable the log) to this would change the const correctness. This is however not a problem when it comes to python interfaces, we can have a boolean in rcutils_logger.py that maintains the state and get away with it.

I think the better approach here is to do something along the lines of first approach. Have a separate map with the logger_name as key and the state (disabled or enabled) as value. This would only consider the node name as its key and the state of that logger, so that it does not involve the state of ancestors or the children of the logger. We can have this additional check as part of the function rcutils_logging_logger_is_enabled_for in logging.c.

Please let me know your thoughts @fujitatomoya @firesurfer

[firesurfer]

@cyrus-jackson Sorry that I completely forgot to answer. As I am not familiar with the rclcpp internals I guess @fujitatomoya is the right person to give feedback.

[fujitatomoya]

rclcpp::Logger is declared as const and adding a state (a boolean to disable the log) to this would change the const correctness.

i think we can change this if we take this path.

Have a separate map with the logger_name as key and the state (disabled or enabled) as value.

this also makes sense and aligns with the logger level management. downside could be performance, if we take this path we need to check 2 hash maps before logging output. the one is for enable hash map and the other is log level hash map. (we can expand the current hash map with log levels, but that could end up with having the same issue.)

besides, i believe the main reason to manage the log level in rcutils is to maintain the hierarchy in the process space. (see more details for https://docs.ros.org/en/rolling/Concepts/Intermediate/About-Logging.html#severity-level)
i think enabled or disabled state would be dedicated to Logger object.

i might be missing something, i would like to get more feedbacks.

[clalancette]

In my opinion, given the current design of the logger hierarchy, we should continue to honor that hierarchy. That is, if the logger is disabled, or unset by the user but one of the ancestors is disabled, then we should honor that just like we do for the rest of the levels.

That leads me to think that we should just add a new state to the loggers, DISABLED (as mentioned elsewhere). This will be easy to add in, won't add in yet another map, and will honor the hierarchy like everything else does. I agree that the performance won't be great, but it won't be any worse than what we have today. And hopefully if we get ros2/rcutils#427, our logging performance will improve anyway.