Skip to content

build: keep the symbol table in release binaries for field profiling #849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
membphis merged 5 commits into from
Jul 30, 2026
Merged

build: keep the symbol table in release binaries for field profiling #849

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
57530dd
build: add a profiling cargo profile for on-CPU flame graphs
membphis Jul 30, 2026
1dc1587
build: add a profiling cargo profile for readable flame graphs
membphis Jul 30, 2026
82a4bbb
build: keep symbols and line tables in release binaries
membphis Jul 30, 2026
fafa2f1
ci: guard the shipped binary's profiling contract
membphis Jul 30, 2026
038524c
build: keep only the symbol table in release binaries
membphis Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .github/workflows/docker-image.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,24 @@ jobs:
docker logs smoke-aisix
test -n "$ok"

# The profiling contract from #847: the shipped binary must stay
# profileable in the field, meaning the symbol table and the DWARF
# line tables survive into the image. A future strip step,
# RUSTFLAGS change, or base-image swap would break flame graphs
# silently, so pin it here — and print the size so every PR
# records the real Linux artifact cost.
- name: Verify shipped binary keeps symbols + line tables (#847)
if: github.event_name == 'pull_request'
run: |
set -eux
IMAGE="$(printf '%s\n' "${{ steps.meta.outputs.tags }}" | head -n1)"
cid="$(docker create "$IMAGE")"
docker cp "$cid:/usr/local/bin/aisix" /tmp/aisix-shipped
docker rm "$cid"
ls -l /tmp/aisix-shipped
test "$(nm /tmp/aisix-shipped | grep -c ' [tT] ')" -gt 1000
readelf -S /tmp/aisix-shipped | grep -E '\.debug_line|\.debug_info|\.symtab'

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Require both profiling sections independently.

The alternation on Line 211 passes when only one of .debug_line, .debug_info, or .symtab exists. A binary missing DWARF line tables could therefore pass the profiling guard.

Suggested fix 
-          readelf -S /tmp/aisix-shipped | grep -E '\.debug_line|\.debug_info|\.symtab'
+          readelf -S /tmp/aisix-shipped | grep -q '\.symtab'
+          readelf -S /tmp/aisix-shipped | grep -q '\.debug_line'

As per coding guidelines, verification tests must define verifiable success criteria for every required artifact.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test "$(nm /tmp/aisix-shipped | grep -c ' [tT] ')" -gt 1000
readelf -S /tmp/aisix-shipped | grep -E '\.debug_line|\.debug_info|\.symtab'
test "$(nm /tmp/aisix-shipped | grep -c ' [tT] ')" -gt 1000
readelf -S /tmp/aisix-shipped | grep -q '\.symtab'
readelf -S /tmp/aisix-shipped | grep -q '\.debug_line'
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/docker-image.yml around lines 210 - 211, Update the
profiling verification command near the existing nm check so `.debug_line`,
`.debug_info`, and `.symtab` are each validated independently rather than
through one alternation-based grep. Require every artifact check to succeed
while preserving the existing symbol-count validation.

Source: Coding guidelines


- name: Install cosign
if: github.event_name != 'pull_request'
uses: sigstore/cosign-installer@v3
Expand Down
6 changes: 5 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,14 @@ wiremock = "0.6"
testcontainers = "0.23"
tempfile = "3.13"

# Shipped binaries must stay profileable in the field (perf / cargo
# flamegraph against the very build a user runs), so keep the symbol
# table and DWARF line tables in release artifacts. See issue #847.
[profile.release]
lto = "thin"
codegen-units = 1
strip = "symbols"
debug = "line-tables-only"
strip = "none"

[profile.coverage]
inherits = "dev"
Expand Down
Loading