Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loki: Return an __error_details__ label for any line which incurs a __error__ while being processed #6543

Merged
merged 3 commits into from
Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/logql/log/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ func (lf *LineFormatter) Process(ts int64, line []byte, lbs *LabelsBuilder) ([]b

if err := lf.Template.Execute(lf.buf, lbs.Map()); err != nil {
lbs.SetErr(errTemplateFormat)
lbs.SetErrorDetails(err.Error())
return line, true
}
return lf.buf.Bytes(), true
Expand Down Expand Up @@ -295,6 +296,7 @@ func (lf *LabelsFormatter) Process(_ int64, l []byte, lbs *LabelsBuilder) ([]byt
}
if err := f.tmpl.Execute(lf.buf, data); err != nil {
lbs.SetErr(errTemplateFormat)
lbs.SetErrorDetails(err.Error())
continue
}
lbs.Set(f.Name, lf.buf.String())
Expand Down
31 changes: 30 additions & 1 deletion pkg/logql/log/fmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,11 @@ func Test_lineFormatter_Format(t *testing.T) {
labels.Labels{{Name: "foo", Value: "blip"}, {Name: "bar", Value: "blop"}},
0,
nil,
labels.Labels{{Name: logqlmodel.ErrorLabel, Value: errTemplateFormat}, {Name: "foo", Value: "blip"}, {Name: "bar", Value: "blop"}},
labels.Labels{
{Name: "__error__", Value: "TemplateFormatErr"},
{Name: "foo", Value: "blip"}, {Name: "bar", Value: "blop"},
{Name: "__error_details__", Value: "template: line:1:2: executing \"line\" at <.foo>: foo is not a method but has arguments"},
},
nil,
},
{
Expand Down Expand Up @@ -323,6 +327,20 @@ func Test_lineFormatter_Format(t *testing.T) {
labels.Labels{{Name: "bar", Value: "2"}},
[]byte("1"),
},
{
"template_error",
newMustLineFormatter("{{.foo | now}}"),
labels.Labels{{Name: "foo", Value: "blip"}, {Name: "bar", Value: "blop"}},
0,
nil,
labels.Labels{
{Name: "foo", Value: "blip"},
{Name: "bar", Value: "blop"},
{Name: "__error__", Value: "TemplateFormatErr"},
{Name: "__error_details__", Value: "template: line:1:9: executing \"line\" at <now>: wrong number of args for now: want 0 got 1"},
},
nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -391,6 +409,17 @@ func Test_labelsFormatter_Format(t *testing.T) {
labels.Labels{{Name: "bar", Value: "blop"}},
labels.Labels{{Name: "blip", Value: "- and blop"}, {Name: "bar", Value: "blop"}},
},
{
"template error",
mustNewLabelsFormatter([]LabelFmt{NewTemplateLabelFmt("bar", "{{replace \"test\" .foo}}")}),
labels.Labels{{Name: "foo", Value: "blip"}, {Name: "bar", Value: "blop"}},
labels.Labels{
{Name: "foo", Value: "blip"},
{Name: "bar", Value: "blop"},
{Name: "__error__", Value: "TemplateFormatErr"},
{Name: "__error_details__", Value: "template: label:1:2: executing \"label\" at <replace>: wrong number of args for replace: want 3 got 2"},
},
},
}

for _, tt := range tests {
Expand Down
3 changes: 3 additions & 0 deletions pkg/logql/log/label_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ func (d *BytesLabelFilter) Process(_ int64, line []byte, lbs *LabelsBuilder) ([]
value, err := humanize.ParseBytes(v)
if err != nil {
lbs.SetErr(errLabelFilter)
lbs.SetErrorDetails(err.Error())
return line, true
}
switch d.Type {
Expand Down Expand Up @@ -234,6 +235,7 @@ func (d *DurationLabelFilter) Process(_ int64, line []byte, lbs *LabelsBuilder)
value, err := time.ParseDuration(v)
if err != nil {
lbs.SetErr(errLabelFilter)
lbs.SetErrorDetails(err.Error())
return line, true
}
switch d.Type {
Expand Down Expand Up @@ -292,6 +294,7 @@ func (n *NumericLabelFilter) Process(_ int64, line []byte, lbs *LabelsBuilder) (
value, err := strconv.ParseFloat(v, 64)
if err != nil {
lbs.SetErr(errLabelFilter)
lbs.SetErrorDetails(err.Error())
return line, true
}
switch n.Type {
Expand Down
36 changes: 36 additions & 0 deletions pkg/logql/log/label_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,42 @@ func TestBinary_Filter(t *testing.T) {
{Name: "method", Value: "POST"},
},
},
{
NewDurationLabelFilter(LabelFilterGreaterThan, "duration", 3*time.Second),
labels.Labels{
{Name: "duration", Value: "2weeeeee"},
},
true,
labels.Labels{
{Name: "duration", Value: "2weeeeee"},
{Name: "__error__", Value: "LabelFilterErr"},
{Name: "__error_details__", Value: "time: unknown unit \"weeeeee\" in duration \"2weeeeee\""},
},
},
{
NewBytesLabelFilter(LabelFilterGreaterThan, "bytes", 100),
labels.Labels{
{Name: "bytes", Value: "2qb"},
},
true,
labels.Labels{
{Name: "bytes", Value: "2qb"},
{Name: "__error__", Value: "LabelFilterErr"},
{Name: "__error_details__", Value: "unhandled size name: qb"},
},
},
{
NewNumericLabelFilter(LabelFilterGreaterThan, "number", 100),
labels.Labels{
{Name: "number", Value: "not_a_number"},
},
true,
labels.Labels{
{Name: "number", Value: "not_a_number"},
{Name: "__error__", Value: "LabelFilterErr"},
{Name: "__error_details__", Value: "strconv.ParseFloat: parsing \"not_a_number\": invalid syntax"},
},
},
}
for _, tt := range tests {
t.Run(tt.f.String(), func(t *testing.T) {
Expand Down
19 changes: 19 additions & 0 deletions pkg/logql/log/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ type BaseLabelsBuilder struct {
// nolint:structcheck
// https://github.com/golangci/golangci-lint/issues/826
err string
// nolint:structcheck
errDetails string

groups []string
parserKeyHints ParserHint // label key hints for metric queries that allows to limit parser extractions to only this list of labels.
Expand Down Expand Up @@ -134,6 +136,7 @@ func (b *LabelsBuilder) Reset() {
b.del = b.del[:0]
b.add = b.add[:0]
b.err = ""
b.errDetails = ""
}

// ParserLabelHints returns a limited list of expected labels to extract for metric queries.
Expand All @@ -158,6 +161,19 @@ func (b *LabelsBuilder) HasErr() bool {
return b.err != ""
}

func (b *LabelsBuilder) SetErrorDetails(desc string) *LabelsBuilder {
b.errDetails = desc
return b
}

func (b *LabelsBuilder) GetErrorDetails() string {
return b.errDetails
}

func (b *LabelsBuilder) HasErrorDetails() bool {
return b.errDetails != ""
}

// BaseHas returns the base labels have the given key
func (b *LabelsBuilder) BaseHas(key string) bool {
return b.base.Has(key)
Expand Down Expand Up @@ -229,6 +245,9 @@ func (b *LabelsBuilder) unsortedLabels(buf labels.Labels) labels.Labels {
if b.err != "" {
buf = append(buf, labels.Label{Name: logqlmodel.ErrorLabel, Value: b.err})
}
if b.errDetails != "" {
buf = append(buf, labels.Label{Name: logqlmodel.ErrorDetailsLabel, Value: b.errDetails})
}
return buf
}

Expand Down
1 change: 1 addition & 0 deletions pkg/logql/log/metrics_extraction.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ func (l *streamLabelSampleExtractor) Process(ts int64, line []byte) (float64, La
v, err = l.conversionFn(stringValue)
if err != nil {
l.builder.SetErr(errSampleExtraction)
l.builder.SetErrorDetails(err.Error())
}
}
// post filters
Expand Down
18 changes: 18 additions & 0 deletions pkg/logql/log/metrics_extraction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,24 @@ func Test_labelSampleExtractor_Extract(t *testing.T) {
},
true,
},
{
"not convertable",
mustSampleExtractor(LabelExtractorWithStages(
"foo", ConvertFloat, []string{"bar", "buzz"}, false, false, nil, NoopStage,
)),
labels.Labels{
{Name: "foo", Value: "not_a_number"},
{Name: "bar", Value: "foo"},
},
0,
labels.Labels{
{Name: "__error__", Value: "SampleExtractionErr"},
{Name: "__error_details__", Value: "strconv.ParseFloat: parsing \"not_a_number\": invalid syntax"},
{Name: "bar", Value: "foo"},
{Name: "foo", Value: "not_a_number"},
},
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
3 changes: 3 additions & 0 deletions pkg/logql/log/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func (j *JSONParser) Process(_ int64, line []byte, lbs *LabelsBuilder) ([]byte,

if err := j.readObject(it); err != nil {
lbs.SetErr(errJSON)
lbs.SetErrorDetails(err.Error())
return line, true
}
return line, true
Expand Down Expand Up @@ -296,6 +297,7 @@ func (l *LogfmtParser) Process(_ int64, line []byte, lbs *LabelsBuilder) ([]byte
}
if l.dec.Err() != nil {
lbs.SetErr(errLogfmt)
lbs.SetErrorDetails(l.dec.Err().Error())
return line, true
}
return line, true
Expand Down Expand Up @@ -431,6 +433,7 @@ func (u *UnpackParser) Process(_ int64, line []byte, lbs *LabelsBuilder) ([]byte
entry, err := u.unpack(it, line, lbs)
if err != nil {
lbs.SetErr(errJSON)
lbs.SetErrorDetails(err.Error())
return line, true
}
return entry, true
Expand Down
8 changes: 6 additions & 2 deletions pkg/logql/log/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ func Test_jsonParser_Parse(t *testing.T) {
[]byte(`{n}`),
labels.Labels{},
labels.Labels{
{Name: logqlmodel.ErrorLabel, Value: errJSON},
{Name: "__error__", Value: "JSONParserErr"},
{Name: "__error_details__", Value: "ReadMapCB: expect \" after {, but found n, error found in #2 byte of ...|{n}|..., bigger context ...|{n}|..."},
},
},
{
Expand Down Expand Up @@ -570,7 +571,8 @@ func Test_logfmtParser_Parse(t *testing.T) {
},
labels.Labels{
{Name: "foo", Value: "bar"},
{Name: logqlmodel.ErrorLabel, Value: errLogfmt},
{Name: "__error__", Value: "LogfmtParserErr"},
{Name: "__error_details__", Value: "logfmt syntax error at pos 8 : unexpected '='"},
},
},
{
Expand Down Expand Up @@ -746,6 +748,7 @@ func Test_unpackParser_Parse(t *testing.T) {
labels.Labels{},
labels.Labels{
{Name: "__error__", Value: "JSONParserErr"},
{Name: "__error_details__", Value: "expecting json object(6), but it is not"},
},
[]byte(`"app":"foo","namespace":"prod","_entry":"some message","pod":{"uid":"1"}`),
},
Expand All @@ -755,6 +758,7 @@ func Test_unpackParser_Parse(t *testing.T) {
labels.Labels{{Name: "cluster", Value: "us-central1"}},
labels.Labels{
{Name: "__error__", Value: "JSONParserErr"},
{Name: "__error_details__", Value: "expecting json object(6), but it is not"},
{Name: "cluster", Value: "us-central1"},
},
[]byte(`["foo","bar"]`),
Expand Down
9 changes: 5 additions & 4 deletions pkg/logqlmodel/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import (
// Those errors are useful for comparing error returned by the engine.
// e.g. errors.Is(err,logqlmodel.ErrParse) let you know if this is a ast parsing error.
var (
ErrParse = errors.New("failed to parse the log query")
ErrPipeline = errors.New("failed execute pipeline")
ErrLimit = errors.New("limit reached while evaluating the query")
ErrorLabel = "__error__"
ErrParse = errors.New("failed to parse the log query")
ErrPipeline = errors.New("failed execute pipeline")
ErrLimit = errors.New("limit reached while evaluating the query")
ErrorLabel = "__error__"
ErrorDetailsLabel = "__error_details__"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is probably my biggest question... Is this the right name for this label??

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

)

// ParseError is what is returned when we failed to parse.
Expand Down