Skip to content

feat(frontend): resolve symbol-ref trees after the final merge#5334

Open
jake-kramer wants to merge 5 commits into
symbolizer/symbolref-querybackendfrom
symbolizer/symbolref-frontend
Open

feat(frontend): resolve symbol-ref trees after the final merge#5334
jake-kramer wants to merge 5 commits into
symbolizer/symbolref-querybackendfrom
symbolizer/symbolref-frontend

Conversation

@jake-kramer

@jake-kramer jake-kramer commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Part of a series: symbol-aware tree references (6 of 8)

Full context and series overview: #5317 (PR 1). Compact map:

  1. symbolizer: expose address-level Resolve API (feat(symbolizer): expose address-level symbol resolution as a public API #5317)
  2. proto: SymbolRefTable + tree query/report fields (feat(query): add SymbolRefTable and symbol_refs tree fields to the query API #5318)
  3. model/symbolref: intern table + merge rebasing (feat(model): add symbolref package for symbol-aware tree references #5321)
  4. model/symbolref: unresolved-binary grouping + tree rebuild (feat(model): add unresolved-binary grouping and tree rebuild to symbolref #5322)
  5. symdb: symbol-ref tree resolver output (feat(symdb): add symbol-ref tree resolver output #5332) · querybackend: query mode + aggregation (feat(querybackend): produce and aggregate symbol-ref tree reports #5333)
  6. frontend: orchestration behind a default-off per-tenant flag ← this PR
  7. integration tests + docs (test(symbolizer): add symbol-ref tree integration tests and docs #5335)
  8. symdb: compaction preserves line-less locations (fix(symdb): preserve line-less locations when rewriting partitions without functions #5337) — stacked at the bottom of the series, merges first

Tracking issue: grafana/pyroscope-squad#487. Based on #5333's branch.

What

The activation PR. Behind a new per-tenant flag symbolizer.symbol-ref-trees-enabled (default off), tree and span-profile queries request symbol-ref tree reports instead of being rewritten through the pprof path when symbolization is needed. With the flag off, request construction and the legacy symbolization path are byte-identical to today — that's the rollback story.

When a merged report comes back with unresolved references, the frontend:

  • groups them by binary (symbolref.UnresolvedBinaries),
  • resolves each through the symbolizer's address-level API under bounded concurrency (the existing max-debuginfod-concurrency, shared with the pprof path) and a per-binary timeout (symbolizer.resolve-timeout, default 20s),
  • rebuilds the final tree (symbolref.Rebuild), rendering the existing binary!0xaddr fallback for anything unresolved.

Error semantics: a per-binary timeout degrades that one binary to fallbacks and lets the query succeed; cancellation of the request context fails the query, matching how SymbolizePprof errors are already handled at these call sites.

Observability

A single unsampled counter, symbol_ref_locations_total{result=resolved|miss|timeout}, drives the fallback-rate signal (an alert can't be built on sampled traces). The per-request detail — resolution timing, binary count, unresolved-reference count — lives on a resolveSymbolRefs tracing span instead of dedicated metrics.

Testing

go test ./pkg/frontend/... ./pkg/validation/... -race green. Tests cover: flag off ⇒ byte-identical legacy behavior (existing shouldSymbolize tests untouched); flag on ⇒ symbol-ref request, no legacy wrapper; old-backend/degenerate responses pass through; per-buildID resolution with sorted/deduped addresses and fallback on miss; per-binary timeout ⇒ fallback + query succeeds; request-context cancellation ⇒ query fails; multi-tenant all-tenants gating. golangci-lint clean; whole-repo build clean.

@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-querybackend branch from b35585e to ed670ba Compare July 9, 2026 20:53
@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-frontend branch from 445de3d to 2c09212 Compare July 9, 2026 20:53
@jake-kramer
jake-kramer marked this pull request as ready for review July 9, 2026 21:10
@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-frontend branch from 2c09212 to 8507190 Compare July 13, 2026 14:58
@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-querybackend branch from ed670ba to 0be5140 Compare July 13, 2026 14:58
@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-frontend branch from 8507190 to 6e124dc Compare July 13, 2026 19:05
@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-querybackend branch 2 times, most recently from 08e755d to cf93b5b Compare July 13, 2026 19:27
@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-frontend branch from 6e124dc to 4fa35d0 Compare July 13, 2026 19:27

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4fa35d0. Configure here.

Comment thread pkg/frontend/readpath/queryfrontend/symbol_ref_resolve.go
@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-frontend branch from 4fa35d0 to ae84568 Compare July 13, 2026 19:48
@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-querybackend branch 2 times, most recently from cf67a51 to c2b9c2a Compare July 14, 2026 19:46
@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-frontend branch from ae84568 to 2585589 Compare July 14, 2026 19:46
Comment thread pkg/frontend/readpath/queryfrontend/symbolizer.go Outdated
jake-kramer and others added 5 commits July 15, 2026 21:50
Behind a new per-tenant flag (symbolizer.symbol-ref-trees-enabled,
default off), tree and span-profile queries request symbol-ref tree
reports instead of being rewritten through the pprof path when
symbolization is needed. When a merged report carries unresolved
references, the frontend groups them by binary, resolves each through
the symbolizer's address-level API under bounded concurrency and a
per-binary timeout, and rebuilds the final tree -- unresolved
locations render the existing binary!0xaddr fallback. A per-binary
timeout degrades that binary to fallbacks; cancellation of the request
context fails the request, matching existing symbolization error
handling. The concurrency bound comes from the existing
max-debuginfod-concurrency configuration, shared with the pprof path.

With the flag off, request construction and the legacy symbolization
path are byte-identical to today.

Part of grafana/pyroscope-squad#487.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
-symbolizer.symbol-ref-trees-enabled and -symbolizer.resolve-timeout
appear in -help-all, which TestHelp pins against cmd/pyroscope/
help*.txt.tmpl. Regenerate the golden so TestHelp passes; the flags are
doc:"hidden" so the reference docs were unaffected, but the raw help
output was not.

Part of grafana/pyroscope-squad#487.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rder

lidia's Lookup returns inline chains innermost-first, mirroring pprof
Line order, while symbolref.Rebuild splices resolved chains into rebuilt
stacks root-first, outermost caller first — the order its own tests pin.
buildLookup copied chains through unchanged, so multi-frame inlined
locations rendered with parent and child flipped relative to the legacy
pprof path. Reverse at the lookup boundary.

Reported by cursor bugbot.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
useSymbolRefTrees gated only on the symbol-ref feature flag, so a
tenant with symbolization disabled could still be routed onto the
symbol-ref path. Require limits.SymbolizerEnabled alongside
SymbolRefTreesEnabled.

Part of grafana/pyroscope-squad#487.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nant limit

Rework the symbol-ref request path onto the SymbolMode enum: the
per-tenant flag now sets SYMBOL_MODE_REFS instead of the dropped
symbol_refs bool, and carries the new per-tenant
symbolizer.max-unresolved-locations limit (smallest across request
tenants) on the query for the backend to enforce while building and
merging. The frontend re-checks the merged count before starting any
resolution, guarding against results that predate the backend check,
and consumes UnresolvedBinaries' validating two-value form.

Part of grafana/pyroscope-squad#487.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-frontend branch from 2585589 to f2a55f0 Compare July 16, 2026 01:54
@jake-kramer
jake-kramer force-pushed the symbolizer/symbolref-querybackend branch from c2b9c2a to 9425349 Compare July 16, 2026 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants