[refactor-opportunist] Centralize Beat↔OTel metadata/index contract in a shared package
- Dominant language
- Go
- Stars
- 12.7k
- Forks
- 5k
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 364
Description
## 🏗️ Refactor Proposal
**Summary:** Consolidate the Beat↔OTel log-body/attribute contract into a shared `libbeat/otel/otelcontract` package and migrate both producer (`otelconsumer`) and consumer (`beatprocessor`) to it.
## Problem
The current Beat↔OTel path relies on duplicated string-key contracts spread across components that evolve together:
- `libbeat/otel/otelconsumer/otelconsumer.go:48-52` defines attribute keys locally:
- `esDocumentIDAttribute = "elasticsearch.document_id"`
- `esIndexAttribute = "elasticsearch.index"`
- `libbeat/otel/otelconsumer/otelconsumer.go:281` writes Beat metadata under `"`@metadata`"` in the pdata body.
- `x-pack/otel/processor/beatprocessor/processor.go:238-242` expects that exact `"`@metadata`"` location and shape.
- `x-pack/otel/processor/beatprocessor/processor.go:255-262` writes it back to the same key on round-trip.
This is structural coupling: two modules must stay in lock-step on undocumented literals.
Churn signal supports impact: recent 60-day history includes repeated changes in neighboring OTel and ingest-path files (`libbeat/otel/otelconsumer/otelconsumer.go`, `x-pack/otel/processor/beatprocessor/processor.go`, plus high filestream churn), increasing risk of drift in cross-component contracts.
## Proposed Approach
Introduce a single shared contract package for boundary keys/constants, then migrate all contract users to it incrementally.
What changes:
- Add `libbeat/otel/otelcontract` as the canonical source of shared Beat↔OTel field/attribute names.
- Replace duplicated literals and per-package local constants with references to the shared constants.
What stays the same:
- Event shape and wire behavior remain unchanged (`@metadata`, `elasticsearch.index`, `elasticsearch.document_id` values are identical).
- Existing processors and OTel exporter behavior remain intact.
## Proof of Concept
I partially implemented this refactor on one representative slice to verify viability.
**Files changed:**
- `libbeat/otel/otelcontract/contract.go`
- `libbeat/otel/otelconsumer/otelconsumer.go`
- `x-pack/otel/processor/beatprocessor/processor.go`
- `x-pack/otel/processor/beatprocessor/processor_test.go`
**Before → After:**
1. Centralized constants:
- **Before:** contract literals were duplicated across components.
- **After:** `libbeat/otel/otelcontract/contract.go:21-30` defines:
- `MetadataField = "`@metadata`"`
- `ElasticsearchDocumentIDAttribute = "elasticsearch.document_id"`
- `ElasticsearchIndexAttribute = "elasticsearch.index"`
2. Producer migrated:
- **Before:** `otelconsumer` local consts + direct `"`@metadata`"` write.
- **After:**
- `libbeat/otel/otelconsumer/otelconsumer.go:49-52` maps local aliases to `otelcontract` constants.
- `libbeat/otel/otelconsumer/otelconsumer.go:281` uses `bodyMap.PutEmpty(otelcontract.MetadataField)`.
3. Consumer migrated:
- **Before:** `beatprocessor` unpack/pack used `"`@metadata`"` literals.
- **After:**
- `x-pack/otel/processor/beatprocessor/processor.go:242` reads `GetValue(otelcontract.MetadataField)`.
- `x-pack/otel/processor/beatprocessor/processor.go:249` deletes `otelcontract.MetadataField`.
- `x-pack/otel/processor/beatprocessor/processor.go:261` writes `beatEvent.Fields[otelcontract.MetadataField]`.
4. Round-trip test migrated:
- `x-pack/otel/processor/beatprocessor/processor_test.go:491-497` now uses `otelcontract.MetadataField` in setup and assertion.
**Verification:**
- `go test ./x-pack/otel/processor/beatprocessor -run 'TestConsumeLogsMetadataRoundTripThroughLegacyProcessor|TestConsumeLogs$'` ✅
- `go test ./libbeat/otel/otelconsumer -run 'TestPublish|TestFillLogRecordFromEvent'` ✅
## Incremental Rollout Plan
This refactor can be completed incrementally:
1. **Done (PoC):** introduce `otelcontract` and migrate metadata/index/document-id references in `otelconsumer` and `beatprocessor`.
2. Migrate remaining OTel boundary literals/tests in nearby packages to `otelcontract` (no behavior changes).
3. Remove redundant local aliases/comments and document `otelcontract` as the single boundary contract source.
## Risks and Mitigations
- **Risk:** Hidden contract users still referencing raw literals could drift later.
- **Mitigation:** Follow-up sweep for remaining `"`@metadata`"` / `"elasticsearch.index"` / `"elasticsearch.document_id"` at the Beat↔OTel boundary.
- **Risk:** Behavioral regressions in processor round-trip.
- **Mitigation:** Keep/extend round-trip tests like `TestConsumeLogsMetadataRoundTripThroughLegacyProcessor` during rollout.
## Evidence
- Contract literal producer side: `libbeat/otel/otelconsumer/otelconsumer.go:48-52`, `libbeat/otel/otelconsumer/otelconsumer.go:281`
- Contract literal consumer side: `x-pack/otel/processor/beatprocessor/processor.go:238-242`, `x-pack/otel/processor/beatprocessor/processor.go:255-262`
- Existing round-trip test anchor: `x-pack/otel/processor/beatprocessor/processor_test.go:471-500`
- Duplicate check: no matching open issue found for this specific Beat↔OTel contract centralization (searched issues/PRs for `otelconsumer`, `beatprocessor`, and `"`@metadata`"`).
> [!WARNING]
>
> Firewall blocked 1 domain
>
> The following domain was blocked by the firewall during workflow execution:
>
> - `169.254.169.254`
>> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter:
>
> ```yaml
> network:
> allowed:
> - defaults
> - "169.254.169.254"
> ```
>
> See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information.
>
>
---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Refactor Opportunist](https://github.com/elastic/beats/actions/runs/32737562573)
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Contributor guide
Research direction
Start with libbeat/otel/otelcontract/contract.go and compare remaining boundary literals in libbeat/otel/otelconsumer/otelconsumer.go and x-pack/otel/processor/beatprocessor/processor.go, including the round-trip test in processor_test.go. Run the verification commands listed in the issue, then sweep nearby OTel packages and tests. Done means contract users reference the shared constants and existing event shape and tests remain unchanged.
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