-
Notifications
You must be signed in to change notification settings - Fork 426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Logs from child loggers are not sent to /rosout #1694
Comments
Thank you for reporting this issue, but I am afraid it is not a bug. If you expect e.g.
FYI. If you are interested in how I would like to hear others' opinions. |
@iuhilnehc-ynos, ok, I see. It is not completely logical to me that logging to /rosout should behave differently than logging to terminal and file (when enabled). If others agree, then this could maybe become a feature request instead? In any case, I appreciate the feedback and explanation! |
Actually, I'm surprised by this behavior. I would have expected that what @rasmusan described should work. For example, there are places where we use
So it's something maybe we should look into. |
There was a bit of an issue with node name uniqueness when the rosout feature was originally developed, but I didn't see anything discussing sub loggers. It was likely not considered properly or child loggers came afterward: Anyway, I would expect that child loggers that share a root name with a node should be published. Further more, I expect that all log messages should be logged to rosout by someone. Like I said, I wasn't involved with the implementation or review (for the most part) of the rosout feature pull request, but I am surprised to find that only a select few things make it to rosout, but there might have been good reasons for that. I can imagine also that we may have desired to publish more log messages, but went for the incremental improvement of publishing some of them until we had time to solve issues related to publishing them all. @hidmic @jacobperron I think you guys helped review the original rosout pull request, do you remember these decisions being made/discussed? |
+1 on this. i see that currently this cannot be done but it would be nice to have this. |
Memories from 2019? That's in cold storage already :) So, quick inspection suggests the problem is that there's no concept of a child logger outside I wonder if and how this is working in |
Yeah, any discussions are ancient history to me, but I'm pretty sure the subject of sub-loggers never came up when implementing rosout. |
Unfortunately It doesn't as well. I realized it after this PR. Any short term plans for this? I think this is extremely important for people using logging tools that rely on /rosout (e.g foxglove); currently we are missing a considerable amount of information in our "rosout" logs |
There have been open PRs to address this issue in |
Thank you @fujitatomoya for pushing this effort forward! |
Just FYI, rcply fix is ros2/rclpy#1084 |
As this issue was reported for rclcpp, I think it should be closed after #1717 is merged. |
@fujitatomoya @iuhilnehc-ynos
I don´t have a strong opionion about this (after all, nobody seems to be missing those non-node logs on /rosout), but I think it should be formally decided to leave it as it is, or to reopen this issue (or a new one). |
Actually, it's tricky: int main(int argc, char * argv[])
{
rclcpp::init(argc, argv);
auto node = std::make_shared<rclcpp::Node>("LoggerNode");
RCLCPP_INFO(node->get_logger(), "Log 1");
RCLCPP_INFO(node->get_logger().get_child("child"), "Log 2");
RCLCPP_INFO(rclcpp::get_logger("LoggerNode.child"), "Log 3");
rclcpp::shutdown();
return 0;
} This works as expected: int main(int argc, char * argv[])
{
rclcpp::init(argc, argv);
auto node = std::make_shared<rclcpp::Node>("LoggerNode");
auto childnode = node->get_logger().get_child("child");
RCLCPP_INFO(node->get_logger(), "Log 1");
RCLCPP_INFO(node->get_logger().get_child("child"), "Log 2");
RCLCPP_INFO(rclcpp::get_logger("LoggerNode.child"), "Log 3");
rclcpp::shutdown();
return 0;
} |
sorry for answering question with question. but how can we publish the log w/o having corresponding node? |
This does not seem a blocking issue? E.g. by creating a node on initialization (e.g. in |
@jrutgeer let me clarify this, you are suggesting that logger should be working to send data to btw, probably we can open another issue for further discussion. |
Bug report
Required Info:
Steps to reproduce issue
Run this code:
Expected behavior
Terminal output:
File output to /home/user/.ros/log/logging_tester_81312_1623765894893:
Output on /rosout:
Actual behavior
Output to terminal and to file is as expected. Output to /rosout only contains the top-level log, and not the child log:
Additional information
The child logs are omitted no matter if the child logger is stored as a member.
The text was updated successfully, but these errors were encountered: