Fix topic statistics source collision for multiple subscriptions

[Saigirish23]

Description

When topic statistics are enabled for multiple subscriptions within the same node,
all statistics messages currently use the same measurement_source_name
because SubscriptionTopicStatistics only uses the node name when constructing
the source identifier.

This makes it difficult to determine which subscription produced a particular
statistics message when a node subscribes to multiple topics.

This PR addresses the issue by including the subscribed topic name in the
statistics source identifier. The resulting identifier is constructed as
node_name/topic_name, allowing statistics from different subscriptions within
the same node to be distinguished.

Changes included in this PR:

• Update SubscriptionTopicStatistics to accept the subscribed topic name.
• Construct the statistics source identifier using both the node name and topic name.
• Update the subscription creation path to pass the topic name when topic statistics are enabled.
• Update existing tests affected by the constructor change.
• Add a regression test (test_multiple_subscriptions_differentiated) to verify that statistics generated by multiple subscriptions in the same node have distinct source identifiers.

Fixes #2756

Is this user-facing behavior change?

Yes.

When topic statistics are enabled, statistics messages published on
/statistics will now contain unique measurement_source_name values for
different subscriptions within the same node. This makes it possible to
correctly identify which subscription produced a given statistics message.

Did you use Generative AI?

Yes.

I used ChatGPT (GPT-5.5) to assist with investigating the issue, discussing
possible implementation approaches, reviewing code changes, and drafting parts
of the regression test and PR description. All code changes, testing, and final
content were reviewed and validated manually.

Additional Information

A similar fix was initially proposed for the Humble branch, but it could not be
merged because the required constructor change introduces an API change that is
not suitable for a stable release. This PR targets the Rolling branch, where
such changes are permitted.

[fujitatomoya]

i do not think this PR solves the problem.

it uses get_node_base_interface()->get_name(), which is the unqualified node name to publish the statistics message. that "fix" just trades the parsing ambiguity for a collision that is two nodes both named bar, one at /foo/bar and one at /baz/bar, each subscribing to some_topic, both yield bar/some_topic, which is the very same-source-collision #2756 is trying to eliminate, just moved up a level to the namespace.

[fujitatomoya]

can we build this PR successfully and all test pass via colcon test? it looks like we can't apply template arguments to a non-template class...

[Saigirish23]

Thanks for taking a look.

You're right regarding the collision. My change only differentiates subscriptions using the unqualified node name and topic name, so nodes with the same name in different namespaces could still collide. I overlooked that get_node_base_interface()->get_name() does not include the namespace.

I'll take another look at how the source identifier is constructed and see whether a fully-qualified node name can be used instead.

I'll also revisit the test changes and verify that the updated constructor usage builds correctly and that the affected tests pass.

[Saigirish23]

Thanks for the review.

I've updated the implementation to use get_node_base_interface()->get_fully_qualified_name() instead of get_name() when constructing the statistics source identifier. This ensures that nodes with the same name in different namespaces produce distinct identifiers and avoids the collision scenario you described.

I also reviewed the constructor changes and confirmed that SubscriptionTopicStatistics is only constructed through create_subscription() (aside from the test helper), so the updated identifier is applied consistently.

Regarding the build verification: I attempted to rebuild and run the relevant tests in a Rolling Docker environment. After resolving several missing dependencies, I ran into an unrelated source/binary mismatch in the container environment outside the files modified by this PR, so I was not able to complete a full local test run. The updated changes have been pushed, and the CI results should provide validation in the official Rolling environment.

Please let me know if you'd prefer an additional regression test that explicitly covers the same node name used in different namespaces.

[jmachowinski]

The member now has a different meaning, thus the name is not matching any more.

[Saigirish23]

Good point. Since the identifier now includes both the node and topic information, 'node_name_' no longer accurately described what the member stores. I've renamed it to 'measurement_source_name_' and pushed the update.

[jmachowinski]

Note, we had a closer look a the whole topic statistic subsystem and figured out it is in a half baked state.
There is a tendency to remove it from rclcpp in the near future and replace it by an API hook interface that could be
used by a community pkg.

[Saigirish23]

Note, we had a closer look a the whole topic statistic subsystem and figured out it is in a half baked state. There is a tendency to remove it from rclcpp in the near future and replace it by an API hook interface that could be used by a community pkg.

Thanks for the context. My intention with this PR is to address the source-collision issue described in #2756 with a minimal, targeted change. I'm happy to adjust the approach if there is a preferred direction for fixes in this area.

[jmachowinski]

Pulls: #3171
Gist: https://gist.githubusercontent.com/jmachowinski/35ee15790561a228347236b347b42162/raw/443b61f527d7def270daba97ee84140aa6136900/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19516

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[jmachowinski]

The tests need fixing, see CI for details

[Saigirish23]

Thanks for running CI.

I tried accessing the Jenkins job output, but I don't appear to have permission to view the logs. Could you share the failing test name and the relevant assertion or error message from the CI output? That would help me reproduce the failure and work on a fix.

[jmachowinski]

you should be able to log into the ci using your github account and then see the log

[Saigirish23]

Thanks for the suggestion. I logged into ci.ros2.org with my GitHub account, but I'm still getting an "Access Denied" page when trying to open both the 'ci_launcher' job and the linked 'ci_linux' build. Is there an additional permission required to view those logs, or could you point me to the correct URL?

[cottsay]

I logged into ci.ros2.org with my GitHub account, but I'm still getting an "Access Denied" page when trying to open both the 'ci_launcher' job and the linked 'ci_linux' build.

Sorry about that, this was a mistake on my part. Please try again.

[Saigirish23]

Thanks for pointing me to the CI logs.

The failure was due to compile errors in the regression test: the test was using an incorrect 'SubscriptionTopicStatistics' constructor invocation and a non-existent 'EmptyPublisher' helper. I've fixed both issues and pushed an update.