Skip to content

Conversation

@azhinu
Copy link
Contributor

@azhinu azhinu commented Jun 25, 2020

Added aggregation filter for easier log management. Also, readme file has been supplemented.

@whyscream
Copy link
Owner

@azhinu : thanks for your PR. I wasn't aware of the logstash aggregation plugin. When reading docs at https://www.elastic.co/guide/en/logstash/current/plugins-filters-aggregate.html#plugins-filters-aggregate-description, it seems that careful consideration and additional configuration is needed for the plugin to work.

Before considering a merge of this PR, the README would need to contain at least:

  • a note that using this file is optional
  • the setup needs additional configuration, and careful documentation reading.

Also some kind of test that show that this works as expected would be very nice.

@whyscream whyscream merged commit f14a1cb into whyscream:master Jul 2, 2020
@azhinu azhinu deleted the feature branch July 2, 2020 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants