Skip to content

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

Draft
rubberduck203 wants to merge 1 commit into
mainfrom
fix/telemetry-child-span-nesting
Draft

Nest child group spans under their dependent group's span (#172)#346
rubberduck203 wants to merge 1 commit into
mainfrom
fix/telemetry-child-span-nesting

Conversation

@rubberduck203

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)

🤖 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>
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