feat: add reverse span links, show span link detail - #3011
Conversation
🦋 Changeset detectedLatest commit: b8819a4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Review process: Full human review — logic, architecture, edge cases. Stats
|
Greptile SummaryThe PR adds reverse span-link discovery, resolves linked-span metadata, and improves navigation between mutually linked spans.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/app/src/components/linkedSpans.tsx | Adds shared query construction, result normalization, hooks, and metadata rendering for forward and reverse span links. |
| packages/app/src/components/DBRowOverviewPanel.tsx | Integrates resolved forward-link details and the new reverse-link section into the row overview. |
| packages/app/src/components/DBRowSidePanel.tsx | Adds origin-aware source-frame navigation so reciprocal span-link hops return through the existing breadcrumb. |
| packages/app/src/components/SpanLinksSubpanel.tsx | Enhances forward links with resolved span names and metadata while preserving the unresolved fallback. |
| packages/app/src/components/SpanLinkedFromSubpanel.tsx | Adds the reverse-link list UI and its trace-navigation actions. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Selected span] --> B[Resolve forward links]
A --> C[Query spans linking back]
B --> D[Linked span details]
C --> E[Linked from details]
D --> F[Open linked trace]
E --> F
F --> G{Targets origin row?}
G -->|Yes| H[Pop breadcrumb]
G -->|No| I[Push source frame]
Reviews (3): Last reviewed commit: "feat: add reverse span links, show span ..." | Re-trigger Greptile
9885865 to
5208e25
Compare
E2E Test Results✅ All tests passed • 320 passed • 1 skipped • 1185s
Tests ran across 4 shards in parallel. |
Deep Review✅ No critical issues found. The feature is wired correctly end-to-end: Note on coverage: of the dispatched fan-out, the 🔵 P3 nitpicks (4)
Reviewers (2 completed): performance, previous-comments — plus direct orchestrator verification of correctness, SQL-safety, and breadcrumb navigation state. Testing gaps: No latency benchmark of the unindexed reverse |
5208e25 to
b8819a4
Compare
pulpdrew
left a comment
There was a problem hiding this comment.
LGTM, and seems to work well.
Left a suggestion for another option if we're worried about the performance impact.
| ...(source.kind === SourceKind.Trace && source.serviceNameExpression | ||
| ? [ | ||
| { | ||
| valueExpression: source.serviceNameExpression, | ||
| alias: LINKED_SPAN_ALIASES.SERVICE_NAME, | ||
| }, | ||
| ] | ||
| : []), | ||
| ...(source.kind === SourceKind.Trace && source.durationExpression | ||
| ? [ | ||
| { | ||
| valueExpression: getDurationMsExpression(source), | ||
| alias: LINKED_SPAN_ALIASES.DURATION_MS, | ||
| }, | ||
| ] | ||
| : []), |
There was a problem hiding this comment.
nit: It might be nice to update some of these types to source: TTraceSource; so these checks aren't necessary, assuming we should typically only be working with trace sources in most of these functions.
| // (a scan of ~2 daily partitions). If that gets slow (~1B spans/day), | ||
| // adding to otel_traces: |
There was a problem hiding this comment.
Another option here is to gate this behind a button, say "Show spans linking to this span" in the UI to dispatch a potentially slow search, instead of always dispatching it when opening the panel
| // A link often points right back at the row one level up (forward and | ||
| // reverse span links come in pairs). Pop back to that breadcrumb instead | ||
| // of pushing an endless A <-> B trail. Only when no nav drilldown sits | ||
| // on top: popOne would pop the nav entry, not the source frame. |
|
One other callout is that it may be good to add an E2E test for this functionality, to prevent regressions in the future. |
Summary
Span links were only usable from the consumer side: a span showed the links it carries, as bare "Open trace" actions. There was no way to start from a producer span and find the consumer spans that link back to it.
SourceFramegains an optional, source-agnosticoriginRowIdfor this.useQueriedChartConfigbounded to a time window anchored on the selected span, skewed in the direction links point in time: referencing spans run at/after the span (−1h/+24h), linked-to spans ran at/before it (−24h/+1h).Screenshots or video
Untitled.mov
How to test on Vercel preview
Preview routes:
/searchSteps:
Links.SpanId != [])References