fixes #342 add rfc option for syslogudp handler #343
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -16,6 +16,7 @@ | ||||||
| use Symfony\Component\Config\Definition\ConfigurationInterface; | ||||||
| use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; | ||||||
| use Monolog\Logger; | ||||||
| use Monolog\Handler\SyslogUdpHandler; | ||||||
|
|
||||||
| /** | ||||||
| * This class contains the configuration information for the bundle | ||||||
| @@ -162,7 +163,7 @@ | ||||||
| * - [level]: level name or int value, defaults to DEBUG | ||||||
| * - [bubble]: bool, defaults to true | ||||||
| * - [ident]: string, defaults to | ||||||
| * - [rfc]: RFC3164 or RFC5424, defaults to RFC5424 | ||||||
| * | ||||||
| * - swift_mailer: | ||||||
| * - from_email: optional if email_prototype is given | ||||||
| @@ -495,7 +496,10 @@ public function getConfigTreeBuilder() | ||||||
| ->scalarNode('title')->defaultNull()->end() // pushover | ||||||
| ->scalarNode('host')->defaultNull()->end() // syslogudp & hipchat | ||||||
| ->scalarNode('port')->defaultValue(514)->end() // syslogudp | ||||||
| ->enumNode('rfc') | ||||||
| ->values([SyslogUdpHandler::RFC5424, SyslogUdpHandler::RFC3164]) | ||||||
|
There was a problem hiding this comment. using raw value (cf snippet bellow) would allow people to use the RFC, without triggering exception for composer 1
Suggested change
There was a problem hiding this comment. #343 (comment) here @Tobion suggest to not to use plain values, which point of view should I adhere to There was a problem hiding this comment. It's always better to use constant, that's why @Tobion suggested to not use plain value when a constant exists. In this case (and only for the value There was a problem hiding this comment. Ok, i'll change it to scalarNode with default value |
||||||
| ->defaultValue(SyslogUdpHandler::RFC5424) | ||||||
| ->end() // syslogudp | ||||||
jderusse marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| ->arrayNode('publisher') | ||||||
| ->canBeUnset() | ||||||
| ->beforeNormalization() | ||||||
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.
What about RFC5424e?
If we add this, we probably need to be careful, not to rely on the constant as UdpHandler in Monolog 1 does not support it
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.
That's why i'm not sure about this RFC at this moment.
Should it be added at UdpHandler in Monolog 1? Or should it be added in some other place and imported alongside with SuslogUdpHandler for Monolog v1.x?
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 am not sure how actively maintained v1 is, i.e. whether new features are accepted and if this is considered a new feature.
Both UdpHandler v1 and v2 will just ignore invalid values as far as I can tell. I think we can just allow users to put in whatever int they want, maybe add a comment for where to find the proper values (e.g. "see UdpHandler RFC-constants") and not bother with the constants at all. Not sure what others think about this, though.
edit: I just looked at the handlers again and it seems specifying an invalid int will at least trigger a notice about an illegal array offset, but I think that could be easily fixed using an
array_key_exists()-check with a fallback.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.
Well, i think we should leave this story as is at this moment, we can add support for RFC5424e into another PR to speed up the acceptance of the current request