Skip to content

Commit

Permalink
Ensure correct number of tags parsed
Browse files Browse the repository at this point in the history
This commit fixes a parsing bug that was causing extra tags to be
generated when the incoming point contained escaped commas.

As an optimisation, the slice of tags associated with a point was
being pre-allocated using the number of commas in the series key as a hint to
the appropriate size. The hinting did not consider literal comma values in
the key though, and so it was possible for extra (empty) tag key and
value pairs to be part of the tags structure associated with a parsed
point.
  • Loading branch information
e-dard committed Mar 14, 2018
1 parent d428231 commit 6b350ed
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 5 deletions.
10 changes: 5 additions & 5 deletions models/points.go
Original file line number Diff line number Diff line change
Expand Up @@ -1551,12 +1551,12 @@ func parseTags(buf []byte) Tags {
return nil
}

tags := make(Tags, bytes.Count(buf, []byte(",")))
p := 0
// Series keys can contain escaped commas, therefore the number of commas
// in a series key only gives an estimation of the upper bound on the number
// of tags.
tags := make(Tags, 0, bytes.Count(buf, []byte(",")))
walkTags(buf, func(key, value []byte) bool {
tags[p].Key = key
tags[p].Value = value
p++
tags = append(tags, Tag{Key: key, Value: value})
return true
})
return tags

This comment has been minimized.

Copy link
@stuartcarnie

stuartcarnie Mar 14, 2018

Contributor

Ugh, the change I made was missing

  	return tags[:p]
Expand Down
36 changes: 36 additions & 0 deletions models/points_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,42 @@ func BenchmarkMarshal(b *testing.B) {
tags.HashKey()
}
}
func TestPoint_Tags(t *testing.T) {
examples := []struct {
Point string
Tags models.Tags
}{
{`cpu value=1`, models.Tags{}},
{"cpu,tag0=v0 value=1", models.NewTags(map[string]string{"tag0": "v0"})},
{"cpu,tag0=v0,tag1=v0 value=1", models.NewTags(map[string]string{"tag0": "v0", "tag1": "v0"})},
{`cpu,tag0=v\ 0 value=1`, models.NewTags(map[string]string{"tag0": "v 0"})},
{`cpu,tag0=v\ 0\ 1,tag1=v2 value=1`, models.NewTags(map[string]string{"tag0": "v 0 1", "tag1": "v2"})},
{`cpu,tag0=\, value=1`, models.NewTags(map[string]string{"tag0": ","})},
{`cpu,ta\ g0=\, value=1`, models.NewTags(map[string]string{"ta g0": ","})},
{`cpu,tag0=\,1 value=1`, models.NewTags(map[string]string{"tag0": ",1"})},
{`cpu,tag0=1\"\",t=k value=1`, models.NewTags(map[string]string{"tag0": `1\"\"`, "t": "k"})},
}

for _, example := range examples {
t.Run(example.Point, func(t *testing.T) {
pts, err := models.ParsePointsString(example.Point)
if err != nil {
t.Fatal(err)
} else if len(pts) != 1 {
t.Fatalf("parsed %d points, expected 1", len(pts))
}

// Repeat to test Tags() caching
for i := 0; i < 2; i++ {
tags := pts[0].Tags()
if !reflect.DeepEqual(tags, example.Tags) {
t.Fatalf("got %#v (%s), expected %#v", tags, tags.String(), example.Tags)
}
}

})
}
}

func TestPoint_StringSize(t *testing.T) {
testPoint_cube(t, func(p models.Point) {
Expand Down

0 comments on commit 6b350ed

Please sign in to comment.