Skip to content

Commit 8ea7f48

Browse files
committed
Cleanup unused spec ids
Remove pid from attach and do all unloading at unload time and don't attach many times Cleanup rtld test Remove duplicate code Trigger test process to be processed immediately to get rtld.Loader and Attach to happen faster.
1 parent 35a3e68 commit 8ea7f48

File tree

13 files changed

+298
-244
lines changed

13 files changed

+298
-244
lines changed

.github/workflows/unit-test-on-pull-request.yml

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -246,15 +246,27 @@ jobs:
246246
sudo go test ./interpreter/... -v -run "TestIntegration/(node-local-nightly|node-latest)"
247247
248248
distro-qemu-tests:
249-
name: Full distro QEMU tests (kernel ${{ matrix.kernel }})
249+
name: Distro QEMU tests (${{ matrix.kernel }} ${{ matrix.target_arch }})
250250
runs-on: ubuntu-24.04
251251
timeout-minutes: 15
252252
strategy:
253253
matrix:
254-
kernel:
255-
#- 5.10.217 # 5.10 doesn't have bpf cookies
256-
- 5.15.159
257-
- 6.8.10 # Post-6.6, supports multi-uprobe
254+
include:
255+
- { target_arch: amd64, kernel: 5.4.276 }
256+
- { target_arch: amd64, kernel: 5.10.217 }
257+
- { target_arch: amd64, kernel: 5.15.159 }
258+
- { target_arch: amd64, kernel: 6.1.91 }
259+
- { target_arch: amd64, kernel: 6.6.31 }
260+
- { target_arch: amd64, kernel: 6.8.10 }
261+
- { target_arch: amd64, kernel: 6.9.1 }
262+
- { target_arch: amd64, kernel: 6.12.16 }
263+
- { target_arch: amd64, kernel: 6.16 }
264+
265+
# ARM64 (NOTE: older ARM64 kernels are not available in Cilium repos)
266+
- { target_arch: arm64, kernel: 6.6.31 }
267+
- { target_arch: arm64, kernel: 6.8.4 }
268+
- { target_arch: arm64, kernel: 6.9.1 }
269+
- { target_arch: arm64, kernel: 6.12.16 }
258270
steps:
259271
- name: Clone code
260272
uses: actions/checkout@v4
@@ -263,15 +275,32 @@ jobs:
263275
with:
264276
go-version-file: go.mod
265277
cache-dependency-path: go.sum
278+
- name: Set up environment
279+
uses: ./.github/workflows/env
266280
- name: Install dependencies
267281
run: |
268282
sudo apt-get update -y
269-
sudo apt-get install -y qemu-system-x86 debootstrap systemtap-sdt-dev
283+
case "${{ matrix.target_arch }}" in
284+
amd64) sudo apt-get -y install qemu-system-x86;;
285+
arm64) sudo apt-get -y install qemu-system-arm;;
286+
*) echo >&2 "bug: bad arch selected"; exit 1;;
287+
esac
288+
sudo apt-get install -y debootstrap systemtap-sdt-dev
270289
- name: Download kernel
271290
run: |
272291
cd test/distro-qemu
292+
case "${{ matrix.target_arch }}" in
293+
amd64) export QEMU_ARCH=x86_64;;
294+
arm64) export QEMU_ARCH=aarch64;;
295+
*) echo >&2 "bug: bad arch selected"; exit 1;;
296+
esac
273297
./download-kernel.sh ${{ matrix.kernel }}
274-
- name: Run RTLD tests in QEMU
298+
- name: Run Full Distro tests in QEMU
275299
run: |
276300
cd test/distro-qemu
301+
case "${{ matrix.target_arch }}" in
302+
amd64) export QEMU_ARCH=x86_64;;
303+
arm64) export QEMU_ARCH=aarch64;;
304+
*) echo >&2 "bug: bad arch selected"; exit 1;;
305+
esac
277306
./build-and-run.sh ${{ matrix.kernel }}

