Skip to content

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

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

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

Conversation

@zhongkechen

@zhongkechen zhongkechen commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Add OpenTelemetry conformance handlers for user-function and callback tracing contexts (22–23), invocation retries reported as RETRYING/UNSET (24), caught callback failure replay (25), external callback completion and later replay (26), and a chained child that suspends and resumes through a durable wait (27). These handlers exercise the public Core 3 / OTel 2 APIs in both telemetry views.

Case 21 retains its handler and local replay regression tests, but cloud coverage is explicitly NOT_IMPLEMENTED until the shared history requirement accepts both valid wait-completion orderings. Cases 25–26 are enabled in both cloud suites and the ordinary local test suite. Keep their failures as merge gates until the prerequisite bug fixes merge; do not skip, xfail, or suppress them. The existing case 17 handler remains active; its additional callback-error regression is deferred.

The case 11 callback/replay fix and regression test have merged in #864 and are absent from this PR's diff against main.

Review decisions and simpler alternatives

  • Case 21 history ordering: Both current shared requirements assign event 5 to InvocationCompleted and event 6 to WaitSucceeded. The execution-view cloud run observed WaitSucceeded 63 ms earlier while the execution succeeded with before-after. The wait timer and invocation cleanup can finish in either order. Follow the review's deferral option: remove both cloud functions and their build targets and declare both cases pending. The invocation catalog imposes the same ordering, so deferring only the execution view leaves the same risk. Increasing the wait merely reduces the race; it does not establish an ordering guarantee. Re-enable cloud case 21 after a coordinated shared-requirement correction preserves the replay assertions while accepting both orders. Local tests still require each step body to run once, PENDING/SUCCEEDED invocations, and one export per completed operation in the execution view.
  • Case 23 error checks: Retain rejection of an unrelated helper failure and of unexpected success. Silently accepting either outcome can hide a failed probe or skip the retry-strategy callback that this scenario must exercise. Extend the existing public-runner test with both outcomes in both views, keeping the original successful-scenario parent/count assertions intact. Each negative case asserts the failure message and that execution does not continue into the virtual child. Removing either check makes its two tests fail.
  • Case 22 test harness: The existing OTel branch-isolation tests cover context variables, baggage, and replay in their own handlers; the invocation-resume tests cover handler/step spans. Neither verifies every parent and raw export count from the actual case 22 handler deployed by this PR. Reusing only those tests would miss changes to that handler's child, parallel, and map callbacks. Running case 22 without synchronization could serialize siblings and miss a context leak. Keep four test-only barriers around the existing probes to force overlap at sibling bodies and nested steps; they do not inject or repair tracing contexts. The entry wrapper supplies unrelated ambient host instrumentation and observes restoration on the actual invocation threads. Moving this synchronization into deployed handlers or adding a shared harness would add unnecessary runtime or cross-test coupling.

Workflow and prerequisite fixes

The shared OpenTelemetry workflow and test catalog default to the conformance repository's main branch. Manual runs retain the conformance_test_ref override; SDK handlers run from the commit under test. The orchestrator runs invocation and execution suites. Legacy long-running templates and local contract tests remain, but the orchestrator no longer schedules that suite.

Cases 25 and 26 are registered in the SAM template and Makefile, and their local tests are collected by the existing root test configuration and hatch run test:cov. The prerequisite issues remain open:

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

Case 25 asserts the intended local-runner contract in #862: detailed callback-failure history includes Payload: {} with Truncated: false; metadata-only history omits the payload and sets Truncated: true. This exact empty Error.Payload service shape remains unverified against deployed AWS Lambda. The restored test inspects raw spans and records the invocation when each span is exported; timestamp-only checks cannot establish delivery timing. It uses ordinary pytest diagnostics, with no configurable proof-file output or JSON reporting mechanism.

Case 26 asserts inside the observed step that completion has already reached the exporter, and preserves raw export-count checks at both later barriers and final completion.

The additional case 17 regression remains deferred. Its source is preserved in 6484e577; before restoring it, correct its history expectations so included diagnostic payloads are untruncated and metadata-only history neither exposes payloads nor reconstructs error details.

Validation

