Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
60dd643
feat(civisibility): add process-isolated test retries
tonyredondo Jul 11, 2026
4bf30fb
fix(civisibility): harden process retry cleanup
tonyredondo Jul 11, 2026
6a19dce
fix(civisibility): stabilize process retry tests
tonyredondo Jul 11, 2026
a6788f2
fix(civisibility): harden process retry behavior
tonyredondo Jul 12, 2026
ee69464
refactor(civisibility): consolidate process retry implementation
tonyredondo Jul 12, 2026
e6e4902
refactor(civisibility): deduplicate process retry harnesses
tonyredondo Jul 12, 2026
22a804b
test(civisibility): benchmark retry execution modes
tonyredondo Jul 12, 2026
464d287
refactor(civisibility): isolate orchestrion entrypoints
tonyredondo Jul 13, 2026
f488151
refactor(civisibility): reduce process retry implementation
tonyredondo Jul 13, 2026
ce6a69a
fix(civisibility): register process retry transport settings
tonyredondo Jul 13, 2026
d5d7a6f
fix(civisibility): stabilize process retry validation
tonyredondo Jul 13, 2026
f36e598
Merge branch 'main' into tony/feat/process-isolated-test-retries
tonyredondo Jul 13, 2026
1026824
Merge branch 'main' into tony/feat/process-isolated-test-retries
tonyredondo Jul 13, 2026
3657b45
feat(civisibility): support process retries for EFD and A2F
tonyredondo Jul 13, 2026
c26f7e5
feat(civisibility): expand process-isolated retry support
tonyredondo Jul 14, 2026
0069e75
fix(civisibility): satisfy retry fixture lint
tonyredondo Jul 14, 2026
2579888
fix(civisibility): preserve retry shutdown semantics
tonyredondo Jul 14, 2026
081c46e
perf(civisibility): improve parallel EFD process retries
tonyredondo Jul 15, 2026
2a212c5
feat(civisibility): add EFD kill switch
tonyredondo Jul 15, 2026
78320b7
feat(civisibility): add EFD retry limit
tonyredondo Jul 15, 2026
9e4279a
fix(civisibility): align EFD config implementation
tonyredondo Jul 15, 2026
e240947
chore(civisibility): preserve config formatting
tonyredondo Jul 15, 2026
85024b3
test(civisibility): stabilize process retry timing tests
tonyredondo Jul 15, 2026
3371da2
fix(civisibility): prevent nested retries in process children
tonyredondo Jul 15, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions internal/civisibility/CIVISIBILITY_OVERVIEW.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
- `testing.go`, `testingT.go`, `testingB.go`, and related files manage session/module/suite creation, attach tags (including module/suite counters), handle chatty output, skip logic, coverage capture, log streaming, and telemetry emission. They integrate with `integrations.GetSettings()`, `net` clients, and `logs`.
- Hierarchical identity plumbing: `testIdentity` (module, suite, base name, full name, path segments) plus `matchTestManagementData` allow subtests to resolve directives like `TestParent/SubChild`. `getTestManagementData` reports whether a directive was an exact match or inherited from an ancestor.
- `instrumentation.go` wires wrappers around test functions, stores execution metadata (retries, new/modified flags, quarantined/disabled states, attempt-to-fix ownership), and coordinates with backend settings for retries/EFD/ITR. It leverages `unsafe` pointers and reflective lookup to map `testing` internals, guarded by `sync` primitives. Subtests consult the parent execution metadata to decide whether they should wrap themselves or defer to the parent-run retry loop.
- Process-isolated retry mode is opt-in through `DD_CIVISIBILITY_RETRY_EXECUTION_MODE=process` when Test Optimization enables a supported retry family. The first attempt still runs in the parent process. Auto Test Retries, Early Flake Detection, and Attempt-to-Fix retries owned by top-level tests instrumented through explicit `gotesting.RunM` or Orchestrion can run later attempts in fresh test-binary subprocesses, while the parent remains the only owner of CI Visibility session/module/suite/test spans. Parallel EFD launches retry subprocesses concurrently within both the EFD concurrency bound and the global process-retry limiter. Attempt-to-Fix takes precedence over EFD and Auto Test Retries when their metadata overlaps. When Go coverage is active, only the first parent attempt contributes coverage; retry children omit coverage output flags and `GOCOVERDIR`, so their counters are not merged into the parent profile. Children skip CI Visibility initialization, direct manual CI Visibility APIs return no-op objects, and child-side Test Optimization helpers suppress CI Visibility side effects and return a background context while preserving native Go test-control behavior. Child-mode `InitializeCIVisibilityMock` returns a non-nil no-op tracer whose `StartSpan` method returns `nil`. The child writes only structured result JSON; the parent captures stdout/stderr into bounded private files, translates the result into retry spans tagged with `test.retry.execution_mode=process`, forwards bounded output through the normal CI Visibility test-log path when logs are enabled, and deletes private temp files after bookkeeping. Process mode falls back to the existing in-process behavior for subtest-owned retry identities, benchmarks, fuzzing, `shuffle=on`, ambiguous argv cases, unsupported process-tree containment, and pre-consumption subprocess setup failures. Private Go `testing.T` or `testing.M` layout drift makes process mode ineligible with `testing_t_layout_unsupported` or `testing_m_layout_unsupported`; timeout, missing or malformed child results, unreaped child state, containment loss, and non-zero child exit are failed process retry attempts. The process-attempt timeout starts after concurrency-slot acquisition and covers subprocess preparation, launch, and execution; queueing is bounded by the parent deadline, and a complete retry group can exceed one process-attempt timeout. The raw timeout configuration default is `""` and its effective runtime default is `10m`; a shorter positive parent test deadline reduces that limit.
- Process retry children are re-executed Go test binaries, not forked test-function RPC workers. Package `init` and user `TestMain` setup before `m.Run()` run again before the child-mode hook can clear root workloads. CI Visibility integrations snapshot and scrub private transport keys during their own package initialization, so test bodies and later descendants see a clean environment, but unrelated packages may initialize earlier and observe that internal metadata. Resources kept open by the still-running parent can conflict with child startup. Child result JSON `pass` or `skip` is overridden by non-zero subprocess exit as `process_exit`; child `fail` or structured panic keeps its child test failure classification unless stronger subprocess evidence such as signal termination, timeout, unreaped state, missing result, or malformed result wins. An unreaped child stops its retry group and disables later process launches for the test binary; groups with no consumed process attempt continue through in-process retries. Child attempts use parent-owned private temp directories/files, no stdin, controlled `-test.timeout`, bounded output drains, and portable process-containment helpers. Those helpers terminate ordinary descendants that remain in the containment unit; deliberately detached or daemonized descendants are outside the V1 cleanup contract and remain the test's responsibility.
- Attempt-to-fix ownership rules:
1. Parent-only directives orchestrate retries and tag success/failure; children inherit attempt-to-fix tagging but emit no retry spans.
2. Child-only directives wrap the subtest locally (when feature flag enabled and exact match present) while leaving the parent neutral.
Expand Down Expand Up @@ -88,5 +90,6 @@

