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.

Sign up for GitHub

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

Jump to bottom

Add filebeat modules config reloading #4566

Merged
merged 5 commits into from
Jul 5, 2017
Merged

Add filebeat modules config reloading #4566

merged 5 commits into from
Jul 5, 2017

Conversation

exekias
Copy link
Contributor

@exekias exekias commented Jun 27, 2017

This PR adds support for module reloading, mimicking current prospectors functionality. The mechanism is the same, just define:

filebeat.config.modules:
  enabled: true
  path: configs/*.yml
  reload.enabled: true
  reload.period: 10s

@exekias exekias added Filebeat Filebeat in progress Pull request is currently in progress. needs_docs review labels Jun 27, 2017
@exekias exekias mentioned this pull request Jun 27, 2017
Merged
4 tasks
@exekias exekias force-pushed the filebeat-modules-reload branch 6 times, most recently from 4d30d80 to 759420a Compare June 27, 2017 22:36
@exekias exekias removed the in progress Pull request is currently in progress. label Jun 28, 2017
tsg
tsg reviewed Jul 3, 2017
View reviewed changes
CHANGELOG.asciidoc Outdated
@@ -68,6 +68,7 @@ https://github.com/elastic/beats/compare/v6.0.0-alpha2...master[Check the HEAD d
- Add support for loading Xpack Machine Learning configurations from the modules, and added sample configurations for the Nginx module. {pull}4506[4506]
- Add udp prospector type. {pull}4452[4452]
- Enabled Cgo which means libc is dynamically compiled. {pull}4546[4546]
- Add module config reloading mechanism {pull}4566[4566]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably mark this as beta, since the config reloading in metricbeat is also beta.

tsg
tsg reviewed Jul 3, 2017
View reviewed changes
filebeat/crawler/crawler.go Outdated
logp.Beta("Loading separate prospectors is enabled.")

c.reloader = cfgfile.NewReloader(configModules)
// TODO add beatVersion here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you pass it above, so you could add it now?

@tsg
Copy link
Contributor

tsg commented Jul 3, 2017

What I didn't see in this PR is where are the Ingest Nodes pipelines loaded. They need to be registered with the Elasticsearch output so they are loaded on each re-connect. See here.

tsg
tsg reviewed Jul 3, 2017
View reviewed changes
filebeat/fileset/factory.go Outdated
"github.com/elastic/beats/filebeat/registrar"
)

// Factory is a factory for registrars
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really a factory of registrars? Or did you mean a factory of modules or prospectors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm, this was copied from here: https://github.com/elastic/beats/blob/master/filebeat/prospector/factory.go#L11, I'll better put factory of modules

@exekias
Copy link
Contributor Author

exekias commented Jul 3, 2017

Thanks for the review, I'm adding on demand pipeline load for dynamic modules, what do you think about ML jobs? I'm inclined to leave them out of this

@exekias exekias force-pushed the filebeat-modules-reload branch 6 times, most recently from 27abf9c to b5a3e3a Compare July 3, 2017 16:12
@tsg
Copy link
Contributor

tsg commented Jul 4, 2017

@exekias Yeah, ML jobs and dashboards should be on setup only, so not on reload.

tsg
tsg approved these changes Jul 5, 2017
View reviewed changes
Copy link
Contributor

@tsg tsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@tsg tsg merged commit 63c6784 into elastic:master Jul 5, 2017
@dedemorton dedemorton mentioned this pull request Jul 25, 2017
Closed
42 tasks
@dedemorton
Copy link
Contributor

Removing needs_docs tag. Docs were added in #4930

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsg tsg tsg approved these changes

Assignees
No one assigned
Labels
Filebeat Filebeat review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@exekias @tsg @dedemorton