Skip to content

Commit

Permalink
Validate labels (#528)
Browse files Browse the repository at this point in the history
* validate label set coming through WAL Appender

Closes #527

* changelog

* tests
  • Loading branch information
rfratto committed Apr 9, 2021
1 parent 50a6ba8 commit 1bdc571
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ can be found at [#317](https://github.com/grafana/agent/issues/317).

# Main (unreleased)

- [BUGFIX] Validate that incoming scraped metrics do not have an empty label
set or a label set with duplicate labels, mirroring the behavior of
Prometheus. (@rfratto)

# v0.13.0 (2021-02-25)

The primary branch name has changed from `master` to `main`. You may have to
Expand Down
12 changes: 12 additions & 0 deletions pkg/prom/wal/wal.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/prometheus/prometheus/pkg/timestamp"
"github.com/prometheus/prometheus/pkg/value"
"github.com/prometheus/prometheus/storage"
"github.com/prometheus/prometheus/tsdb"
"github.com/prometheus/prometheus/tsdb/record"
"github.com/prometheus/prometheus/tsdb/wal"
)
Expand Down Expand Up @@ -557,6 +558,17 @@ func (a *appender) Add(l labels.Labels, t int64, v float64) (uint64, error) {
return series.ref, a.AddFast(series.ref, t, v)
}

// Ensure no empty or duplicate labels have gotten through. This mirrors the
// equivalent validation code in the TSDB's headAppender.
l = l.WithoutEmpty()
if len(l) == 0 {
return 0, errors.Wrap(tsdb.ErrInvalidSample, "empty labelset")
}

if lbl, dup := l.HasDuplicateLabelNames(); dup {
return 0, errors.Wrap(tsdb.ErrInvalidSample, fmt.Sprintf(`label name "%s" is not unique`, lbl))
}

a.w.mtx.Lock()
ref := a.w.nextRef
a.w.nextRef++
Expand Down
24 changes: 24 additions & 0 deletions pkg/prom/wal/wal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,30 @@ import (
"github.com/stretchr/testify/require"
)

func TestStorage_InvalidSeries(t *testing.T) {
walDir, err := ioutil.TempDir(os.TempDir(), "wal")
require.NoError(t, err)
defer os.RemoveAll(walDir)

s, err := NewStorage(log.NewNopLogger(), nil, walDir)
require.NoError(t, err)
defer func() {
require.NoError(t, s.Close())
}()

app := s.Appender(context.Background())

_, err = app.Add(labels.Labels{}, 0, 0)
require.Error(t, err, "should reject empty labels")

_, err = app.Add(labels.Labels{{Name: "a", Value: "1"}, {Name: "a", Value: "2"}}, 0, 0)
require.Error(t, err, "should reject duplicate labels")

// Sanity check: valid series
_, err = app.Add(labels.Labels{{Name: "a", Value: "1"}}, 0, 0)
require.NoError(t, err, "should not reject valid series")
}

func TestStorage(t *testing.T) {
walDir, err := ioutil.TempDir(os.TempDir(), "wal")
require.NoError(t, err)
Expand Down

0 comments on commit 1bdc571

Please sign in to comment.