Skip to content
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

ros2: support client/service instrumentation #127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

christophebedard
Copy link
Member

@christophebedard christophebedard commented Nov 24, 2024

This adds support for the new client/service (i.e., RPC) instrumentation in ROS 2, see ros2/ros2_tracing#145.

  1. In the objects analysis, create client and service objects
  2. In the messages analysis, create the following instances:
    1. Request publication
    2. Request take and callback
    3. Response publication
    4. Response take
    5. Message transport for requests and responses
  3. In the messages dataprovider, display the above instances

There is one limitation. Normal message publications and message takes have instrumentation that provides a "start time." For example, for message publications, the ros2:rclcpp_publish tracepoint is the start and the ros2:rmw_publish tracepoint is the end of a message publication. This allows us to attribute a duration to the publication and therefore display a time graph state. However, we only have a single tracepoint for client/service-related publication/take instances, so we do not have any duration data. For now, just hardcode a 5000 ns duration so that time graph states are visible enough.


Example:
Screenshot from 2024-11-24 11-53-36
(trace: 2024-11-23__test_tracetools__test_service_pingpong__service-e2e.zip)

Clients (S↗️) and services (S↘️) are displayed under nodes (🔲) on the left. Chronological explanation:

  1. The /ping client under the /test_service_ping node sends a request (short-ish time graph state from which the outgoing arrow starts)
  2. The /ping service under the /test_service_pong node receives (takes) the request (thinner time graph state that the incoming arrow points to, right before the long time graph state)
  3. The /ping service calls executes a callback for the request (long time graph state)
  4. The /ping service sends a response inside the request callback (time at which the outgoing arrow starts)
  5. The /ping client receives (takes) the request (thinner time graph state that the incoming arrow points to)

This adds support for the new client/service (i.e., RPC) instrumentation
in ROS 2, see ros2/ros2_tracing#145.

1. In the objects analysis, create client and service objects
2. In the messages analysis, create the following instances:
    1. Request publication
    2. Request take and callback
    3. Response publication
    4. Response take
    5. Message transport for requests and responses
3. In the messages dataprovider, display the above instances

There is one limitation. Normal message publications and message takes
have instrumentation that provides a "start time." For example, for
message publications, the `ros2:rclcpp_publish` tracepoint is the start
and the `ros2:rmw_publish` tracepoint is the end of a message
publication. This allows us to attribute a duration to the publication
and therefore display a time graph state. However, we only have a single
tracepoint for client/service-related publication/take instances, so we
do not have any duration data. For now, just hardcode a 5000 ns duration
so that time graph states are visible enough.

Signed-off-by: Christophe Bedard <[email protected]>
@christophebedard
Copy link
Member Author

christophebedard commented Nov 24, 2024

I have to say that the ROS 2 plugin (especially the main analysis/view: messages) could use some serious refactoring/improvements, even before this PR. I'll try to find some time to do some cleanup in a separate PR.

Copy link
Contributor

@MatthewKhouzam MatthewKhouzam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good,

@MatthewKhouzam
Copy link
Contributor

Uh... @christophebedard want to merge this? You're a committer after all :)

@christophebedard
Copy link
Member Author

I was waiting for the new ROS 2 instrumentation to be reviewed just in case, but it's taking a bit longer than anticipated. Hopefully this week!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants