feat: Parser plugin restructuring#8791
Merged
reimda merged 51 commits intoinfluxdata:masterfrom Jan 12, 2022
Merged
Conversation
ssoroka
suggested changes
Mar 16, 2021
Contributor
ssoroka
left a comment
There was a problem hiding this comment.
overall I think it's headed in the right direction. some thoughts:
- I don't think you can remove parsers.Parsers without breaking backwards compatibility, it's part of the contract we maintain to plugins.
- I don't feel like giving parsers their own runners makes sense in the way it does for inputs/etc, because they really belong to the plugin using them.
- FYI there's future parser work related to stream parsers, and supporting multiple parsers per input plugin at some point.
3 tasks
sspaink
reviewed
Nov 11, 2021
08b4dce to
7252f1a
Compare
Member
Author
|
@sspaink, @reimda, @powersj, @MyaLongmire here is the discussed update. Would love to get your feedback! |
12b9a40 to
0c02235
Compare
sspaink
approved these changes
Dec 3, 2021
Contributor
sspaink
left a comment
There was a problem hiding this comment.
Great work! I think this looks good.
Contributor
added 9 commits
January 12, 2022 14:23
…ward-compatibility.
…. perform AND operations for input/parser misses.
… the old way otherwise.
added 17 commits
January 12, 2022 14:31
… with external plugins.
0c02235 to
8109072
Compare
Contributor
|
📦 Looks like new artifacts were built from this PR. Expand this list to get them here ! 🐯Artifact URLs |
reimda
approved these changes
Jan 12, 2022
Contributor
reimda
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks for working on this. It will be nice to switch them all over to the new system and then remove the old compatibility code!
3 tasks
13 tasks
powersj
pushed a commit
to powersj/telegraf
that referenced
this pull request
Jan 21, 2022
powersj
pushed a commit
to powersj/telegraf
that referenced
this pull request
Jan 21, 2022
Member
Author
|
@etycomputer could you please submit a PR adding this as a test-case? Your help would be very much appreciated! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Required for all PRs:
This PR restructures the parser plugin infrastructure to match those of other plugins (e.g. input, aggregators,...) with a central registry and config definitions (including TOML tags) in the parser code itself. This reliefs new parsers for spraying the code all over the project and allow them to keep all in one place (the parser code).
By using the new infrastructure you get some goodies for free like support of logging via definition of
Log telegraf.Logger 'toml:"-"'and internal plugin statisticsmetrics_parsedandparse_time_ns. We keep backward compatibility so parsers not ported yet are still supported.This PR currently only converts the CSV parser to the new interface as a PoC and fixes the fallout.
The main benefit is that now, implementing new parser plugins has the same procedure as all other plugins. You write your self-contained parser in a sub-directory below
plugins/parsers, add it toplugins/parsers/all/all.goand update the documentation. No need anymore to add TOML exceptions inconfigor fight withplugins/parsers/registration.go.