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

modify FACILITY_LABELS,add security/authorization2 and clock2 #34

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TaeRoen
Copy link

@TaeRoen TaeRoen commented Dec 20, 2016

The FACILITY_LABELS list is not similar to rfc3164 and rfc5424‘s Facility list, it lose repeating option security/authorization and clock. Where i set my facility to local7,it will return me to local5. So, i add security/authorization2 and clock2,make the FACILITY_LABELS list conform to rfc. Thanks.

@karmi
Copy link

karmi commented Dec 20, 2016

Hi @TaeRoen, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

@TaeRoen
Copy link
Author

TaeRoen commented Dec 20, 2016 via email

@breml
Copy link
Collaborator

breml commented Dec 20, 2016

Hi @TaeRoen, good catch, thank you.

I asked my self if, we are going to change the labels anyway, we should harmonize the used labes in this plugin with the labels used in logstash-input-syslog. This would inpact daemon, which is called system in the input plugin as well as the two which are added in this PR, because these do not have distinct names for security/authorization and clock. Other than that, there are 3 labels, for which only the upper-/lower-case differs (UUCP, FTP, NTP).

In my opinion these labels should be harmonized, because I think it should be possible to use the logstash-input-syslog together with logstash-output-syslog without the need of additional filters, which adjust the labels. What do you think?

CC: @jordansissel @suyograo

@breml
Copy link
Collaborator

breml commented Dec 20, 2016

@TaeRoen
Copy link
Author

TaeRoen commented Dec 20, 2016

@breml Thanks

I just read the logstash-input-syslog‘s code,I agree with your opinion.It should be security/authorization and clock rather than security/authorization2 and clock2.

@breml
Copy link
Collaborator

breml commented Feb 16, 2017

@TaeRoen sorry for let you down on this one for such a long time. I just yesterday opened a the bug logstash-plugins/logstash-input-syslog#37 on logstash-input-plugin to get more attention on this one.
Today I again did some thinking and one way of implementing this one would be to make the facilities and the severities configurable the same was as it is done in logstash-input-plugin (see https://github.com/logstash-plugins/logstash-input-syslog/blob/master/lib/logstash/inputs/syslog.rb#L47). This way we could manage the backwards compatibility while still allow the user to set these values to what ever he likes.

@TaeRoen I hope you don't mind if I reopen this PR until a decision is take, how to go forward with this one? The problem you have pointed out is still valid and therefore should be resolved (even if the way, how we want to handle it, is not yet clear).
@suyograo do you mind to have a look at this PR?

@breml breml reopened this Feb 16, 2017
@jsvd jsvd closed this Feb 27, 2017
@jsvd jsvd reopened this Feb 27, 2017
@jsvd jsvd closed this Apr 4, 2017
@jsvd jsvd reopened this Apr 4, 2017
@avinson
Copy link

avinson commented Oct 19, 2017

@jsvd @jordansissel @breml @karmi

sorry to be a bother but could we merge this please? it's only a two line change and would fix a major issue.. i don't understand holding this up over an unsigned commit. thanks!

@TaeRoen
Copy link
Author

TaeRoen commented Oct 19, 2017

@avinson
sorry to signed CLA before create the pull request,the following URL is my signed Individual Contributor License Agreement.
Individual Contributor License Agreement

@cla-checker-service
Copy link

❌ Author of the following commits did not sign a Contributor Agreement:
e7470f8

Please, read and sign the above mentioned agreement if you want to contribute to this project

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.

5 participants