Proposal: move dds tracepoints to RWM layers about ros2_tracing HOT 1 CLOSED

Thanks for bringing this up! I agree that this has a lot of upsides.

The goals for the DDS instrumentation, in order of importance, are:

1. Collect the necessary information to be able to link published messages to received messages. This is of course the source_timestamp value. The DDS writer/reader fields are only used to link the dds:write/dds:read to their corresponding publisher/subscription.
   1. We do collect the DDS topic name, but it's not that useful
2. Have a tracepoint as close to the network/transport as possible so that we know exactly(-ish) when we've sent or received the message
   1. This (and symmetry) is the only reason I created dds:read, since it doesn't collect the source_timestamp
   2. The RMW and DDS tracepoints are very close together, so we don't strictly need anything in DDS

Therefore, if we can achieve that directly in RMW, then yeah it's even better. My only concerns are:

1. Are the proposed DDS API call changes acceptable? I assume so
2. Are there any potential side effects if we use, for example with Cyclone DDS, std::chrono::system_clock::now().time_since_epoch().count() instead of dds_time() for the source_timestamp on the publisher's side? It's not really a problem for tracing, since we only use them as IDs, but it could affect other users.

To add to your proposed changes, we could do a number of other optimizations:

1. (Merge rmw_publish with dds:write and rmw_take with dds:create, and remove the DDS writer/reader fields)
2. Remove dds:create_writer and dds:create_reader, since we don't need the DDS writer/reader fields
3. Remove rmw_publisher_init & rmw_subscription_init, since they're only used to link the ROS 2 pubs/subs to their DDS writer/reader through their GIDs

Regarding making a number of tracepoints effectively optional, if we remove the DDS writer, then we lose the link between rmw_publish and its publisher, since rmw_publish doesn't include the publisher (whereas rmw_take does include the subscription). However, we could always change rmw_publish to include the publisher. As you said, it would be valuable to reduce the number of required tracepoints.

Can you try to implement your proposed changes to make sure it works? No need for all the optimizations I mentioned above. Then you could open PRs and we could discuss with the rmw_* maintainers.

from ros2_tracing.