elastic / elastic/elastic-agent

[otel-collector-integration] Subprocess Stop timeout can still block indefinitely via deferred WaitGroup wait

Open
#14,742 2 comments 0 reactions 0 assignees View on GitHub
bug Team:Elastic-Agent-Control-Plane
Dominant language
Go
Stars
275
Forks
264
Avg merge
1d 23h
Merged PRs (30d)
312

Description

## Findings

### 1. `procHandle.Stop()` timeout is ineffective and can still block indefinitely

**Priority:** 1 (lifecycle reliability)

**Location**
- `internal/pkg/otel/manager/execution_subprocess.go:487-489`
- `internal/pkg/otel/manager/execution_subprocess.go:514-519`
- `internal/pkg/otel/manager/execution_subprocess.go:350-357`

**Evidence**
`Stop()` is structured with an unconditional deferred wait:

```go
func (s *procHandle) Stop(waitTime time.Duration) {
defer s.wg.Wait()
...
select {
case <-time.After(process.KillReapTime):
s.logger.Errorf("timed out waiting for supervised collector process %d to be reaped after SIGKILL", s.processInfo.PID)
case <-s.processDoneCh:
}
}
```

The only place `processDoneCh` is closed is the `waitProcess` goroutine:

```go
go func() {
defer s.wg.Done()
procState, procErr = s.waitProcess()
cancel()
close(s.processDoneCh)
}()
```

So even after `KillReapTime` timeout fires, `defer s.wg.Wait()` still blocks until that goroutine exits.

**What is wrong**
The timeout path logs a timeout but does not bound `Stop()` latency in practice; if process reap is delayed/stuck, `Stop()` still waits forever on `wg.Wait()`.

**Why it matters**
This affects normal operational flows that call `Stop()` (agent shutdown, restart, re-exec). In the exact failure mode the timeout is intended to handle (collector not reaped promptly), the manager can hang instead of completing shutdown, extending telemetry outage and leaving collector lifecycle in a wedged state.

**Suggested fix direction**
Remove unconditional `defer s.wg.Wait()` from `Stop()`. Use bounded waiting semantics tied to `processDoneCh`/context so timeout truly bounds return time, while still allowing cleanup goroutines to finish asynchronously.

**Failing test to add**
- **Target package/file:** `internal/pkg/otel/manager/execution_subprocess_test.go`
- **Test scenario:**
1. Build a `procHandle` where `waitProcess` blocks (or sleeps beyond `process.KillReapTime`) and therefore does not close `processDoneCh` in time.
2. Call `Stop(waitTime)` in a goroutine.
3. Assert `Stop()` returns within `waitTime + process.KillReapTime + delta`.
- **Current behavior:** test blocks/fails because deferred `wg.Wait()` prevents timely return.

## Translation fields audited and found correctly mapped
During this sweep, the following translation paths were reviewed and did not meet reporting threshold:
- `bulk_max_size` mapping into queue batching (`internal/pkg/otel/translate/output_elasticsearch.go:123-126`)
- retry config enable/interval mapping (`internal/pkg/otel/translate/output_elasticsearch.go:180-197`)
- unsupported output settings rejected explicitly (`internal/pkg/otel/translate/output_elasticsearch.go:237-247`)
- auth mapping (`user/password/api_key`) (`internal/pkg/otel/translate/output_elasticsearch.go:155-157`)

## Duplicate check
- Compared against existing open prefix issue: `#13300` (`[otel-collector-integration] High-severity OTel collector integration defects (data loss + status misreporting)`).
- This lifecycle deadlock pattern is not listed there.

---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Sweeper: OTel Collector and Beats Receiver Integration](https://github.com/elastic/elastic-agent/actions/runs/26879446061)

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
> - [x] expires on Jun 10, 2026, 10:51 AM UTC

Contributor guide

Open the contributing guide

Research direction

Start with procHandle.Stop, waitProcess, and the processDoneCh handling in internal/pkg/otel/manager/execution_subprocess.go, then read the related lifecycle tests. Run the targeted tests in internal/pkg/otel/manager/execution_subprocess_test.go; done means Stop returns within the stated wait and reap timeout even when process reaping is delayed.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, observability
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
57/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.