Skip to content

Commit 9240ea8

Browse files
[prometheus] Update the error check to handle invalid and empty content header (#47085) (#47119)
(cherry picked from commit 647d455) Co-authored-by: Aman <[email protected]>
1 parent aac9ce1 commit 9240ea8

File tree

3 files changed

+95
-10
lines changed

3 files changed

+95
-10
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# REQUIRED
2+
# Kind can be one of:
3+
# - breaking-change: a change to previously-documented behavior
4+
# - deprecation: functionality that is being removed in a later release
5+
# - bug-fix: fixes a problem in a previous version
6+
# - enhancement: extends functionality but does not break or fix existing behavior
7+
# - feature: new functionality
8+
# - known-issue: problems that we are aware of in a given version
9+
# - security: impacts on the security of a product or a user’s deployment.
10+
# - upgrade: important information for someone upgrading from a prior version
11+
# - other: does not fit into any of the other categories
12+
kind: enhancement
13+
14+
# REQUIRED for all kinds
15+
# Change summary; a 80ish characters long description of the change.
16+
summary: Improve the Prometheus helper to handle multiple content types including blank and invalid headers.
17+
18+
# REQUIRED for breaking-change, deprecation, known-issue
19+
# Long description; in case the summary is not enough to describe the change
20+
# this field accommodate a description without length limits.
21+
# description:
22+
23+
# REQUIRED for breaking-change, deprecation, known-issue
24+
# impact:
25+
26+
# REQUIRED for breaking-change, deprecation, known-issue
27+
# action:
28+
29+
# REQUIRED for all kinds
30+
# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
31+
component: metricbeat
32+
33+
# AUTOMATED
34+
# OPTIONAL to manually add other PR URLs
35+
# PR URL: A link the PR that added the changeset.
36+
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
37+
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
38+
# Please provide it if you are adding a fragment for a different PR.
39+
# pr: https://github.com/owner/repo/1234
40+
41+
# AUTOMATED
42+
# OPTIONAL to manually add other issue URLs
43+
# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
44+
# If not present is automatically filled by the tooling with the issue linked to the PR number.
45+
# issue: https://github.com/owner/repo/1234

metricbeat/helper/prometheus/textparse.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package prometheus
1919

2020
import (
2121
"errors"
22+
"fmt"
2223
"io"
2324
"math"
2425
"mime"
@@ -484,9 +485,14 @@ func histogramMetricName(name string, s float64, qv string, lbls string, t *int6
484485
func ParseMetricFamilies(b []byte, contentType string, ts time.Time, logger *logp.Logger) ([]*MetricFamily, error) {
485486
// Fallback to text/plain if content type is blank or unrecognized.
486487
parser, err := textparse.New(b, contentType, textMediaType, false, false, false, labels.NewSymbolTable())
487-
// This check allows to continue where the content type is blank but the parser is non-nil. Returns error on all other cases.
488-
if err != nil && !strings.Contains(err.Error(), "non-compliant scrape target sending blank Content-Type, using fallback_scrape_protocol") {
489-
return nil, err
488+
// This check allows to continue where the content type is blank/invalid but the parser is non-nil. Returns error on all other cases.
489+
if parser == nil {
490+
if err != nil {
491+
return nil, err
492+
493+
}
494+
495+
return nil, fmt.Errorf("no parser returned for contentType %q", contentType)
490496
}
491497

492498
var (

metricbeat/helper/prometheus/textparse_test.go

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -900,19 +900,24 @@ redis_connected_clients{instance="rough-snowflake-web"} 10.0`
900900
require.ElementsMatch(t, expected, result)
901901
}
902902

903-
func TestBlankContentTypeHeader(t *testing.T) {
904-
input := `
903+
func TestParseMetricFamiliesContentTypes(t *testing.T) {
904+
inputPrometheus := `
905905
# TYPE process_cpu_total counter
906906
# HELP process_cpu_total Some help.
907907
process_cpu_total 4.20072246e+06
908+
`
909+
910+
inputOpenMetricsType := `# TYPE process_cpu_total counter
911+
# HELP process_cpu_total Some help.
912+
process_cpu_total 4200722.46
913+
# EOF
908914
`
909915

910916
expected := []*MetricFamily{
911917
{
912918
Name: stringp("process_cpu_total"),
913919
Help: stringp("Some help."),
914920
Type: "counter",
915-
Unit: nil,
916921
Metric: []*OpenMetric{
917922
{
918923
Label: []*labels.Label{},
@@ -925,9 +930,38 @@ process_cpu_total 4.20072246e+06
925930
},
926931
}
927932

928-
result, err := ParseMetricFamilies([]byte(input), "", time.Now(), nil)
929-
if err != nil {
930-
t.Fatal("ParseMetricFamilies for blank content type returned an error.")
933+
tests := []struct {
934+
name string
935+
contentType string
936+
wantErr bool
937+
}{
938+
{"plain", "text/plain", false},
939+
{"plain_charset", "text/plain; charset=utf-8", false},
940+
{"plain_version_0_0_4", "text/plain; version=0.0.4; charset=utf-8", false},
941+
{"plain_version_1.0.0", "text/plain; version=1.0.0; charset=utf-8", false},
942+
{"json", "application/json", false},
943+
{"html", "text/html; charset=utf-8", false},
944+
{"empty_content_type", "", false},
945+
{"openmetrics", "application/openmetrics-text", false},
946+
{"octet_stream", "application/octet-stream", false},
947+
}
948+
949+
for _, tt := range tests {
950+
t.Run(tt.name, func(t *testing.T) {
951+
input := inputPrometheus
952+
953+
if tt.contentType == OpenMetricsType {
954+
input = inputOpenMetricsType
955+
}
956+
957+
result, err := ParseMetricFamilies([]byte(input), tt.contentType, time.Now(), logptest.NewTestingLogger(t, "test"))
958+
if tt.wantErr {
959+
require.Error(t, err, "expected error for %s", tt.contentType)
960+
return
961+
}
962+
963+
require.NoError(t, err, "unexpected error for %s", tt.contentType)
964+
require.ElementsMatch(t, expected, result)
965+
})
931966
}
932-
require.ElementsMatch(t, expected, result)
933967
}

0 commit comments

Comments
 (0)