-
Notifications
You must be signed in to change notification settings - Fork 101
Add optional argument forcing syslog parcing to one of both RFCs #1051
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
base: main
Are you sure you want to change the base?
Conversation
Allows giving priority to the older RFC 3164 over the newer RFC 5424. Defaults to the original behaviour of using the newer format over the older.
Ditch support for structured data inside RFC3164 messages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @itkovian, overall this looks good but please undo the formatting changes so that it is easier to review.
91535e8
to
dac266f
Compare
e670d8e
to
42a78be
Compare
f5ddf86
to
1d24034
Compare
1d24034
to
9e2d37b
Compare
@pront Formatting addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @itkovian, this looks good. Please add a changelog entry and address the failing checks.
@pront If I want fmt to succeed, the formatting will change (at least on my machine). |
strip-ansi-escapes = { version = "0.2", optional = true } | ||
snap = { version = "1", optional = true } | ||
syslog_loose = { version = "0.21", optional = true } | ||
syslog_loose = { git = "https://github.com/itkovian/syslog-loose", rev = "72bb2608ff942112d688626143b9bf8187a2f24f", optional = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you submit this as a PR to the original repo please. I'll look into it asap..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but it's not really doing what you want as mentioned in StephenWakely/syslog-loose#37 I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see. Yeah we can't really just remove structured data for 3164, that would be quite a breaking change. Despite not being strictly in the spec, there are systems such as rsyslog that push out 3164 messages with structured data. The joys of syslog!
I realise this issue has been outstanding for a while now, so I'll see if I can get some time to bump it up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be good with version 0.22 of syslog_loose
now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try and change this asap. Thank you!
Allows giving priority to the older RFC 3164 over the newer RFC 5424. Defaults to the original behaviour of using the newer format over the older.
This PR address #1043.