feat(otel): add in-repo OTel conformance handlers - #664
Conversation
| 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], | ||
| ) |
There was a problem hiding this comment.
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.
| 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], | |
| ) |
Codex AI reviewFound one correctness issue in the full long-running retry path. Current tests only verify static wiring, so effective retry timing remains uncovered. Reviewed commit |
| 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], | ||
| ) |
There was a problem hiding this comment.
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.
| 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], | |
| ) |
Claude AI reviewThis 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:
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 Reviewed commit |
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.