interpreter/gpu/cuda.go

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ type data struct {
5050
type Instance struct {
5151
interpreter.InstanceStubs
5252
path string
53-
link interpreter.LinkCloser
5453
pid libpf.PID
5554
}
5655

@@ -68,9 +67,9 @@ func Loader(ebpf interpreter.EbpfHandler, info *interpreter.LoaderInfo) (interpr
6867
if err != nil {
6968
return nil, err
7069
}
70+
7171
// We use the existence of the .note.stapsdt section to determine if this is a
72-
// process that has libparcagpucupti.so loaded. Its cheaper and more reliable than loading
73-
// the symbol table.
72+
// process that has libparcagpucupti.so loaded.
7473
probes, err := ef.ParseUSDTProbes()
7574
if err != nil {
7675
return nil, err
@@ -96,7 +95,6 @@ func Loader(ebpf interpreter.EbpfHandler, info *interpreter.LoaderInfo) (interpr
9695
return nil, nil
9796
}
9897

99-
10098
func (d *data) Attach(ebpf interpreter.EbpfHandler, pid libpf.PID, _ libpf.Address,
10199
_ remotememory.RemoteMemory) (interpreter.Instance, error) {
102100
// Maps usdt probe name to ebpf program name.
@@ -115,12 +113,19 @@ func (d *data) Attach(ebpf interpreter.EbpfHandler, pid libpf.PID, _ libpf.Addre
115113
progNames[i] = "usdt_parcagpu_cuda_kernel"
116114
}
117115
}
118-
lc, err := ebpf.AttachUSDTProbes(pid, d.path, "cuda_probe", d.probes, cookies, progNames, true)
119-
if err != nil {
120-
return nil, err
116+
117+
var lc interpreter.LinkCloser
118+
if d.link == nil {
119+
var err error
120+
lc, err = ebpf.AttachUSDTProbes(pid, d.path, "cuda_probe", d.probes, cookies, progNames)
121+
if err != nil {
122+
return nil, err
123+
}
124+
log.Debugf("[cuda] parcagpu USDT probes attached for %s", d.path)
125+
d.link = lc
126+
} else {
127+
log.Debugf("[cuda] parcagpu USDT probes already attached for %s", d.path)
121128
}
122-
log.Debugf("[cuda] parcagpu USDT probes attached for %s", d.path)
123-
d.link = lc
124129

125130
// Create and register fixer for this PID
126131
fixer := &gpuTraceFixer{
@@ -129,24 +134,14 @@ func (d *data) Attach(ebpf interpreter.EbpfHandler, pid libpf.PID, _ libpf.Addre
129134
}
130135

131136
gpuFixers.Store(pid, fixer)
132-
133137
return &Instance{
134-
link: lc,
135138
path: d.path,
136139
pid: pid,
137140
}, nil
138141
}
139142

140-
// Detach removes the fixer for this PID and closes the link if needed.
141143
func (i *Instance) Detach(_ interpreter.EbpfHandler, _ libpf.PID) error {
142144
gpuFixers.Delete(i.pid)
143-
144-
if i.link != nil {
145-
log.Debugf("[cuda] parcagpu USDT probes closed for %s", i.path)
146-
if err := i.link.Detach(); err != nil {
147-
return err
148-
}
149-
}
150145
return nil
151146
}
152147

interpreter/instancestubs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func (m *EbpfHandlerStubs) DeleteProcData(libpf.InterpreterType, libpf.PID) erro
7373
}
7474

7575
func (mockup *EbpfHandlerStubs) AttachUSDTProbes(libpf.PID, string, string, []pfelf.USDTProbe,
76-
[]uint64, []string, bool) (LinkCloser, error) {
76+
[]uint64, []string) (LinkCloser, error) {
7777
return nil, nil
7878
}
7979

interpreter/rtld/rtld.go

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ type data struct {
2222
// instance represents a per-PID instance of the dlopen interpreter
2323
type instance struct {
2424
interpreter.InstanceStubs
25-
lc interpreter.LinkCloser
2625
}
2726

2827
// Loader detects if the ELF file contains the dlopen symbol in its dynamic symbol table
@@ -37,7 +36,6 @@ func Loader(ebpf interpreter.EbpfHandler, info *interpreter.LoaderInfo) (interpr
3736
// Look for the dlopen symbol in the dynamic symbol table
3837
sym, err := ef.LookupSymbol("dlopen")
3938
if err != nil || sym == nil {
40-
// No dlopen symbol found, this library doesn't support dynamic loading
4139
return nil, nil
4240
}
4341

@@ -52,26 +50,21 @@ func Loader(ebpf interpreter.EbpfHandler, info *interpreter.LoaderInfo) (interpr
5250
// Attach attaches the uprobe to the dlopen function
5351
func (d *data) Attach(ebpf interpreter.EbpfHandler, pid libpf.PID, bias libpf.Address,
5452
_ remotememory.RemoteMemory) (interpreter.Instance, error) {
55-
// Attach uprobe to dlopen using the address stored during Loader
56-
lc, err := ebpf.AttachUprobe(pid, d.path, d.address, "uprobe_dlopen")
57-
if err != nil {
58-
return nil, fmt.Errorf("failed to attach uprobe to dlopen: %w", err)
53+
var lc interpreter.LinkCloser
54+
if d.lc == nil {
55+
// Attach uprobe to dlopen using the address stored during Loader
56+
var err error
57+
lc, err = ebpf.AttachUprobe(pid, d.path, d.address, "uprobe_dlopen")
58+
if err != nil {
59+
return nil, fmt.Errorf("failed to attach uprobe to dlopen: %w", err)
60+
}
61+
d.lc = lc
5962
}
6063

6164
log.Debugf("[dlopen] Attached uprobe to dlopen for PID %d on %s at 0x%x",
6265
pid, d.path, d.address)
6366

64-
d.lc = lc
65-
return &instance{lc: lc}, nil
66-
}
67-
68-
// Detach removes the uprobe
69-
func (i *instance) Detach(_ interpreter.EbpfHandler, pid libpf.PID) error {
70-
log.Debugf("[dlopen] Detach called for PID %d", pid)
71-
if i.lc != nil {
72-
return i.lc.Detach()
73-
}
74-
return nil
67+
return &instance{}, nil
7568
}
7669

7770
// Unload cleans up the uprobe link

interpreter/rtld/rtld_test.go

Lines changed: 20 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Copyright The OpenTelemetry Authors
22
// SPDX-License-Identifier: Apache-2.0
33

4-
//go:build amd64 && !integration
5-
64
package rtld_test
75

86
import (
@@ -15,6 +13,7 @@ import (
1513
"github.com/coreos/pkg/dlopen"
1614
log "github.com/sirupsen/logrus"
1715
"github.com/stretchr/testify/require"
16+
"go.opentelemetry.io/ebpf-profiler/libpf"
1817
"go.opentelemetry.io/ebpf-profiler/metrics"
1918
"go.opentelemetry.io/ebpf-profiler/support"
2019
"go.opentelemetry.io/ebpf-profiler/testutils"
@@ -23,22 +22,32 @@ import (
2322
"go.opentelemetry.io/ebpf-profiler/util"
2423
)
2524

26-
func TestIntegration(t *testing.T) {
25+
func test(t *testing.T) {
2726
if !testutils.IsRoot() {
2827
t.Skip("This test requires root privileges")
2928
}
3029

30+
// Enable debug logging for CI debugging
31+
if os.Getenv("DEBUG_TEST") != "" {
32+
log.SetLevel(log.DebugLevel)
33+
}
34+
3135
// Create a context for the tracer
3236
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
3337
defer cancel()
3438

3539
// Start the tracer with all tracers enabled
3640
traceCh, trc := testutils.StartTracer(ctx, t,
37-
tracertypes.AllTracers(),
41+
tracertypes.IncludedTracers(0),
3842
&testutils.MockReporter{},
3943
false)
4044
defer trc.Close()
4145

46+
trc.StartPIDEventProcessor(ctx)
47+
48+
// tickle tihs process to speed things up
49+
trc.ForceProcessPID(libpf.PID(uint32(os.Getpid())))
50+
4251
// Consume traces to prevent blocking
4352
go func() {
4453
for {
@@ -73,70 +82,20 @@ func TestIntegration(t *testing.T) {
7382

7483
// Check that the metric was incremented
7584
return finalCount > initialCount
76-
}, 10*time.Second, 50*time.Millisecond)
85+
}, 10*time.Second, 100*time.Millisecond)
7786
}
7887

79-
func TestIntegrationSingleShot(t *testing.T) {
80-
if !testutils.IsRoot() {
81-
t.Skip("This test requires root privileges")
82-
}
83-
84-
// Enable debug logging for CI debugging
85-
if os.Getenv("DEBUG_TEST") != "" {
86-
log.SetLevel(log.DebugLevel)
87-
}
88+
func TestIntegration(t *testing.T) {
89+
test(t)
90+
}
8891

89-
// Override HasMultiUprobeSupport to force single-shot mode
92+
func TestIntegrationSingleShot(t *testing.T) {
93+
// Override HasMultiUprobeSupport to force single-shot mode on newer kernels.
9094
multiUProbeOverride := false
9195
util.SetTestOnlyMultiUprobeSupport(&multiUProbeOverride)
9296
defer util.SetTestOnlyMultiUprobeSupport(nil)
9397

94-
// Create a context for the tracer
95-
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
96-
defer cancel()
97-
98-
// Start the tracer with all tracers enabled
99-
traceCh, trc := testutils.StartTracer(ctx, t,
100-
tracertypes.AllTracers(),
101-
&testutils.MockReporter{},
102-
false)
103-
defer trc.Close()
104-
105-
// Consume traces to prevent blocking
106-
go func() {
107-
for {
108-
select {
109-
case <-ctx.Done():
110-
return
111-
case <-traceCh:
112-
// Discard traces
113-
}
114-
}
115-
}()
116-
117-
// retry a few times to get the metric, our process has to be detected and
118-
// the dlopen uprobe has to attach.
119-
require.Eventually(t, func() bool {
120-
// Get the initial metric value
121-
initialCount := getEBPFMetricValue(trc, metrics.IDDlopenUprobeHits)
122-
//t.Logf("Initial dlopen uprobe metric count: %d", initialCount)
123-
124-
// Use dlopen to load a shared library
125-
// libm is a standard math library that's always present
126-
lib, err := dlopen.GetHandle([]string{
127-
"/lib/x86_64-linux-gnu/libm.so.6",
128-
"libm.so.6",
129-
})
130-
require.NoError(t, err, "Failed to open libm.so.6")
131-
defer lib.Close()
132-
133-
// Get the metrics after dlopen
134-
finalCount := getEBPFMetricValue(trc, metrics.IDDlopenUprobeHits)
135-
//t.Logf("Final dlopen uprobe metric count: %d", finalCount)
136-
137-
// Check that the metric was incremented
138-
return finalCount > initialCount
139-
}, 10*time.Second, 50*time.Millisecond)
98+
test(t)
14099
}
141100

142101
func getEBPFMetricValue(trc *tracer.Tracer, metricID metrics.MetricID) uint64 {

interpreter/types.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,17 +117,13 @@ type EbpfHandler interface {
117117
// AttachUSDTProbes attaches an eBPF program to USDT probes in the specified binary.
118118
//
119119
// Parameters:
120-
// - pid: The process ID. Required for older kernels (pre-6.6) that cannot attach to shared
121-
// libraries without a PID. On newer kernels with multi-uprobe support, this is ignored
122-
// when probeAll is true.
120+
// - pid: The process ID. Required for getting path to exe via procfs.
123121
// - path: Full path to the binary containing the USDT probes.
124122
// - multiProgName: Name of eBPF program to use for multi-uprobe attachment (newer kernels).
125123
// - probes: The USDT probe definitions to attach to.
126124
// - cookies: Optional cookies to pass to the eBPF program (one per probe, or nil).
127125
// - singleProgNames: eBPF program names for single-shot attachment (older kernels, one
128126
// per probe).
129-
// - probeAll: If true and the kernel supports it, attach to all processes using this
130-
// binary. If false, only attach to the specified pid.
131127
//
132128
// Returns:
133129
// - LinkCloser: A handle to the attached probes. The caller must:
@@ -136,14 +132,13 @@ type EbpfHandler interface {
136132
// 2. Call LinkCloser.Detach() from Instance.Detach() to detach from the specific PID
137133
// 3. Call LinkCloser.Unload() from Data.Unload() to fully clean up the eBPF program
138134
AttachUSDTProbes(pid libpf.PID, path, multiProgName string, probes []pfelf.USDTProbe,
139-
cookies []uint64, singleProgNames []string, probeAll bool) (LinkCloser, error)
135+
cookies []uint64, singleProgNames []string) (LinkCloser, error)
140136

141137
// AttachUprobe attaches an eBPF uprobe to a function at a specific offset in a binary
142138
AttachUprobe(pid libpf.PID, path string, offset uint64, progName string) (LinkCloser, error)
143139
}
144140

145141
type LinkCloser interface {
146-
Detach() error
147142
Unload() error
148143
}
149144

0 commit comments

Comments
 (0)