search

grafana / beyla

eBPF-based autoinstrumentation of web applications and network metrics
https://grafana.com/oss/beyla-ebpf/
Apache License 2.0
1.23k stars 78 forks source link

Allow customizing the base path and don't mount the BPF fs if it is already mounted #741

Closed dashpole closed 1 month ago

dashpole commented 1 month ago

Fixes https://github.com/grafana/beyla/issues/732

This enables beyla to be run without the SYS_ADMIN capability. See https://github.com/dashpole/beyla/pull/1 for more details.

The code to determine if a mount is a bpf mount is based on https://github.com/cilium/cilium/blob/5130d33a835638c78dda2572d7dc92091ffb3dc1/pkg/mountinfo/mountinfo_linux.go#L27. Importing that would add a dependency on github.com/cilium/cilium, which the project doesn't currently have.

I could:

Let me know which you would prefer, and if there is anything else needed (docs updates, etc) for the change.

mariomac commented 1 month ago

Nice! It looks good to me. What do you think would be the correct approach to provide unit/integration tests for this feature?

mariomac commented 1 month ago

With respect to what to do with the Cilium imported code, I think that copying it and give proper attribution in the file headers is OK. Maybe also update this sentence: https://github.com/grafana/beyla/blob/main/NOTICE#L14-L16

dashpole commented 1 month ago

I've updated the copyright headers and the NOTICE file.

For unit/integration testing, i've followed the model used by cilium, which is to have some unit tests that are run as privileged. I have unit tests for IsMountFS, as well as mountBpfPinPath and unmountBpfPinPath that are only run when the PRIVILEGED_TESTS env var is set. I've added a test-privileged case to the Makefile, which can be run with sudo make test-privileged.

But it isn't run as a presubmit right now, as that setup looks complex: https://github.com/cilium/cilium/blob/e3ea2ed7c1250f6f3410c3fe4d6b1f51ea759d65/.github/workflows/conformance-runtime.yaml#L374

grcevski commented 1 month ago

We have an intermittent test issue, I've kicked off a re-run on the integration tests. I think it should pass now...