feat(prometheus): robust handling for duplicate metrics without HELP headers #1375
Conversation
kongfei605
changed the title
There was a problem hiding this comment.
Pull request overview
This PR enhances the Prometheus parser to handle duplicate metrics that lack HELP headers by introducing a nested splitting strategy. The parser now splits metrics by both HELP and TYPE headers, allowing it to properly parse metric streams where metrics are delimited only by TYPE headers.
Key Changes
- Implemented nested byte splitting: first by HELP headers, then by TYPE headers within each HELP segment
- Added empty segment filtering to skip whitespace-only content
- Enhanced error logging to include the actual metric content that failed to parse
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
parser/prometheus/parser.go
Outdated
| typeMetrics := bytes.Split(metrics[i], typeHeaderBytes) | ||
| for j := range typeMetrics { | ||
| if j != 0 { | ||
| typeMetrics[j] = append(append([]byte(nil), typeHeaderBytes...), typeMetrics[j]...) | ||
| } | ||
|
|
||
| if len(bytes.TrimSpace(typeMetrics[j])) == 0 { | ||
| continue | ||
| } | ||
|
|
||
| err := p.parse(typeMetrics[j], slist) | ||
| if err != nil { | ||
| log.Println("E! parse metrics failed, error:", err, "metrics:", string(typeMetrics[j])) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The new nested splitting logic for handling metrics without HELP headers is not covered by tests. This is complex logic with several edge cases (metrics with only TYPE headers, metrics with both HELP and TYPE headers, empty segments, etc.) that should be tested. The codebase has comprehensive test coverage in other areas (elasticsearch, keepalived, etc. collectors have extensive test files), so this parser should follow the same pattern.
parser/prometheus/parser.go
Outdated
| metrics := bytes.Split(buf, metricHeaderBytes) | ||
| for i := range metrics { | ||
| if i != 0 { | ||
| metrics[i] = append(append([]byte(nil), MetricHeaderBytes...), metrics[i]...) | ||
| metrics[i] = append(append([]byte(nil), metricHeaderBytes...), metrics[i]...) | ||
| } | ||
| err := p.parse(metrics[i], slist) | ||
| if err != nil { | ||
| log.Println("E! parse metrics failed, error:", err, "metrics:", metrics[i]) | ||
|
|
||
| typeMetrics := bytes.Split(metrics[i], typeHeaderBytes) | ||
| for j := range typeMetrics { | ||
| if j != 0 { | ||
| typeMetrics[j] = append(append([]byte(nil), typeHeaderBytes...), typeMetrics[j]...) | ||
| } |
There was a problem hiding this comment.
The nested splitting approach creates multiple intermediate byte slice allocations. Each split operation creates a new slice, and prepending headers with double append creates additional allocations. For large metric payloads, this could result in significant memory overhead.
Consider alternative approaches:
- Use a single-pass parser that scans for both HELP and TYPE headers
- Pre-allocate slices with estimated capacity
- Use bytes.Index to find delimiters without creating intermediate slices
The current approach processes the same data multiple times (split by HELP, then each result split by TYPE), which could impact performance when handling high-volume metric scrapes.
parser/prometheus/parser.go
Outdated
| if len(bytes.TrimSpace(typeMetrics[j])) == 0 { | ||
| continue | ||
| } |
There was a problem hiding this comment.
The empty check only happens after splitting by TYPE headers, not after splitting by HELP headers. This asymmetry could lead to unnecessary parse attempts on empty or whitespace-only segments from the HELP split. Consider adding a similar check in the outer loop at line 88 to skip empty segments early and avoid the nested TYPE splitting altogether for empty content.
parser/prometheus/parser.go
Outdated
|
|
||
| err := p.parse(typeMetrics[j], slist) | ||
| if err != nil { | ||
| log.Println("E! parse metrics failed, error:", err, "metrics:", string(typeMetrics[j])) |
There was a problem hiding this comment.
The log message now includes the full metric content as a string, which could be quite large for metrics with many labels or long help text. Consider truncating the output or logging only a summary (e.g., first N bytes or first line) to avoid flooding logs, especially when parsing failures are frequent.
1 participant
No description provided.