Skip to content

Commit 8ce1cc1

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

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"
@@ -480,9 +481,14 @@ func histogramMetricName(name string, s float64, qv string, lbls string, t *int6
480481
func ParseMetricFamilies(b []byte, contentType string, ts time.Time, logger *logp.Logger) ([]*MetricFamily, error) {
481482
// Fallback to text/plain if content type is blank or unrecognized.
482483
parser, err := textparse.New(b, contentType, textMediaType, false, false, false, labels.NewSymbolTable())
483-
// This check allows to continue where the content type is blank but the parser is non-nil. Returns error on all other cases.
484-
if err != nil && !strings.Contains(err.Error(), "non-compliant scrape target sending blank Content-Type, using fallback_scrape_protocol") {
485-
return nil, err
484+
// This check allows to continue where the content type is blank/invalid but the parser is non-nil. Returns error on all other cases.
485+
if parser == nil {
486+
if err != nil {
487+
return nil, err
488+
489+
}
490+
491+
return nil, fmt.Errorf("no parser returned for contentType %q", contentType)
486492
}
487493

488494
var (

metricbeat/helper/prometheus/textparse_test.go

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

899-
func TestBlankContentTypeHeader(t *testing.T) {
900-
input := `
899+
func TestParseMetricFamiliesContentTypes(t *testing.T) {
900+
inputPrometheus := `
901901
# TYPE process_cpu_total counter
902902
# HELP process_cpu_total Some help.
903903
process_cpu_total 4.20072246e+06
904+
`
905+
906+
inputOpenMetricsType := `# TYPE process_cpu_total counter
907+
# HELP process_cpu_total Some help.
908+
process_cpu_total 4200722.46
909+
# EOF
904910
`
905911

906912
expected := []*MetricFamily{
907913
{
908914
Name: stringp("process_cpu_total"),
909915
Help: stringp("Some help."),
910916
Type: "counter",
911-
Unit: nil,
912917
Metric: []*OpenMetric{
913918
{
914919
Label: []*labels.Label{},
@@ -921,9 +926,38 @@ process_cpu_total 4.20072246e+06
921926
},
922927
}
923928

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

0 commit comments

Comments
 (0)