Logstash: [DISCUSS] useless check in codec

[colinsurprenant]

The line, plain and s3-plain codecs perform this type of check in the encode method:

 if event.is_a? LogStash::Event and @format
   ...

The check for event.is_a? LogStash::Event is useless since this was to prevent encoding the legacy signaling events like the SHUTDOWN event. Now this signals are processed OOB from the processing queue.

I suggest we remove that check.

Any objections?

[andrewvc]

None on my end!

[colinsurprenant]

All above PRs ready for review.

[jsvd]

When was the inbound signaling removed? I see references to the Shutdown event in releases like logstash 5.2.0, are we taking care of this restriction? I.E. if you run bin/logstash-plugin update on logstash 5.2.0 will things break?

[andrewvc]

@jsvd IIRC the shutdown event isn't actually passed to plugins since LS 2.2 or so ( I forget the exact version ). IIRC the ng pipeline used a poison pill, but never passed it to plugins.

[colinsurprenant]

as @andrewvc mentioned, the concept of in-band signaling using the LogStash::SHUTDOWN event was removed starting in 5.0. We still do have the concept of a LogStash::SignalEvent::ShutdownEvent which LogStash::SHUTDOWN now point to but this is now used in out-of-band signalling using a separate signal queue and these are never pushed to the plugins.

[jsvd]

Thanks folks, I confirmed this too (also in 2.4.x).

[colinsurprenant]

all done.