Skip to content

feat(otel): add in-repo OTel conformance handlers - #664

Merged
ayushiahjolia merged 2 commits into
mainfrom
feat/otel-conformance-handlers
Aug 20, 2026
Merged

feat(otel): add in-repo OTel conformance handlers#664
ayushiahjolia merged 2 commits into
mainfrom
feat/otel-conformance-handlers

Conversation

@ayushiahjolia

Copy link
Copy Markdown
Contributor

Issue #, if available:
aws/aws-durable-execution-conformance-tests#90

Description of changes:
Port the Python OTel conformance handlers and SAM templates from aws/aws-durable-execution-conformance-tests into a new package so a PR runs them against its own commit, mirroring the non-OTel conformance package and the Java SDK's conformance-tests-otel module.

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

@ayushiahjolia
ayushiahjolia marked this pull request as ready for review August 20, 2026 18:21
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 20, 2026 18:42 — with GitHub Actions Inactive
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 20, 2026 18:42 — with GitHub Actions Inactive
@ayushiahjolia
ayushiahjolia merged commit edc9cb4 into main Aug 20, 2026
19 of 27 checks passed
@ayushiahjolia
ayushiahjolia deleted the feat/otel-conformance-handlers branch August 20, 2026 19:06
Comment on lines +34 to +41
retry_strategy = create_retry_strategy(
RetryStrategyConfig(
max_attempts=2,
initial_delay=Duration.from_seconds(long_delay_seconds(event)),
backoff_rate=1.0,
jitter_strategy=JitterStrategy.NONE,
retryable_error_types=[RuntimeError],
)

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

RetryStrategyConfig.max_delay defaults to 300 seconds, so create_retry_strategy caps the requested delay. The default 82,800-second run therefore retries after five minutes and completes hours early, breaking otel-long-running-2. Set max_delay to the requested delay and add coverage for delays above 300 seconds.

Suggested change
retry_strategy = create_retry_strategy(
RetryStrategyConfig(
max_attempts=2,
initial_delay=Duration.from_seconds(long_delay_seconds(event)),
backoff_rate=1.0,
jitter_strategy=JitterStrategy.NONE,
retryable_error_types=[RuntimeError],
)
delay = Duration.from_seconds(long_delay_seconds(event))
retry_strategy = create_retry_strategy(
RetryStrategyConfig(
max_attempts=2,
initial_delay=delay,
max_delay=delay,
backoff_rate=1.0,
jitter_strategy=JitterStrategy.NONE,
retryable_error_types=[RuntimeError],
)

@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

Found one correctness issue in the full long-running retry path. Current tests only verify static wiring, so effective retry timing remains uncovered.

Reviewed commit bfd6f104f6dce7b13e2437a42e2ea6ef6bc7ab3b. Workflow run

Comment on lines +35 to +41
RetryStrategyConfig(
max_attempts=2,
initial_delay=Duration.from_seconds(long_delay_seconds(event)),
backoff_rate=1.0,
jitter_strategy=JitterStrategy.NONE,
retryable_error_types=[RuntimeError],
)

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.

Claude AI review

The long retry backoff is silently capped at 5 minutes, so otel-long-running-2 never exercises a long durable backoff.

RetryStrategyConfig.max_delay defaults to Duration.from_minutes(5) (300s), and this config only overrides initial_delay. In create_retry_strategy the effective delay is min(initial_delay_seconds, max_delay_seconds), so with the launch-phase default delay_seconds=82800 the step retries after 300s instead of ~23 hours. Every other long-running handler (otel_long_running_1_wait, _3_callback, _4_chained_invoke) honors the full long_delay_seconds(event) via a direct context.wait, so this handler is inconsistent and under-tests the scenario it exists to verify.

Fix: also set max_delay to the requested delay so the backoff is not clamped.

Suggested change
RetryStrategyConfig(
max_attempts=2,
initial_delay=Duration.from_seconds(long_delay_seconds(event)),
backoff_rate=1.0,
jitter_strategy=JitterStrategy.NONE,
retryable_error_types=[RuntimeError],
)
RetryStrategyConfig(
max_attempts=2,
initial_delay=Duration.from_seconds(long_delay_seconds(event)),
max_delay=Duration.from_seconds(long_delay_seconds(event)),
backoff_rate=1.0,
jitter_strategy=JitterStrategy.NONE,
retryable_error_types=[RuntimeError],
)

@github-actions

Copy link
Copy Markdown
Contributor

Claude AI review

This PR ports the Python OTel conformance handlers and SAM templates in-repo and rewires the CI workflow to run them from the commit under test. The wiring is well-tested (template/handler/Makefile contract tests, workflow-contract tests) and the handlers use the SDK's public API correctly in almost every case.

One confirmed correctness issue:

  • packages/aws-durable-execution-sdk-python-conformance-tests-otel/src/otel_long_running_2_retry.py:35-41 — the long retry backoff is silently capped at 5 minutes because RetryStrategyConfig.max_delay defaults to Duration.from_minutes(5) and is not overridden, while initial_delay is set to the (up to 86400s) requested delay. create_retry_strategy takes min(initial_delay, max_delay), so the "long durable backoff" this scenario verifies never actually occurs on the multi-hour launch/check cycle.

Residual test risk: the in-repo contract tests validate template/handler wiring and static structure only; they cannot catch runtime backoff semantics like the capped max_delay above, and the actual span assertions live in the external conformance runner, so the long-running suite's real behavior is not exercised by anything merged in this repo.

Reviewed commit bfd6f104f6dce7b13e2437a42e2ea6ef6bc7ab3b. 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.

2 participants