Update Orchestrion to v1.12.0 and preserve partial uploads - #214
Update Orchestrion to v1.12.0 and preserve partial uploads#214tonyredondo wants to merge 11 commits into
Conversation
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
Please try again by commenting @autotest review.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73172ce3bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| defaultOrchestrionVersion = "v1.9.0" | ||
| defaultDDTraceGoVersion = "v2.9.0" | ||
| defaultOrchestrionVersion = "v1.12.0" | ||
| defaultDDTraceGoVersion = "v2.9.1" |
There was a problem hiding this comment.
In a newly guided workspace that accepts this default, bootstrap generates a central wrapper with orchestrion_mode = "test_optimization", but the changed integration harness itself states at tools/tests/integration/run_mock_server_tests.sh:4035-4041 that v2.9.1 lacks the required Go-testing Orchestrion package and therefore tests the hidden raw target instead. Consequently, the normal --config=test-optimization target produced by guided onboarding cannot build; use a tracer version containing that package or avoid generating this mode until one is available.
AGENTS.md reference: AGENTS.md:L168-L170
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d719c47e34
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| DD_TRACE_GO_VERSION="v2.9.1" | ||
| ORCHESTRION_VERSION="v1.12.0" |
There was a problem hiding this comment.
The advertised v1.2.0 tuple still pins commit 69953536..., whose bootstrap defaults to Orchestrion v1.9.0 and whose vendored integration predates the v1.12-specific source patches, but these two lines now advertise v2.9.1/v1.12.0. Consumers copying this published tuple therefore combine an older archive with unsupported newer tool versions instead of reproducing the release; keep the old values here until the archive commit is updated, or publish a commit containing this integration.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| defaultRulesGoRepoName = "io_bazel_rules_go" | ||
| defaultOrchestrionVersion = "v1.9.0" | ||
| defaultDDTraceGoVersion = "v2.9.0" | ||
| defaultOrchestrionVersion = "v1.12.0" |
There was a problem hiding this comment.
When guided bootstrap or WORKSPACE snippet generation receives --runtime-version below 1.25.0, this new default is still emitted even though Orchestrion v1.12.0 requires Go 1.25.0 or newer. Guided bootstrap can consequently finish after modifying MODULE.bazel, go.mod, and generated files, only for the first enabled Bazel build to fail while building Orchestrion with the requested older SDK; validate the selected Orchestrion/runtime pair before any writes.
Useful? React with 👍 / 👎.
tonyredondo
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3222be11d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [[ "$DO_UPLOAD" -eq 1 ]]; then | ||
| upload_runtime_args=("${runtime_args[@]}") | ||
| if [[ -n "$UPLOAD_REPORT_JSON" ]]; then | ||
| upload_runtime_args+=("--report-json=$UPLOAD_REPORT_JSON") |
There was a problem hiding this comment.
When --upload is used and the dry run fails because --validate-enrichment found missing required tags, this unconditional branch immediately reruns the uploader without --validate-enrichment; the normal upload path only calls validate_payload, so the same enrichment-invalid payload is posted rather than skipped while other valid payloads continue. The PowerShell wrapper has the same behavior. Preserve the dry-run per-payload result or enforce equivalent validation during the real upload so only fresh valid payloads are sent.
AGENTS.md reference: AGENTS.md:L158-L161
Useful? React with 👍 / 👎.
| if !stale { | ||
| continue |
There was a problem hiding this comment.
When a cache-producing action holds this lock for longer than cacheLockStaleAfter (10 minutes), a waiter now loops past the 60-second timeout until the lock ages into the stale state, removes it even though the original owner is still running, and acquires a second lock. The original owner's deferred release can then delete the new owner's lock, allowing concurrent promoteCacheTempDir calls that remove and replace the same cache entry while readers use it. Long Orchestrion builds should either time out without stealing an active lock or maintain an owner/heartbeat mechanism; the identical change affects all three vendored forks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Two edge cases can lose valid data. A missing BEP stops partial uploads, and a cache owner that runs for more than ten minutes can lose its active lock.
🤖 Datadog Autotest · Commit 3222be1 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| fi | ||
|
|
||
| if [[ "$doctor_status" -eq 0 && "$dry_run_status" -eq 0 && "$DO_UPLOAD" -eq 1 ]]; then | ||
| if [[ "$DO_UPLOAD" -eq 1 ]]; then |
There was a problem hiding this comment.
One failed or interrupted target can prevent the upload of valid sibling failure data.
Assertion details
- Input: One test invocation does not create a usable BEP. Another invocation creates a valid fresh payload and BEP.
- Expected:
Report the bad target. Upload valid fresh payloads from other targets. Apply the fix to the Bash, PowerShell, and generated Go flows. Add missing-BEP coverage for each flow. - Actual:
The upload call passes all planned BEP paths. Required artifact staging exits on the first missing or invalid BEP before it scans valid payloads.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| } | ||
| continue | ||
| } | ||
| if !stale { |
There was a problem hiding this comment.
Concurrent writers can corrupt the shared Orchestrion cache or read incomplete cache content.
Assertion details
- Input: Two actions use the same cache entry. The first action holds the lock for more than ten minutes.
- Expected:
Do not remove a lock while its owner is active. Use an owner check or a heartbeat. Apply the fix to all three vendored rules_go trees and generated patches. Add a test where the owner stays active past the stale limit. - Actual:
The new retry loop waits until the lock directory is ten minutes old. It then removes the directory without proof that the owner stopped. The first owner can later remove a new owner's lock too.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba0c3dd263
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [[ "$DO_UPLOAD" -eq 1 ]]; then | ||
| upload_runtime_args=("${runtime_args[@]}") | ||
| if [[ -n "$UPLOAD_REPORT_JSON" ]]; then | ||
| upload_runtime_args+=("--report-json=$UPLOAD_REPORT_JSON") |
There was a problem hiding this comment.
When one target has a remote-only test.outputs reference while another has a valid local payload, this unconditional upload attempt still cannot deliver the local payload: the wrapper always passes --freshness-mode=required, and validate_bep_remote_only_outputs exits before any upload loop as soon as it sees the remote-only row. Thus both the dry run and this real-upload invocation abort wholesale instead of preserving the available payload; the PowerShell path has the same behavior. Handle the unavailable row as a per-output failure and continue with locally available fresh outputs while preserving the failure status.
AGENTS.md reference: AGENTS.md:L158-L161
Useful? React with 👍 / 👎.
1 participant
Summary
Orchestrion v1.12.0 requires Go 1.25.0 or newer.
Why
The v1.12 resolver tracks test variants for generated test mains and recursively resolves the correct dependency archives. The previous v1.9 integration disabled that recursive toolexec path for Bazel compatibility; this update keeps the upstream behavior and narrows the local patch to Bazel filesystem and invocation-context differences.
The offline proxy downloads both the resolved module graph and the exact configured dd-trace-go pins. This keeps normal Go module resolution intact while ensuring synthetic Orchestrion dependencies can resolve a consumer's prerelease pin without network access.
A failed test can still produce a valid Test Optimization payload containing the failure details. Validation failures and missing sibling outputs are therefore reported without discarding valid fresh payloads from the same invocation. The final status still reports the earliest test, doctor, dry-run, or upload failure.
The resolver source downloaded by the repository rule can use the checkout's native line endings. Building the replacement text with the detected line ending keeps the same patch semantics on Linux, macOS, and Windows.
Validation
python3 tools/dev/generate_rules_go_fork_maps.py --checkpython3 tools/dev/materialize_rules_go_fork.py check --allpython3 tools/dev/check_release_archive_contents.pypython3 tools/dev/verify_rules_go_profiles.py --public-denylist tools/dev/private_leak_public_denylist.txt --bazel <macOS Bazel wrapper>v2.9.1-rc.3offline-proxy coverage./bazelw test //... --noexperimental_split_xml_generation(476 tests passed before the partial-upload follow-up)./bazelw test //tools/tests/python:python_tools_test --noexperimental_split_xml_generation./bazelw test //modules/go/tools/dd_topt_go_bootstrap:bootstrap_test --noexperimental_split_xml_generation