Adding configurable sincedb directory and file prefix

[phutchins]

No description provided.

[elasticsearch-release]

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

[phutchins]

Any word on getting this merged?

[untergeek]

@phutchins it looks like there are merge conflicts. Could you please rebase your code?

Also, please make sure the versions match up correctly and add a blurb to the CHANGELOG that matches your version number and changes.

[phutchins]

@untergeek done. Let me know if theres anything more needed. Thanks!

[untergeek]

Thanks for that. What's the use case here? Why is sincedb_path insufficient?

[phutchins]

The use case is that sincedb_path doesn't allow you to set the path independently from the filename. If you have multiple file inputs it is nice to be able to control this. For instance, we run logstash inside of a docker container and I wanted to expose the sincedb files in a specific directory so that it can be shared with the host and be long lived after rebuilding the container.

It also allows for customizing the file prefix while still keeping the hash so that filter changes will build a new since_db file.

These changes however keep the existing behavior as is as to not break anything...

[ph]

@phutchins I see the use case now, its mostly for ease of administration, I understand people running mulitples logstash instances with file input can be a bit of a pain to deal with. But, I am still a little worried if this change will confuse our users more, if this plugin define 3 options around the same context. But, I think if we make it clear in the doc what happen and when and which config option will be used, it will be fine. (Who know we might even deprecated the sincedb_path in favor of more granular options?)

I think this PR, would benefit to have some test to cover the changes, I dont know if the current test cover all the cases of the sincedb_path.

Also since you are adding a new config option that will conflict with an existing one I think we should raise a ConfigurationError to the users if they try to define both in the configuration, I am talking about sincedb_path and sincedb_dir

Also, there is a bug in that part of the code:
https://github.com/logstash-plugins/logstash-input-file/pull/85/files#diff-01d90892ef2cb12636c54f203f26518eR202

The right side of this statement will never be executed, since path_hex will never be nil.

sincedb_file = @sincedb_file_prefix + path_hex || ".sincedb_" + path_hex

Also I think there is another bug, I believe the default values for config option will be nil so doing
@sincedb_file_prefix + path_hex will raise this error NoMethodError: undefined method+' for nil:NilClass`

We can easily fix that by defining a default value of ".sincedb" for the @sincedb_file_prefix option.

WDTY @untergeek @phutchins?

[phutchins]

@ph great, I see the bugs and I'll take care of them as well as look into writing some tests around the new features. I'll raise the ConfigurationError as you mention as well.

Also, I can update the documentation to reflect the use cases as simply as possible.

[ph]

@phutchins Sound like a good plan +1 :)