Improve content-type error handling

- Call err everywhere
- Change log message to underscore-separated field

Followup on #10186

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
This commit is contained in:
Julien Pivotto 2022-02-08 11:01:37 +01:00
parent 8d8ce641a4
commit f695df843f
3 changed files with 13 additions and 21 deletions

View file

@ -64,7 +64,7 @@ type Parser interface {
// //
// This function always returns a valid parser, but might additionally // This function always returns a valid parser, but might additionally
// return an error if the content type cannot be parsed. // return an error if the content type cannot be parsed.
func New(b []byte, contentType string) (p Parser, warning error) { func New(b []byte, contentType string) (Parser, error) {
if contentType == "" { if contentType == "" {
return NewPromParser(b), nil return NewPromParser(b), nil
} }

View file

@ -37,62 +37,54 @@ func TestNewParser(t *testing.T) {
for name, tt := range map[string]*struct { for name, tt := range map[string]*struct {
contentType string contentType string
validateParser func(*testing.T, Parser) validateParser func(*testing.T, Parser)
warning string err string
}{ }{
"empty-string": { "empty-string": {
contentType: "",
validateParser: requirePromParser, validateParser: requirePromParser,
warning: "",
}, },
"invalid-content-type-1": { "invalid-content-type-1": {
contentType: "invalid/", contentType: "invalid/",
validateParser: requirePromParser, validateParser: requirePromParser,
warning: "expected token after slash", err: "expected token after slash",
}, },
"invalid-content-type-2": { "invalid-content-type-2": {
contentType: "invalid/invalid/invalid", contentType: "invalid/invalid/invalid",
validateParser: requirePromParser, validateParser: requirePromParser,
warning: "unexpected content after media subtype", err: "unexpected content after media subtype",
}, },
"invalid-content-type-3": { "invalid-content-type-3": {
contentType: "/", contentType: "/",
validateParser: requirePromParser, validateParser: requirePromParser,
warning: "no media type", err: "no media type",
}, },
"invalid-content-type-4": { "invalid-content-type-4": {
contentType: "application/openmetrics-text; charset=UTF-8; charset=utf-8", contentType: "application/openmetrics-text; charset=UTF-8; charset=utf-8",
validateParser: requirePromParser, validateParser: requirePromParser,
warning: "duplicate parameter name", err: "duplicate parameter name",
}, },
"openmetrics": { "openmetrics": {
contentType: "application/openmetrics-text", contentType: "application/openmetrics-text",
validateParser: requireOpenMetricsParser, validateParser: requireOpenMetricsParser,
warning: "",
}, },
"openmetrics-with-charset": { "openmetrics-with-charset": {
contentType: "application/openmetrics-text; charset=utf-8", contentType: "application/openmetrics-text; charset=utf-8",
validateParser: requireOpenMetricsParser, validateParser: requireOpenMetricsParser,
warning: "",
}, },
"openmetrics-with-charset-and-version": { "openmetrics-with-charset-and-version": {
contentType: "application/openmetrics-text; version=1.0.0; charset=utf-8", contentType: "application/openmetrics-text; version=1.0.0; charset=utf-8",
validateParser: requireOpenMetricsParser, validateParser: requireOpenMetricsParser,
warning: "",
}, },
"plain-text": { "plain-text": {
contentType: "text/plain", contentType: "text/plain",
validateParser: requirePromParser, validateParser: requirePromParser,
warning: "",
}, },
"plain-text-with-version": { "plain-text-with-version": {
contentType: "text/plain; version=0.0.4", contentType: "text/plain; version=0.0.4",
validateParser: requirePromParser, validateParser: requirePromParser,
warning: "",
}, },
"some-other-valid-content-type": { "some-other-valid-content-type": {
contentType: "text/html", contentType: "text/html",
validateParser: requirePromParser, validateParser: requirePromParser,
warning: "",
}, },
} { } {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
@ -101,11 +93,11 @@ func TestNewParser(t *testing.T) {
p, err := New([]byte{}, tt.contentType) p, err := New([]byte{}, tt.contentType)
tt.validateParser(t, p) tt.validateParser(t, p)
if tt.warning == "" { if tt.err == "" {
require.NoError(t, err) require.NoError(t, err)
} else { } else {
require.Error(t, err) require.Error(t, err)
require.Contains(t, err.Error(), tt.warning) require.Contains(t, err.Error(), tt.err)
} }
}) })
} }

View file

@ -1418,12 +1418,12 @@ type appendErrors struct {
} }
func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string, ts time.Time) (total, added, seriesAdded int, err error) { func (sl *scrapeLoop) append(app storage.Appender, b []byte, contentType string, ts time.Time) (total, added, seriesAdded int, err error) {
p, warning := textparse.New(b, contentType) p, err := textparse.New(b, contentType)
if warning != nil { if err != nil {
level.Debug(sl.l).Log( level.Debug(sl.l).Log(
"msg", "Invalid content type on scrape, using prometheus parser as fallback", "msg", "Invalid content type on scrape, using prometheus parser as fallback.",
"content-type", contentType, "content_type", contentType,
"err", warning, "err", err,
) )
} }