-
Notifications
You must be signed in to change notification settings - Fork 9
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
Rust PROBE frontend #20
Conversation
All in all, great work! I have a few minor comments. Improved serialization format: completely agree. Gzipped tar of jsonlines is great and cross-platform. Do you want to take over the task of parsing that in Python or should I give it to one of the undergrads? Dependencies: Yes, static dependencies can be used liberally. We should think about Cargo machete or eventually just for build times. Multithreading and fast: thanks Rust. We should eventually include a hyperfine microbenchmarks in the test suite. I have a note tracking that task. Hacky build process: that's a tricky problem. Other than making Library path: See my note on the code for avoiding a path lookup in favor of hardcoding a location (I think that's actually better in this case), since it is more predictable, and we can strictly control it during package management.
Tests, we're working on some "end-to-end tests" (from |
I can do that, as mentioned above, It might be best to just break off the data-manipulation parts of this as a library and then make python bindings, both for speed and so there's a single source of truth on the json schema for any given version. Thoughts? |
Version
|
Task Tracker
|
Update the .envrc for probe_frontend to automatically run make on libprobe, as well as export the __PROBE_LIB environment variable so that the frontend can find the libprobe.so
Co-authored-by: Sam Grayson <sam@samgrayson.me>
Removes a lot of details that are usually abstracted by libraries when printing for humans. In contrast with some of the code review comments I left mtime and errno field because I feel those are useful for human readers; even if all a human is likley to do with errno is "was it zero?"
This commit addresses serveral points raised durring the last code-review (and a few that weren't): - Restructured arena.rs to read top-down. - Refactored child process code to use std::process instead of subprocess crate. - Cleaned up arena pointer resolition to use less unsafe code. - [INFO] Better log messages.
Co-authored-by: Sam Grayson <sam@samgrayson.me>
the structs now get converted to camel case at the rust level instead of the python level meaning python doesn't have to do any type name translation. Also strips out redundant field prefixes from the rust structs (which also solves the issue for python).
building the probe cli with `cargo build` inside the devShell will still produce a dynamically linked binary, but building with `nix build .#probe-cli` should produce a statically linked binary
Okay, that wasn't elegant, next time I need to figure out the more proper way to rebase main. |
Goals
A performant and safe (some unsafe code is required, but I've tried to keep it to a minimum) libprobe frontend with minimal runtime dependencies that doesn't require loading path injection (such as the
PYTHONPATH
injection the currentPROBE
script does) in order to be as environment agnostic as possible.Current coverage
Currently, only the
record
anddump
commands from the python implementation have been implemented.Benefits
Currently, this rust frontend has the following benefits over the python implementation:
Improved Serialization Format
Rather than retaining the raw C-structs emitted by libprobe as a serialization format, this frontend marshals the raw arena data into rust data structures and from there serializes them into a more cross-platform format: each
probe_log
is a gzip compressed tar archive containing a folder structure much like a libprobe arena directory (<PID>/<EXEC_EPOCH>/<TID>
), but instead of being a directory, each<TID>
is a jsonlines file, where each line is an Op serialized as json. In addition, there is a file named_metadata
at the root of the archive containing a json object that includes the entry PID (that is, the PID of the process directly launched by the frontend), as well as copious OS and hardware details.Dependencies
While this frontend does use several external dependencies, due to rust's build system and language architecture they are statically linked into the binary at build-time. According to
ldd
the only runtime dependency (excludingld.so
, compiler runtime(s), and vDSO), is libc.Multithreading
While single commands or short scripts don't benefit much from this due to the overhead of creating threads, the data-marshaling and serialization is done in a stream-based manner using functional patterns that can be trivially parallelized, currently it parallelizes each PID automatically, although ideally I'd like to eventually replace this with some form of heuristic.
Fast
While I haven't done extensive testing, testing a moderately complex command (
bash -c "nvim +q; lsd -la /etc"
) withhyperfine
, the rust frontend performed 139% faster (2.39x the speed) than the python frontend. According to the rust frontend, running that command produced 2478 Ops, while the python frontend crashed when trying to dump its log.Known Issues
hacky build process
Currently the
prov_ops.h
header file has to be coppied fromlibprobe/include
toprobe_frontend/include
manually or with thebuild.sh
script before it can be built with either nix or cargo. This is rather hacky and I'd like a better solution, suggestions appreciated. :)Library path
I have yet to come up with a truly elegant way for finding the location of
libprobe.so
currently there is a flag--lib-path
as well as an environment variable__PROBE_LIB
that can be set to the directory containinglibprobe.so
andlibprobe-dbg.so
. For the purposes of development, the.envrc
file inprobe_frontend
automatically sets__PROBE_LIB
to the canonicalized version of../libprobe/build
.rusage
I'm using a manually defined version of the
rusage
struct because the glibc version defines each field as a union over two integral types for kernel/userland compatibility reasons, this turns the (de)serialization traits forrusage
from something trivial that can be autogenerated by rust-bindgen to a massive headache that needs lots of unsafe code, so I just defined my own version without the unions inbuild.rs
.Before Merging
The following things need to be done before this PR is ready to be merged:
While much of the code is somewhat repetitive boilerplate, there are parts (such as anywhere there's unsafe code) that should definitely have more documentation.
This should be pretty simple, but it needs better logging, as well as adding a logging facility to actually display the logs created by the log crate.
TestsWhile the rust compiler can guarantee a lot, the addition of some actual unit tests, especially around the unsafe code, are necessary before I'd consider this production ready.Large scale unit testing abandoned in favor of end-to-end testing.
In the Future
These are some features I'd like to bring to the rust frontend in the future:
ssh
supportEspecially with PR ssh wrapper for PROBE #19 in the works, I'd like to add support for remote probe-ing over ssh.
Better nix packages
I'd like to make a wrapper nix package around the base crate package and libprobe itself that automatically sets the library path.
Library
I'd like to break out some of the core functionality of the crate into a library so that other people could use it as a starting point for something like a more specialized frontend.
Background Serialization
Ideally once a process exits it's arena directory could be serialized in the background to reduce the significant delay when the last process exits and serveral tens of thousands of Ops need to be processed.
Requested Feedback
Some of the things I'd specifically like feedback on are: