[plugins] Remove support for go plugins#45840
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
652d12c to
7de76a1
Compare
bf2d2c9 to
404c3ca
Compare
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
| github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption => github.com/elastic/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption v1.1.0-elastic | ||
| github.com/apoydence/eachers => github.com/poy/eachers v0.0.0-20181020210610-23942921fe77 //indirect, see https://github.com/elastic/beats/pull/29780 for details. | ||
| github.com/dop251/goja => github.com/elastic/goja v0.0.0-20190128172624-dd2ac4456e20 | ||
| github.com/elastic/go-quark => github.com/leehinman/go-quark v0.3.1 |
There was a problem hiding this comment.
This will have to be switched back to official elastic go-quark repo. The only change between this and official v0.3.0 is a backport of elastic/go-quark@12841a4 which allows linking to the correct GLIBC version depending on OS arch.
There was a problem hiding this comment.
I wonder if we can just require the particular commit instead of a fork here, since we plan to go back to the elastic package anyway.
There was a problem hiding this comment.
My approval is pending the removal of this replace statement :)
mauri870
left a comment
There was a problem hiding this comment.
LGTM, I did a quick search and could not find any more references to plugins in the codebase.
rdner
left a comment
There was a problem hiding this comment.
I thought the whole idea behind plugins was to let users to extend Beats functionality.
How can we be so sure that by deleting the plugin support we would not disrupt our users?
Some of the plugin support was added by external contributors (autodiscover). I personally don't have enough confidence to completely delete the plugin support.
| @@ -1,48 +0,0 @@ | |||
| // Licensed to Elasticsearch B.V. under one or more contributor | |||
There was a problem hiding this comment.
The autodiscover plugin support was an external contribution #6457
This makes me believe someone somewhere might be using it. How can we be sure?
There was a problem hiding this comment.
Perhaps we can send an email to the contributors mailing list?
There was a problem hiding this comment.
If so, I think it should not be limited to just autodiscover plugins.
There was a problem hiding this comment.
Plugins originally came from #3217 and depend on https://pkg.go.dev/plugin#hdr-Warnings.
There are no tests or documentation, the chances of anyone successfully using this is very low but I can find references to people trying to use them in discuss:
https://discuss.elastic.co/search?q=plugin%20%23elastic-stack%3Abeats
- https://discuss.elastic.co/t/problems-with-custom-beat-plugin/231389
- https://discuss.elastic.co/t/is-it-possible-to-develop-a-custom-plugin-for-filebeat/339952
That's someone trying and failing to write a plugin and another one that claims plugins aren't supported by a non-elastic user. A lot of the time "plugin" is used for someone else like "module" or writing a custom processor or output in a forked Beats repository.
I think I'm pro removal overall, if we were getting a lot of questions about this we probably would have deprecated it already.
|
Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services) |
|
Pinging @elastic/sec-linux-platform (Team:Security-Linux Platform) |
|
@leehinman what's the status of this one? Does it need to get tracked in the global paths follow up work issue? |
Not strickly required, but it does reduce the amount of work in global paths follow up issue. I added a link there. I should be able to get back this next week. |
97e81ca to
6912a87
Compare
|
go-quark v0.4.0 fixes the building on arm linux, but changes signature of |
|
This pull request is now in conflicts. Could you fix it? 🙏 |
|
Given this PR is old, in conflict and uses the old release-note format, I've opened a new one here: #51320 |
Proposed commit message
Remove support for go
plugins. No plugins were ever shipped and this removes the plugin registry which was a package level global variable. The package level global variable needs to be removed for beatreceivers where multiple instance of beat receivers will exist in one process.Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.Disruptive User Impact
Should be none since we didn't ship any plugins. However, if someone compiled their own plugins this would be a breaking change.
Author's Checklist
How to test this PR locally
Run integration tests
Related issues
Use cases
Screenshots
Logs