Skip to content

Conversation

@emersonknapp
Copy link
Contributor

@emersonknapp emersonknapp commented Oct 13, 2025

Description

(Note: I've hesitated on opening this contribution because I'm not sure that we'll agree on its value at a design level. However I've decided that maintaining my fork separately is doing more harm than good, and need to open the serious discussion)

Allows a user to set an environment variable RMW_IMPLEMENTATION_WRAPPER, in addition to RMW_IMPLEMENTATION.

This variable will pre-load an additional shared library that acts as the first pass of the rwm implementation. However, instead of just being called, this library also receives function pointers for passthrough to the "real" RMW_IMPLEMENTATION

This feature allows plugging in some "middleware-middleware" to do arbitrary processing on calls going through RMW at that layer, before going on to the implementation, without depending on changing rcl, or locking in to any of the implementations (Fast-DDS, Cyclone, Zenoh, etc))

EDIT: removed most of the disussion on Topic Statistics

Use case note: I have been using this feature stably on a fork for about 2 years in production to enable rmw_stats_shim which intercepts calls to rmw_publish and rmw_receive to compute and publish topic statistics on all publishers and subscriptions throughout an entire application. That library is not part of this review and is only mentioned to illustrate the type of thing this PR enables.

Is this user-facing behavior change?

Optionally.

Did you use Generative AI?

No

Additional Information

…e shim implementation and reduce function call indirects

Signed-off-by: Emerson Knapp <[email protected]>
Signed-off-by: Emerson Knapp <[email protected]>
@emersonknapp emersonknapp changed the title RMW Implementation Wrapper/Shim library RMW implementation library wrapper/shim Oct 13, 2025
@emersonknapp emersonknapp moved this to In review in Robograph Planning Oct 13, 2025
@ros-discourse
Copy link

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

https://discourse.openrobotics.org/t/ros-pmc-minutes-for-october-14-2025/50533/1

@peci1
Copy link

peci1 commented Oct 15, 2025

I really like the idea of getting pub/sub stats for free. However, as you say, logically, this thing seems to stand aside a bit... Have you tested the performance impacts of using the wrapped symbols? I guess that with no wrapper, the impact is zero?

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

just out of curiosity, are you trying to replace TopicStatisticsOptions in rclcpp with this feature?

I am not sure if the topic is the user frontend to get the statistics... this also goes to TopicStatisticsOptions in rclcpp. (which btw, I do not really like the design of TopicStatisticsOptions in rclcpp either because of the following reasons.)

  • in the real application, it queries the statistics to adjust the parameter in real time. searching the appropriate topic or the concerned data from the giant topic would not be ideal to react in real time application. i think it makes more sense to provide the statistics API on the endpoint (means that user application can call the statistics API on the endpoint such as publisher and subscription), so that the application can get the statistics on the endpoint and adjust the parameter in real time for process local scope.
  • for the ROS 2 system statistics or observability, how about creating the service (which should be configurable) to pull the statistics for the node as it needs to do (i am suggesting the pull design based on service instead of push design based on topics)? so that collector can scrape the statistics as it needs to do based on the each endpoint requirement, without getting interrupted by unconcerned statistics? basically i think that it is not up to the application whether it needs to publish the statistics or not, it is system application to get the statistics as it needs to. (that said, the application should always expose the statistics ready.)
  • or maybe, push and pull design can be configurable by system?

@emersonknapp
Copy link
Contributor Author

emersonknapp commented Oct 22, 2025

@peci1 I haven't been able to think of a very good way to test the overhead. Removing consideration of what the wrapper is doing with each call - because this is fully dependent on the implementation details of the loaded wrapper library - the overhead of the redirect is simply that: a redirect, one extra function (pointer) call. With no wrapper loaded, the added overhead is not "zero" - there is an extra if() statement added to check if the wrapper function is set. But it is effectively zero.

@fujitatomoya these are good discussion points - but I actually want to avoid having that discussion here. Mentioning it in the description was a risk.

This PR creates a generic way to intercept calls through the RMW API with a shim library.
I want to make sure that we review it in that context.

Topic Statistics via the rmw_stats_shim library is one way I am using this feature. But, I am not proposing here or anywhere else that we consider that library for standardization or review in a ROS 2 core context. That's a conversation for later in a different place.

@emersonknapp
Copy link
Contributor Author

I have updated the PR description to reflect my wish to leave Topic Statistics out of the review discussion

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.

5 participants