Skip to content

Add Rust native symbolization library and C API wrapper#267

Merged
christos68k merged 2 commits intomainfrom
ck/symblib
Jan 24, 2025
Merged

Add Rust native symbolization library and C API wrapper#267
christos68k merged 2 commits intomainfrom
ck/symblib

Conversation

@christos68k
Copy link
Copy Markdown
Member

@christos68k christos68k commented Dec 6, 2024

This PR adds the Rust native symbolization framework that we developed at Elastic and now making available in OpenTelemetry as part of the ebpf-profiler project. We can use it to implement automatic on-target symbolization of native binaries (e.g. Go executables) but also to enable an OTel bulk native symbol upload protocol that we (Profiling SIG) can come together and specify.

Building through Cargo works and all tests are passing.

NOTE: This is the initial PR that introduces the Rust symbolization framework but does not resolve packaging and artifact questions. We may decide to split this out into a separate repository post-merge.

@christos68k christos68k requested review from a team as code owners December 6, 2024 20:50
@christos68k christos68k requested a review from athre0z December 6, 2024 20:54
@christos68k christos68k changed the title Add Rust native symbolization library and C wrapper Add Rust native symbolization library and C API wrapper Dec 6, 2024
@korniltsev
Copy link
Copy Markdown
Contributor

nit: Can we add target/ to .gitignore ?

christos68k and others added 2 commits January 2, 2025 12:27
This commit adds the Rust native symbolization framework that we developed
at Elastic and now making available in OpenTelemetry as part of the
ebpf-profiler project. We can use it to implement automatic on-target
symbolization of native binaries (e.g. Go executables) but also to enable an
OpenTelemetry bulk native symbol upload protocol.

Co-authored-by: Joel Höner <joel@elastic.co>
Co-authored-by: Victor Michel <victor.michel@elastic.co>
Co-authored-by: Florian Lehner <florian.lehner@elastic.co>
Co-authored-by: Tim Rühsen <tim.ruhsen@elastic.co>
Co-authored-by: Davide Girardi <davide.girardi@elastic.co>
Copy link
Copy Markdown
Member

@athre0z athre0z left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks so much for going through the effort of open-sourcing it!

NOTE: This is the initial PR that introduces the Rust symbolization framework but does not resolve packaging and artifact questions. We may decide to split this out into a separate repository post-merge.

Yeah: it would be best to have this in a separate repo at some point, but I also understand that this simplifies the contribution / donation process, so temporarily having it here is fine with me.

Copy link
Copy Markdown
Contributor

@korniltsev korniltsev left a comment

Choose a reason for hiding this comment

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

<3 Thank you for open sourcing this!

}
};

eprintln!("Processing {:?}", &unit);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
eprintln!("Processing {:?}", &unit);
debug!("Processing {:?}", &unit);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There are a few such statements, leaving as-is for now and can switch to debug! once we start using the library from the agent.

