open-telemetry / opentelemetry-ebpf-profiler

The production-scale datacenter profiler (C/C++, Go, Rust, Python, Java, NodeJS, .NET, PHP, Ruby, Perl, ...)
Apache License 2.0
2.31k stars 244 forks source link

Bundled binaries in repository #111

Closed umanwizard closed 1 month ago

umanwizard commented 1 month ago

Currently the repository bundles (at least) two binaries for the compiled eBPF code: ./support/ebpf/tracer.ebpf.amd64 and ./support/ebpf/tracer.ebpf.arm64

This makes it a bit easier to reuse the profiler as a library (since the client application doesn't need to rebuild the eBPF binaries itself), but it makes it much harder to change the unwinder code: we have to change the code, then remember to build on two different machines (for each architecture) and commit. It also isn't reproducible: who knows what flags compiler version, etc. the blobs in the repo were built with?

I'm opening this issue to discuss whether this design should be changed.

athre0z commented 1 month ago

Related: https://github.com/open-telemetry/opentelemetry-ebpf-profiler/issues/44

It also isn't reproducible: who knows what flags compiler version, etc. the blobs in the repo were built with?

The ones specified in the Makefile. We have validated that the build is reproducible as long as the same version of clang is being used, including when cross compiling.

then remember to build on two different machines (for each architecture)

You can cross-compile it:

cd support/ebpf
make TARGET_ARCH=amd64
make TARGET_ARCH=arm64

That's what we do. But yeah, we'll at the very least have to set up a builder image soon to ensure that everyone ends up using the same version of clang + we can verify it in CI.

umanwizard commented 1 month ago

We have validated that the build is reproducible as long as the same version of clang is being used, including when cross compiling.

Right, but nobody knows if what exists in the repo was actually built using that version of clang and those flags (or even what code it is built with).

Unless there is a step in CI that checks that the code in the repo compiles to something bit-for-bit identical to the checked-in version. If there is not already, I'd suggest adding this.

OK, I think this issue can be considered a duplicate of #44, so I'll close it.