ci: reuse OpenTelemetry conformance workflows - #69
Conversation
This comment has been minimized.
This comment has been minimized.
Claude AI reviewThis PR replaces the duplicated per-language OTel workflows (Java/Python/TypeScript) with three shared reusable workflows ( One confirmed regression: the long-running worker does not define Residual test risk: the workflow contract tests only parse/assert YAML text and never execute the reusable workflows, so a missing runtime environment variable such as Reviewed commit |
…orkflows # Conflicts: # .github/workflows/opentelemetry-suite.yml # packages/aws-durable-execution-conformance-tests-otel/tests/test_java_examples.py
| OTEL_VIEW: ${{ inputs.view }} | ||
| STATE_ARTIFACT: python-otel-long-running-${{ inputs.view }}-${{ inputs.aws_region || 'us-west-2' }}-state | ||
| TEST_NAME: p-olr-${{ inputs.view == 'invocation' && 'i' || 'e' }}${{ inputs.phase == 'short' && '-short' || '' }} | ||
| STATE_ARTIFACT: ${{ inputs.language }}-otel-long-running-${{ inputs.view }}-${{ inputs.aws_region || 'us-west-2' }}-state |
There was a problem hiding this comment.
Codex AI review
High: The JavaScript preset now searches for javascript-otel-long-running-*, but the deleted workflow stored active runs as typescript-otel-long-running-* with a typescript-otel-long-running-state.json payload. The first post-merge daily run will miss that state, launch a new js-olr-* stack, and leave the existing t-olr-* execution unchecked. Add a legacy artifact/file-key input, fall back to and migrate the TypeScript artifact before choosing launch, and test this transition.
| collector_compatible_runtime: nodejs22.x | ||
| collector_otlp_endpoint: http://localhost:4318 | ||
| # Clean up rolled-back stacks created before the JavaScript rename. | ||
| legacy_stack_prefix: durable-execution-conformance-tests-typescript |
There was a problem hiding this comment.
Codex AI review
Medium: This preserves only the older durable-execution-conformance-tests-typescript-* fallback. The workflow being removed owns conformance-tests-typescript-{backend}-{inv|exec}, and the S3 job does not consume this input. Those stable --no-cleanup stacks will become unreachable, including rollback-complete stacks that the old workflow would retire. Preserve a separate stable resource key or explicitly handle the actual TypeScript backend/view and S3 names during migration.
| if [[ ! "$LANGUAGE" =~ ^[a-z][a-z0-9-]{0,19}$ ]]; then | ||
| echo "language must contain 1-20 lowercase resource-safe characters" | ||
| exit 1 | ||
| fi | ||
| if [[ ! "$RESOURCE_PREFIX" =~ ^[a-z][a-z0-9-]{0,11}$ ]]; then | ||
| echo "resource_prefix must contain 1-12 lowercase resource-safe characters" | ||
| exit 1 |
There was a problem hiding this comment.
Codex AI review
Medium: These accepted lengths exceed Lambda's 64-character function-name limit. The mandatory Dash0 invocation suite generates conformance-tests-${language}-dash0-inv-otel-invocation-18-target, allowing at most 10 language characters, while a short long-running run allows at most 7 resource_prefix characters. Datadog is stricter still. Derive resource names from a validated short key or tighten validation per enabled backend/phase, and add boundary tests for every generated function name.
Codex AI reviewFound three workflow issues affecting JavaScript migration and reusable input safety. Live GitHub Actions/AWS behavior remains a residual risk because review was read-only. Reviewed commit |
Summary
languageas the single runtime/resource key and rename TypeScript runtime-facing paths to JavaScriptTesting
hatch run test:all(341 passed)hatch run types:checkhatch fmt --checkactionlint .github/workflows/*.ymlgit diff --checknpm run typechecknpm run build