Parser processor #4551
Parser processor #4551
Conversation
plugins/processors/parser/parser.go
Outdated
| // holds a default sample config | ||
| var SampleConfig = ` | ||
|
|
||
| ## specify the name of the field[s] whose value will be parsed |
There was a problem hiding this comment.
Double space the sample config and remove the empty line on line 20.
plugins/processors/parser/parser.go
Outdated
|
|
||
| for _, metric := range metrics { | ||
| for _, key := range p.parseFields { | ||
| value := metric.Fields()[key] |
There was a problem hiding this comment.
It might not matter, but we could probably do if value, ok := metric.Fields()[key]; ok { here, to ensure there is a field value for the key.
Enhance tests Ensure useable by telegraf
plugins/processors/parser/parser.go
Outdated
| ) | ||
|
|
||
| type Parser struct { | ||
| Config parsers.Config `toml:"config"` |
There was a problem hiding this comment.
Use an embedded field for the parsers.Config:
type Parser struct {
parsers.Config `toml:"config"`
This should make it so you can specify the options directly on the Parser object, similar to how it is set in input plugins:
[[processors.parser]]
parse_fields = ["message"]
original = "merge"
data_format = "json"
tag_keys = ["verb", "request"]
plugins/processors/parser/parser.go
Outdated
| } | ||
| sMetrics := []telegraf.Metric{} | ||
| for _, key := range p.ParseFields { | ||
| if value, ok := metric.Fields()[key]; ok { |
There was a problem hiding this comment.
Don't use .Fields() since it allocates a new map, (I really need to add a comment suggesting that it not be used). Instead use .FieldList(). You will probably want to reorder the loops since it think ParseFields should always be shorter than Fields.
plugins/processors/parser/parser.go
Outdated
| sMetrics := []telegraf.Metric{} | ||
| for _, key := range p.ParseFields { | ||
| if value, ok := metric.Fields()[key]; ok { | ||
| strVal := fmt.Sprintf("%v", value) |
There was a problem hiding this comment.
If it's not a string then log a warning and skip.
plugins/processors/parser/parser.go
Outdated
| log.Printf("E! [processors.parser] could not parse field %v: %v", key, err) | ||
| switch p.Original { | ||
| case "keep": | ||
| return metrics |
There was a problem hiding this comment.
This doesn't seem right... don't we need to parse all fields before we can return?
There was a problem hiding this comment.
Good catch, the tests missed this.
plugins/processors/parser/parser.go
Outdated
| case "keep": | ||
| return metrics | ||
| case "merge": | ||
| nMetrics = metrics |
There was a problem hiding this comment.
I'm all confused by the variable names: rMetric, sMetric, nMetric. Can you try to improve these? The Metric part is less interesting than the r, s, n parts, since we can easily lookup the type.
plugins/processors/parser/parser.go
Outdated
| } | ||
| sMetrics = append(sMetrics, nMetrics...) | ||
| } else { | ||
| fmt.Println("key not found", key) |
There was a problem hiding this comment.
I would probably ignore this situation to allow fields to be optional.
plugins/processors/parser/parser.go
Outdated
| } | ||
|
|
||
| func (p Parser) setName(name string, metrics ...telegraf.Metric) []telegraf.Metric { | ||
| if len(metrics) == 0 { |
There was a problem hiding this comment.
Don't need this, if you remove it then it will just range over nothing and return metrics.
plugins/processors/parser/parser.go
Outdated
| return nil | ||
| } | ||
|
|
||
| rMetric := metrics[0] |
There was a problem hiding this comment.
This seems like a problem if more than one metric is passed in to Apply. Secretly this never happens but the interface doesn't promise this.
There was a problem hiding this comment.
More than one metric does get passed into this function, it is intended for all the tags and fields to get merged down to the first metric
There was a problem hiding this comment.
More than one metric does get passed into this function, it is intended for all the tags and fields to get merged down to the first metric
danielnelson
force-pushed
the
fieldparser
branch
from
e5ba245
to
d5aec65
Compare
Required for all PRs:
closes #4427