Skip to content

feat(otel): add Workflow span to InvocationOtelPlugin - #593

Open
SilanHe wants to merge 2 commits into
mainfrom
feat/otel-invocation-workflow-span
Open

feat(otel): add Workflow span to InvocationOtelPlugin#593
SilanHe wants to merge 2 commits into
mainfrom
feat/otel-invocation-workflow-span

Conversation

@SilanHe

@SilanHe SilanHe commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Issue #, if available: aws/aws-durable-execution-conformance-tests#23 (comment)

Description of changes: InvocationOtelPlugin now emits a deterministic, execution-scoped Workflow root span (span ID derived from the execution ARN, INTERNAL, started at execution_start_time), exported once on a terminal invocation (SUCCEEDED->OK, FAILED->ERROR; non-terminal dropped unexported). Created unconditionally since this plugin has no default/owned tracer-provider distinction. Operation and attempt spans link to the Workflow span while remaining parented to the invocation span. The span name is configurable via a new workflow_span_name constructor kwarg (default "Workflow").

The "invocation" span is renamed to "Invocation" to match the capitalization for the "Workflow" and the configuration in other language's Otel plugins.

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

@SilanHe
SilanHe force-pushed the feat/otel-invocation-workflow-span branch from caacb55 to 3a9ba39 Compare July 28, 2026 21:07
InvocationOtelPlugin now emits a deterministic, execution-scoped
Workflow root span (span ID derived from the execution ARN, INTERNAL,
started at execution_start_time), exported once on a terminal
invocation (SUCCEEDED->OK, FAILED->ERROR; non-terminal dropped
unexported). Created unconditionally since this plugin has no
default/owned tracer-provider distinction. Operation and attempt spans
link to the Workflow span while remaining parented to the Invocation
span. The workflow span name is configurable via a new
workflow_span_name constructor kwarg.

Span display names are capitalized in both plugins: "Workflow" (default)
and "Invocation".
@SilanHe
SilanHe force-pushed the feat/otel-invocation-workflow-span branch from 3a9ba39 to 5c3adc9 Compare July 28, 2026 21:16
@SilanHe SilanHe changed the title feat(otel): add workflow span to InvocationOtelPlugin feat(otel): add Workflow span to InvocationOtelPlugin Jul 28, 2026
@SilanHe
SilanHe marked this pull request as ready for review July 28, 2026 21:29
@SilanHe
SilanHe temporarily deployed to ai-pr-review-runtime July 28, 2026 21:29 — with GitHub Actions Inactive
@SilanHe
SilanHe temporarily deployed to ai-pr-review-runtime July 28, 2026 21:29 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown

Codex AI review

  • Medium packages/aws-durable-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel/invocation_plugin.py:419: Non-terminal invocations abandon a recording Workflow span without ending it. Span processors receive on_start but never on_end, potentially leaking processor state on every suspension/retry. Store only a synthetic SpanContext for links, then create and end the real span on terminal completion; add a processor lifecycle test.

  • Medium packages/aws-durable-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel/invocation_plugin.py:353 and execution_plugin.py:279: Changing the established span name from invocation to Invocation breaks existing dashboards, queries, and the documented telemetry contract. Preserve the lowercase default, or introduce the rename through an explicit compatibility option and update public documentation.

Reviewed commit 5c3adc9dbc33baa54056684bd417571db0023d80. Workflow run

@github-actions

Copy link
Copy Markdown

Claude AI review

Summary

This PR adds an execution-scoped Workflow root span to InvocationOtelPlugin (deterministic span ID from the execution ARN, INTERNAL kind, anchored to execution_start_time, exported once on a terminal invocation) and links operation/attempt spans to it while keeping them parented to the invocation span. The core logic is sound: the Workflow span is created as a parentless root in an empty Context(), shares the deterministic execution trace ID via the shared DeterministicIdGenerator, and terminal-status export/status mapping (SUCCEEDED→OK, FAILED→ERROR, non-terminal dropped unended) is correct and mirrors the existing ExecutionOtelPlugin. Per-invocation state (_workflow_span) is properly reset in on_invocation_end to avoid leaks across warm Lambda reuses. Test coverage for the new behavior (terminal/non-terminal export, determinism, root-parenting, invocation-link, configurable name) is thorough.

Findings

  1. Undocumented span rename invocationInvocationinvocation_plugin.py:353 (changed line 334 in diff / new source line) and execution_plugin.py:279. Span names are an observable contract; the PR description only mentions the new Workflow span and does not call out this rename. Downstream X-Ray/OTel queries, dashboards, or span-derived metrics keyed on the literal invocation will silently stop matching. Additionally it leaves stale artifacts:
    • packages/aws-durable-execution-sdk-python-otel/README.md:227 still documents an "invocation" span.
    • Example-package assertions test/plugin/test_otel_plugin.py:75 and test/otel/test_otel_logger_example.py:69 now pass only incidentally by matching the durable.invocation.* attribute keys, not the span name — so they no longer validate the span name.
      Fix: if intentional, update the README and example assertions (assert "Invocation") and document the breaking name change; otherwise revert both call sites to "invocation". (Posted inline.)

Residual test risk

  • The two example integration tests (test_otel_plugin.py, test_otel_logger_example.py) are cloud-mode X-Ray tests that were not updated for the rename; they will not catch a span-name regression because their assertions coincidentally match attribute-key substrings. They should be tightened to assert the actual span name.
  • No test asserts the Workflow span's trace ID equals the invocation span's trace ID (only span-id determinism and root-parenting are checked), though this is exercised indirectly via parenting/linking.

Reviewed commit 5c3adc9dbc33baa54056684bd417571db0023d80. Workflow run

@SilanHe

SilanHe commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

I think we will be covering the Residual test risk in our conformance testing.

@SilanHe
SilanHe had a problem deploying to ai-pr-review-runtime July 28, 2026 23:48 — with GitHub Actions Failure
@SilanHe
SilanHe had a problem deploying to ai-pr-review-runtime July 28, 2026 23:48 — with GitHub Actions Failure
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