Exec input with Graphite protocol support #637
Conversation
…af mertic paser to the internal/encoding direcotry.
henrypfhu
changed the title
henrypfhu
changed the title
henrypfhu
changed the title
| # the command to run | ||
| command = "/usr/bin/mycollector --foo=bar" | ||
| # Shell/commands array | ||
| commands = ["/tmp/test.sh","/tmp/test2.sh"] |
There was a problem hiding this comment.
This is a breaking change. You'll need to change unit tests for this. Also exec should continue to support users that have the command = specified for backwards compatibility.
…with old command configuration.
| func (e *Exec) SampleConfig() string { | ||
| return sampleConfig | ||
| } | ||
|
|
||
| func (e *Exec) Description() string { | ||
| return "Read flattened metrics from one or more commands that output JSON to stdout" | ||
| return "Read metrics from one or more commands that output graphite line protocol to stdout" |
… exe description...
| return &Parser{graphiteParser: parser} | ||
| } | ||
|
|
||
| func (p *Parser) Parse(dataFormat string, out []byte, acc telegraf.Accumulator) error { |
There was a problem hiding this comment.
Parse functions should not be specific to any plugin, just make two functions here with signatures:
func (p *Parser) Parse(dataFormat string, buf []byte) ([]telegraf.Metric, error)
func (p *Parser) ParseLine(dataFormat, line string) (telegraf.Metric, error)
There was a problem hiding this comment.
and they should both support JSON, influx, and graphite
|
@henrypfhu This is looking very good so far, I like the structure I just have a few comments on the generic parser 👍 Since the graphite template parser was mostly taken from the influxdb repo, can you also add the unit tests from that repo? (you might need to remove a few of them because of the tags removal) |
| @@ -158,7 +158,9 @@ Currently implemented sources: | |||
| * disque | |||
| * docker | |||
| * elasticsearch | |||
| * exec (generic JSON-emitting executable plugin) | |||
| * exec (generic line-protocol-emitting executable plugin, support JSON, influx and graphite) | |||
| * socket (generic line protocol listen input service, support influx and graphite) | |||
There was a problem hiding this comment.
this PR doesn't include socket or tail
|
Hi @sparrc, from tomorrow I will be in vacation fro Chinese Spring Festival, these days I will be offline. Unit test cases will be postponed. Actually, I already tested all scenarios manually. So far I fixed all bugs I could found. Thx... |
|
Thanks for the heads-up on your holiday time @henrypfhu, hope you enjoy it :-) I squashed your commits into one and am now working on a new branch based off your changes (see #655). I'm going to close this PR because I have some ideas and changes that I would like to make on how the generic parsers feature is going to look, and it's also very relevant to an MQTT consumer plugin that I was coincidentally working on at the same time. Thanks for your great work getting this started! |
According to @sparrc suggestion, I combined the graphite feature into the exec plugin, and refactor the parser module into internal/encoding directory.
I open this new PR for exec plugin.
Old PR please refer to #627