Add tracepoint for generic publisher/subscriber

[h-suzuki-isp]

Hi, I'm developed member of CARET for real-time analysis using ros2tracing.

In communication through GenericPublisher/Subscription, message tracking was not possible due to a lack of trace data. In this PR, added trace points to the GenericPublisher and GenericSubscription.

• GenericPublisher I have added the following trace points.

  • tracepoint : rclcpp_publish

• GenericSubscription There was a recent PR that made create_generic_subscription go through AnySubscriptionCallback. This change caused callback to be captured by the lambda, resulting in it being copied and callback address was changed. In this case, the addresses of trace points outputed at initialization and runtime were different. Therefore, I modified to ensure that they have the same callback address . And, I have added the following trace points.

  • tracepoint : rclcpp_subscription_init
  • tracepoint : rclcpp_subscription_callback_added

Additionally, with these changes, I submitted a PR to the following repository.

• rcl https://github.com/ros2/rcl/pull/1136
• rmw_cyclonedds https://github.com/ros2/rmw_cyclonedds/pull/485
• rmw_fastrtps https://github.com/ros2/rmw_fastrtps/pull/748
• rmw_connextdds https://github.com/ros2/rmw_connextdds/pull/145
• ros2_tracing https://github.com/ros2/ros2_tracing/pull/97

[fujitatomoya]

@h-suzuki-isp thanks for the PRs. should we also add TRACETOOLS_TRACEPOINT to https://github.com/ros2/rmw_connextdds accordingly?

[h-suzuki-isp]

@fujitatomoya Thank you for your review. I'm not fluent in English, so I apologize if my message wasn't clear.

thanks for the PRs. should we also add TRACETOOLS_TRACEPOINT to https://github.com/ros2/rmw_connextdds accordingly?

If Connext DDS is the DDS officially supported by ROS 2, I believe it's appropriate to add tracepoints to rmw_connextdds as well. Therefore, I've created a pull request for rmw_connextdds as follows. https://github.com/ros2/rmw_connextdds/pull/145 Since rmw_publish already seemed to be added, I only added rmw_take. I haven't delved into rmw_connextdds in detail. so I'm not confident about the implementation, but it would probably be like this.

[fujitatomoya]

If Connext DDS is the DDS officially supported by ROS 2

yes it is, https://github.com/ros2/ros2/blob/30e9231f04d6e971de4d9bcc3cc98733f8a89d23/ros2.repos#L302-L305

[Zard-C]

[christophebedard]

Thank you for the PR, this is really useful! It would be really great if you could add some tests to test_tracetools to cover this new instrumentation:

1. A test similar to test_subscription.py but for a generic subscription
   • This would for example help make sure that the callback address is valid and that the trace data is coherent
2. A test similar to test_pub_sub.py but for a generic publisher and a generic subscription
   • This would for example help make sure that the *_publish and *_take instrumentation is valid for serialized messages (i.e., we can track messages from end to end)

You would probably need to create copies of the test_ping & test_pong nodes that use generic subscriptions/publishers instead of normal subscriptions/publishers.

[h-suzuki-isp]

@christophebedard Thank you for your review and explanation about test_tracetools. Based on this, I have created following PR. https://github.com/ros2/ros2_tracing/pull/97 However, the CI is still failing, so I am investigating the cause.

[h-suzuki-isp]

@christophebedard I'd like you to review this. The reason CI is failing is because it's not using the fixed branch, but local tests are passing.

[christophebedard]

CI:

• Linux
• Linux-aarch64
  • (Note that this doesn't test rmw_connextdds because it's not available on aarch64)
• Windows

It includes the PRs mentioned in the PR description above:

• (this PR)
• https://github.com/ros2/rcl/pull/1136
• https://github.com/ros2/rmw_cyclonedds/pull/485
• https://github.com/ros2/rmw_fastrtps/pull/748
• https://github.com/ros2/rmw_connextdds/pull/145
• https://github.com/ros2/ros2_tracing/pull/97

[ros-discourse]

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-jazzy-jalisco-released/37862/9