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

[membphis]

What

Keeps the symbol table in release binaries (strip = "symbols" → strip = "debuginfo"), so sampling profilers (cargo-flamegraph, samply, perf) produce readable function-level on-CPU flame graphs against the exact build users run. All DWARF stays out of the shipped artifact. The docker PR job now guards this contract.

Part of #847.

Why

aisix ships as a single binary, and performance problems surface on the build users actually run — asking a user to reproduce on a special diagnostic build is not a workable support model. With strip = "symbols", shipped binaries carried no symbol table at all, so a flame graph taken against a deployment degraded to bare-address towers.

Two earlier revisions of this PR were rejected in review:

1. A separate opt-in [profile.profiling] — rejected because the shipped artifact itself must be the profileable one.
2. debug = "line-tables-only" + strip = "none" on release — rejected on measured cost: with codegen-units = 1 + thin LTO, the line tables alone measure ~142 MB (~200k inlined-instance records), roughly quadrupling the shipped binary for file:line attribution that hotspot work rarely needs.

Final shape: symbol table only. Function-level frames cover field hotspot profiling, and .eh_frame (always present — panic unwinding needs it) keeps DWARF-based stack walking in perf working. When a deep dive needs file:line and inline attribution, an ad-hoc build is one override away (documented in the manifest comment):

CARGO_PROFILE_RELEASE_DEBUG=line-tables-only CARGO_PROFILE_RELEASE_STRIP=none cargo build --release

strip = "debuginfo" is the modern cargo default for this situation, pinned explicitly so the contract survives toolchain-default drift; it also drops the DWARF that the precompiled std would otherwise leak into the binary.

Cost / impact

• Binary size: shipped Linux binary goes 46.7 MiB (pre-PR :dev image baseline, fully stripped) → 53.8 MiB — +7.1 MiB / +15.2%, CI-measured by this PR's docker job. The macOS local build agrees (+6.0 MiB / +14.9%). For contrast, the rejected line-tables revision measured roughly 4x.
• Runtime: none — the symbol table lives in non-loadable sections; codegen flags are untouched (lto = "thin", codegen-units = 1)
• Shipped binaries now expose internal function names; accepted as the support-model tradeoff. Build-path strings in rodata (panic/tracing metadata) ship today already and are unaffected either way; --remap-path-prefix is tracked in Establish the on-CPU profiling workflow: adopt cargo-flamegraph and add a dedicated profiling build profile #847 as a follow-up for both.

CI guard

docker-image.yml's PR path now extracts /usr/local/bin/aisix from the built image, asserts the text-symbol count, and prints the artifact size — a future Dockerfile strip step, RUSTFLAGS change, or base-image swap cannot silently break field profiling again, and every PR records the real Linux artifact cost.

Usage

cargo flamegraph --bin aisix -- <args>   # release is cargo flamegraph's default profile
flamegraph --pid <aisix-pid>             # attach to a running deployment

Verification

• CI: the new "Verify shipped binary keeps its symbol table (Establish the on-CPU profiling workflow: adopt cargo-flamegraph and add a dedicated profiling build profile #847)" step passed on this head — Linux artifact 56,424,392 bytes, 42,290 text symbols, .eh_frame/.eh_frame_hdr present (perf's DWARF unwinding needs them)
• Local: full release build (46.4 MiB, 42,029 text symbols on macOS), then an end-to-end profiling session against this exact build — mock upstream, 32-connection load, 380k requests at 12.7k req/s, cargo flamegraph produced a fully symbolized graph: 2,747 unique frames, zero address-only frames, 269 aisix-crate frames (aisix_proxy::chat::*, aisix_obs::metrics::*, provider bridge frames all readable)

The remaining #847 checklist items (a Linux perf session, rust-lld --no-rosegment verification, the internal guide) stay tracked in the issue.

[coderabbitai]

📝 Walkthrough

Walkthrough

Updates release builds to retain line-table and symbol information, and adds pull-request verification that the container’s aisix binary preserves profiling data.

Changes

Profiling-ready release artifacts

Layer / File(s)	Summary
Update release profiling settings Cargo.toml	The release profile retains line tables and disables stripping; the separate profiling profile is removed.
Verify container binary profiling data .github/workflows/docker-image.yml	Pull-request workflow checks the container binary’s symbol table and DWARF debug sections.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: jarvis9443, moonming

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
E2e Test Quality Review	✅ Passed	PASS: The PR-only check exercises the real Docker build-to-image path and validates the shipped binary inside the container with clear, explicit assertions.
Security Check	✅ Passed	No security issues found: the changes only alter build/debug settings and add CI checks; they don't log, persist, or expose secrets.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: retaining symbols in release binaries for profiling.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch profiling-build-profile

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

.github/workflows/docker-image.yml (1)

201-205: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Pass action output through env before using it in Bash.

Line 205 expands the metadata output directly into generated shell source. GitHub recommends environment variables for expression values in inline scripts; the current PR tag configuration normally yields a sanitized pr-<number>, so this is future-proofing rather than a demonstrated exploit. (docs.github.com)

Suggested hardening

       - name: Verify shipped binary keeps symbols + line tables (`#847`)
+        env:
+          IMAGE_TAGS: ${{ steps.meta.outputs.tags }}
         if: github.event_name == 'pull_request'
         run: |
           set -eux
-          IMAGE="$(printf '%s\n' "${{ steps.meta.outputs.tags }}" | head -n1)"
+          IMAGE="$(printf '%s\n' "$IMAGE_TAGS" | head -n1)"

🤖 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 201 - 205, Update the
“Verify shipped binary keeps symbols + line tables (`#847`)” step to pass
steps.meta.outputs.tags through the step’s env configuration, then read that
environment variable in the Bash command instead of interpolating the GitHub
expression directly into shell source.

Source: Linters/SAST tools

🤖 Prompt for all review comments with 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.

Inline comments:
In @.github/workflows/docker-image.yml:
- Around line 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.

---

Nitpick comments:
In @.github/workflows/docker-image.yml:
- Around line 201-205: Update the “Verify shipped binary keeps symbols + line
tables (`#847`)” step to pass steps.meta.outputs.tags through the step’s env
configuration, then read that environment variable in the Bash command instead
of interpolating the GitHub expression directly into shell source.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 59d54687-8384-4c65-8851-82d660c8c892

📥 Commits

Reviewing files that changed from the base of the PR and between 57530dd and fafa2f1.

📒 Files selected for processing (2)

• .github/workflows/docker-image.yml
• Cargo.toml

[coderabbitai]

🎯 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