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

enhancement(syslog source): Add drop_invalid option #1455

Closed
wants to merge 4 commits into from
Closed

enhancement(syslog source): Add drop_invalid option #1455

wants to merge 4 commits into from

Conversation

StephenWakely
Copy link
Contributor

Fixes #746

I've added a new option to the syslog source - drop_invalid, default true.

When this option is false, if the message fails to parse, it doesn't drop the event - instead it creates an event with the message field populated, timestamp set to the current time.

Hopefully this pull request will work better. Many apologies for messing up that last one!

Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
@StephenWakely
Copy link
Contributor Author

I think the test-stable-kubernetes test failure is it running out of memory. I was getting similar errors on my laptop until I increased the swap space.

@binarylogic
Copy link
Contributor

Thanks @FungusHumungus, we'll review this week and get it merged.

@StephenWakely
Copy link
Contributor Author

Thinking about it, you may want to shelve this one in favor of issue #1454.

Technically with RFC 3164 there is no such thing as an invalid message. If the message can't be parsed in some format, then the whole message becomes the message text without any additional fields. So there will not be any invalid messages that can be dropped.

Potentially one may consider, for example, a message that doesn't have a valid facility to be invalid, but I imagine decisions like this would differ from user to user and could be handled by a transform instead.

@binarylogic
Copy link
Contributor

Technically with RFC 3164 there is no such thing as an invalid message. If the message can't be parsed in some format, then the whole message becomes the message text without any additional fields.

Yeah, that's the issue with RFC 3164. I'd be ok with that as long as we take care to default the timestamp and host fields since those are marked as required outputs. I'm curious what @lukesteensen thinks. In general, I'm in favor of making a best effort to parse common syslog formats versus following the spec exactly.

Copy link
Member

@lukesteensen lukesteensen left a comment

Choose a reason for hiding this comment

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

Thanks for this @FungusHumungus! Looks good to me.

@lukesteensen
Copy link
Member

Oh, whoops, forgot which PR I was looking at. I agree with the strategy of shelving this in favor of introducing a more relaxed parser that will not reject messages as invalid.

I'll go ahead and close this for now, but we can reopen if we decide against that plan.

@binarylogic
Copy link
Contributor

Thanks @FungusHumungus, apologies for the misdirection. I agree that the new parsing strategy would be a better approach. Appreciate your thought around this.

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.

Add drop_invalid option to syslog source
3 participants