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

Require goplugin build flag to enable go plugin support #6393

Merged
merged 1 commit into from
Sep 20, 2019

Conversation

danielnelson
Copy link
Contributor

It has come to my attention that the Go plugin support added in #6024 had an unexpected and hefty cost in terms of binary size, to the tune of about 19MB.

Despite this feature been released in 1.12, I propose we add a build flag and disable it on official Telegraf builds. Based on my testing of this feature, it is quite difficult to get plugins working without building the plugin and Telegraf together on the same host, and the extra binary size is just too much for a feature that will be rarely used.

To build with Go plugin support add -tags goplugin to the build command:

go build -tags goplugin -ldflags "-w -s -X main.commit=9e3f918d -X main.branch=HEAD" ./cmd/telegraf

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@danielnelson danielnelson added the breaking change Improvement to Telegraf that requires changes to the plugin or agent; for minor/major releases label Sep 13, 2019
@danielnelson danielnelson added this to the 1.12.2 milestone Sep 13, 2019
@danielnelson
Copy link
Contributor Author

cc @Timidger @adrianlzt @mtp401

@adrianlzt
Copy link
Contributor

Thanks for the update.
I was trying to use the plugin feature but it does not work with Windows so I return to using a normal plugin with a custom build.

@danielnelson danielnelson merged commit 776e92f into master Sep 20, 2019
@danielnelson danielnelson deleted the build-flag-goplugins branch September 20, 2019 23:50
danielnelson added a commit that referenced this pull request Sep 21, 2019
bitcharmer pushed a commit to bitcharmer/telegraf that referenced this pull request Oct 18, 2019
@mildebrandt
Copy link

@danielnelson I'm working on a way to build our internal plugins and package them into the telegraf docker container. Correct me if I'm wrong, but it sounds like the docker container won't include a build of telegraf that supports plugins. I was hoping to build on top of the existing container, insert my plugins/conf files, and repackage. With this PR, it sounds like I need to separately build/host/support a telegraf binary/container if I want plugin support.

Is that correct? If so, any chance of Influx making a separate docker container with a telegraf binary that supports plugins as part of the release cycle?

@danielnelson
Copy link
Contributor Author

That is true, you will need to make an entirely new docker container.

I'm not sure we would want to package a docker container with this feature, since there are a bunch of requirements that have to line up between the Telegraf build and the plugin build: Go compiler version, GOPATH, CGO_ENABLED, and potentially other details. I wonder if it can be made compatible enough where it can be taken and reused without issue.

That said, I think we would be willing to add an AMD64 .deb/.rpm package with the tag enabled and you could also open a new issue for the docker image and we can see what sort of demand there is.

@mildebrandt
Copy link

Thanks, a package that enables plugin support would also be welcome. I've opened this to request it: #7003

@mildebrandt
Copy link

And I've opened this for the docker container: influxdata/influxdata-docker#369

athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Improvement to Telegraf that requires changes to the plugin or agent; for minor/major releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants