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

fixes #342 add rfc option for syslogudp handler #343

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Fix call to undefined method pushProcessor on handler that does not implement ProcessableHandlerInterface
* Use "use_locking" option with rotating file handler
* Add ability to specify custom Sentry hub service
* Add ability to specify RFC for SyslogUdpHandler

## 3.6.0 (2020-10-06)

Expand Down
5 changes: 4 additions & 1 deletion DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -162,6 +163,7 @@
* - [level]: level name or int value, defaults to DEBUG
* - [bubble]: bool, defaults to true
* - [ident]: string, defaults to
* - [rfc]: SyslogUdpHandler::RFC3164 (0), SyslogUdpHandler::RFC5424 (1) or 2 (for SyslogUdpHandler::RFC5424e, monolog 1 does not support it) defaults to SyslogUdpHandler::RFC5424
*
* - swift_mailer:
* - from_email: optional if email_prototype is given
Expand Down Expand Up @@ -408,7 +410,7 @@ public function getConfigTreeBuilder()
->booleanNode('use_locking')->defaultFalse()->end() // stream and rotating
->scalarNode('filename_format')->defaultValue('{filename}-{date}')->end() //rotating
->scalarNode('date_format')->defaultValue('Y-m-d')->end() //rotating
->scalarNode('ident')->defaultFalse()->end() // syslog and syslogudp
->scalarNode('ident')->defaultValue('php')->end() // syslog and syslogudp
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a bc break. The https://github.com/Seldaek/monolog/blob/main/src/Monolog/Handler/SyslogHandler.php does not have 'php' as default value. So it used an empty string (casted from false) before.

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

yes, but @Tobion is right here, this configuration is also used by both Syslog and SyslogUdp.
The change should keep compatibility with the 2 handlers.

Could you also please add a test to cover this case?

->scalarNode('logopts')->defaultValue(LOG_PID)->end() // syslog
->scalarNode('facility')->defaultValue('user')->end() // syslog
->scalarNode('max_files')->defaultValue(0)->end() // rotating
Expand Down Expand Up @@ -494,6 +496,7 @@ public function getConfigTreeBuilder()
->scalarNode('title')->defaultNull()->end() // pushover
->scalarNode('host')->defaultNull()->end() // syslogudp & hipchat
->scalarNode('port')->defaultValue(514)->end() // syslogudp
->scalarNode('rfc')->defaultValue(SyslogUdpHandler::RFC5424)->end() // syslogudp
->arrayNode('publisher')
->canBeUnset()
->beforeNormalization()
Expand Down
9 changes: 9 additions & 0 deletions DependencyInjection/MonologExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -519,9 +519,18 @@ private function buildHandler(ContainerBuilder $container, $name, array $handler
$handler['level'],
$handler['bubble'],
]);

if ($handler['ident']) {
$definition->addArgument($handler['ident']);
} else {
$handler['ident'] = 'php';
$definition->addArgument($handler['ident']);
}

if (isset($handler['rfc'])) {
jderusse marked this conversation as resolved.
Show resolved Hide resolved
$definition->addArgument($handler['rfc']);
}

break;

case 'swift_mailer':
Expand Down
1 change: 1 addition & 0 deletions Resources/config/schema/monolog-1.0.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
<xsd:attribute name="host" type="xsd:string" />
<xsd:attribute name="source" type="xsd:string" />
<xsd:attribute name="port" type="xsd:integer" />
<xsd:attribute name="rfc" type="xsd:integer" />
<xsd:attribute name="action-level" type="level" />
<xsd:attribute name="passthru-level" type="level" />
<xsd:attribute name="min-level" type="level" />
Expand Down
49 changes: 49 additions & 0 deletions Tests/DependencyInjection/ConfigurationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Bundle\MonologBundle\Tests\DependencyInjection;

use Monolog\Handler\SyslogUdpHandler;
use Monolog\Logger;
use PHPUnit\Framework\TestCase;
use Symfony\Bundle\MonologBundle\DependencyInjection\Configuration;
Expand Down Expand Up @@ -407,6 +408,54 @@ public function testWithRedisHandler()
$this->assertEquals('monolog_redis_test', $config['handlers']['redis']['redis']['key_name']);
}

public function testWithSyslogUdpHandler()
{
$configs = [
[
'handlers' => [
'syslogudp' => [
'type' => 'syslogudp',
'host' => '127.0.0.1',
'port' => 514,
'facility' => 'USER',
'level' => 'ERROR',
'ident' => null,
'rfc' => SyslogUdpHandler::RFC3164
]
]
]
];

$config = $this->process($configs);

$this->assertEquals('syslogudp', $config['handlers']['syslogudp']['type']);
$this->assertEquals('127.0.0.1', $config['handlers']['syslogudp']['host']);
$this->assertEquals(514, $config['handlers']['syslogudp']['port']);
$this->assertEquals(0, $config['handlers']['syslogudp']['rfc']);

$configs = [
[
'handlers' => [
'syslogudp' => [
'type' => 'syslogudp',
'host' => '127.0.0.1',
'port' => 514,
'facility' => 'USER',
'ident' => false,
'level' => 'ERROR'
]
]
]
];

$config = $this->process($configs);

$this->assertEquals('syslogudp', $config['handlers']['syslogudp']['type']);
$this->assertEquals('127.0.0.1', $config['handlers']['syslogudp']['host']);
$this->assertEquals(514, $config['handlers']['syslogudp']['port']);
$this->assertEquals(1, $config['handlers']['syslogudp']['rfc']);
}

/**
* @group legacy
*/
Expand Down
28 changes: 27 additions & 1 deletion Tests/DependencyInjection/MonologExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use InvalidArgumentException;
use Monolog\Handler\FingersCrossed\ErrorLevelActivationStrategy;
use Monolog\Handler\RollbarHandler;
use Monolog\Handler\SyslogUdpHandler;
use Monolog\Logger;
use Monolog\Processor\UidProcessor;
use Symfony\Bridge\Monolog\Processor\SwitchUserTokenProcessor;
Expand Down Expand Up @@ -225,7 +226,32 @@ public function testSyslogHandlerWithLogopts()

$handler = $container->getDefinition('monolog.handler.main');
$this->assertDICDefinitionClass($handler, 'Monolog\Handler\SyslogHandler');
$this->assertDICConstructorArguments($handler, [false, 'user', \Monolog\Logger::DEBUG, true, LOG_CONS]);
$this->assertDICConstructorArguments($handler, ['php', 'user', \Monolog\Logger::DEBUG, true, LOG_CONS]);
}

public function testSyslogHandlerForEmptyIdent()
{
$container = $this->getContainer(
[
[
'handlers' => [
'syslogudp' => [
'type' => 'syslogudp',
'host' => '127.0.0.1',
'port' => 514,
'facility' => 'USER',
'level' => 'ERROR',
'ident' => null,
'rfc' => SyslogUdpHandler::RFC5424,
]
]
]
]
);

$expectedArguments = ['127.0.0.1', 514, 'USER', true, 400, 'php', 1];
$definition = $container->getDefinition('monolog.handler.syslogudp');
$this->assertDICConstructorArguments($definition, $expectedArguments);
}

public function testRollbarHandlerCreatesNotifier()
Expand Down