Skip to content

Commit

Permalink
Merge pull request #11074 from damnever/fix/datamodelvalidation
Browse files Browse the repository at this point in the history
Validate the metric name and label names
  • Loading branch information
roidelapluie committed Dec 8, 2022
2 parents 7189f91 + 9979024 commit bb323db
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 0 deletions.
14 changes: 14 additions & 0 deletions model/labels/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"strconv"

"github.com/cespare/xxhash/v2"
"github.com/prometheus/common/model"
)

// Well-known label names used by Prometheus components.
Expand Down Expand Up @@ -312,6 +313,19 @@ func (ls Labels) WithoutEmpty() Labels {
return ls
}

// IsValid checks if the metric name or label names are valid.
func (ls Labels) IsValid() bool {
for _, l := range ls {
if l.Name == model.MetricNameLabel && !model.IsValidMetricName(model.LabelValue(l.Value)) {
return false
}
if !model.LabelName(l.Name).IsValid() || !model.LabelValue(l.Value).IsValid() {
return false
}
}
return true
}

// Equal returns whether the two label sets are equal.
func Equal(ls, o Labels) bool {
if len(ls) != len(o) {
Expand Down
56 changes: 56 additions & 0 deletions model/labels/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,62 @@ func TestLabels_WithoutEmpty(t *testing.T) {
}
}

func TestLabels_IsValid(t *testing.T) {
for _, test := range []struct {
input Labels
expected bool
}{
{
input: FromStrings(
"__name__", "test",
"hostname", "localhost",
"job", "check",
),
expected: true,
},
{
input: FromStrings(
"__name__", "test:ms",
"hostname_123", "localhost",
"_job", "check",
),
expected: true,
},
{
input: FromStrings("__name__", "test-ms"),
expected: false,
},
{
input: FromStrings("__name__", "0zz"),
expected: false,
},
{
input: FromStrings("abc:xyz", "invalid"),
expected: false,
},
{
input: FromStrings("123abc", "invalid"),
expected: false,
},
{
input: FromStrings("中文abc", "invalid"),
expected: false,
},
{
input: FromStrings("invalid", "aa\xe2"),
expected: false,
},
{
input: FromStrings("invalid", "\xF7\xBF\xBF\xBF"),
expected: false,
},
} {
t.Run("", func(t *testing.T) {
require.Equal(t, test.expected, test.input.IsValid())
})
}
}

func TestLabels_Equal(t *testing.T) {
labels := FromStrings(
"aaa", "111",
Expand Down
4 changes: 4 additions & 0 deletions scrape/scrape.go
Original file line number Diff line number Diff line change
Expand Up @@ -1609,6 +1609,10 @@ loop:
err = errNameLabelMandatory
break loop
}
if !lset.IsValid() {
err = fmt.Errorf("invalid metric name or label names: %s", lset.String())
break loop
}

// If any label limits is exceeded the scrape should fail.
if err = verifyLabelLimits(lset, sl.labelLimits); err != nil {
Expand Down
45 changes: 45 additions & 0 deletions scrape/scrape_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1020,6 +1020,51 @@ func TestScrapeLoopSeriesAdded(t *testing.T) {
require.Equal(t, 0, seriesAdded)
}

func TestScrapeLoopFailWithInvalidLabelsAfterRelabel(t *testing.T) {
s := teststorage.New(t)
defer s.Close()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

target := &Target{
labels: labels.FromStrings("pod_label_invalid_012", "test"),
}
relabelConfig := []*relabel.Config{{
Action: relabel.LabelMap,
Regex: relabel.MustNewRegexp("pod_label_invalid_(.+)"),
Separator: ";",
Replacement: "$1",
}}
sl := newScrapeLoop(ctx,
&testScraper{},
nil, nil,
func(l labels.Labels) labels.Labels {
return mutateSampleLabels(l, target, true, relabelConfig)
},
nopMutator,
s.Appender,
nil,
0,
true,
0,
nil,
0,
0,
false,
false,
nil,
false,
)

slApp := sl.appender(ctx)
total, added, seriesAdded, err := sl.append(slApp, []byte("test_metric 1\n"), "", time.Time{})
require.ErrorContains(t, err, "invalid metric name or label names")
require.NoError(t, slApp.Rollback())
require.Equal(t, 1, total)
require.Equal(t, 0, added)
require.Equal(t, 0, seriesAdded)
}

func makeTestMetrics(n int) []byte {
// Construct a metrics string to parse
sb := bytes.Buffer{}
Expand Down

0 comments on commit bb323db

Please sign in to comment.