-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
feat(serializers): Add a serializer based on go templates #13656
Conversation
Hmmm. I was very cut and pasty with this PR. Now reading #13606, I'm thinking I shouldn't fully copy the way it's being done in the template processor plugin, as it looks like we are moving away from that method. |
@gazpachoking right. You can just use the metric directly and the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gazpachoking this already looks very beautiful, thanks for your contribution. I do have some smaller comments and one request. The request is regarding the single vs. batch serialization. I think you should match the template input to what is intended by the user as the user cannot know which method is called.
I don't believe the latest ci failure is to do with my changes. |
Hmm, working with the go templating engine a bit has shown that it's a bit inflexible (without some helpers.) I'm thinking we should include the sprig functions here. Thoughts? |
Hmm sprig doesn't appear to be maintained or updated in ~year. Is there a use-case that you could provide for using them versus not using the library? |
I was having trouble trying to comma separate tags or metrics without putting a trailing comma. Really it's sorta convoluted even with sprig with tags, since their dict functions don't work directly on the tags map (they work fine on the fields map though.) Here's what I did using sprig, I couldn't even figure out how to do it at all without it.
The toJson fromJson dance is the convoluted bit I mentioned. That converts the tag map from I picked it because Helm uses it, and it's already used in the postgresql output plugin. I'm fine with removing it though, but I would like to figure out if doing commas between things is possible, as that seems like it might be a common use case. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice update @gazpachoking! Have some more comments...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the PR. @gazpachoking can you please show one config in the README where all available options are included to give users an overview of what is there!? You might then show more config-examples below for different use-cases....
Added the batch_template option in the example. Also noticed that the TOML parser doesn't allow embedding double quotes inside a basic multi-line string, which the spec says should be fine. (it also doesn't seem to like single quotes within a multi-line literal string) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks @gazpachoking for the cool serializer!
Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
Thank you @gazpachoking! This serializer is exactly what I need! When I try to use it with [[outputs.file]] it works just fine. Full config:
I'm using telegraf-1.28.0~ff54c603_linux_amd64.tar.gz from the link above. |
This is not a valid option for the socket_writer. The socket writer writes each metric as it comes, not in batches. |
@ven0hm Do you need the batch format for your purposes, or are individual metrics fine? Allowing batch format in the socket_writer plugin would be a valid feature request. |
Required for all PRs
resolves #13637
Adds a new serializer that lets the user define a go template for the serialization format.
I haven't really used Go before, so this might need a bit of review.