fix(otel): end recording spans on non-terminal

[ayushiahjolia]

Issue #, if available:
#642

Description of changes:

• Workflow span: created+ended once, on the terminal invocation (both plugins).
• Attempt span: created+ended in the same invocation (both plugins).
• Operation span: invocation plugin → one segment per invocation (fresh ID + link across invocations); execution plugin → one span total, created+ended in the invocation where the operation terminates (held as a non-recording placeholder while it spans invocations).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[zhongkechen]

Addressed in 0090a01. InvocationOtelPlugin now reuses an existing recording CONTEXT span during same-invocation re-entry instead of overwriting it. The lifecycle tests now assert identity reuse for single and nested contexts and verify the original span stops recording after the terminal operation hook. Validation: 250 OTel tests passed, OTel type checking passed, and Hatch formatting is clean.

[github-actions]

Codex AI review

No actionable findings. Residual risk is limited to the unexecuted ADOT/OpenTelemetry compatibility matrix because this review was restricted to static inspection.

Reviewed commit ab539a717c3c09a1cd9228f7caf47b1e523c138d. Workflow run

[github-actions]

Claude AI review · Finding arf_v1_dcmwvtn22nrbcgqkeggzqzrdnv

The new reentry guard (existing_span is not None and existing_span.is_recording(): span = existing_span) only fires when info.operation_type is OperationType.CONTEXT. For a STEP attempt, execution always takes the else branch and calls self._start_span(...), whose unchanged body unconditionally does self._operation_spans[registry_key] = span, silently overwriting any span already registered under that attempt's span_key without ending it first.

This is exactly the "timed in-process resume re-enters the same operation" scenario this PR fixes for CONTEXT here, and fixes for every span type (attempts included) in ExecutionOtelPlugin._start_span, which now ends and marks a stale recording span (durable.span.replaced_on_reentry) before replacing it. In InvocationOtelPlugin, a re-entered STEP attempt's previous span is dropped from the registry without .end() ever being called on it, so it is never exported — a real telemetry gap. ExecutionOtelPlugin now has test_reentered_step_attempt_releases_the_previous_scope covering this for attempts; InvocationOtelPlugin has no equivalent test or fix.

Extend the is_recording() reuse check to STEP attempts too, or better, move an equivalent "end the stale recording span before replacing it" check into InvocationOtelPlugin._start_span itself (mirroring the execution plugin's fix) so every caller benefits, and add a regression test mirroring the execution-plugin one.

[github-actions]

Claude AI review · Finding arf_v1_63zklrnsxmau6x72ea3mxj3ipz

end_time is recomputed via ensure_end_after_start(info.start_time, info.end_time) here, but it was already computed identically a few lines above (right after the matching-start check) and used for the _note_parent_end calls. info is a frozen dataclass, so neither input can have changed in between, making this a redundant recomputation. It's harmless today but is dead code that risks silently diverging from the first computation on a future edit (e.g. someone updates only one of the two call sites). Delete this line and reuse the end_time already in scope.

Suggested change

end_time = ensure_end_after_start(info.start_time, info.end_time)

[github-actions]

Claude AI review

Reviewed the OTel plugin changes that switch open Workflow/operation spans to non-recording DurableParentSpan placeholders and add cross-invocation timestamp aggregation, deterministic-ID export guarding, and re-entry handling for ExecutionOtelPlugin/InvocationOtelPlugin.

Most of the issues flagged in earlier review rounds on this PR are now fixed in this diff: DurableParentSpan.note_start_time/note_end_time/normalized_* are now guarded by a threading.Lock, _end_open_recording_spans() now tags truncated attempt spans with durable.span.truncated_at_invocation_boundary before ending them, and ExecutionOtelPlugin.on_operation_end now dedupes exports per operation via _ended_operation_ids combined with the non-recording placeholder, addressed by new tests (test_suspend_then_resume_operation_exports_one_deterministic_span, test_incomplete_attempt_is_marked_when_invocation_ends, etc.).

Two issues from the prior round remain unresolved in this revision, listed below (one moderate telemetry-integrity gap, one minor dead-code nit). No other new correctness, determinism, or thread-safety issues were found in the changed code; test coverage for the new behavior is otherwise thorough. Residual risk: the missing STEP-attempt-reentry test/fix for InvocationOtelPlugin (see comment) means that specific resume scenario is unverified for that plugin even though its ExecutionOtelPlugin counterpart is now tested.

Reviewed commit ab539a717c3c09a1cd9228f7caf47b1e523c138d. Workflow run