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

Config conditionals ignored for flushed filter events #1431

Closed
dmlary opened this Issue Jun 9, 2014 · 4 comments

Comments

Projects
None yet
4 participants
@dmlary

dmlary commented Jun 9, 2014

While working on a custom filter for logstash version 1.4.1, I encountered an issue where events that are returned by the flush method are not verified against configuration conditionals.

I have the following configuration that calls my toy flush filter. The flush filter is a simple implementation that caches the first event, and releases it through the flush mechanism (see code below). After the filter, if impossible thing is found in the non_existent_field, add the tag should not exist. This tag should never be added to the event.

# Logstash configuration
input { stdin { type => syslog } }

filter {
  flush {
    add_tag => ['flushed']
  }
  if 'impossible thing' in [non_existent_field] {
    mutate {
      add_tag => ['should not exist']
    }
  }
}

output { stdout { codec => rubydebug } }

To execute this configuration and allow the flusher to execute, I run the following command:

# echo a message and hold STDIN open for a while so flush can be called
(echo "generic log message"; sleep 20 )| /opt/logstash/bin/logstash -f debug.conf -p .

The output from this configuration includes the should not exist tag set:

{
       "message" => "generic log message",
      "@version" => "1",
    "@timestamp" => "2014-06-09T16:40:48.277Z",
          "type" => "syslog",
          "host" => "ast03-deuce",
          "tags" => [
        [0] "flushed",
        [1] "should not exist"
    ]
}

The code for the flush filter is this:

require "logstash/filters/base"
require "logstash/namespace"

class LogStash::Filters::Flush < LogStash::Filters::Base
  config_name "flush"
  milestone 1

  def register
    @cached = nil
  end

  # Cache one event, pass all others through
  def filter(event)
    return if @cached

    event.cancel
    @cached = event
    return
  end

  # flush our cached event if we have one
  def flush
    return unless @cached
    event = @cached
    @cached = nil
    event.uncancel
    filter_matched(event)
    [event]
  end
end
@jordansissel

This comment has been minimized.

Show comment
Hide comment
@jordansissel

jordansissel Jun 9, 2014

Contributor

This will be fixed when #1260 is merged

Contributor

jordansissel commented Jun 9, 2014

This will be fixed when #1260 is merged

@colinsurprenant

This comment has been minimized.

Show comment
Hide comment
@colinsurprenant

colinsurprenant Aug 6, 2014

Contributor

I will add the fix for this in #1545

Contributor

colinsurprenant commented Aug 6, 2014

I will add the fix for this in #1545

@colinsurprenant

This comment has been minimized.

Show comment
Hide comment
@colinsurprenant

colinsurprenant Aug 8, 2014

Contributor

this is now fixed in #1545. @dmlary it'd be great if you could test it let us know how it work.

Contributor

colinsurprenant commented Aug 8, 2014

this is now fixed in #1545. @dmlary it'd be great if you could test it let us know how it work.

@dmlary

This comment has been minimized.

Show comment
Hide comment
@dmlary

dmlary Aug 9, 2014

With a change to Logstash::Filters::Flush#flush to support the new options argument, #1545 resolves this issue.

dmlary commented Aug 9, 2014

With a change to Logstash::Filters::Flush#flush to support the new options argument, #1545 resolves this issue.

jordansissel added a commit that referenced this issue Sep 5, 2014

Add recurse method for doing breadth-first traversal of the AST
This will be used by the filter flush compiler

Add generation of a flush lambda for each filter.

This allows filters to flush and have any generated events proceed
downward through the config as you would expect, respecting any
branches or future plugins.

Die on IOError which occurs when reading from a closed STDIN

Make filter_flusher invoke the new (and correct, I hope!) way to flush.

- On shutdown, we will also flush all filters.
- The flusher thread will terminate if we are shutting down.

Clarify the comment

Fix comment generation in the code to avoid newlines.

Add 'max_age' setting to multiline for flushing.

This setting chooses how long (in seconds) an event is considered to be
fresh before it will be automatically flushed.

This is useful for:
* slow log sources to get the 'last event' flushed,
* transaction-id-style events that have no obvious "end" event and also
  are mixed among other-id events in the same stream.

Also:
- Make filters have no teardown by default.
- Remove 'enable_flush' since it is not needed anymore; flush is always
  enabled.

refactor flush

new spool filter and specs, mainly for testing flushing

turn off unrelated test error for now

fix the flush logic, fix the flush compiles code to not include output section

synchronize cross-thread  access to @pending

refactor for performance and readability

synchronize cross-thread access to @spool

unused code

input:udp removed boggus ShutdownSignal handling, morphed loop do into while true, cosmetic reformat

use transcient events and not exceptions for in-flow pipeline signaling

inline flushing into  filterworker

removed now unnecessary flushing thread safety

fix conditionals bug for new events generated by filters & specs

spec for issue #793

performance tweeks

simplify filter handling of events and new_events

this removes unecessary duplication when treating the original event as
a special case (different from new_events generated by a filter).
Also, since @filter_func only outputs non-cancelled events, some checks
were also removed.

Move multiple filter specs to a filter_chains file

append events generated by a filter using unshift instead of insert

closes #793, closes #1429, closes #1431, closes #1548

@jordansissel jordansissel added this to the v1.5.0 milestone Nov 11, 2014

ajchalk added a commit to catalyst/logstash that referenced this issue Dec 2, 2014

Add recurse method for doing breadth-first traversal of the AST
This will be used by the filter flush compiler

Add generation of a flush lambda for each filter.

This allows filters to flush and have any generated events proceed
downward through the config as you would expect, respecting any
branches or future plugins.

Die on IOError which occurs when reading from a closed STDIN

Make filter_flusher invoke the new (and correct, I hope!) way to flush.

- On shutdown, we will also flush all filters.
- The flusher thread will terminate if we are shutting down.

Clarify the comment

Fix comment generation in the code to avoid newlines.

Add 'max_age' setting to multiline for flushing.

This setting chooses how long (in seconds) an event is considered to be
fresh before it will be automatically flushed.

This is useful for:
* slow log sources to get the 'last event' flushed,
* transaction-id-style events that have no obvious "end" event and also
  are mixed among other-id events in the same stream.

Also:
- Make filters have no teardown by default.
- Remove 'enable_flush' since it is not needed anymore; flush is always
  enabled.

refactor flush

new spool filter and specs, mainly for testing flushing

turn off unrelated test error for now

fix the flush logic, fix the flush compiles code to not include output section

synchronize cross-thread  access to @pending

refactor for performance and readability

synchronize cross-thread access to @spool

unused code

input:udp removed boggus ShutdownSignal handling, morphed loop do into while true, cosmetic reformat

use transcient events and not exceptions for in-flow pipeline signaling

inline flushing into  filterworker

removed now unnecessary flushing thread safety

fix conditionals bug for new events generated by filters & specs

spec for issue elastic#793

performance tweeks

simplify filter handling of events and new_events

this removes unecessary duplication when treating the original event as
a special case (different from new_events generated by a filter).
Also, since @filter_func only outputs non-cancelled events, some checks
were also removed.

Move multiple filter specs to a filter_chains file

append events generated by a filter using unshift instead of insert

closes elastic#793, closes elastic#1429, closes elastic#1431, closes elastic#1548

Conflicts:
	spec/conditionals/test.rb

@tbragin tbragin added the v1.5.0 label Jun 18, 2015

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