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

implementation of the logstash.yml settings file #4856

Closed
wants to merge 12 commits into from

Conversation

jsvd
Copy link
Member

@jsvd jsvd commented Mar 21, 2016

This settings.yml located at LOGSTASH_HOME will override the defaults settings but can still be overridden itself by command line arguments.

So the setting precedence is, from weakest to strongest: default settings < yaml settings < cli settings

TODO
make conf/logstash.yml location configurable using env var CONF_DIR or similar

@ph
Copy link
Contributor

ph commented Mar 21, 2016

On it.

# Set the number of workers that will, in parallel, execute the filters+outputs
# stage of the pipeline.
#
# This defaults to half the number of the host's CPU cores.
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we have changed that to use all the core.

@ph
Copy link
Contributor

ph commented Mar 21, 2016

@jsvd I like the code, I think the only thing I would try to squeak in for this PR is to encapsulate this logic into his own class like a Config or a Settings class and we could make it behave like hash/Enum for the accessors. Doing this would allow us future improvements PR with this:

  1. Decouple the config from the actual logic.
  2. Create better validation for the configuration using the visitor patterns, theses validations could be read from a directory and executed when we call a #validate
  3. Necessary coercion could happen in the config class.
  4. The internal data could be saved in an object instead of the raw value and keep track of what changed the value.
  5. The global configuration of logstash should be made available to the plugins, lets say a plugin need a since_db like the file and the s3 input. A since DB datastore could be defined at the config level and reused by multiple plugins.
  6. Drop clamp for better control.
  7. Config Management could be use to update the config store, We could also implement the observer pattern, so when the config is changed the children could act on theses changes if needed.
  8. The config could return a richer object or factories in some case?

@ph
Copy link
Contributor

ph commented Mar 21, 2016

I think most of the suggestions/ideas could be encapsulated in the config object and add improvements to the config system without having to change anything else in the core classes, except the one removing clamp :P

@jsvd
Copy link
Member Author

jsvd commented May 6, 2016

closed in favor of #5251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants