[refactor-opportunist] Consolidate filestream runtime-settings normalization across parsing layers
- Dominant language
- Go
- Stars
- 12.7k
- Forks
- 5k
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 370
Description
## 🏗️ Refactor Proposal
**Summary:** Consolidate filestream `clean_inactive` runtime normalization into a shared helper used by both outer filestream config and `input-logfile` manager parsing, then extend the same pattern to the remaining duplicated runtime fields.
## Problem
`filestream` still has a split runtime configuration boundary where behavior is normalized in more than one layer.
Concrete code evidence:
- `filebeat/input/filestream/config.go:79` says: `"clean_inactive" is parsed, again, and used by internal/input-logfile/manager.go`.
- `filebeat/input/filestream/config.go:88-99` also documents that `take_over`, `allow_deprecated_id_duplication`, and `legacy_clean_inactive` are independently parsed by `InputManager.Create`.
- `filebeat/input/filestream/internal/input-logfile/manager.go:168` says: `All those values are duplicated from the Filestream configuration`.
This seam is active/high-churn:
- `git log --since='60 days ago' --oneline -- filebeat/input/filestream/config.go filebeat/input/filestream/input.go filebeat/input/filestream/internal/input-logfile/manager.go | wc -l` -> `8` commits.
- Recent commits include `#53136`, `#53122`, `#52797`, `#52326`, `#52147`, `#52103`, `#51278`, `#52095`.
## Proposed Approach
Create a shared runtime-settings helper in `internal/input-logfile` for behavior that must remain identical across both parsing paths, and migrate call sites incrementally.
This preserves existing behavior while removing drift-prone duplicated logic. The first proven step is consolidating `clean_inactive == 0` normalization.
## Proof of Concept
I partially implemented this refactor on one representative slice to verify viability.
**Files changed:**
- `filebeat/input/filestream/internal/input-logfile/settings.go`
- `filebeat/input/filestream/internal/input-logfile/settings_test.go`
- `filebeat/input/filestream/internal/input-logfile/manager.go`
- `filebeat/input/filestream/input.go`
**Before → After:**
`filebeat/input/filestream/input.go:153`
```go
c.CleanInactive = loginp.NormalizeCleanInactive(c.CleanInactive, c.LegacyCleanInactive)
```
`filebeat/input/filestream/internal/input-logfile/manager.go:197`
```go
settings.CleanInactive = NormalizeCleanInactive(settings.CleanInactive, settings.LegacyCleanInactive)
```
New shared helper:
`filebeat/input/filestream/internal/input-logfile/settings.go:24-31`
```go
func NormalizeCleanInactive(cleanInactive time.Duration, legacyCleanInactive bool) time.Duration {
if !legacyCleanInactive && cleanInactive == 0 {
return -1
}
return cleanInactive
}
```
Added focused coverage:
`filebeat/input/filestream/internal/input-logfile/settings_test.go:27-61`
```go
func TestNormalizeCleanInactive(t *testing.T) { ... }
```
**Verification:**
- `go test ./filebeat/input/filestream/internal/input-logfile -run TestNormalizeCleanInactive` -> `ok`
- `go test ./filebeat/input/filestream -run TestConfigValidate` -> `ok`
## Incremental Rollout Plan
This refactor can be completed incrementally:
1. Migrate shared behavior-only logic first (already proven for `clean_inactive` normalization).
2. Extract a shared `RuntimeSettings` contract in `internal/input-logfile` for other duplicated fields (`take_over`, `allow_deprecated_id_duplication`, `read_until_eof`, backoff/close interval).
3. Switch `filestream` and `InputManager.Create` to that shared contract and remove duplicated comments/logic from both sides.
## Risks and Mitigations
- Risk: config-compatibility drift while unifying parsing.
- Mitigation: keep migration behavior-preserving and add table-driven compatibility tests per field.
- Risk: widening scope too quickly across all fields.
- Mitigation: migrate one field/group at a time with targeted tests before next step.
## Evidence
- Structural duplication comments:
- `filebeat/input/filestream/config.go:79`
- `filebeat/input/filestream/config.go:88-99`
- `filebeat/input/filestream/internal/input-logfile/manager.go:168`
- Partial implementation (working slice):
- `filebeat/input/filestream/internal/input-logfile/settings.go:22-32`
- `filebeat/input/filestream/input.go:153`
- `filebeat/input/filestream/internal/input-logfile/manager.go:197`
- Duplicate check against prior opportunist findings:
- `/tmp/previous-findings.json` (no matching open issue for this filestream runtime parsing seam)
---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Refactor Opportunist](https://github.com/elastic/beats/actions/runs/34853928377)
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Contributor guide
Research direction
Start with filebeat/input/filestream/config.go and input.go, then compare filebeat/input/filestream/internal/input-logfile/manager.go with settings.go and settings_test.go. Run the focused normalization and ConfigValidate tests before examining the remaining duplicated runtime fields. Done means shared behavior is covered by compatibility tests and both parsing paths retain existing behavior without duplicated normalization logic.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100