On Python 3.13.17:

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, and test_chained_invoke_functions_point_at_their_target: include cloud cases 22–27 and case 27's targets; case 21 stays out of the cloud mappings/build targets while its shared history contract is pending.
  • test_active_callback_and_retry_handlers_keep_their_mapping: verifies that cases 25 and 26 use their restored replay handlers, alongside the existing case 17 and 24 mappings.
  • test_handler_modules_are_exactly_the_expected_set: includes the new active handlers plus case 21's retained local handler.
  • test_deferred_requirements_are_explicit: only case 21 remains pending in both views, with a reason and complete accounting for all 27 cases. Cases 25 and 26 are active requirements.
  • test_callback_probes_keep_their_actual_sdk_parent: adds unexpected helper failure/success outcomes to protect both error checks; the original successful-scenario assertions are unchanged.
  • test_opentelemetry_conformance_caller_uses_current_workflow_contract: expects the shared workflow and catalog to default to main.

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 22:06 — with GitHub Actions Active
Comment thread packages/aws-durable-execution-sdk-python-conformance-tests-otel/template.yaml Outdated
@github-actions

This comment has been minimized.

Base automatically changed from fix/otel11-replay-barrier to main October 27, 2026 23:02
Preserve the shared workflow main reference and active OTel coverage for
cases 21-24 and 27. Keep cases 25-26 deferred and retain the merged
case 11 replay fix, dependency updates, and MicroVM worker changes.

Validation: 81 focused conformance and workflow tests pass. Formatting,
lint, whitespace, and the shared workflow contract checks pass.
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 28, 2026 18:30 — with GitHub Actions Active
Defer cloud case 21 in both views until its shared history requirement
accepts both wait-completion orderings. Preserve local replay coverage.

Exercise case 23 helper failures and unexpected successes through the
public runner in both views. Each error guard is protected by tests
that fail when it is removed.

Validation: 85 focused conformance and workflow tests pass. Package
formatting, lint, and whitespace checks pass.
Comment thread packages/aws-durable-execution-sdk-python-conformance-tests-otel/template.yaml Outdated
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 28, 2026 18:35 — with GitHub Actions Active
@github-actions

This comment has been minimized.

Enable cases 25 and 26 in both cloud views and the local test suite.
Keep their regressions failing until the prerequisite fixes merge.

Preserve the callback completion export-time assertion and both later
replay barriers. Use raw span observations instead of configurable
proof-file reporting, and document the intended empty-history contract.

Validation: 89 focused tests pass and four expected regression cases
fail. Formatting, lint, and the 63 template contract tests pass.
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 28, 2026 18:47 — with GitHub Actions Active
Comment on lines +112 to +114
assert len(targets) == 1, (
"target completion must reach the exporter before the observed step"
)

This comment was marked as outdated.

Comment on lines +27 to +28
if not trace.get_current_span().get_span_context().is_valid:
raise RuntimeError(f"No active span for conformance.{label}")

This comment was marked as outdated.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 28, 2026 21:08 — with GitHub Actions Active
Comment on lines +112 to +114
assert len(targets) == 1, (
"target completion must reach the exporter before the observed step"
)

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_ysa6hd7vcryj2bv5pbshd64jxt

[P1] Land the prerequisites before merging this coverage

These tests are collected by the root testpaths and run through ordinary CI's hatch run test:cov. The base's complete_callback_success() still does not populate UpdatedOperationIds, preventing the terminal export required here. This diff contains none of the prerequisite fixes, and the PR's validation reports four failures across cases 25 and 26. Merging therefore breaks ordinary CI, beyond the cloud conformance jobs. Land the required runtime/testing fixes and rebase before merging these tests, preserving their regression assertions.

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 +27 to +28
if not trace.get_current_span().get_span_context().is_valid:
raise RuntimeError(f"No active span for conformance.{label}")

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_or7fkvw6kbgygouxnc37cdzunw

[P3] Revisit the untested probe rejection guards

Rule 4, “Every branch has a test that fails without it,” remains unmet here and in case 23's identical probe guard. Every scenario supplies an active SDK span, including the no-ambient cases; case 23's negative tests change the helper outcome, not the tracing context. Removing either rejection leaves the existing assertions unchanged. Return to the design phase under rule 8 to establish whether these guards add a necessary contract beyond the existing parentage assertions and choose the minimum demonstrably tested 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

Two unresolved findings:

  • CI prerequisites: packages/aws-durable-execution-sdk-python-conformance-tests-otel/tests/test_external_callback_completion_replay.py:112.
  • Rule 4 violation: packages/aws-durable-execution-sdk-python-conformance-tests-otel/src/otel_22_user_function_context.py:27.

Reviewed all changed files and rules 1–9. Tests were not executed during this read-only review.

Reviewed commit 2cb44c2d124117efa9da31e15d1159581d9b876d. Workflow run

This branch was successfully deployed

1 active deployment
ai-pr-review-runtime — 2cb44c2d Deployed Oct 28, 2026 by zhongkechen via ai-pr-review / Codex review / Generate Codex review #2599
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