Skip to content

vtselect: return spanSet with corresponding spans#201

Merged
jiekun merged 5 commits into
masterfrom
fix/tempo-span-set
Jul 2, 2026
Merged

vtselect: return spanSet with corresponding spans#201
jiekun merged 5 commits into
masterfrom
fix/tempo-span-set

Conversation

@jiekun

@jiekun jiekun commented Jul 1, 2026

Copy link
Copy Markdown
Member

Describe Your Changes

This pull request is another implementation of #174

The Tempo-compatible /select/tempo/api/search endpoint populated spanSets[].spans[] with the trace root span (carrying a synthesized nestedSetParent=-1 and only service.name) instead of the spans that actually satisfied the TraceQL filter.

Grafana's Tempo datasource deep-links to spanSets[].spans[].spanID, so clicking a result row always focused the root span rather than the matched child span (real Tempo focuses the matched span). matched was also coupled to root-span presence, returning 0 when the root had not yet been ingested even though child spans matched.

When searching for the traces, we do it in 2 steps:

  1. find trace_id that match the filter condition.
  2. find all spans that match the trace_id.

This pull request change the 2nd step to "find root span, or spans that match the filter condition for trace_id in step 1".

As a result those spans will be placed to the spanset in response.

Checklist

The following checks are mandatory:

@cubic-dev-ai cubic-dev-ai 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.

6 issues found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/vtselect/traces/tempo/tempo.qtpl">

<violation number="1" location="app/vtselect/traces/tempo/tempo.qtpl:140">
P2: This drops the existing top-level spanSet compatibility field from search responses. Keep spanSet alongside spanSets with the same rendered data unless the API removal is intentional and coordinated.</violation>

<violation number="2" location="app/vtselect/traces/tempo/tempo.qtpl:154">
P1: Root spans that satisfy the TraceQL filter are dropped from spanSets, so root-only traces report no matched spans. Include the root in the rendered span set when it is the matched span, or append it during summary construction.</violation>
</file>

<file name="app/vtselect/traces/tempo/tempo.qtpl.go">

<violation number="1" location="app/vtselect/traces/tempo/tempo.qtpl.go:406">
P2: `startTimeUnixNano` is now encoded as a JSON number, but Tempo search span-set timestamps are strings. This can break Tempo-compatible clients that unmarshal this field as a string.</violation>

<violation number="2" location="app/vtselect/traces/tempo/tempo.qtpl.go:412">
P2: `durationNanos` is now encoded as a JSON number instead of Tempo's string format. Clients expecting Tempo-compatible search metadata may fail to decode the span set.</violation>
</file>

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

Comment thread app/vtselect/traces/tempo/tempo.qtpl
Comment thread app/vtselect/traces/tempo/tempo.qtpl
Comment thread app/vtselect/traces/tempo/tempo.qtpl.go
Comment thread app/vtselect/traces/tempo/tempo.qtpl.go
Comment thread app/vtselect/traces/tempo/tempo.go
Comment thread app/vtselect/traces/tempo/tempo.go Outdated

@cubic-dev-ai cubic-dev-ai 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.

1 issue found across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

Comment thread app/vtselect/traces/tempo/tempo.go
@jiekun jiekun merged commit f331774 into master Jul 2, 2026
9 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.

1 participant