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

Added option 'keep_start_event' #35

Merged
merged 1 commit into from
Jul 31, 2018

Conversation

chermenin
Copy link
Contributor

@chermenin chermenin commented Oct 13, 2017

The option 'keep_start_event' identifies the behavior in cases when several "start events" were received before an "end event". It's boolean value. If it's set to first (default value), the first "start event" will be used; if it's set to last, the last "start event" will be used.

@chermenin
Copy link
Contributor Author

Guys, what about to review and merge this PR?

@chermenin
Copy link
Contributor Author

@jsvd could you review this PR too?

@jblesener
Copy link

Would like to see this functionality as well.

# before an "end event" for a concrete ID. If it's set to `true` (default
# value), the first "start event" will be used; if it's set to `false`,
# the last "start event" will be used.
config :keep_first_start_event, :validate => :boolean, :required => false, :default => true
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather make the two possible behaviours of either keeping the first or the last start event by making this property a string:

config :keep_start_event, :validate => ['first', 'last'], :required => false, :default => 'first'`

Copy link
Member

Choose a reason for hiding this comment

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

Also, please also document this option in the docs/index.asciidoc file

@jsvd
Copy link
Member

jsvd commented Jul 30, 2018

@chermenin thanks for the changes! do you mind rebasing the PR against master and bumping the minor version + adding a changelog entry so I'll publish the plugin right away? you can find the guidelines on changelog entries in https://github.com/elastic/logstash/blob/master/CONTRIBUTING.md#when-should-i-add-an-entry-to-the-changelogmd

@chermenin chermenin force-pushed the start_events_behavior branch from 451750c to b1bac4e Compare July 31, 2018 06:48
@chermenin chermenin force-pushed the start_events_behavior branch from b1bac4e to df7f86b Compare July 31, 2018 06:56
@chermenin chermenin changed the title Added option 'keep_first_start_event' Added option 'keep_start_event' Jul 31, 2018
Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

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

LGTM

@jsvd jsvd merged commit 34ea976 into logstash-plugins:master Jul 31, 2018
@jsvd
Copy link
Member

jsvd commented Jul 31, 2018

Version 4.1.0 as been published, thanks for the contribution @chermenin !

@chermenin chermenin deleted the start_events_behavior branch July 31, 2018 12:24
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