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

Fixed race condition in the Wavefront parser #5764

Merged
merged 3 commits into from
Apr 25, 2019
Merged
Changes from 2 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
72 changes: 49 additions & 23 deletions plugins/parsers/wavefront/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io"
"log"
"strconv"
"sync"
"time"

"github.com/influxdata/telegraf"
Expand All @@ -22,18 +23,23 @@ type Point struct {
Tags map[string]string
}

// Parser represents a parser.
type WavefrontParser struct {
parsers *sync.Pool
defaultTags map[string]string
}

// PointParser is a thread-unsafe parser and must be kept in a pool.
type PointParser struct {
s *PointScanner
buf struct {
tok []Token // last read n tokens
lit []string // last read n literals
n int // unscanned buffer size (max=2)
}
scanBuf bytes.Buffer // buffer reused for scanning tokens
writeBuf bytes.Buffer // buffer reused for parsing elements
Elements []ElementParser
defaultTags map[string]string
scanBuf bytes.Buffer // buffer reused for scanning tokens
writeBuf bytes.Buffer // buffer reused for parsing elements
Elements []ElementParser
parent *WavefrontParser
}

// Returns a slice of ElementParser's for the Graphite format
Expand All @@ -47,9 +53,41 @@ func NewWavefrontElements() []ElementParser {
return elements
}

func NewWavefrontParser(defaultTags map[string]string) *PointParser {
func NewWavefrontParser(defaultTags map[string]string) *WavefrontParser {
wp := &WavefrontParser{defaultTags: defaultTags}
wp.parsers = &sync.Pool{
New: func() interface{} {
log.Printf("D! [parsers.wavefront] Creating new parser for %s", wp)
Copy link
Contributor

Choose a reason for hiding this comment

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

(This is the ci test failure) I would just remove the for %s part completely. Default tags never change unless Telegraf is reloaded and are shared on all inputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

return NewPointParser(wp)
},
}
return wp
}

func NewPointParser(parent *WavefrontParser) *PointParser {
elements := NewWavefrontElements()
return &PointParser{Elements: elements, defaultTags: defaultTags}
return &PointParser{Elements: elements, parent: parent}
}

func (p *WavefrontParser) ParseLine(line string) (telegraf.Metric, error) {
buf := []byte(line)

metrics, err := p.Parse(buf)
if err != nil {
return nil, err
}

if len(metrics) > 0 {
return metrics[0], nil
}

return nil, nil
}

func (p *WavefrontParser) Parse(buf []byte) ([]telegraf.Metric, error) {
pp := p.parsers.Get().(*PointParser)
defer p.parsers.Put(pp)
return pp.Parse(buf)
}

func (p *PointParser) Parse(buf []byte) ([]telegraf.Metric, error) {
Expand All @@ -61,6 +99,8 @@ func (p *PointParser) Parse(buf []byte) ([]telegraf.Metric, error) {
buf = append(buf, []byte("\n")...)
}

//log.Printf("D! [parsers.wavefront] Received data: %s", string(buf))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!


points := make([]Point, 0)

buffer := bytes.NewBuffer(buf)
Expand Down Expand Up @@ -91,21 +131,7 @@ func (p *PointParser) Parse(buf []byte) ([]telegraf.Metric, error) {
return metrics, nil
}

func (p *PointParser) ParseLine(line string) (telegraf.Metric, error) {
buf := []byte(line)
metrics, err := p.Parse(buf)
if err != nil {
return nil, err
}

if len(metrics) > 0 {
return metrics[0], nil
}

return nil, nil
}

func (p *PointParser) SetDefaultTags(tags map[string]string) {
func (p *WavefrontParser) SetDefaultTags(tags map[string]string) {
p.defaultTags = tags
}

Expand All @@ -119,7 +145,7 @@ func (p *PointParser) convertPointToTelegrafMetric(points []Point) ([]telegraf.M
tags[k] = v
}
// apply default tags after parsed tags
for k, v := range p.defaultTags {
for k, v := range p.parent.defaultTags {
tags[k] = v
}

Expand Down