elastic / elastic/beats

[refactor-opportunist] Consolidate Prometheus and OpenMetrics metric processing

Open
#51,268 1 comment 0 reactions 0 assignees View on GitHub
needs_team
Dominant language
Go
Stars
12.7k
Forks
5k
Avg merge
2d 1h
Merged PRs (30d)
370

Description

## 🏗️ Refactor Proposal

**Summary:** Consolidate the duplicated Prometheus/OpenMetrics metric-family-to-event assembly pipeline into one shared processor while keeping each format's metric adapters separate.

## Problem

`metricbeat/helper/prometheus` and `metricbeat/helper/openmetrics` currently maintain nearly identical event assembly code, so changes to label grouping, info metrics, extra fields, and mapped/unmapped label behavior need to be made twice.

Concrete evidence:

- `metricbeat/helper/prometheus/prometheus.go:139-253` and `metricbeat/helper/openmetrics/openmetrics.go:129-242` duplicated the full `ProcessMetrics` loop: metric lookup, value extraction, option processing, label/key-label grouping, `infoMetric` merging, event construction, and `ExtraFields` application.
- `metricbeat/helper/prometheus/prometheus.go:285-303` and `metricbeat/helper/openmetrics/openmetrics.go:274-292` duplicated `getEvent` and `getLabels` helpers.
- `metricbeat/helper/prometheus/prometheus.go:124-137` and `metricbeat/helper/openmetrics/openmetrics.go:114-127` define equivalent `MetricsMapping` shapes.
- `metricbeat/helper/prometheus/label.go:20-59` and `metricbeat/helper/openmetrics/label.go:20-59` define the same label mapping abstraction.

This is structural duplication, not cosmetic duplication: both helpers parse into shared `prometheus.MetricFamily` / `prometheus.OpenMetric` types, but the central event assembly logic is still forked.

## Proposed Approach

Introduce a shared metric-family processor in `metricbeat/helper/prometheus` that owns the common event assembly pipeline. Prometheus and OpenMetrics should keep their public mapping types and per-format metric value extraction, but pass small adapters into the shared processor for:

- metric field/value access
- metric configuration conversion
- metric processing options
- label mapping
- info-metric detection

This preserves existing package APIs while removing the duplicated pipeline logic incrementally.

## Proof of Concept

I partially implemented this refactor on one representative slice to verify viability: `ProcessMetrics` now delegates shared event assembly to `prometheus.ProcessMetricFamilies`, while `openmetrics.ProcessMetrics` adapts its package-local types into that shared processor.

**Files changed:**

- `metricbeat/helper/prometheus/prometheus.go`
- `metricbeat/helper/openmetrics/openmetrics.go`

**Before → After:**

Before, `openmetrics.ProcessMetrics` contained a full duplicate copy of the Prometheus event assembly loop. The POC replaces that with an adapter call:

```go
func (p *openmetrics) ProcessMetrics(families []*prometheus.MetricFamily, mapping *MetricsMapping) ([]mapstr.M, error) {
events := prometheus.ProcessMetricFamilies(
families,
mapping.Metrics,
mapping.Labels,
mapping.ExtraFields,
func(m MetricMap) string { return m.GetField() },
func(m MetricMap, metric *prometheus.OpenMetric) interface{} { return m.GetValue(metric) },
func(m MetricMap) prometheus.MetricMappingConfig { return newMetricMappingConfig(m.GetConfiguration()) },
func(m MetricMap) []MetricOption { return m.GetOptions() },
func(m MetricMap) bool { return m == nil },
func(m MetricMap) bool {
_, ok := m.(*infoMetric)
return ok
},
)

return events, nil
}
```

The shared processor lives in `metricbeat/helper/prometheus/prometheus.go` and covers the former duplicated event map, label conversion, info metric merge, and extra-field handling.

**Verification:**

- `cd /home/runner/work/beats/beats && gofmt -w metricbeat/helper/prometheus/prometheus.go metricbeat/helper/openmetrics/openmetrics.go && go test ./metricbeat/helper/prometheus ./metricbeat/helper/openmetrics`
- Result: `ok github.com/elastic/beats/v7/metricbeat/helper/prometheus 0.023s` and `ok github.com/elastic/beats/v7/metricbeat/helper/openmetrics 0.015s`

## Incremental Rollout Plan

This refactor can be completed incrementally:

1. Land the proven `ProcessMetrics` consolidation slice for Prometheus/OpenMetrics event assembly.
2. Fold the duplicated label helpers (`Label`, `KeyLabel`, `commonLabel`) into a shared label mapping helper or aliases while preserving package-level constructors.
3. Evaluate the remaining duplicated helper functions (`CompilePatternList`, `MatchMetricFamily`) and metric option/configuration types for safe consolidation after the processing path is shared.

## Risks and Mitigations

- Public API compatibility: Keep `prometheus.MetricsMapping`, `openmetrics.MetricsMapping`, `prometheus.Label`, and `openmetrics.Label` constructors intact during migration.
- Generic helper readability: Keep adapters thin and covered by the existing Prometheus/OpenMetrics helper tests; avoid forcing OpenMetrics metric types to become Prometheus types.
- Behavior drift during rollout: Migrate one duplicated surface at a time and run both helper package test suites after each slice.

## Evidence

- Prior findings checked in `/tmp/previous-findings.json`; this does not overlap with existing refactor-opportunist issues for tracer helpers, Beat manager bootstrap, file auth, vSphere mapping, or default input plugin lists.
- Focused GitHub duplicate searches for `Prometheus OpenMetrics duplicated helper refactor` returned no matching open issue or PR.
- The repository does not currently have a `refactor-opportunist` label, so no label is applied.

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

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

Contributor guide

Open the contributing guide

Research direction

Start with ProcessMetrics in metricbeat/helper/prometheus/prometheus.go and metricbeat/helper/openmetrics/openmetrics.go, then compare the duplicated getEvent, getLabels, MetricsMapping, and label helpers named in the issue. Run gofmt and go test ./metricbeat/helper/prometheus ./metricbeat/helper/openmetrics. Done means the shared event assembly is consolidated incrementally, package APIs remain compatible, and both helper test suites pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, prometheus
Domain
observability-sre
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.