Late span reparenting

[omnibs]

A child span can end after its parent span has ended. This can happen in a few scenarios:

• a thread started inside a span and joined outside it
• fire-and-forget tasks
• lazy streaming values created inside a Task but consumed after it returns

In these cases, the child was getting lost, because it was being attached to a parent that had already been copied out of its IORef and into its parent.

The solution we found is:

• detecting when this happens
• trying to reparent to the root span
• promoting the child to a new root if the root itself is closed

Alternatively, we could have added support for attaching children to finished parents, by keeping IORefs around until the root is processed. That was a much more invasive change, and for the use-case we're handling while we hit this, reparenting to root looks good, so I decided to go with that.

ps: i have tested integrating this into the NoRedInk monorepo pre-merge and have confirmed the fix

[Copilot]

Pull request overview

This PR hardens Platform tracing so spans that outlive their intended parent are no longer silently dropped. It introduces an explicit “open vs closed” span lifecycle to make parent finalization and child attachment atomic, enabling late spans to be reparented to the trace root (or emitted as a new root when the trace is already closed), and adds tests to cover these scenarios.

Changes:

• Introduce SpanTarget/SpanState and TraceRoot to make span completion/attachment races atomic and make repeated finalization idempotent.
• Reparent “late” child spans to the still-open trace root; if the root is closed, report the child as a separate root span.
• Add a comprehensive test suite for late span reparenting behavior and concurrency races; document the fix in the changelog.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
nri-prelude/src/Platform/Internal.hs	Adds atomic span state management and late-span reparenting/reporting behavior; refactors handler construction accordingly.
nri-prelude/tests/PlatformSpec.hs	Adds targeted tests covering late span reparenting, idempotent finalization, and concurrent parent/child finalization races.
nri-prelude/CHANGELOG.md	Documents the behavioral fix and clarifies that the public API and serialized TracingSpan format are unchanged.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[micahhahn]

Fascinating.

I hadn't really investigated the span trace code until ruminating on this PR.

One question I have... is onFinishRoot the point where we actually serialize the spans (to log-explorer, honeycomb, etc)? I think in this case where a child has not finalized the changes in this PR will still result in the child being silently dropped.

Attaching a child to the parent in the child's finish handler seems wrong to me (in the way the code works before this PR). It seems better to me if the parent adds the child to its list of children at the moment startChildTracingSpan so it's aware of all its children.

Could we modify TracingSpan to look like this?

data TracingSpan f = TracingSpan
{ 
... 
  children :: [f TracingSpan]
}

We would have TracingSpan IORef as the type for the duration while we are still collecting spans and then finally when asked to finalize the root we could walk down the tree resolving the IORef TracingSpans as go and converting the overall type to TracingSpan Identity (isomorphic to how it currently is). This would allows us to mark incomplete children as incomplete which could be very useful!

[omnibs]

onFinishRoot is where we kick off reporting, yes!

Attaching to a parent after onFinishRoot has kicked off would normally result in a dead span, but this code tries to avoid that by promoting the span to a new root, which will run its own onFinishRoot and get reported, but i'll be honest i haven't tested this case e2e bc it's not within what we're trying to address in the monorepo.

deployer does have a case like this, for its slack slash commands, where the http response must return within a strict time limit, so we kick off slow state machine updates in a separate thread. I'm curious to see whether it works after we merge back.

Adding children at start doesn't sound like a great solution to me, because onFinishRoot would have to make bad calls on unfinished spans. It would have to set some fake duration for the incomplete child, and possibly miss sub-spans that haven't been reported yet. Another possible issue, which i haven't tested, is how observability tooling handles displaying child spans that overflow their parents.. if it doesn't handle it well, onFinishRoot might have to set the fake end timestamp to be min(incomplete_span_end, parent_end), which is misleading.

Letting the child choose what to do when its parent is closed sounds better, so we can reparent or promote to root, like we're doing here. It's also a much smaller surface-area changeset :S

[omnibs]

@micahhahn friendly nudge in case you missed the answer above (not sure if you get notified w/o @'ing)

[micahhahn]

Tricky stuff!

This is a big improvement over silently dropping spans, thanks for the work on this. LGTM!

[omnibs]

Recording here a suggestion from Micah on Slack which I really liked:

Promoting a span to root non-deterministically is not great. We could do both:

• Record unfinished child spans, marking them as unfinished
• Promote them to a new root trace

Ideally, the unfinished child has a clear link to the promoted root (referencing the new root's trace_id?)