feat(civisibility): add process-isolated test retries#5020
feat(civisibility): add process-isolated test retries#5020tonyredondo wants to merge 25 commits into
Conversation
Config Audit |
|
BenchmarksBenchmark execution time: 2026-07-16 11:21:40 Comparing candidate commit 4ba55cc in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 326 metrics, 0 unstable metrics, 1 flaky benchmarks without significant changes.
|
Keep timeout setup failures terminal, preserve fatal subtest panic semantics, make lifecycle tests deterministic, and restrict feature documentation to the CI Visibility-owned area.
|
@codex 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: ce6a69affe
ℹ️ 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".
Support process-isolated retries for EFD, Attempt-to-Fix, coverage-aware children, and Orchestrion-owned tests. Preserve coordinated in-process fallback when a parallel EFD batch cannot launch a child.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0069e75455
ℹ️ 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".
|
|
||
| // CIVisibilityRetryExecutionModeEnvironmentVariable selects how retry attempts are executed. | ||
| // Supported values are "in_process" and "process". The default is "in_process". | ||
| CIVisibilityRetryExecutionModeEnvironmentVariable = "DD_CIVISIBILITY_RETRY_EXECUTION_MODE" |
There was a problem hiding this comment.
This adds a new public DD_CIVISIBILITY_RETRY_* configuration surface, but the commit does not update the top-level README.md or CONTRIBUTING.md. The repository AGENTS.md requires those docs for significant tracer customization features and important testing/building options; without them, users and contributors will not discover process retry mode or its validation guidance. Please add the required top-level documentation alongside these new env vars.
Useful? React with 👍 / 👎.
| "DD_CIVISIBILITY_INTERNAL_RETRY_PROCESS_ATTEMPT": {}, | ||
| "DD_CIVISIBILITY_INTERNAL_RETRY_PROCESS_CHILD": {}, |
There was a problem hiding this comment.
Registering the private DD_CIVISIBILITY_INTERNAL_RETRY_PROCESS_* transport keys as supported makes a user-provided environment marker pass internal/env validation and activate child mode during package init. If DD_CIVISIBILITY_INTERNAL_RETRY_PROCESS_CHILD=true leaks into a normal test environment without the matching result-path/test-name fields, the gotesting child path treats the process as a malformed retry child and can clear workloads or exit instead of running the parent test session. These parent-to-child transport variables should be read through a private path rather than advertised in the supported configuration allowlist.
Useful? React with 👍 / 👎.
Bits has a CI fix ready🟢 Investigated · 🟢 Fix prepared · ⚪ Validation skipped · 🟠 Ready
View in Datadog | Reviewed commit 78320b7 |
![datadog-datadog-us1-prod[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The isLastRetry semantics change from == 1 to == 0 is correct for the pre-reservation model: reserve() decrements the global FTR budget atomically before returning, so a budget-of-1 produces remaining=0 post-reservation, and the == 0 check correctly identifies it as the last retry. The race-detected suite (FTR atomicity, budget sharing, EFD fallback, process workload isolation) passes cleanly; six new adversarial tests added covering the five highest-risk properties confirmed no behavioral regressions.
📊 Validated against 6 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 4ba55cc · What is Autotest? · Any feedback? Reach out in #autotest
1 participant
What does this PR do?
This PR adds an experimental, opt-in process-isolated execution mode for Go test retries.
The first test execution remains in the parent process. Each later retry runs by re-executing the same test binary in a fresh subprocess, while the parent remains the sole owner of CI Visibility lifecycle and telemetry.
Supported retry families
Process mode supports parent-owned retries for top-level tests across:
The mode works with both explicit
gotesting.RunM(m)instrumentation and Orchestrion-owned test instrumentation, including the hybrid case where both entrypoints are present.Retry execution model
For every eligible retry, the parent:
The child does not initialize or flush CI Visibility. Manual CI Visibility APIs return no-op objects, child-side Test Optimization helpers preserve native
testing.Tbehavior without CI Visibility side effects, and retry spans remain parent-owned.Structured child results preserve pass/fail/skip state, skip reasons, panic and ordinary failure metadata, runtime-Goexit handling, timing, and retry identity. Parent retry spans are tagged with
test.retry.execution_mode=processand expose process failure kinds without forwarding raw private output as error metadata.Retry accounting and EFD behavior
test body + user cleanupduration, excluding CI Visibility and coverage-processing overhead.Coverage behavior
Coverage remains supported with a first-attempt-only contract:
GOCOVERDIR; andProcess lifecycle and containment
Compatibility and fallback
The existing
in_processbackend remains the default.Process mode falls back to in-process retries before consuming a process attempt when isolation is unavailable or unsafe, including:
shuffle=onor ambiguous test-binary arguments;testing.Tortesting.Mlayouts; andOnce a child attempt has been consumed, process failures stay represented as that retry attempt rather than silently re-running the same attempt in-process.
A fresh process intentionally reruns package initialization and pre-
m.Run()TestMaincode. The selected test body is still the only test workload executed by the retry child.Configuration
DD_CIVISIBILITY_RETRY_EXECUTION_MODEin_processprocessto enable process-isolated retries.DD_CIVISIBILITY_RETRY_PROCESS_MAX_CONCURRENCY1DD_CIVISIBILITY_RETRY_PROCESS_TIMEOUT10m, reduced by a shorter parent deadline.DD_CIVISIBILITY_EARLY_FLAKE_DETECTION_ENABLEDtruefalsedisables EFD even when remote settings enable it.DD_CIVISIBILITY_EARLY_FLAKE_DETECTION_MAX_RETRIES-10disables EFD retries while keeping EFD classification.DD_CIVISIBILITY_INTERNAL_PARALLEL_EARLY_FLAKE_DETECTION_ENABLEDfalseThe private
DD_CIVISIBILITY_INTERNAL_RETRY_PROCESS_*variables are child-transport implementation details, not user-facing configuration.Tests and benchmarks
The PR adds focused coverage for:
RunM, Orchestrion-only, and hybrid ownership;Benchmarks compare:
in_processversusprocessretry overhead; andMotivation
In-process retries reuse the same Go process, so package globals, leaked goroutines, mutated runtime state, and other process-wide side effects can survive between attempts. A fresh test-binary subprocess provides a stronger isolation boundary while retaining parent-owned CI Visibility spans, retry accounting, final status, and logging.
The process mode is intentionally opt-in and keeps conservative fallbacks for execution shapes where the library cannot guarantee safe selection, containment, or cleanup.
Validation
Validated locally on the final implementation with:
Additional notes:
go.modorgo.sumchanges;internal/civisibility; and