-
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
Load Filebeat modules pipelines on -setup #3394
Conversation
See also the discussion here: #3159 (comment) |
3cd9e60
to
bb51843
Compare
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.
Some thoughts on -setup
flag:
Running filebeat with -setup
is probably a manual process and is rarely automated. This would lead me more to the conclusion it should not continue after having the setup completed.
On the other hand, for a getting started experience it would be great if it continues. Also assuming it is automated and there is one server that always starts with -setup, it would mean updating to most recent dashboards or others could be done and this server could still harvest directly log files, without needing a restart.
A downside of having it running directly is that when stopping filebeat after -setup it could lead to a few duplicated events potentially, when no shutdown_timeout
is used.
So kind of both options are not optimal. What if we would have an additional flag? We could reuse -once
here that in case it is set, it will only run the setup. Or have a new flag with a better name that indicates it should only do the setup (-setup-only
)?
@@ -19,6 +20,7 @@ import ( | |||
) | |||
|
|||
var once = flag.Bool("once", false, "Run filebeat only once until all harvesters reach EOF") |
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.
group together under var(
?
bb51843
to
eae83d1
Compare
@@ -63,11 +67,33 @@ func New(b *beat.Beat, rawConfig *common.Config) (beat.Beater, error) { | |||
return fb, nil | |||
} | |||
|
|||
// Setup is called on user request (the -setup flag) to do the initial Beat setup. | |||
func (fb *Filebeat) Setup(b *beat.Beat) error { |
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.
Didn't we have a Setup step in the 1.0 beats interface? Old days :-)
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.
Yeah :). I didn't add it yet into the interface but I have a feeling that's where it will eventually get there.
] | ||
# print(" ".join(cmd) + "\n") |
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 be removed
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 replaced the print with outputting to the log. Having the command already built in the logs is fantastic for troubleshooting :)
eae83d1
to
d3da1e5
Compare
d3da1e5
to
7edb639
Compare
The main reason to add the |
@@ -240,3 +241,29 @@ func (reg *ModuleRegistry) GetProspectorConfigs() ([]*common.Config, error) { | |||
} | |||
return result, nil | |||
} | |||
|
|||
func (reg *ModuleRegistry) Setup(esClient *elasticsearch.Client) error { |
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.
do we need the full elasticsearch.Client
here? If we define an interface with methods required, it get's us an idea, well, what's really required for ModuleRegistry + we can easily mock the client in tests.
This adds the `-setup` CLI flag, which, for now, makes Filebeat load the pipelines at startup. In case Elasticsearch is not available when Filebeat is started with `-setup`, Filebeat will exit with an error. This also exposes an Elasticsearch client from the output.
1a83b00
to
920388a
Compare
This adds the
-setup
CLI flag, which, for now, makes Filebeat load the pipelines at startup. In case Elasticsearch is not available when Filebeat is started with-setup
, Filebeat will exit with an error.This is the first version that passes the modules tests without the help of
filebeat.py
🎉 .This also exposes an Elasticsearch client from the output.
A few more things to note/discuss:
-setup
to Filebeat only. As we add support to loading the dashboards as well, we'll probably want to move the flag in libbeat and make it available for all Beats.-setup
exits on errors but continues with a normal Filebeat run in case of success. The idea is that you would use-setup
to make the initial setup but also to test it. After that is successful, you would typically stop Filebeat and start it in the background via the init scripts (so without-setup
).filebeat -setup
, the user gets an error directly, it doesn't "wait" for ES.Connection.request
is made public asConnection.Request
. This goes in the direction of separating the client. I only needed it in tests, though, so I can take a different approach if needed.-setup
will iterate through them and use the first one that responds to an HTTP ping.Part of #3159.