carma-platform icon indicating copy to clipboard operation
carma-platform copied to clipboard

Logging for library type of code in ROS2 is not configurable by carma-config

Open MishkaMN opened this issue 2 years ago • 6 comments

Types of Issue

  • [X] Anomaly report (something appears to not work correctly)
  • [ ] Enhancement request (describe the enhancement being requested)
  • [ ] Other (please ensure the description clarifies why the issue doesn’t fall into either of the above categories)

Descriptive summary

Library code such as carma_wm_ros2 or basic_autonomy_ros2 is not able to be configured by log level in carma config.

General working way of logging should be written in C++ in a form of: RCLCPP_DEBUG_STREAM(rclcpp::get_logger("package_name_without_any_namespace"), "Logs to print" ); such as RCLCPP_DEBUG_STREAM(rclcpp::get_logger("localization_manager"), "Logs to print" );

Various packages may not be following this style in develop branch, but they should be changed to above in order to work in every scenario.

However, only known scenario where this doesn't work (no known solution exists yet) is in the previously mentioned library codes. Such as below configuration doesn't print the "Logs to print": in carma config: log4j.logger.ros.carma_wm_ros2=DEBUG in C++ code in carma_wm_ros2: RCLCPP_DEBUG_STREAM(rclcpp::get_logger("carma_wm_ros2"), "Logs to print" );

The developer should research why it doesn't work, and come up with a solution.

Carma version where this issue was discovered

carma-system-4.2.0+ develop

Expected behavior

Log levels in carma_wm_ros2 and basic_autonomy_ros2 can be changed by carma-config.

Actual behavior

See above.

Steps to reproduce the actual behavior

See above.

Related work

MishkaMN avatar Oct 11 '22 15:10 MishkaMN

Currently the team is rebuilding the required logs in the library code as RCLCPP_ERROR_STREAM to force it to print

MishkaMN avatar Oct 11 '22 15:10 MishkaMN

There was another issue related to this where logger level set in carma-config was not setting the log levels for lines using node's logger, so the logger's name had to be hard-coded. This was because ROS2's logger names include namespace with dots for example: guidance.plugins.yield_plugin. However, there was a bug in carma_ros2_utils/carma_ros2_utils/launch/generate_log_levels.py where it is hardcoded to consider only first part in "dot separated string name", which would be guidance in this case.

MishkaMN avatar Sep 26 '23 06:09 MishkaMN

The reason why the library ones do not work is because in ROS2 Foxy, it is only possible to set the node's log level from launch python file using --ros-args --log-level commands. carma_ros2_utils saves the logger levels from the .conf file and uses that mapping when creating nodes from launch file. However, since there is no nodes are created with names carma_wm or basic_autonomy, those logging lines are ignored.

MishkaMN avatar Sep 26 '23 06:09 MishkaMN

Closed by usdot-fhwa-stol/carma-utils#151

adamlm avatar Sep 26 '23 14:09 adamlm

Reopening this issue because:

  1. the fix was still not complete without carma_ros2_utils change in cpp where the component_manager only sets the logger level using node->get_node_base_interface()->get_name(), which doesn't have namespace. Using get_fully_qualified_name() still also has problems because the logger's name is automatically created with . instead of '/' when the node has namespace. FIX: 1 quick fix would be to do string modification to pass the correct fully qualified name with . separation.
  2. Library still not configurable even with above fix because as mentioned befor:

The reason why the library ones do not work is because in ROS2 Foxy, it is only possible to set the node's log level from launch python file using --ros-args --log-level commands. carma_ros2_utils saves the logger levels from the .conf file and uses that mapping when creating nodes from launch file. However, since there is no nodes are created with names carma_wm or basic_autonomy, those logging lines are ignored.

MishkaMN avatar Dec 10 '23 18:12 MishkaMN

We didn't have resources to attend to this issue under VRU use case. So still needs to be addressed

MishkaMN avatar Mar 07 '24 19:03 MishkaMN