Use labelbuilder in syntax.ParseLabels to remove empty label values (#7355)

There's a bug in TSDB where empty label values alter the series
fingerprints used in chunk addresses, but are stripped out and create
different fingerprints in our TSDB based index. I plan to open a PR to
Prometheus, but I also prefer normalizing labels here by removing empty
label values.

According to
[Prometheus](https://prometheus.io/docs/concepts/data_model/),
> A label with an empty label value is considered equivalent to a label
that does not exist.

Our bug comes from the Prometheus model `LabelBuilder` stripping out
empty label values,
[here](https://github.com/grafana/loki/blob/main/vendor/github.com/prometheus/prometheus/model/labels/labels.go#L419-L423).
This means that `Labels.NewBuilder({job="foo",bar=""}) => {job="foo"}`.
The
[labels.Hash()](https://github.com/grafana/loki/blob/main/vendor/github.com/prometheus/prometheus/model/labels/labels.go#L136-L182)
function does not skip empty label values, meaning it's easy to end up
with different fingerprints for effectively the same series (an empty
label value is supposed to be equivalent to a missing label in
prometheus, but they generate different label hashes).
pull/7361/head
Owen Diehl 4 years ago committed by GitHub
parent c657857e95
commit eaa0919e53
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 3
      pkg/logql/syntax/parser.go
  2. 24
      pkg/logql/syntax/parser_test.go

@ -203,5 +203,6 @@ func ParseLabels(lbs string) (labels.Labels, error) {
return nil, err
}
sort.Sort(ls)
return ls, nil
return labels.NewBuilder(ls).Labels(), nil
}

@ -3274,3 +3274,27 @@ func TestParseLogSelectorExpr_equalityMatcher(t *testing.T) {
})
}
}
func TestParseLabels(t *testing.T) {
for _, tc := range []struct {
desc string
input string
output labels.Labels
}{
{
desc: "basic",
input: `{job="foo"}`,
output: []labels.Label{{Name: "job", Value: "foo"}},
},
{
desc: "strip empty label value",
input: `{job="foo", bar=""}`,
output: []labels.Label{{Name: "job", Value: "foo"}},
},
} {
t.Run(tc.desc, func(t *testing.T) {
got, _ := ParseLabels(tc.input)
require.Equal(t, tc.output, got)
})
}
}

Loading…
Cancel
Save