Skip to content

Commit e6e6070

Browse files
committed
Catch up with Go labels review
1 parent ee16e4e commit e6e6070

5 files changed

Lines changed: 65 additions & 68 deletions

File tree

.github/workflows/env/action.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ runs:
3030
sudo apt-get install -y curl unzip gcc-aarch64-linux-gnu \
3131
libc6-arm64-cross qemu-user-binfmt libc6:arm64 \
3232
musl-dev:amd64 musl-dev:arm64 musl-tools binutils-aarch64-linux-gnu
33+
- name: Set up Go
34+
uses: actions/setup-go@v5
35+
with:
36+
go-version-file: go.mod
37+
cache-dependency-path: go.sum
38+
id: go
3339
- name: Install Rust
3440
uses: dtolnay/rust-toolchain@stable
3541
with:

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

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,6 @@ jobs:
1515
uses: actions/checkout@v4
1616
- name: Set up environment
1717
uses: ./.github/workflows/env
18-
- name: Set up Go
19-
uses: actions/setup-go@v5
20-
with:
21-
go-version-file: go.mod
22-
check-latest: true
23-
cache-dependency-path: go.sum
24-
id: go
2518
- name: Check for changes in licenses of dependencies
2619
run: |
2720
make legal
@@ -41,13 +34,6 @@ jobs:
4134
uses: actions/checkout@v4
4235
- name: Set up environment
4336
uses: ./.github/workflows/env
44-
- name: Set up Go
45-
uses: actions/setup-go@v5
46-
with:
47-
go-version-file: go.mod
48-
check-latest: true
49-
cache-dependency-path: go.sum
50-
id: go
5137
- name: Get linter version
5238
id: linter-version
5339
run: (echo -n "version="; make linter-version) >> "$GITHUB_OUTPUT"
@@ -75,13 +61,6 @@ jobs:
7561
uses: actions/checkout@v4
7662
- name: Set up environment
7763
uses: ./.github/workflows/env
78-
- name: Set up Go
79-
uses: actions/setup-go@v5
80-
with:
81-
go-version-file: go.mod
82-
check-latest: true
83-
cache-dependency-path: go.sum
84-
id: go
8564
- name: Cache coredump modules
8665
uses: actions/cache@v4
8766
with:
@@ -94,6 +73,8 @@ jobs:
9473
run: make rust-tests
9574
- name: Tests
9675
run: make test TARGET_ARCH=${{ matrix.target_arch }}
76+
- name: Sudo Tests
77+
run: make sudo-test TARGET_ARCH=${{ matrix.target_arch }}
9778

