Skip to content

fix(otel): leave pending invocation spans unset - #599

Closed
zhongkechen wants to merge 1 commit into
mainfrom
codex/otel-pending-invocation-status-unset
Closed

fix(otel): leave pending invocation spans unset#599
zhongkechen wants to merge 1 commit into
mainfrom
codex/otel-pending-invocation-status-unset

Conversation

@zhongkechen

Copy link
Copy Markdown
Contributor

Summary

  • leave invocation span status unset when the durable invocation is pending
  • preserve durable.invocation.status=PENDING for lifecycle filtering
  • align both OTel plugin implementations and their status-mapping tests

Testing

  • hatch run dev-otel:test (92 passed)
  • hatch run types:check
  • hatch fmt --check in packages/aws-durable-execution-sdk-python-otel

@zhongkechen
zhongkechen had a problem deploying to ai-pr-review-runtime July 29, 2026 05:42 — with GitHub Actions Failure
@zhongkechen
zhongkechen had a problem deploying to ai-pr-review-runtime July 29, 2026 05:42 — with GitHub Actions Failure
@zhongkechen
zhongkechen requested a review from SilanHe July 29, 2026 05:44
@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime July 29, 2026 16:34 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown

Claude AI review

No actionable findings.

This PR changes the OTel invocation-span status mapping for PENDING invocations from OK to UNSET in both plugin implementations, and updates the corresponding tests. The review confirms:

  • Consistency across implementations: execution_plugin.py:300 and invocation_plugin.py:358 are updated symmetrically, and both explanatory comments now correctly describe PENDING/RETRY -> UNSET.
  • Test alignment: The two parametrized status-mapping tests (test_execution_plugin.py:325, test_invocation_plugin.py:178) were both updated to expect StatusCode.UNSET for PENDING.
  • Attribute preservation intact: durable.invocation.status is still set unconditionally via set_attribute (execution_plugin.py:296, invocation_plugin.py:350), so PENDING remains queryable for lifecycle filtering, as the PR description claims.
  • No stale references: The other two PENDING tests in test_execution_plugin.py (lines 152, 315) assert only span export/non-export, not status code, so they remain valid. No docs, examples, or other packages depend on the old PENDING -> OK mapping.

The change is semantically sound: a PENDING invocation is non-terminal, so leaving the span status UNSET (rather than asserting OK) is the more accurate representation and aligns it with the existing RETRY handling.

Residual test risk: low. The behavioral change is fully covered by the updated parametrized tests. There is no explicit assertion that a PENDING invocation span still carries the durable.invocation.status=PENDING attribute when the span status is UNSET (the attribute-preservation guarantee is exercised only indirectly), but this is a minor coverage gap rather than a defect in the change.

Reviewed commit e402b22f9f3651a12ef43991a4aadb2655320c75. Workflow run

@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime July 29, 2026 16:46 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown

Codex AI review

No actionable findings. Residual risk is low: both plugin implementations have focused unit coverage, though tests were not independently run under the review constraints.

Reviewed commit e402b22f9f3651a12ef43991a4aadb2655320c75. Workflow run

@zhongkechen
zhongkechen deleted the codex/otel-pending-invocation-status-unset branch July 29, 2026 16:52
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.

2 participants