search

fluent-plugins-nursery / fluent-plugin-systemd

This is a fluentd input plugin. It reads logs from the systemd journal.
Apache License 2.0
153 stars 43 forks source link

Enhance Filtering Documentation #57

Closed Jitsusama closed 6 years ago

Jitsusama commented 6 years ago

This PR is related to issue #56.

It seeks to significantly expand upon the filters parameter documentation in order to hopefully make understanding this property much easier for the users of this excellent plugin.

Jitsusama commented 6 years ago

I think it's safe to say that the build failure wasn't due to the markdown changes I made. It looks like your linter is not very happy.

errm commented 6 years ago

Yeah, don't worry about the linter ... it is fixed on the 1.0 branch... we don't care here though ...

Jitsusama commented 6 years ago

Groovy. Let me know if you have any questions or corrections or even state how much you despise these changes :smile:.

Jitsusama commented 6 years ago

@errm; I'm still a newb with fluentd, so I was not aware of the name overload. I agree, removing ambiguity is ideal, and sticking with domain language is always a good thing. With that said, I think sticking with journalctl's matches would make the most sense.

Now, changing the name would be a breaking change, so that should probably be done in the 1.0.0 branch. You might even want to continue supporting both in 1.0.0 until you release 1.1.0 or something. Would you like for me to update my V1-Enhance-Filtering-Documentation branch to refer to it as matches instead, and leave the master branch alone (in regards to this particular change)?

errm commented 6 years ago

That sounds like a plan ...

We have the facility to deprecate params so it should be simple to change the name but keep the old one working too (for now)

Jitsusama commented 6 years ago

I've updated this PR with what I hope are the requested changes, and also updated PR #58 with the same, while also adding details on the filters deprecation/replacement with matches.

errm commented 6 years ago

Great... this is a nice addition, so I will merge it straight away ... I will need to fix the implementation first on the v1 branch :)

Thanks so much for helping out ... the docs were the only thing really left to fixup here so this is super useful..

Jitsusama commented 6 years ago

You are very welcome. And I must say, this has been one of my more pleasant PR experiences. Thanks for being so speedy in responding and clearly defining your desires!