elastic / elastic/elastic-agent
[otel-collector-integration] Subprocess Stop timeout can still block indefinitely via deferred WaitGroup wait
- 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
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