Comment on lines +27 to +37
impl From<SymblibString> for Option<String> {
fn from(maybe_str: SymblibString) -> Self {
if maybe_str.0.is_null() {
None
} else {
let cstr = unsafe { CString::from_raw(maybe_str.0) };
mem::forget(maybe_str);
Some(cstr.into_string().unwrap())
}
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
impl From<SymblibString> for Option<String> {
fn from(maybe_str: SymblibString) -> Self {
if maybe_str.0.is_null() {
None
} else {
let cstr = unsafe { CString::from_raw(maybe_str.0) };
mem::forget(maybe_str);
Some(cstr.into_string().unwrap())
}
}
}

I think it is not used.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Leaving it in for now, can revisit in the future.

/// every range found in the executable. The user_data pointer is passed to
/// the visitor untouched and may be NULL.
#[no_mangle]
pub unsafe extern "C" fn symblib_rangeextr(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know this first iteration is not immediately intended to be used in the profiler itself, however since there are ideas/plans to do this in the future I will just dump my thougths.
This API is not very convenient to use for ontarget symbolization because paths will likely be relative to /proc/{pid}/root. Maybe we can add an extra argument rootfs so that resolving debugaltlink is resolved in the same root fs. Or maybe as an alternative we can create an alternative API accepting file descriptors, so that rust lib does not need to worry about rootfs at all.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, feel free to open an issue so that this doesn't get lost in the noise.

symbfile format
===============

`symbfile` is our custom file format for efficiently storing large amounts of
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please correct me if I'm wrong, but symbfile format seems not suitable for quick address => symbol lookups, only for storing/transfer. Are there any plans to opensource the lookup data structure for use?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

symbfile is meant to be used for storage/transfer of symbol data. Efficient point lookups akin to blazesym is something we need to add to symblib. Essentially, we need both point lookups and bulk symbol extraction. Right now symblib does the latter but not the former and we're discussing how to proceed on getting that implemented.

@@ -0,0 +1,145 @@
// Copyright The OpenTelemetry Authors
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this file was hand written, but it may worth generating the header using cbindgen to be always in sync with rust code

///
/// Currently implemented using the Zydis library.
#[derive(Debug)]
pub struct Amd64InstrDecoder(zydis::Decoder);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you considered implementing small subset of the disassembler in a rust?
Asking because of security reasons. I would be cautious to run a disassembler written in C on potentially untrusted user data, especially if running as root during ontarget symbolization.

It would be nice at least to document somehow, that the amd64 retpad extractor is using C library for security visibility.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Other than with ARM64, it's unfortunately not possible to implement a subset of the disassembler for AMD64. It's a variable length instruction set, so you need to be able to correctly decode each instruction in the program in order to find the start of the next one.

@christos68k christos68k merged commit edb1430 into main Jan 24, 2025
@christos68k christos68k deleted the ck/symblib branch January 24, 2025 21:30
florianl added a commit that referenced this pull request Feb 17, 2025
Use macro debug instead of eprintln for debug messages.

Follow up to #267 (comment).

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
bhavnajindal added a commit to instana/opentelemetry-ebpf-profiler that referenced this pull request Mar 12, 2025
Sync from upstream (2025-03-12)

Florian Lehner <florianl@users.noreply.github.com> symblib: expose API for single point lookups (open-telemetry#380)
Co-authored-by: GitHub <noreply@github.com>
Tolya Korniltsev <korniltsev.anatoly@gmail.com> chore: remove unused controller.Config fields (open-telemetry#387)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> libpf: drop unused code (open-telemetry#386)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> tracehandler: drop metadataWarnInhib (open-telemetry#385)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> Go: update to go.opentelemetry.io/otel@v1.35.0 (open-telemetry#383)
Co-authored-by: GitHub <noreply@github.com>
Christos Kalkanis <christos.kalkanis@elastic.co> processmanager: Don't synchronize a process that's waiting cleanup (open-telemetry#379)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> CI: use latest LTS kernel in tests (open-telemetry#382)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> Makefile: add cargo clean to target clean (open-telemetry#381)
Co-authored-by: GitHub <noreply@github.com>
Christos Kalkanis <christos.kalkanis@elastic.co> Switch semantics for process.executable.name (open-telemetry#306)
Co-authored-by: GitHub <noreply@github.com>
Tim Rühsen <tim.ruhsen@elastic.co> Stabilize CI / integration tests (open-telemetry#378)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> Docker fixup (open-telemetry#375)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> Docker: fix rust set up (open-telemetry#371)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> tracer: attach to all kprobes with prefix for off CPU profiling (open-telemetry#370)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> Go: update to Go 1.23 (open-telemetry#372)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> support: generate *ProcInfo types with cgo (open-telemetry#367)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> process: reuse and preallocate memory (open-telemetry#355)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> rust: preparations to integrate Rust (open-telemetry#360)
Co-authored-by: GitHub <noreply@github.com>
Christos Kalkanis <christos.kalkanis@elastic.co> Switch to OTel metrics (open-telemetry#348)
Co-authored-by: GitHub <noreply@github.com>
Tolya Korniltsev <korniltsev.anatoly@gmail.com> cargo: remove unused workspace dependency declarations (open-telemetry#364)
Co-authored-by: GitHub <noreply@github.com>
Tolya Korniltsev <korniltsev.anatoly@gmail.com> reporter: add custom gRPC dial options (open-telemetry#363)
Co-authored-by: GitHub <noreply@github.com>
umanwizard <brennan@umanwizard.com> Various fixes to node/V8 (open-telemetry#333)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> doc: fix path of tooling (open-telemetry#361)
Co-authored-by: GitHub <noreply@github.com>
OpenTelemetry Bot <107717825+opentelemetrybot@users.noreply.github.com> Add FOSSA scanning workflow (open-telemetry#357)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> rust: use macro for debug output (open-telemetry#356)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> symblib/gosym: add single point lookup (open-telemetry#346)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> README: provide devfiler v0.14.0 (open-telemetry#354)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> CI: skip environment setup (open-telemetry#353)
Co-authored-by: GitHub <noreply@github.com>
Richard Chukwu <79311274+RichardChukwu@users.noreply.github.com> Improve contributor guide (open-telemetry#349)
Co-authored-by: GitHub <noreply@github.com>
Christos Kalkanis <christos.kalkanis@elastic.co> Fix build (open-telemetry#350)
Co-authored-by: GitHub <noreply@github.com>
Christos Kalkanis <christos.kalkanis@elastic.co> processinfo: refactor process metadata (open-telemetry#344)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> reporter/pdata: do no generate profiles if there are no events (open-telemetry#347)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> README: provide devfiler v0.13.0 (open-telemetry#343)
Co-authored-by: GitHub <noreply@github.com>
Christos Kalkanis <christos.kalkanis@elastic.co> processmanager: Fix process exit regression (open-telemetry#337) (open-telemetry#338)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> libpf: drop Hash64 (open-telemetry#340)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> cargo: set license field (open-telemetry#336)
Co-authored-by: GitHub <noreply@github.com>
Damien Mathieu <42@dmathieu.com> Use dummy support for any non-arm64 and non-amd64 archs (open-telemetry#335)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> rust: drop anyhow dependency (open-telemetry#334)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> support: use cgo to generate Go constants from eBPF (open-telemetry#332)
Co-authored-by: GitHub <noreply@github.com>
Christos Kalkanis <christos.kalkanis@elastic.co> processmanager: Don't log inside critical areas (open-telemetry#328)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> CI: add test for Rust components (open-telemetry#326)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> processmanager: simplify API and return early (open-telemetry#325)
Co-authored-by: GitHub <noreply@github.com>
Christos Kalkanis <christos.kalkanis@elastic.co> Add Rust native symbolization library and C API wrapper (open-telemetry#267)
Co-authored-by: GitHub <noreply@github.com>
Christos Kalkanis <christos.kalkanis@elastic.co> Metrics for trace event perf event monitor (open-telemetry#322)
Co-authored-by: GitHub <noreply@github.com>
Christos Kalkanis <christos.kalkanis@elastic.co> Delayed processing for ProcessManager.pidToProcessInfo (open-telemetry#321)
Co-authored-by: GitHub <noreply@github.com>
Christos Kalkanis <christos.kalkanis@elastic.co> Rework SymbolizationComplete (open-telemetry#307)
Co-authored-by: GitHub <noreply@github.com>
Tim Rühsen <tim.ruhsen@elastic.co> Amend -off-cpu-threshold value (open-telemetry#316)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> reporter/collector: fix reporting issue (open-telemetry#319)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> reporter: move pkg samples from internal to public (open-telemetry#314)
Co-authored-by: GitHub <noreply@github.com>
Florian Lehner <florianl@users.noreply.github.com> README: provide devfiler v0.11.0 (open-telemetry#313)
Co-authored-by: GitHub <noreply@github.com>
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.

6 participants