## Getting Involved
- When touching `integrations/gotesting`, run both the legacy controller suite and the subtest matrix (`go test ./internal/civisibility/integrations/gotesting/...`). Many scenarios spawn subprocesses; enable debug logging for verbose traces.
- For process-isolated retry changes, run `Bypass=true go test ./internal/civisibility/integrations/gotesting -run 'TestProcessRetryChildCleanupSupported|TestProcessRetryTestingMReflectionDrift|TestProcessRetryEligible|TestProcessRetryChildResult(Fail|Skip)|TestRunTestWithRetryFallsBackBeforeConsumingProcessAttempt|TestRunTestWithRetryUnreapedChildStopsFurtherProcessRetries|TestRunProcessRetryAttemptHonorsParentDeadlineWhileWaitingForLimiter' -count=1`, `go test ./internal/civisibility/integrations/gotesting/retryprocess -count=1 -v`, `go test -cover ./internal/civisibility/integrations/gotesting/retryprocess -run '^TestProcessRetryCoverageUsesFirstParentAttempt$' -count=1`, and `(cd internal/orchestrion/_integration && GOWORK=off go run -mod=readonly github.com/DataDog/orchestrion go test -mod=readonly ./retryprocess -count=1)`.
- Any change to retry ownership or metadata propagation should be mirrored in the harness scenarios and in `docs/SUBTEST_FEATURE_IMPLEMENTATION.md` to keep documentation synchronized.
- Utility changes often require updating fixtures or provider expectations; leverage the existing test suites instead of ad-hoc scripts.
33 changes: 33 additions & 0 deletions internal/civisibility/constants/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ const (
// This environment variable should be set to "0" or "false" to disable the flaky retry feature.
CIVisibilityFlakyRetryEnabledEnvironmentVariable = "DD_CIVISIBILITY_FLAKY_RETRY_ENABLED"

// CIVisibilityEarlyFlakeDetectionEnabledEnvironmentVariable kill-switch that allows explicitly disabling EFD even if the remote setting is enabled.
// This environment variable should be set to "0" or "false" to disable the early flake detection feature.
CIVisibilityEarlyFlakeDetectionEnabledEnvironmentVariable = "DD_CIVISIBILITY_EARLY_FLAKE_DETECTION_ENABLED"

// CIVisibilityEarlyFlakeDetectionMaxRetriesEnvironmentVariable caps the number of EFD retries per test without increasing the backend setting.
// A negative value preserves the retry counts configured by the backend.
CIVisibilityEarlyFlakeDetectionMaxRetriesEnvironmentVariable = "DD_CIVISIBILITY_EARLY_FLAKE_DETECTION_MAX_RETRIES"

// CIVisibilityGitUploadEnabledEnvironmentVariable kill-switch that allows explicitly disabling CI Visibility repository upload.
// This environment variable should be set to "0" or "false" to skip uploading git metadata while keeping CI Visibility enabled.
CIVisibilityGitUploadEnabledEnvironmentVariable = "DD_CIVISIBILITY_GIT_UPLOAD_ENABLED"
Expand All @@ -47,6 +55,31 @@ const (
// CIVisibilityTotalFlakyRetryCountEnvironmentVariable indicates the maximum number of retry attempts for the entire session.
CIVisibilityTotalFlakyRetryCountEnvironmentVariable = "DD_CIVISIBILITY_TOTAL_FLAKY_RETRY_COUNT"

// 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"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Document new retry process configuration

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 👍 / 👎.


Comment thread
tonyredondo marked this conversation as resolved.
// CIVisibilityRetryProcessTimeoutEnvironmentVariable optionally bounds each process retry child attempt.
CIVisibilityRetryProcessTimeoutEnvironmentVariable = "DD_CIVISIBILITY_RETRY_PROCESS_TIMEOUT"

// CIVisibilityRetryProcessMaxConcurrencyEnvironmentVariable optionally caps concurrent process retry children.
CIVisibilityRetryProcessMaxConcurrencyEnvironmentVariable = "DD_CIVISIBILITY_RETRY_PROCESS_MAX_CONCURRENCY"

// CIVisibilityInternalRetryProcessChild marks a re-executed test binary as a process-retry child.
CIVisibilityInternalRetryProcessChild = "DD_CIVISIBILITY_INTERNAL_RETRY_PROCESS_CHILD"

// CIVisibilityInternalRetryProcessResultPath carries the child result JSON path.
CIVisibilityInternalRetryProcessResultPath = "DD_CIVISIBILITY_INTERNAL_RETRY_PROCESS_RESULT_PATH"

// CIVisibilityInternalRetryProcessTestName carries the selected top-level test name for a child retry.
CIVisibilityInternalRetryProcessTestName = "DD_CIVISIBILITY_INTERNAL_RETRY_PROCESS_TEST_NAME"

// CIVisibilityInternalRetryProcessAttempt carries the retry attempt index for a child retry.
CIVisibilityInternalRetryProcessAttempt = "DD_CIVISIBILITY_INTERNAL_RETRY_PROCESS_ATTEMPT"

// CIVisibilityInternalRetryProcessReason carries the retry reason for a child retry.
CIVisibilityInternalRetryProcessReason = "DD_CIVISIBILITY_INTERNAL_RETRY_PROCESS_REASON"

// CIVisibilityTestManagementEnabledEnvironmentVariable indicates if the test management feature is enabled.
CIVisibilityTestManagementEnabledEnvironmentVariable = "DD_TEST_MANAGEMENT_ENABLED"

Expand Down
3 changes: 3 additions & 0 deletions internal/civisibility/constants/test_tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ const (
// TestRetryReason indicates the reason for retrying the test
TestRetryReason = "test.retry_reason"

// TestRetryExecutionMode indicates how a retry attempt was executed.
TestRetryExecutionMode = "test.retry.execution_mode"

// TestEarlyFlakeDetectionRetryAborted indicates a retry abort reason by the early flake detection feature
TestEarlyFlakeDetectionRetryAborted = "test.early_flake.abort_reason"

Expand Down
65 changes: 61 additions & 4 deletions internal/civisibility/integrations/civisibility.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,16 @@ var (

// closeActions holds CI visibility close actions.
closeActions []ciVisibilityCloseAction

// closeActionsMutex synchronizes access to closeActions.
// preCloseActions holds barriers that must finish before regular close actions run.
preCloseActions []ciVisibilityCloseAction
// ciVisibilityShutdownDone closes after the current shutdown owner finishes.
ciVisibilityShutdownDone chan struct{}
// waitForCIVisibilityShutdown blocks shutdown callers that do not own the
// Initialized -> Exiting transition. It is a variable for deterministic tests.
waitForCIVisibilityShutdown = func(done <-chan struct{}) { <-done }

// closeActionsMutex synchronizes access to closeActions, preCloseActions, and
// ciVisibilityShutdownDone.
closeActionsMutex sync.Mutex

// mTracer contains the mock tracer instance for testing purposes
Expand All @@ -81,6 +89,9 @@ func EnsureCiVisibilityInitialization() {

// InitializeCIVisibilityMock initialize the mocktracer for CI Visibility usage
func InitializeCIVisibilityMock() mocktracer.Tracer {
if IsProcessRetryChild() {
return &processRetryNoopMockTracer{}
}
internalCiVisibilityInitialization(func([]tracer.StartOption) {
// Set the library to test mode
civisibility.SetTestMode()
Expand All @@ -92,6 +103,9 @@ func InitializeCIVisibilityMock() mocktracer.Tracer {

// internalCiVisibilityInitialization runs the one-time CI Visibility bootstrap and wires the selected tracer initializer into it.
func internalCiVisibilityInitialization(tracerInitializer func([]tracer.StartOption)) {
if IsProcessRetryChild() {
return
}
ciVisibilityInitializationOnce.Do(func() {
civisibility.SetState(civisibility.StateInitializing)
defer civisibility.SetState(civisibility.StateInitialized)
Expand Down Expand Up @@ -163,6 +177,9 @@ func (handler *ciVisibilitySignalHandler) run() {
return
}
exitCiVisibility(false)
if handler.stopping.Load() {
return
}
ciVisibilitySignalExitFunc(1)
case <-handler.stop:
return
Expand Down Expand Up @@ -258,33 +275,73 @@ func PushCiVisibilityCloseAction(action ciVisibilityCloseAction) {
closeActions = append([]ciVisibilityCloseAction{action}, closeActions...)
}

// TryPushCiVisibilityPreCloseAction adds a barrier that runs before regular
// close actions, without holding closeActionsMutex while the barrier executes.
func TryPushCiVisibilityPreCloseAction(action ciVisibilityCloseAction) bool {
closeActionsMutex.Lock()
defer closeActionsMutex.Unlock()
state := civisibility.GetState()
if state == civisibility.StateExiting || state == civisibility.StateExited {
return false
}
preCloseActions = append([]ciVisibilityCloseAction{action}, preCloseActions...)
return true
}

// ExitCiVisibility executes all registered close actions and stops the tracer.
func ExitCiVisibility() {
if IsProcessRetryChild() {
return
}
markCIVisibilitySignalHandlerStopping()
exitCiVisibility(true)
}

// exitCiVisibility executes CI Visibility shutdown and optionally stops the
// signal handler. Signal-triggered shutdown skips that wait to avoid self-deadlock.
func exitCiVisibility(stopSignalHandler bool) {
closeActionsMutex.Lock()
if civisibility.GetState() != civisibility.StateInitialized {
done := ciVisibilityShutdownDone
closeActionsMutex.Unlock()
log.Debug("civisibility: already closed or not initialized")
if done != nil {
waitForCIVisibilityShutdown(done)
}
if stopSignalHandler {
stopCIVisibilitySignalHandler()
}
return
}

civisibility.SetState(civisibility.StateExiting)

done := make(chan struct{})
ciVisibilityShutdownDone = done
barriers := append([]ciVisibilityCloseAction(nil), preCloseActions...)
preCloseActions = nil
closeActionsMutex.Unlock()
if stopSignalHandler {
defer stopCIVisibilitySignalHandler()
}
defer civisibility.SetState(civisibility.StateExited)
defer func() {
closeActionsMutex.Lock()
civisibility.SetState(civisibility.StateExited)
if ciVisibilityShutdownDone == done {
close(done)
ciVisibilityShutdownDone = nil
}
closeActionsMutex.Unlock()
}()
log.Debug("civisibility: exiting")
for _, barrier := range barriers {
barrier()
}

closeActionsMutex.Lock()
defer closeActionsMutex.Unlock()
defer func() {
closeActions = []ciVisibilityCloseAction{}
preCloseActions = []ciVisibilityCloseAction{}
log.Debug("civisibility: flushing and stopping the logger")
logs.Stop()
log.Debug("civisibility: flushing and stopping tracer")
Expand Down
40 changes: 40 additions & 0 deletions internal/civisibility/integrations/civisibility_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,18 @@ func ensureSettingsInitialization(serviceName string) {
ciSettings.EarlyFlakeDetection.Enabled = false
}

// check if early flake detection is disabled by env-vars
if ciSettings.EarlyFlakeDetection.Enabled && !internal.BoolEnv(constants.CIVisibilityEarlyFlakeDetectionEnabledEnvironmentVariable, true) {
log.Warn("civisibility: early flake detection was disabled by the environment variable")
ciSettings.EarlyFlakeDetection.Enabled = false
}
earlyFlakeDetectionMaxRetries := internal.IntEnv(constants.CIVisibilityEarlyFlakeDetectionMaxRetriesEnvironmentVariable, -1)
slowTestRetries := &ciSettings.EarlyFlakeDetection.SlowTestRetries
slowTestRetries.FiveS = capEarlyFlakeDetectionRetries(slowTestRetries.FiveS, earlyFlakeDetectionMaxRetries)
slowTestRetries.TenS = capEarlyFlakeDetectionRetries(slowTestRetries.TenS, earlyFlakeDetectionMaxRetries)
slowTestRetries.ThirtyS = capEarlyFlakeDetectionRetries(slowTestRetries.ThirtyS, earlyFlakeDetectionMaxRetries)
slowTestRetries.FiveM = capEarlyFlakeDetectionRetries(slowTestRetries.FiveM, earlyFlakeDetectionMaxRetries)

// check if flaky test retries is disabled by env-vars
if ciSettings.FlakyTestRetriesEnabled && !internal.BoolEnv(constants.CIVisibilityFlakyRetryEnabledEnvironmentVariable, true) {
log.Warn("civisibility: flaky test retries was disabled by the environment variable")
Expand Down Expand Up @@ -270,6 +282,13 @@ func ensureSettingsInitialization(serviceName string) {
})
}

func capEarlyFlakeDetectionRetries(retries, maxRetries int) int {
if maxRetries >= 0 && retries > maxRetries {
return maxRetries
}
return retries
}

// logSettingsFetchError reports a failed or empty CI Visibility settings response.
func logSettingsFetchError(err error) {
if err != nil {
Expand Down Expand Up @@ -400,48 +419,69 @@ func ensureAdditionalFeaturesInitialization(_ string) {

// GetSettings gets the settings from the backend settings endpoint
func GetSettings() *net.SettingsResponseData {
if IsProcessRetryChild() {
return &net.SettingsResponseData{}
}
// call to ensure the settings features initialization is completed (service name can be null here)
ensureSettingsInitialization("")
return &ciVisibilitySettings
}

// GetKnownTests gets the known tests data
func GetKnownTests() *net.KnownTestsResponseData {
if IsProcessRetryChild() {
return &net.KnownTestsResponseData{}
}
// call to ensure the additional features initialization is completed (service name can be null here)
ensureAdditionalFeaturesInitialization("")
return &ciVisibilityKnownTests
}

// GetTestManagementTestsData gets the test management tests data
func GetTestManagementTestsData() *net.TestManagementTestsResponseDataModules {
if IsProcessRetryChild() {
return &net.TestManagementTestsResponseDataModules{}
}
// call to ensure the additional features initialization is completed (service name can be null here)
ensureAdditionalFeaturesInitialization("")
return &ciVisibilityTestManagementTests
}

// GetFlakyRetriesSettings gets the flaky retries settings
func GetFlakyRetriesSettings() *FlakyRetriesSetting {
if IsProcessRetryChild() {
return &FlakyRetriesSetting{}
}
// call to ensure the additional features initialization is completed (service name can be null here)
ensureAdditionalFeaturesInitialization("")
return &ciVisibilityFlakyRetriesSettings
}

// GetSkippableTests gets the skippable tests from the backend
func GetSkippableTests() map[string]map[string][]net.SkippableResponseDataAttributes {
if IsProcessRetryChild() {
return nil
}
// call to ensure the additional features initialization is completed (service name can be null here)
ensureAdditionalFeaturesInitialization("")
return ciVisibilitySkippables
}

// GetSkippableTestsResponse gets the full skippable-tests response from the backend.
func GetSkippableTestsResponse() *net.SkippableTestsResponse {
if IsProcessRetryChild() {
return nil
}
// call to ensure the additional features initialization is completed (service name can be null here)
ensureAdditionalFeaturesInitialization("")
return ciVisibilitySkippablesResponse
}

// GetImpactedTestsAnalyzer gets the impacted tests analyzer
func GetImpactedTestsAnalyzer() *impactedtests.ImpactedTestAnalyzer {
if IsProcessRetryChild() {
return nil
}
// call to ensure the additional features initialization is completed (service name can be null here)
ensureAdditionalFeaturesInitialization("")
return ciVisibilityImpactedTestsAnalyzer
Expand Down
Loading
Loading