elastic / elastic/beats

[refactor-opportunist] Extract shared tracer path-validation helpers for Filebeat inputs

Open
#49,084 1 comment 0 reactions 0 assignees View on GitHub
needs_team
Dominant language
Go
Stars
12.7k
Forks
5k
Avg merge
2d 15m
Merged PRs (30d)
385

Description

## 🏗️ Refactor Proposal

**Summary:** Centralize request-tracer path validation/resolution in `httplog` to remove repeated logic across Filebeat inputs.

## Problem
Tracer path validation and runtime path resolution are duplicated across multiple inputs/providers, which increases maintenance cost and drift risk.

Concrete duplication evidence:
- Validation pattern repeated in multiple configs:
- `x-pack/filebeat/input/cel/config.go:308`
- `x-pack/filebeat/input/httpjson/config_request.go:192`
- `x-pack/filebeat/input/http_endpoint/config.go:149` (before this PoC)
- `x-pack/filebeat/input/entityanalytics/provider/jamf/conf.go:193`
- `x-pack/filebeat/input/entityanalytics/provider/okta/conf.go:323`
- `x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/graph/graph.go:145`
- Runtime resolve+guard pattern repeated in multiple runners:
- `x-pack/filebeat/input/cel/input.go:181`
- `x-pack/filebeat/input/httpjson/input.go:190`
- `x-pack/filebeat/input/http_endpoint/input.go:121` (before this PoC)
- `x-pack/filebeat/input/entityanalytics/provider/jamf/jamf.go:119`
- `x-pack/filebeat/input/entityanalytics/provider/okta/okta.go:124`
- `x-pack/filebeat/input/entityanalytics/provider/azuread/fetcher/graph/graph.go:397`

Churn signal (last 60 days): `x-pack/filebeat/input/internal/httplog/roundtripper*.go` and `x-pack/filebeat/input/http_endpoint/*` show repeated edits, indicating this boundary is actively changing.

## Proposed Approach
Add shared helpers in `x-pack/filebeat/input/internal/httplog` for:
1. validating tracer path is within the per-input logs directory (config-time), and
2. resolving `*`-expanded tracer path and failing if it escapes logs dir (run-time).

Then migrate each input/provider incrementally to these helpers while preserving existing error messages and behavior.

## Proof of Concept
I partially implemented this refactor on one representative slice (`http_endpoint`) to verify viability.

**Files changed:**
- `x-pack/filebeat/input/internal/httplog/roundtripper.go`
- `x-pack/filebeat/input/http_endpoint/config.go`
- `x-pack/filebeat/input/http_endpoint/input.go`

**Before → After:**

- Added shared helpers in `httplog`:
- `ValidatePathInLogsFor` (`roundtripper.go:60`)
- `ResolvePathInLogsForRun` (`roundtripper.go:73`)

- `http_endpoint` config validation:
- Before: inline `IsPathInLogsFor` + `!ok` error block
- After: single call `httplog.ValidatePathInLogsFor(inputName, c.Tracer.Filename)` (`config.go:148`)

- `http_endpoint` runtime resolution:
- Before: inline `ResolvePathInLogsFor` + `!ok` error block
- After: single call `httplog.ResolvePathInLogsForRun(inputName, path)` (`input.go:120`)

**Verification:**
- `go test ./x-pack/filebeat/input/http_endpoint ./x-pack/filebeat/input/internal/httplog`
- Result: both packages pass (`ok .../http_endpoint`, `ok .../internal/httplog`)
- Existing tests for the migrated slice still pass.

## Incremental Rollout Plan
This refactor can be completed incrementally:
1. ✅ (Proved) Migrate `http_endpoint` to shared helpers.
2. Migrate `httpjson` and `cel` config/runtime tracer checks to shared helpers.
3. Migrate entityanalytics providers (`azuread`, `jamf`, `okta`) and remove remaining duplicated blocks.

## Risks and Mitigations
- **Risk 1:** Error-message compatibility regressions in tests.
- **Mitigation:** Preserve current message formats in helper APIs (separate config-time and run-time helpers).
- **Risk 2:** Hidden behavior differences between inputs.
- **Mitigation:** Migrate one input at a time and run package-scoped tests per migrated module.

## Evidence
- Duplicate call sites listed above via code search in `x-pack/filebeat/input/**`.
- PoC implementation and passing tests in the three changed files listed above.
- Checked for overlap: no open issue found for this specific tracer-path refactor (`repo:elastic/beats is:issue is:open "request tracer path" refactor` returned 0).

---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Refactor Opportunist](https://github.com/elastic/beats/actions/runs/22372233214)

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

Contributor guide

Open the contributing guide

Research direction

Start with the existing helpers and migrated http_endpoint files: x-pack/filebeat/input/internal/httplog/roundtripper.go, input/http_endpoint/config.go, and input.go. Run the package tests listed in the issue, then migrate the httpjson and cel paths before the azuread, jamf, and okta providers. Done means duplicated validation and runtime resolution are removed while error messages, behavior, and package tests remain unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
devops
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.