Skip to content

test(otel): cover replay and chained execution - #796

Open
zhongkechen wants to merge 8 commits into
mainfrom
test/python-otel-conformance-20261021
Open

zhongkechen wants to merge 8 commits into
mainfrom
test/python-otel-conformance-20261021

Conversation

@zhongkechen

@zhongkechen zhongkechen commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Add the remaining OpenTelemetry conformance coverage through the public Core 3 / OTel 2 APIs: skip completed step bodies (21), retain user-function and callback tracing contexts (22–23), report invocation retries as RETRYING/UNSET (24), and suspend/resume a chained child through a durable wait (27).

Merge order: #864 first, then this PR. Case 11's callback/replay fix and regression test are now owned by #864. This PR is stacked on fix/otel11-replay-barrier, so its review diff excludes that fix. After #864 merges, retarget this PR to main.

The shared OpenTelemetry workflow tracks the conformance repository's main branch. The test catalog defaults to main for pull requests, pushes, and manual runs; manual runs retain the conformance_test_ref override. SDK handlers run from the commit under test.

Coverage and follow-up

This PR supplies handlers for cases 21–24 and 27, replacing their pending-coverage declarations from #864. Cases 25–26 remain explicitly NOT_IMPLEMENTED in both telemetry views because they depend on separate runtime/testing fixes. The existing case 17 handler remains active; its additional callback-error regression is deferred.

The deferred source is available in 6484e577.

  • Case 25: terminal failure status #853 and empty-error history/runtime consistency #862.
  • Case 26: completion telemetry #854, callback update production #860, and input acknowledgment #861.
  • The additional case 17 callback-error test also requires snapshot isolation #852.

Before restoring those tests, correct their history expectations: included diagnostic payloads are untruncated; metadata-only history omits payloads and cannot reconstruct error details. The empty Error.Payload service shape is unverified against deployed AWS Lambda. Preserve case 26's assertion inside the observed step that completion has already reached the exporter, plus raw export counts at both later barriers.

The shared orchestrator runs invocation and execution suites. It no longer schedules the legacy long-running suite; existing long-running templates and local contract tests remain.

Validation

After splitting out #864, on Python 3.13.17:

  • hatch run test:all packages/aws-durable-execution-sdk-python-conformance-tests-otel/tests -q --tb=short: 75 passed.
  • The case 11 handler and regression test are byte-identical to fix(otel): restore case 11 replay coverage #864 and absent from this PR's diff against its new base.
  • Formatting, commit-message validation, and git diff --check: passed.
  • The previous workflow-default change passed 10 focused workflow tests and validated all 15 inputs and 7 secrets against the shared workflow schema.

The latest cloud run at f56cf00b passed three of four telemetry jobs. The S3 execution-view job failed case 21 because WaitSucceeded preceded InvocationCompleted by 63 ms, opposite to its fixed history expectation. The execution succeeded and returned before-after; that timing issue is unchanged by this split. The new standalone PR does not include case 21.

Existing test expectation changes

  • test_templates_map_every_otel_requirement, test_templates_declare_only_the_expected_functions, test_function_names_identify_the_requirement, test_makefile_builds_every_function, test_every_registered_handler_exists, test_handler_modules_are_exactly_the_expected_set, and test_chained_invoke_functions_point_at_their_target: extend the expected mappings, modules, functions, and targets for cases 21–24 and 27.
  • test_deferred_requirements_are_explicit: reduce pending cases from 21–27 to 25–26 because this PR supplies the other handlers, retaining issue references and complete catalog accounting.
  • test_opentelemetry_conformance_caller_uses_current_workflow_contract: expect the shared workflow and catalog to use main, matching the requested default.

Case 27 companion PRs: JS #1068 and Java #824.

@github-actions

This comment has been minimized.

