-
Notifications
You must be signed in to change notification settings - Fork 24
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
Add Rust toolchains and example rust library to Collector #1959
base: master
Are you sure you want to change the base?
Conversation
cmake/FindRust.cmake
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a slightly modified version of the cmake module taken from here: https://github.com/micahsnyder/cmake-rust-demo/blob/main/cmake/FindRust.cmake
The main addition is an include directory to pass to the build commands so we can centralise cbindgen headers
collector/lib/rust/host/src/info.rs
Outdated
} | ||
|
||
fn filter_by_key<T: Into<PathBuf>>(&self, path: T, name: &str) -> String { | ||
if let Ok(file) = File::open(path.into()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can probably remove one level of indentation with a let-else here
if let Ok(file) = File::open(path.into()) { | |
let Ok(file) = File::open(path.into()) else { return Default::default(); }; |
let root = PathBuf::from(std::env::var("COLLECTOR_HOST_PATH").unwrap_or("/host".to_string())); | ||
root.join(path.into()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let root = PathBuf::from(std::env::var("COLLECTOR_HOST_PATH").unwrap_or("/host".to_string())); | |
root.join(path.into()) | |
let root = std::env::var("COLLECTOR_HOST_PATH").unwrap_or("/host".to_string()); | |
PathBuf::from(root).join(path.into()) |
let mut distro = String::new(); | ||
let mut hostname = String::new(); | ||
|
||
if let Ok(file) = File::open("/etc/os-release") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to host_path
this as well?
if let Ok(file) = File::open("/etc/os-release") { | |
if let Ok(file) = File::open(host_path("/etc/os-release")) { |
|
||
for line in reader.lines() { | ||
match line { | ||
Ok(s) if s.starts_with("ID") => os = s["ID ".len()..].to_owned(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I'm wondering, do we need to add the =
to these?
Ok(s) if s.starts_with("ID") => os = s["ID ".len()..].to_owned(), | |
Ok(s) if s.starts_with("ID=") => os = s["ID=".len()..].to_owned(), |
let hostname_paths = vec![ | ||
host_path("/etc/hostname"), | ||
host_path("/proc/sys/kernel/hostname"), | ||
]; | ||
for path in hostname_paths { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a vector? I would assume you can just use an array here
let hostname_paths = vec![ | |
host_path("/etc/hostname"), | |
host_path("/proc/sys/kernel/hostname"), | |
]; | |
for path in hostname_paths { | |
for path in [ | |
host_path("/etc/hostname"), | |
host_path("/proc/sys/kernel/hostname"), | |
] { |
|
||
/// Whether or not this host machine is CORE-OS | ||
pub fn is_coreos(&self) -> bool { | ||
self.os_id().eq("coreos") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use standard ==
in these checks?
self.os_id().eq("coreos") | |
self.os_id() == "coreos" |
collector/lib/rust/host/src/info.rs
Outdated
let locations = vec![ | ||
// try canonical vmlinux BTF through sysfs first | ||
("/sys/kernel/btf/vmlinux".to_string(), false), | ||
// fall back to trying to find vmlinux on disk otherwise | ||
(format!("/boot/vmlinux-{}", self.kernel.release), false), | ||
( | ||
format!( | ||
"/lib/modules/{}/vmlinux-{}", | ||
self.kernel.release, self.kernel.release | ||
), | ||
false, | ||
), | ||
( | ||
format!("/lib/modules/{}/build/vmlinux", self.kernel.release), | ||
false, | ||
), | ||
( | ||
format!("/usr/lib/modules/{}/kernel/vmlinux", self.kernel.release), | ||
true, | ||
), | ||
( | ||
format!("/usr/lib/debug/boot/vmlinux-{}", self.kernel.release), | ||
true, | ||
), | ||
( | ||
format!("/usr/lib/debug/boot/vmlinux-{}.debug", self.kernel.release), | ||
true, | ||
), | ||
( | ||
format!("/usr/lib/debug/lib/modules/{}/vmlinux", self.kernel.release), | ||
true, | ||
), | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let locations = vec![ | |
// try canonical vmlinux BTF through sysfs first | |
("/sys/kernel/btf/vmlinux".to_string(), false), | |
// fall back to trying to find vmlinux on disk otherwise | |
(format!("/boot/vmlinux-{}", self.kernel.release), false), | |
( | |
format!( | |
"/lib/modules/{}/vmlinux-{}", | |
self.kernel.release, self.kernel.release | |
), | |
false, | |
), | |
( | |
format!("/lib/modules/{}/build/vmlinux", self.kernel.release), | |
false, | |
), | |
( | |
format!("/usr/lib/modules/{}/kernel/vmlinux", self.kernel.release), | |
true, | |
), | |
( | |
format!("/usr/lib/debug/boot/vmlinux-{}", self.kernel.release), | |
true, | |
), | |
( | |
format!("/usr/lib/debug/boot/vmlinux-{}.debug", self.kernel.release), | |
true, | |
), | |
( | |
format!("/usr/lib/debug/lib/modules/{}/vmlinux", self.kernel.release), | |
true, | |
), | |
]; | |
let release = &self.kernel.release; | |
let locations = vec![ | |
// try canonical vmlinux BTF through sysfs first | |
("/sys/kernel/btf/vmlinux".to_string(), false), | |
// fall back to trying to find vmlinux on disk otherwise | |
(format!("/boot/vmlinux-{release}"), false), | |
(format!("/lib/modules/{release}/vmlinux-{release}"), false), | |
(format!("/lib/modules/{release}/build/vmlinux"), false), | |
(format!("/usr/lib/modules/{release}/kernel/vmlinux"), true), | |
(format!("/usr/lib/debug/boot/vmlinux-{release}"), true,), | |
(format!("/usr/lib/debug/boot/vmlinux-{release}.debug"), true), | |
(format!("/usr/lib/debug/lib/modules/{release}/vmlinux"), true), | |
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also directly call host_path
on the ones that need to be mapped and drop from a vector of tuples to a vector of paths.
collector/lib/rust/host/src/info.rs
Outdated
for location in locations { | ||
let path = if location.1 { | ||
host_path(location.0) | ||
} else { | ||
PathBuf::from(location.0) | ||
}; | ||
|
||
if path.exists() { | ||
return true; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for location in locations { | |
let path = if location.1 { | |
host_path(location.0) | |
} else { | |
PathBuf::from(location.0) | |
}; | |
if path.exists() { | |
return true; | |
} | |
} | |
for (path, on_host) in locations { | |
let path = if on_host { | |
host_path(path) | |
} else { | |
PathBuf::from(path) | |
}; | |
if path.exists() { | |
return true; | |
} | |
} |
} | ||
|
||
impl KernelVersion { | ||
pub fn new(release: &str, version: &str, machine: &str) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need these to be &str? Can't we use straight up String? Most places it looks like we already have those, it would save creating/copying strings.
4ec4d61
to
6c88ace
Compare
Description
Adds a Rust toolchain into Collector, allowing us to write Rust crates and call into them from C++. The idea is that we can gradually increase the amount of Rust in the project and take advantage of its helpful features including memory / thread safety.