Skip to content

fix(otel): close child spans before parent contexts - #598

Open
zhongkechen wants to merge 1 commit into
mainfrom
codex/fix-otel-invocation-containment
Open

fix(otel): close child spans before parent contexts#598
zhongkechen wants to merge 1 commit into
mainfrom
codex/fix-otel-invocation-containment

Conversation

@zhongkechen

Copy link
Copy Markdown
Contributor

Summary

  • close pending invocation-view spans in reverse registration order
  • preserve child span containment when invocation shutdown closes an open callback context
  • add regression coverage for wait-for-callback parent and child spans

Root cause

InvocationOtelPlugin.on_invocation_end drained its parent-first span registry in insertion order. For wait-for-callback invocations, this ended the parent context before its callback child, producing the 45 us and 41 us containment violations in cases 10 and 17.

Failure: https://github.com/aws/aws-durable-execution-conformance-tests/actions/runs/30420590979/job/90476416155?pr=40

Verification

  • hatch run dev-otel:test (93 passed)
  • hatch run test:all (3063 passed, 2 skipped)
  • hatch run types:check
  • hatch fmt --check

@zhongkechen
zhongkechen had a problem deploying to ai-pr-review-runtime July 29, 2026 04:56 — with GitHub Actions Failure
@zhongkechen
zhongkechen had a problem deploying to ai-pr-review-runtime July 29, 2026 04:56 — with GitHub Actions Failure
@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime July 29, 2026 05:56 — with GitHub Actions Inactive
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime July 29, 2026 05:56 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown

Codex AI review

No actionable findings. Residual risk is limited to static review; tests were not executed per review constraints.

Reviewed commit fbc36e370bdc250614ae0da8359381d17f1447aa. Workflow run

@github-actions

Copy link
Copy Markdown

Claude AI review

The change is a correct, minimal, well-scoped fix. No actionable findings.

Fix correctnesson_invocation_end (invocation_plugin.py:342-344) now drains pending spans via reversed(self._operation_spans) instead of insertion order, closing children before parents. The parent-first insertion invariant it relies on is structurally guaranteed: _resolve_parent_span (invocation_plugin.py:198-214) refuses to start a child span until its parent is registered, and Python dicts preserve first-insertion order across in-place updates, so a parent can never sort after its own child. Reverse traversal therefore restores containment for arbitrary nested/sibling topologies, not only the two reported wait-for-callback cases.

End-time containment — pending spans are end()ed without explicit timestamps, stamping the current wall clock; ending child→parent sequentially with the invocation span (None) ended last yields monotonically non-decreasing end times, satisfying child.end_time <= parent.end_time. The None-skip-then-end-separately handling is preserved.

Thread safety / compat — the registry snapshot is still taken under self._operation_spans_lock; reversed() over a dict is fine on the >=3.11 floor.

Test (test_invocation_plugin.py:202-246) reproduces the exact failing topology (open CONTEXT/WAIT_FOR_CALLBACK parent + open CALLBACK child at PENDING invocation end) and asserts parentage and end-time containment. <= assertions tolerate coarse clock resolution, so it is not flaky.

No public API, serialization, or replay/determinism behavior is affected.

Residual test risk: Low. The end-time containment assertion depends on wall-clock monotonicity between sequential end() calls rather than injected durable timestamps; a future refactor ending pending spans concurrently or reusing a captured timestamp would not be caught by this test.

Reviewed commit fbc36e370bdc250614ae0da8359381d17f1447aa. Workflow run

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