9879
check-binary-blobs:
9980
name: Check for differences in the eBPF and Rust binary blobs
@@ -128,21 +109,13 @@ jobs:
128109
fi
129110
130111
build-integration-test-binaries:
131-
name: Build integration test binaries (${{ matrix.target_arch }}-${{ matrix.go_version}})
112+
name: Build integration test binaries (${{ matrix.target_arch }})
132113
runs-on: ubuntu-24.04
133114
timeout-minutes: 10
134115
strategy:
135116
matrix:
136117
target_arch: [amd64, arm64]
137-
go_version: [1.23, 1.24]
138118
steps:
139-
- name: Set up Go
140-
uses: actions/setup-go@v5
141-
with:
142-
go-version: ${{ matrix.go_version }}
143-
check-latest: true
144-
cache-dependency-path: go.sum
145-
id: go
146119
- name: Clone code
147120
uses: actions/checkout@v4
148121
- name: Set up environment
@@ -152,17 +125,16 @@ jobs:
152125
- name: Upload integration test binaries
153126
uses: actions/upload-artifact@v4
154127
with:
155-
name: integration-test-binaries-${{ matrix.target_arch }}-${{ matrix.go_version}}
128+
name: integration-test-binaries-${{ matrix.target_arch }}
156129
path: support/*.test
157130

158131
integration-tests:
159-
name: Integration tests (v${{ matrix.kernel }} ${{ matrix.target_arch }} ${{ matrix.go_version}})
132+
name: Integration tests (v${{ matrix.kernel }} ${{ matrix.target_arch }})
160133
runs-on: ubuntu-24.04
161134
needs: build-integration-test-binaries
162135
timeout-minutes: 10
163136
strategy:
164137
matrix:
165-
go_version: [1.23, 1.24]
166138
include:
167139
# List of available kernels here:
168140
# https://github.com/cilium/ci-kernels/pkgs/container/ci-kernels/versions?filters%5Bversion_type%5D=tagged
@@ -186,13 +158,6 @@ jobs:
186158
steps:
187159
- name: Clone code
188160
uses: actions/checkout@v4
189-
- name: Set up Go
190-
uses: actions/setup-go@v5
191-
with:
192-
go-version: ${{ matrix.go_version }}
193-
check-latest: true
194-
cache-dependency-path: go.sum
195-
id: go
196161
- name: Install dependencies
197162
run: |
198163
sudo apt-get update -y
@@ -205,7 +170,7 @@ jobs:
205170
sudo mv ~/go/bin/bluebox /usr/local/bin/.
206171
- name: Fetch integration test binaries
207172
uses: actions/download-artifact@v4
208-
with: { name: "integration-test-binaries-${{ matrix.target_arch }}-${{ matrix.go_version}}" }
173+
with: { name: "integration-test-binaries-${{ matrix.target_arch }}" }
209174
- name: Fetch precompiled kernel
210175
run: |
211176
install -d ci-kernels

Makefile

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@ vanity-import-fix: $(PORTO)
117117
test: generate ebpf test-deps
118118
go test $(GO_FLAGS) -tags $(GO_TAGS) ./...
119119

120+
sudo-test: integration-test-binaries
121+
(cd support && sudo ./go_labels.test -test.v)
122+
120123
TESTDATA_DIRS:= \
121124
nativeunwind/elfunwindinfo/testdata \
122125
libpf/pfelf/testdata \
@@ -129,9 +132,14 @@ test-deps:
129132

130133
TEST_INTEGRATION_BINARY_DIRS := tracer processmanager/ebpf support go_labels
131134

132-
integration-test-binaries: generate ebpf
133-
# Call it a ".test" even though it isn't to get included into bluebox initramfs
134-
go build -o ./support/go_labels_canary.test ./go_labels
135+
# These binaries are named ".test" to get included into bluebox initramfs
136+
support/go_labels_canary1.23.test: ./go_labels/*.go
137+
GOTOOLCHAIN=go1.23.7 go build -tags $(GO_TAGS) -o $@ ./go_labels
138+
139+
support/go_labels_canary1.24.test: ./go_labels/*.go
140+
GOTOOLCHAIN=go1.24.1 go build -tags $(GO_TAGS) -o $@ ./go_labels
141+
142+
integration-test-binaries: generate ebpf support/go_labels_canary1.23.test support/go_labels_canary1.24.test
135143
$(foreach test_name, $(TEST_INTEGRATION_BINARY_DIRS), \
136144
(go test -ldflags='-extldflags=-static' -trimpath -c \
137145
-tags $(GO_TAGS),static_build,integration \

go_labels/main.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ func randomString(n int) string {
2525
// to work with qemu/bluebox testing harness. A statically linked go test built binary doesn't
2626
// work with the go labels extractor ebpf program, not sure yet if this is a bug.
2727
func main() {
28-
labels := pprof.Labels("l1", randomString(16), "l2", randomString(16), "l3", randomString(16))
28+
labels := pprof.Labels(
29+
"l1", "label1"+randomString(16),
30+
"l2", "label2"+randomString(24),
31+
"l3", "label3"+randomString(48))
2932
lastUpdate := time.Now()
3033
pprof.Do(context.TODO(), labels, func(context.Context) {
3134
//nolint:revive

go_labels/main_test.go

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"context"
77
"os"
88
"os/exec"
9+
"strings"
910
"testing"
1011

1112
"github.com/stretchr/testify/require"
@@ -18,33 +19,47 @@ func TestGoCustomLabels(t *testing.T) {
1819
t.Skip("root privileges required")
1920
}
2021

21-
ctx, cancel := context.WithCancel(context.Background())
22-
2322
r := &testutils.MockReporter{}
2423
enabledTracers, _ := tracertypes.Parse("")
2524
enabledTracers.Enable(tracertypes.GoLabels)
26-
traceCh, _ := testutils.StartTracer(ctx, t, enabledTracers, r)
27-
28-
// Use a separate exe for getting labels as the bpf code doesn't seem to work with
29-
// go test static binaries at the moment, not clear if that's a problem with the bpf
30-
// code or a bug/fact of life for static go binaries and getting g from TLS.
31-
cmd := exec.Command("./go_labels_canary.test")
32-
err := cmd.Start()
33-
require.NoError(t, err)
25+
traceCh, _ := testutils.StartTracer(context.Background(), t, enabledTracers, r)
26+
for _, tc := range []string{
27+
"./go_labels_canary1.23.test",
28+
"./go_labels_canary1.24.test",
29+
} {
30+
t.Run(tc, func(t *testing.T) {
31+
// Use a separate exe for getting labels as the bpf code doesn't seem to work with
32+
// go test static binaries at the moment, not clear if that's a problem with the bpf
33+
// code or a bug/fact of life for static go binaries and getting g from TLS.
34+
cmd := exec.Command(tc)
35+
err := cmd.Start()
36+
require.NoError(t, err)
3437

35-
// Wait 1 second for traces to arrive.
36-
for trace := range traceCh {
37-
if trace == nil {
38-
continue
39-
}
40-
if len(trace.CustomLabels) > 0 {
41-
for k, v := range trace.CustomLabels {
42-
t.Logf("Custom label: %s=%s", k, v)
38+
for trace := range traceCh {
39+
if trace == nil {
40+
continue
41+
}
42+
if len(trace.CustomLabels) > 0 {
43+
for k, v := range trace.CustomLabels {
44+
switch k {
45+
case "l1":
46+
require.Len(t, v, 22)
47+
require.True(t, strings.HasPrefix(v, "label1"))
48+
case "l2":
49+
require.Len(t, v, 30)
50+
require.True(t, strings.HasPrefix(v, "label2"))
51+
case "l3":
52+
require.Len(t, v, 47)
53+
require.True(t, strings.HasPrefix(v, "label3"))
54+
default:
55+
t.Fail()
56+
}
57+
}
58+
break
59+
}
4360
}
44-
break
45-
}
61+
_ = cmd.Process.Signal(os.Kill)
62+
_ = cmd.Wait()
63+
})
4664
}
47-
cancel()
48-
_ = cmd.Process.Signal(os.Kill)
49-
_ = cmd.Wait()
5065
}

0 commit comments

Comments
 (0)