-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Enable Cgo for Filebeat & Heartbeat #4546
Conversation
jenkins, package it |
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.
Probably worth a changelog entry
filebeat/Makefile
Outdated
@@ -7,6 +7,10 @@ TEST_ENVIRONMENT?=true | |||
GOX_FLAGS=-arch="amd64 386 arm ppc64 ppc64le" | |||
ES_BEATS?=.. | |||
|
|||
TARGETS?="windows/amd64 windows/386 darwin/amd64" | |||
TARGETS_OLD?="linux/amd64 linux/386" | |||
CGO=true |
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.
Should we change this in the libbeat Makefile to make it default for all beats?
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 worry it would complicate the lives of the community beats. Also, it's still not enabled for Winlogbeat, but we could overwrite it in that one.
Overall, it seemed like a less risky move to do it per Beat, but it's true that we'll have to be careful to do this whenever we add a new Beat. Any strong arguments to make it the default?
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.
My only argument would be that if it breaks somewhere we know earlier. I see the community beats complication but would hope they use our packaging scripts, so they get it out of the box?
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.
Ok, I was on the fence about it, so I changed the PR to make CGO=true
the default in all Beats, and overwrite it to false
in Winlogbeat.
jenkins, package it |
Packaging tests are green: http://build-eu-00.elastic.co/job/beats-package-PR/107/ |
This is to enable plugin support, and to enable future support (during the 6.x cycle) for importing other C libraries. Metricbeat & Packetbeat already had Cgo enabled. We're leaving Winlogbeat without Cgo because we don't foresee a reason for it to need it. This change also affects the community Beats, but they can revert to pure-go by adding `CGO=false` in their Makefile. Switch Cgo to be the default
It was all green, but I rebased it to solve a CHANGELOG conflict. |
This is to enable plugin support, and to enable future support (during the
6.x cycle) for importing other C libraries.
Metricbeat & Packetbeat already had Cgo enabled. We're leaving Winlogbeat
without Cgo because we don't foresee a reason for it to need it.
This change also affects the community Beats, but they can revert to pure-go
by adding
CGO=false
in their Makefile.