[refactor-opportunist] Consolidate duplicated entityanalytics keep_alive config into a shared provider helper
- Dominant language
- Go
- Stars
- 12.7k
- Forks
- 5k
- Avg merge
- 2d 15m
- Merged PRs (30d)
- 385
Description
## 🏗️ Refactor Proposal
**Summary:** Replace duplicated `keep_alive` config/validation/settings logic in entityanalytics providers with one shared `provider.KeepAlive` type.
## Problem
`keep_alive` is implemented repeatedly with the same fields, validation checks, and transport conversion logic, which increases change surface and drift risk.
Concrete duplication before this refactor slice:
- `x-pack/filebeat/input/entityanalytics/provider/jamf/conf.go:132-162`
- `x-pack/filebeat/input/entityanalytics/provider/okta/conf.go:251-281`
- Similar copies still exist outside this slice:
- `x-pack/filebeat/input/httpjson/config_request.go:80-110`
- `x-pack/filebeat/input/cel/config.go:255-285`
The duplicated logic includes identical error paths (`"max_idle_connections must not be negative"`, etc.) and conversion to `httpcommon.WithKeepaliveSettings`.
## Proposed Approach
Create a shared helper type in `x-pack/filebeat/input/entityanalytics/provider` and migrate provider configs to use it:
- Shared type: `provider.KeepAlive`
- Shared behavior: `Validate()` and `Settings()`
- Provider-specific request configs keep ownership of the `keep_alive` field wiring, but delegate behavior to shared code.
This reduces repeated logic while preserving config schema and runtime behavior.
## Proof of Concept
I partially implemented this refactor on one representative slice to verify viability.
**Files changed:**
- `x-pack/filebeat/input/entityanalytics/provider/keepalive.go` (new shared helper)
- `x-pack/filebeat/input/entityanalytics/provider/jamf/conf.go`
- `x-pack/filebeat/input/entityanalytics/provider/okta/conf.go`
- `x-pack/filebeat/input/entityanalytics/provider/jamf/jamf.go`
- `x-pack/filebeat/input/entityanalytics/provider/okta/okta.go`
- `x-pack/filebeat/input/entityanalytics/provider/jamf/conf_test.go`
- `x-pack/filebeat/input/entityanalytics/provider/okta/conf_test.go`
**Before → After:**
- Before: each provider defined a local `type keepAlive struct` plus duplicated `Validate()` and `settings()` methods.
- After: providers use `provider.KeepAlive`:
- `x-pack/filebeat/input/entityanalytics/provider/jamf/conf.go:89`
- `x-pack/filebeat/input/entityanalytics/provider/okta/conf.go:208`
- Shared methods now live in:
- `x-pack/filebeat/input/entityanalytics/provider/keepalive.go:23-47`
- Call sites now use shared conversion method:
- `x-pack/filebeat/input/entityanalytics/provider/jamf/jamf.go:189`
- `x-pack/filebeat/input/entityanalytics/provider/okta/okta.go:194`
**Verification:**
- `go test ./x-pack/filebeat/input/entityanalytics/provider ./x-pack/filebeat/input/entityanalytics/provider/jamf ./x-pack/filebeat/input/entityanalytics/provider/okta` ✅
- `go test ./x-pack/filebeat/input/entityanalytics/provider/...` ✅
- Existing keep-alive behavior tests still pass:
- `x-pack/filebeat/input/entityanalytics/provider/jamf/conf_test.go:146-168`
- `x-pack/filebeat/input/entityanalytics/provider/okta/conf_test.go:166-187`
## Incremental Rollout Plan
This refactor can be completed incrementally:
1. Migrate entityanalytics providers to shared `provider.KeepAlive` (completed in this PoC).
2. Extract a second shared helper in an input-level common package for other HTTP-style inputs and migrate one consumer (for example `httpjson`).
3. Migrate remaining keep-alive duplicates (`cel`, `streaming/websocket`, and any other HTTP config structs), then remove local duplicates.
## Risks and Mitigations
- **Risk 1: Config unpack/validation behavior changes subtly.**
- **Mitigation:** preserve field tags and error strings; keep existing package tests passing.
- **Risk 2: Cross-package helper introduces coupling concerns.**
- **Mitigation:** keep shared helper narrowly scoped to pure config validation/translation with no provider runtime dependencies.
## Evidence
- Duplicate struct/method definitions identified in:
- `x-pack/filebeat/input/entityanalytics/provider/jamf/conf.go:132-162` (pre-change)
- `x-pack/filebeat/input/entityanalytics/provider/okta/conf.go:251-281` (pre-change)
- Remaining duplicates after PoC:
- `x-pack/filebeat/input/httpjson/config_request.go:80-110`
- `x-pack/filebeat/input/cel/config.go:255-285`
- Duplicate-check queries for open tracking returned no matching issue/PR for this specific keep-alive consolidation:
- `repo:elastic/beats is:open is:issue keep_alive max_idle_connections entityanalytics provider refactor`
- `repo:elastic/beats is:open is:pr keep_alive max_idle_connections entityanalytics provider refactor`
> [!NOTE]
>
> 🔒 Integrity filter blocked 67 items
>
> The following items were blocked because they don't meet the GitHub integrity level.
>
> - [#42900](https://github.com/elastic/beats/issues/42900) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#34089](https://github.com/elastic/beats/issues/34089) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#45719](https://github.com/elastic/beats/issues/45719) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#40276](https://github.com/elastic/beats/issues/40276) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#30911](https://github.com/elastic/beats/issues/30911) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#50630](https://github.com/elastic/beats/issues/50630) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#32116](https://github.com/elastic/beats/issues/32116) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#33048](https://github.com/elastic/beats/issues/33048) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#19921](https://github.com/elastic/beats/issues/19921) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#30769](https://github.com/elastic/beats/issues/30769) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#637](https://github.com/elastic/beats/issues/637) `search_issues`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#50843](https://github.com/elastic/beats/pull/50843) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#50841](https://github.com/elastic/beats/pull/50841) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#50875](https://github.com/elastic/beats/pull/50875) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#50357](https://github.com/elastic/beats/pull/50357) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - [#50834](https://github.com/elastic/beats/pull/50834) `search_pull_requests`: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
> - ... and 51 more items
>
> To allow these resources, lower `min-integrity` in your GitHub frontmatter:
>
> ```yaml
> tools:
> github:
> min-integrity: approved # merged | approved | unapproved | none
> ```
>
>
---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Refactor Opportunist](https://github.com/elastic/beats/actions/runs/26407026865)
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Contributor guide
Research direction
Start by reviewing x-pack/filebeat/input/entityanalytics/provider/keepalive.go and the existing provider tests, then inspect the remaining duplicates in x-pack/filebeat/input/httpjson/config_request.go and x-pack/filebeat/input/cel/config.go. Run the listed entityanalytics provider tests first. Done means the targeted configurations share validation and settings behavior while preserving field tags, error strings, runtime behavior, and passing package tests.
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
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100