Skip to content

fix(querybackend): forward and apply span selector in pprof and tree query paths#5273

Merged
simonswine merged 2 commits into
grafana:mainfrom
simonswine:20260618_span-selector-fix
Jun 19, 2026
Merged

fix(querybackend): forward and apply span selector in pprof and tree query paths#5273
simonswine merged 2 commits into
grafana:mainfrom
simonswine:20260618_span-selector-fix

Conversation

@simonswine

Copy link
Copy Markdown
Contributor

SelectMergeSpanProfile returned all samples regardless of span IDs in the V2 read path due to two bugs:

  1. backendTreeSymbolizer rewrites QUERY_TREEQUERY_PPROF for symbolization, but PprofQuery had no span_selector field — selector was silently dropped.
  2. queryPprof always called AddSamplesFromParquetRow with no span filtering even when a selector was present.

Adds span_selector to PprofQuery proto, forwards it through the symbolizer, and applies filtering in the pprof backend. Also guards both backends against blocks without a SpanID column.

…query paths

The span selector was silently dropped in two places in the V2 read path,
causing SelectMergeSpanProfile to return all matching samples regardless
of the requested span IDs.

Bug 1 (symbolizer path): backendTreeSymbolizer rewrites QUERY_TREE to
QUERY_PPROF for symbolization, but PprofQuery had no span_selector field,
so the selector was always lost. Fix: add span_selector to PprofQuery proto
and forward it in symbolizer.go.

Bug 2 (pprof backend): queryPprof always called AddSamplesFromParquetRow
(no span filtering) even when a span selector was present. Fix: mirror the
queryTree span-filter logic — resolve the SpanID column and route through
AddSamplesWithSpanSelectorFromParquetRow when a selector is given.

Also guard both queryTree and queryPprof against blocks that lack a SpanID
column: early-return an empty report rather than reading the wrong column
(column index 0 / StacktraceID) as span IDs.

Removes the TODO comments that tracked this missing implementation.
@simonswine simonswine marked this pull request as ready for review June 18, 2026 14:56
Comment thread pkg/querybackend/query_pprof.go
marcsanmi
marcsanmi previously approved these changes Jun 19, 2026

@marcsanmi marcsanmi 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.

LGTM

Comment on lines +73 to +76
if !columns.HasSpanID() {
// Block has no SpanID column: no samples can match the span selector.
return &queryv1.Report{Pprof: &queryv1.PprofReport{Query: query.Pprof.CloneVT()}}, nil
}

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.

mostly curious, under what circumstances would this happen?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

A very old block, so something like from Phlare v0.x

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.

impossible with v2 then :)

@simonswine simonswine merged commit 506c6f3 into grafana:main Jun 19, 2026
34 checks passed
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.

3 participants