fix(runtime): enter Python tracing span after parent context setup

[SaitejaKommi]

Summary

Ensure Python operator tracing spans are entered only after the OpenTelemetry parent context has been established, preserving the correct tracing context during on_event callback execution.

Fixes #2862

Root Cause

In binaries/runtime/src/operator/python.rs, the tracing span was entered before the incoming OpenTelemetry parent context was attached via span.set_parent(cx).

As a result, the span became active without the correct parent context, causing Python operator execution to be detached from the intended trace hierarchy.

Solution

• Move let _enter = span.enter(); to execute after the #[cfg(feature = "telemetry")] block sets the parent context.
• Preserve the entered span for the entire lifetime of the Python on_event callback by keeping the guard alive until the callback completes.

Testing

• ✅ cargo fmt --all -- --check
• ✅ cargo check -p dora-runtime

Scope

This PR is intentionally limited to binaries/runtime/src/operator/python.rs and only changes the ordering of tracing span entry relative to parent context initialization. No functional behavior or public APIs were modified.

[trunk-io]

Merging to main in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[SaitejaKommi]

Hi @phil-opp ,could you please review this PR when you have a moment? All CI checks have passed.

[phil-opp]

Automated review by Claude — this is a fully automated review; no human has vetted it.

No issues found. set_parent/record act on the span handle regardless of whether it's entered, so deferring span.enter() until after the parent context is attached correctly ensures the on_event execution runs under a span with the right parent. The guard still lives for the full callback, and with the telemetry feature off the reordering is a no-op.

---

Generated by Claude Code

[phil-opp]

The premise doesn't hold. #2862 reports the code as let _ = span.enter(); with the guard dropped immediately — but binaries/runtime/src/operator/python.rs:192 on main is already let _enter = span.enter();. The guard is bound and lives to the end of the Python::attach closure, so the span is already active across the whole on_event call.

This PR also doesn't do what the issue asked (bind the guard). It moves the existing binding below the telemetry block, under a different rationale — that the span was entered "without the correct parent context". That reorder is a no-op:

• span.record("input_id", …), span.set_parent(cx) and span.context() all act on the span handle, not on Span::current(). Whether the span is entered is irrelevant to all three.
• set_parent writes the parent into the span's extensions; before or after enter() gives the same result, and either way it runs before on_event is called.
• Drop order is unchanged: _enter is still declared ahead of py_event/status_enum, so it is still dropped after them.

So there's no behavior change to merge, and the description ("causing Python operator execution to be detached from the intended trace hierarchy") describes something that wasn't happening.

There is one defensible reason to keep it: consistency. send_output in the same file (lines 340-360) already does set_parent first and enter after. If you reframe this as a cosmetic alignment with that — no bug, no behavior change — the change itself is fine. As written, I'd close this and #2862.

If you want the real instance of the pattern #2862 describes: binaries/daemon/src/running_dataflow.rs:326 genuinely is let _ = span.enter();, so that "tick" span never becomes active. let _guard = span.enter(); is the right fix there. Worth checking the impact before writing it up, though — the only nearby consumer is span.context(), which reads the handle directly, so it may well be latent rather than user-visible.

(generated with Claude)

[SaitejaKommi]

The premise doesn't hold. #2862 reports the code as let _ = span.enter(); with the guard dropped immediately — but binaries/runtime/src/operator/python.rs:192 on main is already let _enter = span.enter();. The guard is bound and lives to the end of the Python::attach closure, so the span is already active across the whole on_event call.

This PR also doesn't do what the issue asked (bind the guard). It moves the existing binding below the telemetry block, under a different rationale — that the span was entered "without the correct parent context". That reorder is a no-op:

• span.record("input_id", …), span.set_parent(cx) and span.context() all act on the span handle, not on Span::current(). Whether the span is entered is irrelevant to all three.
• set_parent writes the parent into the span's extensions; before or after enter() gives the same result, and either way it runs before on_event is called.
• Drop order is unchanged: _enter is still declared ahead of py_event/status_enum, so it is still dropped after them.

So there's no behavior change to merge, and the description ("causing Python operator execution to be detached from the intended trace hierarchy") describes something that wasn't happening.

There is one defensible reason to keep it: consistency. send_output in the same file (lines 340-360) already does set_parent first and enter after. If you reframe this as a cosmetic alignment with that — no bug, no behavior change — the change itself is fine. As written, I'd close this and #2862.

If you want the real instance of the pattern #2862 describes: binaries/daemon/src/running_dataflow.rs:326 genuinely is let _ = span.enter();, so that "tick" span never becomes active. let _guard = span.enter(); is the right fix there. Worth checking the impact before writing it up, though — the only nearby consumer is span.context(), which reads the handle directly, so it may well be latent rather than user-visible.

(generated with Claude)

Thanks for the detailed explanation,I understand the issue was based on an incorrect premise, so I'll close this PR and investigate the actual occurrence you pointed out instead..