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

Declare a dependency on the (Linux-only) LTTng userspace tracer #1177

Closed
1 task done
christophebedard opened this issue Sep 1, 2021 · 10 comments
Closed
1 task done
Assignees

Comments

@christophebedard
Copy link
Member

christophebedard commented Sep 1, 2021

Feature request

Feature description

This is a follow-up to #957 and relates to ros2/ros2_tracing#23

I would like to discuss declaring a dependency on the LTTng userspace tracer (LTTng-UST) on Linux. I'd like to make sure that this change would be welcomed. My goal would be for this to be implemented a fair amount of time before the next release (Humble).

Core ROS 2 packages are instrumented with calls to functions provided by the tracetools package. These functions contain:

  1. LTTng tracepoints if LTTng is installed/detected and tracetools is re-compiled/overlayed
  2. nothing when building from source if LTTng isn't installed/detected
  3. nothing in debians/Linux binaries
  4. nothing in any code built for/on non-Linux platforms (macOS, Windows)

and

  1. the function calls themselves can be completely removed by building instrumented core packages with -DTRACETOOLS_DISABLED=ON

This LTTng dependency isn't declared. This was initially for simplicity (especially since this is a platform-specific dependency) and because of some concerns about overhead.

This means that this feature currently isn't available out-of-the-box; users need to manually install LTTng and build from source (or at least just tracetools), since it's not installed when installing debians or running rosdep install. This has led to some questions and issues:

As mentioned in #957 (comment), the LTTng userspace tracer is less "invasive" than the kernel tracer. I don't think we need to declare a dependency on the kernel tracer. Users could install it manually if they want to record kernel data for their analyses.

Also, in practice, the overhead is really negligible if tracepoints are not explicitly enabled: https://github.com/lttng/lttng-ust/blob/1f8a8ec9581af89d98aec47de9ad9e25087cd54a/include/lttng/tracepoint.h#L53-L62. Users/companies that do not want to ship a ROS 2-based product with LTTng can completely disable/remove it by setting the right CMake option (see item 5 above) and building from source, which I assume is something they are already doing.

Some references:

Implementation considerations

New dependencies for tracetools:

  • liblttng-ust-dev: LTTng-UST
  • lttng-tools: LTTng CLI (not strictly necessary, but recommended to be able to actually use the tracepoints)

These would of course need to be installed on the Linux ci.ros2.org executors.

If the tracing tests turn out to create problems on ci.ros2.org, skipping them could be an option, but I think it would probably be fine.

Any other platform-related considerations?

Pull requests

@clalancette clalancette self-assigned this Sep 16, 2021
@christophebedard
Copy link
Member Author

@clalancette friendly ping 😃 I see that you self-assigned this. Is there any way I can help push this forward?

@christophebedard
Copy link
Member Author

@clalancette friendly ping

@mjcarroll
Copy link
Member

I think that having the tracepoints available by default is a reasonable request.

The value of being able to trace without rebuilding large portions of the workspace is helpful for users trying to diagnose performance issues.
Additionally, as time goes on, it would be nice to be able to build more analysis tooling around the tracing capabilities, and having it easily available would be valuable.

The overhead of the user-space trace points seems minimal compared to the value of having the tracepoints in.

The only concern I can think of would be the availability of lttng-ust on our supported platforms which is currently:

RHEL 9: (2.12?)
Ubuntu Jammy: (2.13.1)[https://packages.ubuntu.com/jammy/liblttng-ust1]
Debian Bullseye: (2.12.1)[https://packages.debian.org/bullseye/liblttng-ust0]
OpenEmbedded: (2.13.5)[http://cgit.openembedded.org/openembedded-core/tree/meta/recipes-kernel/lttng]

@cottsay I may need your input on RHEL, it appears to be 2.12, but I'm not sure I was using the correct package listing.

@mjcarroll
Copy link
Member

Also of note, the users who are deeply concerned with performance (on the level of lttng serspace tracepoints being an issue) are likely not using our pre-compiled binaries and are instead compiling themselves.

@cottsay
Copy link
Member

cottsay commented Jan 25, 2023

cottsay I may need your input on RHEL, it appears to be 2.12, but I'm not sure I was using the correct package listing.

It is indeed 2.12.0:

$ dnf list -q --available --showduplicates lttng-ust
Available Packages
lttng-ust.x86_64          2.12.0-6.el9          rhel-9-for-x86_64-appstream-rpms

@christophebedard
Copy link
Member Author

christophebedard commented Jan 25, 2023

I can work on the main ros2_tracing PR to enable the instrumentation by default.

The only concern I can think of would be the availability of lttng-ust on our supported platforms which is currently:

This is kind of related to ros2/ros2_tracing#16. If all current Linux platforms have LTTng-UST >= 2.12, we should be fine, but things could still be improved.

@mjcarroll
Copy link
Member

Yes, 2.12 is going to be the least common denominator for rolling. I think we would have no intention of backporting this into released distributions.

@christophebedard
Copy link
Member Author

christophebedard commented Jan 25, 2023

I think we would have no intention of backporting this into released distributions.

Agreed

@clalancette
Copy link
Contributor

One thing I will note is that RHEL-9 is aspirational for Rolling at this time. We may or may not get it in before Iron, so Iron may end up supporting RHEL-8 still. I'm fine if we end up disabling tracing by default on RHEL-8, but we should keep that in mind.

@christophebedard
Copy link
Member Author

The PRs have been merged, so I'm closing this.

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

No branches or pull requests

4 participants