Fix bug with topic statistics for intra process comms

[roman-y-wu]

Description

Fixes #2911

Is this user-facing behavior change?

Yes, this patch updates SubscriptionIntraProcess to pass along the topic statistics pointer and call the statistics handler during intra-process message deliveries. This ensures that topic statistics—such as message age and publishing period—are still computed when use_intra_process_comms is enabled, which is important for users monitoring performance metrics.

Did you use Generative AI?

No.

Additional Information

n/a

[fujitatomoya]

thanks for creating PR. a couple of comments.

• did you try to build with your local environment, it seems build is failing. see https://build.ros2.org/job/Rpr__rclcpp__ubuntu_noble_amd64/718/console
• test would be idea to make sure the statistics works okay. could you also add the test case with intra-process communication for test_subscription_topic_statistics.cpp?

[fujitatomoya]

as we know, we do not have message info from rmw implementation with intra-process communication.
if we do this, the message age statistics will be always 0 because msg_info.source_timestamp is equal to time.
i understand we can generate the statistics with this change, but is this really useful for user application? i am not even sure about this. instead of generating fake statistics, maybe warning once that tells intra-process communication does not provide age statistics would be better for the user application? or maybe it is always age is zero?

[fujitatomoya]

at least for period statistics, this can enables it to calculate intra and inter-process communication messages all together. i think that is the right thing to do.

[luca-della-vedova]

Hi @romanwu10

You stated in your description that you did not use Generative AI for this PR:

Did you use Generative AI?

No.

However, it seems extremely similar to this PR I noticed on your private repo that explicitly states that it was generated through ChatGPT.
Can you clarify on whether you did or didn't use GenAI for this contribution?

[jayyoung]

Hi @fujitatomoya and @roman-y-wu just wondering if we can revive this PR/discussion a bit? I wanted to share some real-world validation of this fix in the hope of reviving it. We've been using this in a production context for about a month and it resolves a critical issue for us.

Background/use-case
At Dexory we're running a fleet of 126 autonomous warehouse robots, each running multiple lidar pipelines all with the flag use_intra_process_comms: true in component containers. We rely on ROS 2 topic statistics to monitor lidar health in production: specifically message_period to detect sensors that have stopped publishing, and message_age as a secondary latency check.

Before this fix, all statistics from our lidar subscriptions when IPC was on reported NaN values, even if the sensor was publishing data and working normally. This made our monitoring tools unable to distinguish a healthy sensor from a dead one.

What we did
We ported this PR onto a fork of rclcpp @ 29.5.6 (almost-latest kilted), with some tiny modifications: we set msg_info.source_timestamp = now at receive time in the IPC path, to prevent message_age from computing some gigantic value (Unix epoch in ms). We also added a type-erased function to pass the stats handler into SubscriptionIntraProcess, which we discovered avoids a circular include chain we saw that occurs on kilted when including subscription_topic_statistics.hpp directly from subscription_intra_process.hpp. This might have just been a consequence of the original PR/fix being old and things having shifted since it was first implemented, so just some plumbing needed here for us to get it running.

Our kilted port + update with the above is here: botsandus#1

Our Results
We have been running this fix in production across our full fleet of 126 robots for over a month with no issues, and it has allowed us to catch legitimate issues in our lidar pipeline when they arise. For us: message_period is now meaningful for statistics from IPC subscriptions, the message_age reports 0ms, which we are OK with because our reasoning is that IPC delivery should have no (or.. arbitrarily near-zero) transport latency anyhow, so this is expected.

Re: the concern about message_age accuracy
We agree with @fujitatomoya earlier comment that message_age will not be meaningful for IPC. However we'd argue this is acceptable for the following reasons:

• IPC delivery should be near-zero latency so a meaningful age stat isn't really possible or necessary (at least for our use-case)
• message_period which is accurate is the more useful metric for our health monitoring use-case anyway
• The RCLCPP_WARN_ONCE log relays the limitation to end users, so there should be no surprises here.

Overall, IMO, thee alternative of reporting NaN is strictly worse for users trying to monitor systems health via the stats topics, and the fix here is more sensible behaviour for IPC use-cases.

On the implementation
The PR as written compiles on the version of rolling it was made against, but on kilted we hit a circular include, which is worth being aware of. This is in the linked PR on our fork above.

We'd love to see this merged. Is there anything we can do to help move it forward, like additional tests,revised implementation, or anything else you folks might need? we are happy to assist where we can, and happy to validate on our fleets.

cc: @tonynajjar