Skip to content

Nest child group spans under their dependent group's span (#172) - #346

Merged
rubberduck203 merged 1 commit into
mainfrom
fix/telemetry-child-span-nesting
Jul 27, 2026
Merged

rubberduck203 merged 1 commit into
mainfrom
fix/telemetry-child-span-nesting

Conversation

@rubberduck203

@rubberduck203 rubberduck203 commented Jul 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • RunGroups::execute parented every group's tracing span directly to doctor run regardless of requires, so dependencies showed up as siblings after their dependent instead of nested inside its span (Telemetry: Child group spans aren't properly nested #172). Replaced the flat traversal with a RunGraph — a DAG reconstructed from the planned run order and each group's requires — and walk it recursively, opening a dependent's span before its dependencies so it temporally encloses them.
  • Roots parent directly to doctor run; a dependency shared by more than one group still runs exactly once, nested under whichever dependent reaches it first.
  • Fixes two gaps a review of this change surfaced: a requires cycle (unvalidated anywhere today) previously left every member of the cycle unreachable from any root and silently dropped it from the run while still reporting success; a group missing from group_actions would have orphaned its entire dependency subtree instead of just itself.

Test plan

  • cargo test — 205/205 passing, including 12 new tests (RunGraph unit tests, diamond/multi-root/fail-fast/cycle/missing-container behavior tests, and two span-nesting tests using a purpose-built tracing capture layer)
  • cargo clippy -- -D warnings and cargo fmt --all -- --check clean
  • Negative-control checks: reverted each of the four fixes individually and confirmed its dedicated test fails exactly as expected
  • Stress-tested the runner test module for flakiness (15+ repeated runs, all green)
  • Manually verified against local Jaeger using the issue's own MVCE — confirmed group mvce is the parent of group child-group and temporally encloses it (217484–217595 vs. 217504–217544)

Downloaded the MVCE from #172 and ran it against jaeger.
image

🤖 Generated with Claude Code

`RunGroups::execute` parented every group's tracing span directly to
`doctor run`, regardless of `requires`, so a group's dependencies showed
up as siblings after it instead of nested inside its span. Replace the
flat traversal with a `RunGraph` (a DAG reconstructed from the planned
run order and each group's `requires`) and walk it recursively, opening
a dependent's span before its dependencies so it temporally encloses
them. Roots parent to `doctor run`; a dependency shared by more than one
group still runs once, nested under whichever dependent reaches it
first.

Also closes two gaps a review of this change surfaced: a `requires`
cycle (unvalidated anywhere today) previously left every member
unreachable from any root and silently dropped from the run, and a
group missing from `group_actions` would have orphaned its entire
dependency subtree instead of just itself.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@rubberduck203
rubberduck203 marked this pull request as ready for review July 27, 2026 16:40
@rubberduck203
rubberduck203 merged commit fed1172 into main Jul 27, 2026
12 checks passed
@rubberduck203
rubberduck203 deleted the fix/telemetry-child-span-nesting branch July 27, 2026 18:33
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