@zhongkechen zhongkechen mentioned this pull request Oct 21, 2026
7 of 11 tasks
@zhongkechen
zhongkechen added this pull request to stack #800 October 21, 2026 19:32
@zhongkechen
zhongkechen removed this pull request from stack #800 October 22, 2026 17:25
@zhongkechen
zhongkechen added this pull request to stack #809 October 22, 2026 17:25
@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime October 22, 2026 17:28 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime October 22, 2026 21:03 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 27, 2026 20:43 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 27, 2026 21:15 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 27, 2026 21:41 — with GitHub Actions Active
Comment on lines +127 to +130
if error.message != HELPER_FAILURE:
raise
else:
raise RuntimeError("Expected the intentional helper failure")

This comment was marked as outdated.

Comment on lines +53 to +57
("Otel21CompletedStepReplay", "otel-invocation-21"),
("Otel22UserFunctionContext", "otel-invocation-22"),
("Otel23CallbackFunctionContext", "otel-invocation-23"),
("Otel24InvocationRetryStatus", "otel-invocation-24"),
("Otel27ChainedInvokeSuspend", "otel-invocation-27"),

This comment was marked as outdated.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen changed the base branch from main to fix/otel11-replay-barrier October 27, 2026 22:05
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 27, 2026 22:06 — with GitHub Actions Active
BuildMethod: makefile
TestingMetadata:
TestDescription:
- otel-execution-21

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex AI review · Finding arf_v1_iqv2alr2znzezrkkaokhczcccj

[P2] Resolve the known history-order failure before enabling case 21

This activates case 21 in execution-view CI despite the unchanged failure documented in the PR: WaitSucceeded preceded InvocationCompleted by 63 ms. The wait timer runs independently of invocation cleanup, so that ordering can legitimately occur while the execution still returns before-after. The shared requirement's fixed ordering therefore produces false failures for valid SDK behavior. Land a coordinated requirement correction that accepts both valid orderings while preserving the replay assertions, or retain the pending declaration until that correction lands.

To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.

Comment on lines +127 to +130
if error.message != HELPER_FAILURE:
raise
else:
raise RuntimeError("Expected the intentional helper failure")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex AI review · Finding arf_v1_qhb22zbhwqp6f4uxhumsw3g7n3

[P2] Return the untested error guards to the design phase

Rule 4, “Every branch has a test that fails without it,” remains unsatisfied. Both test parametrizations use the helper that raises HELPER_FAILURE; neither exercises a different error message or an unexpected successful return. Removing either defensive rejection leaves the test results unchanged. Return to the design phase under rule 8 and decide whether these cases need meaningful coverage or whether the fixed fixture permits a simpler approach.

To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.

Comment on lines +90 to +95
barriers: dict[str, Barrier] = {}
for pair in (
("parallel-a", "parallel-b"),
("parallel-step-a", "parallel-step-b"),
("map-0", "map-1"),
("map-step-0", "map-step-1"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex AI review · Finding arf_v1_zhaqzag3s27vy6z7ak6uvvprpg

[P3] Document simpler alternatives for the added harness

Rule 2, “Simplify before you add,” requires the description to identify simpler options considered and explain why they do not work when logic is added. This introduces a label-indexed barrier harness and callback wrapper, but the description lists coverage and validation without that comparison. Existing OTel e2e tests already exercise branch isolation and replay, making the boundary between extending existing coverage and adding this harness worth establishing. Return to the design phase under rule 8 and record the alternatives and the reason for the chosen minimum approach.

To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.

@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

Found one CI issue and two approach issues:

  • Known case 21 cloud failure remains enabled: packages/aws-durable-execution-sdk-python-conformance-tests-otel/template.yaml:900.
  • Untested error guards remain: packages/aws-durable-execution-sdk-python-conformance-tests-otel/src/otel_23_callback_function_context.py:127.
  • Added harness logic lacks the required simplification rationale: packages/aws-durable-execution-sdk-python-conformance-tests-otel/tests/test_user_function_context.py:90.

Review was read-only; tests were inspected but not executed.

Reviewed commit dc736d5c77f0f8c5be9a15a2d00a2ac47f3e1691. Workflow run

Base automatically changed from fix/otel11-replay-barrier to main October 27, 2026 23:02

This branch was successfully deployed

1 active deployment
ai-pr-review-runtime — dc736d5c Deployed Oct 27, 2026 by zhongkechen via ai-pr-review / Codex review / Generate Codex review #2527
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